fix(hooks): codex-gate-check.sh stale-certification loophole (#437)#438
Merged
Conversation
A CERTIFIED/REVIEWED handoff.json status was checked as a literal string with no freshness check -- any number of commits made after certification would sail through the gate on the same stale status forever. Proven live during the v1.84.0 release: 2 real post-certification commits both passed the gate on a round-11 CERTIFIED handoff that was never re-issued. Certification now records commit_sha (HEAD at cert time) in handoff.json; the gate compares it to current HEAD and treats a mismatch -- or a missing field, e.g. an old-format handoff.json predating this fix -- as stale (exit 2, same as an uncertified commit). This allows exactly one commit after certification and blocks the next one until re-cert. CODEX_GATE_SKIP=1 remains the logged-justification escape hatch. TDD: 2 new tests proven RED against the unmodified hook, GREEN after; 2 existing tests updated to real git-repo fixtures with a matching commit_sha (191/191 hook tests green, up from 189). 3-round Codex cross-model review: round 1 caught a second, duplicate copy of the review protocol in the wizard doc missing the commit_sha instruction. Round 2's mutation testing proved the first regression test (a whole-file count comparison) had real slack, and surfaced a third, entirely separate flow-diagram copy that had also been missed. Rebuilt the test as a per-line proximity check and proved it against all 3 mutation scenarios independently before round 3 certified clean. Also fixes a latent bug in tests/test-self-update.sh's multi-reviewer checks (bare ? treated as literal in basic regex, not a quantifier), exposed while trimming skills/sdlc/SKILL.md back under its char budget. Version bump 1.85.0 -> 1.86.0.
CI's test-cowork-drift.sh caught cowork/skills/sdlc/SKILL.md drifting from the canonical skills/sdlc/SKILL.md -- the commit_sha instructions added for #437 in the previous commit never got copied to this second tracked location. Byte-for-byte cp of content already certified in PR #438's round-3 review. Fast-tracked through a scoped 2-round Codex check (round 1 correctly flagged the fix wasn't actually staged yet; round 2 confirmed staging + re-verified the diff-empty/tests-pass content).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
codex-gate-check.shonly checked.reviews/handoff.json'sstatusfield as a literal string — any number of commits after certification would sail through the gate on the same stale status forever. Proven live during v1.84.0: 2 real post-certification commits both passed the gate on a stale round-11 CERTIFIED handoff.commit_sha(HEAD at cert time); the gate compares it to current HEAD and blocks a mismatch or missing field as stale (exit 2). Allows exactly one commit after certification, blocks the next until re-cert.CODEX_GATE_SKIP=1unchanged as the escape hatch.tests/test-self-update.sh's multi-reviewer checks (bare?is literal in basic regex, not a quantifier), exposed while trimmingskills/sdlc/SKILL.mdback under its char budget.Test plan
test-hooks.sh— 191/191 (up from 189)test-doc-consistency.sh— 53/53 (new per-line CERTIFIED/commit_sha check, proven against 3 independent mutations)test-self-update.sh— 153/153test-skill-graduations.sh— 6/6 (char budget)test-workflow-triggers.sh— 170/170