Merge pull request #4560 from open-gsd/test/4513-batch2-git-plumbing

2026-09-09 06:53:14 -04:00
parent 137f3115a1 342bd8ca6b
commit fd37b6a171
7 changed files with 89 additions and 33 deletions
--- a/.changeset/zesty-seals-click.md
+++ b/.changeset/zesty-seals-click.md
@@ -0,0 +1,5 @@
+---
+type: Security
+pr: 4560
+---
+**Pinned the transitive `hono` dependency to `>=4.13.5`** — fixes a moderate-severity path-traversal/DoS advisory chain (GHSA-gqvv-2mrq-wpjv, GHSA-g6gw-c38x-mqfc, GHSA-crvj-82cr-hjcx) in `hono <4.13.5`, pulled in transitively via `@anthropic-ai/claude-agent-sdk` -> `@modelcontextprotocol/sdk`. Discovered as a newly-published advisory blocking `tests/npm-integrity-gate.test.cjs` while verifying an unrelated PR; fixed inline per this repo's no-defer policy rather than left for a separate PR. (#4513)
--- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json
+++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json
@@ -26,7 +26,6 @@
  "tests/code-review-pipeline-regression.test.cjs",
  "tests/code-review.test.cjs",
  "tests/commands.test.cjs",
-  "tests/commit-files-pathspec.test.cjs",
  "tests/config-get-default.test.cjs",
  "tests/cursor-hook-workspace-roots.test.cjs",
  "tests/cursor-hooks.test.cjs",
@@ -46,8 +45,6 @@
  "tests/gen-health-docs.test.cjs",
  "tests/gen-section-manifest.test.cjs",
  "tests/gen-state-md-docs.test.cjs",
-  "tests/git-base-branch.test.cjs",
-  "tests/git-fixture.test.cjs",
  "tests/graphify.test.cjs",
  "tests/gsd-check-update-worker-platform-gate.test.cjs",
  "tests/gsd-mcp-server-bin.test.cjs",
