test(release): complete credential-free brand pilot - #436
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f4172178c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Important
The new release-environment-preflight gate does not actually block the desktop signing path — signing-inputs being force-skipped reports skipped, which the desktop job already accepts. Two other issues below are worth a look before this stops being a pilot branch.
Reviewed changes — initial review of the single commit on xuan/code-561 against base xuan/code-559. No product code is touched; both files are CI.
- New
credential-free-validationjob — unconditional (needs: prepare, noif:), no secrets and noenvironment:. Checksapps/desktop/e2e/fixtures/pilot-e2e-v1.jsonagainst a pinned sha256, runs the Electron config canary under Xvfb, then production Hermes export plus Android and iOSexpo prebuild, and uploadsruntime-validation.jsonrecordingdeploymentIdentity = {kind:"local-static-origin", providerDeploymentId:null}. - New
release-environment-preflightgate —if: inputs.sign,environment: release; reads thereleaseenvironment throughgh apiand fails unless it carriesrequired_reviewersand a non-nulldeployment_branch_policy.signing-inputsis rewired to need it, and workflowpermissionsgainsdeployments: read. - New per-brand
desktop-validationjob —inputs.build && !inputs.sign; downloadsbrand-render-<brand>, moves the rendered bundle aside, runs the canary, asserts the canary did not recreateapps/desktop/generated, restores it, then binds a fixedvalidation.desktop.jsonto the brand's bundle and identity viarelease-artifact-cli.mts. - Three source-contract tests — slice the workflow YAML between job keys with
indexOfand assert raw substrings, pinning the credential-free job's independence, the preflight's fail-closed shape, and the desktop job's evidence binding.
Verified along the way and not problems: the pinned fixture hash matches the file on disk; smoke:export and expo prebuild work without a rendered mobile config because apps/mobile/app.config.ts:24-36 only throws on a partial generated/ set (and ci.yml:256 already runs it that way); pnpm --dir apps/mobile exec is equivalent to mobile-validation's working-directory; and bare xvfb-run matches existing CI usage.
ℹ️ Per-brand desktop-validation fans out over a brand-independent workload
Moving apps/desktop/generated aside is forced, not sloppy — loadGeneratedConfigBundle throws when both a generated bundle and MAIN_VITE_CONFIG_BOOTSTRAP are present (apps/desktop/scripts/config-bundle.mts:101-106), so the canary cannot run with the rendered bundle in place. The consequence is that every matrix leg executes a byte-identical fixture run, and the evidence step then binds that brand-independent result to the brand's bundle and identity. mobile-validation is the opposite: it keeps the rendered config in place and validates it.
Technical details
# Per-brand desktop validation exercises no per-brand input
## Affected sites
- `.github/workflows/release-brand-matrix.yml:403-405` — `strategy.matrix` fans out over every brand.
- `.github/workflows/release-brand-matrix.yml:433` — `mv apps/desktop/generated "$rendered"` removes the only brand-specific input before the canary runs.
- `.github/workflows/release-brand-matrix.yml:447-460` — a fixed `validation.desktop.json` literal is bound to `apps/desktop/generated/{config-build-bundle,brand-identity}.json` after the bundle is restored.
## Required outcome
- Either the per-brand fan-out exercises something brand-specific, or the Electron canary runs once and the per-brand step is limited to the provenance binding.
- A reader of `brand-validation-desktop-<brand>` should not be able to mistake it for evidence that `<brand>`'s rendered config was exercised in Electron.
## Open questions for the human
- Is the fan-out deliberate (N Electron rebuilds for identical work, to keep the artifact shape symmetric with `mobile-validation`), or an oversight?
- Should the desktop evidence JSON record that the rendered bundle was deliberately withheld, alongside the `pilotFixtureSha256` it already records?ℹ️ Nitpicks
credential-free-validationcarries noif:, so it also runs on plan-only dispatches (matrix_jsonwithbuild=false) and on sign/upload runs, where it duplicatesdesktop-validationandmobile-validation. Validating a matrix JSON now costs a full Electron build plus two Expo prebuilds — intentional?expect(validation).not.toContain('environment: release')is raw-text, so the block formenvironment:\n name: releaseslips past it.
Claude Opus | 𝕏
There was a problem hiding this comment.
Important
The committed pilot matrix cannot drive a build=true run — brand-matrix.cjs:232 rejects it — so none of the inputs.build-gated machinery this PR adds or hardens is reachable with it, and the new test that "pins" the matrix never exercises that path. Separately, the environment repoint has no way back to release.
Reviewed changes — delta between the prior pullfrog review (9f41721) and a7f15df, which reshapes the pilot from a single unconditional job into a per-target matrix and moves the whole workflow onto an isolated environment.
- Matrixed
credential-free-validation— now fans out overfromJSON(needs.prepare.outputs.targets)(brands ×desktop/ios/android), with platform-conditional steps, a per-legruntimefield, andcredential-free-<brand>-<platform>artifact names. - Rewrote
release-environment-preflight—if: inputs.sign→if: inputs.build, dropped its ownenvironment:, swappedgithub.tokenforsecrets.PILOT_ENVIRONMENT_ADMIN_TOKENwith an explicit empty-token failure, retargetedrelease→pilot-nonproduction, and tightened the jq assertion to require a non-emptyreviewersarray plus a concrete branch policy. - Gated
render-inputson the preflight — this is what actually closes the desktop signing bypass from the last review (thread resolved). - Repointed all eight
environment: releasesites inrelease-brand-matrix.ymltopilot-nonproduction, and threaded a newrelease_environmentworkflow_callinput (defaultrelease) throughbuild-desktop.ymlandbuild-mobile.yml. - Added
.github/release/brand-matrices/code-561-pilot.json— two canary brands, all three platforms,distribution.desktop/distribution.mobilebothnull. - Added three contract tests — the pilot matrix plan, the reusable-workflow environment threading, and the rewritten preflight shape.
Traced and confirmed not problems: the preflight gate is now airtight (a failed preflight skips render-inputs → render, so desktop's needs.render.result == 'success' blocks it and the 'skipped' escape hatch on signing-inputs no longer matters; build=false, sign=true is still killed in prepare); '${{ matrix.platform }}' inside run: blocks is not a shell-injection vector because platform comes from the fixed PLATFORMS const in brand-matrix.cjs:4; and release-desktop.yml calls build-desktop.yml without release_environment, so the production desktop train still resolves to release.
⚠️ The pilot environment repoint has no way back to release
All eight environment: release sites in release-brand-matrix.yml are now hardcoded to pilot-nonproduction — including publish-desktop (R2 sync) and publish-mobile (TestFlight / Play internal) — and both reusable-workflow call sites pass release_environment: pilot-nonproduction as a literal. Nothing parameterizes it, and brand-matrix.test.mjs:319 asserts expect(workflow).not.toContain('environment: release'), so restoring the production environment is now a test failure rather than a config change. The release_environment input added to build-desktop.yml / build-mobile.yml is exactly the right seam, but this workflow never uses it as a seam.
Technical details
# `release-brand-matrix.yml` is pinned to the pilot environment with no exit
## Affected sites
- `.github/workflows/release-brand-matrix.yml:235,252,289,569,649,725` — `environment: pilot-nonproduction` literals, covering render, signing-input validation, publish preflight, store submission, and R2 upload.
- `.github/workflows/release-brand-matrix.yml:418,557` — `release_environment: pilot-nonproduction` passed as a literal to `build-desktop.yml` / `build-mobile.yml`.
- `.github/workflows/release-brand-matrix.yml:219` — the preflight only ever inspects `pilot-nonproduction`.
- `.github/scripts/brand-matrix.test.mjs:319` — `expect(workflow).not.toContain('environment: release')` locks the repoint in.
## Required outcome
- Either the workflow can select its environment (so a real brand release is possible without editing YAML and a test), or the PR states explicitly that this file is pilot-only and names the commit/PR that reverts it.
- Whatever the answer, `pilot-nonproduction` must own every credential the eight repointed jobs read (`CONFIG_PUBLISHER_TOKEN`, the Apple/Azure signing set, `EXPO_TOKEN`, and the per-brand `*_R2_*` secrets) or those jobs fail at the first secret read rather than at a gate.
## Open questions for the human
- Is this a permanent split (brand matrix ships through a nonproduction environment by design) or a temporary pilot state that reverts before this reaches `master`?
- If it is meant to be selectable, should it be a `workflow_dispatch` input, or derived from `inputs.upload` so only real uploads demand the production environment?ℹ️ Nitpicks
credential-free-validationstill carries noif:, and the round-2 fan-out multiplies the cost noted last review: a plan-onlymatrix_jsondispatch now spendsbrands × 3heavyweight jobs (6 for the pilot matrix), including two byte-identical Electron runs and four fullpnpm install+ Expo cycles.build-desktop.yml:139's comment still reads "Signing is gated by thereleaseenvironment (secrets + tag policy)", but line 140 now resolves to whateverrelease_environmentthe caller passes.
Claude Opus | 𝕏
There was a problem hiding this comment.
Important
e37d4a1 reverts round 2's isolated-pilot environment, which moves this PR's new fail-closed preflight onto the shared production release environment. Provisioning release the way the gate's own error message suggests would block the tag-triggered desktop release train, and the PR description still describes the pilot-nonproduction design that no longer exists anywhere in the tree.
Reviewed changes — delta between the prior pullfrog review (a7f15df) and e37d4a1, which undoes the environment repoint flagged last round.
- Reverted all six
environment: releasesites inrelease-brand-matrix.ymlfrompilot-nonproductionback torelease, and changed both reusable-workflow call sites to passrelease_environment: release(lines 418, 557). - Retargeted
release-environment-preflightat thereleaseenvironment — renamed its token tosecrets.RELEASE_ENVIRONMENT_ADMIN_TOKENand updated thegh apipath, the jq.nameassertion, the temp-file name, and both error messages. - Documented
RELEASE_ENVIRONMENT_ADMIN_TOKENindocs/ENVIRONMENT.md, including why it cannot be stored inside the environment it inspects. - Rewrote the environment assertions in
brand-matrix.test.mjs—not.toContain('environment: release')gave way to occurrence counts (sixenvironment: release, tworelease_environment: release) alongside the existing per-job identity checks onrender-inputsandsigning-inputs.
Traced and confirmed not problems: no reference to pilot-nonproduction or PILOT_ENVIRONMENT_ADMIN_TOKEN survives anywhere in the tree; the preflight gate stays airtight through the revert (a failed preflight skips render-inputs → render, so desktop's needs.render.result == 'success' blocks the signing path, and the signing-inputs == 'skipped' branch is only reachable on a legitimately unsigned run where build-desktop.yml:140 resolves environment to ''); and both new occurrence counts match the files exactly. The round-1 thread about deployments: read causing a 403 is resolved — the PAT switch means the workflow permissions: block no longer governs that gh api call.
⚠️ The pilot's fail-closed gate now applies to the production release environment
release-environment-preflight runs on if: inputs.build and hard-fails unless the repo carries RELEASE_ENVIRONMENT_ADMIN_TOKEN and the release environment has a non-empty required-reviewer rule plus a deployment branch policy. Last round that gate pointed at an environment nobody used; now it points at release, which release-desktop.yml, build-desktop.yml and publish-mobile-metadata.yml all share. The PR body says release is currently unprotected, so satisfying this gate means changing production release configuration — and adding required reviewers to release inserts a manual approval step into the tag-triggered desktop release train, which today runs unattended after release-please pushes the tag.
Technical details
# The credential-free pilot now gates the shared production `release` environment
## Affected sites
- `.github/workflows/release-brand-matrix.yml:203-228` — the preflight now inspects `release` and fails closed for every `build=true` dispatch.
- `.github/workflows/release-brand-matrix.yml:233,250` — `render-inputs` and `signing-inputs` both `needs: [prepare, release-environment-preflight]`, so nothing downstream runs until the environment is provisioned.
- `.github/workflows/release-desktop.yml:49` and `.github/workflows/build-desktop.yml:95,140` — the tag-triggered desktop train enters the same `release` environment.
- `.github/workflows/publish-mobile-metadata.yml:21` — also `environment: release`.
- PR description, "Live gate" section — still asserts the gate targets `pilot-nonproduction` and that `pilot-nonproduction` does not exist. Neither string appears in the tree after `e37d4a1`.
## Required outcome
- The PR description reflects what the branch actually does: the gate inspects `release`, and the new prerequisite is a repository-level `RELEASE_ENVIRONMENT_ADMIN_TOKEN` plus protection rules on `release`.
- The consequence for `release-desktop.yml` is stated and accepted before merge — either the desktop train is expected to wait on reviewer approval from now on, or the required-reviewer rule needs a scope that excludes it.
- `RELEASE_ENVIRONMENT_ADMIN_TOKEN` exists before this lands, otherwise the first `build=true` dispatch after merge fails at the preflight with `RELEASE_ENVIRONMENT_ADMIN_TOKEN is required to inspect environment protection`.
## Open questions for the human
- Was gating the shared `release` environment the intent of the revert, or was the intent only to stop hardcoding a nonexistent environment (in which case the preflight could target whatever `release_environment` the run resolves to)?
- Should `release-desktop.yml` be exempt from the required-reviewer rule, given it is the automated post-tag train described in `docs/RELEASE.md`?ℹ️ Nitpicks
- The
release_environmentseam is now inert: both call sites inrelease-brand-matrix.ymlpass the literal defaultrelease,release-desktop.ymlomits the input entirely, and noworkflow_dispatchinput feeds it — sobuild-desktop.ymlandbuild-mobile.ymlcarry aworkflow_callinput no caller varies, plus three test assertions pinning plumbing nothing exercises. Either wire it to a dispatch input or drop it. expect(workflow.split(' environment: release')).toHaveLength(7)(brand-matrix.test.mjs:317) pins a bare occurrence count, so moving the environment off one job and onto another stays green.render-inputsandsigning-inputsare additionally checked by identity, butrender,publish-preflight,publish-mobileandpublish-desktopare covered only by the total.
Claude Opus | 𝕏
## Summary - render publisher code from protected `CONFIG_PUBLISHER_REPO@publisherGitSha` and structural data from protected `CONFIG_SOURCE_REPO@sourceGitSha` - require different canonical `arcboxlabs/repository` identities and independently enforce reviewed-`master` ancestry, redirects, exact checkout identity, symlink/path contracts, role contracts, and canonical schema equality - mint two separate short-lived installation tokens from `BOT_APP_ID` / `BOT_APP_PRIVATE_KEY`, each restricted to one selected repository with Contents read only - confine the App private key to pinned trusted token-mint steps and discard both checkout tokens before selected publisher code executes - keep Environment metadata inspection on built-in `GITHUB_TOKEN` with `actions: read`; no config PAT or Environment-admin token contract exists - pin the nonproduction pilot evidence to publisher `986d9f21403df53bc932f511eb1b5f0bb634d48d` and source `a1ed4d666721c3aed0d563aaea42fce8b5f945b5` - retain the CI checkout that proves org App access to both exact private-repository pins This is a non-draft follow-up stacked on #436. Acme/Zenith remain nonproduction examples under `examples/acme-zenith`; no production data or destinations are added. ## Protected `release` Environment contract - `CONFIG_PUBLISHER_REPO=arcboxlabs/linkcodehq` - `CONFIG_SOURCE_REPO=arcboxlabs/linkcode-config` - organization secrets `BOT_APP_ID` and `BOT_APP_PRIVATE_KEY` `CONFIG_PUBLISHER_TOKEN`, `CONFIG_SOURCE_TOKEN`, and `RELEASE_ENVIRONMENT_ADMIN_TOKEN` are intentionally absent. ## Local validation - `pnpm check:ci` (format, lint with existing warnings, typecheck) - `pnpm test` - focused release workflow tests (21 passed) - parsed all four changed Actions YAML files
|
Your Claude subscription has hit its usage limit. It resets at Aug 15, 9pm (UTC). Re-trigger Pullfrog after the reset, or add an Add repo secret → · Model settings → · Setup docs → · Ask in Discord →
|
|
Your Claude subscription has hit its usage limit. It resets at Aug 15, 9pm (UTC). Re-trigger Pullfrog after the reset, or add an Add repo secret → · Model settings → · Setup docs → · Ask in Discord →
|
|
Your Claude subscription has hit its usage limit. It resets at Aug 15, 9pm (UTC). Re-trigger Pullfrog after the reset, or add an Add repo secret → · Model settings → · Setup docs → · Ask in Discord →
|
|
Your Claude subscription has hit its usage limit. It resets at Aug 15, 9pm (UTC). Re-trigger Pullfrog after the reset, or add an Add repo secret → · Model settings → · Setup docs → · Ask in Discord →
|
b915e88 to
5a0c4db
Compare
|
Your Claude subscription has hit its usage limit. It resets at Aug 15, 9pm (UTC). Re-trigger Pullfrog after the reset, or add an Add repo secret → · Model settings → · Setup docs → · Ask in Discord →
|

Summary
masterwhile preserving its merge lineagereleaseEnvironment variablesCONFIG_PUBLISHER_REPOandCONFIG_SOURCE_REPOBOT_APP_ID/BOT_APP_PRIVATE_KEYpublisherGitShaandsourceGitShain release manifests/provenance; reject malformed, cross-org, swapped, redirected, symlinked, unreviewed, or mismatched repositories/commitsFinal integration head
a26a78ca59b41c66c80138ed81ab38efe83ccdc3includes98b77395085bd9183ead3f9813c6b25374ea34f9, which merges livemaster@084933c68f4d31f8e50bfc7d2996d3d4fef89184into the full release stack.The exact-head follow-up closes the privileged workflow paths reported by CodeQL and review: workflow code and built client are the same reviewed
github.shaalready reachable frommaster; untrusted/manual paths cannot populate pnpm caches; signing and upload credentials are isolated in exact per-brand protected Environments with fixed secret names; environment branch/tag policies are checked exactly; and credential-free evidence no longer claims a brand it did not exercise.Validation on the final head
pnpm check:ci— passed (existing warnings only)pnpm test— 2,859 passed, 5 skippedbuild=truefailed closed because fixtures intentionally contain no delivery inputsExact-head GitHub checks are running. Merge remains blocked until required checks, review-thread resolution, and the repository's required approval are satisfied.
Configuration boundary
Protected
releaseEnvironment variables:CONFIG_PUBLISHER_REPO,CONFIG_SOURCE_REPO,CONFIG_RELEASE_REVISION, andCONFIG_RELEASE_KEYRINGS. The two repository variables are configurable, must be distinct canonicalowner/repositoryidentities, and are currently restricted to ownerarcboxlabsby the org-installed App policy. No repository PAT is used. Environment metadata inspection uses the built-inGITHUB_TOKENwithactions: read.Signing, upload, store, and observability inputs are isolated in exact
release-<brandId>Environments selected by a reviewed matrix. Production readiness still depends on reviewed LinkCode production structural data and release credentials/inputs; this PR does not claim those exist or are externally verified.This final integration covers the commits in #429; do not merge #429 separately after this PR lands.