golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#3802): resolve the heredoc body before validating the commit subject (#3816)

Browse Source 
* fix(#3802): resolve the heredoc body before validating the commit subject

With hooks.community: true, gsd-validate-commit.sh blocked EVERY heredoc-form
commit with CONVENTIONAL_COMMITS_VIOLATION regardless of the message, including
Claude Code's own documented idiom:

    git commit -m "$(cat <<'EOF'
    feat(auth): add login flow
    EOF
    )"

Reproduced before changing anything: conforming heredoc -> exit 2; plain
-m "feat(auth): add login flow" -> exit 0.

Root cause is the extraction regex `-m[[:space:]]+"([^"]+)"`. Bash `[^"]`
matches newlines, so the capture ran from the quote after -m to the FINAL quote
at `)"`, swallowing the whole span. `head -1` then returned the literal
`$(cat <<'EOF'` as the subject, which can never satisfy Conventional Commits.

Fixed by not answering a regex bug with another regex. hooks/lib/git-cmd.js
already exists because "a naive regex misses all three" invocation forms, and
extractBranchArgument is the established precedent for pulling an argument off a
git command line. extractCommitSubject joins it on the same tokenizeShellLike
seam — which, checked first, already returns the entire heredoc span as ONE
token, leaving only "resolve the body to its first line" as new logic.

Because the walk starts at the subcommand, `git -C <path> commit` and
env-prefixed invocations now extract correctly too — forms the raw string scan
never handled.

Deliberately unchanged, and pinned as such: a glued `-mfeat: x` and
`--message=...` still yield no message, exactly as the regex left them. The fix
stays scoped to the reported defect rather than widening on a true observation.

Two things I got wrong and corrected by measuring rather than reasoning:

  - I expected `git commit -m ""` to be blocked. Checked against the ORIGINAL
    hook: allowed before, allowed now, identical. The scanner drops the empty
    token so it takes the null path. My expectation was wrong, not the code.
  - That exposed a false comment I had just written, claiming the exit-status
    split prevents silently allowing `-m ""`. It does not. The split IS
    load-bearing, but for a heredoc whose body's first line is blank, which
    resolves to an empty subject and is correctly blocked. The comment now names
    the real case and records that `-m ""` is not it.

Tests at both layers: 9 unit rows on extractCommitSubject beside its sibling in
tests/worktree-safety.test.cjs, and 5 behavioral rows piping real PreToolUse
payloads through the hook in tests/hooks-opt-in.test.cjs. Replacing
firstLineOfMessageArg with a plain first-line return reds 8 of them across both
files. (A first mutation attempt silently no-opped and reported green — the
mutated body is echoed in the transcript for the run that counted.)

Out of scope, per the issue: the hooks.commit_types config surface, split off by
the maintainer as #3811 and explicitly sequenced after this.

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): confine the fix to heredoc resolution, closing four regressions

Codex review of the first attempt. It was right, and the finding is one my own
rules already name: a true observation is not a licence to widen the diff.

The first attempt replaced the shell's `-m` extraction with a token walk. That
looked like the better abstraction — this module exists precisely because a
naive regex misses invocation forms — but selecting WHICH argument is the
message was never the defect, and changing it regressed four forms that
upstream allowed, plus opened a bypass:

  - `git commit -- -m WIP`             -- introduces pathspecs; `-m` is a path
  - `git commit --amend && echo -m WIP` a later command's flag became the message
  - `git commit -m "" --allow-empty-message`  the shared scanner drops empty
                                        tokens, so the next flag became the
                                        message
  - `git commit -m WIP`                unquoted argument
  - `-m "WIP notes <<EOF\nfix: smuggled subject"` was ALLOWED — the opener was
    recognised unanchored, so validation skipped past the real, non-conforming
    subject. An enforcement bypass, not a misclassification.

Now confined to the actual defect. The shell's `-m` capture is restored byte for
byte, and only the subject-from-message step is delegated, to a PURE STRING
helper `resolveCommitSubject()` that never tokenizes. Verified as a differential
against the upstream hook run inside the real tree: the only behaviours that
change are the two intended heredoc rows (2 -> 0); all four forms above read
identical, and the bypass case blocks.

That differential also corrected my own control. An earlier comparison ran the
upstream hook from a scratch directory, where its `lib/` could not resolve
`../../gsd-core/bin/lib/token-scanner.cjs`, so the classifier failed open and
reported exit 0 for everything. That made a real regression look pre-existing.
Re-run inside the tree, `<<-"TAG"` (a double-quoted tag nested in the
double-quoted argument) is genuinely pre-existing — the capture truncates — and
is now recorded as a known limitation rather than silently "fixed".

Also fixed from the review:
  - `<<-` strips leading TABS from body lines; returning the raw line blocked a
    conforming message.
  - a non-identifier tag such as `END-MSG` is a valid bash word and was rejected.
  - an immediately-following terminator is an EMPTY message, not a subject.
  - a node/library failure now falls back to the previous `head -1` instead of
    skipping validation, so a broken extractor degrades to old behaviour rather
    than becoming a new silent-allow path.

Tests strengthened per the review: the opener-spelling rows now assert BOTH
directions per spelling, since "conforming passes" alone would also pass if the
resolver returned an empty subject for a spelling it failed to parse. Added
differential rows pinning the five previously-allowed forms, and a row for the
bypass. Dropped two rows whose comments claimed the raw scan could not handle
`-C`/env-prefix invocations — it could; the claim was wrong.

Replacing resolveCommitSubject with a plain first-line return reds 9 rows across
both files. (Mutant body echoed in the transcript; an earlier mutation attempt
on this branch silently no-opped and reported green.)

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): keep the installed hook runtime-neutral

`hooks/lib/git-cmd.js` ships into every runtime, including hermes and qwen,
where tests/install.test.cjs enforces that no Claude reference leaks into the
installed tree. My JSDoc named the idiom after the runtime that documents it.

Reworded to describe the SHAPE rather than the vendor; the runtime is still
named in the changeset, which feeds CHANGELOG.md where such references are
allowed, and in the tests, which are not installed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(#3802): backfill changeset pr number

The fragment shipped with the documented `pr: 0` placeholder, which the
changeset lint treats as always-silent, because the number does not exist until
the PR is opened. Backfilled to 3816 now that it does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): close the truncated-capture hole, add the required test artifacts

Review round 1. Major 3 was the one that mattered, and it disproved a claim I
had stated in falsifiable form — the PR body said only two behaviours change;
the differential found five.

Major 3 — an embedded `"` truncates the `-m` capture, so the resolver received a
PREFIX of the real subject and the length gate measured the wrong string. Before
this fix the whole form was blocked outright, so the gate was unreachable; the
fix opened the path and then mismeasured it. A new enforcement hole, so it is
CLOSED here rather than declared.

Closed precisely rather than bluntly. A first attempt refused to resolve any body
with no terminator, which also blocked commits whose SUBJECT was intact and whose
quote sat further down the body — a false positive of its own. Truncation is only
fatal to the line it lands IN, and a captured line is complete exactly when
another line follows it, because the capture kept its newline. So an unterminated
body whose subject line is followed by more text stays measurable; only a subject
line running to the end of a truncated capture falls back to the opener, which
fails the format gate exactly as this form did before the fix.

Major 1 — fast-check property rows for the new parser, via the shared seeded
setup helper rather than requiring fast-check directly, per repo convention:
totality (a security property here, since an exception on this path fails OPEN),
idempotency, and that the result is always a single line drawn from the input —
the third catches a resolver that concatenated or trimmed while satisfying the
first two.

Major 2 — the 72-char gate is now exercised at {71, 72, 73} on the RESOLVED
heredoc subject, with the fixture length asserted so a mis-built fixture cannot
silently pass. 92 chars did not show which side of `> 72` the code sits on.

Minor 1 — leading blank body lines are skipped, as git's cleanup=whitespace does.
A conforming commit written that way was still blocked, which is the same defect
class #3802 reports.

Nit 1 — a backslash-escaped delimiter (`<<\EOF`) is now the same delimiter rather
than failing closed on a delimiter that includes the backslash.

Nit 5 — changeset trimmed from 2,208 chars of design note to the user-visible
change.

Mutation discipline, including a correction to my own: dropping the truncation
guard reds the unit rows, and the pre-review naive shape reds the hook-level row
too. My first mutant did NOT distinguish the hook row — removing the guard made
an empty slice and blocked for an unrelated reason, so the row passed and looked
proven. Only mutating to the actual pre-review shape showed it discriminates.

Verified: `npm run lint:ci` exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(#3802): measure the subject as git does — strip trailing whitespace, split CRLF

git's cleanup=whitespace strips whitespace at BOTH ends of a line; the
resolver handled only the leading direction, so a 72-char subject with
trailing spaces measured 75 and stayed blocked — the defect class #3802
reports, surviving one round further (review of #3816, Major 2). The
resolved subject now drops trailing spaces and tabs; the plain non-
heredoc path is untouched, keeping the fix confined to heredoc
resolution. The length-gate boundary rows gain dirty fixtures: 72+3
trailing spaces passes, 73+1 stays blocked on LENGTH.

split('\n') left \r on every body line, so on CRLF input the delimiter
never matched: the truncation guard was inert, an empty message resolved
to 'EOF\r', and a real 72-char subject measured 73. Split on /\r?\n/
(Minor 3).

The three property tests never reached the parser — the pinned-seed
fc.string corpus contained no newline and no opener, so every property
reduced to f(s) === s (Major 1). The generator now constructs heredoc-
shaped input (all opener spellings, <<- tabs, optional terminator, CRLF)
and each property asserts a floor on inputs its corpus actually resolved.
All new rows proved failing-first against the pre-fix resolver.

Also records the unquoted-delimiter expansion limit as one JSDoc
sentence (Informational 5).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): close two recognition bypasses, pin the dquoted-delimiter limit

Codex whole-PR review found two enforcement bypasses in the resolver:

- The opener's path prefix was \S*, which accepted `id;/bin/cat` — the
  resolver then validated the heredoc BODY while bash runs `id` first
  and git's real subject is id's OUTPUT. The prefix is now a
  path-character class; any shell metacharacter fails recognition and
  the form falls back to the opener line and the format gate.

- The blank-line skip used JavaScript trim(), whose Unicode whitespace
  class skips lines git KEEPS: a NBSP first body line resolved to the
  SECOND line while git's real subject is the NBSP line (verified
  against git stripspace — the c2a0 bytes survive). Blank is now git's
  ASCII space/tab only; a Unicode-blank line is returned and fails the
  format gate, the same fail-closed direction git takes.

Both proven failing-first at resolver AND hook level. Also: the
<<"TAG" spelling is recorded as a documented limit — the -m capture
stops at the delimiter's own quote so the caller can never deliver it
(fail closed; widening the capture would change every embedded-quote
case) — with a hook-level row pinning the limit; and the derivation
property no longer accepts '' unconditionally, only for heredoc-shaped
input, so a conditional constant-'' regression can't satisfy the corpus
floor unnoticed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): recognition whitespace is ASCII, and '' answers to the generator

Codex round 2: the opener's \s accepted Unicode whitespace bash does
not split on — $(<NBSP>/bin/cat was recognized here while bash reads
<NBSP>/bin/cat as the executable NAME, so recognition claimed a
substitution that does not run cat. Every whitespace position in the
recognition is now [ \t], the same ASCII rule as the blank-line skip,
proven failing-first.

The derivation property's ''-acceptance now consults GENERATION-TIME
metadata: the heredoc generator records whether it built an empty
message (terminator reachable, all scanned lines ASCII-blank, <<- tab
stripping accounted for), and '' is accepted exactly then — a resolver
conditionally degrading to '' on non-empty heredocs now fails, closing
the residual round-1 permissiveness without re-deriving resolver logic.

The changeset no longer overstates the opener spellings: it names the
capture-deliverable set and the documented <<"EOF" limit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): nothing after the terminator escapes measurement

Round-3 BLOCKER: everything after the heredoc terminator was silently
discarded, so `-m "$(cat <<'EOF'\nfeat: ok\nEOF\n) <200 a's>"` —
one 200+ char real subject once bash substitutes — measured 8 chars and
dodged COMMIT_SUBJECT_TOO_LONG, a hole the base did not have. The
canonical idiom's tail is exactly one closing-paren line; any other tail
now falls back to the opener line and the format gate, the pre-fix
behaviour for the whole form. Proven failing-first at resolver and hook
level, including the glued-text and second-substitution variants.

Also from round 3: `cat<<'EOF'` (no space) is legal bash and now
resolves — the token before << is still literally cat; the env-prefixed
and option-terminated spellings join the JSDoc KNOWN LIMIT list instead
(fail closed, modelling bash prefix words is cost with no reported
user); the changeset states the embedded-quote truncation limit for the
message body, not just the <<"EOF" spelling; the dquoted unit and hook
rows now cross-reference each other; and the fast-check setup helper's
docstring no longer claims property-file exclusivity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): glued text outside the closing quote must not shrink the measurement

Codex on the round-3 guard: bash concatenates -m "$(…)"suffix into ONE
argument, but the capture holds only the quoted part — so the resolver
measured the heredoc body (8 chars) for a 200+ char real subject, a
net-new length-gate bypass the base did not have (base measured the
opener and blocked). When the closing quote is followed by anything but
whitespace or end-of-command, the hook now skips the resolver and keeps
the pre-fix first-line subject: the heredoc form fails the format gate
exactly as on base, and the plain single-line form keeps base behavior
unchanged — both pinned as differential rows, the glued-suffix row
proven failing-first against the unguarded script.