--- a/package-lock.json
+++ b/package-lock.json
@@ -3615,9 +3615,9 @@
      }
    },
    "node_modules/hono": {
-      "version": "4.13.0",
-      "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.0.tgz",
-      "integrity": "sha512-jhunvfHWxd7J5EFfSgH4xsYJzSe/lfqbUCxiyyeaQasUsXeEHXtzVid+7EOGByc5JnFa23SSFL3Y2RV/z1T+eQ==",
+      "version": "4.13.7",
+      "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.7.tgz",
+      "integrity": "sha512-c8/gF9ac8Y78/agExVocyLevgR+JlpNB444Py0FSX8pJoPdYUfUzRcXtYEYGwt6l19qIlVZPN5Mfsw9jFShmQQ==",
      "license": "MIT",
      "engines": {
        "node": ">=16.9.0"
--- a/package.json
+++ b/package.json
@@ -85,7 +85,8 @@
  "overrides": {
    "qs": ">=6.15.2",
    "body-parser": ">=2.3.0",
-    "@hono/node-server": ">=2.0.5"
+    "@hono/node-server": ">=2.0.5",
+    "hono": ">=4.13.5"
  },
  "optionalDependencies": {
    "fallow": "^2.70.0"
--- a/tests/commit-files-pathspec.test.cjs
+++ b/tests/commit-files-pathspec.test.cjs
@@ -22,7 +22,7 @@ const { createTempGitProject, cleanup, runGsdTools } = require('./helpers.cjs');
 const { execFileSync } = require('node:child_process');
 const { gitOrThrow } = require('./helpers/git-fixture.cjs');
 // #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs.
-const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
+const { GIT_TIMEOUT_MS, PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
 const {
  bareCommandName, tokenize, shellDashCPayloads, commentPortion,
  ISSUE_REF_RE, declarationReason, isDeclared, isUntrackedDeclaration,
@@ -433,6 +433,16 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
 // shared DEFAULT_GIT_TIMEOUT_MS norm.
 const STAGING_GIT_TIMEOUT_MS = 5000;

+/**
+ * `commitWithFailingAdd`/`subrepoCommitWithFailingAdd` below spawn
+ * `process.execPath -e <script>` where the script mocks `execGit` in-process
+ * (no real git subprocess runs) and calls `cmdCommit`/`cmdCommitToSubrepo`.
+ * Not the same class as `PROBE_TIMEOUT_MS` (15000ms) below — kept at its
+ * pre-existing, more generous value rather than lowered without a bench
+ * citation proving 15000ms is safe for this harness.
+ */
+const MOCKED_GIT_SPAWN_TIMEOUT_MS = 30000;
+
 const LIB = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib');

 /**
@@ -488,7 +498,7 @@ cmdCommit(${JSON.stringify(cwd)}, 'docs: map existing codebase', ${JSON.stringif

  const run = spawnSync(process.execPath, ['-e', script], {
    encoding: 'utf8',
-    timeout: 30000,
+    timeout: MOCKED_GIT_SPAWN_TIMEOUT_MS,
    killSignal: 'SIGKILL',
    env: { ...process.env, GSD_TEST_MODE: '1' },
  });
@@ -548,7 +558,7 @@ cmdCommitToSubrepo(${JSON.stringify(cwd)}, 'feat: subrepo change', ${JSON.string
 `;
  const run = spawnSync(process.execPath, ['-e', script], {
    encoding: 'utf8',
-    timeout: 30000,
+    timeout: MOCKED_GIT_SPAWN_TIMEOUT_MS,
    killSignal: 'SIGKILL',
    env: { ...process.env, GSD_TEST_MODE: '1' },
  });
@@ -3195,7 +3205,7 @@ projection.execGit = (args, opts) => {
 };
 cmdCommit(${JSON.stringify(cwd)}, 'docs: probe', ${JSON.stringify(files)}, false, false, false);
 `;
-    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
+    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
    if (run.status !== 0 && !run.stdout) {
      throw new Error(`probe crashed: ${run.stderr}`);
    }
@@ -3207,9 +3217,16 @@ cmdCommit(${JSON.stringify(cwd)}, 'docs: probe', ${JSON.stringify(files)}, false
    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3886-'));
    fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true });
    fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
-    execFileSync('git', ['init', '-b', 'main'], { cwd: tmpDir, timeout: 15_000 });
-    execFileSync('git', ['config', 'user.email', 't@example.com'], { cwd: tmpDir, timeout: 15_000 });
-    execFileSync('git', ['config', 'user.name', 'T'], { cwd: tmpDir, timeout: 15_000 });
+    // `GIT_TIMEOUT_MS` (15000ms, "plumbing READS") rather than the heavier
+    // `GIT_FIXTURE_TIMEOUT_MS` (60000ms, "fixture CONSTRUCTION calls —
+    // init/config/add/commit") that helpers/git-fixture.cjs's own docstring
+    // says this exact call shape belongs to. Not reclassified: doing so would
+    // RAISE this site's bound 15000ms -> 60000ms with no bench citation
+    // proving the smaller value is unsafe here — out of scope for a
+    // rename-only migration. Flagged, not silently resolved either way.
+    execFileSync('git', ['init', '-b', 'main'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
+    execFileSync('git', ['config', 'user.email', 't@example.com'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
+    execFileSync('git', ['config', 'user.name', 'T'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
  });
  afterEach(() => cleanup(tmpDir));

@@ -3247,7 +3264,7 @@ projection.execGit = (args, opts) => {
 };
 cmdCommit(${JSON.stringify(tmpDir)}, 'docs: probe', ['.planning/STATE.md'], false, false, false);
 `;
-    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
+    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
    const result = JSON.parse(run.stdout);
    assert.equal(result.reason, 'commit_timeout', 'the timeout gate must win over the nothing-to-commit text in partial output');
    assert.equal(result.timed_out, true);
@@ -3268,7 +3285,7 @@ projection.execGit = (args, opts) => {
 };
 cmdCommit(${JSON.stringify(tmpDir)}, 'docs: probe', ['.planning/STATE.md'], false, false, false);
 `;
-    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
+    const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
    const result = JSON.parse(run.stdout);
    assert.equal(result.committed, false);
    assert.equal(result.reason, 'commit_failed');
--- a/tests/git-base-branch.test.cjs
+++ b/tests/git-base-branch.test.cjs
@@ -38,6 +38,33 @@ const { runHook } = require('./helpers/process-seam.cjs');
 // #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs.
 const { GIT_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');

+/**
+ * The #3819 executor pre-commit protected-branch guard test harness below
+ * invokes a bash script with fake `git`/`gsd_run` shell functions defined
+ * INLINE — no real subprocess fan-out happens, so this is lighter than
+ * `HOOK_FANOUT_TIMEOUT_MS` (60000ms, sized for a hook that fans out to
+ * nested shell spawns). Pre-existing value, unchanged by this migration.
+ */
+const EXECUTOR_GUARD_TIMEOUT_MS = 10000;
+
+/**
+ * These four constants are NOT bounds on this suite's own subprocess calls —
+ * they are the EXACT timeout values production code (gitBaseBranch's
+ * trySymbolicRef / tryRemoteShow / tryLocalBranch / gitWorktreeInfoInternal,
+ * in src/, out of this batch's scope) hardcodes when calling its own git
+ * execution callback. Each test below asserts the captured call args to
+ * prove production didn't drift from its documented bound. Kept as FOUR
+ * separate constants rather than consolidated, even where values coincide
+ * today (TIER2 and TIER4 are both 5000ms) — collapsing them would let two
+ * independently-implemented production tiers drift from each other while
+ * the test kept passing, which defeats the point of a pinned-value
+ * assertion. All four are pre-existing values, unchanged by this migration.
+ */
+const TIER2_SYMBOLIC_REF_TIMEOUT_MS = 5000;
+const TIER3_REMOTE_SHOW_TIMEOUT_MS = 15000;
+const TIER4_LOCAL_BRANCH_TIMEOUT_MS = 5000;
+const WORKTREE_INFO_TIMEOUT_MS = 5000;
+
 // ─── helpers ──────────────────────────────────────────────────────────────────

 /**
@@ -1267,7 +1294,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /protected\/default branch/);
@@ -1283,7 +1310,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 0, result.stderr);
    assert.match(result.stdout, /GUARD_PASSED/);
@@ -1298,7 +1325,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /protected\/default branch/);
@@ -1313,7 +1340,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /not in the agent-\* \/ worktree-agent-\* \/ worktree-wf_\* namespace/);
@@ -1328,7 +1355,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 0, result.stderr);
    assert.match(result.stdout, /GUARD_PASSED/);
@@ -1343,7 +1370,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /detached/);
@@ -1374,7 +1401,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /protected\/default branch/);
@@ -1391,7 +1418,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 1, result.stderr);
    assert.match(result.stderr, /protected\/default branch/);
@@ -1412,7 +1439,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
    });
    t.after(() => cleanup(scriptDir));

-    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
+    const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });

    assert.strictEqual(result.exitCode, 0, result.stderr);
    assert.match(result.stdout, /GUARD_PASSED/);
@@ -1896,7 +1923,7 @@ describe('#3057 W3: trySymbolicRef — tier-2 output that resolves to nothing',
    });
    assert.deepStrictEqual(seen, [{
      args: ['symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD'],
-      opts: { cwd: '/some/cwd', timeout: 5_000 },
+      opts: { cwd: '/some/cwd', timeout: TIER2_SYMBOLIC_REF_TIMEOUT_MS },
    }]);
  });
 });
@@ -1953,7 +1980,7 @@ describe('#3057 W3: tryRemoteShow — tier-3 output that is present but not auth
    });
    assert.deepStrictEqual(seen, [{
      args: ['remote', 'show', 'origin'],
-      opts: { cwd: '/some/cwd', timeout: 15_000 },
+      opts: { cwd: '/some/cwd', timeout: TIER3_REMOTE_SHOW_TIMEOUT_MS },
    }]);
  });
 });
@@ -2006,7 +2033,7 @@ describe('#3057 W3: tryLocalBranch — non-empty stdout that names neither main
    });
    assert.deepStrictEqual(seen, [{
      args: ['branch', '--list', 'main', 'master'],
-      opts: { cwd: '/some/cwd', timeout: 5_000 },
+      opts: { cwd: '/some/cwd', timeout: TIER4_LOCAL_BRANCH_TIMEOUT_MS },
    }]);
  });
 });
@@ -2184,8 +2211,8 @@ describe('#3057 W3: gitWorktreeInfoInternal — no work tree, and git failing mi
    });
    gitBaseBranch.gitWorktreeInfoInternal('/some/cwd', { execGit: git });
    assert.deepStrictEqual(git.calls, [
-      { args: ['rev-parse', '--is-inside-work-tree'], opts: { cwd: '/some/cwd', timeout: 5000 } },
-      { args: ['rev-parse', '--show-toplevel'], opts: { cwd: '/some/cwd', timeout: 5000 } },
+      { args: ['rev-parse', '--is-inside-work-tree'], opts: { cwd: '/some/cwd', timeout: WORKTREE_INFO_TIMEOUT_MS } },
+      { args: ['rev-parse', '--show-toplevel'], opts: { cwd: '/some/cwd', timeout: WORKTREE_INFO_TIMEOUT_MS } },
    ]);
  });
 });
--- a/tests/git-fixture.test.cjs
+++ b/tests/git-fixture.test.cjs
@@ -31,6 +31,15 @@ const runGitSpy = mock.method(processSeam, 'runGit');
 after(() => mock.restoreAll());

 const { gitOrThrow, throwIfFailed, toLegacyResult, DEFAULT_GIT_TIMEOUT_MS } = require('./helpers/git-fixture.cjs');
+
+/**
+ * Deliberately NOT DEFAULT_GIT_TIMEOUT_MS or any real class norm — chosen
+ * specifically to differ from it, to prove gitOrThrow's timeoutMs override
+ * plumbing actually threads a caller-supplied value through rather than
+ * silently keeping the default. See the E11 test's own
+ * assert.notEqual(..., DEFAULT_GIT_TIMEOUT_MS) a few lines below.
+ */
+const ARBITRARY_OVERRIDE_TIMEOUT_MS = 12345;
 const { createTempDir, cleanup } = require('./helpers.cjs');

 /**
@@ -161,10 +170,10 @@ describe('git-fixture: E — gitOrThrow', () => {

  test('E11: explicit timeoutMs overrides the default', () => {
    runGitSpy.mock.resetCalls();
-    gitOrThrow(['--version'], { cwd: dir, timeoutMs: 12345 });
+    gitOrThrow(['--version'], { cwd: dir, timeoutMs: ARBITRARY_OVERRIDE_TIMEOUT_MS });
    assert.equal(runGitSpy.mock.calls.length, 1);
-    assert.equal(runGitSpy.mock.calls[0].arguments[1].timeoutMs, 12345);
-    assert.notEqual(12345, DEFAULT_GIT_TIMEOUT_MS);
+    assert.equal(runGitSpy.mock.calls[0].arguments[1].timeoutMs, ARBITRARY_OVERRIDE_TIMEOUT_MS);
+    assert.notEqual(ARBITRARY_OVERRIDE_TIMEOUT_MS, DEFAULT_GIT_TIMEOUT_MS);
  });

  test('E12: shell-string args are rejected', () => {