fix(#4492): index the suffix window instead of pattern-matching it (#4539)

* fix(#4492): index the suffix window instead of pattern-matching it

`MSG_SUFFIX="${CMD#*"$MSG_MATCH"}"` is quadratic in the -m message. bash tries
every prefix length and compares the whole matched literal at each, and
MSG_MATCH is BASH_REMATCH[0] — the entire `-m "..."` — so the cost grows with
the thing being scanned. Measured on the real hook: 10.0s at 64KB, 22.0s at
96KB, 30.2s at 112KB, 40.1s at 128KB. `bash -x` with an EPOCHREALTIME PS4
attributes 10.116s of a 10.2s run to that one expansion, which computes an
empty string. Conforming and non-conforming cost the same, so this is the path
every commit takes, and Claude Code blocks on PreToolUse hooks.

MSG_PREFIX on the line above has already located the match, so the suffix is
arithmetic rather than a search. Same first-occurrence assumption both
expansions always made — MSG_MATCH is a literal substring of CMD by
construction. Equivalence checked across 480 comparisons on bash 3.2.57 and
5.3.15 under C, UTF-8 and SJIS locales, including multibyte text, repeated
matches, metacharacters and invalid bytes.

Three regression rows, all deliberately on the RESOLVE=1 path so they pin the
suffix scan alone and do not depend on the separate #4429 SIGPIPE fix:
non-conforming and conforming 112KB heredocs, plus a suffix-window row whose
padding sits before the heredoc opener's newline so the COMMAND is large while
the message stays small. Red against the true base — all three killed at the
10s bound with the head -1 sites still present — and green with only this
change.

Fixture sizes stay under Linux MAX_ARG_STRLEN (131072 on a 4KB-page kernel).
Above it execve fails, the classifier cannot launch and the hook fails open, so
a larger fixture measures the argument limit rather than the suffix scan; an
earlier 131225-byte draft passed on base AND head for exactly that reason.
Every row asserts empty stderr, which is what separates "validated" from
"failed open".

The bound is enforced by killing the process GROUP, not the direct child: the
hook spawns a node classifier that inherits stdout, so killing only bash can
leave the pipe open and `close` never arrives. `local/no-elapsed-assertion`
forbids asserting on elapsed time, and `{ timeout }` is inert on a synchronous
body, so the rows are async and the kill is the signal.

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

* chore(#4492): add changeset

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>

.changeset/gentle-dogs-climb.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 4539
+---
+**`git commit` with a large `-m` message is no longer slow** — the commit-message validator hook computed the text after the message with a pattern match that is quadratic in the message length, on the path every commit takes and before the pass/fail branch, so conforming and non-conforming messages cost the same: 10.0s at a 64KB message, 30.2s at 112KB. Claude Code blocks on PreToolUse hooks, so that was dead time in front of the user. The suffix is now derived by arithmetic from the match already located on the preceding line — byte-identical output, flat 0.2s at every size measured. (#4492)

hooks/gsd-validate-commit.sh

@@ -218,7 +218,24 @@ if [ "$CLASSIFY_STATUS" = "0" ]; then
       # 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"}"
+      # Indexed, not searched (#4492). `${CMD#*"$MSG_MATCH"}` is quadratic in
+      # the message: bash walks every prefix length and compares the whole
+      # matched literal at each one, and MSG_MATCH is BASH_REMATCH[0] — the
+      # entire `-m "..."` — so the cost grows with the thing being scanned.
+      # Measured on the path EVERY commit takes (conforming and non-conforming
+      # cost the same): 10.0 s at a 64 KB message, 22.0 s at 96 KB, 30.2 s at
+      # 112 KB. Sizes stop there deliberately — a single argument above Linux's
+      # MAX_ARG_STRLEN (131072 on a 4 KB-page kernel) never reaches this code
+      # at all, because execve fails and the hook fails open, so a larger
+      # "measurement" would be timing the wrong thing.
+      #
+      # MSG_PREFIX above has already located the match, so the suffix is
+      # arithmetic rather than a search: skip the prefix and the match. This
+      # removes the quadratic SEARCH; the expansion still counts characters and
+      # materialises a substring, so it is linear in the command, not O(1).
+      # Same first-occurrence assumption both expansions here always made —
+      # MSG_MATCH is a literal substring of CMD by construction.
+      MSG_SUFFIX="${CMD:$(( ${#MSG_PREFIX} + ${#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

tests/hooks-opt-in.test.cjs

@@ -20,6 +20,7 @@ const assert = require('node:assert/strict');
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
+const { spawn } = require('child_process');
 const { runHook } = require('./helpers/process-seam.cjs');
 const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
 
@@ -1716,3 +1717,115 @@ describe('hook security tests', { skip: isWindows ? 'bash hooks require unix she
     assert.strictEqual(result.status, 0, `Malformed config should be treated as disabled: ${result.status}`);
   });
 });
+
+
+// ─── #4492: a large `-m` message must not cost quadratic time ───────────────
+
+describe('validate-commit: a large -m message is validated in bounded time (#4492)',
+  { skip: isWindows ? 'bash hooks require unix shell' : false }, () => {
+    let tmpDir;
+    beforeEach(() => { tmpDir = createTempProject(); writeConfigWithHooks(tmpDir, true); });
+    afterEach(() => { cleanup(tmpDir); });
+
+    const HOOK = path.join(HOOKS_DIR, 'gsd-validate-commit.sh');
+
+    // Local, not in tests/helpers/timeouts.cjs: that module is for shared norms
+    // and says a call site with a differing bound keeps its own constant and
+    // justification. This is one regression's defect-detection threshold, not a
+    // norm. The quadratic `${CMD#*"$MSG_MATCH"}` this replaced costs 30.2s on
+    // the 112KB rows below while the indexed form costs 0.2s, so 10000ms sits
+    // 3x under the regressed cost and 50x over the fixed one.
+    const LARGE_MESSAGE_BOUND_MS = 10000;
+
+    // Async, bounded, and it reaps the whole process GROUP. Four constraints,
+    // each learned by measurement rather than reasoning:
+    //
+    //   1. `assert.ok(elapsed < BOUND)` is banned by `local/no-elapsed-assertion`.
+    //   2. `{ timeout }` on a SYNCHRONOUS body is inert — spawnSync blocks the
+    //      event loop so the runner's timer never fires. A draft of these rows
+    //      was sync with `{ timeout: 20000 }` and PASSED at 38.5s unfixed.
+    //   3. Killing only the direct child is not enough: the hook spawns a node
+    //      classifier that inherits stdout, so if IT stalls the pipe stays open
+    //      and `close` never arrives even after bash dies. Hence `detached` plus
+    //      a process-group kill.
+    //   4. Sizes must stay under Linux's MAX_ARG_STRLEN (131072 on a 4KB-page
+    //      kernel). Over it, execve fails, the classifier cannot launch, and the
+    //      hook fails open — the row then measures the argument limit instead of
+    //      the suffix scan.
+    //
+    // The bound is enforced by killing, which turns "too slow" into an ordinary
+    // observable (`status === null`, `signal === 'SIGKILL'`) a normal assertion
+    // reads.
+    const runCmdAsync = (command) => new Promise((resolve, reject) => {
+      const child = spawn('bash', [HOOK], { cwd: tmpDir, env: hookEnv, detached: true });
+      let stdout = '', stderr = '', settled = false;
+      const killer = setTimeout(() => {
+        try { process.kill(-child.pid, 'SIGKILL'); } catch { try { child.kill('SIGKILL'); } catch { /* already gone */ } }
+      }, LARGE_MESSAGE_BOUND_MS);
+      const settle = (fn, v) => { if (!settled) { settled = true; clearTimeout(killer); fn(v); } };
+      child.on('error', (err) => settle(reject, err));
+      child.stdin.on('error', () => { /* hook exited before reading stdin; `close` carries the verdict */ });
+      child.stdout.on('data', (d) => { stdout += d; });
+      child.stderr.on('data', (d) => { stderr += d; });
+      child.on('close', (status, signal) => settle(resolve, { status, stdout, stderr, signal }));
+      child.stdin.end(JSON.stringify({ tool_input: { command } }));
+    });
+
+    const bodyOf = (kb) => Array.from({ length: Math.ceil((kb * 1024) / 63) }, () => 'x'.repeat(62)).join('\n');
+    const heredocArg = (body) => `"$(cat <<'EOF'\n${body}\nEOF\n)"`;
+
+    // Every row asserts this. An empty stderr separates "the hook validated"
+    // from "the hook fell over and failed open" — the same status to a caller.
+    const assertValidated = (r, what) => {
+      assert.notStrictEqual(r.signal, 'SIGKILL',
+        `${what}: hook was killed at the ${LARGE_MESSAGE_BOUND_MS}ms bound — the quadratic ` +
+        'suffix scan is back (unfixed cost on this row is ~30s)');
+      assert.strictEqual(r.stderr, '',
+        `${what}: hook wrote to stderr, so it degraded instead of validating: ${r.stderr.slice(0, 200)}`);
+    };
+
+    // Deliberately a NON-conforming subject on the heredoc path. That keeps the
+    // row on RESOLVE=1, where the subject comes from the node resolver rather
+    // than the `... | head -1` fallback — so this pins the suffix scan ALONE and
+    // does not depend on the separate #4429 SIGPIPE fix. (An earlier draft used
+    // a trailing `--cleanup=verbatim`, which sets RESOLVE=0 and made the row
+    // fail with 141 until #4429 was also fixed; that coupling was a property of
+    // the fixture, not of the fix.)
+    test('a 112KB message is blocked on its subject, not stalled by the scan', async () => {
+      const r = await runCmdAsync(`git commit --allow-empty -m ${heredocArg(`WIP invalid\n${bodyOf(112)}`)}`);
+      assertValidated(r, '112KB non-conforming');
+      assert.strictEqual(r.status, 2,
+        `a non-conforming subject must still be refused no matter how long the body is; got ${r.status}`);
+    });
+
+    test('a 112KB conforming message is allowed, not stalled by the scan', async () => {
+      const r = await runCmdAsync(`git commit --allow-empty -m ${heredocArg(`feat: ${'x'.repeat(66)}\n${bodyOf(112)}`)}`);
+      assertValidated(r, '112KB conforming');
+      assert.strictEqual(r.status, 0,
+        `a conforming subject stays conforming no matter how long the body is; got ${r.status}`);
+    });
+
+    // The correctness half: the row that would catch an off-by-one in the index
+    // arithmetic that replaced the pattern match. The padding sits BEFORE the
+    // heredoc opener's newline, so the command is large while the MESSAGE stays
+    // tiny — which keeps this off the RESOLVE=0 fallback too. `--cleanup=verbatim`
+    // is reachable only through MSG_SUFFIX: without it the command is allowed,
+    // with it the subject is refused. A suffix computed one byte wrong drops the
+    // option and returns 0 — the ACCEPT direction, which is the dangerous one.
+    test('the suffix window still sees an option after a 112KB command', async () => {
+      const pad = ' '.repeat(112 * 1024);
+      const base = `git commit -m "$(cat${pad} <<'EOF'\nfeat: x\nEOF\n)"`;
+
+      const control = await runCmdAsync(base);
+      assertValidated(control, 'control');
+      assert.strictEqual(control.status, 0,
+        'control: with no trailing option the command is allowed — without this the ' +
+        'assertion below could pass on a hook that blocks everything');
+
+      const withOption = await runCmdAsync(`${base} --cleanup=verbatim`);
+      assertValidated(withOption, 'with trailing option');
+      assert.strictEqual(withOption.status, 2,
+        'a trailing --cleanup=verbatim is reachable only through the suffix window, so ' +
+        'blocking here proves the window survived the command length');
+    });
+  });