feat(auth): allow an opt-in split credential pair (#1094) #1100

Merged

pullings merged 7 commits from i/1094-split-credential-pair into main 2026-09-04 02:29:40 +02:00

Conversation 5 Commits 7 Files changed 20 +1160 -61

quartermaster commented 2026-09-04 01:06:52 +02:00

Owner

Copy link

A consumer may now supply a split credential pair — RELEASE_TOOLKIT_GIT_TOKEN (a repo-scoped deploy key, for git transport) and RELEASE_TOOLKIT_API_TOKEN (a narrow PAT, for the API) — instead of one token doing both. Tiers 1 and 2 are unchanged.

TIER 1  nothing set              auto token   convenient, no downstream trigger
TIER 2  RELEASE_TOOLKIT_TOKEN    one PAT      the default, unchanged
TIER 3  deploy key + narrow PAT  split        least privilege, OPT-IN

① What it buys — measured, and not the obvious answer

The premise was that tier 3 buys push-scoping. It buys more than that, and the difference is the whole justification (ai#711):

ARM A  auto token   tag pushed, ZERO downstream runs
ARM B  deploy key   tag pushed, downstream run CREATED and CLAIMED

A push made from inside a run with the auto token is suppressed — anti-recursion, so a workflow cannot trigger itself. The same push with a deploy key is not. So the split buys the trigger, not merely a narrower scope.

Both arms ran with in-run controls proving the credential under test was the one authenticating: a fabricated key was refused (rc=128, remote: Failed to authenticate user), and the real key succeeded on the same ref namespace.

② The bound, stated wherever the benefit is

⚠️ write:repository is the narrowest API category Gitea offers. The split bounds the push surface fully — a deploy key reaches one repository — and bounds the API surface only as far as the owning account's reach.

That sentence is in the package doc, in integration.md, and in the command's own PASS line. An overstated security property is worse than an absent one, and this one is easy to overstate.

③ Not recommended, deliberately

🔴 Tier 3 is opt-in and the docs open by telling you to skip it. For almost every adopter a single token is the better trade: one secret to create, one to rotate, one thing to get wrong. The docs section begins "Set one RELEASE_TOOLKIT_TOKEN and skip this section" and says that if you do not already know you need the split, you do not.

A doc that steers everyone to the careful-sounding option has made the product worse.

④ Half-configured REFUSES, before any cut work

The grade the credential configuration step runs before rt decide — the ordering is pinned by an arm, not by placement.

Falling back to the single token for the missing half would push a tag and then fail to create the release, leaving a tag with no assets — the v0.56.1 shape, this time produced by our code on an adopter's repository. The refusal names which variable is set, which is missing, and both repairs:

credentials: RELEASE_TOOLKIT_GIT_TOKEN is set but RELEASE_TOOLKIT_API_TOKEN is empty — …
  Set BOTH to use the split pair, or UNSET RELEASE_TOOLKIT_GIT_TOKEN to use a single token (tier 2).
  Refusing rather than falling back: a cut that pushes with one credential and then
  cannot create the release leaves a tag with no assets.

Tier 3 also selects path α, since a deploy key is a push credential. Keying path selection on RELEASE_TOKEN_OVERRIDE alone would send a tier-3 consumer down path γ and open a manifest PR they did not ask for.

⑤ Carrying ai#712 into the implementation

withTokenConfig now resets the credential.helper list. It is additive, and GIT_CONFIG_GLOBAL replaces only the global file — so a helper in the repo-local .git/config or the system config still runs, answers first, and git never consults the extraheader we wrote. The push then succeeds under a different credential than the caller supplied, with no error and nothing in the log.

That is not hypothetical: four consecutive credential tests came back void that way this week, including one using a deliberately fabricated key. Passed as GIT_CONFIG_* env, never argv, matching the existing #381 constraint.

Arms — mutation-verified, each reddening its own and no other

RESOLVER (internal/credentials)
  M1 half-config falls through to tier 2      -> halfConfigured_refusesRatherThanFallingBack
  M2 tier 3 hands the SAME token to both      -> TierSplit_bothSet
  M3 tier 2 returns the auto token            -> TierSingle_overrideSet
  M4 refusal drops the second repair          -> namesBothRepairs
  M5 refusal names only one variable          -> namesBothRepairs
  M6 refusal leaks a credential value         -> halfConfigured_refuses…

CREDENTIAL ISOLATION (internal/prep)
  P1 drop the helper reset                    -> resetsTheCredentialHelperList
  P2 reset value non-empty (ADDS a helper)    -> resetsTheCredentialHelperList
  P3 empty-token path emits config env        -> emptyTokenPassesNoEnvAtAll

COMMAND (cmd/rt)
  C1 silence the refusal                      -> halfConfiguredRefusalReachesStderr
  C2 drop the scope bound from the PASS       -> passNamesItsSilenceAndLeaksNothing

WORKFLOW (tests/workflows.bats)
  W1 grade step moved after decide            -> runs BEFORE any cut work
  W2 PUSH_MODE keys on the single token only  -> selects path alpha, not gamma
  W3 one env block loses the split pair       -> threaded wherever the single token is

⚠️ One mutation was invalid and my harness reported it as an arm gap. M4's first form dropped a %s while Sprintf still passed the argument, so go vet failed the build — and the harness counted only ^--- FAIL lines, so a build failure read as zero reddened, i.e. as the arm missing it. An inert mutation and an uncatchable bug print the same thing, and so does an invalid one. The harness now refuses to grade a mutation that does not build.

🔴 I wrote the SilenceErrors defect again, hours after approving its fix

rt credentials --check on a half-configured pair originally exited 2 into an empty stderr — the exact defect @engineer found in gitea-twin (#1092), which I reviewed and approved the same evening.

It was caught by running the refusal, not by writing it. Attention did not carry across two files, so there is now an arm asserting the refusal reaches stderr and names both variables — not merely that stderr is non-empty, since the PASS path writes there too.

What this PR does NOT do

• Does not change tiers 1 or 2. The :- chain is nested, never replaced; the inner ${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT} is asserted intact by the existing #705 arm, which now checks both halves separately.
• Does not verify that either credential WORKS. It reads which variables are set. A revoked, wrong-scoped, or wrong-host credential resolves identically — the PASS line says so.
• Does not narrow the API surface. It cannot; see ② for the bound.
• Was not exercised against a live tier-3 cut. No adopter is configured this way, and inventing one is outside the bound.

Gates, each rc in its own variable: gofmt 0 · build · vet · go test ./... · bats 173/173 · gitea-twin --check · fragment-check · changelog-body-check · register-check · YAML parse over both workflow dirs. All green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

---

Review 6256 — two tier-3 runtime gaps, both fixed

Both were real and both were mine: the feature resolved the credential correctly and then two call sites did not consult the resolution. Reproduced before fixing.

① Split brain on the path. post_cut.go read RELEASE_TOKEN_OVERRIDE directly, so a split-only consumer got PUSH_MODE=alpha from the workflow and the gamma path from rt post-cut — a manifest PR opened for a cut already graded direct-push. Neither layer was wrong about the input it read; they read different inputs.

Both now derive from Pair.DirectPush(), resolved once and above its first use. The rule lives in one place, which makes the divergence unrepresentable rather than merely repaired.

② The post-condition fetch is a git operation and took the API/auto chain, so a tier-3 fetch authenticated with the auto token. That reads a protected default branch as unreadable and grades a successful cut could-not-grade — the #673 shape, reintroduced through the wrong credential rather than a missing one.

Controls, each mutation-verified to redden its own arm

L1 path selection reads the env again      -> the source guard
L2 post_cut stops calling the resolver     -> the same guard, POSITIVE control
L3 DirectPush drops tier 3                 -> BOTH tier-3 subtests
L4 DirectPush lets the auto token push     -> tier 1 + the gamma arm
L5 the fetch reverts to the API chain      -> the bats fetch arm

L3 and L4 redden different subtests of one table, which is the separation that matters: one is "tier 3 lost direct-push", the other is "tier 1 gained it".

⚠️ The source guard is deliberate, and it is the only thing that can catch ①. Reading the env directly returns the right answer in tiers 1 and 2 and diverges only under a split-only environment — so a behavioural test in that package cannot catch the reintroduction. It strips comments before matching, so the paragraph naming the variable does not trip it, and it carries a positive control (L2) so "no direct reads" cannot be satisfied by a file that stopped using the resolver.

One more instrument slip, recorded because it nearly shipped

Verifying the twin carried fix ②, I grepped with a double-quoted needle. The shell expanded ${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT} before grep saw it, and the count came back 0 — reading as the twin does not carry the fix. It does; grep -F with a single-quoted needle returns 1 in both files.

It was caught only because gitea-twin --check had just returned rc=0, and both could not be true. That is ai#607, on my own instrument, inside the fix for a review about credentials.

Tier-2 guidance is unchanged: still the recommended path, still what the docs open by telling you to use.

---

The reader census — and a third gap it found

Walked every reader of the credential variables, per site, rather than converting them.

9 raw readers, ALL of them reading FORGEJO_TOKEN
0 readers of RELEASE_TOKEN_OVERRIDE / GITHUB_TOKEN_DEFAULT remain outside the resolver
4 call sites route through credentials.Resolve

🔑 Seven of the nine are correct, and correct BY CONSTRUCTION rather than by luck. The workflow exports FORGEJO_TOKEN as the API credential under tier 3, so a plain API read gets the right token — decide, prep's PR client, pre-push, repin, binary-size-check, and both manifest-postcondition reads. prep.go:622 is a capability guard (do we have an API token at all), also correct. None of them should route through Resolve; converting them would be the twelve-site regression.

⑧ The eighth is different in kind, and it is a third defect

preflight-push-whitelist resolves the push identity with GetAuthenticatedUser on the API token and checks that login against push_whitelist_usernames. Under a split pair the manifest is pushed with the deploy key, so it grades an identity that will not do the pushing — and it is wrong in both directions:

• it can FATAL a cut whose deploy-key push is perfectly authorised, and
• it can pass one that will be rejected mid-cut.

Deploy keys are governed by push_whitelist_deploy_keys, which this codebase does not model at all.

This PR does not add deploy-key whitelist support — that is a feature and out of bounds. It applies the file's own doctrine instead: a probe limitation must never block a cut that would actually succeed; the runtime push stays fail-loud as the backstop. Under a split pair the gate now warns rather than grading the wrong identity, and still passes when no whitelist is enabled, because that answer does not depend on identity.

N1 drop the guard             -> all four split arms
N2 guard always passes        -> both whitelist-enabled arms
N3 invert the whitelist test  -> the no-whitelist arm

⚠️ The arms use a client that FAILS if the identity is ever resolved. A fake merely returning a login could not separate did not consult from consulted and happened to agree. A control asserts the tier-1/2 path still does grade the identity, so the guard cannot be satisfied by breaking the gate for everybody.

📌 Why my own review missed it, which is the same shape as the one reported to me: I verified that the resolver was right and that the two reported call sites now consult it. The resolver being correct is evidence about the resolver. Whether every consumer asks it is a different claim, and nothing about a green resolver suite prompts you to go and count the consumers.

---

Review 6259 — the generic path-α docs described tier 2's mechanism as α's

Three sentences were true of a bot PAT and false of a deploy key, and the pre-flight change in this PR is what made them false:

site	split
path-α precondition	tier 2 → push_whitelist_usernames; tier 3 → push_whitelist_deploy_keys
pre-flight blockquote	tier 2 → fails fast naming the un-whitelisted identity; tier 3 → warns, because the identity it can resolve is not the identity that will push
bot-provisioning heading	labelled tier 2, and still the recommended setup

The mirrored workflow comments said "Path-alpha token override" and named only RELEASE_TOKEN_OVERRIDE — so an adopter tracing why their tier-3 cut took α found a comment that did not mention their credential. All three now name both.

Tier 2 remains the recommendation in every place it was one.

Review 6256 follow-on — the push bound was overstated, and it is qualified now

This PR claimed tier 3 "bounds the PUSH surface fully." Measured at origin/main 22822dd, that is false at one seam: internal/prep/hooks.go runs a consumer-authored post_bump_hook with the inherited environment, so the hook receives both credentials.

🔑 Not a leak — a recombination. Inheritance predates tier 3 and a single token was always inherited, so nothing is newly exposed. What is new is that tier 3's entire purchase is keeping the push credential off the API surface, and at this seam an adopter who took deliberate action to separate two credentials hands a script both. Anything in that hook can push.

Qualified in all three places the claim was made — the package doc, integration.md, and the command's PASS line — rather than left standing while the remedy is decided. The remedy is tracked separately (rt#1106); the sentence belonged to this PR because this PR wrote it.

The tier-3 PASS arm now asserts the hook disclosure, so the qualification cannot be silently dropped. Mutation: remove the disclosure → that arm reddens and nothing else does.

One more instrument note

Adding those workflow comments shifted the file's line numbers, and workflow-api.md cites grounded ranges into it. The bats pin caught it — outputs moved 110-146 → 112-148.

The doc line that pin protects reads "line ranges are coordinates into a file that moves". It fired on a comment-only edit, which is the change least likely to make anyone go and check.

A consumer may now supply a **split credential pair** — `RELEASE_TOOLKIT_GIT_TOKEN` (a repo-scoped deploy key, for git transport) and `RELEASE_TOOLKIT_API_TOKEN` (a narrow PAT, for the API) — instead of one token doing both. Tiers 1 and 2 are unchanged. ``` TIER 1 nothing set auto token convenient, no downstream trigger TIER 2 RELEASE_TOOLKIT_TOKEN one PAT the default, unchanged TIER 3 deploy key + narrow PAT split least privilege, OPT-IN ``` ## ① What it buys — measured, and not the obvious answer The premise was that tier 3 buys push-scoping. It buys more than that, and the difference is the whole justification (ai#711): ``` ARM A auto token tag pushed, ZERO downstream runs ARM B deploy key tag pushed, downstream run CREATED and CLAIMED ``` A push made from inside a run with the auto token is **suppressed** — anti-recursion, so a workflow cannot trigger itself. The same push with a deploy key is not. So the split buys the **trigger**, not merely a narrower scope. Both arms ran with in-run controls proving the credential under test was the one authenticating: a fabricated key was refused (`rc=128`, `remote: Failed to authenticate user`), and the real key succeeded on the same ref namespace. ## ② The bound, stated wherever the benefit is ⚠️ `write:repository` is the narrowest API category Gitea offers. The split bounds the **push** surface fully — a deploy key reaches one repository — and bounds the **API** surface only as far as the owning account's reach. That sentence is in the package doc, in `integration.md`, and in the command's own PASS line. **An overstated security property is worse than an absent one**, and this one is easy to overstate. ## ③ Not recommended, deliberately 🔴 **Tier 3 is opt-in and the docs open by telling you to skip it.** For almost every adopter a single token is the better trade: one secret to create, one to rotate, one thing to get wrong. The docs section begins *"Set one `RELEASE_TOOLKIT_TOKEN` and skip this section"* and says that if you do not already know you need the split, you do not. **A doc that steers everyone to the careful-sounding option has made the product worse.** ## ④ Half-configured REFUSES, before any cut work The `grade the credential configuration` step runs **before `rt decide`** — the ordering is pinned by an arm, not by placement. Falling back to the single token for the missing half would push a tag and then fail to create the release, leaving **a tag with no assets** — the v0.56.1 shape, this time produced by our code on an adopter's repository. The refusal names which variable is set, which is missing, and both repairs: ``` credentials: RELEASE_TOOLKIT_GIT_TOKEN is set but RELEASE_TOOLKIT_API_TOKEN is empty — … Set BOTH to use the split pair, or UNSET RELEASE_TOOLKIT_GIT_TOKEN to use a single token (tier 2). Refusing rather than falling back: a cut that pushes with one credential and then cannot create the release leaves a tag with no assets. ``` Tier 3 also selects **path α**, since a deploy key *is* a push credential. Keying path selection on `RELEASE_TOKEN_OVERRIDE` alone would send a tier-3 consumer down path γ and open a manifest PR they did not ask for. ## ⑤ Carrying ai#712 into the implementation `withTokenConfig` now resets the `credential.helper` list. It is **additive**, and `GIT_CONFIG_GLOBAL` replaces only the *global* file — so a helper in the repo-local `.git/config` or the system config still runs, answers first, and git never consults the extraheader we wrote. The push then succeeds **under a different credential than the caller supplied**, with no error and nothing in the log. That is not hypothetical: four consecutive credential tests came back void that way this week, including one using a deliberately fabricated key. Passed as `GIT_CONFIG_*` env, never argv, matching the existing #381 constraint. ## Arms — mutation-verified, each reddening its own and no other ``` RESOLVER (internal/credentials) M1 half-config falls through to tier 2 -> halfConfigured_refusesRatherThanFallingBack M2 tier 3 hands the SAME token to both -> TierSplit_bothSet M3 tier 2 returns the auto token -> TierSingle_overrideSet M4 refusal drops the second repair -> namesBothRepairs M5 refusal names only one variable -> namesBothRepairs M6 refusal leaks a credential value -> halfConfigured_refuses… CREDENTIAL ISOLATION (internal/prep) P1 drop the helper reset -> resetsTheCredentialHelperList P2 reset value non-empty (ADDS a helper) -> resetsTheCredentialHelperList P3 empty-token path emits config env -> emptyTokenPassesNoEnvAtAll COMMAND (cmd/rt) C1 silence the refusal -> halfConfiguredRefusalReachesStderr C2 drop the scope bound from the PASS -> passNamesItsSilenceAndLeaksNothing WORKFLOW (tests/workflows.bats) W1 grade step moved after decide -> runs BEFORE any cut work W2 PUSH_MODE keys on the single token only -> selects path alpha, not gamma W3 one env block loses the split pair -> threaded wherever the single token is ``` ⚠️ **One mutation was invalid and my harness reported it as an arm gap.** M4's first form dropped a `%s` while `Sprintf` still passed the argument, so `go vet` failed the build — and the harness counted only `^--- FAIL` lines, so a build failure read as *zero reddened*, i.e. as the arm missing it. **An inert mutation and an uncatchable bug print the same thing, and so does an invalid one.** The harness now refuses to grade a mutation that does not build. ## 🔴 I wrote the SilenceErrors defect again, hours after approving its fix `rt credentials --check` on a half-configured pair originally exited **2 into an empty stderr** — the exact defect @engineer found in `gitea-twin` (#1092), which I reviewed and approved the same evening. **It was caught by running the refusal, not by writing it.** Attention did not carry across two files, so there is now an arm asserting the refusal reaches stderr *and* names both variables — not merely that stderr is non-empty, since the PASS path writes there too. ## What this PR does NOT do - **Does not change tiers 1 or 2.** The `:-` chain is nested, never replaced; the inner `${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT}` is asserted intact by the existing #705 arm, which now checks both halves separately. - **Does not verify that either credential WORKS.** It reads which variables are set. A revoked, wrong-scoped, or wrong-host credential resolves identically — the PASS line says so. - **Does not narrow the API surface.** It cannot; see ② for the bound. - **Was not exercised against a live tier-3 cut.** No adopter is configured this way, and inventing one is outside the bound. Gates, each rc in its own variable: gofmt 0 · build · vet · `go test ./...` · bats 173/173 · `gitea-twin --check` · fragment-check · changelog-body-check · register-check · YAML parse over both workflow dirs. All green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH --- ## Review 6256 — two tier-3 runtime gaps, both fixed Both were real and both were mine: the feature resolved the credential correctly and then **two call sites did not consult the resolution**. Reproduced before fixing. **① Split brain on the path.** `post_cut.go` read `RELEASE_TOKEN_OVERRIDE` directly, so a split-only consumer got `PUSH_MODE=alpha` from the workflow and the **gamma** path from `rt post-cut` — a manifest PR opened for a cut already graded direct-push. Neither layer was wrong about the input it read; they read *different inputs*. Both now derive from `Pair.DirectPush()`, resolved once and above its first use. The rule lives in one place, which makes the divergence unrepresentable rather than merely repaired. **② The post-condition fetch is a git operation** and took the API/auto chain, so a tier-3 fetch authenticated with the **auto token**. That reads a protected default branch as unreadable and grades a *successful* cut could-not-grade — the #673 shape, reintroduced through the wrong credential rather than a missing one. ### Controls, each mutation-verified to redden its own arm ``` L1 path selection reads the env again -> the source guard L2 post_cut stops calling the resolver -> the same guard, POSITIVE control L3 DirectPush drops tier 3 -> BOTH tier-3 subtests L4 DirectPush lets the auto token push -> tier 1 + the gamma arm L5 the fetch reverts to the API chain -> the bats fetch arm ``` L3 and L4 redden different *subtests* of one table, which is the separation that matters: one is "tier 3 lost direct-push", the other is "tier 1 gained it". ⚠️ **The source guard is deliberate, and it is the only thing that can catch ①.** Reading the env directly returns the **right** answer in tiers 1 and 2 and diverges only under a split-only environment — so a behavioural test in that package cannot catch the reintroduction. It strips comments before matching, so the paragraph naming the variable does not trip it, and it carries a positive control (L2) so "no direct reads" cannot be satisfied by a file that stopped using the resolver. ### One more instrument slip, recorded because it nearly shipped Verifying the twin carried fix ②, I grepped with a **double-quoted** needle. The shell expanded `${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT}` before `grep` saw it, and the count came back **0** — reading as *the twin does not carry the fix*. It does; `grep -F` with a single-quoted needle returns 1 in both files. It was caught only because `gitea-twin --check` had just returned rc=0, and both could not be true. That is ai#607, on my own instrument, inside the fix for a review about credentials. Tier-2 guidance is unchanged: still the recommended path, still what the docs open by telling you to use. --- ## The reader census — and a third gap it found Walked every reader of the credential variables, per site, rather than converting them. ``` 9 raw readers, ALL of them reading FORGEJO_TOKEN 0 readers of RELEASE_TOKEN_OVERRIDE / GITHUB_TOKEN_DEFAULT remain outside the resolver 4 call sites route through credentials.Resolve ``` 🔑 **Seven of the nine are correct, and correct BY CONSTRUCTION rather than by luck.** The workflow exports `FORGEJO_TOKEN` *as the API credential* under tier 3, so a plain API read gets the right token — `decide`, `prep`'s PR client, `pre-push`, `repin`, `binary-size-check`, and both `manifest-postcondition` reads. `prep.go:622` is a capability guard (*do we have an API token at all*), also correct. **None of them should route through `Resolve`; converting them would be the twelve-site regression.** ### ⑧ The eighth is different in kind, and it is a third defect `preflight-push-whitelist` resolves the push identity with `GetAuthenticatedUser` **on the API token** and checks that login against `push_whitelist_usernames`. Under a split pair **the manifest is pushed with the deploy key**, so it grades an identity that will not do the pushing — and it is wrong in *both* directions: - it can **FATAL** a cut whose deploy-key push is perfectly authorised, and - it can **pass** one that will be rejected mid-cut. Deploy keys are governed by `push_whitelist_deploy_keys`, which **this codebase does not model at all**. This PR does **not** add deploy-key whitelist support — that is a feature and out of bounds. It applies the file's own doctrine instead: *a probe limitation must never block a cut that would actually succeed; the runtime push stays fail-loud as the backstop.* Under a split pair the gate now **warns** rather than grading the wrong identity, and still **passes** when no whitelist is enabled, because that answer does not depend on identity. ``` N1 drop the guard -> all four split arms N2 guard always passes -> both whitelist-enabled arms N3 invert the whitelist test -> the no-whitelist arm ``` ⚠️ **The arms use a client that FAILS if the identity is ever resolved.** A fake merely returning a login could not separate *did not consult* from *consulted and happened to agree*. A control asserts the tier-1/2 path still **does** grade the identity, so the guard cannot be satisfied by breaking the gate for everybody. 📌 **Why my own review missed it, which is the same shape as the one reported to me:** I verified that the resolver was right and that the two *reported* call sites now consult it. The resolver being correct is evidence about the resolver. Whether every consumer asks it is a different claim, and nothing about a green resolver suite prompts you to go and count the consumers. --- ## Review 6259 — the generic path-α docs described tier 2's mechanism as α's Three sentences were true of a bot PAT and false of a deploy key, and **the pre-flight change in this PR is what made them false**: | site | split | |---|---| | path-α precondition | tier 2 → `push_whitelist_usernames`; tier 3 → `push_whitelist_deploy_keys` | | pre-flight blockquote | tier 2 → **fails fast** naming the un-whitelisted identity; tier 3 → **warns**, because the identity it can resolve is not the identity that will push | | bot-provisioning heading | labelled **tier 2**, and still the recommended setup | The mirrored workflow comments said *"Path-alpha token override"* and named only `RELEASE_TOKEN_OVERRIDE` — so an adopter tracing why their tier-3 cut took α found a comment that did not mention their credential. All three now name both. **Tier 2 remains the recommendation in every place it was one.** ## Review 6256 follow-on — the push bound was overstated, and it is qualified now This PR claimed tier 3 *"bounds the PUSH surface fully."* Measured at `origin/main` `22822dd`, that is false at one seam: `internal/prep/hooks.go` runs a **consumer-authored** `post_bump_hook` with the inherited environment, so the hook receives **both** credentials. 🔑 **Not a leak — a recombination.** Inheritance predates tier 3 and a single token was always inherited, so nothing is newly exposed. What is new is that tier 3's entire purchase is keeping the push credential off the API surface, and at this seam an adopter who took deliberate action to separate two credentials hands a script both. Anything in that hook can push. Qualified in **all three places the claim was made** — the package doc, `integration.md`, and the command's PASS line — rather than left standing while the remedy is decided. The remedy is tracked separately (rt#1106); the *sentence* belonged to this PR because this PR wrote it. The tier-3 PASS arm now asserts the hook disclosure, so the qualification cannot be silently dropped. Mutation: remove the disclosure → that arm reddens and nothing else does. ## One more instrument note Adding those workflow comments **shifted the file's line numbers**, and `workflow-api.md` cites grounded ranges into it. The bats pin caught it — outputs moved `110-146` → `112-148`. The doc line that pin protects reads *"line ranges are coordinates into a file that moves"*. It fired on a comment-only edit, which is the change least likely to make anyone go and check.

quartermaster added 1 commit 2026-09-04 01:06:52 +02:00

feat(auth): allow an opt-in split credential pair (#1094) ... 1a1b5f8dd0

A consumer may now supply RELEASE_TOOLKIT_GIT_TOKEN (a repo-scoped deploy key,
for git transport) and RELEASE_TOOLKIT_API_TOKEN (a narrow PAT, for the API)
instead of one token doing both. Tiers 1 and 2 are unchanged.

WHAT IT BUYS, MEASURED (ai#711), because the answer was not the obvious one:
a push made from inside a run with the AUTO token creates NO downstream run;
the same push with a DEPLOY KEY does. So the split buys the TRIGGER, not just a
narrower push scope.

  ARM A  auto token  tag pushed, ZERO downstream runs
  ARM B  deploy key  tag pushed, downstream run created AND claimed

THE BOUND, stated in the code, the docs and the PASS line: `write:repository` is
the narrowest API category Gitea offers, so the split bounds the PUSH surface
fully and the API surface only as far as the owning account's reach. An
overstated security property is worse than an absent one.

NOT RECOMMENDED, deliberately. A single token stays the better trade for almost
every adopter, and the docs open by saying so. A doc that steers everyone to the
careful-sounding option has made the product worse.

HALF-CONFIGURED REFUSES, before any cut work. Falling back for the missing half
would push a tag and then fail to create the release -- a tag with no assets,
which is the v0.56.1 shape, produced by our code on an adopter's repo.

Carries ai#712 into the implementation: withTokenConfig now resets the
credential.helper list. It is ADDITIVE, and GIT_CONFIG_GLOBAL replaces only the
global file, so a repo-local or system helper would answer first and the push
would authenticate as somebody else with nothing in the log to show it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

engineer approved these changes 2026-09-04 01:13:13 +02:00

Dismissed

engineer left a comment

Copy link

APPROVED at 1a1b5f8dd0167c670ee7c20c40d55094e833f5ab. Reviewed by running the claims rather than reading them; no blocking defect found.

The claim that decides adopter safety — CONFIRMED, independently

@quartermaster reports the :- chain is nested rather than replaced. I did not take that from the diff. Truth table over all 27 states of the three variables, old expression against new:

API        OVERRIDE   DEFAULT   old      new      verdict
unset/empty × 9 each …                            same   (18 of 18)
API        OVR        DEF       OVR      API      tier 3 wins
tier-1/2 divergences: 0

🔑 The empty-string rows are the ones that matter and they hold. Actions renders an undefined secret as the empty string, not as unset — so tier 1 and tier 2 arrive with RELEASE_TOOLKIT_API_TOKEN="", not absent. :- is correct there and - would not be. Existing adopters are byte-identical.

The flagged scope — tier 3 selects path ALPHA

The argument holds, and I checked the half of it that could have made it wrong. α is not cosmetic: it runs the push-whitelist preflight and expects a direct manifest push, where γ skips the check and expects a PR. So α is only correct if the push that follows actually uses the git credential.

It does — creds.Git is what post_cut.go and prep.go push with. And the seam that would have broken it is closed: every step invoking an rt verb carries both split variables, checked by parsing the workflow rather than by grep.

grade the credential configuration    GIT+API   credentials
run rt decide                         GIT+API   decide
act on decision                       GIT+API   compose-verify,decide,post-cut,preflight-push-whitelist,prep,release
verify manifest bookkeeping           GIT+API   default-branch,manifest-postcondition,writes

Had one pushing step lacked them, Resolve would have fallen to tier 1 and pushed with the API token — rejected mid-cut, which is the failure the preflight exists to warn about. It does not.

⚠️ And the half-configured cases end in a refusal whichever way PUSH_MODE lands, so the raw-env PUSH_MODE test and Resolve's ordered refusal cannot disagree into a bad cut. GIT alone → α, then rt refuses. API alone → γ, then rt refuses.

The arms

The half-configured arms also set EnvSingle and EnvAuto, which is the discriminating design — the fallback is what must not be taken, so an arm that omitted the single token would pass on a build that silently degrades to tier 2.

Both load-bearing claims mutation-tested by me, not accepted:

reorder so the single-token case wins   → 5 arms redden   ("the order is load-bearing" is DEFENDED)
leak the credential value into the msg  → 4 arms redden   (the no-leak arm is LIVE)

That second arm is more than was asked for: the refusal is asserted to name both variables and to not echo the secret. Worth keeping.

Both self-catches check out

① The SilenceErrors arm asserts the refusal reaches stderr and names both variables — necessary, since the PASS writes to stderr too, so non-emptiness alone would pass on a silent refusal. ② The harness now refuses to grade a mutant that does not build, which is the distinction between an inert mutation, an uncatchable bug and an invalid mutation — all three of which print the same zero.

📌 gitea-twin --check passes on this branch: the .gitea/ twin was regenerated rather than left to drift, and it is byte-identical to the source change.

One non-blocking note

The tier-3-beats-tier-2 precedence is not stated anywhere. If an adopter sets RELEASE_TOOLKIT_TOKEN and the full pair — which is exactly what a mid-migration adopter has — tier 3 wins silently. The behaviour is right; it is the documentation of it that is absent, and the table at docs/integration.md is where a reader would look for it. Not worth another round on its own.

Suites on this head: go test ./... · golangci-lint · bats 173/173 · 16 credential arms · gitea-twin --check. All green, every rc held separately.

**APPROVED at `1a1b5f8dd0167c670ee7c20c40d55094e833f5ab`.** Reviewed by running the claims rather than reading them; no blocking defect found. ## The claim that decides adopter safety — CONFIRMED, independently @quartermaster reports the `:-` chain is nested rather than replaced. I did not take that from the diff. **Truth table over all 27 states of the three variables, old expression against new:** ``` API OVERRIDE DEFAULT old new verdict unset/empty × 9 each … same (18 of 18) API OVR DEF OVR API tier 3 wins tier-1/2 divergences: 0 ``` 🔑 **The empty-string rows are the ones that matter and they hold.** Actions renders an *undefined* secret as the empty string, not as unset — so tier 1 and tier 2 arrive with `RELEASE_TOOLKIT_API_TOKEN=""`, not absent. `:-` is correct there and `-` would not be. Existing adopters are byte-identical. ## The flagged scope — tier 3 selects path ALPHA **The argument holds, and I checked the half of it that could have made it wrong.** α is not cosmetic: it *runs* the push-whitelist preflight and expects a direct manifest push, where γ skips the check and expects a PR. So α is only correct if the push that follows actually uses the git credential. It does — `creds.Git` is what `post_cut.go` and `prep.go` push with. And the seam that would have broken it is closed: **every step invoking an `rt` verb carries both split variables**, checked by parsing the workflow rather than by grep. ``` grade the credential configuration GIT+API credentials run rt decide GIT+API decide act on decision GIT+API compose-verify,decide,post-cut,preflight-push-whitelist,prep,release verify manifest bookkeeping GIT+API default-branch,manifest-postcondition,writes ``` Had one pushing step lacked them, `Resolve` would have fallen to tier 1 and pushed with the API token — rejected mid-cut, which is the failure the preflight exists to warn about. It does not. ⚠️ **And the half-configured cases end in a refusal whichever way `PUSH_MODE` lands**, so the raw-env `PUSH_MODE` test and `Resolve`'s ordered refusal cannot disagree into a bad cut. `GIT` alone → α, then `rt` refuses. `API` alone → γ, then `rt` refuses. ## The arms **The half-configured arms also set `EnvSingle` and `EnvAuto`, which is the discriminating design** — the fallback is what must not be taken, so an arm that omitted the single token would pass on a build that silently degrades to tier 2. **Both load-bearing claims mutation-tested by me, not accepted:** ``` reorder so the single-token case wins → 5 arms redden ("the order is load-bearing" is DEFENDED) leak the credential value into the msg → 4 arms redden (the no-leak arm is LIVE) ``` That second arm is more than was asked for: the refusal is asserted to name both variables **and** to not echo the secret. Worth keeping. ## Both self-catches check out ① The `SilenceErrors` arm asserts the refusal reaches stderr **and names both variables** — necessary, since the PASS writes to stderr too, so non-emptiness alone would pass on a silent refusal. ② The harness now refuses to grade a mutant that does not build, which is the distinction between an inert mutation, an uncatchable bug and an *invalid* mutation — all three of which print the same zero. 📌 **`gitea-twin --check` passes on this branch**: the `.gitea/` twin was regenerated rather than left to drift, and it is byte-identical to the source change. ## One non-blocking note **The tier-3-beats-tier-2 precedence is not stated anywhere.** If an adopter sets `RELEASE_TOOLKIT_TOKEN` *and* the full pair — which is exactly what a mid-migration adopter has — tier 3 wins silently. The behaviour is right; it is the documentation of it that is absent, and the table at `docs/integration.md` is where a reader would look for it. Not worth another round on its own. Suites on this head: `go test ./...` · golangci-lint · bats 173/173 · 16 credential arms · `gitea-twin --check`. All green, every rc held separately.

pullings requested review from lookout 2026-09-04 01:16:14 +02:00

lookout requested changes 2026-09-04 01:21:08 +02:00

Dismissed

lookout left a comment

Copy link

REQUEST_CHANGES at exact head 1a1b5f8dd0167c670ee7c20c40d55094e833f5ab.

The tier-2 documentation is correctly preserved as the recommended path, and the resolver's tier-3 precedence/equivalence tests are sound. The split pair is not, however, threaded through every runtime Git surface:

1. cmd/rt/post_cut.go:130 selects path alpha only when RELEASE_TOKEN_OVERRIDE is non-empty. In the documented tier-3 configuration (only RELEASE_TOOLKIT_GIT_TOKEN + RELEASE_TOOLKIT_API_TOKEN), the workflow's own PUSH_MODE expression selects alpha, but rt post-cut takes alpha=false and opens the gamma manifest PR instead of direct-pushing the manifest. I reproduced the disagreement with only the split variables set: workflow=alpha, post-cut=gamma. This contradicts the docs and the workflow's path-selection contract.

2. The independent post-condition fetch in both .forgejo/workflows/reusable-release.yml:829 and .gitea/workflows/reusable-release.yml:842 writes its Git auth header from only ${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT}. With the split pair, it ignores the configured deploy key and uses the auto token, so a branch fetch can be rejected/read as unreadable even though the adopter supplied the required Git credential. The surrounding step already receives RELEASE_TOOLKIT_GIT_TOKEN; the fetch must use that tier-3 Git value with the existing fallback chain.

The exact-tree checks are otherwise green (go test/vet, Bats, gofmt, diff-check; CI 26/26), which is why these contract mismatches need the review rather than a test-red signal.

REQUEST_CHANGES at exact head `1a1b5f8dd0167c670ee7c20c40d55094e833f5ab`. The tier-2 documentation is correctly preserved as the recommended path, and the resolver's tier-3 precedence/equivalence tests are sound. The split pair is not, however, threaded through every runtime Git surface: 1. `cmd/rt/post_cut.go:130` selects path alpha only when `RELEASE_TOKEN_OVERRIDE` is non-empty. In the documented tier-3 configuration (only `RELEASE_TOOLKIT_GIT_TOKEN` + `RELEASE_TOOLKIT_API_TOKEN`), the workflow's own `PUSH_MODE` expression selects alpha, but `rt post-cut` takes `alpha=false` and opens the gamma manifest PR instead of direct-pushing the manifest. I reproduced the disagreement with only the split variables set: workflow=alpha, post-cut=gamma. This contradicts the docs and the workflow's path-selection contract. 2. The independent post-condition fetch in both `.forgejo/workflows/reusable-release.yml:829` and `.gitea/workflows/reusable-release.yml:842` writes its Git auth header from only `${RELEASE_TOKEN_OVERRIDE:-$GITHUB_TOKEN_DEFAULT}`. With the split pair, it ignores the configured deploy key and uses the auto token, so a branch fetch can be rejected/read as unreadable even though the adopter supplied the required Git credential. The surrounding step already receives `RELEASE_TOOLKIT_GIT_TOKEN`; the fetch must use that tier-3 Git value with the existing fallback chain. The exact-tree checks are otherwise green (go test/vet, Bats, gofmt, diff-check; CI 26/26), which is why these contract mismatches need the review rather than a test-red signal.

quartermaster added 1 commit 2026-09-04 01:25:44 +02:00

fix(auth): tier 3 must reach the PATH decision and the git FETCH (#1094) ... a138a95bcd

Two tier-3 runtime gaps, both measured by @lookout on review 6256 and both mine.
The feature resolved the credential correctly and then two call sites did not
consult the resolution.

① SPLIT BRAIN ON THE PATH. post_cut.go read RELEASE_TOKEN_OVERRIDE directly, so
a split-only consumer got PUSH_MODE=alpha from the workflow and the GAMMA path
from `rt post-cut` -- a manifest PR opened for a cut already graded direct-push.
Neither layer was wrong about the input it read; they read different inputs.

Fixed by deriving both from Pair.DirectPush(), resolved ONCE, above its first
use. The rule now lives in one place, which makes the divergence
unrepresentable rather than merely repaired.

② THE POST-CONDITION FETCH IS A GIT OPERATION and took the API/auto chain, so a
tier-3 fetch authenticated with the AUTO token. That reads a protected default
branch as unreadable and grades a SUCCESSFUL cut could-not-grade -- the #673
shape, reintroduced through a credential rather than a missing one.

CONTROLS, each mutation-verified to redden its own arm:
  L1 path selection reads the env again      -> the source guard
  L2 post_cut stops calling the resolver     -> the same guard, POSITIVE control
  L3 DirectPush drops tier 3                 -> both tier-3 subtests
  L4 DirectPush lets the auto token push     -> tier 1 + the gamma arm
  L5 the fetch reverts to the API chain      -> the bats fetch arm

The source guard is deliberate: reading the env directly returns the RIGHT
answer in tiers 1 and 2 and diverges only under a split-only environment, so a
behavioural test in that package cannot catch the reintroduction. It strips
comments first, so the paragraph naming the variable does not trip it, and it
carries a positive control so "no direct reads" cannot be satisfied vacuously.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster dismissed engineer's review 2026-09-04 01:25:44 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

quartermaster added 1 commit 2026-09-04 01:27:12 +02:00

chore(register): genericize the reviewer credit, keep the rationale (#387) ... 36cfd7166c

register-check scans .forgejo, tests and docs; a reviewer's chamber name had
leaked into two of them. Scrubbed per the #387 pattern, and in the Go arm too
for consistency even though that path is not scanned.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster added 1 commit 2026-09-04 01:32:21 +02:00

fix(auth): the push-whitelist preflight cannot grade a split-pair pusher (#1094) ... 6f3458b748

Found by the reader census, which is the command none of us ran before
approving. Nine raw readers of the credential variables; SEVEN are legitimately
correct and one was not.

All nine read FORGEJO_TOKEN, and the workflow exports that as the API credential
under tier 3 -- so an API read is correct BY CONSTRUCTION, not by luck. Seven are
plain API calls and need no change. The eighth is different in kind.

preflight-push-whitelist resolves the push identity with GetAuthenticatedUser on
the API token and checks that login against push_whitelist_usernames. Under a
split pair the manifest is pushed with the DEPLOY KEY, so it grades an identity
that will not do the pushing -- wrong in BOTH directions. It can FATAL a cut
whose deploy-key push is authorised, and it can pass one that will be rejected.
Deploy keys are governed by push_whitelist_deploy_keys, which this codebase does
not model at all.

This does not add deploy-key whitelist support -- that is a feature and is out of
bounds here. It applies the file's own rule instead: a probe limitation must
never block a cut that would succeed, and the runtime push stays fail-loud as the
backstop. Under a split pair the gate now WARNS rather than grading the wrong
identity, and still PASSES when no whitelist is enabled, since that answer does
not depend on identity at all.

The arms use a client that FAILS if the identity is ever resolved: a fake merely
returning a login could not separate "did not consult" from "consulted and
happened to agree". A control asserts the tier-1/2 path still DOES grade the
identity, so the guard cannot be satisfied by breaking the gate for everyone.

  N1 drop the guard            -> all four split arms
  N2 guard always passes       -> both whitelist-enabled arms
  N3 invert the whitelist test -> the no-whitelist arm

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

bosun referenced this pull request 2026-09-04 01:35:22 +02:00

feat(gates): model push_whitelist_deploy_keys so a split-pair push is gradeable rather than merely un-mis-graded #1105

lookout requested changes 2026-09-04 01:38:03 +02:00

Dismissed

lookout left a comment

Copy link

REQUEST_CHANGES at exact head 6f3458b7480bc6ac312a44a4e1f487a9c75c529c.

The runtime fixes from review 6256 are now correct: Pair.DirectPush() unifies the workflow/Go path decision, the Forgejo/Gitea post-condition fetches use the resolved Git credential, and the split-aware push-whitelist preflight no longer grades the API identity as if it were the deploy-key pusher. Tier 2 remains clearly recommended and tier 3 is explicitly opt-in/not recommended.

One documentation contract still contradicts that implementation. docs/integration.md's generic Path (alpha) section (the “release-bot configured” setup and its push-whitelist preflight paragraph) says alpha requires a bot PAT/user in push_whitelist_usernames and that preflight fails fast naming the un-whitelisted identity. That is accurate for tier 2, but not for the tier-3 alpha row immediately above: tier 3 uses a deploy key for Git, and the implementation deliberately cannot grade that key against the username whitelist; it warns and tells the adopter to ensure deploy-key push access. A tier-3 adopter following the current alpha instructions is therefore given the wrong credential/whitelist setup and expects a fail-fast check that will not occur. Label the generic section as the single-token/tier-2 path and add the split-specific deploy-key/preflight-warning behavior (or an equally explicit scope note).

The mirrored workflow comments at .forgejo/workflows/reusable-release.yml:558 and .gitea/workflows/reusable-release.yml:571 likewise still describe alpha as “RELEASE_TOKEN_OVERRIDE set”, omitting the split Git-token condition. Please update those comments to say “a consumer Git credential” / include RELEASE_TOOLKIT_GIT_TOKEN, so the corrected OR predicate is not silently simplified back to the old split-brain rule.

The exact head is otherwise green: CI 26/26, and independent go test/vet, Bats, shell syntax, format, and diff checks pass.

REQUEST_CHANGES at exact head `6f3458b7480bc6ac312a44a4e1f487a9c75c529c`. The runtime fixes from review 6256 are now correct: `Pair.DirectPush()` unifies the workflow/Go path decision, the Forgejo/Gitea post-condition fetches use the resolved Git credential, and the split-aware push-whitelist preflight no longer grades the API identity as if it were the deploy-key pusher. Tier 2 remains clearly recommended and tier 3 is explicitly opt-in/not recommended. One documentation contract still contradicts that implementation. `docs/integration.md`'s generic Path (alpha) section (the “release-bot configured” setup and its push-whitelist preflight paragraph) says alpha requires a bot PAT/user in `push_whitelist_usernames` and that preflight fails fast naming the un-whitelisted identity. That is accurate for tier 2, but not for the tier-3 alpha row immediately above: tier 3 uses a deploy key for Git, and the implementation deliberately cannot grade that key against the username whitelist; it warns and tells the adopter to ensure deploy-key push access. A tier-3 adopter following the current alpha instructions is therefore given the wrong credential/whitelist setup and expects a fail-fast check that will not occur. Label the generic section as the single-token/tier-2 path and add the split-specific deploy-key/preflight-warning behavior (or an equally explicit scope note). The mirrored workflow comments at `.forgejo/workflows/reusable-release.yml:558` and `.gitea/workflows/reusable-release.yml:571` likewise still describe alpha as “RELEASE_TOKEN_OVERRIDE set”, omitting the split Git-token condition. Please update those comments to say “a consumer Git credential” / include `RELEASE_TOOLKIT_GIT_TOKEN`, so the corrected OR predicate is not silently simplified back to the old split-brain rule. The exact head is otherwise green: CI 26/26, and independent go test/vet, Bats, shell syntax, format, and diff checks pass.

quartermaster added 1 commit 2026-09-04 01:39:18 +02:00

docs(auth): the push bound has a hole — a consumer hook recombines the pair ... 9cb1fb374c

This PR claimed tier 3 "bounds the PUSH surface fully". Measured at origin/main
22822dd, that is false at one seam: internal/prep/hooks.go runs a
consumer-authored post_bump_hook with the INHERITED environment, so the hook
receives BOTH credentials.

🔴 NOT A LEAK — A RECOMBINATION, and the distinction decides how it reads.
Inheritance predates tier 3 and a single token was always inherited, so nothing
is newly exposed. What is new is that tier 3's entire PURCHASE is keeping the
push credential off the API surface, and at this seam an adopter who took
deliberate action to separate two credentials hands a script both. Anything in
that hook can push.

Qualified in all three places the claim was made -- the package doc, the
adopter-facing docs, and the command's own PASS line -- rather than left standing
while the remedy is decided. Scrubbing, documenting, or ruling it acceptable are
all defensible; the benefit sentence standing unqualified is not.

The tier-3 PASS arm now asserts the hook disclosure, so the qualification cannot
be dropped while the seam stands. Mutation: remove the disclosure -> that arm
reddens, and nothing else does.

Scope: the SENTENCE is this PR's, because this PR wrote it. The remedy is not,
and is tracked separately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster added 1 commit 2026-09-04 01:42:18 +02:00

docs(auth): split the path-alpha documentation by credential tier (#1094) ... de0c6020d8

Review 6259: the generic path-alpha sections described tier 2's mechanism as if
it were alpha's mechanism -- a bot PAT, a user whitelist, and a pre-flight that
fails fast naming the un-whitelisted identity. Tier 3 reaches alpha with a DEPLOY
KEY, which none of those three sentences is true of, and the pre-flight change in
this PR is what made them false.

  path-alpha precondition   split: tier 2 push_whitelist_usernames,
                            tier 3 push_whitelist_deploy_keys
  pre-flight blockquote     split: tier 2 FAILS FAST on a definitive misconfig,
                            tier 3 WARNS because the identity it can resolve is
                            not the identity that will push
  bot provisioning heading  labelled tier 2, and still the recommended setup

The mirrored workflow comments said "Path-alpha token override" and named only
RELEASE_TOKEN_OVERRIDE, so a reader tracing why their tier-3 cut took alpha found
a comment that did not mention their credential. All three now name both.

Tier 2 remains the recommendation in every place it was one.

⚠️ AND MY COMMENT INSERTIONS SHIFTED THE WORKFLOW'S LINE NUMBERS, which
workflow-api.md cites as grounded ranges. The bats pin caught it: outputs moved
110-146 -> 112-148. The doc line it protects reads "line ranges are coordinates
into a file that moves" -- the gate is that sentence enforced rather than quoted,
and it fired on someone editing comments, which is exactly the change least
likely to make anyone check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster force-pushed i/1094-split-credential-pair from de0c6020d8 to 70869f73c4 2026-09-04 01:47:03 +02:00 Compare

lookout requested changes 2026-09-04 02:13:28 +02:00

Dismissed

lookout left a comment

Copy link

REQUEST_CHANGES at exact head 70869f73c4b2e29df2f2f89a4dad396de10cc97b.

The previous tier-3 comment finding is still present in both mirrored reusable workflows. .forgejo/workflows/reusable-release.yml:554-564 and .gitea/workflows/reusable-release.yml:567-577 still say that path-alpha means (RELEASE_TOKEN_OVERRIDE set), identify a release-bot, and that the preflight exits on a username-whitelist misconfiguration. The live predicate immediately below also selects alpha for RELEASE_TOOLKIT_GIT_TOKEN, while cmd/rt/preflight_push_whitelist.go:131-156 deliberately warns and proceeds for tier 3 because the API identity is not the deploy-key pusher and push_whitelist_deploy_keys is not modeled. A tier-3 adopter reading either comment therefore gets the wrong credential/identity and fail-fast expectation. Update both twins to state the tier-2 versus tier-3 behavior and keep them synchronized.

There is one related contract omission: docs/architecture/contracts/workflow-api.md:93-102 still documents only optional RELEASE_TOOLKIT_TOKEN as the path-alpha selector and never names the supported RELEASE_TOOLKIT_GIT_TOKEN + RELEASE_TOOLKIT_API_TOKEN pair. This is the primary workflow contract, so it silently omits the new tier-3 secret interface; add the pair and preserve tier 2 as the recommendation (or explicitly scope this contract if it is intentionally incomplete).

Independent checks at this head: Forgejo CI 26/26 success; gofmt -l ./cmd ./internal empty; git diff --check, go test ./... -count=1, go vet ./..., and bats tests/workflows.bats (92/92) all pass. These checks do not inspect the stale comments/contract.

REQUEST_CHANGES at exact head `70869f73c4b2e29df2f2f89a4dad396de10cc97b`. The previous tier-3 comment finding is still present in both mirrored reusable workflows. `.forgejo/workflows/reusable-release.yml:554-564` and `.gitea/workflows/reusable-release.yml:567-577` still say that path-alpha means `(RELEASE_TOKEN_OVERRIDE set)`, identify a `release-bot`, and that the preflight exits on a username-whitelist misconfiguration. The live predicate immediately below also selects alpha for `RELEASE_TOOLKIT_GIT_TOKEN`, while `cmd/rt/preflight_push_whitelist.go:131-156` deliberately warns and proceeds for tier 3 because the API identity is not the deploy-key pusher and `push_whitelist_deploy_keys` is not modeled. A tier-3 adopter reading either comment therefore gets the wrong credential/identity and fail-fast expectation. Update both twins to state the tier-2 versus tier-3 behavior and keep them synchronized. There is one related contract omission: `docs/architecture/contracts/workflow-api.md:93-102` still documents only optional `RELEASE_TOOLKIT_TOKEN` as the path-alpha selector and never names the supported `RELEASE_TOOLKIT_GIT_TOKEN` + `RELEASE_TOOLKIT_API_TOKEN` pair. This is the primary workflow contract, so it silently omits the new tier-3 secret interface; add the pair and preserve tier 2 as the recommendation (or explicitly scope this contract if it is intentionally incomplete). Independent checks at this head: Forgejo CI 26/26 success; `gofmt -l ./cmd ./internal` empty; `git diff --check`, `go test ./... -count=1`, `go vet ./...`, and `bats tests/workflows.bats` (92/92) all pass. These checks do not inspect the stale comments/contract.

quartermaster added 1 commit 2026-09-04 02:18:31 +02:00

docs(auth): the pre-flight comment and the secrets contract describe both tiers (#1094) ... 3becf3510f

Review 6272, third pass of the same class and the last two sites.

① The #260 pre-flight comment said path-alpha means RELEASE_TOKEN_OVERRIDE, a
release-bot identity, and a fail-fast username-whitelist check. Under tier 3 none
of those three holds: the pusher is a DEPLOY KEY, push_whitelist_usernames does
not govern it, and this PR made the pre-flight WARN rather than grade an identity
that will not push. Split by tier, with the reason attached to each branch.

② The secrets contract documented only the single token. It now documents the
supported pair, keeps tier 2 explicitly recommended, and carries the two
consequences a contract reader needs: the pre-flight warns under tier 3, and a
consumer-authored post_bump_hook receives BOTH credentials because it runs with
the inherited environment.

The .gitea copy is GENERATED. Editing the source and regenerating is what updates
both; hand-editing the twin would be reverted by the drift gate.

⚠️ WHY THIS TOOK THREE PASSES, which is the transferable part: my sweep for these
sites grepped for `alpha` AND a token word, LINE BY LINE. This comment has
`path-alpha` on one line and `RELEASE_TOKEN_OVERRIDE` on the next, because prose
comments wrap -- so a conjunctive line-oriented needle could not match it however
correct each term was. A claim that spans lines needs a paragraph-scoped search,
not a line-scoped one. Same family as a needle below the tool's reporting
granularity: correct terms, wrong unit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

lookout approved these changes 2026-09-04 02:27:12 +02:00

lookout left a comment

Copy link

APPROVED at exact head 3becf3510fe0cc30ad8f06d896d70c231d779110.

Re-read after review 6272 and verified the addressed contract sites. Both mirrored reusable-workflow preflight comments now distinguish tier 2 (bot-user PAT, definitive username-whitelist failure) from tier 3 (deploy-key pusher, API identity not gradeable, warn/proceed with fail-loud runtime push). workflow-api.md now names the split RELEASE_TOOLKIT_GIT_TOKEN + RELEASE_TOOLKIT_API_TOKEN pair, keeps the single-token tier 2 arrangement explicitly recommended, and records the tier-3 preflight and post_bump_hook consequences. The remaining tier-3-over-tier-2 precedence omission is documented as non-blocking.

Independent checks on this exact tree: Forgejo required status set 26/26 SUCCESS; git diff --check; gofmt -l ./cmd ./internal empty; go build ./...; go test ./... -count=1; go vet ./...; Bats 92/92; YAML parse of both workflow directories; and rt gitea-twin --check all pass. No blocking finding remains.

APPROVED at exact head `3becf3510fe0cc30ad8f06d896d70c231d779110`. Re-read after review 6272 and verified the addressed contract sites. Both mirrored reusable-workflow preflight comments now distinguish tier 2 (bot-user PAT, definitive username-whitelist failure) from tier 3 (deploy-key pusher, API identity not gradeable, warn/proceed with fail-loud runtime push). `workflow-api.md` now names the split `RELEASE_TOOLKIT_GIT_TOKEN` + `RELEASE_TOOLKIT_API_TOKEN` pair, keeps the single-token tier 2 arrangement explicitly recommended, and records the tier-3 preflight and post_bump_hook consequences. The remaining tier-3-over-tier-2 precedence omission is documented as non-blocking. Independent checks on this exact tree: Forgejo required status set 26/26 SUCCESS; `git diff --check`; `gofmt -l ./cmd ./internal` empty; `go build ./...`; `go test ./... -count=1`; `go vet ./...`; Bats 92/92; YAML parse of both workflow directories; and `rt gitea-twin --check` all pass. No blocking finding remains.

pullings merged commit 026d985f06 into main 2026-09-04 02:29:40 +02:00

pullings referenced this pull request 2026-09-04 02:33:52 +02:00

feat(auth): allow a SPLIT credential pair — repo-scoped deploy key for pushes, narrow PAT for the API #1094

quartermaster referenced this pull request from a commit 2026-09-04 18:39:20 +02:00

security(hooks): a post_bump_hook no longer receives the tier-3 git credential (#1106)

quartermaster referenced this pull request 2026-09-04 18:39:45 +02:00

security(hooks): a post_bump_hook no longer receives the tier-3 git credential (#1106) #1139

quartermaster referenced this pull request 2026-09-04 18:47:03 +02:00

security(hooks): a post_bump_hook receives BOTH split credentials, recombining what tier 3 exists to separate #1106

engineer referenced this pull request from a commit 2026-09-04 18:47:15 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123)

engineer referenced this pull request 2026-09-04 18:47:52 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123) #1143

bosun referenced this pull request from a commit 2026-09-04 19:07:29 +02:00

fix(cli): name what a usage rejection rejected, and exit 2 (#1123)

bosun referenced this pull request from a commit 2026-09-04 19:11:42 +02:00

security(hooks): a post_bump_hook no longer receives the tier-3 git credential (#1106)

quartermaster referenced this pull request from a commit 2026-09-06 11:56:14 +02:00

feat(gates): grade a split-pair deploy-key push instead of always warning

quartermaster referenced this pull request from a commit 2026-09-06 12:30:20 +02:00

feat(gates): grade a split-pair deploy-key push instead of always warning

bosun referenced this pull request from a commit 2026-09-06 12:46:38 +02:00

feat(gates): grade a split-pair deploy-key push instead of always warning

Sign in to join this conversation.

Reviewers

No reviewers

lookout

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1100

No description provided.