carlos / platform Public

Download this file

1	Reviewed `2d9d528e`. No files modified; findings are based on source inspection. No tests were run.
2
3	1. **R3-1 — P1 — §§7.2, 12, 16: I1(b) is false because an intervening write can restore the original ETag.**
4
5	`DirStore` derives its ETag from the object’s content and compares that hash at replacement time ([store.go:279](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/store.go:279), [store.go:333](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/store.go:333)). It does not prove that nobody wrote the object.
6
7	A concrete interleaving, possible entirely through console operations:
8
9	- `edge` contains a root-signed pointer to A, with history `[B, …]`, no fast-track flag, and `promoted_at` in the current second.
10	- A deploy request reads that pointer and ETag. Its subsequent authorization read sees no pipeline.
11	- An owner installs a pipeline containing `edge`, with no approval or passkey requirement.
12	- The owner rolls `edge` back twice, A → B → A, within that same second.
13	- The deploy request resumes and successfully replaces the pointer using its original ETag.
14
15	Rollback swaps the head and first history entry while preserving the audit ([release.go:1054](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/release.go:1054)). Two rollbacks restore those fields exactly. Signing truncates time to seconds and signs the resulting payload deterministically ([release.go:371](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/release.go:371)), so the original signed bytes—and content hash—can return.
16
17	This breaks I1(b) for either a waiting `Signer.Promote` or `Signer.Rollback`. It also falls outside the stated exception, which requires **no write to C** during the window. I1(a) still holds.
18
19	**Correction:** State the actual guarantee as matching the observed ETag, and explicitly account for intervening writes that restore it, or introduce a generation mechanism that detects those writes. Add this rollback interleaving to the tests. Create-only writes likewise prove absence at write time, not uninterrupted absence after the read. This is separate from Paul’s decision about accepting the pipeline-install window.
20
21	2. **R3-2 — P2 — §§7.2, 12: The no-op path has an additional uncovered outcome and cannot satisfy the repeated conflict test as written.**
22
23	Consider:
24
25	- A repeat deploy promotion reads A at `edge`.
26	- The new no-op hook reads no pipeline and authorizes.
27	- An owner installs a pipeline and promotes B to `edge`.
28	- The deploy request returns successful `noop` for A.
29
30	The existing no-op branch returns without a conditional write or pointer recheck ([release.go:887](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/release.go:887)); the handler reports that success ([apicli.go:889](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/console/apicli.go:889)). Adding authorization before the return does not add conflict detection.
31
32	I1 governs writes, so it does not forbid this outcome. The named exception does not describe it either: another writer changed C, and the deploy request writes nothing. Consequently, §12’s instruction to repeat the conflict races with the requested version already at the head needs a different expected result.
33
34	**Correction:** Specify no-op success as an observation of the earlier pointer, with authorization checked separately. Document this outcome and give no-op races their own assertions. Do not promise a conflict unless the mechanism actually detects one.
35
36	3. **R3-3 — P2 — §§7.2–7.3, 12: Moving the hook loses the scope-specific refusal before ladder errors.**
37
38	The handler can read no pipeline and pass its early check. An owner then installs a pipeline whose entry is `preview`, excluding `edge`. `PromoteOnLadder` reads that pipeline and rejects the requested `edge` promotion during its order check, before calling `Signer.Promote` and reaching the relocated hook ([release.go:930](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/release.go:930), [release.go:951](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/release/release.go:951)).
39
40	The handler consequently returns the existing 409 ladder error ([apicli.go:882](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/console/apicli.go:882)), rather than §7.3’s credential-and-rungs 403. Rev 2’s hook immediately after the ladder’s pipeline read would have produced the specified refusal.
41
42	**Correction:** Retain an early scope check on the ladder’s pipeline read, in addition to the authoritative check after the pointer read. Test this race using a pipeline that excludes the requested rung. Keep both additions conditional on a supplied hook so existing callers retain their behavior.
43
44	4. **R3-4 — P2 — §9: The mixed-version explanation cites the wrong location and skips the actual unknown-role refusal.**
45
46	`authcli.go:411` is a comment; the `allow.Permits` check is at [authcli.go:430](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/console/authcli.go:430). More significantly, an unknown stored service role is rejected earlier by `CLIPrincipal` ([svcprincipal.go:126](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/design-deploy-credential/internal/console/svcprincipal.go:126)). `cliRoute` then returns the not-signed-in response before reaching its role gate.
47
48	**Correction:** Cite both checks accurately. Test an unknown stored role through the resolver and registered doors, expecting authentication refusal; test role-set exclusion separately. The fail-closed conclusion remains valid.
49
50	**Verdict: request changes.**