Conversation
…keeps the founder-held secret (#9533)
🚀 Preview deployed
|
|
governance: FAIL @ 4ffdaf9 content:3bda1fedf4fa — ADR id 0405 collides with a landed record This retracts the PASS above at the same head. That verdict asserted "0405 itself is The finding —
|
|
review-code: FAIL @ 4ffdaf9 content:3bda1fedf4fa — ADR citations address a record number already taken Graded against the amended acceptance-criteria block on #9533 (appended 2026-09-21, 8 rows, route 1 only) plus the founder's standing ruling row. The earlier route-2 criteria above it are superseded and were not graded. The engineering here is good and the two fences are real. It fails on one thing: every ADR citation in this class points at the wrong record. The finding — the cited record number is already taken
The repair is the renumber plus these seven carry-throughs. Nothing about the code's behavior changes. Details and the gate's own text are in the Per-criterion2 — 3 — the committed key can never reach a non-preview stage, and the deploy path shows which value each stage resolves to. Discharged, and I checked both fences independently rather than taking the claim. Fence one holds alone. The predicate is Fence two holds alone, and it is genuinely on the boot path. The echo. The resolving step prints which source the stage took on both arms, so a deploy log answers the question rather than leaving it to be read out of a nested YAML ternary. 4 — 5 — 6 — 8 — a Findings1. const root = yield* Effect.result(discoverRepoRoot(options.cwd));
if (!Result.isFailure(root) && root.success !== undefined) { ... }
return classifyAuthSecret(options.env[AUTH_SECRET_ENV] ?? "", {_tag: "Ambient", ...});
2. Deviations
Nothing undisclosed that this gate could see. CI
Verdict-written: 2026-09-21T06:11:34Z |
|
review-doc: FAIL @ 4ffdaf9 content:3bda1fedf4fa — the decision record's id collides with a landed record Graded against the amended acceptance-criteria block on #9533 (appended 2026-09-21, 8 rows, route 1 only). The route-2 criteria above it are superseded and were not graded. Three doc-class files: The finding — the record cannot land at 0405
That check sits outside the branch ruleset's four required contexts, so it does not block the queue on its own — which is the reason to name it here rather than assume the merge gate will. The repair reaches this class in three places beyond the record itself:
Seven more citations sit in the code class; they are listed in the Per-criterion1 — a fixed preview signing key is committed at a named path, with a note beside it saying it is preview-only and deliberately public. Discharged. 7 — a decision record cites the ruling comment and writes the blast radius down. Substantively discharged; only the number is wrong.
The record also discloses its own carve-out under Note, not a finding
DeviationsThe disclosure's third entry covers the Verdict-written: 2026-09-21T06:12:16Z |
|
review-skill: FAIL @ 4ffdaf9 content:3bda1fedf4fa — review-ui's contract still documents the retired secret route Two skill-class files: That read is what this FAIL rests on. The edited paragraphs are correct. The paragraphs the change left behind are not, and three of them still teach the retired route. Findings1.
Every clause there is now false. The committed key is the default source and does not appear at all. And the sentence describes, verbatim, the route this PR's own criterion 5 required removing — "re-export the repo-wide value from the ci-credentials stack's alchemy state" is the 2.
The head's 3. fabrika review-ui render --pr $pr_number --out forced --surface /welcome:auth --flag welcome-banner=on --auth-secret-from <file>Fourteen lines above it, the rewritten paragraph now tells the reader they need no credential and that the verb resolves the key off the checkout with no flag. The example then hands them All three are the same defect with one cause: the diff edited the two places that describe the secret source and left the three places that use or enumerate it. I named all three in this one body deliberately rather than leaving the next round to find the second and third — a repair that fixes only what the hunks made obvious will come back. What is correctThe edits that were made are accurate and well written.
Editorial
DeviationsNothing in this class was undisclosed. The PR's disclosure covers the test and pattern changes; it does not claim the contract was fully carried through, and it does not assert otherwise. Verdict-written: 2026-09-21T06:13:04Z |
…contract off the retired secret route (#9533)
|
Repair round 1 at 1 — the ADR id collision (
|
|
governance: PASS @ c323974 content:d3d1f0435b3d — no contradiction, no weakening Round 2, at head Scope
Whether this diff needs a code-owner approval is a separate question CODEOWNERS answers, not this namespace's. The corpus halfThe id. The record in the diff is
The sweep. The hand read, because the sweep is citation-independent and a disagreement about what a value means shares no vocabulary. Questions this change answers, and where the standing corpus sits on each:
The gate half
Read by hand, because an unanchored invariant is invisible to that scan and this diff is about a fence:
VerdictPASS. No contradiction with standing law and no weakening of a guard. The round-1 FAIL's cause — an id already claimed on |
|
review-code: PASS @ c323974 content:d3d1f0435b3d — all 8 criteria discharged at head, CI green Round 2, at head The contract graded is the last acceptance-criteria block on #9533, under Execution evidence
No local re-run of the suite: CI is the execution evidence, and this reviewer's worktree does not stand at the head. Per-criterion1. A fixed preview signing key is committed at a named path, with a note beside it saying it is preview-only and deliberately public. Discharged. 2.
3. The committed key can never reach a non-preview stage, and the deploy path shows which value each stage resolves to. Discharged, and I re-checked both fences rather than carrying round 1's word for it.
4. 5. 6. 7. A decision record cites the ruling and writes the blast radius down. Discharged; graded in full in the 8. A That render was taken at head Fan-outType design. Silent failure. One finding, and it is out of scope: Test gaps. The pure The disclosed out-of-scope change
Deviations
Nothing undisclosed that this gate could see. VerdictPASS. All 8 rows discharged at this head, CI green, one out-of-scope silent-failure finding routed to #9637 and one test gap named. Verdict-written: 2026-09-21T06:38:24Z |
|
review-doc: PASS @ c323974 content:d3d1f0435b3d — id free and derived, no stale reference, criterion 7 discharged Round 2, at head The round-1 FAIL's cause, re-derived hereRound 1 failed this class on an ADR id already claimed on
Criterion 7, the doc-class row"A decision record cites the ruling comment above and writes the blast radius down: a forged preview session reaches only that PR's own throwaway D1 (ADR 0349), preview logins are open to anyone who reads the repo, and the production worker rejects the committed key." Discharged, all four parts, and each is in the record rather than gestured at:
The record also answers the cross-stage question the criteria block raised, with Portability
Editorial craft —
|
|
review-skill: PASS @ c323974 content:d3d1f0435b3d — both files read whole, no retired route survives, docs match the verb Round 2, at head Both files were read end to end at the scoped head, out of the object database, not as hunks. That is the whole point of this round: round 1 failed this class because a stale sentence sat outside the hunks it had graded, and the way to not repeat it is to finish both documents in one pass and name every contradiction in one body. Criterion 5, the prose half"…and its visitor-answer message stops pointing a seat at the No trace of the retired route survives in either file. Searching both whole documents for Four spots, all four fixed. Round 1 named three and the repair says it found a fourth by reading both files whole. Reading them whole myself, I find the same four and no fifth:
The documented behaviour matches the verb, including the refusal strings.
One sentence I checked and am not calling stale. Portability
Editorial craft —
|
|
ship: awaiting control-plane approval — PR #9635 @ c323974 → human This PR is control-plane (4 owners resolved from CODEOWNERS at the base ref). Every machine gate at
The head is also 3 commits behind No enqueue, no merge, no deploy. Merge intent disarmed at site |
1 participant
Fixes #9533.
Preview workers stop verifying logins with the production signing secret. Every
pr-<n>preview now deploys with the key committed atinfra/preview-auth-key/key.txt, which is public on purpose, so any agent on any machine can sign a preview session with no setup and no credential. Production,auditand every hand-named stage keep the founder-heldBETTER_AUTH_SECRET. This is the founder's route-1 ruling on #9533, transcribed.The committed key cannot reach a non-preview stage, and that rests on structure rather than on a note saying not to:
.github/workflows/deploy.ymlpicks the value in its own step and echoes which source the stage took. The predicate isisPreviewStage(/^pr-\d+$/) inapps/web/worker/environment.ts— deliberately notenvironmentForStage(...) === "preview", which is fail-open on the stage axis and would hand a hand-named stage the public key. The founder-held secret is the default; the committed key is the exception.judgeAuthSecretinapps/web/worker/preview-auth-key.tsrejects anypreview_-prefixed secret on anyENVIRONMENTthat is notpreview, andBetterAuthLivethrows on it. A worker that would verify forgeable sessions serves nothing. The check keys on the prefix, not the literal bytes, so a rotation cannot outrun it; a unit test reads the real committed file and asserts it carries that prefix.review-ui renderresolves the committed key off the checkout it stands in — no--auth-secret-from, no environment variable, no ci-credentials read.--auth-secret-fromstill overrides, and the ambient$BETTER_AUTH_SECRETremains only as a fallback for a checkout with no committed key. No refusal points a seat at$ALCHEMY_PASSWORDany more.The blast-radius check is written down in
.decisions/0406-preview-workers-sign-with-a-committed-public-key.md, which cites the ruling comment in its own text. Summary: a forged preview session reaches one preview origin's worker and its own throwaway per-PR D1 (ADR 0349), anyone who reads the repo can forge one, and production rejects the key twice over. The cross-stage question the criteria asked — does anything expect a session signed on one stage to verify on another — is answered no, withinfra/depochecked by name: it is absent from.github/app-roster.jsonand from every workflow, both its stacks are hand-deployed, its sharing is production-to-production, and its doorman authenticates a pasaportapiKeyrather than a session cookie.Live evidence for the last criterion
A
review-ui renderof an:authsurface against this PR's own preview (pr-9635, head4ffdaf9), run with$BETTER_AUTH_SECRETexplicitly unset and no--auth-secret-from, so the committed key resolved from the checkout is the only source the cookie could have been signed with.preview-seed test-accountseeded the two tiers on this preview's own D1 first.Exit 0, not 11. Before recording that shot the verb asked the preview's own
/api/auth/get-sessionfrom the same browser context and required a user back at theyazartier, so the signature verified against what the worker actually deployed with. The capture path is omitted here because it is machine-local.Deviations
render-verb.ts's visitor-answer message and nothing about its tests. Did: changed therefuses a placeholder-prefixed ambient secretcase to assert the new route text (the committed key path, and noALCHEMY_PASSWORD) instead of the old--auth-secret-frompointer. Why: the criterion required that message to change, and the assertion pinned the exact sentence it required changing. Disposition: stated here; the case still proves the same refusal, only against the message the ruling asks for.capture/auth.unit.test.ts's helpers. Did: gave itsverifyhelper an optional secret parameter, defaulting to the existing fixture. Why: the new case verifies a signature made with the real committed key, and the helper was pinned to one module-level constant. Disposition: stated here; every existing call site is unchanged and passes the default..gitleaks.tomlallowlist entry for the committed key, and corrected the two lines in.patterns/alchemy-ci-cd.mdthat said the one Actions secret is handed to every stage. Why: the allowlist entry keeps the credential scanner from reding a value the founder ruled must be committed, and the pattern lines became false with this change. Disposition: stated here..github/workflows/pr-cleanup.ymlpassingsecrets.BETTER_AUTH_SECRETinto thealchemy destroyof a preview stage. Why: it is a teardown, not a serving worker — the value only has to resolve for a config read — and it is apull_request_targetworkflow whose failure leaks a worker and a D1, so it is not something to change alongside this. Disposition: recorded in ADR 0406's own text and filed as Preview teardown still hands the founder-held BETTER_AUTH_SECRET to a pr-<n> stage #9636.resolveAuthSecret's root-discovery branch. Did: made a faileddiscoverRepoRootrefuse on11naming the unreadable ancestor, instead of falling through to the ambient variable, and added a unit case for it. Why:discoverRepoRootkeeps "could not look" on itsEchannel and "no repo here" onundefined, and the fall-through reported the first as the second; thereview-codeverdict raised it as an advisory on this round. Disposition: stated here; the exit code is unchanged and only the message a seat reads improves.