feat(cli): parametrized staging release environment with dedicated release tag and package-manager coverage - #90
feat(cli): parametrized staging release environment with dedicated release tag and package-manager coverage#90tkkhq wants to merge 7 commits into
Conversation
…oduction default and staging opt-in
…staging bypasses redirect
There was a problem hiding this comment.
Pull request overview
Adds a dedicated staging release channel while preserving production defaults.
Changes:
- Adds staging-tag build and release workflows.
- Adds staging installers and Homebrew packaging.
- Makes version reporting and upgrades environment-aware.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/publish-cli.yml |
Publishes staging releases. |
README.md |
Links staging installation guidance. |
docs/installation.md |
Documents staging installation and upgrades. |
internal/cmd/root/root.go |
Reports compiled environment. |
internal/cmd/root/root_test.go |
Updates version-output tests. |
internal/config/config.go |
Detects the compiled environment. |
internal/config/environment_label_test.go |
Tests environment detection. |
internal/update/install_method.go |
Adds Homebrew staging detection. |
internal/update/update.go |
Prevents cross-channel staging upgrades. |
internal/update/update_test.go |
Tests staging upgrade behavior. |
packaging/homebrew/volcano-staging.rb |
Defines the staging formula template. |
scripts/ci/resolve-cli-build-env.sh |
Resolves environment-specific build values. |
scripts/install-volcano.sh |
Installs and verifies staging binaries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| STABLE_SEMVER_RE='^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$' | ||
| NIGHTLY_SEMVER_RE='^v0\.0\.[0-9]+-nightly\.[0-9]{8}\.[0-9]+$' | ||
| if [[ ! "$REF_NAME" =~ $STABLE_SEMVER_RE ]] && [[ ! "$REF_NAME" =~ $NIGHTLY_SEMVER_RE ]]; then | ||
| if [[ "$REF_NAME" =~ $STAGING_SEMVER_RE ]]; then |
| tags: | ||
| - "v*.*.*" | ||
| - "!v*.*.*-nightly.*" | ||
| - "staging-v*.*.*" |
| on_macos do | ||
| on_arm do | ||
| url "https://github.com/Kong/volcano-cli/releases/download/staging/volcano-macos-arm64" | ||
| sha256 "0000000000000000000000000000000000000000000000000000000000000000" |
| Staging builds stay on the staging channel: a Homebrew `volcano-staging` install | ||
| upgrades with `brew upgrade volcano-staging`, and `volcano upgrade` on any other | ||
| staging build re-runs the staging installer rather than replacing it with a | ||
| production `latest` binary. |
…on the shared package
tkkhq
left a comment
There was a problem hiding this comment.
Code Review — PR #90: feat(cli): parametrized staging release environment with dedicated release tag and package-manager coverage
Verification: No user-level verification baselines are configured, so sandboxed baseline execution was disabled (verification executes unsandboxed PR code and is opt-in only). Validation was source-grounded: read the full .github/workflows/publish-cli.yml, confirmed resolve-release declares the release_tag/release_title outputs consumed by the new publish-npm-staging job, and queried the repository tag rulesets via gh api to confirm the production release tags ruleset covers only refs/tags/v*.*.* and does not cover staging-v*. Did not run go build/go test (no baseline profile); the working tree was left untouched.
Overview
This PR promotes "staging" to a first-class, parametrized release channel instead of the temporary compiled-default swap in #68. Production stays the default and its build output is unchanged. A new staging-v<X.Y.Z> tag trigger builds against the staging environment (api.staging.volcano.dev), publishes an immutable staging-vX.Y.Z prerelease plus a moving staging alias (both --latest=false), and a new publish-npm-staging job (gated on environment == 'staging') publishes the shared @volcano.dev/cli package under the staging dist-tag only.
Supporting changes keep a single volcano command per channel: scripts/ci/resolve-cli-build-env.sh is parametrized by an explicit ENVIRONMENT with a ref-vs-ENVIRONMENT mismatch guard; internal/config gains CompiledDefaultAPIURL()/CompiledEnvironmentLabel(); volcano --version reports the baked environment; volcano upgrade becomes channel-sticky (@staging, brew upgrade volcano-staging, or a redirect to the staging installer); scripts/install-volcano.sh gains a cosign-verified staging selector; scripts/npm/download.js maps X.Y.Z-staging back to staging-vX.Y.Z; and a prebuilt volcano-staging.rb Homebrew formula is added. Docs and focused tests accompany the code.
Strengths
- Production build path is preserved: the stable/nightly branches keep identical URLs/client-id resolution, and
ENVIRONMENTdefaults toproductionwith a hard ref-vs-ENVIRONMENTmismatch guard so astaging-v*tag can never bake production config (or vice versa). - Strong channel guard rails for legitimate runs: staging publishes are
--prerelease --latest=false,publish-npmstays gated onprerelease == 'false', andpublish-npm-stagingis gated onenvironment == 'staging'and always uses--tag staging, so a legitimate staging build can never become npmlatestor the GitHublatestrelease. - The npm-version→release-tag mapping round-trips correctly:
resolve-cli-build-env/npm_versionstampX.Y.Z-staginganddownload.js's^(\d+\.\d+\.\d+)-staging(?:\.\d+)?$maps it back to the immutablestaging-vX.Y.Zrelease. - Upgrade routing is cleanly channel-sticky via the
compiledEnvironmentLabelpackage-level indirection, with focused regression tests for npm@staging,brew upgrade volcano-staging, and the script-install redirect (the redirect test deliberately sets no HTTPClient so any network attempt fails the test). - Asset-existence verification (ranged GET) runs before the npm publish, and the staging installer verifies signatures against a
staging-v<x.y.z>identity regex rather than a fixed identity. Documentation covers every staging install/upgrade path.
Findings — 2 total (2 inline, 0 summary-only)
All 2 findings are attached inline below this review.
Correctness / Security / Performance
- Correctness: Env-label derivation, the npm
X.Y.Z-staging→staging-vX.Y.Zmapping, upgrade routing, and therelease_tagoutput wiring all check out; the only correctness issue is the concurrent staging-alias race (P2). - Security: The new
staging-v*publish trigger runs on a tag namespace not covered by the admin-gatedproduction release tagsruleset, widening the privileged-publish/OIDC attack surface to any write-access account (P1); legitimate runs correctly cannot become npm/GitHublatest. - Performance: No performance/resource regressions; the pre-publish asset-verification curl loop is bounded (5 targets + checksums, capped retries).
Verdict
Suggested verdict: request_changes
The Go and shell logic is sound and well-tested, but the new staging-v* release trigger operates on an unprotected tag namespace while equivalent production tags are locked to admins and the release integration, giving any write-access account access to the workflow's npm-OIDC/contents: write publish surface (P1, blocking). A separate P2 race on the shared staging alias should also be addressed. Extend the tag ruleset to cover staging-v* before merging.
Confidence: 0.82
| tags: | ||
| - "v*.*.*" | ||
| - "!v*.*.*-nightly.*" | ||
| - "staging-v*.*.*" |
There was a problem hiding this comment.
[P1] Protect the staging-v tag namespace before adding the publish trigger*
.github/workflows/publish-cli.yml:10 adds - "staging-v*.*.*" to the tag triggers of a workflow whose jobs hold contents: write and id-token: write (npm OIDC trusted publishing and GitHub release creation).
The repository's only tag-scoped ruleset — production release tags (verified via gh api repos/Kong/volcano-cli/rulesets/17333957) — includes refs/tags/v*.*.* (excluding nightly) and restricts create/update/delete to the Admin role and the Release Please integration. staging-v* matches no tag ruleset, so any account with Write access can push a staging-vX.Y.Z tag pointing at an arbitrary crafted commit.
Because a tag-triggered run executes the workflow file at the tagged commit, that commit can carry a modified publish-cli.yml (the validate-release-ref ancestry check is itself part of the attacker-controlled workflow and can be removed). Such a run can mint the npm OIDC token and publish arbitrary code to @volcano.dev/cli — under @staging by default, or @latest if the attacker drops --tag staging — and create GitHub releases via contents: write, forging cosign signatures accepted by the new staging identity regex.
Condition to trigger: an actor with Write access (or a compromised write-scoped token/integration). This is the same privileged publish surface production tags already have, but the PR grants it to an unprotected namespace, removing the admin-only gate. Extend the tag ruleset to cover staging-v* (or otherwise restrict who can push these tags) before merging the trigger.
| git tag -f staging "$GITHUB_SHA" | ||
| git push origin refs/tags/staging --force | ||
| publish_release staging "$RELEASE_TITLE" "$STAGING_ALIAS_NOTES" --prerelease --latest=false |
There was a problem hiding this comment.
[P2] Serialize staging-alias updates across concurrent staging releases
.github/workflows/publish-cli.yml:386-388 force-moves the shared staging tag and republishes the staging alias release on every staging build:
git tag -f staging "$GITHUB_SHA"
git push origin refs/tags/staging --force
publish_release staging ... --prerelease --latest=false
The workflow concurrency.group is publish-cli-${{ github.ref_name }} with cancel-in-progress: false, so two different staging-vX.Y.Z tags land in separate groups and can run concurrently. Their --force tag pushes and --clobber asset uploads can interleave, leaving the moving staging release (consumed by the curl staging selector and the volcano-staging Homebrew formula) pointing at mixed assets/checksums or an older build. The immutable staging-vX.Y.Z releases are unaffected, so the corruption is invisible to the per-tag asset verification.
Unlike the existing nightly alias — which only ever runs from main under the single publish-cli-main group and therefore serializes — staging tags do not serialize with each other. Condition: two staging releases cut close together. Consider a fixed concurrency group (e.g. publish-cli-staging-alias) for the alias update, or otherwise guard the moving-tag move.
Tracking
Summary
volcanocommand and the single@volcano.dev/clipackage across every channel: you deliberately choose production (@latest/volcanoformula/plain installer) or staging (@staging/volcano-stagingformula/VOLCANO_VERSION=staging), and a staging install replaces a production one (useVOLCANO_API_URLat runtime if you need both at once);volcano --versionreports the baked environment + API URLscripts/ci/resolve-cli-build-env.shby an explicitENVIRONMENT(production default, staging opt-in) with a ref-vs-ENVIRONMENTmismatch guard so astaging-v*tag can never bake production config (or vice versa), plus a hard failure whenVOLCANO_FIRST_PARTY_DEVICE_CLIENT_ID_STAGINGis unsetstaging-v<X.Y.Z>tag trigger to.github/workflows/publish-cli.ymlthat builds against staging and publishes an immutablestaging-vX.Y.Zrelease plus a movingstagingalias, both prerelease/--latest=false; production GA (stable taglatest) is unchanged and nightly stays off npmstagingdist-tag on the shared package via a newpublish-npm-stagingjob:resolve-releasestampsX.Y.Z-staging, andscripts/npm/download.jsmaps that back to the immutablestaging-vX.Y.Zrelease sonpm i -g @volcano.dev/cli@stagingresolves the right binary;publish-npm(latest) stays gated onprerelease == 'false'so staging never becomes npm lateststagingselector toscripts/install-volcano.sh(installsvolcano, cosign-verified against the@refs/tags/staging-v<x.y.z>identity) and a prebuilt-binarypackaging/homebrew/volcano-staging.rbformula that installsvolcanoandconflicts_withthe production formulavolcano upgradechannel-sticky: npm/pnpm/yarn/bun staging builds upgrade with@staging, Homebrew-staging withbrew upgrade volcano-staging, and install-script staging builds are redirected to the staging installer instead of self-replacing with a productionlatestbinaryVerification
go build ./...go vet ./...go test ./...bash -n scripts/ci/resolve-cli-build-env.sh scripts/install-volcano.shpython3 -c "import yaml; yaml.safe_load(open('.github/workflows/publish-cli.yml'))"node -e "require('./scripts/npm/download.js')"and verified theX.Y.Z-staging[.N]->staging-vX.Y.Zrelease-tag mapping round-trips with the CI version stampruby -c packaging/homebrew/volcano-staging.rbresolve-cli-build-env.shfor production (stable tag + main; byte-for-byte unchanged), the staging tag, both mismatch guards, and the missing-staging-var failureresolve-release+ publish dispatch branch selection for staging/stable/main refs@staging, brewvolcano-staging, and the install-script redirect