| 1 | # Review brief: the deploy credential spec |
| 2 | |
| 3 | You are Astra reviewing a design spec independently of its author. Your |
| 4 | output is a findings report, not an edit. Do not modify any file. |
| 5 | |
| 6 | ## What to read |
| 7 | |
| 8 | 1. The spec under review: |
| 9 | `docs/superpowers/specs/2026-10-05-deploy-credential-design.md`. |
| 10 | §3's rulings R1 to R4 are fixed. Report a mechanism that fails one. Do |
| 11 | not re-argue one. |
| 12 | 2. The code it fits into, in this worktree: |
| 13 | - `internal/svcrole/svcrole.go` (roles, `Allow`, `Permits`); |
| 14 | - `internal/console/authcli.go` (`cliRoute` near 411, the route table |
| 15 | from 470 to 740, how a principal is resolved from a bearer token); |
| 16 | - `internal/console/svcprincipal.go` (`Principal`, |
| 17 | `NewServicePrincipal`, rotate, revoke) and |
| 18 | `internal/console/authstore.go` (the `cli_tokens` schema near 183 to |
| 19 | 255); |
| 20 | - `internal/console/apicli.go` (`apiAccountAppBy` near 58, |
| 21 | `handleAPIPromote` near 701, `handleAPIRollback` near 970, |
| 22 | `handleAPIShip` near 1569); |
| 23 | - `internal/console/controlplane.go` (`refuseServiceControlPlane`); |
| 24 | - `internal/console/apiservices.go` and `internal/console/svcpage.go` |
| 25 | (the two mint doors); |
| 26 | - `internal/release/ladder.go`, `grant.go`, `signer.go`, `pipeline.go`, |
| 27 | `serial.go`; |
| 28 | - the CLI at the repo root: `apimode.go` (`resolveMode`), `auth.go`, |
| 29 | `services.go`, `canary.go`, and the promote, rollback and ship verbs |
| 30 | in `cli.go`; |
| 31 | - `README.md` lines 100 to 185, and `docs/site/cli.md` around 1319 to |
| 32 | 1440. |
| 33 | |
| 34 | ## What matters most |
| 35 | |
| 36 | - **Escapes from the rung scope.** Look for any way a `deploy` credential |
| 37 | moves a pointer outside its rungs: a channel name `release.Rung` maps |
| 38 | unexpectedly, the proposal path, a pending approval it can create or |
| 39 | execute, rollback's own paths, the hotfix flag, a pipelined app whose |
| 40 | pipeline names a rung, a version-target or tag door, or any other door |
| 41 | on the §6 list that writes more than the spec says. Check §7.1's order |
| 42 | and §7.2's placement against the handlers as written. |
| 43 | - **Doors.** Is §6's list complete for `carlos ship`, `promote`, |
| 44 | `rollback` and `channels` in API mode as the CLI actually calls them? |
| 45 | Does any door on the list expose or change more than a deploy pipeline |
| 46 | needs? Is anything missing that makes the CLI fail? |
| 47 | - **Mint and read-back (§5).** Can either mint door bypass the rules? Can |
| 48 | rotate, or any other write to `cli_tokens`, produce a `deploy` row with |
| 49 | no rungs or a non-`deploy` row with rungs? Is fail-closed at resolve |
| 50 | achievable where the spec puts it? |
| 51 | - **Mixed versions (§9).** Is "an old console refuses an unknown role at |
| 52 | every door" true of `cliRoute` as written? Is there any path that |
| 53 | resolves a service token without going through `cliRoute`, or treats an |
| 54 | unknown role as something permissive? |
| 55 | - **The CLI in CI (§8).** Does reading `CARLOS_SERVICE_TOKEN` in |
| 56 | `resolveMode` collide with anything: the in-instance variable of the |
| 57 | same name, box-side callers, `carlos auth` verbs, the `--owner` path? |
| 58 | Is the refusal beside a store variable consistent with the existing |
| 59 | `--console` ruling? |
| 60 | - **§7.4's trade.** The limit moves from a grant every box verifies to a |
| 61 | check in the console. Is the exposure stated honestly? Is there a cheap |
| 62 | way to keep a box-side check that the spec missed, without breaking R3 |
| 63 | or R4? |
| 64 | - **Claims about the code.** Every `file:line` and every "already does" |
| 65 | claim: check it at this commit. Report each one that is wrong. |
| 66 | |
| 67 | ## How to report |
| 68 | |
| 69 | Number findings R1-1 onward. For each: severity (P1 breaks a trust |
| 70 | guarantee or makes the design unbuildable; P2 a contract or claim defect |
| 71 | to fix before the plan; P3 advisory), the section, the problem, the |
| 72 | evidence with citations, a proposed correction. End with a verdict: |
| 73 | accept, accept with corrections, or request changes. Do not soften, do |
| 74 | not pad, do not restate the spec. |
| 75 | |
| 76 | ## Round 2 |
| 77 | |
| 78 | Rev 2 answers your round 1 (`CODEX-FINDINGS-DEPLOY-CREDENTIAL-R1.md`); §15 |
| 79 | lists each disposition. Read rev 2 cold, then check: |
| 80 | |
| 81 | - Does each accepted finding's fix actually close it, as written? |
| 82 | - §7.2: is "one snapshot, equivalent to promote-then-install" true of |
| 83 | `PromoteOnLadder` and `Signer.Rollback` as they are? Is there any state a |
| 84 | `deploy` credential can produce that no serial order produces? Is |
| 85 | declining full serialization defensible, or does it leave R2 open? |
| 86 | - §7.4: can the console mint and use a request-local grant the way the |
| 87 | spec says, with `MintKeyGrant`, `ScopedSigner` and `verifyPointer` as |
| 88 | written? Check the minted-at lower bound, the five-minute lifetime |
| 89 | against `promoted_at`, history and tag writes that a scoped signer makes |
| 90 | or cannot make, and whether any box-side verifier rejects a grant it has |
| 91 | not seen before. |
| 92 | - Anything the rev 2 changes broke that rev 1 had right. |
| 93 | |
| 94 | Number findings R2-1 onward. Same severities, same verdict line. |
| 95 | |
| 96 | ## Round 3 |
| 97 | |
| 98 | Rev 3 answers your round 2 (`CODEX-FINDINGS-DEPLOY-CREDENTIAL-R2.md`); §15 |
| 99 | lists each disposition. R2-1 is accepted in part: the serializability |
| 100 | claim is withdrawn, the hook moves between the pointer read and the |
| 101 | pointer write, and §7.2 states invariant I1 and one window it does not |
| 102 | cover. Whether to close that window is put to Paul in §16 and is not |
| 103 | yours to rule on. What is yours: |
| 104 | |
| 105 | - **Is I1 true** of the mechanism as written, for `Signer.Promote` and |
| 106 | `Signer.Rollback` as they are in this worktree? Give an interleaving |
| 107 | that breaks (a) or (b) if one exists. |
| 108 | - **Is the uncovered window stated completely?** Give any outcome a |
| 109 | `deploy` credential can produce that is neither forbidden by I1 nor |
| 110 | inside the window §7.2 names. Consider first promotes (no pointer, a |
| 111 | create-only write), the no-op path, rollback's history, the handler's |
| 112 | proposal path, and conflict handling. |
| 113 | - Does the hook's placement change behaviour for a caller that passes no |
| 114 | hook? |
| 115 | - Anything rev 3 broke that rev 2 had right, and any wrong citation. |
| 116 | |
| 117 | Number findings R3-1 onward. Same severities, same verdict line. |
| 118 | |
| 119 | ## Round 4 |
| 120 | |
| 121 | Rev 4 answers your round 3 (`CODEX-FINDINGS-DEPLOY-CREDENTIAL-R3.md`); §15 |
| 122 | lists each disposition. I1 is gone. §7.2 now claims two properties, G1 and |
| 123 | G2, and one sentence for what they leave open. Whether to close that gap |
| 124 | is §16, Paul's decision, and not yours to rule on. What is yours: |
| 125 | |
| 126 | - **Are G1 and G2 true** of the mechanism as written, for a first promote, |
| 127 | a replacing promote, a repeat promote and a rollback, against |
| 128 | `DirStore` and `S3Store` as they are? Give an interleaving that breaks |
| 129 | either. |
| 130 | - **Is the open sentence complete?** Give any outcome a `deploy` |
| 131 | credential can produce that G1 and G2 do not forbid and that sentence |
| 132 | does not describe. A new member of the class the sentence already |
| 133 | describes is not a finding; say so if that is all you find. |
| 134 | - Are the three hook call points consistent with each other and with the |
| 135 | handler's own check? Can any path reach a write, a proposal or a |
| 136 | success response for a `deploy` credential without the hook having run |
| 137 | on a pipeline read at least as recent as the spec says? |
| 138 | - Is §12's randomized test specified well enough to catch the faults of |
| 139 | rounds 1 to 3? Would its two mutants fail it? |
| 140 | - Anything rev 4 broke, and any wrong citation. |
| 141 | |
| 142 | Number findings R4-1 onward. Same severities, same verdict line. |
| 143 | |
| 144 | ## Round 5 |
| 145 | |
| 146 | Rev 5 answers your round 4 (`CODEX-FINDINGS-DEPLOY-CREDENTIAL-R4.md`); §15 |
| 147 | lists each disposition. G2 is now stated per backend, and §12's randomized |
| 148 | test asserts G1 and G2 only, on recorded events. The directory store's |
| 149 | cross-handle limit is recorded in §14 as existing and separate; §16 is |
| 150 | still Paul's. |
| 151 | |
| 152 | Check that the two round 4 fixes close what you found, that the oracle as |
| 153 | now written is implied by G1 and G2 and that both mutants fail it, and |
| 154 | that rev 5 broke nothing and cites correctly. If you have no finding at P1 |
| 155 | or P2, say so and give the verdict. |
| 156 | |
| 157 | Number findings R5-1 onward. Same severities, same verdict line. |
| 158 | |
amadan