From cfa7b246faad88bf782988c5cecbe7471d673f74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:50:00 +0200 Subject: [PATCH 1/4] Keep the Gate-B cycle across a WIP amend --no-edit (0.13.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook recognized a WIP commit only by `-m "wip…"` in the Bash command, so `git commit --amend --no-edit` on a `WIP:` commit - which keeps the WIP message - was read as a real commit, and PostToolUse cleared the Gate-B fingerprint, pass count and fresh count. Reproduced under sh and dash in a throwaway repository before the fix. is_wip_commit now also accepts an allow-listed plain form: one line of exactly `git commit` followed only by --amend, --no-edit (both required), --no-verify, -a, --all, -q or --quiet; no quote, #, backslash or shell metacharacter; no custom core.commentChar/commentString; and HEAD's subject in the hook's repository starting with "wip". Every other spelling resets as before, the safe direction under invariant 2. An allow-list because Gate B found a new bypass in each deny-list: quoted, split and abbreviated message flags, -e, git -C, jq-free truncation at an escaped quote, comments, pathspec decoys, --no-amend, and global -c. Section 19b of codex-gate.test.sh covers the positive case and every refused form; each repair's new assertions were shown to fail on the previous hook. todos.md records what stays open. dev-workflow 0.13.1 -> 0.13.2. Gate B: six logical passes; closed on pass 6, Blocker- and Major-free at the floor. Passes 1-5 were full calls; pass 6 was two sequential calls (spec, then quality) against the same base and head ids, so the hook counted seven calls for six passes. In pass 5 both branches wrote the spec file's content from one reviewer (the quality branch's finding); both files were well-formed and the combined finding set is the same. Pass 4 withdrew global `git -c` support that pass 1 had asked for (a require/withdraw pair, one tell; the tell threshold is two). Collected, not repaired: pass 6's Minor (the todos.md annotation omits the comment-character condition); pass 1's Minor that `git -c ... commit --amend --no-edit` still resets. No story cited, so no evidence entry is owed. cycle kn7rl4p223; floor 3 per none; hook reminder threshold absent cycle kn7rl4p223; Gate B (passes 1-6, codex): Findings 5,6,4,3,1,2. Blockers 0,0,0,0,0,0. Majors 2,6,4,3,1,0. Human exceptions: none --- .../gate-b-quality-kn7rl4p223-pass-1.md | 4 ++ .../gate-b-quality-kn7rl4p223-pass-2.md | 4 ++ .../gate-b-quality-kn7rl4p223-pass-3.md | 3 ++ .../gate-b-quality-kn7rl4p223-pass-4.md | 3 ++ .../gate-b-quality-kn7rl4p223-pass-5.md | 2 + .../gate-b-quality-kn7rl4p223-pass-6.md | 2 + .../gate-b-spec-kn7rl4p223-pass-1.md | 3 ++ .../gate-b-spec-kn7rl4p223-pass-2.md | 4 ++ .../gate-b-spec-kn7rl4p223-pass-3.md | 3 ++ .../gate-b-spec-kn7rl4p223-pass-4.md | 2 + .../gate-b-spec-kn7rl4p223-pass-5.md | 2 + .../gate-b-spec-kn7rl4p223-pass-6.md | 2 + .../dev-workflow/.claude-plugin/plugin.json | 2 +- plugins/dev-workflow/CHANGELOG.md | 13 +++++ plugins/dev-workflow/hooks/codex-gate.sh | 28 ++++++++++- plugins/dev-workflow/hooks/codex-gate.test.sh | 48 +++++++++++++++++++ todos.md | 8 ++++ 17 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-1.md create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-2.md create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-3.md create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-4.md create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-5.md create mode 100644 .context/codex-reviews/gate-b-quality-kn7rl4p223-pass-6.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-2.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-3.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-4.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-5.md create mode 100644 .context/codex-reviews/gate-b-spec-kn7rl4p223-pass-6.md diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-1.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-1.md new file mode 100644 index 0000000..f71abd8 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-1.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:773 | The new message-setting exclusion misses valid shell-quoted options and Git long-option abbreviations: on a WIP HEAD both git commit --amend --no-edit '-m' 'real message' and git commit --amend --no-edit --mes='real message' are classified as WIP | PreToolUse skips Gate B and, when a commit-msg hook rejects the real closing attempt, PostToolUse preserves the fingerprint, pass count and fresh count instead of the required conservative reset; reproduced with actual Git attempts returning 1 | Restrict the new exemption to command forms whose message-preserving arguments are positively recognized, treating quoted or abbreviated message-setting options and other uncertain forms as non-WIP; add failed-closing regression cases +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:773 | The exclusion scans the entire command and mistakes Git global -c and -C options for commit message-setting flags; git -c core.quotePath=false commit --amend --no-edit still resets on a WIP HEAD | Common Git invocations continue to destroy the Gate-B cycle despite preserving the WIP message, leaving the requested fix incomplete for those forms | Distinguish options before the commit subcommand from its message-setting options without redesigning commit detection, and cover global -c and -C in the regression suite +MINOR | high | plugins/dev-workflow/CHANGELOG.md:31-32; plugins/dev-workflow/hooks/codex-gate.sh:767-770 | New prose says any message-setting amend disqualifies the WIP exemption and resets, but the retained first branch accepts git commit --amend -m 'WIP: fixes' immediately and preserves the cycle | The shipped changelog and hook explanation misstate the reset boundary and contradict the intentionally retained behavior | Qualify both claims to state that the new HEAD-based exemption rejects message-setting flags while the existing explicit -m WIP exemption remains in force +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-2.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-2.md new file mode 100644 index 0000000..40187e2 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-2.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:778 | The short-option exclusion omits -e, so git commit --amend --no-edit -e is classified as WIP even though Git opens the editor and can replace the WIP subject | PreToolUse suppresses Gate-B on a real closing attempt; if the commit fails and HEAD stays WIP, PostToolUse also retains the fingerprint and both counters, violating invariant 2 | Include e in the short-option exclusion and add standalone, quoted and combined -e regressions for both hook phases under sh and dash. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:775 | The new exemption trusts the jq-free input_field result even when an escaped double quote truncates it: git commit --amend --no-edit "-m" "real" loses the message-setting flag before this scan | Without jq, a real closing attempt receives the WIP note and a failed commit preserves gateB, passCount and freshCount; this newly unsafe fallback violates invariants 2 and 4 | Refuse the new exemption when fallback extraction is incomplete or contains undecoded escapes, or decode the command reliably; add correctly JSON-escaped double-quoted flag cases through the jq-free runner. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:779 | HEAD is read from repo_root even when the command uses git -C to commit in another repository; with WIP HEAD in the hook cwd and non-WIP HEAD in the target, git -C commit --amend --no-edit is incorrectly classified as WIP | A non-WIP closing attempt suppresses the Gate-B reminder and preserves the current cycle state instead of taking the required reset path, violating invariant 2 | Resolve the effective repository for the HEAD check or conservatively decline this exemption when repository-changing arguments or commands make the target uncertain; test a WIP cwd against a non-WIP target. +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-3.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-3.md new file mode 100644 index 0000000..a473024 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-3.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:784 | Shell quote concatenation bypasses message-option rejection: git commit --amend --no-edit -''m real and git commit --amend --no-edit --''message=real qualify as WIP although the shell passes -m or --message to Git | PreToolUse suppresses the Gate-B reminder and a failed closing attempt leaves HEAD WIP so PostToolUse retains gateB/passCount/freshCount, violating invariant 2 and the required reset for message-setting attempts; reproduced under sh and dash | Conservatively reject unsupported quoting in the new exemption, leaving the existing -m WIP branch unchanged, or recognize only a tightly bounded safe token form; add Pre/Post regression assertions with a rejecting commit-msg hook +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:782-783 | Raw substring searches mistake comments and filenames for effective options: git commit --amend # --no-edit and git commit ./file--amend--no-edit both qualify; git commit --amend --no-edit --no-amend also qualifies after cancelling amend | Actual editor-driven closing attempts receive the WIP note and failed attempts retain the Gate-B fingerprint and counters; comment and filename cases reproduced with an editor writing a real message and a rejecting commit-msg hook under sh and dash, violating invariant 2 | Require effective standalone --amend and --no-edit options within a bounded supported command form; decline comments, option-value/pathspec decoys and overriding --no-amend; add Pre/Post regression assertions +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-4.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-4.md new file mode 100644 index 0000000..7d8236f --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-4.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:781 | The unrestricted -c argument accepts Bash brace expansion: git -c {core.quotePath=false,-C,other} commit --amend --no-edit matches the exemption but executes git -c core.quotePath=false -C other commit --amend --no-edit | With the hook repository on a WIP HEAD and other on a non-WIP HEAD, PreToolUse suppresses Gate B and PostToolUse retains the fingerprint and both counters after a real closing attempt; reproduced with the hook under sh and dash, violating AGENTS.md invariant 2 | Remove global -c from this bounded exemption or constrain its tokens to literal nonexpanding syntax; add Pre/Post regression assertions for brace expansion +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:781 | Arbitrary global -c settings can change the retained message: git -c core.commentChar=W -c commit.cleanup=strip commit --amend --no-edit strips a WIP subject and promotes the next noncomment body line to the subject | The actual amend becomes non-WIP while PreToolUse reports cycle preserved and suppresses Gate B; a rejected attempt leaves the WIP HEAD and also escapes the PostToolUse reset, violating AGENTS.md invariant 2 | Exclude global -c overrides or permit only explicitly safe settings and fail conservatively for message-affecting settings; cover a WIP subject with non-WIP body and rejecting commit-msg hook +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-5.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-5.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-5.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-6.md b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-6.md new file mode 100644 index 0000000..e00da5d --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-kn7rl4p223-pass-6.md @@ -0,0 +1,2 @@ +MINOR | high | todos.md:498-501 | The fix annotation describes the allow-listed command and WIP HEAD as sufficient but omits the core.commentChar/core.commentString configuration guard | The documented plain amend still resets when either key is configured, even core.commentChar=#, so the annotation overstates which cases are fixed | Include the absence of both configuration keys as a prerequisite or reference is_wip_commit for the exact conditions +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-1.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-1.md new file mode 100644 index 0000000..b1e2f12 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-1.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:773 | The new no-message guard misses valid quoted and abbreviated message flags: git commit --amend --no-edit '-m' '' and git commit --amend --no-edit --mes='' are classified as WIP on a WIP HEAD despite the requirement that message-setting closing attempts still reset | Both real Git commands abort for an empty message while retaining the WIP HEAD, and PostToolUse now preserves Gate-B state and both counters; PreToolUse also suppresses the closing-attempt reminder for these flag forms | Restrict the new exemption to unambiguous message-preserving invocations, conservatively rejecting quoted or abbreviated message-setting flags, and add failed-attempt regressions for both forms +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:773 | The short-option guard scans the whole Bash command and mistakes Git's global -c configuration option for git commit's message-reediting -c: git -c core.hooksPath=/dev/null commit --amend --no-edit is rejected from the WIP exemption | A successful amend retaining HEAD's WIP subject still clears Gate-B state and both counters, so the requested reset fix remains broken for ordinary per-command Git configuration | Distinguish global Git options from commit message-setting options, conservatively fall back for ambiguous commands, and add a WIP amend regression with global git -c +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-2.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-2.md new file mode 100644 index 0000000..38ddd54 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-2.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:778 | The new exemption rejects --edit but omits its short alias -e: git commit --amend --no-edit -e on a WIP HEAD is classified as WIP, although Git opens the editor and can replace the message. Verified with an editor writing a real closing subject and a rejecting commit-msg hook. | Under both sh and dash, PreToolUse suppresses Gate B and PostToolUse preserves gateB, passCount and freshCount when the closing attempt fails and HEAD remains WIP, contrary to the requested closing-attempt reset and invariant 2. | Include e in the rejected short-option set and add PreToolUse/PostToolUse regression assertions for -e, including a failed closing attempt. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:775-779 | The new exemption trusts the incomplete command returned by the jq-free input_field fallback (lines 34-35): git commit --amend --no-edit "-m" "real message" is truncated at the first JSON-escaped quote, hiding the message-setting flag. The jq path correctly rejects this command; the jq-free path now accepts it as WIP. | Reproduced under sh and dash without jq: PreToolUse emits the WIP exemption and PostToolUse preserves all three Gate-B state files on a failed closing attempt with WIP HEAD. Previously this command reset; the regression violates the requested message-flag exclusion and invariants 2 and 4. | Make the new exemption decline when command extraction is incomplete, or extract the complete escaped JSON string correctly; add double-quoted message-flag regression cases using the jq-free runner. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:779 | The new HEAD lookup always reads repo_root even when git -C selects another repository; from WIP repository A, git -C /path/to/B commit --amend --no-edit is exempted although repository B has a non-WIP HEAD | This newly suppresses the Gate-B reminder and preserves cycle state for an amend on a non-WIP HEAD, contrary to the explicit requirement that such attempts keep resetting; reproduced with two temporary repositories | Conservatively refuse this exemption for commands that redirect the repository, or resolve their effective target before checking its subject; add a regression with WIP invocation root and non-WIP target +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-3.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-3.md new file mode 100644 index 0000000..c5746fc --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-3.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:784 | The message-option rejection examines raw shell spelling: git commit --amend --no-edit -'m' 'real closing message' passes the new exemption although the shell supplies -m to Git. Verified that Git accepts it and replaces the subject; with a rejecting commit-msg hook HEAD remains WIP. | PreToolUse suppresses Gate B and PostToolUse preserves the fingerprint, pass count and fresh count after this failed real closing attempt, contrary to the explicit requirement and invariant 2; the base hook correctly resets under both sh and dash. | Restrict the new exemption to an unambiguous argument grammar and fall through on unsupported quoting, or safely recognize shell quote concatenation without executing the command; add Pre/Post regression assertions for this message-setting form. +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:782 | The required --amend and --no-edit checks accept substrings anywhere in the remaining command, and the shell-syntax guard permits comments. Thus git commit --amend # --no-edit receives the WIP exemption even though Git never receives --no-edit and opens the message editor. | A real interactive closing attempt suppresses the Gate-B reminder; when its editor fails and leaves HEAD WIP, all three Gate-B state files survive. Reproduced under sh and dash, whereas the base hook resets. This violates the requirement to exempt only an amend that keeps the message and invariant 2. | Require actual standalone option tokens before any pathspec separator and reject comments or other unparsed syntax conservatively; add Pre/Post coverage for --no-edit appearing only in a comment or path argument. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-4.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-4.md new file mode 100644 index 0000000..01f8c07 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-4.md @@ -0,0 +1,2 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:781 | The unrestricted global -c operand admits shell brace expansion: with a WIP HEAD locally and a non-WIP HEAD in /tmp/other, git -c {core.quotePath=false,-C,/tmp/other} commit --amend --no-edit passes this allow-list, but Bash expands it into git -c core.quotePath=false -C /tmp/other commit --amend --no-edit. The hook checks the local WIP subject instead of the actual target, violating the requirement that a non-WIP amend remain a closing attempt. | Reproduced under both sh and dash: PreToolUse reports Codex cycle preserved and PostToolUse retains gateB, passCount and freshCount; the base hook resets all three. The expanded command can successfully amend the non-WIP commit, suppressing the Gate-B reminder and missing the required reset (invariant 2). | Remove optional global -c support or constrain its operands to literal shell-safe tokens, rejecting brace and pathname expansion; add PreToolUse and PostToolUse regression assertions for the expanded foreign-repository command. +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-5.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-5.md new file mode 100644 index 0000000..bcc139a --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-5.md @@ -0,0 +1,2 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.sh:785 | The plain-command exemption assumes --no-edit preserves the WIP subject without checking effective Git cleanup settings: with HEAD message "WIP: snapshot\n\nReal subject", core.commentChar=W and commit.cleanup=strip, the accepted git commit --amend --no-edit actually creates subject "Real subject" | PreToolUse incorrectly suppresses Gate-B on a real closing amend, violating invariant 2; the later PostToolUse reset cannot restore the missed pre-commit reminder | Decline the exemption when effective cleanup settings can remove the WIP subject, or verify the effective cleaned message remains WIP, and add a configured-repository regression for the plain command +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-6.md b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-6.md new file mode 100644 index 0000000..23ce14b --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-kn7rl4p223-pass-6.md @@ -0,0 +1,2 @@ +MINOR | high | todos.md:498-501 | The new fix annotation describes the command allow-list and WIP HEAD as sufficient for preserving the cycle but omits the core.commentChar/core.commentString guard implemented at codex-gate.sh:787. | A repository with core.commentChar=W satisfies every documented condition yet the plain git commit --amend --no-edit still resets all three Gate-B state files; the requested backlog annotation therefore overstates the fixed scope. | Reference is_wip_commit as the authoritative predicate, or include the requirement that neither comment configuration key is set in any effective Git configuration scope. +END OF FINDINGS (1 total) diff --git a/plugins/dev-workflow/.claude-plugin/plugin.json b/plugins/dev-workflow/.claude-plugin/plugin.json index e6e4238..27db375 100644 --- a/plugins/dev-workflow/.claude-plugin/plugin.json +++ b/plugins/dev-workflow/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "dev-workflow", "displayName": "Cross-Model Review Workflow", - "version": "0.13.1", + "version": "0.13.2", "description": "Spec-driven workflow with two independent cross-model review gates, an append-only hardening ledger with an escalation ladder, and repo-enforced quality. Requires the superpowers plugin.", "author": { "name": "Daniel Sänger", diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index 47c492a..cbf0c25 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -22,6 +22,19 @@ unambiguously, still fails. Deleting only a plugin's *manifest* while the direct keeps shipping fails too. AGENTS.md invariant 12 carries the complete list. +## 0.13.2 + +- **`git commit --amend --no-edit` on a `WIP:` commit no longer resets the Gate-B cycle.** The + hook recognized a WIP commit only by `-m "wip…"` in the command, so an amend that keeps the + WIP message was read as a real commit and its counters and fingerprint were cleared. It now + also counts an amend as a WIP commit when `HEAD`'s subject starts with `wip` and the + command is, on one line, exactly `git commit` followed only by `--amend`, + `--no-edit`, `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with both `--amend` and + `--no-edit` present, no quote, `#`, backslash or shell metacharacter anywhere, and no custom + `core.commentChar`/`core.commentString` in the repository. Any + other amend, including one on a non-WIP `HEAD`, still resets; the existing `-m "wip…"` + recognition is unchanged. Regression assertions in `codex-gate.test.sh`. + ## 0.13.1 - **The Named residual loses a dangling reference.** Its closing clause, "as the standing diff --git a/plugins/dev-workflow/hooks/codex-gate.sh b/plugins/dev-workflow/hooks/codex-gate.sh index b7ae064..7ef66fb 100755 --- a/plugins/dev-workflow/hooks/codex-gate.sh +++ b/plugins/dev-workflow/hooks/codex-gate.sh @@ -760,7 +760,33 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # an empty HEAD..HEAD range pre-commit). Treating it as a real commit would fire a # spurious STOP and reset the very counters the review loop is accumulating — the # documented workaround would fight the hook. So: gentle note, no reset. -is_wip_commit() { printf '%s' "$1" | grep -Eiq -- "-m[[:space:]]*['\"]?[[:space:]]*wip"; } +# +# `git commit --amend --no-edit` carries no `-m` but keeps HEAD's message, so on a WIP +# HEAD it is a WIP commit too. That is true whether it succeeds or fails — either way +# HEAD's subject is still WIP afterwards — so PreToolUse and PostToolUse can both read +# it — but it reads HEAD of the repository the hook runs in, and a command string is not +# its arguments, so THIS exemption is an allow-list, never a deny-list: one line of exactly +# `git commit` followed only by `--amend`, `--no-edit`, `--no-verify`, +# `-a`, `--all`, `-q` or `--quiet`, both of the first two present. No quote, `#`, backslash +# (the jq-free reader truncates at an escaped quote and leaves one) or shell metacharacter +# anywhere, so no quoting trick, comment, pathspec, `cd`, `git -C` or chained command can +# qualify. Global `git -c` is refused too: its operand can expand into `-C ` or set +# `commit.cleanup` and strip the WIP subject — and for the same reason a repository that +# sets its own comment character is refused, since cleanup can then strip a `WIP` line. +# Anything else falls through to the reset, the safe direction. The `-m "wip…"` match above +# is separate and unchanged. +is_wip_commit() { + printf '%s' "$1" | grep -Eiq -- "-m[[:space:]]*['\"]?[[:space:]]*wip" && return 0 + case $1 in *' +'*) return 1 ;; esac + printf '%s' "$1" | grep -q '[;&|<>$`()\\#"]' && return 1 + printf '%s' "$1" | grep -q "'" && return 1 + printf '%s' "$1" | grep -Eq '^[[:space:]]*git[[:space:]]+commit([[:space:]]+(--amend|--no-edit|--no-verify|-a|--all|-q|--quiet))+[[:space:]]*$' || return 1 + printf '%s ' "$1" | grep -Eq '[[:space:]]--amend[[:space:]]' || return 1 + printf '%s ' "$1" | grep -Eq '[[:space:]]--no-edit[[:space:]]' || return 1 + git -C "$repo_root" config --get-regexp '^core\.comment(char|string)$' >/dev/null 2>&1 && return 1 + git -C "$repo_root" log -1 --format=%s 2>/dev/null | grep -Eiq '^[[:space:]]*wip' +} # A commit that stages all tracked changes (-a / -am / --all) also sweeps in # tracked-but-unstaged edits that `git diff --cached` alone won't show, so the diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index 6ed4d8c..56040c1 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -570,6 +570,54 @@ run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command" commitpost [ ! -f "$count" ] && pass "non-WIP commit still resets counters" || fail "non-WIP commit still resets counters" +# 19b. `git commit --amend --no-edit` on a WIP HEAD keeps the WIP message, so it is +# cycle-internal too — but it carries no `-m`, and a command-string match alone reset it. +# The empty commit keeps HEAD's tree, so no fingerprint input moves; it is undone below. +git commit -q --allow-empty -m 'WIP: snapshot' >/dev/null 2>&1 +amendpost() { run "{\"hook_event_name\":\"PostToolUse\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"$1\"}}" >/dev/null; } +reset_all; rev; rev +out=$(wip "git commit --amend --no-edit") +printf '%s' "$out" | grep -q 'Codex cycle preserved' && pass "amend --no-edit on WIP HEAD -> WIP note" || fail "amend --no-edit on WIP HEAD -> WIP note" +amendpost "git commit --amend --no-edit" +[ "$(cat "$count" 2>/dev/null)" = 2 ] && [ -f "$state" ] && pass "amend --no-edit on WIP HEAD preserves the cycle" || fail "amend --no-edit on WIP HEAD preserves the cycle" +# A new message is a closing act (or a stray commit) and still resets +amendpost "git commit --amend --no-edit -m 'real message'" +[ ! -f "$count" ] && pass "amend with -m on WIP HEAD still resets" || fail "amend with -m on WIP HEAD still resets" +# Forms outside the allow-list: message flags (quoted, split, abbreviated, -e/--edit), another +# repository, a comment, a pathspec decoy, --no-amend cancelling --amend, and any global +# `git -c` (its operand can expand into `-C ` or strip the WIP subject via cleanup). +# Each would fail against a rejecting commit-msg hook and leave HEAD WIP, so only the +# command decides here. +for c in "git commit --amend --no-edit '-m' 'real'" "git commit --amend --no-edit --mes=real" "git commit --amend --no-edit --edit" \ + "git commit --amend --no-edit -e" "git -C /elsewhere commit --amend --no-edit" "cd /elsewhere && git commit --amend --no-edit" \ + "git commit --amend --no-edit -''m real" "git commit --amend # --no-edit" "git commit ./file--amend--no-edit" \ + "git commit --amend --no-edit --no-amend" "git -c {core.quotePath=false,-C,/elsewhere} commit --amend --no-edit" \ + "git -c core.quotePath=false commit --amend --no-edit"; do + reset_all; rev + printf '%s' "$(wip "$c")" | grep -q 'Codex cycle preserved' && fail "not WIP: $c" || pass "not WIP: $c" + amendpost "$c" + [ ! -f "$count" ] && pass "resets: $c" || fail "resets: $c" +done +# A custom comment character lets cleanup strip the WIP line, so the plain form is refused too +git config core.commentChar W +reset_all; rev +printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle preserved' && fail "custom commentChar: no WIP note" || pass "custom commentChar: no WIP note" +amendpost "git commit --amend --no-edit" +[ ! -f "$count" ] && pass "custom commentChar: resets" || fail "custom commentChar: resets" +git config --unset core.commentChar +# Without jq the reader stops at an escaped quote, so `"-m"` would vanish; the leftover +# backslash must still decline the exemption. (JSON: \" inside the command string.) +reset_all; rev +out=$(nojq_run '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":"git commit --amend --no-edit \"-m\" \"real\""}}') +printf '%s' "$out" | grep -q 'Codex cycle preserved' && fail "jq-free: double-quoted -m gets no WIP note" || pass "jq-free: double-quoted -m gets no WIP note" +nojq_run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit --amend --no-edit \"-m\" \"real\""}}' >/dev/null +[ ! -f "$count" ] && pass "jq-free: double-quoted -m still resets" || fail "jq-free: double-quoted -m still resets" +git reset -q --soft HEAD~1 >/dev/null 2>&1 +# On a non-WIP HEAD the kept message is a real one, so the reset stands +reset_all; rev; rev +amendpost "git commit --amend --no-edit" +[ ! -f "$count" ] && pass "amend --no-edit on non-WIP HEAD still resets" || fail "amend --no-edit on non-WIP HEAD still resets" + # 20. FINDING F — a Codex server whose tools the gates can't attribute reset_all out=$(codextool mcp__codex__codex) diff --git a/todos.md b/todos.md index 659b42a..2246c3d 100644 --- a/todos.md +++ b/todos.md @@ -495,6 +495,14 @@ backlog. reading git state rather than the command string). Prefer the latter if the class recurs; log it through `harden-finding` when it does. *Trigger: the next change that touches the hook, or a second observed cycle reset.* + **Reset direction fixed in 0.13.2** for the plain form only: `is_wip_commit` also + accepts a one-line `git commit` whose only arguments are `--amend`, `--no-edit` (both + required), `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with no quote, `#`, + backslash or shell metacharacter, when `HEAD`'s subject starts with `wip`. An + allow-list, because four Gate-B passes found a new bypass in every deny-list. Still + open, deliberately, all in the safe direction: any other spelling of a WIP amend + (global `git -c`, `cd … &&`, `-F`/`-C` with a WIP message) still resets, and a heredoc + merely containing `WIP:` or `git commit` still fires. - [x] **P2 — risk/security profiles, and the derived validation mode.** Shipped: two human-confirmed axes in the story header, a mode derived as `max(risk, security)`, From c9e274b230483e6ac4a70c3cac36b480066c327e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:48:25 +0200 Subject: [PATCH 2/4] Refuse the amend exemption when a message hook can rewrite it (PR #30 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile on PR #30: a prepare-commit-msg or commit-msg hook can rewrite the message of `git commit --amend --no-edit`, while the new exemption read HEAD's WIP subject before the commit and so suppressed the Gate-B reminder for what became a real closing commit (reproduced; --no-verify does not skip prepare-commit-msg). The exemption now also refuses when core.hooksPath is set at all, or when the default hooks directory holds a prepare-commit-msg or commit-msg hook; hook contents are not read. The older -m "wip…" path has the same exposure and is recorded as open in todos.md, not changed here. Section 19c tests the real lifecycle - PreToolUse, an actual amend, PostToolUse - under sh and dash: no message hook (WIP kept, cycle kept), aborted amend (cycle kept), rewriting prepare-commit-msg, rejecting commit-msg, core.hooksPath, and a newline-ending core.hooksPath. It runs with no global or system git config and in a fresh hooks directory, the original moved aside and restored, so it cannot write to a developer's real or template-linked hooks. CHANGELOG and todos.md state every condition, the comment character and hooks ones included. Gate B: three logical passes, each two sequential calls (spec, then quality) against the same base and head ids; closed on pass 3, Blocker- and Major-free at the floor. Collected, not repaired: pass 3's Minor that section 19b's positive assertions fail when a developer has a global core.hooksPath (not set here or in CI); pass 1's Minor that CLAUDE.md §5 and docs/architecture.md describe WIP recognition by message rather than by command (recorded in todos.md). No story cited, so no evidence entry is owed. cycle ny9c33v3o2; floor 3 per none; hook reminder threshold absent cycle ny9c33v3o2; Gate B (passes 1-3, codex): Findings 4,3,1. Blockers 1,0,0. Majors 1,1,0. Human exceptions: none --- .../gate-b-quality-ny9c33v3o2-pass-1.md | 4 ++ .../gate-b-quality-ny9c33v3o2-pass-2.md | 3 + .../gate-b-quality-ny9c33v3o2-pass-3.md | 2 + .../gate-b-spec-ny9c33v3o2-pass-1.md | 2 + .../gate-b-spec-ny9c33v3o2-pass-2.md | 2 + .../gate-b-spec-ny9c33v3o2-pass-3.md | 2 + plugins/dev-workflow/CHANGELOG.md | 11 +-- plugins/dev-workflow/hooks/codex-gate.sh | 22 ++++-- plugins/dev-workflow/hooks/codex-gate.test.sh | 72 +++++++++++++++++++ todos.md | 14 +++- 10 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-1.md create mode 100644 .context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-2.md create mode 100644 .context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-3.md create mode 100644 .context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-2.md create mode 100644 .context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-3.md diff --git a/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-1.md b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-1.md new file mode 100644 index 0000000..0719560 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-1.md @@ -0,0 +1,4 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.test.sh:621,642-657 | The new lifecycle tests resolve hooksd through rev-parse without first overriding inherited core.hooksPath; with a global hooks directory, their writes, chmods and removals target the developer's real pre-commit, prepare-commit-msg and commit-msg hooks outside the sandbox. An isolated global-config reproduction confirmed an existing hook was overwritten and deleted. | Running the test suite can destroy user hook files and permissions. | Configure a repository-local hooksPath pointing to a dedicated sandbox directory before the first test commit, and restore that isolated setting after the custom hooksPath scenario; add a regression using an isolated global config and untouched sentinel hooks. +MINOR | high | plugins/dev-workflow/hooks/codex-gate.sh:793-796 | Command substitution strips trailing newlines from the hooks directory returned by rev-parse. A valid core.hooksPath ending in a newline therefore makes the presence checks inspect a different directory. Reproduced with an executable rejecting commit-msg: PreToolUse emitted the WIP exemption, the amend failed, and PostToolUse retained passCount=2. | This uncommon but valid hooksPath bypasses the required hook-presence refusal, suppressing the reminder and missing the conservative reset. | Preserve the path bytes and command status when capturing rev-parse output, or conservatively refuse paths whose representation cannot be preserved; cover a trailing-newline hooksPath under sh and dash. +MINOR | high | CLAUDE.md:1318-1320; plugins/dev-workflow/commands/workflow-init.md:1507-1509; docs/architecture.md:90-93 | These passages still promise that a WIP-prefixed commit message preserves the counters. The new guard resets even a plain amend --no-edit whose message hook accepts the commit without changing its WIP subject, a shape exempt at the review base. | The workflow documentation and scaffolded instructions overstate the exemption and contradict the new behavior, contrary to prompt-standards item 7 and AGENTS.md's mechanism-accuracy rule. | Qualify preservation as applying to commands recognized by the hook and document the conservative amend restrictions, keeping CLAUDE.md and the inline template synchronized. +END OF FINDINGS (3 total) diff --git a/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-2.md b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-2.md new file mode 100644 index 0000000..8757641 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-2.md @@ -0,0 +1,3 @@ +MAJOR | high | plugins/dev-workflow/hooks/codex-gate.test.sh:625-628,650-665 | The hooks-directory guard compares path strings without excluding symlinks. The earlier git init can inherit init.templateDir or GIT_TEMPLATE_DIR containing a hooks symlink; Git copies it into .git/hooks, so both compared strings are .git/hooks even when it resolves outside the sandbox. Reproduced with a temporary template and shared hooks directory. | The new redirections overwrite shared pre-commit, prepare-commit-msg or commit-msg hooks, and cleanup removes them, destroying developer files outside the test repository. Disabling global configuration after initialization does not remove copied symlinks. | Initialize the test repository with an explicitly controlled empty template and create its own hooks directory, or reject external/symlinked hook directories and individual hook paths before any writes; add a sandboxed regression using a template symlink. +MINOR | high | plugins/dev-workflow/CHANGELOG.md:34-35; todos.md:502-503 | Both edited exemption-condition lists require only absence of message hooks in the effective directory, but codex-gate.sh:795 refuses every configured core.hooksPath, including an empty directory or /dev/null. | The requested exhaustive annotations still promise cycle preservation in configurations where the hook resets it. | State explicitly in both descriptions that core.hooksPath must be unset, in addition to neither message hook existing in the default hooks directory. +END OF FINDINGS (2 total) diff --git a/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-3.md b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-3.md new file mode 100644 index 0000000..eda3403 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-ny9c33v3o2-pass-3.md @@ -0,0 +1,2 @@ +MINOR | high | plugins/dev-workflow/hooks/codex-gate.test.sh:623 | Git configuration isolation begins after section 19b, whose positive WIP assertions now require core.hooksPath to be unset; a temporary global config pointing core.hooksPath at an empty directory reproduces two failures at lines 580 and 582 | The new guard correctly refuses the exemption, but the test expects preservation, so a legitimate developer configuration makes the quality suite fail | Move configuration and default-hook isolation before section 19b and preserve inherited environment values when restoring section-local configuration +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-1.md b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-1.md new file mode 100644 index 0000000..bce0a83 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-1.md @@ -0,0 +1,2 @@ +BLOCKER | high | plugins/dev-workflow/hooks/codex-gate.sh:793 | Command substitution strips trailing newline bytes belonging to a valid core.hooksPath directory name, so the presence checks inspect a different directory and grant the amend exemption despite an effective message hook | Reproduced under sh and dash: a prepare-commit-msg hook rewrites the actual amend while PreToolUse emits the WIP note; a rejecting commit-msg hook also leaves the pass count preserved after PostToolUse. This violates the required hook-presence refusal and invariant 2 | Preserve the exact hooks path when capturing rev-parse output while retaining fail-closed lookup errors, and add a real-lifecycle regression with a newline-ending core.hooksPath +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-2.md b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-2.md new file mode 100644 index 0000000..f04f323 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-2.md @@ -0,0 +1,2 @@ +MINOR | high | plugins/dev-workflow/CHANGELOG.md:34-35; todos.md:502-503 | The requested exhaustive exemption conditions remain incomplete: both descriptions require only absence of message hooks in the effective hooks directory, but codex-gate.sh:795 rejects any configured core.hooksPath, even an empty directory with no hooks. | A plain WIP amend meeting every documented condition can still reset the cycle; reproduced under sh and dash with core.hooksPath pointing to an empty directory. | State in both descriptions that core.hooksPath must be unset and that the default hooks directory must contain neither message hook. +END OF FINDINGS (1 total) diff --git a/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-3.md b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-3.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-ny9c33v3o2-pass-3.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index cbf0c25..a0e92e9 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -30,10 +30,13 @@ AGENTS.md invariant 12 carries the complete list. also counts an amend as a WIP commit when `HEAD`'s subject starts with `wip` and the command is, on one line, exactly `git commit` followed only by `--amend`, `--no-edit`, `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with both `--amend` and - `--no-edit` present, no quote, `#`, backslash or shell metacharacter anywhere, and no custom - `core.commentChar`/`core.commentString` in the repository. Any - other amend, including one on a non-WIP `HEAD`, still resets; the existing `-m "wip…"` - recognition is unchanged. Regression assertions in `codex-gate.test.sh`. + `--no-edit` present, no quote, `#`, backslash or shell metacharacter anywhere, no custom + `core.commentChar`/`core.commentString`, no `core.hooksPath` set at all, and no + `prepare-commit-msg` or `commit-msg` hook in the default hooks directory, since either hook + can rewrite the message. Any other amend, including one on a non-WIP `HEAD`, still resets; + the existing `-m "wip…"` recognition is unchanged, and so is its exposure to such hooks. + Regression assertions in `codex-gate.test.sh`, including real amends with and without + message hooks. ## 0.13.1 diff --git a/plugins/dev-workflow/hooks/codex-gate.sh b/plugins/dev-workflow/hooks/codex-gate.sh index 7ef66fb..9bae595 100755 --- a/plugins/dev-workflow/hooks/codex-gate.sh +++ b/plugins/dev-workflow/hooks/codex-gate.sh @@ -761,11 +761,11 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # spurious STOP and reset the very counters the review loop is accumulating — the # documented workaround would fight the hook. So: gentle note, no reset. # -# `git commit --amend --no-edit` carries no `-m` but keeps HEAD's message, so on a WIP -# HEAD it is a WIP commit too. That is true whether it succeeds or fails — either way -# HEAD's subject is still WIP afterwards — so PreToolUse and PostToolUse can both read -# it — but it reads HEAD of the repository the hook runs in, and a command string is not -# its arguments, so THIS exemption is an allow-list, never a deny-list: one line of exactly +# `git commit --amend --no-edit` carries no `-m` and normally keeps HEAD's message, so on +# a WIP HEAD it is usually a WIP commit too, and PreToolUse and PostToolUse both read +# HEAD's subject for it. "Usually" is why this exemption refuses whenever anything could +# change that message: it reads HEAD of the repository the hook runs in, and a command +# string is not its arguments, so it is an allow-list, never a deny-list: one line of exactly # `git commit` followed only by `--amend`, `--no-edit`, `--no-verify`, # `-a`, `--all`, `-q` or `--quiet`, both of the first two present. No quote, `#`, backslash # (the jq-free reader truncates at an escaped quote and leaves one) or shell metacharacter @@ -773,6 +773,13 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # qualify. Global `git -c` is refused too: its operand can expand into `-C ` or set # `commit.cleanup` and strip the WIP subject — and for the same reason a repository that # sets its own comment character is refused, since cleanup can then strip a `WIP` line. +# So is one with a `prepare-commit-msg` or `commit-msg` hook in its hooks directory: either +# may rewrite the message, `--no-verify` does not skip the first, and the hook's content is +# not read — its presence is enough. Any `core.hooksPath` at all is refused outright rather +# than resolved, since a path this shell cannot carry exactly (a trailing newline, say) +# would send the check to the wrong directory. The +# `-m "wip…"` path has the same exposure to message hooks; it predates this and is left +# as it was, recorded in todos.md. # Anything else falls through to the reset, the safe direction. The `-m "wip…"` match above # is separate and unchanged. is_wip_commit() { @@ -785,6 +792,11 @@ is_wip_commit() { printf '%s ' "$1" | grep -Eq '[[:space:]]--amend[[:space:]]' || return 1 printf '%s ' "$1" | grep -Eq '[[:space:]]--no-edit[[:space:]]' || return 1 git -C "$repo_root" config --get-regexp '^core\.comment(char|string)$' >/dev/null 2>&1 && return 1 + git -C "$repo_root" config --get core.hooksPath >/dev/null 2>&1 && return 1 + _hooks=$(git -C "$repo_root" rev-parse --git-path hooks 2>/dev/null) || return 1 + [ -n "$_hooks" ] || return 1 + case $_hooks in /*) ;; *) _hooks="$repo_root/$_hooks" ;; esac + [ -e "$_hooks/prepare-commit-msg" ] || [ -e "$_hooks/commit-msg" ] && return 1 git -C "$repo_root" log -1 --format=%s 2>/dev/null | grep -Eiq '^[[:space:]]*wip' } diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index 56040c1..1eb9b60 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -612,7 +612,79 @@ out=$(nojq_run '{"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input": printf '%s' "$out" | grep -q 'Codex cycle preserved' && fail "jq-free: double-quoted -m gets no WIP note" || pass "jq-free: double-quoted -m gets no WIP note" nojq_run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"command":"git commit --amend --no-edit \"-m\" \"real\""}}' >/dev/null [ ! -f "$count" ] && pass "jq-free: double-quoted -m still resets" || fail "jq-free: double-quoted -m still resets" +# 19c. The real lifecycle: PreToolUse, an ACTUAL `git commit --amend --no-edit`, PostToolUse. +# Git refuses to amend an empty commit into an empty one, so 19b's empty WIP commit is +# replaced by one that adds a file; it is removed again below. Hook scripts live in the git +# dir or the sandbox, never in the worktree. +# These cases write hook scripts, so they run with no global or system git config — an +# inherited core.hooksPath would otherwise aim the writes at the developer's real hooks — +# and into a fresh hooks directory: the one `git init` made may be a symlink copied from a +# template, so it is moved aside (mv moves a link, never its target) and restored below. +GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1; export GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM git reset -q --soft HEAD~1 >/dev/null 2>&1 +printf 'wip\n' > wip.txt; git add wip.txt >/dev/null 2>&1; git commit -q -m 'WIP: snapshot' >/dev/null 2>&1 +hooksd=$(git rev-parse --git-path hooks) +if [ "$hooksd" = "$(git rev-parse --git-dir)/hooks" ] && { [ ! -e "$hooksd" ] && [ ! -L "$hooksd" ] || mv "$hooksd" "$sandbox/orig-hooks"; } \ + && mkdir "$hooksd"; then :; else + fail "19c: could not set up the test repo's own hooks directory ($hooksd); lifecycle cases skipped" + hooksd=$sandbox/refused +fi +# shellcheck disable=SC2016 # $1 belongs to the git hook, expanded when git runs it +rewrite_hook='#!/bin/sh +printf "real subject\n" > "$1" +' +lifecycle() { # runs one real amend between the two hook events; leaves $pre, $subj, $moved + reset_all; rev; rev + pre=$(wip "git commit --amend --no-edit") + before=$(git rev-parse HEAD) + # A same-second amend of an unchanged tree recreates the same commit, so stage a change + printf 'x\n' >> wip.txt; git add wip.txt >/dev/null 2>&1 + git commit -q --amend --no-edit >/dev/null 2>&1 + [ "$(git rev-parse HEAD)" != "$before" ] && moved=yes || moved=no + subj=$(git log -1 --format=%s) + amendpost "git commit --amend --no-edit" +} +# (a) no message hook: the amend keeps the WIP subject and the cycle survives +lifecycle +printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = yes ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ + && pass "real amend, no message hook: WIP note, subject kept, cycle preserved" || fail "real amend, no message hook: WIP note, subject kept, cycle preserved" +# (b) an aborted amend with no message hook leaves HEAD WIP, so the cycle stays open +printf '#!/bin/sh\nexit 1\n' > "$hooksd/pre-commit"; chmod +x "$hooksd/pre-commit" +lifecycle +[ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] && pass "aborted real amend (pre-commit): cycle preserved" || fail "aborted real amend (pre-commit): cycle preserved" +rm -f "$hooksd/pre-commit" +# (c) prepare-commit-msg rewrites the subject into a real one: no WIP note before, reset after +printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" +lifecycle +{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "message hook: no WIP note before the amend" || pass "message hook: no WIP note before the amend" +[ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "message hook: subject rewritten, cycle reset" || fail "message hook: subject rewritten, cycle reset" +rm -f "$hooksd/prepare-commit-msg"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +# (d) commit-msg rejects the amend: HEAD stays WIP, but a message hook is present, so reset +printf '#!/bin/sh\nexit 1\n' > "$hooksd/commit-msg"; chmod +x "$hooksd/commit-msg" +lifecycle +{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "rejecting commit-msg: no WIP note" || pass "rejecting commit-msg: no WIP note" +[ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ ! -f "$count" ] && pass "rejecting commit-msg: HEAD still WIP, cycle reset" || fail "rejecting commit-msg: HEAD still WIP, cycle reset" +rm -f "$hooksd/commit-msg" +# (e) any core.hooksPath refuses the exemption; this one also rewrites the subject +mkdir -p "$sandbox/hooks" +printf '%s' "$rewrite_hook" > "$sandbox/hooks/prepare-commit-msg"; chmod +x "$sandbox/hooks/prepare-commit-msg" +git config core.hooksPath "$sandbox/hooks" +lifecycle +{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "core.hooksPath message hook: no WIP note" || pass "core.hooksPath message hook: no WIP note" +[ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "core.hooksPath message hook: subject rewritten, cycle reset" || fail "core.hooksPath message hook: subject rewritten, cycle reset" +git config --unset core.hooksPath; rm -rf "$sandbox/hooks"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 +# a hooksPath ending in a newline is refused too, not resolved to a neighbouring directory +git config core.hooksPath "$sandbox/nl +" +reset_all; rev +[ "$(git log -1 --format=%s)" = 'WIP: snapshot' ] && ! { printf '%s' "$(wip "git commit --amend --no-edit")" | grep -q 'Codex cycle preserved'; } \ + && pass "newline-ending core.hooksPath on a WIP HEAD: no WIP note" || fail "newline-ending core.hooksPath on a WIP HEAD: no WIP note" +git config --unset core.hooksPath +git reset -q --soft HEAD~1 >/dev/null 2>&1; git rm -q --cached wip.txt >/dev/null 2>&1; rm -f wip.txt +if [ "$hooksd" != "$sandbox/refused" ]; then + rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd" +fi +unset GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM # On a non-WIP HEAD the kept message is a real one, so the reset stands reset_all; rev; rev amendpost "git commit --amend --no-edit" diff --git a/todos.md b/todos.md index 2246c3d..09e7ebc 100644 --- a/todos.md +++ b/todos.md @@ -498,11 +498,21 @@ backlog. **Reset direction fixed in 0.13.2** for the plain form only: `is_wip_commit` also accepts a one-line `git commit` whose only arguments are `--amend`, `--no-edit` (both required), `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with no quote, `#`, - backslash or shell metacharacter, when `HEAD`'s subject starts with `wip`. An + backslash or shell metacharacter, when `HEAD`'s subject starts with `wip`, no custom + `core.commentChar`/`core.commentString` is set, no `core.hooksPath` is set at all, and + the default hooks directory has no `prepare-commit-msg` or `commit-msg` hook (either + could rewrite the message). An allow-list, because four Gate-B passes found a new bypass in every deny-list. Still open, deliberately, all in the safe direction: any other spelling of a WIP amend (global `git -c`, `cd … &&`, `-F`/`-C` with a WIP message) still resets, and a heredoc - merely containing `WIP:` or `git commit` still fires. + merely containing `WIP:` or `git commit` still fires. **Open in the unsafe direction, + and older than this fix:** the `-m "wip…"` path trusts the typed message, so a + `prepare-commit-msg` or `commit-msg` hook that rewrites it into a real one gets the WIP + note and keeps the cycle (PR #30 review). Not fixed there, by scope. Also collected + there: `CLAUDE.md` §5 Mechanics (and the `workflow-init` copy) and + `docs/architecture.md` say a `wip`-prefixed commit *message* keeps the counters, but + the hook reads the command and a few git settings, not the message — a two-copy prompt + fix for a later change. - [x] **P2 — risk/security profiles, and the derived validation mode.** Shipped: two human-confirmed axes in the story header, a mode derived as `max(risk, security)`, From f9ca1fc4d8016236954817721cc0bed3900ca3b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:05:46 +0200 Subject: [PATCH 3/4] Refuse the amend exemption when the hooks directory holds any hook (PR #30 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile's second round on PR #30: a pre-commit hook can install a message hook mid-commit, so refusing only prepare-commit-msg and commit-msg was not enough. The amend exemption now refuses whenever the default hooks directory holds any entry but *.sample files (hidden, non-executable and dangling ones included), or cannot be listed; core.hooksPath is still refused outright. This is deliberately stricter than Git's own definition of an active hook, reads no hook code, and sees the directory only when the hook runs. The -m "wip…" path keeps its older exposure, recorded in todos.md. The test suite now runs with GIT_CONFIG_GLOBAL=/dev/null, GIT_CONFIG_NOSYSTEM=1 and an empty GIT_TEMPLATE_DIR from before its first git init, so a developer's core.hooksPath, comment character or template hooks neither change the results nor receive writes (checked by hand with a hostile HOME config and two sentinel hook directories: suite green, sentinels unchanged). Lifecycle cases added: only *.sample files (kept), an amend aborted by a held index.lock with no hooks (kept), and a pre-commit hook that installs a rewriting prepare-commit-msg (reset). The aborting pre-commit case now expects a reset, since under the any-hook rule its presence refuses the exemption. CodeRabbit's requests to edit the historical Codex findings files are not taken: those files record what a pass found. Gate B: one logical pass (spec, then quality, same base and head ids), NO FINDINGS on both branches; closed on the zero-finding exit. No story cited, so no evidence entry is owed. cycle r06l8px6lh; floor 3 per none; hook reminder threshold absent cycle r06l8px6lh; Gate B (passes 1, codex): Findings 0. Blockers 0. Majors 0. Human exceptions: none --- .../gate-b-quality-r06l8px6lh-pass-1.md | 2 + .../gate-b-spec-r06l8px6lh-pass-1.md | 2 + plugins/dev-workflow/CHANGELOG.md | 14 +++--- plugins/dev-workflow/hooks/codex-gate.sh | 23 +++++++--- plugins/dev-workflow/hooks/codex-gate.test.sh | 45 ++++++++++++++----- todos.md | 9 ++-- 6 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-r06l8px6lh-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-r06l8px6lh-pass-1.md diff --git a/.context/codex-reviews/gate-b-quality-r06l8px6lh-pass-1.md b/.context/codex-reviews/gate-b-quality-r06l8px6lh-pass-1.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-r06l8px6lh-pass-1.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-spec-r06l8px6lh-pass-1.md b/.context/codex-reviews/gate-b-spec-r06l8px6lh-pass-1.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-r06l8px6lh-pass-1.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/plugins/dev-workflow/CHANGELOG.md b/plugins/dev-workflow/CHANGELOG.md index a0e92e9..95715b4 100644 --- a/plugins/dev-workflow/CHANGELOG.md +++ b/plugins/dev-workflow/CHANGELOG.md @@ -31,12 +31,14 @@ AGENTS.md invariant 12 carries the complete list. command is, on one line, exactly `git commit` followed only by `--amend`, `--no-edit`, `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with both `--amend` and `--no-edit` present, no quote, `#`, backslash or shell metacharacter anywhere, no custom - `core.commentChar`/`core.commentString`, no `core.hooksPath` set at all, and no - `prepare-commit-msg` or `commit-msg` hook in the default hooks directory, since either hook - can rewrite the message. Any other amend, including one on a non-WIP `HEAD`, still resets; - the existing `-m "wip…"` recognition is unchanged, and so is its exposure to such hooks. - Regression assertions in `codex-gate.test.sh`, including real amends with and without - message hooks. + `core.commentChar`/`core.commentString`, no `core.hooksPath` set at all, and nothing but + `*.sample` files in the default hooks directory — a rule stricter than Git's own, since a + message hook can rewrite the message and an earlier hook can install one mid-commit; the + check sees the directory only as it is when the hook runs. Any other amend, including one + on a non-WIP `HEAD`, still resets; the existing `-m "wip…"` recognition is unchanged, and + so is its exposure to such hooks. Regression assertions in `codex-gate.test.sh`, including + real amends with and without hooks; the suite now runs without global or system git + config and with an empty init template. ## 0.13.1 diff --git a/plugins/dev-workflow/hooks/codex-gate.sh b/plugins/dev-workflow/hooks/codex-gate.sh index 9bae595..dafb749 100755 --- a/plugins/dev-workflow/hooks/codex-gate.sh +++ b/plugins/dev-workflow/hooks/codex-gate.sh @@ -773,13 +773,16 @@ is_commit() { printf '%s' "$1" | grep -Eq '(^|[^[:alnum:]])git[[:space:]].*commi # qualify. Global `git -c` is refused too: its operand can expand into `-C ` or set # `commit.cleanup` and strip the WIP subject — and for the same reason a repository that # sets its own comment character is refused, since cleanup can then strip a `WIP` line. -# So is one with a `prepare-commit-msg` or `commit-msg` hook in its hooks directory: either -# may rewrite the message, `--no-verify` does not skip the first, and the hook's content is -# not read — its presence is enough. Any `core.hooksPath` at all is refused outright rather +# So is one whose hooks directory holds anything but `*.sample` files — a deliberately +# stricter rule than Git's own (Git runs only executable files with hook names): any other +# entry refuses, executable or not, because a message hook can rewrite the message +# (`--no-verify` does not skip `prepare-commit-msg`) and an earlier hook such as +# `pre-commit` can install one mid-commit. Hook contents are never read. A hooks directory +# this check cannot list refuses too. Any `core.hooksPath` at all is refused outright rather # than resolved, since a path this shell cannot carry exactly (a trailing newline, say) -# would send the check to the wrong directory. The -# `-m "wip…"` path has the same exposure to message hooks; it predates this and is left -# as it was, recorded in todos.md. +# would send the check to the wrong directory. The check sees the directory as it is when +# the hook runs, not what another process does afterwards. The `-m "wip…"` path has the +# same exposure to hooks; it predates this and is left as it was, recorded in todos.md. # Anything else falls through to the reset, the safe direction. The `-m "wip…"` match above # is separate and unchanged. is_wip_commit() { @@ -796,7 +799,13 @@ is_wip_commit() { _hooks=$(git -C "$repo_root" rev-parse --git-path hooks 2>/dev/null) || return 1 [ -n "$_hooks" ] || return 1 case $_hooks in /*) ;; *) _hooks="$repo_root/$_hooks" ;; esac - [ -e "$_hooks/prepare-commit-msg" ] || [ -e "$_hooks/commit-msg" ] && return 1 + if [ -e "$_hooks" ] || [ -L "$_hooks" ]; then + [ -d "$_hooks" ] && [ -r "$_hooks" ] && [ -x "$_hooks" ] || return 1 + for _f in "$_hooks"/* "$_hooks"/.[!.]* "$_hooks"/..?*; do + [ -e "$_f" ] || [ -L "$_f" ] || continue + case ${_f##*/} in *.sample) ;; *) return 1 ;; esac + done + fi git -C "$repo_root" log -1 --format=%s 2>/dev/null | grep -Eiq '^[[:space:]]*wip' } diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index 1eb9b60..23fe13e 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -36,6 +36,12 @@ work=$(mktemp -d) # for a reason no label mentions. sandbox=$(mktemp -d) trap 'rm -rf "$work" "$sandbox"' EXIT +# The whole suite runs with no global or system git config and an empty init template, so +# a developer's core.hooksPath, comment character or template hooks can neither change +# what the hook decides nor be written to by the cases that install hook scripts. +mkdir "$sandbox/template" +GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TEMPLATE_DIR=$sandbox/template +export GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM GIT_TEMPLATE_DIR cd "$work" || exit 1 git init -q git config user.email t@t; git config user.name t @@ -616,11 +622,9 @@ nojq_run '{"hook_event_name":"PostToolUse","tool_name":"Bash","tool_input":{"com # Git refuses to amend an empty commit into an empty one, so 19b's empty WIP commit is # replaced by one that adds a file; it is removed again below. Hook scripts live in the git # dir or the sandbox, never in the worktree. -# These cases write hook scripts, so they run with no global or system git config — an -# inherited core.hooksPath would otherwise aim the writes at the developer's real hooks — -# and into a fresh hooks directory: the one `git init` made may be a symlink copied from a -# template, so it is moved aside (mv moves a link, never its target) and restored below. -GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1; export GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM +# These cases write hook scripts into a fresh hooks directory. The suite's empty template +# means `git init` made none, but whatever is there is moved aside (mv moves a link, never +# its target) and restored below, so no write can reach a directory outside this repo. git reset -q --soft HEAD~1 >/dev/null 2>&1 printf 'wip\n' > wip.txt; git add wip.txt >/dev/null 2>&1; git commit -q -m 'WIP: snapshot' >/dev/null 2>&1 hooksd=$(git rev-parse --git-path hooks) @@ -644,15 +648,37 @@ lifecycle() { # runs one real amend between the two hook events; leaves $pre, $s subj=$(git log -1 --format=%s) amendpost "git commit --amend --no-edit" } -# (a) no message hook: the amend keeps the WIP subject and the cycle survives +# (a) no hooks at all: the amend keeps the WIP subject and the cycle survives lifecycle printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = yes ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ - && pass "real amend, no message hook: WIP note, subject kept, cycle preserved" || fail "real amend, no message hook: WIP note, subject kept, cycle preserved" -# (b) an aborted amend with no message hook leaves HEAD WIP, so the cycle stays open + && pass "real amend, no hooks: WIP note, subject kept, cycle preserved" || fail "real amend, no hooks: WIP note, subject kept, cycle preserved" +# (a2) *.sample files are the one thing the hooks directory may hold +printf '#!/bin/sh\nexit 1\n' > "$hooksd/commit-msg.sample"; chmod +x "$hooksd/commit-msg.sample" +lifecycle +printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = yes ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ + && pass "real amend, only *.sample hooks: cycle preserved" || fail "real amend, only *.sample hooks: cycle preserved" +rm -f "$hooksd/commit-msg.sample" +# (b) an amend aborted with no hook present (a held index lock) leaves HEAD WIP: cycle kept +: > "$(git rev-parse --git-dir)/index.lock" +lifecycle +rm -f "$(git rev-parse --git-dir)/index.lock" +printf '%s' "$pre" | grep -q 'Codex cycle preserved' && [ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] \ + && pass "aborted real amend, no hooks: cycle preserved" || fail "aborted real amend, no hooks: cycle preserved" +# (b2) Before PR #30's review this case expected preservation: a pre-commit hook that +# aborts. Under the any-hook rule its mere presence refuses the exemption, so it resets. printf '#!/bin/sh\nexit 1\n' > "$hooksd/pre-commit"; chmod +x "$hooksd/pre-commit" lifecycle -[ "$moved" = no ] && [ "$subj" = 'WIP: snapshot' ] && [ "$(cat "$count" 2>/dev/null)" = 2 ] && pass "aborted real amend (pre-commit): cycle preserved" || fail "aborted real amend (pre-commit): cycle preserved" +{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "pre-commit hook present: no WIP note" || pass "pre-commit hook present: no WIP note" +[ "$moved" = no ] && [ ! -f "$count" ] && pass "pre-commit hook present, amend aborted: cycle reset" || fail "pre-commit hook present, amend aborted: cycle reset" rm -f "$hooksd/pre-commit" +# (f) a pre-commit hook that installs a rewriting prepare-commit-msg mid-commit +printf '%s' "$rewrite_hook" > "$sandbox/rewrite-hook" +printf '#!/bin/sh\ncp "%s" "%s/prepare-commit-msg" && chmod +x "%s/prepare-commit-msg"\n' "$sandbox/rewrite-hook" "$hooksd" "$hooksd" > "$hooksd/pre-commit" +chmod +x "$hooksd/pre-commit" +lifecycle +{ printf '%s' "$pre" | grep -q 'Codex cycle preserved'; } && fail "pre-commit installing a message hook: no WIP note" || pass "pre-commit installing a message hook: no WIP note" +[ "$subj" = 'real subject' ] && [ ! -f "$count" ] && pass "pre-commit installing a message hook: subject rewritten, cycle reset" || fail "pre-commit installing a message hook: subject rewritten, cycle reset" +rm -f "$hooksd/pre-commit" "$hooksd/prepare-commit-msg" "$sandbox/rewrite-hook"; git commit -q --amend -m 'WIP: snapshot' >/dev/null 2>&1 # (c) prepare-commit-msg rewrites the subject into a real one: no WIP note before, reset after printf '%s' "$rewrite_hook" > "$hooksd/prepare-commit-msg"; chmod +x "$hooksd/prepare-commit-msg" lifecycle @@ -684,7 +710,6 @@ git reset -q --soft HEAD~1 >/dev/null 2>&1; git rm -q --cached wip.txt >/dev/nul if [ "$hooksd" != "$sandbox/refused" ]; then rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd" fi -unset GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM # On a non-WIP HEAD the kept message is a real one, so the reset stands reset_all; rev; rev amendpost "git commit --amend --no-edit" diff --git a/todos.md b/todos.md index 09e7ebc..efaa7c3 100644 --- a/todos.md +++ b/todos.md @@ -500,15 +500,16 @@ backlog. required), `--no-verify`, `-a`, `--all`, `-q` or `--quiet`, with no quote, `#`, backslash or shell metacharacter, when `HEAD`'s subject starts with `wip`, no custom `core.commentChar`/`core.commentString` is set, no `core.hooksPath` is set at all, and - the default hooks directory has no `prepare-commit-msg` or `commit-msg` hook (either - could rewrite the message). An + the default hooks directory holds nothing but `*.sample` files (stricter than Git's + own rule: a message hook could rewrite the message, and an earlier hook could install + one mid-commit; the check sees the directory only when the hook runs). An allow-list, because four Gate-B passes found a new bypass in every deny-list. Still open, deliberately, all in the safe direction: any other spelling of a WIP amend (global `git -c`, `cd … &&`, `-F`/`-C` with a WIP message) still resets, and a heredoc merely containing `WIP:` or `git commit` still fires. **Open in the unsafe direction, and older than this fix:** the `-m "wip…"` path trusts the typed message, so a - `prepare-commit-msg` or `commit-msg` hook that rewrites it into a real one gets the WIP - note and keeps the cycle (PR #30 review). Not fixed there, by scope. Also collected + hook that rewrites it into a real one — or installs one that does — gets the WIP note + and keeps the cycle (PR #30 review). Not fixed there, by scope. Also collected there: `CLAUDE.md` §5 Mechanics (and the `workflow-init` copy) and `docs/architecture.md` say a `wip`-prefixed commit *message* keeps the counters, but the hook reads the command and a few git settings, not the message — a two-copy prompt From 48e3fdad6654c9dab9c9fdc364d94fa4c2997838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20S=C3=A4nger?= <20968534+dsnger@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:48:35 +0200 Subject: [PATCH 4/4] Drop inherited command-scope git config in the hook suite (PR #30 review) CodeRabbit on PR #30: GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM do not clear command-scope config handed down through GIT_CONFIG_PARAMETERS or GIT_CONFIG_COUNT, so an inherited core.hooksPath could point at an external directory; 19c's guard would then fail while its lifecycle cases still ran real commits that execute those hooks. The suite now unsets both variables before its first git init, and a failed 19c setup exits the suite instead of continuing. Checked by hand: with GIT_CONFIG_PARAMETERS pointing core.hookspath at an external sentinel hook, the suite passes and the sentinel is unchanged. Test harness only; no hook change. Gate B: one logical pass (spec, then quality, same base and head ids), NO FINDINGS on both branches; closed on the zero-finding exit. No story cited, so no evidence entry is owed. cycle rb0hhnphfm; floor 3 per none; hook reminder threshold absent cycle rb0hhnphfm; Gate B (passes 1, codex): Findings 0. Blockers 0. Majors 0. Human exceptions: none --- .../codex-reviews/gate-b-quality-rb0hhnphfm-pass-1.md | 2 ++ .../codex-reviews/gate-b-spec-rb0hhnphfm-pass-1.md | 2 ++ plugins/dev-workflow/hooks/codex-gate.test.sh | 11 ++++++----- 3 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 .context/codex-reviews/gate-b-quality-rb0hhnphfm-pass-1.md create mode 100644 .context/codex-reviews/gate-b-spec-rb0hhnphfm-pass-1.md diff --git a/.context/codex-reviews/gate-b-quality-rb0hhnphfm-pass-1.md b/.context/codex-reviews/gate-b-quality-rb0hhnphfm-pass-1.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-quality-rb0hhnphfm-pass-1.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/.context/codex-reviews/gate-b-spec-rb0hhnphfm-pass-1.md b/.context/codex-reviews/gate-b-spec-rb0hhnphfm-pass-1.md new file mode 100644 index 0000000..8d781f4 --- /dev/null +++ b/.context/codex-reviews/gate-b-spec-rb0hhnphfm-pass-1.md @@ -0,0 +1,2 @@ +NO FINDINGS +END OF FINDINGS (0 total) diff --git a/plugins/dev-workflow/hooks/codex-gate.test.sh b/plugins/dev-workflow/hooks/codex-gate.test.sh index 23fe13e..0c7b66f 100644 --- a/plugins/dev-workflow/hooks/codex-gate.test.sh +++ b/plugins/dev-workflow/hooks/codex-gate.test.sh @@ -39,7 +39,9 @@ trap 'rm -rf "$work" "$sandbox"' EXIT # The whole suite runs with no global or system git config and an empty init template, so # a developer's core.hooksPath, comment character or template hooks can neither change # what the hook decides nor be written to by the cases that install hook scripts. +# Command-scope config handed down through the environment is dropped too. mkdir "$sandbox/template" +unset GIT_CONFIG_PARAMETERS GIT_CONFIG_COUNT GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TEMPLATE_DIR=$sandbox/template export GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM GIT_TEMPLATE_DIR cd "$work" || exit 1 @@ -630,8 +632,9 @@ printf 'wip\n' > wip.txt; git add wip.txt >/dev/null 2>&1; git commit -q -m 'WIP hooksd=$(git rev-parse --git-path hooks) if [ "$hooksd" = "$(git rev-parse --git-dir)/hooks" ] && { [ ! -e "$hooksd" ] && [ ! -L "$hooksd" ] || mv "$hooksd" "$sandbox/orig-hooks"; } \ && mkdir "$hooksd"; then :; else - fail "19c: could not set up the test repo's own hooks directory ($hooksd); lifecycle cases skipped" - hooksd=$sandbox/refused + # The cases below run real commits, which would execute whatever hooks that directory holds + fail "19c: could not set up the test repo's own hooks directory ($hooksd); aborting the suite" + exit 1 fi # shellcheck disable=SC2016 # $1 belongs to the git hook, expanded when git runs it rewrite_hook='#!/bin/sh @@ -707,9 +710,7 @@ reset_all; rev && pass "newline-ending core.hooksPath on a WIP HEAD: no WIP note" || fail "newline-ending core.hooksPath on a WIP HEAD: no WIP note" git config --unset core.hooksPath git reset -q --soft HEAD~1 >/dev/null 2>&1; git rm -q --cached wip.txt >/dev/null 2>&1; rm -f wip.txt -if [ "$hooksd" != "$sandbox/refused" ]; then - rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd" -fi +rm -rf "$hooksd"; { [ -e "$sandbox/orig-hooks" ] || [ -L "$sandbox/orig-hooks" ]; } && mv "$sandbox/orig-hooks" "$hooksd" # On a non-WIP HEAD the kept message is a real one, so the reset stands reset_all; rev; rev amendpost "git commit --amend --no-edit"