amadan
carlos / platform Public
Reviewed 2d9d528e. No files modified; findings are based on source inspection. No tests were run.
- R3-1 — P1 — §§7.2, 12, 16: I1(b) is false because an intervening write can restore the original ETag.
DirStore derives its ETag from the object’s content and compares that hash at replacement time (store.go:279, store.go:333). It does not prove that nobody wrote the object.
A concrete interleaving, possible entirely through console operations:
- edge contains a root-signed pointer to A, with history [B, …], no fast-track flag, and promoted_at in the current second. - A deploy request reads that pointer and ETag. Its subsequent authorization read sees no pipeline. - An owner installs a pipeline containing edge, with no approval or passkey requirement. - The owner rolls edge back twice, A → B → A, within that same second. - The deploy request resumes and successfully replaces the pointer using its original ETag.
Rollback swaps the head and first history entry while preserving the audit (release.go:1054). Two rollbacks restore those fields exactly. Signing truncates time to seconds and signs the resulting payload deterministically (release.go:371), so the original signed bytes—and content hash—can return.
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.
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.
- R3-2 — P2 — §§7.2, 12: The no-op path has an additional uncovered outcome and cannot satisfy the repeated conflict test as written.
Consider:
- A repeat deploy promotion reads A at edge. - The new no-op hook reads no pipeline and authorizes. - An owner installs a pipeline and promotes B to edge. - The deploy request returns successful noop for A.
The existing no-op branch returns without a conditional write or pointer recheck (release.go:887); the handler reports that success (apicli.go:889). Adding authorization before the return does not add conflict detection.
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.
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.
- R3-3 — P2 — §§7.2–7.3, 12: Moving the hook loses the scope-specific refusal before ladder errors.
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, release.go:951).
The handler consequently returns the existing 409 ladder error (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.
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.
- R3-4 — P2 — §9: The mixed-version explanation cites the wrong location and skips the actual unknown-role refusal.
authcli.go:411 is a comment; the allow.Permits check is at authcli.go:430. More significantly, an unknown stored service role is rejected earlier by CLIPrincipal (svcprincipal.go:126). cliRoute then returns the not-signed-in response before reaching its role gate.
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.
Verdict: request changes.