The property generator's ''-oracle now models the post-terminator guard
it previously predated: expectEmpty requires the FIRST reachable
terminator to be followed by the one canonical closing-paren line, so a
resolver regressing to '' on a non-canonical tail (e.g. a body line that
doubles as an early terminator) fails the derivation property instead of
being blessed by stale metadata.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: retrigger CI — the previous wave never started (Actions queue stall)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(#3802): only resolve a heredoc whose body bash does not rewrite

Round-4 review found two net-new enforcement bypasses: commands the base
hook blocked (exit 2) that this branch allowed (exit 0). Both reproduced as
a base-vs-head differential against the real hook, not inferred.

The predicate "may I resolve this?" was computed from the resolver's input
string alone, while two of its determinants live outside that string:

  1. WHICH -m quote arm produced the input. Inside -m '...' bash performs no
     command substitution, so $(cat <<'EOF' is literal text and git's real
     subject is the opener line. The resolver ran on both arms, so all four
     delimiter spellings went 2 -> 0 on the sq arm — reachable by the
     ordinary slip of typing ' for ". The hook now records MSG_QUOTE and
     gates the resolver on dq; sq keeps head -1, exact base parity.

  2. WHETHER the delimiter suppresses expansion. Only <<'D', <<"D" and <<\D
     do; a bare <<D is expanded by bash before git sees it. Resolving the
     literal dodged the format gate (feat: $UNSET_VAR reaches git as feat:)
     and the length gate (feat: ${LONG} reaches it at any length). The
     opener regex now separates the backslash-quoted and bare alternatives
     and refuses the bare one — the same fail-closed rule the metacharacter,
     truncation and post-terminator guards already follow.

A test row asserted exit 0 for a bare-delimiter body, so the suite defended
the second bypass and the fix could not land without editing a test that
read as intentional. That row and its two unit counterparts now assert the
block, per RULESET.TESTS.delete-bad-tests. Two unrelated rows used <<-EOF
to exercise tab stripping; they move to <<-'EOF' so each tests what it names.

Scoping the adjacency guard to the matched arm — required by the fix above —
also removes a spurious block (round-4 Minor 1): a double-quoted heredoc
whose body mentioned a glued single-quoted token tripped the sq arm.

The JSDoc claimed <<"EOF" was unreachable through the caller and that the
bare-delimiter gap was pre-existing. Round 4 disproved both; both corrected
here, along with the matching changeset sentence.

Verified: 7 bypass commands now block at head (was allow), the #3802 fix and
plain-form parity are unchanged across 8 control commands, hooks-opt-in 44/44,
worktree-safety 401/401, property-test non-vacuity 73/200 against a floor of
20, lint:ci exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3802): resolve only where the captured text is provably git's subject

Codex review of the full PR found two more inputs where the validated text
is not the subject git receives, both net-new bypasses (base 2 -> head 0),
plus one escalation of round-4 Minor 2. All reproduced here against the real
hook and confirmed against real commits before fixing.

BLOCKER — the matched -m need not be git's message. The capture is a search
over the whole command and the double-quoted arm runs first, so it could
select a -m that is not the subject at all. git concatenates multiple -m
values and takes the FIRST as the subject, so

    git commit -m 'WIP first' -m "$(cat <<'EOF' … )"

commits the subject `WIP first` while the hook validated the heredoc. Same
for an unquoted earlier -m, for a heredoc after `--` (a pathspec, not a
message), and for one belonging to a later `&& echo`. The mis-selection is
pre-existing; resolving it is what made it a bypass. The hook now resolves
only when nothing before the matched -m could have been an earlier message,
an end-of-options marker, or another command.

BLOCKER — cleanup mode is part of the predicate. The resolver skips leading
blank lines and strips trailing whitespace because git's DEFAULT
cleanup=whitespace does. Under --cleanup=verbatim git does neither, so a
72-char subject plus three trailing spaces is committed at 75 bytes while
the hook measured 72 — COMMIT_SUBJECT_TOO_LONG dodged. This one hides from
`git log --pretty=%s`, which strips trailing whitespace in its own output;
the raw commit object shows 75 vs 72. Any named mode other than whitespace,
in either the --cleanup= or -c commit.cleanup= form, now refuses to resolve.

MAJOR — recognition trusted any path ending in /cat, so a planted
`../evil/cat` printing `WIP injected` had its heredoc body validated while
git's real subject was `WIP injected`. Only a bare `cat` or an absolute path
is recognised now. A bare `cat` shadowed on PATH is a documented residual and
is not fixable from a string — nor a meaningful boundary, since planting an
executable already allows running git directly.

The changeset and the JSDoc both asserted that a `"` anywhere in the message
blocks. Measured false: a `"` on a later body line resolves fine, because the
subject completes before the truncation point; only a `"` in the subject line
blocks. The changeset also listed <<"EOF" as covered when it measures 2/2.
Both rewritten to claim only what is measured, and the residual false
positives are now named.

Verified: 4 + 2 + 3 new bypass commands now block, with non-vacuity controls
proving the default path still resolves; all round-4 maintainer blockers stay
closed; the #3802 fix and plain-form parity unchanged across 7 controls;
hooks-opt-in 47/47, worktree-safety 402/402, property non-vacuity 73/200
against a floor of 20, lint:ci exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EAbQy7n4mLMB7h3TnZ8GdG

* fix(#3802): scope the cleanup-mode guard to the command outside the message

The guard scanned the whole $CMD for `--cleanup=` / `commit.cleanup=`,
and the heredoc BODY sits verbatim inside $CMD, so any conforming
message that merely MENTIONED the token was refused, fell back to the
opener line, and was blocked with CONVENTIONAL_COMMITS_VIOLATION. These
are ordinary English in this repository, whose own hooks and docs
discuss cleanup modes constantly. Reproduced against the real hook:
`fix: document commit.cleanup=strip behavior` blocked, the same message
without the token allowed (review of #3816, round 5 — BLOCKER).

Scoping to $MSG_PREFIX alone, as prescribed, would have reopened the
round-4 length-gate bypass the guard exists for: git accepts the flag on
EITHER side of -m, and `git commit -m "<heredoc>" --cleanup=verbatim` is
caught today only because the scan is command-wide. Measured, not
assumed. The scan now covers MSG_PREFIX + MSG_SUFFIX — the whole command
minus the one span that is message text — joined with a space so a token
cannot be forged across the seam.

Swept the guard class rather than the reported instance. The adjacency
guard does not share the defect: an in-body `-m "foo"bar` is refused by
the already-documented embedded-quote capture limit (any `"` in the
subject line truncates the capture), and an in-body `-m ` without quotes
resolves and is allowed. Deliberately untouched.

Both directions pinned failing-first: the three false-positive rows red
against the unscoped guard, and the trailing-flag row reds against
prefix-only scoping. Each mutation was echoed back to prove it landed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XogDtuuuGQEfsWaLSaZCLB

* fix(#3802): read commit options the way bash hands them to git

Round 6 reported the adjacency guard scanning all of $CMD for a glued
`-m "..."`, so a glued -m belonging to a chained-after command refused a
heredoc that was never truncated. Glue is a property of the ONE character
following the matched span, so that character is now the whole window.
Separators and redirections are excluded because bash does not
concatenate across them: in `-m "msg"&& echo hi` the argument ends at the
quote, so there is no truncated capture to defend against.

An independent full-PR pass then found three accept-direction defects
this PR had introduced in earlier rounds, each measured against a real
commit by reading the raw commit object — `git log --pretty=%s` strips
the trailing whitespace that makes the length wrong and hides it:

  --cle=verbatim         git accepts any unambiguous prefix of a long
                         option, so the mode was set by a token that is
                         not the literal --cleanup. 75-byte subject
                         recorded, 72 measured.
  -am 'WIP first'        git reads this as -a -m, so the real subject is
                         `WIP first` and the heredoc is only the second
                         message. The scan looked for a standalone -m.
  --clean""up=verbatim   bash removes quotes before git sees the
  -""m                   argument, so a spliced spelling is the same
                         option and matched no literal.

The two option-name scans now read their window with quote characters
removed, which is what bash does to it, and the cleanup class covers
git's abbreviations. The adjacency test deliberately keeps the raw text:
it asks about a literal character position, not an option name.

Narrowing the cleanup window to git's own command segment was tried and
reverted. `;`, `&` and `|` end a command only outside quotes, and this is
a substring scan, not a parse: an unconditional trim cut the window short
on `--author "a&b"`, and a quote-aware trim still cut it on `--author
a\&b`. Each hid a real trailing --cleanup=verbatim and accepted a 75-byte
subject. The resulting false positive — a --cleanup carried by a chained
command refuses the commit — is documented and pinned instead. Refusing a
commit git would take is recoverable; accepting an over-long subject is
not.

Sixteen rows in tests/hooks-opt-in.test.cjs. Seven mutations, including
both reverted narrowings, so no dead end can be reintroduced silently.

* fix(#3802): close six accept-direction bypasses in the resolve guards

Round 7's FIRST-MESSAGE GUARD Major does not reproduce. Measured against the
real hook in a complete tree at the reviewed head: the classifier gate runs
before any guard, so `git add -A && git commit …` (git->add stops on a
non-commit subcommand) and `cd dir && git commit …` (the first executable is
not git) exit 0 without a guard being evaluated. The control is the proof — a
subject the bare form blocks with CONVENTIONAL_COMMITS_VIOLATION exits 0 in
both chained forms, so the hook never validated them and cannot be
over-blocking them. The guard is unchanged; scoping this scan to $MSG_PREFIX
alone is what reopened the round-4 trailing-flag bypass.

The class was real, though, one shape further out: `FOO=bar; git commit …` IS
classified and then refused, because assignment detection is prefix-anchored
and the tokenizer does not split operators. Pinned as a counterexample and
disclosed rather than generalised away; narrowing it means changing
isGitSubcommand, the shared git-commit detector every gating hook uses, and it
fails closed.

Six accept-direction bypasses are fixed. Each let the hook resolve and ALLOW a
commit whose real subject the rules refuse; the three that turn on git's
recorded subject were confirmed against the RAW COMMIT OBJECT, since
`git log --pretty=%s` strips trailing whitespace and hid two of them:

  --cleanup=whitespace -m <72+spaces> --cleanup=verbatim  git kept 75 bytes
  -mWIP -m <heredoc>                                      git recorded `WIP`
  --mes=WIP -m <heredoc>                                  git recorded `WIP`
  -\m WIP -m <heredoc>                                    git recorded `WIP`
  git commit --amend --no-edit \n echo -m <heredoc>        echo's argument read
  --squash=HEAD -m <heredoc>                              `squash! …`

Causes: one BASH_REMATCH inspected only the FIRST cleanup directive while git
applies the last, so multiplicity now refuses rather than guesses at an
argument order a substring scan cannot recover; the option scan required a
trailing space or `=`, missing attached values and long-option abbreviations;
dequoting removed quotes but not the syntactic backslashes bash also removes;
the separator scan omitted newline; and --squash/--fixup have git compose the
subject, so the supplied message is not the subject at all. Every fix widens
refusal, the direction this file documents as recoverable.

The multiplicity count first broke the hook outright: the script runs under
`set -euo pipefail` and grep exits 1 when it matches nothing, which is the
common case, so every ordinary commit died at exit 1 with no verdict. Guarded,
and only caught because the probe runs the real hook rather than the scan.

Five new rows, all five proven red against the pre-fix hook, each carrying a
non-vacuity assertion that the canonical single-`-m` heredoc still resolves.
Changeset corrected on three counts: "all fail-closed" was wrong (persistent
commit.cleanup fails OPEN, as do the -C/-c/-F/-t message sources), "global
options are all walked through" was too broad, and the chained-before claim
now states what is measured.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018FUAVz49BghqxoJgwt7EW9

* fix(#3802): stop the separator and glue classes matching a literal backslash

Round 8's Major, with two corrections to its account.

`;`, `&` and `|` are metacharacters inside `[[ ]]`, so an inline bracket
class must escape each one. POSIX bracket expressions have no escape
mechanism of their own, so on bash 3.2 -- the system /bin/bash on macOS,
already a supported target here per the `declare -A` ban in
tests/install.test.cjs -- those backslashes reach the regex engine and add
a literal `\` to the class. bash 4+ consumes them, which is why this is
invisible on a modern bash. The hazard is specific to bracket
expressions: `\(` outside one is made literal correctly on every version,
and the subject validator and the `-m` capture classes were checked and
are unaffected.

The prescribed fix is not taken, because it does not parse. Inline
`[;&|]` is a bash SYNTAX ERROR on 3.2 and on 5.3 alike -- the backslashes
exist to get the metacharacters past the `[[ ]]` parser, so removing them
leaves an unparseable script. Each class is held in a variable and
expanded unquoted on the right of `=~` instead, which is a plain regex on
both versions.

One root cause, consequences in BOTH directions. The reported half is the
separator scan over-blocking. The half not reported is the accept
direction, and it is the more serious: the glue class is NEGATED, so on
bash 3.2 a backslash-glued suffix fell inside the exclusion and the hook
RESOLVED a heredoc it should have declined -- measured exit 0 on 3.2
against the unfixed hook, exit 2 everywhere else, with a letter-glued
control refused in all four cells.

The reported repro is not actually fixed by this, and the changeset says
so. A `\`-newline line continuation carries a literal newline, which the
round-7 separator guard refuses on every bash, so that shape stays
blocked with or without this change. Narrowing the newline guard is not
attempted: telling a continuation from a separator by substring scan is
the class that was tried twice in earlier rounds and reverted both times,
and an escaped backslash sitting immediately before a real newline is
indistinguishable from a continuation. Disclosed as a known fail-closed
limit instead.

Every new row runs under each bash on the machine. Against the unfixed
hook both bash 3.2 rows go red while all four bash 5.3 rows stay green --
written the ordinary way these rows would run under PATH bash, pass
against the broken hook, and prove nothing. Two non-vacuity controls per
interpreter prove the validator is reached rather than passing
everything. All 8 rows of the established differential harness are
byte-identical before and after on both versions: no regression, no new
refusal.

* fix(#3802): remove the $ of a dollar-quote from the option-name scans

Independent round-8 review, accept direction.

The option-name windows are dequoted so they match "the command as bash
hands it to git" -- round 6 removed quote characters, round 7 removed
syntactic backslashes. Both passes missed that bash has two further
quoting forms whose introducer is a `$`: `$'...'` and `$"..."`. Removing
the quote characters alone left that `$` stranded INSIDE the option name,
so `-$"m"` dequoted to `-$m` and matched no literal, while bash passed a
real `-m` to git.

Measured on bash 3.2.57 and 5.3.15 against a real repository: the hook
allowed

    git commit --allow-empty -$"m" WIP -m "$(cat <<'EOF'
    fix: a perfectly ordinary conforming subject
    EOF
    )"

with exit 0, and `git cat-file -p HEAD` recorded the subject `WIP`.

The comparison that establishes this is HEAD-internal, not a differential:
the same command spelled `-m WIP` is refused (exit 2). The merge-base
refuses EVERY heredoc form, including a perfectly conforming one, so its
exit 2 on this input says nothing about whether any guard fired -- it is
the absence of the feature, not a working check. The same miss covered
`$'m'`, spliced `--message`, `--cleanup`, `--squash` and `--fixup`.

An option NAME finished by a command substitution -- `--clean$(printf
up)=verbatim` -- is a different problem and gets its own guard: bash runs
a program to complete the name, so the argv git receives is not derivable
from this string at all, and resolution is refused rather than guessed.
The guard is scoped to the NAME: the class is a `-`-leading token whose
characters up to the substitution contain no `=`. A substitution
supplying a VALUE -- the ordinary `--author="$(git config user.name)"`,
spaced or glued, in either window -- is untouched and still resolves,
pinned in both directions. It is a SHAPE, not a segmentation of the
command line; segmenting was tried twice in earlier rounds and reverted
both times, and that reasoning stands.

Both new rows fail against the unfixed tree with their own assertions,
proven in a complete worktree at the previous head rather than a hook
copied out of its tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): recognise a canonical cat, not any absolute path ending in /cat

Independent round-8 review, accept direction.

Round 4 restricted heredoc-opener recognition to an absolute path, after a
relative `./cat` was measured being trusted to echo its stdin. It stopped
at "absolute", so any absolute path ENDING in `/cat` was still trusted --
the same claim the round-4 reasoning had rejected one spelling earlier.

Measured on bash 3.2.57 and 5.3.15 against a real commit: with an
executable at `/.../fake-cat/cat` printing `WIP injected`, the hook
validated the conforming heredoc body and allowed the commit (exit 0)
while `git cat-file -p HEAD` recorded the subject `WIP injected`. The
same command through `./cat` was already refused, which is the control
that shows this is the round-4 class one spelling out rather than a new
one.

Recognition is now the canonical system locations -- bare `cat`,
`/bin/cat`, `/usr/bin/cat` -- which is the only identity claim a string
can support. `/usr/local/bin` is deliberately excluded: it is
user-writable on ordinary machines, which is the plantable case this
guard exists for. Anything else falls back to the opener line and the
format gate: fail closed, exactly the pre-fix behaviour for the form.

The pre-existing residual is unchanged and still documented: a bare `cat`
shadowed earlier on PATH is indistinguishable here, and is not a
meaningful boundary -- anyone able to plant an executable on PATH can run
`git commit` directly. This hook stays an authoring guard, not a security
control.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): an option name carrying a shell expansion is unresolvable

Independent review, round 9, accept direction. Four more spellings, and a
change of strategy that is the actual point of this commit.

Rounds 6, 7 and 8 each tried to EMULATE what bash does to an argument
before git sees it -- round 6 removed quote characters, round 7 syntactic
backslashes, round 8 the `$` that introduces a dollar-quote -- and each
round review found another transform that had been missed. Round 9 found
four more. All measured on bash 3.2.57 and 5.3.15 against a real
repository, each with the plain spelling of the same command as its
control (refused, exit 2) and `git cat-file -p HEAD` for the subject git
actually recorded:

    -$'\155' WIP        hook 0, real subject `WIP`   ANSI-C octal -> m
    -$'\x6d' WIP        hook 0, real subject `WIP`   ANSI-C hex   -> m
    -`printf m` WIP     hook 0, real subject `WIP`   backtick substitution
    x= … -${x}m WIP     hook 0, real subject `WIP`   parameter expansion
    -? WIP              hook 0, real subject `WIP`   pathname expansion

and the same class through the cleanup guard, where git recorded a
75-character subject the length gate had measured as 72:

    --cle$'\141'nup=verbatim, --clean`printf up`=verbatim, --cle?nup=verbatim

The last two settle it. An option name finished by a PARAMETER expansion
depends on a variable's value at run time; one finished by a PATHNAME
expansion depends on the contents of the working directory. Neither is
derivable from the command string at any level of effort, so emulation
cannot be completed -- not "has not been completed yet". A fifth patch in
that direction would have the same shape as the previous four.

The rule is therefore no longer "normalise it and match the literal". It
is: an option NAME carrying a shell expansion or quoting construct is
UNRESOLVABLE, and unresolvable refuses. One rule covers every spelling
above and every spelling nobody has thought of yet, in the fail-closed
direction. The dequoting passes are kept rather than replaced: they still
normalise the deterministic removals, so the guards RECOGNISE
`--clean""up=` and `-\m` as the options they are instead of merely
refusing them, which keeps the existing rows meaningful.

Scope is unchanged and still pinned in both directions: the class is a
`-`-leading token whose characters up to the construct contain no `=`, so
a construct supplying a VALUE -- `--author="$(git config user.name)"`,
the backtick spelling, `--date="${NOW}"`, a glob character inside an
author string, a pathspec after `--` -- still resolves. Nine such forms
are asserted to pass beside the seven that must refuse.

The class is bracket-only and holds no backslash, per round 8: a POSIX
bracket expression has no escape mechanism, and a backslash written
inside one becomes a literal member on bash 3.2.

The new rows fail against the previous head with their own assertion
message, in a complete worktree with the lib built, not a copied hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* docs(#3802): disclose and pin the two spellings the round-9 class over-blocks

A scoped review of the round-9 class asked one question -- does it refuse a
conforming heredoc commit that the previous head accepted -- and found two
spellings that it does. Both measured on bash 3.2.57 and 5.3.15, previous
head 518d97b64 exit 0, current head exit 2:

    git commit -S$SIGNING_KEY -m <conforming heredoc>
    git commit -m <conforming heredoc> -- -*.txt

Disclosed and pinned rather than narrowed, for two reasons.

Narrowing is not available cheaply. Dropping the bare `$` member reopens
`-$xm`: with `xm=m` bash hands git a real `-m`, which is the parameter
expansion bypass the round-9 commit exists to close. Skipping tokens after
`--` means deciding where git's options end from a substring scan, which
is the class this file has already reverted twice for opening
accept-direction holes -- a `--` inside a quoted value (`--author "a -- b"`)
would truncate the window and hide a real trailing directive.

And the limits are narrower than they look, because in both cases the
spelling a developer actually reaches for still resolves:

    -S "$KEY"  and  --gpg-sign="$KEY"        resolve
    '-*.txt', "-*.txt", ':(exclude)-*.txt'   resolve

The pathspec one is worth stating precisely: a glob only reaches git AS a
pathspec when it is quoted, because an unquoted one is expanded by the
shell before git is executed. So the refused spelling is not passing a
glob to git at all, and the spellings that do are unaffected.

Refusing a commit git would take is the recoverable direction; accepting a
non-conforming subject is not. That is the trade this file already makes
everywhere else, and it is made explicitly here.

Nine rows pin the working spellings beside the three that refuse, so a
later narrowing cannot silently drop the cases that must keep working.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

* fix(#3802): join backslash-newline continuations before the resolve guards

Round 9's Major, with a correction to its diagnosis.

The cited bracket classes at :223 and :260 no longer exist -- round 8
moved both into SEP_CLASS and GLUE_CLASS, and a lone backslash before -m
resolves (exit 0) at the reviewed head on both bash 3.2.57 and 5.3.15.
What refuses the repro is the NEWLINE a `\`-continuation carries: round
7's separator guard reads any newline in a window as a command boundary,
and `git commit \` newline `  -m "$(cat <<'EOF' …` was refused for that
reason. Round 8 disclosed it as a fail-closed limit; round 9 calls the
idiom common and the limit a Major, and it is fixed here.

It was left as a limit because "is this newline a continuation" looked
like the segmentation question this file has reverted twice. It is not:
bash's rule is local and character-level. A newline preceded by an ODD
run of backslashes is a continuation and bash removes both; an EVEN run
(`\\` then newline) is a literal backslash followed by a real newline,
which IS a separator. Both scan windows are joined that way immediately
after they are cut from the command and before any dequote copy is
derived, in three bash-3.2-safe parameter expansions: every `\\` pair is
parked on \x01, any backslash-newline that remains is a lone one and is
removed, then the pairs are restored.

Measured on both bashes, both directions:

    git commit \<nl>  -m <heredoc>                 2 -> 0   the fix
    git commit \\<nl>  -m <heredoc>                2 -> 2   literal \ + real separator
    git commit<nl>  -m <heredoc>                   2 -> 2   bare newline
    -m <heredoc>\<nl>suffix                        2 -> 2   bash glues it; the glue guard sees it glued
    git commit … \<nl>  --allow-empty<nl>echo -m … 2 -> 2   the REAL newline still separates

The prescribed `[\;&|]` is not taken: a backslash written inside a
bracket expression becomes a literal member on bash 3.2, which is the
round-8 defect from the other side.

Rows run under each bash on the machine. The fix row fails against the
previous head in a complete worktree with the lib built; the four control
rows were measured against that same head and were already refused, so
they pin existing behaviour rather than the change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TadqrpTE2m6gCB7CaNNLcy

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Behruz Nassre Esfahani
2026-09-04 20:51:43 -07:00
committed by GitHub
parent c27da5e993
commit 4499933807
6 changed files with 2009 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
.changeset/lucky-pandas-commit.md Normal file
View File

@@ -0,0 +1,82 @@
---
type: Fixed
pr: 3816
---
**The commit-message hook no longer blocks every heredoc-form commit** — with `hooks.community: true`, `gsd-validate-commit.sh` rejected `git commit -m "$(cat <<'EOF' … EOF)"` with `CONVENTIONAL_COMMITS_VIOLATION` whatever the message said, because its `-m` capture matches across newlines and the message's first line was the literal `$(cat <<'EOF'` rather than the subject. That opener is a standard agent-authored commit idiom, so enabling the toggle — which also carries the session-state and phase-boundary hooks — made that pattern fail every time.
The subject is now resolved from the captured message before validation, for the canonical form: a single `-m "…"` holding one `$(cat …)` substitution, under git's default `cleanup=whitespace`. Resolution handles the delimiter spellings bash does not expand (`<<'EOF'` and `<<\EOF`, with or without `<<-`, spaced or space-free), the leading tabs `<<-` strips, CRLF line endings, and both directions of `cleanup=whitespace` — leading blank lines are skipped, and trailing whitespace on the subject is not counted against the 72-character limit.
Everywhere the validated text could differ from the subject git actually receives, resolution is refused and the commit stays blocked exactly as it was before this change. That covers: a `-m '…'` single-quoted argument, in which bash performs no command substitution at all; a bare `<<EOF` delimiter, whose body bash expands; a `-m` that is not git's first message argument, since git concatenates multiple `-m` values and takes the first as the subject; an explicit `--cleanup=` or `-c commit.cleanup=` mode other than `whitespace`, including git's
abbreviated spellings of it (`--cle=verbatim` and anything else that is an unambiguous prefix);
a message argument claimed by a bundled short option, since git reads `-am 'first'` as `-a -m` and
takes that first message as the subject; a `cat` reached by anything but a canonical system path (`cat`, `/bin/cat`, `/usr/bin/cat`), since an arbitrary executable merely named `cat` is not known to echo its stdin; a substitution composed with more text on either side of the terminator; and a `"` inside the subject line itself, which the quote-bounded capture cannot span.
Option names are handled in two layers, because trying to reproduce bash's argument processing
by itself does not terminate. The first layer normalises the removals that are deterministic —
quote characters, syntactic backslashes, and the `$` that introduces a dollar-quote — so a spliced
spelling like `--clean""up=verbatim`, `-""m`, `--clean\up=verbatim`, `-\m`, `-$"m"` or
`--mes$'sage'=WIP` is recognised as the option it actually is rather than slipping past a literal
match.
The second layer is the general rule, and it is what the guarantee rests on: **an option NAME
carrying a shell expansion or quoting construct is unresolvable, and unresolvable refuses.** A
name finished by a command substitution in either spelling (`--clean$(printf up)=verbatim`,
``--clean`printf up`=verbatim``), by an ANSI-C escape (`-$'\155'`, `-$'\x6d'`), by a parameter
expansion (`-${x}m`), or by a pathname expansion (`-?` where a file named `-m` exists) does not
resolve. The last two are the reason the rule is framed this way rather than as a longer list of
removals: a parameter expansion depends on a variable's value at run time and a pathname expansion
on the contents of the working directory, so neither is derivable from the command text at all.
The scope is the NAME — a construct supplying a VALUE, as in the ordinary
`--author="$(git config user.name)"`, is unaffected and still resolves. A message option is also recognised when its value is attached (`-mWIP`,
which git reads as `-m WIP`) and when its name is abbreviated (`--mes=WIP`), and a newline is
treated as a command separator alongside `;`, `&` and `|`, so a later command's `-m` is never
mistaken for this commit's message. Where more than one `cleanup` directive appears, resolution
is refused rather than guessed: git applies the last one, and argument order is not recoverable
from a substring scan. Modes in which git composes the subject itself (`--squash`, `--fixup`)
refuse resolution outright, because the supplied message is not the subject in them at all.
A `\`-newline line continuation — `git commit \` then `-m …` on the next line — is joined before any scan runs, the way bash joins it: a newline preceded by an odd run of backslashes is a continuation and both are removed, while an even run is a literal backslash followed by a real newline, which stays a separator and still refuses. So the ordinary multi-line invocation resolves, and a continuation glued to the closing quote is seen as the glue bash makes of it.
Whether text is glued to the message argument is judged against the one character that
follows it, so a glued `-m` belonging to a command chained after this one no longer
refuses a message that was never truncated.
The character classes those scans use are held in variables rather than written inline. Inline,
each of `;`, `&` and `|` needs a backslash to get past the `[[ ]]` parser, and a POSIX bracket
expression has no escape mechanism of its own — so on bash 3.2, the system `/bin/bash` on macOS,
those backslashes reach the regex engine and add a literal `\` to the class. One cause, both
directions: the separator scan refused a conforming commit whose pre-`-m` text merely contained a
backslash, and the glue scan, whose class is negated, resolved a heredoc whose suffix was glued
with a backslash rather than declining it. The second is the fail-open direction and is the reason
this is fixed rather than documented. Every row covering it runs under each bash on the machine,
because a row run only under bash 4+ — where the shell consumes the backslashes and the classes are
already correct — passes with or without the fix.
Known limits that fail closed — the commit is blocked, never wrongly allowed: an attached short-option value that begins with an expansion (`-S$KEY`, `-S"$KEY"`) is refused, though the detached `-S "$KEY"` and the long `--gpg-sign="$KEY"` both resolve; an UNQUOTED dash-leading glob after `--` (`-- -*.txt`) is refused, though every spelling that actually passes a glob to git as a pathspec — `'-*.txt'`, `"-*.txt"`, `':(exclude)-*.txt'` — resolves, because an unquoted glob is expanded by the shell before git sees it; the `<<"EOF"` delimiter spelling and a closing `)` on its own line remain false positives; a `--cleanup=` carried by a command chained after the commit refuses it as though it were git's own; and a leading assignment followed by a separator, as in `FOO=bar; git commit …`, is read as an assignment prefix, so the commit is recognised and then refused for the separator in its prefix. Which argument the hook captures as the message is unchanged.
Two limits fail OPEN, and are called out separately because they are the direction that matters:
a `cleanup` mode set persistently in git config is invisible to the hook, so under
`commit.cleanup=verbatim` a subject whose trailing whitespace pushes it past 72 characters is
measured without that whitespace and allowed; and the other options that supply a message from
somewhere other than `-m` (`-C`/`--reuse-message`, `-c`/`--reedit-message`, `-F`/`--file`,
`-t`/`--template`) are not detected. The latter is deliberate rather than overlooked: `-c` is
also a git GLOBAL option that legally precedes the subcommand, so scanning for it would refuse
ordinary `git -c key=value commit` invocations, and guessing that trade seemed worse than
naming the gap.
One limit is pre-existing rather than introduced here, and runs in the fail-open direction: the
hook validates a command only when the git invocation itself begins it. Leading environment
assignments, an absolute path to git, and the git global options this classifier knows are all
walked through — that set is finite and does not cover every global option git accepts, so an
unlisted one such as `--config-env` is not walked. Measured: `git add -A && git commit …`,
`cd dir && git commit …`, `git status; git commit …` and `(git commit …)` are not recognised as
commits at all, so they are unchecked in every message form, heredoc or not, while a command
chained *after* the commit (`git commit … && echo done`) is recognised normally. It is not true
of every chained-before shape, though: `FOO=bar; git commit …` tokenizes with `FOO=bar;` read as
an assignment prefix, so it IS recognised and then refused, which is the fail-closed limit noted
above. So the resolution and the limits here describe the commands this hook gates, not every
commit a shell can run. Widening the classifier is a separate change with its own bypass surface
and with blast radius beyond this hook — `isGitSubcommand` is the shared git-commit detector for
every hook that gates on one — so it is deliberately not made here. The two shapes above are
pinned by tests, including non-conforming subjects that prove they are unvalidated rather than
merely permitted. (#3802)

381
hooks/gsd-validate-commit.sh
View File

@@ -104,16 +104,393 @@ if [ "$CLASSIFY_STATUS" != "0" ] && [ "$CLASSIFY_STATUS" != "1" ]; then
exit 0
fi
if [ "$CLASSIFY_STATUS" = "0" ]; then
# Extract message from -m flag
# Extract message from -m flag.
#
# MSG_QUOTE records WHICH arm matched. bash treats the two arms differently
# and the subject step below depends on that difference — see the resolver
# gate (review of #3816, round 4).
MSG=""
MSG_QUOTE=""
MSG_MATCH=""
if [[ "$CMD" =~ -m[[:space:]]+\"([^\"]+)\" ]]; then
MSG="${BASH_REMATCH[1]}"
MSG_QUOTE=dq
MSG_MATCH="${BASH_REMATCH[0]}"
elif [[ "$CMD" =~ -m[[:space:]]+\'([^\']+)\' ]]; then
MSG="${BASH_REMATCH[1]}"
MSG_QUOTE=sq
MSG_MATCH="${BASH_REMATCH[0]}"
fi
if [ -n "$MSG" ]; then
SUBJECT=$(echo "$MSG" | head -1)
# Subject = first line of the message, EXCEPT for the command-substituted
# heredoc form, where the first line is the opener rather than the message:
#
# git commit -m "$(cat <<'EOF'
# feat(auth): add login flow
# EOF
# )"
#
# The capture above spans it whole, because bash `[^"]` matches newlines, so
# `head -1` yielded the literal `$(cat <<'EOF'` and EVERY heredoc-form commit
# was blocked regardless of its message (#3802).
#
# Selection of WHICH argument is the message is unchanged above — only the
# subject-from-message step is delegated. Falls back to the previous `head -1`
# if node or the library is unavailable, so a broken extractor degrades to the
# old behavior instead of becoming a new silent-allow path.
#
# SINGLE-QUOTE GATE (review of #3816, round 4 — BLOCKER). The resolver may
# only run on the DOUBLE-quoted arm. Inside `-m '...'` bash performs NO
# command substitution, so `$(cat <<'EOF'` is literal text and git's real
# subject is that opener line — resolving the body there validates a
# message git never receives. Measured against the real hook, all four
# spellings (`<<'E'`, `<<"E"`, `<<\E`, `<<E`) went base=2 -> head=0: a
# net-new bypass reachable by the ordinary authoring slip of typing `'`
# for `"`. The sq arm therefore keeps the pre-fix `head -1`, which is exact
# base parity.
#
# ADJACENCY GUARD (review of #3816): text glued to the CLOSING quote —
# `-m "$(cat <<'EOF' ... )"suffix` — is concatenated by bash into the SAME
# argument, so the capture above holds only a PREFIX of the real message.
# Resolving a heredoc from a prefix hands the length gate a fraction of the
# real subject: a net-new bypass relative to base, which measured the
# opener line and blocked. When the quote is not followed by whitespace or
# the end of the command, skip the resolver and keep the pre-fix subject
# (first captured line): the heredoc form then fails the format gate
# exactly as it did on base, and the plain single-line form keeps base
# behavior unchanged. The guard is tested against the arm that MATCHED,
# not against both: testing both let a double-quoted heredoc whose BODY
# mentions a glued single-quoted token (`-m "... -m 'foo'bar ..."`) trip
# the sq arm and lose the fix for a message that never had a prefix
# problem (review of #3816, round 4, Minor 1).
# RESOLVER PRECONDITIONS. The resolver may run only where the captured text
# is provably the subject git receives. Each guard names an input where it
# is not; every refusal falls back to `head -1`, the pre-fix subject, which
# fails the format gate exactly as this whole form did before the fix.
RESOLVE=0
if [ "$MSG_QUOTE" = dq ]; then
RESOLVE=1
# Text before the message we matched. The heredoc BODY always sits after
# the match, so this window cannot be contaminated by message content —
# which is what lets the two guards below scan for tokens that would also
# be legal inside a commit message.
MSG_PREFIX="${CMD%%"$MSG_MATCH"*}"
# Text after it. Together, PREFIX and SUFFIX are the whole command MINUS
# the message — the window a guard must use when the token it scans for
# is also legal English inside a commit message, but may legally appear
# on EITHER side of the message on the command line.
MSG_SUFFIX="${CMD#*"$MSG_MATCH"}"
# LINE CONTINUATIONS ARE NOT SEPARATORS (review of #3816, rounds 8 and 9).
# `git commit \` newline ` -m "$(cat <<'EOF' …` is an ordinary way to
# spread an invocation over lines, and every guard below reads a newline in
# a window as a command separator, so the whole form was refused. That was
# disclosed as a fail-closed limit in round 8 because "is this newline a
# continuation" looked like the segmentation question this file has
# reverted twice. It is not: bash's rule is local and character-level. A
# newline preceded by an ODD run of backslashes is a continuation and bash
# removes both; an EVEN run (`\\` then newline) is a literal backslash
# followed by a real newline, which IS a separator. So the windows are
# joined the way bash joins them, in three bash-3.2-safe steps: every `\\`
# pair is parked on \x01, a byte no real command line carries, any
# backslash-newline that remains is a lone (odd) one and is removed, then
# the pairs are restored. Applied to BOTH windows, BEFORE the dequote
# copies are derived, so every scan sees the joined text.
#
# KNOWN OVER-BLOCK, fail-closed: a literal \x01 that IS present in the
# command is restored as `\\`, so an option-shaped token carrying one
# (`-\x01m`) reads as `-\\m`, dequotes to `-m`, and refuses where it did
# not before (independent review, round 9). Refusing is the recoverable
# direction; a control byte in an option name is not a spelling anyone
# types, and it is not a hole in the accept direction.
#
# Direction check: a continuation glued to the closing quote
# (`"$(…)"\` newline `suffix`) joins to `"$(…)"suffix`, which the glue
# guard refuses exactly as bash would have glued it; `\\` + newline keeps
# its newline and is still refused by the separator guard. Measured on
# bash 3.2.57 and 5.3.15 in tests/hooks-opt-in.test.cjs.
CONT_PARK=$'\x01'
MSG_PREFIX="${MSG_PREFIX//\\\\/$CONT_PARK}"
MSG_PREFIX="${MSG_PREFIX//\\$'\n'/}"
MSG_PREFIX="${MSG_PREFIX//$CONT_PARK/\\\\}"
MSG_SUFFIX="${MSG_SUFFIX//\\\\/$CONT_PARK}"
MSG_SUFFIX="${MSG_SUFFIX//\\$'\n'/}"
MSG_SUFFIX="${MSG_SUFFIX//$CONT_PARK/\\\\}"
# QUOTE-SPLICED SPELLINGS (independent review of #3816, round 6). Bash
# removes quotes before git ever sees an argument, so the same option has
# unboundedly many spellings on the command line: `--clean""up=verbatim`
# IS `--cleanup=verbatim` to git, and `-""m` IS `-m`. Both matched no
# literal and were measured ACCEPTING a 75-byte subject the length gate
# had recorded as 72. The guards below therefore scan a copy of their
# window with quote characters removed, which is what bash does to it.
# Only the two OPTION-NAME scans use it; the adjacency test deliberately
# does not, because it asks about a literal character position, and the
# message span itself is excluded from both windows either way.
MSG_PREFIX_DEQ="${MSG_PREFIX//[\"\']/}"
MSG_SUFFIX_DEQ="${MSG_SUFFIX//[\"\']/}"
# BACKSLASH-SPLICED SPELLINGS (independent review of #3816, round 7).
# Quote removal alone was not "the command as bash hands it to git": bash
# also removes syntactic backslashes, so `-\m WIP` IS `-m WIP` and
# `--clean\up=verbatim` IS `--cleanup=verbatim` to git, and both matched
# no literal. Measured: `-\m WIP -m <conforming heredoc>` accepted the
# heredoc while git recorded `WIP`, and a trailing `--clean\up=verbatim`
# accepted a 75-byte subject the length gate measured as 72. Stripped in a
# second pass so the class is unambiguous.
MSG_PREFIX_DEQ="${MSG_PREFIX_DEQ//\\/}"
MSG_SUFFIX_DEQ="${MSG_SUFFIX_DEQ//\\/}"
# DOLLAR-QUOTED SPELLINGS (independent review of #3816, round 8). The two
# passes above still were not "the command as bash hands it to git": bash
# has TWO more quoting forms whose introducer is a `$`, and removing the
# quote characters alone leaves that `$` stranded in the middle of the
# option name. `-$"m"` became `-$m` here while bash passes a real `-m` to
# git, and `--mes$'sage'=WIP` became `--mes$sage=WIP`; neither matched any
# literal, so the first-message guard below never fired. Measured on bash
# 3.2.57 and 5.3.15 against a real repository: the hook allowed
# `-$"m" WIP -m <conforming heredoc>` (exit 0) while `git cat-file -p`
# recorded the subject `WIP` — the same command spelled `-m WIP` is
# refused (exit 2). Stripping `$` closes both dollar-quote forms.
#
# RESIDUAL, and not fixable from a string: an option name assembled by an
# EXPANSION — `-${x}m`, `-$(printf m)` — is not knowable without running
# the command, the same limit this file already documents for expanded
# heredoc bodies. Stripping `$` makes those spellings collapse toward the
# literal too, which over-matches, and over-matching only refuses more.
MSG_PREFIX_DEQ="${MSG_PREFIX_DEQ//\$/}"
MSG_SUFFIX_DEQ="${MSG_SUFFIX_DEQ//\$/}"
# ADJACENCY GUARD (review of #3816): text glued to the CLOSING quote —
# `-m "$(cat <<'EOF' ... )"suffix` — is concatenated by bash into the SAME
# argument, so the capture holds only a PREFIX of the real message, and
# the length gate would measure a fraction of the real subject.
# SCOPE (review of #3816, round 6 — MAJOR). Glue is a property of the ONE
# character following the MATCHED span, so that character is the whole
# window. Scanning $CMD for the shape anywhere refused any conforming
# commit whose command merely CONTAINED a glued `-m` elsewhere —
# `git commit -m "<heredoc>" && echo -m "test"z` stayed blocked with
# CONVENTIONAL_COMMITS_VIOLATION. Base blocks it too, because base blocks
# EVERY heredoc form (that is #3802): this was the fix not reaching the
# shape, measured base=2 -> pre=2 -> post=0, not a regression.
# The separators and redirections are excluded because bash does NOT
# concatenate across them: in `-m "msg"&& echo hi` the argument ends at
# the quote, so there is no truncated capture to defend against.
# The class is held in a VARIABLE, not written inline. Inline, every
# member needs a backslash to get past the `[[ ]]` parser (`;`, `&` and
# `|` are metacharacters there) — and on bash 3.2, the system /bin/bash on
# macOS, those backslashes are passed THROUGH to the regex engine instead
# of being consumed by the shell, silently adding a literal `\` to the
# class. Unquoted expansion of a variable on the right of `=~` is the one
# spelling that is a plain regex on 3.2 and 5.x alike (review of #3816,
# round 8). Writing `[^[:space:];&|()<>]` inline is NOT the fix: it is a
# bash syntax error on both versions.
GLUE_CLASS='^[^[:space:];&|()<>]'
if [[ "$MSG_SUFFIX" =~ $GLUE_CLASS ]]; then RESOLVE=0; fi
# FIRST-MESSAGE GUARD (Codex review of #3816, round 4 — BLOCKER). The
# capture is a SEARCH over the whole command and the double-quoted arm is
# tried first, so it can select a `-m` that is not git's subject at all:
#
# git commit -m 'WIP first' -m "$(cat <<'EOF' -> git concatenates; the
# git commit -m WIP -m "$(cat <<'EOF' subject is `WIP first`
# git commit -m WIP -- -m "$(cat <<'EOF' -> after --, not a message
# git commit -m WIP && echo -m "$(cat <<'EOF' -> belongs to `echo`
#
# All four measured base=2 -> head=0, with git recording the FIRST message
# as the subject (verified against real commits, not the man page). The
# mis-selection is pre-existing; resolving it is what turned it into an
# enforcement bypass. Resolve only when nothing before the match could
# have been an earlier message, an end-of-options marker, or another
# command.
# BUNDLED SHORT OPTIONS (independent review of #3816, round 6). git splits
# `-am 'WIP first'` into `-a -m`, so the real subject is `WIP first` and
# the heredoc is git's SECOND message — measured accepting the heredoc's
# subject while git recorded `WIP first`. A standalone `-m` is therefore
# not the only spelling that claims the message; any short-option cluster
# ending in `m` does.
# ATTACHED VALUES AND --message ABBREVIATIONS (independent review of
# #3816, round 7). The scan required a space or `=` after the option name,
# so two spellings git accepts matched nothing: an ATTACHED short-option
# value (`-mWIP`, which git reads as `-m WIP`) and a long-option
# abbreviation (`--mes=WIP`), the same abbreviation behaviour this file
# already models for `--cleanup`. Both were measured accepting a later
# conforming heredoc while git recorded `WIP` as the subject — confirmed
# against the raw commit object, not `git log --pretty=%s`. The short arm
# therefore drops its trailing requirement entirely: a `-` followed by
# letters ending in `m` claims the message however it is spelled. Wider
# than git's own abbreviation set on purpose — over-matching only refuses
# more, which is the recoverable direction.
# Variable-held for the same bash-3.2 reason as GLUE_CLASS above.
# AN OPTION NAME BUILT BY A COMMAND SUBSTITUTION IS UNRESOLVABLE
# (independent review of #3816, round 8). Stripping `$` above collapses the
# two dollar-QUOTE forms onto their literals, but `--clean$(printf up)=`
# is a different thing: bash RUNS a program to finish the option name, so
# the argv git receives is not derivable from this string at all. Measured
# accepting a 75-byte subject the length gate had recorded as 72.
#
# SCOPED TO THE NAME, NOT THE VALUE. The class is a `-`-leading token whose
# characters up to the substitution contain no `=` — an option NAME being
# assembled. `--author="$(git config user.name)"` and `--author "$(…)"`
# both put the substitution in the VALUE, which this file never models and
# which stays allowed; only `-…$(` before any `=` refuses. Scanned on the
# RAW windows on purpose: the dequoted copies have had their `$` removed,
# so the shape is no longer visible there.
#
# This is a SHAPE, not a segmentation: it never tries to decide where
# git's own command ends. Two attempts at that were reverted for opening
# accept-direction holes, and the reasoning above still stands.
# WIDENED, and the strategy changed with it (independent review, round 9).
# The `$(`-only spelling above was the fourth patch in a row that tried to
# EMULATE what bash does to an argument before git sees it -- round 6
# removed quotes, round 7 backslashes, round 8 the `$` of a dollar-quote,
# and each time review found another transform that had been missed. Round
# 9 found four more, all measured accepting `WIP` as the real subject on
# bash 3.2.57 and 5.3.15 while the plain spelling of the same command is
# refused:
#
# -$'\155' WIP ANSI-C octal escape decodes to `m`
# -$'\x6d' WIP ANSI-C hex escape decodes to `m`
# -`printf m` WIP command substitution, backtick spelling
# x= … -${x}m WIP parameter expansion
# -? WIP pathname expansion, with a file named `-m`
#
# The last two settle the strategy: an option name finished by a PARAMETER
# expansion depends on a variable's runtime value, and one finished by a
# PATHNAME expansion depends on the contents of the working directory.
# Neither is derivable from the command string at any level of effort, so
# emulation cannot be completed -- not "has not been completed yet".
#
# So the rule is no longer "normalise it and match the literal". It is: an
# option NAME containing a shell expansion or quoting construct is
# UNRESOLVABLE, and unresolvable refuses. One rule covers every spelling
# above, and every spelling nobody has thought of yet, in the fail-closed
# direction. The dequoting passes above are kept: they still normalise the
# deterministic removals so the guards RECOGNISE `--clean""up=` and `-\m`
# rather than merely refusing them, which keeps the existing rows honest.
#
# SCOPED TO THE NAME, NOT THE VALUE, exactly as before: the class is a
# `-`-leading token whose characters up to the substitution contain no `=`.
# `--author="$(git config user.name)"` and `--author "$(…)"` put the
# construct in the VALUE and still resolve, pinned in both directions.
# Scanned on the RAW windows, because the dequoted copies have had `$` and
# the quote characters removed and the shape is no longer visible there.
#
# The class is bracket-only and holds no backslash, per round 8: a POSIX
# bracket expression has no escape mechanism, and a backslash written
# inside one becomes a literal member on bash 3.2.
SUBST_NAME_CLASS='(^|[[:space:]])-[^[:space:]=]*[$`?*[]'
if [[ "$MSG_PREFIX" =~ $SUBST_NAME_CLASS ]] \
|| [[ "$MSG_SUFFIX" =~ $SUBST_NAME_CLASS ]]; then RESOLVE=0; fi
SEP_CLASS='[;&|]'
if [[ "$MSG_PREFIX_DEQ" =~ (^|[[:space:]])(-[a-zA-Z]*m|--m[a-z]*([=[:space:]]|$)) ]] \
|| [[ "$MSG_PREFIX" =~ (^|[[:space:]])--([[:space:]]|$) ]] \
|| [[ "$MSG_PREFIX" =~ $SEP_CLASS ]] \
|| [[ "$MSG_PREFIX" == *$'\n'* ]]; then RESOLVE=0; fi
# NEWLINE IS A COMMAND SEPARATOR TOO (independent review of #3816, round
# 7) — the test above. The separator scan covered `;`, `&` and `|` but not
# a literal newline, so a LATER command's heredoc-shaped `-m` was taken
# for this commit's message:
#
# git commit --amend --no-edit
# echo -m "$(cat <<'EOF'
# fix: conforming text unrelated to the commit
# EOF
# )"
#
# The classifier recognises the leading commit, the capture reaches across
# the newline into `echo`'s argument, and a conforming string with no
# relationship to the commit was validated and allowed. Tested as a glob
# rather than folded into the bracket class, because a literal newline
# inside a bash regex bracket expression is not portably expressible.
# CLEANUP-MODE GUARD (Codex review of #3816, round 4 — BLOCKER). The
# resolver skips leading blank lines and strips trailing whitespace
# because git's DEFAULT cleanup=whitespace does. Under
# `--cleanup=verbatim` git does neither, so a 72-char subject plus three
# trailing spaces is committed as a 75-byte subject while the hook
# measured 72 — COMMIT_SUBJECT_TOO_LONG dodged (measured base=2 -> head=0;
# confirmed by reading the raw commit object, since `git log --pretty=%s`
# strips trailing whitespace in its own output and hides it).
# Any named mode other than `whitespace` refuses. A mode set persistently
# in git config is invisible here and stays a documented residual limit.
# SCOPE (review of #3816, round 5 — BLOCKER). This scan must exclude the
# message. `--cleanup=` and `commit.cleanup=` are ordinary English inside
# a commit message — this repository's own hooks and docs discuss them
# constantly — and the heredoc BODY sits verbatim inside $CMD, so
# scanning $CMD refused to resolve any conforming message that merely
# MENTIONED the token, blocking it with CONVENTIONAL_COMMITS_VIOLATION.
# Scanning $MSG_PREFIX alone (the fix as first prescribed) would reopen
# the bypass this guard exists for: git accepts the flag on either side
# of -m, and `git commit -m "<heredoc>" --cleanup=verbatim` is caught
# today only because the scan is command-wide. PREFIX + SUFFIX keeps both
# positions covered while excluding the one span that is message text.
# The two are joined with a space so a token cannot be forged across the
# seam out of a prefix tail and a suffix head.
# KNOWN LIMIT, deliberately fail-closed (#3816, round 6). This window is
# the whole command minus the message, so a `--cleanup=` that belongs to a
# DIFFERENT command — `git commit -m "<heredoc>" && echo --cleanup=verbatim`
# — also refuses, and a conforming commit git would accept stays blocked.
# Narrowing it to git's own segment was tried and reverted: deciding where
# git's command ends needs a shell parse, and a substring scan is not one.
# Trimming at the first `;&|` cut the window short whenever a separator sat
# inside an ordinary argument — `--author "a&b"`, and equally `--author
# a\&b` — which hid a REAL trailing `--cleanup=verbatim` and ACCEPTED a
# 75-byte subject the length gate had measured as 72. Two successive
# narrowings each reopened that hole on a shape the previous one missed, so
# the scan stays wide: refusing a commit git would take is recoverable,
# accepting an over-long subject is not.
# ABBREVIATIONS (independent review of #3816, round 6). git accepts any
# unambiguous prefix of a long option, so `--cle=verbatim` sets the mode
# while matching no literal `--cleanup` — measured accepting a 75-byte
# subject recorded as 72. The class is deliberately wider than git's own
# abbreviation set: over-matching only refuses more, which is the safe
# direction, and no other `--cl` option exists for git commit.
# LAST DIRECTIVE WINS, AND ONE MATCH CANNOT SEE IT (independent review of
# #3816, round 7). A bash regex yields ONE BASH_REMATCH, so only the
# FIRST cleanup directive was inspected — and git applies the LAST one.
# `--cleanup=whitespace -m <heredoc> --cleanup=verbatim` therefore read as
# mode=whitespace, resolution stayed enabled, and a 72-character subject
# plus trailing spaces was accepted while git recorded 75 bytes with the
# whitespace preserved (confirmed against the raw commit object). Deciding
# WHICH directive is last needs an argv order this substring scan does not
# have, so multiplicity itself refuses: more than one directive is
# unresolvable, not "probably fine". Single-directive behaviour is
# unchanged.
CLEANUP_WINDOW="$MSG_PREFIX_DEQ $MSG_SUFFIX_DEQ"
# `|| true` is load-bearing: this script runs under `set -euo pipefail`,
# and grep exits 1 when it matches NOTHING — which is the common case, a
# command with no cleanup directive at all. Without it the pipeline's
# non-zero status killed the hook outright (exit 1, no verdict) for every
# ordinary commit. Caught by running the real hook rather than the scan.
CLEANUP_HITS=$( { printf '%s' "$CLEANUP_WINDOW" | grep -oE '(--cl[a-z]*|commit\.cleanup)[=[:space:]]+[^[:space:]]+' || true; } | wc -l | tr -d ' ')
if [ "${CLEANUP_HITS:-0}" -gt 1 ]; then
RESOLVE=0
elif [[ "$CLEANUP_WINDOW" =~ (--cl[a-z]*|commit\.cleanup)[=[:space:]]+([^[:space:]]+) ]]; then
if [ "${BASH_REMATCH[2]}" != "whitespace" ]; then RESOLVE=0; fi
fi
# GIT-GENERATED SUBJECTS (independent review of #3816, round 7). With
# `--squash=<commit>` or `--fixup=<commit>` git composes the subject
# itself — measured recording `squash! base: something` while a conforming
# heredoc supplied via -m sailed through. The supplied message is not the
# subject in these modes at all, so there is nothing here worth measuring
# and resolution is refused outright. Abbreviations included for the same
# reason as --cleanup's. Deliberately NOT extended to the other
# message-SOURCE options (-C/--reuse-message, -c/--reedit-message,
# -F/--file, -t/--template): `-c` is also a git GLOBAL option that legally
# precedes the subcommand, so a scan for it would refuse ordinary
# `git -c k=v commit` invocations. Those remain a disclosed gap rather
# than a guessed guard.
if [[ "$MSG_PREFIX_DEQ $MSG_SUFFIX_DEQ" =~ (^|[[:space:]])--(squash|fixup|sq[a-z]*|fix[a-z]*)[=[:space:]] ]]; then RESOLVE=0; fi
fi
if [ "$RESOLVE" = 1 ]; then
SUBJECT=$(GIT_CMD_LIB="$HOOK_DIR/lib/git-cmd.js" MSG="$MSG" node -e "
const {resolveCommitSubject}=require(process.env.GIT_CMD_LIB);
process.stdout.write(resolveCommitSubject(process.env.MSG));
" 2>/dev/null) || SUBJECT=$(echo "$MSG" | head -1)
else
SUBJECT=$(echo "$MSG" | head -1)
fi
# Validate Conventional Commits format
if ! [[ "$SUBJECT" =~ ^(feat|fix|docs|style|refactor|perf|test|build|ci|chore)(\(.+\))?:[[:space:]].+ ]]; then
# Emit a typed `code` field alongside `reason` (#2974). Tests assert

211
hooks/lib/git-cmd.js
View File

@@ -180,4 +180,213 @@ function isGitSubcommand(cmd, sub) {
return tokens[subIdx] === sub;
}
module.exports = { isGitSubcommand, tokenize, extractBranchArgument, skipToSubcommand };
/**
* Resolve a `-m` message argument to its SUBJECT — the first line of the commit
* message — resolving the command-substituted heredoc form to the heredoc
* BODY's first line.
*
* PURE STRING FUNCTION. It deliberately does NOT tokenize or walk the command
* line: selecting *which* argument is the message stays with the caller, exactly
* as before, so this cannot change which commands are validated. An earlier
* revision of this fix did walk tokens and regressed four separate cases —
* `git commit -- -m WIP` (a pathspec), `git commit --amend && echo -m WIP` (a
* later command's flag), `-m "" --allow-empty-message` (the scanner drops empty
* tokens, so the following flag became the subject), and unquoted
* `git commit -m WIP`. All were allowed upstream and would have started being
* blocked. Reported in review of #3802.
*
* The defect this DOES fix: `gsd-validate-commit.sh` captured the message with
* `-m[[:space:]]+"([^"]+)"`, and bash `[^"]` matches newlines, so the widely
* used agent-authored commit idiom
*
* git commit -m "$(cat <<'EOF'
* feat(auth): add login flow
* EOF
* )"
*
* captured the whole span up to the final quote at `)"`. Taking its first line
* yielded the literal `$(cat <<'EOF'`, which can never satisfy Conventional
* Commits, so every heredoc-form commit was blocked regardless of its message.
*
* Recognition is anchored at BOTH ends and requires a command substitution, so
* an ordinary message merely CONTAINING — or ending in — `<<WORD` is not
* mistaken for an opener. Without the `^\$\(` anchor,
* `-m "WIP notes <<EOF\nfix: smuggled subject"` resolved to the second line and
* ALLOWED a non-conforming commit: an enforcement bypass, not just a
* misclassification (review of #3802).
*
* NOT resolved, by design: an UNQUOTED delimiter (`<<EOF`). bash expands `$var`
* and `$(...)` in that body, so the literal text here is not what git receives.
* An earlier revision resolved it anyway and called the gap "the same
* pre-existing limit as expansions in a plain `-m` argument" — that framing was
* wrong on both halves: on base the whole heredoc form was blocked, so this fix
* CREATED the path, and it dodged the length gate as well as the format gate.
* See the expansion guard in the body (review of #3816, round 4).
*
* KNOWN LIMIT: the DOUBLE-QUOTED delimiter spelling (`<<"EOF"`) is resolvable
* here but unreachable through the caller — `gsd-validate-commit.sh`'s
* double-quoted `-m` capture stops at the first `"`, which in that spelling is
* the delimiter's own quote, so the resolver only ever sees a truncated opener
* and the commit stays blocked. Its single-quoted `-m` capture DOES deliver the
* spelling intact, which is why an earlier revision's claim of unreachability
* was false; the caller now gates the resolver on the double-quoted arm alone
* (review of #3816, round 4), so the claim holds again — for that reason, not
* by luck. That is the pre-fix behaviour for the whole form (fail closed, a
* false positive on one rare spelling), and widening the bash capture to span
* inner quotes would change what is captured for EVERY message containing one —
* a regression class this fix deliberately does not touch (Codex review of
* #3816). The same capture truncation blocks a `"` in the SUBJECT LINE itself,
* where it lands inside the line being measured. It does NOT block a `"` on a
* later body line: the subject is already complete before the truncation point,
* so that message resolves and is allowed (measured; an earlier revision of this
* comment and of the changeset claimed a `"` ANYWHERE blocked, which is false —
* Codex review of #3816, round 4). The truncation guard below is what keeps the
* unmeasurable half fail-closed. Two more legal-but-unrecognized spellings
* stay blocked the same fail-closed way: an env-prefixed cat
* (`$(A=1 cat <<'EOF'`) and an option-terminated cat (`$(cat -- <<'EOF'`) —
* recognizing either would mean modelling bash prefix words here, cost with
* no reported user (review of #3816, round 3).
*
* @param {string} messageArg - the raw `-m` argument, already selected by the caller
* @returns {string} the subject to validate
*/
function resolveCommitSubject(messageArg) {
// CRLF-tolerant split. With a bare split('\n') every body line kept its \r,
// so `body.indexOf(delimiter)` never matched on CRLF input: the truncation
// guard was inert, an empty CRLF message resolved to 'EOF\r' instead of '',
// and a real 72-char subject measured 73 (review of #3816). Splitting on
// /\r?\n/ is the repo-wide remedy for this recurring defect class.
const lines = String(messageArg == null ? '' : messageArg).split(/\r?\n/);
// The path prefix is a PATH-CHARACTER class, not \S*: `\S*` accepted
// `id;/bin/cat`, so `$(id;/bin/cat <<'EOF' ...` was resolved to its heredoc
// body while bash actually runs `id` first and git's real subject is `id`'s
// OUTPUT — an enforcement bypass (Codex review of #3816). A prefix carrying
// any shell metacharacter now fails recognition, which falls back to the
// opener line and the format gate: fail closed, exactly the pre-fix
// behaviour for the whole form.
//
// Whitespace inside the recognition is ASCII space/tab — [ \t], never \s —
// because JavaScript \s includes Unicode whitespace bash does NOT split on:
// `$(<NBSP>/bin/cat <<'EOF'` was recognized here while bash reads
// `<NBSP>/bin/cat` as the executable NAME, so recognition claimed a
// substitution that does not run cat (Codex review of #3816, round 2). The
// same ASCII rule as the blank-line skip below, for the same reason.
// `cat[ \t]*<<`, not `+`: bash accepts `cat<<'EOF'` with no space (review of
// #3816, round 3), and recognizing it costs nothing — the token before `<<`
// is still literally `cat`.
//
// The path prefix must be ABSOLUTE (Codex review of #3816, round 4). The old
// `[\w./-]*\/` also accepted `./cat` and `../evil/cat`, so a relative
// executable that merely ENDS in `cat` was trusted to echo its stdin: with a
// planted `../evil/cat` printing `WIP injected`, the resolver validated the
// heredoc body while git's real subject was `WIP injected` (measured
// base=2 -> head=0 against a real commit). Requiring `/` up front costs
// nothing real — `/bin/cat` and a bare `cat` both still resolve.
//
// AN ABSOLUTE PATH IS NOT AN IDENTITY EITHER (independent review of #3816,
// round 8). Round 4 stopped at "must be absolute", so any absolute path
// ENDING in `/cat` was still trusted to echo its stdin — the very thing the
// round-4 reasoning rejected one spelling earlier. With an executable at
// `/some/scratch/dir/cat` printing `WIP injected`, the resolver validated
// the conforming heredoc body while git's real subject was `WIP injected`
// (measured on bash 3.2.57 and 5.3.15 against a real commit: hook exit 0,
// `git cat-file -p` subject `WIP injected`, while the same command through
// `./cat` was already refused). The prefix is now the canonical system
// locations, which is the only claim a string can support. `/usr/local/bin`
// is deliberately excluded: it is user-writable on ordinary machines, which
// is the plantable case this guard exists for.
//
// RESIDUAL, not fixable from a string: a bare `cat` shadowed earlier on PATH
// has the same effect and is indistinguishable here. It is also not a
// meaningful boundary — anyone who can plant an executable on PATH can run
// `git commit` directly — so this hook stays an authoring guard, not a
// security control.
//
// The delimiter alternatives are split so the BARE spelling is its own group:
// `\\(...)` (backslash-quoted) and `(...)` (bare) were one `\\?(...)` branch,
// which conflated the only two spellings that differ in bash. See the
// expansion guard below.
const opener = /^\$\([ \t]*(?:\/(?:usr\/)?bin\/)?cat[ \t]*<<(-?)[ \t]*(?:'([^']+)'|"([^"]+)"|\\([^\s'"();|&<>\\]+)|([^\s'"();|&<>\\]+))[ \t]*$/
.exec(lines[0]);
if (!opener) return lines[0];
// EXPANSION GUARD (review of #3816, round 4 — BLOCKER). Only `<<'D'`, `<<"D"`
// and `<<\D` suppress expansion. A BARE `<<D` is expanded by bash, so the body
// captured here is NOT the text git receives, and measuring it is an
// enforcement bypass in both gates at once:
//
// -m "$(cat <<EOF\nfeat: $UNSET_VAR\nEOF\n)" git gets `feat:` -> format gate dodged
// -m "$(cat <<EOF\nfeat: ${LONG}\nEOF\n)" git gets any length -> length gate dodged
//
// Both measured base=2 -> head=0 against the real hook. This is NOT the
// pre-existing plain-`-m` expansion limit an earlier revision claimed it was:
// on base the whole heredoc form was blocked, so no expansion inside a body
// ever reached an allow — this fix created the path, and closes it here.
// Same rule every other guard in this function follows: when the real subject
// cannot be known, fall back to the opener line, which fails the format gate.
if (opener[5]) return lines[0];
// `<<-` strips leading TABS from every body line, including the terminator.
const stripTabs = opener[1] === '-';
const delimiter = opener[2] || opener[3] || opener[4];
const body = lines.slice(1).map((l) => (stripTabs ? l.replace(/^\t+/, '') : l));
// TRUNCATION GUARD. The capture that produced this argument stops at the first
// `"`, so a message containing one arrives here missing its tail — and its
// terminator. Resolving anyway would hand the length gate a PREFIX of the real
// subject and let an over-long message through, an enforcement hole that did
// not exist before this fix (review of #3802). A body with no terminator is
// therefore not resolved at all: returning the opener line fails the format
// gate, which is exactly what this whole form did before the fix. The fix
// applies where the capture is complete and changes nothing where it is not.
const end = body.indexOf(delimiter);
// POST-TERMINATOR GUARD (review of #3816, round 3 — BLOCKER). Everything
// after the terminator is still part of the real message once bash
// substitutes: `-m "$(cat <<'EOF'\nfeat: ok\nEOF\n) <200 a's>"` expands to a
// single 200+ char subject, but discarding the tail measured 8 and DODGED
// COMMIT_SUBJECT_TOO_LONG — the same prefix-measurement class the truncation
// guard below exists for, missed on the other side of the terminator. The
// canonical idiom's tail is exactly one closing-paren line; anything else
// means the substitution is composed with more text, so fall back to the
// opener line — blocked, the pre-fix behaviour for the whole form.
if (end !== -1) {
const tail = body.slice(end + 1);
if (tail.length !== 1 || !/^[ \t]*\)[ \t]*$/.test(tail[0])) return lines[0];
}
// git's default `cleanup=whitespace` strips leading blank lines, so the subject
// is the first NON-EMPTY body line, not blindly the first one. Taking lines[1]
// returned '' for a body that starts blank and falsely blocked a conforming
// commit — the very defect class #3802 reports (review of #3802).
const scan = end === -1 ? body : body.slice(0, end);
// "Blank" is git's ASCII definition — space and tab — never JavaScript's
// trim(), whose Unicode whitespace class skips lines git KEEPS: a body whose
// first line is a NBSP resolved to the SECOND line while git's real subject
// is the NBSP line — an enforcement bypass (Codex review of #3816, verified
// against `git stripspace`, which preserves the c2a0 bytes). A Unicode-blank
// first line is now returned as the subject and fails the format gate: the
// same fail-closed direction git itself takes.
const idx = scan.findIndex((l) => !/^[ \t]*$/.test(l));
if (idx === -1) return end === -1 ? lines[0] : '';
// Truncation is only fatal to the line it lands IN. A captured line is complete
// exactly when another line follows it, because the capture kept its newline.
// So an unterminated body whose subject line is followed by more text is still
// measurable; only a subject line that runs to the end of a truncated capture
// is not, and that one falls back to the opener — which fails the format gate,
// exactly as this whole form did before the fix. Without this, the length gate
// measured a PREFIX of the real subject and let an over-long message through
// (review of #3802).
if (end === -1 && idx >= body.length - 1) return lines[0];
// git's `cleanup=whitespace` strips whitespace at BOTH ends of the line, not
// just leading blanks. Measuring the raw line rejected `feat: <66 x's>` plus
// three trailing spaces as 75 chars when git's actual subject is a conforming
// 72 — a still-blocked conforming commit, the very defect #3802 reports
// (review of #3816). Trailing only, here: leading blank-LINE handling is the
// findIndex above, and `<<-` leading-tab stripping already happened.
return scan[idx].replace(/[ \t]+$/, '');
}
module.exports = { isGitSubcommand, tokenize, extractBranchArgument, skipToSubcommand, resolveCommitSubject };

3
tests/helpers/fast-check-setup.cjs
View File

@@ -4,7 +4,8 @@
* fast-check-setup.cjs
*
* Shared configuration for all property-based tests. Require this at the
* top of every *.property.test.cjs file before any fc.assert() call.
* top of every test file that calls fc.assert() — property-dedicated
* (*.property.test.cjs) or not — before the first fc.assert().
*
* Settings:
* numRuns: 200 — enough to catch boundary bugs without slow CI

965
tests/hooks-opt-in.test.cjs
View File

@@ -336,6 +336,971 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require
`expected typed code: 'CONVENTIONAL_COMMITS_VIOLATION', got: ${JSON.stringify(parsed)}`);
});
// #3802 — the heredoc `-m` form. Claude Code's own documented commit idiom is
//
// git commit -m "$(cat <<'EOF'
// feat(auth): add login flow
// EOF
// )"
//
// The `-m` capture regex spans it whole, because bash `[^"]` matches newlines,
// so the first line was the literal `$(cat <<'EOF'` and EVERY heredoc-form
// commit was blocked regardless of its message.
const heredoc = (body, open = "<<'EOF'", close = 'EOF') =>
`git commit -m "$(cat ${open}\n${body}\n${close}\n)"`;
const runHookCmd = (command) => spawnHook(path.join(HOOKS_DIR, 'gsd-validate-commit.sh'), {
input: JSON.stringify({ tool_input: { command } }),
encoding: 'utf-8',
cwd: tmpDir,
});
test('validate-commit allows a CONFORMING heredoc-form message', () => {
const result = runHookCmd(heredoc('feat(auth): add login flow'));
assert.strictEqual(result.status, 0,
`a conforming heredoc message must pass; got ${result.status}. stdout: ${result.stdout}`);
});
test('validate-commit still BLOCKS a non-conforming heredoc-form message', () => {
const result = runHookCmd(heredoc('wibble wobble no type here'));
assert.strictEqual(result.status, 2,
'resolving the heredoc body must not become a blanket exemption for the whole form');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
// ─── #3816 round 8 (Major): bracket classes must not smuggle a literal `\` ───
//
// `;`, `&` and `|` are shell metacharacters inside `[[ ]]`, so an inline
// bracket class has to escape each one: `[\;\&\|]`. POSIX bracket expressions
// have no escape mechanism of their own, and on bash 3.2 — the system
// /bin/bash on macOS, already a supported target here (see the `declare -A`
// ban in tests/install.test.cjs) — those backslashes reach the regex engine
// instead of being consumed by the shell, so the class silently gains a
// literal `\` as a member. bash 4+ consumes them, which is why this is
// invisible on a modern bash. The escape is only a hazard INSIDE a bracket
// expression: `\(` outside one is made literal correctly on every version.
//
// One root cause, consequences in BOTH directions:
// the SEPARATOR scan (positive class) OVER-BLOCKED — a conforming commit
// whose pre-`-m` text held a `\` was refused outright;
// the GLUE scan (NEGATED class) UNDER-REFUSED — a `\` glued to the message
// span fell inside the exclusion, so the hook resolved a heredoc it
// should have declined. That is the accept direction, and it is the row
// that matters most below.
//
// The fix holds each class in a variable expanded unquoted on the right of
// `=~`, which is a plain regex on 3.2 and 5.x alike. Writing the class inline
// without the backslashes is NOT the fix: `[[ x =~ [;&|] ]]` is a bash syntax
// error on both versions.
//
// Every row runs under each bash on the machine, because a row run only under
// bash 4+ passes with or without the fix — vacuous, and silently so. Where
// 3.2 is absent the row simply does not appear; that is disclosed here rather
// than papered over.
const BASHES = (() => {
const seen = new Set();
const found = [];
for (const candidate of ['/bin/bash', '/usr/local/bin/bash', '/opt/homebrew/bin/bash']) {
if (!fs.existsSync(candidate)) continue;
const probe = runHook('-c', ['printf %s "$BASH_VERSION"'], {
interpreter: candidate, env: hookEnv, timeoutMs: HOOK_TIMEOUT_MS,
});
const version = (probe.stdout || '').trim();
if (probe.exitCode !== 0 || !version || seen.has(version)) continue;
seen.add(version);
found.push({ path: candidate, version });
}
return found;
})();
const underBash = (bashPath, command) => runHook(
path.join(HOOKS_DIR, 'gsd-validate-commit.sh'), [],
{
interpreter: bashPath,
env: hookEnv,
timeoutMs: HOOK_TIMEOUT_MS,
input: JSON.stringify({ tool_input: { command } }),
cwd: tmpDir,
},
);
for (const bash of BASHES) {
// ACCEPT DIRECTION — the one that cannot be recovered from. Before the fix,
// bash 3.2 measured exit 0 here: the backslash sat inside the negated glue
// class, so the guard never fired and the heredoc was resolved anyway.
test(`a backslash-glued suffix is still refused (bash ${bash.version})`, () => {
const glued = `${heredoc('fix: a perfectly ordinary conforming subject')}\\zzz`;
const result = underBash(bash.path, glued);
assert.strictEqual(result.exitCode, 2,
`bash ${bash.version}: a suffix glued to the message span with a backslash must be `
+ 'refused exactly like any other glued suffix — resolving it is the accept direction. '
+ `stdout: ${result.stdout}`);
});
test(`a letter-glued suffix is still refused (bash ${bash.version})`, () => {
// Non-vacuity control for the row above: proves the glue guard is
// reachable at all under this interpreter, so a refusal there is the
// guard firing rather than the hook refusing everything.
const glued = `${heredoc('fix: a perfectly ordinary conforming subject')}zzz`;
assert.strictEqual(underBash(bash.path, glued).exitCode, 2,
`bash ${bash.version}: the established glued-suffix refusal must be unchanged`);
});
// OVER-BLOCK DIRECTION — the reported half. The backslash sits in the text
// BEFORE `-m`, and deliberately with no newline anywhere before it: a
// newline is refused by the separator guard on every bash, which would make
// this row pass for the wrong reason and prove nothing about the class.
test(`a backslash before -m does not block a conforming commit (bash ${bash.version})`, () => {
const command = `git commit --allow-empty --author=a\\,b -m "$(cat <<'EOF'\n`
+ `fix: a perfectly ordinary conforming subject\nEOF\n)"`;
const result = underBash(bash.path, command);
assert.strictEqual(result.exitCode, 0,
`bash ${bash.version}: a conforming commit must not be refused merely because its `
+ `pre-message text contains a backslash. stdout: ${result.stdout}`);
});
test(`a non-conforming subject is still blocked (bash ${bash.version})`, () => {
// Second non-vacuity control: proves this interpreter reaches the
// validator rather than passing everything — the failure mode that makes
// an allow-row look green for the wrong reason.
const result = underBash(bash.path, 'git commit -m "nope not conventional"');
assert.strictEqual(result.exitCode, 2,
`bash ${bash.version}: the validator must still be reached and still refuse`);
});
}
test('validate-commit measures subject length against the RESOLVED heredoc subject', () => {
// RULESET.TESTS.boundary-coverage: N at {limit-1, limit, limit+1}, not merely
// "very long". The limit is 72, and the gate is `> 72`, so 72 must PASS and
// 73 must block. A trivially-oversized subject alone would not show which
// side of the comparison the code sits on.
const at = (n) => {
const prefix = 'feat(auth): ';
return `${prefix}${'x'.repeat(n - prefix.length)}`;
};
for (const [n, want] of [[71, 0], [72, 0], [73, 2]]) {
const subject = at(n);
assert.strictEqual(subject.length, n, `fixture built wrong: ${subject.length} != ${n}`);
const result = runHookCmd(heredoc(subject));
assert.strictEqual(result.status, want,
`resolved heredoc subject of ${n} chars: expected exit ${want}, got ${result.status}`);
if (want === 2) {
assert.strictEqual(JSON.parse(result.stdout).code, 'COMMIT_SUBJECT_TOO_LONG',
'must fail on LENGTH, not format — a format failure would mean the opener was still '
+ 'being read as the subject');
}
}
// Review of #3816, Major 2: the clean fixtures above cannot see a
// one-directional cleanup=whitespace implementation. git strips TRAILING
// whitespace too, so a 72-char subject plus trailing spaces is a conforming
// commit — measuring the raw 75 chars re-blocks it, the very defect #3802
// reports. And the guard must strip, not blanket-allow: 73 chars plus a
// trailing space is still over-long once stripped.
const dirty72 = runHookCmd(heredoc(`${at(72)} `));
assert.strictEqual(dirty72.status, 0,
`git's actual subject is 72 chars — measuring the raw line as 75 must not block it; `
+ `got ${dirty72.status}: ${dirty72.stdout}`);
const dirty73 = runHookCmd(heredoc(`${at(73)} `));
assert.strictEqual(dirty73.status, 2,
'a 73-char subject stays blocked with trailing whitespace attached — stripping must not '
+ 'become an allowance');
assert.strictEqual(JSON.parse(dirty73.stdout).code, 'COMMIT_SUBJECT_TOO_LONG');
});
test('validate-commit does not resolve a TRUNCATED capture past its own limit', () => {
// Review of #3802, Major 3. An embedded `"` truncates the `-m` capture, so the
// resolver would otherwise measure a PREFIX of the real subject and let an
// over-long message through — an enforcement hole that did not exist before
// this fix. git's real subject here is 100+ chars; the captured prefix is 10.
const result = runHookCmd(`git commit -m "$(cat <<'EOF'\nfeat: aaaa" ${'z'.repeat(90)}\nEOF\n)"`);
assert.strictEqual(result.status, 2,
'a capture with no terminator cannot be measured, so it must fall back to the pre-fix '
+ 'behaviour (blocked) rather than resolving to a prefix that slips under the length gate');
});
test('validate-commit skips leading blank lines in the heredoc body, as git does', () => {
// Review of #3802, Minor 1. git's default cleanup=whitespace strips leading
// blank lines, so this commit's real subject is conforming — blocking it is
// the same false-positive class #3802 reports.
assert.strictEqual(runHookCmd(heredoc('\nfeat(auth): real subject after a blank line')).status, 0,
'the subject is the first NON-empty body line');
});
test('validate-commit resolves the QUOTED heredoc opener spellings, both directions', () => {
// Both directions per spelling, deliberately. Asserting only "conforming
// passes" would also pass if the resolver returned an empty subject for a
// spelling it failed to recognise — an allow, but for the wrong reason
// (review of #3802). Pairing it with a non-conforming body that must BLOCK
// proves the body is genuinely being read.
//
// Only the spellings that SUPPRESS expansion belong here: `<<'D'`, `<<"D"`
// and `<<\D`. The bare spellings moved to the row below, which pins the
// opposite contract (review of #3816, round 4).
for (const [label, open, close, indent] of [
["<<-'TAG' (tab-stripped)", "<<-'MSG'", '\tMSG', '\t'],
["<<'END-MSG' (non-identifier tag)", "<<'END-MSG'", 'END-MSG', ''],
['<<\\TAG (backslash-quoted)', '<<\\EOF', 'EOF', ''],
]) {
assert.strictEqual(runHookCmd(heredoc(`${indent}fix(api): correct status code`, open, close)).status, 0,
`${label}: a conforming message in this spelling must pass`);
assert.strictEqual(runHookCmd(heredoc(`${indent}wibble wobble`, open, close)).status, 2,
`${label}: a NON-conforming message in this spelling must still block — if this passes, the `
+ 'resolver is returning an empty subject rather than reading the body');
}
});
test('validate-commit BLOCKS a bare heredoc delimiter — bash expands that body (round-4 BLOCKER)', () => {
// Review of #3816, round 4. This row previously asserted the OPPOSITE
// (`['bare <<TAG', '<<EOF', 'EOF', '']` expecting exit 0), so the suite
// itself defended the bypass and the fix could not land without editing a
// test that read as intentional. RULESET.TESTS.delete-bad-tests: a test
// asserting the defective behaviour is corrected in the same change as the
// behaviour.
//
// WHY the contract flips: only `<<'D'`, `<<"D"` and `<<\D` suppress
// expansion. A bare `<<D` is expanded by bash, so the body captured by the
// hook is not the text git receives, and resolving it dodges BOTH gates.
// Verified with an argv-printing stub: `-m "$(cat <<EOF\nfeat: $UNSET\nEOF\n)"`
// reaches git as `feat: ` — subject `feat:`, non-conforming — while the
// literal body measured as conforming.
for (const [label, open] of [
['bare <<TAG', '<<EOF'],
['<< TAG (spaced, bare)', '<< EOF'],
['<<-TAG (bare, tab-stripping)', '<<-EOF'],
]) {
const result = runHookCmd(heredoc('fix(api): correct status code', open, 'EOF'));
assert.strictEqual(result.status, 2,
`${label}: must BLOCK even though the literal body looks conforming — bash expands this `
+ 'body, so the validated text is not the text git receives');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION',
`${label}: falls back to the opener line, which fails the format gate`);
}
});
test('validate-commit BLOCKS an expansion inside a bare-delimiter body (round-4 BLOCKER)', () => {
// The measured bypass itself, in both its gate-dodging forms. Non-vacuous:
// each literal body IS conforming and IS within 72 chars, so a resolver
// that measured the literal returns exit 0 — which is what head did before
// this fix (base=2 -> head=0, measured against the real hook).
const expanded = runHookCmd('git commit -m "$(cat <<EOF\nfeat: $UNSET_VAR\nEOF\n)"');
assert.strictEqual(expanded.status, 2,
'git receives `feat: ` (subject `feat:`) once bash expands $UNSET_VAR — the format gate must '
+ 'not be judged against the unexpanded literal');
assert.strictEqual(JSON.parse(expanded.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
const lengthDodge = runHookCmd('git commit -m "$(cat <<EOF\nfeat: ${LONG}\nEOF\n)"');
assert.strictEqual(lengthDodge.status, 2,
'${LONG} expands to any length at all, so measuring the 12-char literal dodges '
+ 'COMMIT_SUBJECT_TOO_LONG — the same prefix-measurement class the truncation and '
+ 'post-terminator guards exist for, through expansion rather than composition');
});
test('validate-commit does not resolve a heredoc in the SINGLE-quoted -m arm (round-4 BLOCKER)', () => {
// Review of #3816, round 4. Inside `-m '...'` bash performs NO command
// substitution, so `$(cat <<'EOF'` is literal text and git's real subject
// is that opener line. Resolving the body there validates a message git
// never receives. All four spellings measured base=2 -> head=0 before this
// fix; reachable by the ordinary slip of typing `'` for `"`.
//
// Non-vacuous by construction: every body below is conforming, so a hook
// that resolves the sq arm returns exit 0 on all four.
//
// The `heredoc()` helper hard-codes the double quote, which is exactly why
// this arm went untested for three rounds — these rows build the command
// directly.
for (const [label, open] of [
['<<"EOF"', '<<"EOF"'],
["<<'EOF'", "<<'EOF'"],
['<<\\EOF', '<<\\EOF'],
['bare <<EOF', '<<EOF'],
['<< EOF (spaced)', '<< EOF'],
]) {
const result = runHookCmd(`git commit -m '$(cat ${open}\nfeat(auth): looks conforming\nEOF\n)'`);
assert.strictEqual(result.status, 2,
`sq arm, ${label}: must BLOCK — bash does not substitute inside single quotes, so git's `
+ "real subject is the literal opener line, not the body");
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
}
});
test('the adjacency guard is scoped to the arm that matched (round-4 Minor 1)', () => {
// Review of #3816, round 4, Minor 1. The guard tested BOTH quote styles
// against the whole command irrespective of which arm produced the
// message, so a double-quoted heredoc whose BODY mentions a glued
// single-quoted token tripped the sq arm and lost the fix for a message
// that never had a prefix problem. Measured 2/2 before, 0 after.
assert.strictEqual(
runHookCmd(heredoc("feat: stop passing -m 'foo'bar to git")).status, 0,
'a glued single-quoted token inside a DOUBLE-quoted heredoc body must not trip the '
+ 'single-quote adjacency arm');
});
test('validate-commit blocks a substitution composed with more text (round-3 BLOCKER)', () => {
// Review of #3816, round 3. bash expands this -m argument to a SINGLE
// 200+ char subject, but the resolver discarded everything after the
// terminator and measured `feat: ok` (8 chars) — a live length-gate
// bypass the base did not have. The post-terminator guard now falls back
// to the opener line, so the form is blocked by the FORMAT gate, the
// pre-fix behaviour for the whole form.
const result = runHookCmd(`git commit -m "$(cat <<'EOF'\nfeat: ok\nEOF\n) ${'a'.repeat(200)}"`);
assert.strictEqual(result.status, 2,
'a heredoc substitution composed with trailing text is one long real subject — resolving '
+ 'the body alone dodges COMMIT_SUBJECT_TOO_LONG');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION',
'fail-closed via the format gate on the opener fallback, matching every other unresolvable shape');
});
test('validate-commit blocks a suffix glued OUTSIDE the closing quote (adjacency guard)', () => {
// Codex review of #3816, round 3. bash concatenates `"$(…)"aaaa…` into ONE
// argument, but the capture holds only the quoted part — so the resolver
// measured `feat: ok` (8 chars) for a 200+ char real subject: a net-new
// length-gate bypass the base did not have (base measured the opener and
// blocked). Glued text after the closing quote now skips the resolver and
// keeps the pre-fix first-line subject, which for the heredoc form is the
// opener — blocked, base parity restored.
const result = runHookCmd(`git commit -m "$(cat <<'EOF'\nfeat: ok\nEOF\n)"${'a'.repeat(200)}`);
assert.strictEqual(result.status, 2,
'a quoted substitution with an adjacent unquoted suffix is one long real subject — the '
+ 'captured prefix must not be resolved and measured on its own');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
test('adjacency on a PLAIN single-line message keeps base behavior (pre-existing, unchanged)', () => {
// Differential pin: on base, `-m "feat: ok"zzz` captured `feat: ok`,
// validated it, and ALLOWED the commit even though bash's real argument is
// `feat: okzzz`. That is a pre-existing capture limit (same family as
// rows 16-20 of the round-3 review's table), and the adjacency guard
// deliberately preserves it rather than widening scope: the guard's job is
// to stop the RESOLVER from measuring a prefix, not to fix the capture.
const result = runHookCmd('git commit -m "feat: ok"zzz');
assert.strictEqual(result.status, 0,
'base allowed this shape; the adjacency guard must not silently change plain-form behavior');
});
test('validate-commit blocks a command smuggled before the cat', () => {
// Codex review of #3816. `$(id;/bin/cat <<'EOF' ...` runs `id` FIRST, so
// git's real subject is id's output — but the resolver read the heredoc
// body and the conforming `fix: smuggled` sailed through: an enforcement
// bypass end to end. Recognition now rejects a path prefix carrying shell
// metacharacters and the whole form falls back to blocked.
const result = runHookCmd(`git commit -m "$(id;/bin/cat <<'EOF'\nfix: smuggled\nEOF\n)"`);
assert.strictEqual(result.status, 2,
'a command substitution that runs anything besides cat cannot have its heredoc body '
+ 'trusted as the subject');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
test('KNOWN LIMIT: the <<"TAG" spelling stays blocked — the capture cannot deliver it', () => {
// This row pins a LIMIT, not desired behaviour (Codex review of #3816).
// The `-m` capture stops at the first `"`, which in this spelling is the
// delimiter's own quote, so the resolver only ever sees a truncated opener
// and even a conforming message is blocked — the pre-fix behaviour for the
// whole form, fail closed. If this row ever starts passing, the capture
// changed: re-review every embedded-quote case before celebrating.
// Counterpart: the UNIT row in tests/worktree-safety.test.cjs proves the
// pure resolver CAN resolve this spelling — the limit is the capture,
// not the parser; the two rows are correct together (review of #3816,
// round 3, N2).
const result = runHookCmd(heredoc('feat(api): conforming subject', '<<"EOF"', 'EOF'));
assert.strictEqual(result.status, 2,
'documented residual false-positive on the double-quoted delimiter spelling');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
test('validate-commit does not treat a message ENDING in <<WORD as a heredoc', () => {
// Enforcement bypass found in review of #3802: an earlier revision recognised
// the opener without anchoring it to a command substitution, so this resolved
// to line 2 and ALLOWED a commit whose real subject is non-conforming.
const result = runHookCmd('git commit -m "WIP notes <<EOF\nfix: smuggled subject"');
assert.strictEqual(result.status, 2,
'the real subject is the non-conforming first line; resolving past it is an ALLOW that '
+ 'smuggles an unvalidated message through');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
test('validate-commit leaves every non-heredoc form exactly as it was', () => {
// Differential pins. Each of these was ALLOWED before this change, and an
// earlier revision that walked tokens to find `-m` started BLOCKING all of
// them (review of #3802). They are not incidental: `--` introduces pathspecs,
// `&&` starts a different command, and the shared scanner drops empty tokens
// so a following flag can be mistaken for the message.
for (const [label, cmd] of [
['-- introduces pathspecs', 'git commit -- -m WIP'],
['a later command\'s flag', 'git commit --amend && echo -m WIP'],
['empty -m before a flag', 'git commit -m "" --allow-empty-message'],
['empty -m before a real -m', 'git commit -m "" -m "fix: real subject"'],
['unquoted -m argument', 'git commit -m WIP'],
]) {
assert.strictEqual(runHookCmd(cmd).status, 0,
`${label}: this form was allowed before #3802 and must stay allowed — widening WHICH `
+ 'argument counts as the message is out of scope for this fix');
}
});
test('validate-commit resolves only git\'s FIRST message argument (round-4 Codex BLOCKER)', () => {
// The `-m` capture is a SEARCH over the whole command and the double-quoted
// arm is tried first, so it could select a `-m` that is not git's subject.
// git CONCATENATES multiple -m arguments and the SUBJECT is the first one —
// verified against real commits, not the man page: for
// `-m 'WIP first' -m "$(cat …)"` git records `WIP first`.
//
// Every row below measured base=2 -> head=0 before this guard. Non-vacuous
// by construction: each heredoc body is conforming, so a hook that resolves
// the wrong -m returns 0 on all four. The counterpart row above
// ("leaves every non-heredoc form exactly as it was") covers these same
// positions with a plain `WIP`, which never activates the resolver — which
// is exactly why this interaction went unnoticed.
const body = "$(cat <<'EOF'\nfeat: accepted body\nEOF\n)";
for (const [label, cmd] of [
['an earlier single-quoted -m', `git commit --allow-empty -m 'WIP first' -m "${body}"`],
['an earlier unquoted -m', `git commit -m WIP -m "${body}"`],
['after -- it is a pathspec, not a message', `git commit -m WIP -- -m "${body}"`],
['it belongs to a later command', `git commit -m WIP && echo -m "${body}"`],
]) {
const result = runHookCmd(cmd);
assert.strictEqual(result.status, 2,
`${label}: git's real subject is the FIRST message, which is non-conforming — resolving `
+ 'the later heredoc validates text git never uses as the subject');
assert.strictEqual(JSON.parse(result.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
}
});
test('validate-commit refuses to resolve under a non-default cleanup mode (round-4 Codex BLOCKER)', () => {
// The resolver strips trailing whitespace and skips leading blank lines
// because git's DEFAULT cleanup=whitespace does. Under `--cleanup=verbatim`
// git does neither, so this subject is committed at 75 bytes while the hook
// measured the stripped 72 — COMMIT_SUBJECT_TOO_LONG dodged (base=2 ->
// head=0). Confirmed by reading the RAW commit object: `git log --pretty=%s`
// strips trailing whitespace in its own output and hides the difference.
const subject72 = `feat: ${'x'.repeat(66)}`;
assert.strictEqual(subject72.length, 72, 'fixture built wrong');
const heredocBody = `"$(cat <<'EOF'\n${subject72} \nEOF\n)"`;
for (const [label, cmd] of [
['--cleanup=verbatim', `git commit --allow-empty --cleanup=verbatim -m ${heredocBody}`],
['-c commit.cleanup=verbatim', `git -c commit.cleanup=verbatim commit --allow-empty -m ${heredocBody}`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 2,
`${label}: git preserves the trailing whitespace, so the real subject is 75 chars — the `
+ 'hook must not measure the stripped form');
}
// Non-vacuity: the DEFAULT mode is the case the fix exists for, and it must
// still resolve and allow. Without these the rows above would pass for a
// hook that simply stopped resolving everything.
for (const [label, cmd] of [
['--cleanup=whitespace', `git commit --allow-empty --cleanup=whitespace -m ${heredocBody}`],
['no cleanup flag', `git commit --allow-empty -m ${heredocBody}`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 0,
`${label}: git strips the trailing whitespace here, so the real subject is a conforming 72`);
}
// SCOPE (review of #3816, round 5 — BLOCKER). The guard scanned the whole
// command, and the heredoc BODY sits verbatim inside it, so a conforming
// message that merely MENTIONED the token was refused and fell back to the
// opener line — blocked with CONVENTIONAL_COMMITS_VIOLATION. These are
// ordinary English in this repository, whose own hooks and docs discuss
// cleanup modes constantly. Every row is a valid Conventional Commit that
// git would accept without complaint.
for (const [label, subject] of [
['commit.cleanup= in the subject', 'fix: document commit.cleanup=strip behavior'],
['--cleanup= in the subject', 'docs: explain --cleanup=verbatim in the hook guide'],
['the token on a later body line', 'fix: correct the guard scope\n\nIt scanned --cleanup=verbatim in the body.'],
]) {
const result = runHookCmd(`git commit -m "$(cat <<'EOF'\n${subject}\nEOF\n)"`);
assert.strictEqual(result.status, 0,
`${label}: the token is message TEXT, not a flag git will act on — refusing to resolve `
+ 'here blocks a commit git would accept');
}
// The scope fix must not shrink to $MSG_PREFIX alone. git accepts the flag
// on EITHER side of -m, so a trailing occurrence is a real mode change and
// must still refuse — this row reds against a prefix-only scoping and is
// what keeps the round-4 length-gate bypass closed from both directions.
assert.strictEqual(
runHookCmd(`git commit --allow-empty -m ${heredocBody} --cleanup=verbatim`).status, 2,
'a --cleanup after the message changes the mode just as one before it does');
});
test('the adjacency guard is scoped to the span it matched (round-6 MAJOR)', () => {
// Glue is a property of the ONE character following the MATCHED span, so
// that character is the whole window. Scanning $CMD for the shape anywhere
// refused any conforming commit whose command merely CONTAINED a glued -m
// elsewhere. Base blocks these too, because base blocks EVERY heredoc form
// (that is #3802), so this is the fix not reaching the shape rather than a
// regression: measured base=2 -> pre=2 -> post=0.
const conforming = "\"$(cat <<'EOF'\nfix: a perfectly ordinary conforming subject\nEOF\n)\"";
assert.strictEqual(
runHookCmd(`git commit -m ${conforming} && echo -m "test"z`).status, 0,
'a glued -m in a chained-after command is in another argv and cannot truncate this capture');
// Bash does not concatenate across a command separator or a redirection: in
// `-m "msg"&& echo hi` the argument ends at the quote, so there is no
// truncated capture and nothing to defend against. This row reds against a
// bare `^[^[:space:]]` test, which is why the class excludes `;&|()<>`.
assert.strictEqual(
runHookCmd(`git commit -m ${conforming}&& echo hi`).status, 0,
'a separator abutting the closing quote ends the argument; it does not glue onto it');
// A suffix that really is glued still refuses.
assert.strictEqual(
runHookCmd(`git commit -m ${conforming}zzzz`).status, 2,
'text glued to the closing quote means the capture holds only a prefix of the real message');
});
test('the cleanup guard scans wide on purpose — narrowing it reopened a length hole', () => {
// The window is the whole command minus the message. That is deliberately
// wider than git's own command, and the cost is a known false positive:
// a --cleanup= belonging to a DIFFERENT command refuses a commit git would
// accept. Narrowing it to git's own segment was tried and reverted, because
// deciding where git's command ends needs a shell parse and a substring
// scan is not one.
const long72 = `feat: ${'x'.repeat(66)}`;
const longHd = `"$(cat <<'EOF'\n${long72} \nEOF\n)"`;
// These rows are the ACCEPT direction and are the reason the guard stays
// wide. Trimming the window at the first `;&|` cut it short whenever a
// separator sat inside an ordinary argument, hiding the real trailing
// --cleanup=verbatim: git then commits the trailing whitespace verbatim and
// the real subject is 75 bytes while the gate measured 72. Both the quoted
// and the backslash-escaped spelling must stay blocked; each reds against
// one of the two narrowings that were attempted.
for (const [label, author] of [
['quoted separator', '"a&b"'],
['quoted pipe', '"a|b"'],
['quoted semicolon', '"a;b"'],
['escaped separator', 'a\\&b'],
['escaped pipe', 'a\\|b'],
['escaped semicolon', 'a\\;b'],
]) {
assert.strictEqual(
runHookCmd(`git commit --allow-empty -m ${longHd} --author ${author} --cleanup=verbatim`).status,
2,
`${label}: a separator inside an argument must not hide the --cleanup that follows it`);
}
// The documented false positive, pinned so the trade-off is visible rather
// than accidental. If this ever needs to pass, it needs a real shell parse.
assert.strictEqual(
runHookCmd(`git commit -m "$(cat <<'EOF'\nfix: a perfectly ordinary conforming subject\nEOF\n)" && echo --cleanup=verbatim`).status,
2,
'known limit: a --cleanup in a later command also refuses — fail-closed, and preferred '
+ 'over the accept-direction hole that narrowing the window reopened');
});
test('option scans read the command the way bash hands it to git (round-6 accept direction)', () => {
// Three ways the same option can be spelled without matching a literal.
// Every row below was measured ACCEPTING a commit whose real subject git
// records as 75 bytes, or whose real subject is a different -m argument
// entirely — verified against real commits by reading the raw commit
// object, since `git log --pretty=%s` strips the trailing whitespace that
// makes the length wrong and hides it. All are base=2 -> pre=0, so each is
// an accept-direction regression this PR introduced before this round.
const long72 = `feat: ${'x'.repeat(66)}`;
const longHd = `"$(cat <<'EOF'\n${long72} \nEOF\n)"`;
const conforming = "\"$(cat <<'EOF'\nfix: a perfectly ordinary conforming subject\nEOF\n)\"";
// git accepts any unambiguous prefix of a long option, so the mode is set
// by a token that is not the literal `--cleanup`. Reds against `--cleanup`.
for (const spelling of ['--cle', '--clea', '--clean', '--cleanu', '--cleanup']) {
assert.strictEqual(
runHookCmd(`git commit --allow-empty -m ${longHd} ${spelling}=verbatim`).status, 2,
`${spelling}=verbatim sets the mode as surely as the unabbreviated spelling does`);
}
// git splits `-am` into `-a -m`, making the FIRST message the subject and
// the heredoc merely the second. Reds against a standalone `-m` scan.
for (const cluster of ['-am', '-sm', '-anm']) {
assert.strictEqual(
runHookCmd(`git commit --allow-empty ${cluster} 'WIP first' -m ${conforming}`).status, 2,
`${cluster} carries git's first message, so the matched heredoc is not the subject`);
}
// Bash removes quotes before git sees the argument, so a spliced spelling
// is the same option. Reds unless the option-name scans are dequoted.
assert.strictEqual(
runHookCmd(`git commit --allow-empty -m ${longHd} --clean""up=verbatim`).status, 2,
'a quote spliced into the option name does not change the option git receives');
assert.strictEqual(
runHookCmd(`git commit --allow-empty -""m 'WIP first' -m ${conforming}`).status, 2,
'a quote spliced into -m does not stop it claiming the first message');
// Dequoting must not spill into the adjacency test, which asks about a
// literal character position rather than an option name. This is the
// round-6 MAJOR and must stay fixed.
assert.strictEqual(
runHookCmd(`git commit -m ${conforming} && echo -m "test"z`).status, 0,
'quotes in a chained-after command must not refuse the matched heredoc');
});
test('the two reported chained-before shapes are never reached by any guard (round-7 pin)', () => {
// Round 7 reported that the FIRST-MESSAGE GUARD's `[;&|]` prefix scan
// blocks `git add -A && git commit -m <heredoc>` and
// `cd dir && git commit -m <heredoc>`. It does not, and cannot: the
// CLASSIFIER GATE runs first and neither shape reaches the guards at all.
// `isGitSubcommand` token-walks from the START of the command — `git`→`add`
// stops on a non-commit subcommand, and a leading `cd` is not git — so the
// hook exits 0 before a single guard is evaluated.
//
// Both rounds 6 and 7 produced this finding by extracting the guard logic
// into a standalone script and feeding it command strings directly, which
// bypasses the gate. These rows exist so the same measurement cannot
// produce a third phantom: they run the REAL hook, end to end.
//
// The NON-CONFORMING rows are what make the pin load-bearing. A row
// asserting only that a conforming chained message exits 0 is satisfied
// both by "resolved correctly" and by "never validated" — the two
// hypotheses under dispute. A message that the bare form blocks, passing
// in the chained form, can only mean the hook never validated it.
const conforming = `"$(cat <<'EOF'
fix: a perfectly ordinary conforming subject
EOF
)"`;
const NONCONFORMING = 'nope not conventional';
// Control FIRST: the hook demonstrably blocks this message when it does
// classify the command. Without this row the two below prove nothing,
// because a hook that blocks nothing at all also "allows" them.
const bare = runHookCmd(`git commit -m "${NONCONFORMING}"`);
assert.strictEqual(bare.status, 2,
'control: the bare form must BLOCK a non-conforming subject — otherwise the chained rows '
+ 'below cannot distinguish "not validated" from "validated and allowed"');
assert.strictEqual(JSON.parse(bare.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
for (const [label, prefix] of [
['stage-then-commit', 'git add -A && '],
['cd-then-commit', 'cd /repo && '],
]) {
// The heredoc shape round 7 says is blocked. It is not.
assert.strictEqual(runHookCmd(`${prefix}git commit -m ${conforming}`).status, 0,
`${label}: a conforming chained heredoc commit is not blocked`);
// ...and the same shape carrying a message the control just proved is
// blockable ALSO exits 0, which is only possible if no validation ran.
assert.strictEqual(runHookCmd(`${prefix}git commit -m "${NONCONFORMING}"`).status, 0,
`${label}: a subject the bare form BLOCKS exits 0 here — proof the classifier gate `
+ 'returns before the guards, so the guards cannot be over-blocking this shape');
}
// COUNTEREXAMPLE, so this row is not misread as a blanket claim about every
// chained-before command (independent review of #3816, round 7). Assignment
// detection is prefix-anchored and the tokenizer does not split operators,
// so `FOO=bar;` is read as an assignment prefix and the classifier DOES
// reach `git commit` — which the separator scan then refuses. That makes it
// a genuine false positive of exactly the class round 7 describes, reachable
// where the two reported shapes are not. Left unfixed deliberately:
// narrowing it means changing `isGitSubcommand`, the shared git-commit
// detector every gating hook uses, and it fails CLOSED (a conforming commit
// is blocked, which is recoverable). Disclosed in the changeset instead.
assert.strictEqual(runHookCmd(`FOO=bar; git commit -m ${conforming}`).status, 2,
'a leading assignment carrying a separator IS classified and then refused — pinned as a '
+ 'known fail-closed false positive, not as desired behaviour. If this ever starts passing, '
+ 'the classifier or tokenizer changed: re-read the chained-before disclosure before '
+ 'celebrating.');
});
// ─── round 7: six accept-direction bypasses found by independent review ───
//
// Every row below was measured base-vs-head against the REAL hook, and the
// three that turn on which subject git actually records were confirmed
// against the RAW COMMIT OBJECT rather than `git log --pretty=%s`, which
// strips trailing whitespace and would have hidden two of them:
//
// git commit -mWIP -m <conforming heredoc> -> subject `WIP`
// --cleanup=whitespace -m <72+spaces> --cleanup=verbatim -> 75 bytes, WS kept
// git commit --squash=<c> -m <conforming heredoc> -> `squash! …`
//
// In every case the hook resolved and validated the heredoc and ALLOWED the
// commit, while git recorded something the rules would have refused. All six
// fixes widen refusal, never narrow it — the direction this file already
// documents as the recoverable one.
const HD_OK = `"$(cat <<'EOF'\nfix: a perfectly ordinary conforming subject\nEOF\n)"`;
const S72 = `feat: ${'x'.repeat(66)}`;
const HD_LONG_DIRTY = `"$(cat <<'EOF'\n${S72} \nEOF\n)"`;
test('the first-message guard recognises attached values and --message abbreviations (round 7)', () => {
// The scan required a space or `=` after the option name, so `-mWIP` (git
// reads it as `-m WIP`) and `--mes=WIP` (git accepts any unambiguous long
// prefix — behaviour this file already models for --cleanup) matched
// nothing. git took `WIP` as the subject; the hook validated the later
// heredoc and allowed it.
assert.strictEqual(S72.length, 72, 'fixture built wrong');
for (const [label, cmd] of [
['attached short-option value', `git commit -mWIP -m ${HD_OK}`],
['--message abbreviation', `git commit --mes=WIP -m ${HD_OK}`],
]) {
const r = runHookCmd(cmd);
assert.strictEqual(r.status, 2,
`${label}: git takes the FIRST message as the subject, so the heredoc is not the subject `
+ 'and must not be resolved and measured as though it were');
assert.strictEqual(JSON.parse(r.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
}
// Non-vacuity: the canonical form still resolves. Without this, the rows
// above would also pass if resolution had simply been disabled outright.
assert.strictEqual(runHookCmd(`git commit -m ${HD_OK}`).status, 0,
'the canonical single -m heredoc must still pass — that is the fix this PR exists for');
});
test('option-name scans remove backslashes, as bash does (round 7)', () => {
// Round 6 dequoted the option-name windows and the changeset claimed they
// are matched "as bash hands it to git". That was not true: bash also
// removes syntactic backslashes, so `-\\m` IS `-m` and `--clean\\up=` IS
// `--cleanup=`, and both matched no literal.
const r1 = runHookCmd(`git commit -\\m WIP -m ${HD_OK}`);
assert.strictEqual(r1.status, 2,
'a backslash spliced into -m does not stop it claiming the first message');
const r2 = runHookCmd(`git commit --allow-empty -m ${HD_LONG_DIRTY} --clean\\up=verbatim`);
assert.strictEqual(r2.status, 2,
'a backslash spliced into --cleanup does not change the mode git applies, and under verbatim '
+ 'the trailing spaces count toward the 72-character limit');
assert.strictEqual(runHookCmd(`git commit -m ${HD_OK}`).status, 0, 'non-vacuity: canonical form still resolves');
});
test('option-name scans remove the $ of a dollar-quote, as bash does (round 8)', () => {
// Round 7 removed quotes and backslashes and the changeset again claimed the
// windows are matched "as bash hands it to git". Still not true: bash has two
// more quoting forms, $'…' and $"…", whose introducer is a `$`. Removing the
// quote characters alone left that `$` stranded INSIDE the option name, so
// `-$"m"` dequoted to `-$m` and matched no literal while bash passed a real
// `-m` to git. Measured on bash 3.2.57 and 5.3.15 against a real repository:
// the hook allowed the command (exit 0) and `git cat-file -p` recorded the
// subject `WIP`, while the same command spelled `-m WIP` was refused.
for (const [label, cmd] of [
['locale-quoted short option', `git commit -$"m" WIP -m ${HD_OK}`],
['ANSI-quoted short option', `git commit -$'m' WIP -m ${HD_OK}`],
['locale-spliced --message', `git commit --mes$"sage"=WIP -m ${HD_OK}`],
['ANSI-spliced --cleanup', `git commit --allow-empty -m ${HD_LONG_DIRTY} --clean$'up'=verbatim`],
]) {
const r = runHookCmd(cmd);
assert.strictEqual(r.status, 2,
`${label}: a dollar-quote is removed by bash before git sees the argument, so it does not `
+ 'stop the option claiming the message');
}
assert.strictEqual(runHookCmd(`git commit -m ${HD_OK}`).status, 0, 'non-vacuity: canonical form still resolves');
});
test('an option NAME carrying a shell expansion is unresolvable (round 9)', () => {
// Rounds 6-8 each tried to EMULATE what bash does to an argument before git
// sees it -- remove quotes, then backslashes, then the `$` of a dollar-quote
// -- and review found another missed transform every time. Round 9 found
// four more, all measured accepting `WIP` as the real subject on bash 3.2.57
// and 5.3.15 while the plain spelling of the same command is refused.
//
// The last two settle the strategy: an option name finished by a PARAMETER
// expansion depends on a variable's runtime value, and one finished by a
// PATHNAME expansion depends on the contents of the working directory.
// Neither is derivable from the command string at all, so the rule is no
// longer "normalise and match the literal" but "an option NAME carrying a
// shell expansion or quoting construct is unresolvable, and unresolvable
// refuses" -- which covers the spellings nobody has thought of yet.
for (const [label, cmd] of [
['$() substitution', `git commit --allow-empty -m ${HD_LONG_DIRTY} --clean$(printf up)=verbatim`],
['backtick substitution', 'git commit --allow-empty -m ' + HD_LONG_DIRTY + ' --clean`printf up`=verbatim'],
['ANSI-C octal escape', `git commit --allow-empty -$'\\155' WIP -m ${HD_OK}`],
['ANSI-C hex escape', `git commit --allow-empty -$'\\x6d' WIP -m ${HD_OK}`],
['parameter expansion', `x= git commit --allow-empty -\${x}m WIP -m ${HD_OK}`],
['pathname expansion', `git commit --allow-empty -? WIP -m ${HD_OK}`],
['short-option substitution', `git commit -$(printf m) WIP -m ${HD_OK}`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 2,
`${label}: an option name the shell finishes cannot be read off the command line, so the `
+ 'later heredoc must not be resolved and measured as though it were the subject');
}
// SCOPE, in both directions. The class is a `-`-leading token whose
// characters up to the construct contain no `=` -- an option NAME being
// assembled. A construct in the VALUE is something this file never models
// and must stay allowed, or the guard refuses the ordinary
// `--author="$(git config user.name)"` idiom. Without these rows the
// assertions above would also pass for a guard that refuses every `$`.
for (const [label, cmd] of [
['spaced value', `git commit --author "$(git config user.name)" -m ${HD_OK}`],
['glued value', `git commit --author="$(git config user.name)" -m ${HD_OK}`],
['backtick value', 'git commit --author="`git config user.name`" -m ' + HD_OK],
['parameter expansion value', `git commit --date="\${NOW}" -m ${HD_OK}`],
['glob character in a value', `git commit --author="a*b <x@y.z>" -m ${HD_OK}`],
['value in the suffix window', `git commit -m ${HD_OK} --author="$(id -un)"`],
['pathspec after --', `git commit -m ${HD_OK} -- src/*.js`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 0,
`${label}: a construct in an option VALUE, or outside an option name entirely, is not an `
+ 'option name being assembled and must still resolve');
}
});
test('the round-9 class over-blocks two spellings, and both have working forms', () => {
// Disclosed rather than narrowed. The class refuses a `-`-leading token that
// carries an expansion before any `=`, and two legitimate-looking spellings
// fall inside it. Narrowing to exclude them was considered and rejected:
// dropping the bare-`$` member reopens `-$xm` (with `xm=m` bash hands git a
// real `-m`), and skipping tokens after `--` means deciding where git's
// options end from a substring scan, which is the class this file has
// already reverted twice for opening accept-direction holes. Refusing a
// commit git would take is the recoverable direction; accepting a
// non-conforming subject is not.
for (const [label, cmd] of [
['attached short-option value from an expansion', `git commit -S$SIGNING_KEY -m ${HD_OK}`],
['attached short-option value, quoted', `git commit -S"$SIGNING_KEY" -m ${HD_OK}`],
['unquoted dash-leading glob after --', `git commit -m ${HD_OK} -- -*.txt`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 2, `${label}: known fail-closed limit of the round-9 class`);
}
// The working spellings, which is what makes the limit acceptable. Note the
// pathspec ones in particular: a glob only reaches git as a PATHSPEC when it
// is quoted, because an unquoted one is expanded by the shell before git
// sees it. So the spelling that actually passes a glob to git is the one
// that resolves here.
for (const [label, cmd] of [
['detached signing key', `git commit -S "$SIGNING_KEY" -m ${HD_OK}`],
['long signing option', `git commit --gpg-sign="$SIGNING_KEY" -m ${HD_OK}`],
['single-quoted glob pathspec', `git commit -m ${HD_OK} -- '-*.txt'`],
['double-quoted glob pathspec', `git commit -m ${HD_OK} -- "-*.txt"`],
['magic pathspec', `git commit -m ${HD_OK} -- ':(exclude)-*.txt'`],
]) {
assert.strictEqual(runHookCmd(cmd).status, 0,
`${label}: the spelling a developer reaches for must still resolve`);
}
});
test('a backslash-newline continuation before -m is joined, not treated as a separator (round 9)', () => {
// `git commit \` newline ` -m <heredoc>` was refused because every guard
// read the newline as a separator — disclosed in round 8 as a fail-closed
// limit, re-raised in round 9 as a Major. bash's rule is local: a newline
// preceded by an ODD run of backslashes is a continuation (both removed);
// an EVEN run is a literal backslash plus a REAL newline. Both windows are
// joined the same way before any guard runs. Not bash-version dependent —
// the join is plain parameter expansion — but run under each bash on the
// machine anyway, since the round-8 classes were.
const bashes = ['/bin/bash', '/opt/homebrew/bin/bash'].filter((b) => fs.existsSync(b));
assert.ok(bashes.length >= 1, 'at least one bash must exist');
const run = (bash, command) => spawnHook(path.join(HOOKS_DIR, 'gsd-validate-commit.sh'), {
input: JSON.stringify({ tool_input: { command } }), encoding: 'utf-8', cwd: tmpDir, interpreter: bash,
});
for (const bash of bashes) {
// The idiom itself, on both sides of the message.
assert.strictEqual(run(bash, `git commit \\\n -m ${HD_OK}`).status, 0,
`${bash}: a continuation before -m is joined by bash, so it must be joined here and resolve`);
assert.strictEqual(run(bash, `git commit --allow-empty \\\n --no-verify \\\n -m ${HD_OK}`).status, 0,
`${bash}: several continuations resolve`);
assert.strictEqual(run(bash, `git commit -m ${HD_OK} \\\n --allow-empty`).status, 0,
`${bash}: a continuation in the suffix window resolves`);
// ACCEPT-DIRECTION CONTROLS. An even run is a literal backslash followed
// by a REAL newline, which is a separator and must still refuse.
const evenRun = run(bash, `git commit \\\\\n -m ${HD_OK}`);
assert.strictEqual(evenRun.status, 2,
`${bash}: backslash-backslash-newline is a literal \\ then a real separator; joining it would let a later command's -m be taken for this commit's`);
// A plain newline is unchanged.
assert.strictEqual(run(bash, `git commit\n -m ${HD_OK}`).status, 2, `${bash}: a bare newline is still a separator`);
// Continuation GLUE: bash joins `"$(…)"\` newline `suffix` into one argument,
// so the glue guard must see it glued and refuse, same as without the newline.
assert.strictEqual(run(bash, `git commit -m ${HD_OK}\\\nsuffix`).status, 2,
`${bash}: a continuation glued to the closing quote is glue, and the joined text must show it`);
// A later command's heredoc-shaped -m after a continuation is STILL a later
// command once the real separator is reached.
assert.strictEqual(run(bash, `git commit --amend --no-edit \\\n --allow-empty\necho -m ${HD_OK}`).status, 2,
`${bash}: joining continuations must not hide the real newline separator that follows`);
}
});
test('a newline is a command separator for the first-message guard (round 7)', () => {
// The separator scan covered `;`, `&` and `|` but not a literal newline, so
// a LATER command's heredoc-shaped -m was taken for this commit's message.
// The classifier recognises the leading `git commit`, and the capture reads
// across the newline into echo's argument — a conforming string with no
// relationship to the commit was validated and the commit allowed.
const r = runHookCmd(`git commit --amend --no-edit\necho -m ${HD_OK}`);
assert.strictEqual(r.status, 2,
"a later command's -m is not this commit's message; a newline separates commands exactly as "
+ '`;` does');
assert.strictEqual(JSON.parse(r.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
});
test('more than one cleanup directive is unresolvable — git applies the LAST (round 7)', () => {
// A bash regex yields ONE BASH_REMATCH, so only the FIRST directive was
// inspected while git applies the last. Measured: mode read as whitespace,
// resolution stayed on, and a 72-character subject plus trailing spaces was
// accepted while git recorded 75 bytes with the whitespace preserved.
// Which directive is last needs an argv order a substring scan does not
// have, so multiplicity itself refuses.
const r = runHookCmd(`git commit --allow-empty --cleanup=whitespace -m ${HD_LONG_DIRTY} --cleanup=verbatim`);
assert.strictEqual(r.status, 2,
'a leading whitespace directive must not vouch for a trailing verbatim one');
// Non-vacuity in BOTH directions: a single whitespace directive still
// resolves, and a single non-whitespace one still refuses. Without these,
// the row above passes for a guard that simply refuses every cleanup.
assert.strictEqual(runHookCmd(`git commit --cleanup=whitespace -m ${HD_OK}`).status, 0,
'one explicit whitespace directive is the documented default and must still resolve');
assert.strictEqual(runHookCmd(`git commit --allow-empty --cleanup=verbatim -m ${HD_LONG_DIRTY}`).status, 2,
'one verbatim directive must still refuse — unchanged behaviour');
});
test('a git-GENERATED subject is never measured against the supplied message (round 7)', () => {
// With --squash/--fixup git composes the subject itself, so the supplied
// message is not the subject at all. Measured recording
// `squash! base: something` while a conforming heredoc sailed through.
for (const [label, opt] of [['--squash', '--squash=HEAD'], ['--fixup', '--fixup=HEAD']]) {
const r = runHookCmd(`git commit ${opt} -m ${HD_OK}`);
assert.strictEqual(r.status, 2,
`${label}: git composes the subject, so there is nothing in the -m text worth measuring`);
assert.strictEqual(JSON.parse(r.stdout).code, 'CONVENTIONAL_COMMITS_VIOLATION');
}
assert.strictEqual(runHookCmd(`git commit -m ${HD_OK}`).status, 0, 'non-vacuity: canonical form still resolves');
});
test('validate-commit does not trust a relative path ending in cat (round-4 Codex MAJOR)', () => {
// Recognition accepted any path ending in `/cat`, so a planted `./cat` or
// `../evil/cat` was trusted to echo its stdin. With such an executable
// printing `WIP injected`, the resolver validated the heredoc body while
// git's real subject was `WIP injected` (measured base=2 -> head=0 against
// a real commit). Only an absolute path or a bare `cat` is recognised now.
//
// RESIDUAL, and not fixable from a string: a bare `cat` shadowed earlier on
// PATH behaves identically and is indistinguishable here. It is also not a
// meaningful boundary — anyone able to plant an executable on PATH can run
// `git commit` directly.
const body = "<<'EOF'\nfeat: accepted body\nEOF\n)";
for (const [label, prog] of [['./cat', './cat'], ['../evil/cat', '../evil/cat'], ['x/cat', 'x/cat']]) {
assert.strictEqual(runHookCmd(`git commit -m "$(${prog} ${body}"`).status, 2,
`${label}: a relative executable merely ENDING in cat is not known to echo its stdin`);
}
// Non-vacuity: the legitimate absolute and bare forms still resolve.
assert.strictEqual(runHookCmd(`git commit -m "$(/bin/cat ${body}"`).status, 0,
'an absolute /bin/cat is the same canonical form and must still resolve');
assert.strictEqual(runHookCmd(`git commit -m "$(cat ${body}"`).status, 0,
'a bare cat is the canonical idiom #3802 is about');
});
test('an ABSOLUTE path is not an identity either (round-8 independent review)', () => {
// Round 4 stopped at "must be absolute", so any absolute path ENDING in
// `/cat` was still trusted to echo its stdin — the very thing the round-4
// reasoning rejected one spelling earlier. With an executable at
// `/some/scratch/dir/cat` printing `WIP injected`, the resolver validated the
// conforming heredoc body while git's real subject was `WIP injected`
// (measured on bash 3.2.57 and 5.3.15 against a real commit: hook exit 0,
// `git cat-file -p` subject `WIP injected`). Recognition is now the canonical
// system locations, the only claim a string can support.
const body = "<<'EOF'\nfeat: accepted body\nEOF\n)";
for (const [label, prog] of [
['a scratch directory', '/tmp/evil/cat'],
['a home directory', '/Users/someone/bin/cat'],
['user-writable /usr/local/bin', '/usr/local/bin/cat'],
]) {
assert.strictEqual(runHookCmd(`git commit -m "$(${prog} ${body}"`).status, 2,
`${label}: an absolute path merely ENDING in cat is not known to echo its stdin`);
}
// Non-vacuity: the canonical spellings must all still resolve, or this guard
// has simply disabled the feature #3802 exists for.
for (const prog of ['cat', '/bin/cat', '/usr/bin/cat']) {
assert.strictEqual(runHookCmd(`git commit -m "$(${prog} ${body}"`).status, 0,
`${prog} is a canonical cat and must still resolve`);
}
});
test('validate-commit allows non-commit commands', () => {
const hookPath = path.join(HOOKS_DIR, 'gsd-validate-commit.sh');
const input = JSON.stringify({

372
tests/worktree-safety.test.cjs
View File

@@ -4359,7 +4359,11 @@ const path = require('node:path');
const fs = require('node:fs');
const ROOT = path.join(__dirname, '..');
const { isGitSubcommand, tokenize, extractBranchArgument } = require(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js'));
// Seeded fast-check convention: the shared setup helper, NOT 'fast-check'
// directly, so numRuns/seed are configured globally before any fc.assert().
// Required by RULESET.TESTS.property-based-testing for the parser added below.
const fc = require('./helpers/fast-check-setup.cjs');
const { isGitSubcommand, tokenize, extractBranchArgument, resolveCommitSubject } = require(path.join(ROOT, 'hooks', 'lib', 'git-cmd.js'));
// ── tokenize ─────────────────────────────────────────────────────────────────
@@ -4457,6 +4461,372 @@ describe('gsd-validate-commit.sh delegates to git-cmd.js', () => {
});
});
// ── resolveCommitSubject (#3802) ─────────────────────────────────────────────
// A PURE STRING helper: it maps an already-selected `-m` argument to the subject
// to validate. It deliberately does not tokenize — an earlier revision walked
// tokens and regressed four cases that upstream allowed (`git commit -- -m WIP`,
// `git commit --amend && echo -m WIP`, `-m "" --allow-empty-message`, and
// unquoted `git commit -m WIP`). Reported in review of #3802.
describe('git-cmd.js resolveCommitSubject', () => {
const sub = (open, body, close) => `$(cat ${open}\n${body}\n${close}\n)`;
test('resolves the heredoc body rather than the opener', () => {
assert.strictEqual(resolveCommitSubject(sub("<<'EOF'", 'feat(auth): add login flow', 'EOF')),
'feat(auth): add login flow');
});
test('accepts the QUOTED opener spellings, which bash does not expand', () => {
// Only the spellings that SUPPRESS expansion may be resolved. The two bare
// rows that used to live here — `<<EOF` and `<< EOF`, both asserted to
// resolve — are the round-4 BLOCKER and now assert the opposite, in the
// dedicated row below (review of #3816, round 4).
// no space before << is legal bash too (review of #3816, round 3)
assert.strictEqual(resolveCommitSubject("$(cat<<'EOF'\nfix: nospace\nEOF\n)"), 'fix: nospace');
// NOTE: resolvable HERE, but unreachable through gsd-validate-commit.sh —
// its DOUBLE-quoted `-m` capture stops at this spelling's own delimiter
// quote. Round 4 disproved the stronger form of this claim: the
// SINGLE-quoted capture delivers the spelling intact, so "unreachable"
// held only for one arm. It holds for both now because the hook gates the
// resolver on the double-quoted arm — a consequence of that gate, not a
// property of the capture alone. The hook-level rows in
// tests/hooks-opt-in.test.cjs pin both halves; all are correct together.
assert.strictEqual(resolveCommitSubject(sub('<<"EOF"', 'fix: dquoted', 'EOF')), 'fix: dquoted');
// A delimiter that is not identifier-shaped is still a valid bash word.
assert.strictEqual(resolveCommitSubject(sub("<<'END-MSG'", 'fix: hyphen tag', 'END-MSG')),
'fix: hyphen tag');
});
test('round 4: a RELATIVE path ending in cat is not recognised', () => {
// Codex review of #3816, round 4. The path class accepted `./cat` and
// `../evil/cat`, so any relative executable merely ENDING in `cat` was
// trusted to echo its stdin. With a planted one printing `WIP injected`,
// the resolver validated the heredoc body while git's real subject was
// `WIP injected` (measured base=2 -> head=0 against a real commit).
// Non-vacuous: each body below is conforming, so a resolver that still
// recognised these returns the body.
for (const prog of ['./cat', '../evil/cat', 'x/cat']) {
assert.strictEqual(resolveCommitSubject(`$(${prog} <<'EOF'\nfix: body\nEOF\n)`),
`$(${prog} <<'EOF'`, `${prog}: a relative path is not a known cat`);
}
});
test('a path-qualified cat is still the same form', () => {
assert.strictEqual(resolveCommitSubject("$(/bin/cat <<'EOF'\nfix: pathed cat\nEOF\n)"),
'fix: pathed cat');
});
test('<<- strips the leading tabs bash strips', () => {
// With `<<-`, bash removes leading TABS from body lines, so the subject the
// user sees has none. Returning the raw line blocked a conforming message.
// The delimiter is QUOTED here because `<<-` and quoting are independent:
// `<<-` controls tab stripping, the quote controls expansion. This row is
// about tab stripping, so it uses a spelling that is resolvable at all —
// a bare `<<-EOF` is refused by the round-4 expansion guard, which is that
// guard's row to assert, not this one's.
assert.strictEqual(resolveCommitSubject("$(cat <<-'EOF'\n\tfix(parser): strip heredoc tabs\n\tEOF\n)"),
'fix(parser): strip heredoc tabs');
});
test('an immediately-following terminator is an EMPTY message, not a subject', () => {
// `$(cat <<'EOF'` then straight to `EOF` — the message is empty, and the
// delimiter must not be mistaken for the subject.
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nEOF\n)"), '');
assert.strictEqual(resolveCommitSubject("$(cat <<-'EOF'\n\tEOF\n)"), '',
'the <<- form strips the tab first, so the terminator still matches');
});
// The security half. Recognition is anchored at BOTH ends and requires a
// command substitution, so a message that merely contains — or ENDS IN —
// `<<WORD` is not an opener.
test('a message ENDING in <<WORD is not a heredoc — this was an enforcement bypass', () => {
// Without the `^$(` anchor this resolved to line 2 and ALLOWED a
// non-conforming commit (review of #3802).
assert.strictEqual(resolveCommitSubject('WIP notes <<EOF\nfix: smuggled subject'),
'WIP notes <<EOF', 'the real subject is the non-conforming first line, and must be judged');
});
test('a message merely containing << is untouched', () => {
assert.strictEqual(resolveCommitSubject('fix(parser): preserve literal <<EOF'),
'fix(parser): preserve literal <<EOF');
assert.strictEqual(resolveCommitSubject('fix(parser): handle a << b shifts'),
'fix(parser): handle a << b shifts');
});
test('a COMMAND smuggled before the cat is not a path — recognition must fail closed', () => {
// Codex review of #3816: `\S*` as the path prefix accepted `id;/bin/cat`,
// so the resolver validated the heredoc BODY while bash runs `id` first and
// git's real subject is id's OUTPUT — an enforcement bypass. A prefix
// carrying any shell metacharacter now fails recognition and falls back to
// the opener line, which the format gate rejects.
assert.strictEqual(resolveCommitSubject("$(id;/bin/cat <<'EOF'\nfix: smuggled\nEOF\n)"),
"$(id;/bin/cat <<'EOF'");
assert.strictEqual(resolveCommitSubject("$(x&&/bin/cat <<'EOF'\nfix: smuggled\nEOF\n)"),
"$(x&&/bin/cat <<'EOF'");
assert.strictEqual(resolveCommitSubject("$(a|b/cat <<'EOF'\nfix: smuggled\nEOF\n)"),
"$(a|b/cat <<'EOF'");
// Round 2: Unicode whitespace after `$(` is NOT bash whitespace — bash
// reads `<NBSP>/bin/cat` as the executable NAME, so recognizing it here
// claimed a substitution that does not run cat. Recognition whitespace is
// ASCII space/tab only.
assert.strictEqual(resolveCommitSubject("$(\u00a0/bin/cat <<'EOF'\nfix: smuggled\nEOF\n)"),
"$(\u00a0/bin/cat <<'EOF'");
// the legitimate path-qualified form is unchanged
assert.strictEqual(resolveCommitSubject("$(/usr/bin/cat <<'EOF'\nfix: pathed\nEOF\n)"),
'fix: pathed');
});
test('a Unicode-blank first line is the SUBJECT — git keeps what trim() skips', () => {
// Codex review of #3816, verified against `git stripspace`: git's blank is
// ASCII space/tab, so a NBSP line is PRESERVED and is the real subject.
// JavaScript's trim() treated it as blank and resolved to the second line —
// validating a line git never uses, an enforcement bypass.
const nbsp = '\u00a0';
assert.strictEqual(resolveCommitSubject(`$(cat <<'EOF'\n${nbsp}\nfix: smuggled\nEOF\n)`), nbsp,
'the NBSP line must be returned (and fail the format gate), never skipped past');
});
test('MAJOR 3: a TRUNCATED capture is not resolved at all', () => {
// The `-m` capture stops at the first `"`, so a message containing one
// arrives here without its tail — and without its terminator. Resolving
// anyway hands the length gate a PREFIX of the real subject and lets an
// over-long message through: an enforcement hole that did not exist before
// this fix. Falling back to the opener fails the format gate, which is what
// this whole form did before the fix (review of #3802).
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: aaaa"), "$(cat <<'EOF'",
'the subject line runs to the end of a truncated capture, so it cannot be measured');
// Truncation is only fatal to the line it lands IN: a captured line is
// complete exactly when another line follows it. A quote further down the
// BODY leaves the subject intact and measurable, so blocking it would be a
// false positive the blunt version of this guard would have introduced.
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: short\nbody with a "), 'feat: short',
'a complete subject line stays measurable even when the capture truncates later');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'"), "$(cat <<'EOF'",
'an opener with no body at all is likewise unresolvable');
});
test('BLOCKER (round 3): text after the terminator is part of the real message', () => {
// `-m "$(cat <<'EOF'\nfeat: ok\nEOF\n) <200 a's>"` expands to ONE long
// subject; discarding the tail measured a PREFIX (8 chars vs 200+) and
// dodged COMMIT_SUBJECT_TOO_LONG — the truncation-guard class from the
// other side of the terminator (review of #3816, round 3). Only the
// canonical single closing-paren line may follow the terminator; anything
// else falls back to the opener and the format gate.
assert.strictEqual(resolveCommitSubject(`$(cat <<'EOF'\nfeat: ok\nEOF\n) ${'a'.repeat(200)}`),
"$(cat <<'EOF'", 'a substitution composed with more text cannot have its body trusted');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: ok\nEOF\n)$(printf x)"),
"$(cat <<'EOF'", 'a second substitution after the close is the same composition');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nEOF\n)feat: sneaky"),
"$(cat <<'EOF'", 'text glued straight onto the closing paren too');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: ok\nEOF"),
"$(cat <<'EOF'", 'a terminator with NO closing line at all is not the canonical shape either');
// the canonical tail still resolves — including an indented or space-padded close
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: ok\nEOF\n)"), 'feat: ok');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\nfeat: ok\nEOF\n\t) "), 'feat: ok');
});
test('MINOR 1: leading blank body lines are skipped, as git does', () => {
// git's default cleanup=whitespace strips leading blank lines, so the real
// subject is the first NON-empty line. Taking lines[1] blindly returned ''
// and falsely blocked a conforming commit — the defect class #3802 reports.
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\n\nfeat: after blank\nEOF\n)"),
'feat: after blank');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\n\n\n \nfeat: after several\nEOF\n)"),
'feat: after several');
});
test('a backslash-escaped delimiter is the same delimiter', () => {
// `<<\\D` suppresses expansion exactly as `<<'D'` does, so it stays
// resolvable. This is the row that makes the bare-delimiter guard below a
// real distinction rather than a blanket refusal: the two spellings differ
// by one character and by whether bash expands the body.
assert.strictEqual(resolveCommitSubject('$(cat <<\\EOF\nfix: backslash tag\nEOF\n)'),
'fix: backslash tag');
});
test('BLOCKER (round 4): a BARE delimiter is not resolved — bash expands that body', () => {
// Review of #3816, round 4. Only `<<'D'`, `<<"D"` and `<<\\D` suppress
// expansion. With a bare `<<D` bash substitutes `$var`, `$(...)` and
// arithmetic into the body BEFORE git sees it, so the literal text here is
// not the subject git receives — resolving it dodged the format gate
// (`feat: $UNSET_VAR` -> git gets `feat:`) and the length gate
// (`feat: ${LONG}` -> git gets any length). Falling back to the opener line
// is the same fail-closed rule the metacharacter, truncation and
// post-terminator guards follow.
//
// Non-vacuous: every body below is conforming, so a resolver that still
// read the body returns the body and these fail.
assert.strictEqual(resolveCommitSubject(sub('<<EOF', 'fix: bare', 'EOF')),
'$(cat <<EOF', 'a bare delimiter must fall back to the opener line');
assert.strictEqual(resolveCommitSubject(sub('<< EOF', 'fix: spaced', 'EOF')),
'$(cat << EOF', 'a spaced bare delimiter is still bare');
assert.strictEqual(resolveCommitSubject(sub('<<-EOF', '\tfix: dashed bare', 'EOF')),
'$(cat <<-EOF', '<<- does not quote the delimiter; it only strips tabs');
// The expansion that makes this a bypass rather than a nicety.
assert.strictEqual(resolveCommitSubject(sub('<<EOF', 'feat: $UNSET_VAR', 'EOF')),
'$(cat <<EOF', "git's real subject here is `feat:` — never the unexpanded literal");
});
// RULESET.TESTS.property-based-testing — this is a parser/transformation on a
// hook path, so the invariants are asserted over generated input rather than
// examples alone. Seeded setup helper, not `fast-check` directly, so numRuns
// and seed are configured before any fc.assert (repo convention).
//
// Review of #3816, Major 1: the previous generator was a bare
// fc.string({maxLength: 400}), whose pinned-seed corpus contained NO newline
// and NO opener — 0 of 200 inputs reached the parser, so all three properties
// reduced to `f(s) === s`. The generator now CONSTRUCTS heredoc-shaped input
// (every opener spelling, <<- tabs, optional terminator, CRLF) alongside plain
// and multi-line strings, and each property PROVES its corpus took the heredoc
// arm: `resolved` counts inputs whose output is not the first line, which only
// the resolver's body-scanning branch can produce.
const delimiterArb = fc.stringMatching(/^[A-Za-z][A-Za-z0-9_-]{0,8}$/);
const bodyLineArb = fc.stringMatching(/^[^\n\r]{0,60}$/);
const heredocArb = fc.record({
delim: delimiterArb,
quote: fc.constantFrom("'", '"', '', '\\'),
dash: fc.boolean(),
spaced: fc.boolean(),
catPath: fc.constantFrom('cat', '/bin/cat'),
body: fc.array(bodyLineArb, { minLength: 0, maxLength: 5 }),
terminated: fc.boolean(),
eol: fc.constantFrom('\n', '\r\n'),
}).map(({ delim, quote, dash, spaced, catPath, body, terminated, eol }) => {
const word = quote === '\\' ? `\\${delim}` : quote ? `${quote}${delim}${quote}` : delim;
const opener = `$(${catPath} <<${dash ? '-' : ''}${spaced ? ' ' : ''}${word}`;
const emitted = [...body.map((l) => (dash ? `\t${l}` : l))];
if (terminated) emitted.push(dash ? `\t${delim}` : delim, ')');
// GENERATION-TIME oracle for the one result the derivation check cannot
// classify by membership: ''. Computed from what the generator KNOWS it
// built — never by re-running resolver logic — so a resolver degrading to
// '' anywhere it should not fails the property (Codex review of #3816,
// rounds 1+2). '' is legitimate exactly when the FIRST reachable
// terminator is followed by the one canonical closing-paren line (the
// round-3 post-terminator guard: any other tail must fall back to the
// opener, never to '') and every scanned line before that terminator is
// ASCII-blank. A body line that reads as the delimiter after <<- tab
// stripping terminates early, and whatever follows it is its tail.
const seen = emitted.map((l) => (dash ? l.replace(/^\t+/, '') : l));
const stop = seen.indexOf(delim);
const tail = stop === -1 ? null : seen.slice(stop + 1);
const canonicalTail = tail !== null && tail.length === 1 && /^[ \t]*\)[ \t]*$/.test(tail[0]);
const expectEmpty = canonicalTail
&& seen.slice(0, stop).every((l) => /^[ \t]*$/.test(l));
return { text: [opener, ...emitted].join(eol), expectEmpty };
});
const messageArb = fc.oneof(
{ weight: 3, arbitrary: heredocArb },
// plain single- and multi-line messages: the subject is the first line
// verbatim, so '' is legitimate only when the first line IS ''.
fc.string({ maxLength: 400 }).map((s) => ({ text: s, expectEmpty: s.split(/\r?\n/)[0] === '' })),
// multi-line plain messages — the old generator never produced a newline
fc.array(bodyLineArb, { minLength: 1, maxLength: 4 })
.map((ls) => ({ text: ls.join('\n'), expectEmpty: ls[0] === '' })),
);
const firstLineOf = (input) => String(input).split(/\r?\n/)[0];
// Floor for the resolved-input count across the seeded corpus. Deliberately
// far below the ~60% heredoc weighting so generator drift cannot flake it,
// while still failing loudly if the corpus stops reaching the parser — the
// exact vacuity Major 1 caught.
const MIN_RESOLVED = 20;
test('property: total — never throws, always returns a string', () => {
// Totality is a SECURITY property here, not tidiness: this runs inside a
// PreToolUse hook whose caller treats a failed extraction as "nothing to
// validate", so an exception fails OPEN. Backed by a corpus that reaches
// the parser, which is what makes the claim about the PARSER and not about
// fc.string pass-through.
let resolved = 0;
fc.assert(fc.property(messageArb, (m) => {
const out = resolveCommitSubject(m.text);
assert.strictEqual(typeof out, 'string');
if (out !== firstLineOf(m.text)) resolved += 1;
}));
assert.ok(resolved >= MIN_RESOLVED,
`only ${resolved} corpus inputs were actually resolved past the first line — the property is `
+ 'running on inputs that never reach the parser again (review of #3816, Major 1)');
for (const odd of [null, undefined, '', '\n', '\n\n\n', '\r\n', '$(cat <<', '$(cat <<-']) {
assert.strictEqual(typeof resolveCommitSubject(odd), 'string', JSON.stringify(odd));
}
});
test('property: idempotent — resolving a resolved subject changes nothing', () => {
let resolved = 0;
fc.assert(fc.property(messageArb, (m) => {
const once = resolveCommitSubject(m.text);
assert.strictEqual(resolveCommitSubject(once), once);
if (once !== firstLineOf(m.text)) resolved += 1;
}));
assert.ok(resolved >= MIN_RESOLVED,
`only ${resolved} corpus inputs were actually resolved — vacuous corpus (review of #3816)`);
});
test('property: the result is a single line derived from an input line by git\'s own strips', () => {
// The subject is a LINE, never a synthesised string: whatever comes back
// must be one of the input's own lines, modulo exactly the transformations
// git itself performs — `<<-` leading-tab stripping and cleanup=whitespace
// trailing-whitespace stripping. A resolver that concatenated lines or
// trimmed anything MORE than that would fail this. The one result
// membership cannot classify — '' — is judged by the GENERATOR's own
// metadata (`expectEmpty`, computed from what it built, not from resolver
// logic), so a resolver conditionally degrading to '' fails loudly (Codex
// review of #3816, rounds 1+2).
let resolved = 0;
fc.assert(fc.property(messageArb, (m) => {
const out = resolveCommitSubject(m.text);
assert.ok(!/[\n\r]/.test(out), 'a subject is one line');
const lines = String(m.text).split(/\r?\n/);
const derivations = (l) => {
const untabbed = l.replace(/^\t+/, '');
return [l, untabbed, untabbed.replace(/[ \t]+$/, '')];
};
if (out === '') {
assert.ok(m.expectEmpty,
`resolved to '' for an input the generator did NOT build as an empty message: `
+ JSON.stringify(m.text));
} else {
assert.ok(lines.some((l) => derivations(l).includes(out)),
`result ${JSON.stringify(out)} is not derived from any line of the input`);
}
if (out !== firstLineOf(m.text)) resolved += 1;
}));
assert.ok(resolved >= MIN_RESOLVED,
`only ${resolved} corpus inputs were actually resolved — vacuous corpus (review of #3816)`);
});
test('MAJOR 2: trailing whitespace is stripped, as git cleanup=whitespace does', () => {
// git strips whitespace at BOTH ends of the line, not just leading blank
// lines. Measuring the raw line rejected a body of `feat: ` + 66 x's + three
// spaces as 75 chars when git's actual subject is a conforming 72 — a
// still-blocked conforming commit, the defect #3802 reports (review of #3816).
const subject72 = `feat: ${'x'.repeat(66)}`;
assert.strictEqual(subject72.length, 72, 'fixture built wrong');
assert.strictEqual(resolveCommitSubject(sub("<<'EOF'", `${subject72} `, 'EOF')), subject72);
assert.strictEqual(resolveCommitSubject(sub("<<'EOF'", 'feat: tab tail\t \t', 'EOF')),
'feat: tab tail', 'tabs are trailing whitespace too');
});
test('MINOR 3: CRLF bodies resolve identically to LF bodies', () => {
// split('\n') left \r on every body line, so the delimiter never matched on
// CRLF input: the truncation guard was inert, an empty CRLF message resolved
// to "EOF\r" instead of '', and a real 72-char subject measured 73
// (review of #3816).
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\r\nfeat: crlf subject\r\nEOF\r\n)"),
'feat: crlf subject');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\r\nEOF\r\n)"), '',
'an empty CRLF message is EMPTY — it used to resolve to the terminator plus \\r');
assert.strictEqual(resolveCommitSubject("$(cat <<'EOF'\r\nfeat: aaaa"), "$(cat <<'EOF'",
'the truncation guard must be live on CRLF input, not defeated by an unmatchable delimiter');
});
test('ordinary messages pass through as their first line', () => {
assert.strictEqual(resolveCommitSubject('feat(auth): add login flow'), 'feat(auth): add login flow');
assert.strictEqual(resolveCommitSubject('feat: subject\n\nBody paragraph.'), 'feat: subject');
assert.strictEqual(resolveCommitSubject(''), '');
assert.strictEqual(resolveCommitSubject(null), '');
assert.strictEqual(resolveCommitSubject(undefined), '');
});
});
// ── extractBranchArgument (#3212 Phase 3, #3414) ─────────────────────────────
// New capability on the shared scanner (design doc §1.2) — not a migration of
// existing duplicated logic; no existing consumer wired to it this phase.