feat(release): consume independent config source - #445
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29b8f73911
ℹ️ 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".
|
|
||
| config-integration: | ||
| name: Cross-repository config contract | ||
| if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} |
There was a problem hiding this comment.
Dependabot pull-request workflows do not receive Actions secrets, but Dependabot branches still belong to this repository, so this condition evaluates true. The job then reaches the credential check with empty BOT_APP_ID/BOT_APP_PRIVATE_KEY, fails, and causes the required all-green job to fail for every npm, Cargo, and Actions update configured in .github/dependabot.yml; exclude dependabot[bot] here just as external forks are excluded.
Useful? React with 👍 / 👎.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The security rework itself is sound — I traced every new fail-closed gate in the render action and none of them fail open, and the deployments: read → actions: read swap is the documented-correct scope. Two things need a human answer before merge: the production render path is now fail-closed by design (which the desktop release train runs through), and the new blocking CI job makes ordinary CI depend on organization App credentials.
Reviewed changes — full initial review of both commits on xuan/code-561-cross-repo-client; CI/release plumbing only, no product code.
- Fixed repository identities —
render-release-configdrops thepublisher-repoinput and hardcodesarcboxlabs/linkcodehqplusarcboxlabs/linkcode-config, so release data controls only the two SHAs and asource-rootchosen from a two-entry allow-list. - Two independent checkouts — the single fetch +
git worktree addis replaced by two shallow fetches, each with its own token,http.followRedirects=false, and a post-checkoutrev-parse HEAD+remote get-url originidentity assertion. - New fail-closed gates — 40-hex lowercase SHA regex, structural-root existence, symlink rejection under the source root, and a byte-exact
cmpof the config repo's schema mirror against the publisher's canonical schema. - Short-lived scoped tokens — every render job pre-flights
BOT_APP_ID/BOT_APP_PRIVATE_KEYand mints twopermission-contents: readinstallation tokens before checking out the selected client ref, and addspersist-credentials: false.CONFIG_PUBLISHER_REPO,CONFIG_PUBLISHER_TOKEN, andRELEASE_ENVIRONMENT_ADMIN_TOKENare removed from workflows,release-inputs.cjs, and the docs. - Environment preflight — reads protection metadata with
${{ github.token }}under workflow-levelactions: read. - New
config-integrationCI job — checks out both private repos at hardcoded SHAs and validatesexamples/acme-zenithwith the publisher's pinned parser; gated intoall-green. - Pilot repin + tests —
code-561-pilot.jsonmoves to publisher986d9f21…/ now-divergent sourcea1ed4d66…;brand-matrix.test.mjsgains a mint-before-checkout assertion and a publisher≠source assertion.
Two things I checked and am not flagging, so they don't get re-litigated: actions: read is the scope GitHub documents for GET /repos/{owner}/{repo}/environments/{name} (the removed deployments: read was the wrong one, and protection_rules.required_reviewers.reviewers is not redacted for non-admin readers), and all-green's success || skipped escape hatch is safe here because config-integration has no needs: and so can only be skipped by its own fork guard.
⚠️ The production render path is fail-closed after this merges, and the desktop release train runs through it
build-desktop.yml and build-mobile.yml pass no source-root, so both default to . — the root that the action's own error text and the new docs/RELEASE.md paragraph describe as "intentionally unavailable until reviewed production data exists". release-desktop.yml invokes build-desktop.yml with sign: true on a v* tag, and render-config already runs on master under if: inputs.sign, so this is a behaviour change on the shipping path rather than new scaffolding. Only release-brand-matrix.yml passes the reachable examples/acme-zenith root.
Technical details
# Production render path defaults to a root the action declares unavailable
## Affected sites
- `.github/actions/render-release-config/action.yml:21` — `source-root` defaults to `"."`.
- `.github/actions/render-release-config/action.yml:168-172` — `.` resolves to the `linkcode-config`
repository root; the step aborts unless that root holds `brands.manifest.yaml` and
`schema/config.schema.json`.
- `.github/workflows/build-desktop.yml:146-154` — `render-config` (runs when `inputs.sign`) omits
`source-root`.
- `.github/workflows/build-mobile.yml:131-140` — `render-config` (runs whenever
`rendered_artifact == ''`) omits `source-root`.
- `.github/workflows/release-desktop.yml:34-41` — tag-triggered release calls `build-desktop.yml`
with `sign: true`.
## Required outcome
- A reader (and the next person to cut a release) can tell whether the desktop/mobile release path is
expected to succeed after this merges.
## Open questions for the human
- Does the `arcboxlabs/linkcode-config` root already carry `brands.manifest.yaml` and
`schema/config.schema.json`? If yes, nothing to do here.
- If not: was production rendering already unreachable for an independent reason (e.g.
`CONFIG_RELEASE_MANIFEST_DESKTOP` unset in the `release` environment)? If it was reachable, this
needs to land with the production data, or the release train needs an explicit hold and a note in
`docs/RELEASE.md` saying signed releases are blocked until then.ℹ️ Nitpicks
.github/workflows/release-brand-matrix.yml:371re-hardcodesexamples/acme-zenithin thecppath 30 lines after passing it assource-root:. It fails loudly if the two drift, but deriving the path from one value would be tidier..github/workflows/ci.yml:354-355asserts the Node and pnpm versions that the two steps immediately above configure — the assertion can only fail ifsetup-node/action-setupsilently ignored their inputs..github/actions/render-release-config/action.yml:184still runspnpm install --frozen-lockfilewithout--ignore-scriptson a publisher checkout whose SHA comes from release data, while the new CI job atci.yml:357does pass it. The install line is untouched by this PR, so it's pre-existing — but the asymmetry is now visible in the same diff.docs/ENVIRONMENT.md:132keepsBOT_APP_ID/BOT_APP_PRIVATE_KEYunder the Release-only secrets heading, butci.ymlnow requires them on every push and internal PR. The cell text says "release and config-render workflows", which doesn't cover CI.
Claude Opus ｜ 𝕏
| if [ -z "$BOT_APP_ID" ] || [ -z "$BOT_APP_PRIVATE_KEY" ]; then | ||
| echo "::error::BOT_APP_ID and BOT_APP_PRIVATE_KEY must be available so CI can read arcboxlabs/linkcodehq and arcboxlabs/linkcode-config" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This job has no environment:, so BOT_APP_ID/BOT_APP_PRIVATE_KEY have to be repository- or organization-level secrets — but docs/ENVIRONMENT.md files them under "Release-only secrets … Set as GitHub repository/environment secrets". If they are currently scoped to the release environment, this step fails on every push to master and every internal PR, and all-green goes red repo-wide.
Worth confirming the actual scoping before merge, and deciding whether an outage in linkcodehq/linkcode-config (or a revoked installation) should be able to block all CI rather than just release CI.
Technical details
# `config-integration` gives ordinary CI a hard dependency on org App credentials
## Affected sites
- `.github/workflows/ci.yml:283-297` — job has no `environment:` key; the guard hard-fails when the
two secrets resolve empty.
- `.github/workflows/ci.yml:363-384` — `all-green` treats any non-`success`/non-`skipped` result as a
failure, so this propagates to the required check.
- `docs/ENVIRONMENT.md:132` — still lists the pair under "Release-only secrets".
## Required outcome
- CI stays green for changes unrelated to the config contract when the credentials or the private
repositories are unavailable, **or** the blocking behaviour is a deliberate, documented choice.
## Open questions for the human
- Are `BOT_APP_ID`/`BOT_APP_PRIVATE_KEY` repo/org secrets today, or environment secrets on `release`?
- Is this job meant to block `all-green`, or should it be advisory (fail-soft) until the config
repositories are stable?| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| with: | ||
| repository: arcboxlabs/linkcodehq | ||
| ref: 986d9f21403df53bc932f511eb1b5f0bb634d48d |
There was a problem hiding this comment.
These two SHAs are hardcoded here (again at lines 332, 352, 353), in .github/release/brand-matrices/code-561-pilot.json, and in docs/RELEASE.md. brand-matrix.test.mjs pins the matrix values to the same literals, but nothing ties ci.yml to the matrix — so when the pilot is repinned, this job keeps validating the old commit pair and the "cross-repository contract" silently stops covering what the release actually renders.
Technical details
# The CI contract job's pins can drift from the matrix it is meant to validate
## Affected sites
- `.github/workflows/ci.yml:323` / `:332` — `ref:` literals for the two checkouts.
- `.github/workflows/ci.yml:352-353` — `rev-parse` assertions repeating the same literals.
- `.github/release/brand-matrices/code-561-pilot.json` — `publisherGitSha` / `sourceGitSha` for all
six manifests.
- `.github/scripts/brand-matrix.test.mjs:104-107` — asserts the matrix against the literals but never
reads `ci.yml`.
## Required outcome
- Repinning the pilot matrix either updates this job automatically, or fails a test until it is
updated.
## Suggested approach (optional)
- Read `publisherGitSha` / `sourceGitSha` out of `code-561-pilot.json` with `jq` in the job and use
them for both checkouts, or extend the existing `brand-matrix.test.mjs` case to assert that
`ci.yml` contains the same two SHAs the matrix pins.1 participant
Summary
arcboxlabs/linkcodehq@publisherGitShaand independentarcboxlabs/linkcode-config@sourceGitShaGITHUB_TOKENplusactions: readCONFIG_PUBLISHER_REPO,CONFIG_PUBLISHER_TOKEN, andRELEASE_ENVIRONMENT_ADMIN_TOKENcontracts986d9f21403df53bc932f511eb1b5f0bb634d48dand configa1ed4d666721c3aed0d563aaea42fce8b5f945b5This 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.Validation
pnpm check:ci(format, lint with existing warnings, typecheck)pnpm test(2,813 passed, 5 skipped)