diff --git a/eslint-rules/no-adhoc-timeout-literal.allowlist.json b/eslint-rules/no-adhoc-timeout-literal.allowlist.json index c9878805c..0733f60d2 100644 --- a/eslint-rules/no-adhoc-timeout-literal.allowlist.json +++ b/eslint-rules/no-adhoc-timeout-literal.allowlist.json @@ -52,19 +52,12 @@ "tests/plan-review-convergence.test.cjs", "tests/policy-160-route0-resume.test.cjs", "tests/prohibition-enforcement.test.cjs", - "tests/prompt-injection-scan.security.test.cjs", "tests/quick-batch.test.cjs", - "tests/read-injection-scanner.property.test.cjs", - "tests/read-injection-scanner.security.test.cjs", "tests/representative-corpus.test.cjs", "tests/review-lane-invocation.test.cjs", "tests/reviewer-manifest-body.test.cjs", "tests/reviewer-trust-disclosure.test.cjs", "tests/run-tests-temp-root.test.cjs", - "tests/secret-scan-lint.security.test.cjs", - "tests/security-prompt-injection.security.test.cjs", - "tests/security-scan.security.test.cjs", - "tests/security.test.cjs", "tests/shell-command-projection-dispatch.test.cjs", "tests/state-document.test.cjs", "tests/state-todos-render.test.cjs", diff --git a/tests/helpers/timeouts.cjs b/tests/helpers/timeouts.cjs index 39a9ad755..2133b616e 100644 --- a/tests/helpers/timeouts.cjs +++ b/tests/helpers/timeouts.cjs @@ -188,6 +188,42 @@ const STAGED_HOOK_SCRIPT_TIMEOUT_MS = 20000; */ const LOOP_HOOK_POINT_CLI_TIMEOUT_MS = 60000; +/** + * A security-scan shell script (secret-scan.sh, secret-scan-lint.sh, + * prompt-injection-scan.sh, base64-scan.sh) invoked with missing or invalid + * CLI arguments -- exits almost instantly with a usage error, no scan work + * performed at all. A distinct, lighter-weight class than a real scan of + * even a single tiny fixture (`QUICK_SPAWN_TIMEOUT_MS`, 10000ms) -- this is + * the fast-fail path, not the scan path. + * + * Deliberately a separate name from `MALFORMED_INPUT_HOOK_TIMEOUT_MS`, even + * though the two currently coincide at the same value: that constant bounds + * a Node hook script fed malformed input (a different runtime, a different + * failure shape), not a bash scan script given no arguments. Collapsing them + * would let a future, independent tune of either value silently move the + * other -- the same reasoning `SEAM_DEFAULT_TIMEOUT_MS` documents for its own + * coincidence with `HOOK_FANOUT_TIMEOUT_MS`. + * + * Shared across 2 files in batch #4519 of the ad hoc timeout literal + * migration, epic #4445 -- every site independently arrived at this exact + * value -- that is why it lives here rather than as a file-local constant. + */ +const SCAN_USAGE_ERROR_TIMEOUT_MS = 5000; + +/** + * A Node hook script (gsd-prompt-guard-hook.js, gsd-read-injection-scanner.js) + * spawned directly and fed malformed JSON on stdin -- expected to fail closed + * gracefully, no subprocess fan-out, no real scan work. A different operation + * family from `SCAN_USAGE_ERROR_TIMEOUT_MS` (a bash scan script given no + * arguments) despite the coincidentally-matching value -- see that constant's + * own doc comment for why the two are kept separate rather than merged. + * + * Shared across 2 files in batch #4519 of the ad hoc timeout literal + * migration, epic #4445 -- every site independently arrived at this exact + * value -- that is why it lives here rather than as a file-local constant. + */ +const MALFORMED_INPUT_HOOK_TIMEOUT_MS = 5000; + module.exports = { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS, @@ -200,4 +236,6 @@ module.exports = { FIXTURE_HOOK_TIMEOUT_SECONDS, STAGED_HOOK_SCRIPT_TIMEOUT_MS, LOOP_HOOK_POINT_CLI_TIMEOUT_MS, + SCAN_USAGE_ERROR_TIMEOUT_MS, + MALFORMED_INPUT_HOOK_TIMEOUT_MS, }; diff --git a/tests/prompt-injection-scan.security.test.cjs b/tests/prompt-injection-scan.security.test.cjs index 7c8d72327..87f70245f 100644 --- a/tests/prompt-injection-scan.security.test.cjs +++ b/tests/prompt-injection-scan.security.test.cjs @@ -32,6 +32,7 @@ const path = require('path'); const { scanForInjection } = require('../gsd-core/bin/lib/security.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); +const { QUICK_SPAWN_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // ─── Configuration ────────────────────────────────────────────────────────── @@ -51,7 +52,7 @@ function scanContent(t, content) { t.after(() => cleanup(dir)); const file = path.join(dir, 'fixture.txt'); fs.writeFileSync(file, `${content}\n`); - const result = runHook(SCAN_SCRIPT, ['--file', file], { interpreter: 'bash', timeoutMs: 10_000 }); + const result = runHook(SCAN_SCRIPT, ['--file', file], { interpreter: 'bash', timeoutMs: QUICK_SPAWN_TIMEOUT_MS }); return result; } diff --git a/tests/read-injection-scanner.property.test.cjs b/tests/read-injection-scanner.property.test.cjs index bed64f101..3c5459046 100644 --- a/tests/read-injection-scanner.property.test.cjs +++ b/tests/read-injection-scanner.property.test.cjs @@ -32,6 +32,13 @@ const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-read-injection-scanner.js'); +/** + * Safety-net ceiling for a single hook invocation — 6x the hook's own + * internal 5s timer. Never itself an assertion target; see the rationale + * in runHook()'s doc comment below. + */ +const READ_INJECTION_SCANNER_SAFETY_NET_TIMEOUT_MS = 30000; + /** * Run the scanner hook with a payload and return its result. * @@ -57,7 +64,10 @@ function runHook(payload) { return { exitCode: 0, stdout: '', skipped: true }; } - const result = runHookSeam(HOOK_PATH, [], { input, timeoutMs: 30000 }); + const result = runHookSeam(HOOK_PATH, [], { + input, + timeoutMs: READ_INJECTION_SCANNER_SAFETY_NET_TIMEOUT_MS, + }); return { exitCode: result.exitCode ?? 1, diff --git a/tests/read-injection-scanner.security.test.cjs b/tests/read-injection-scanner.security.test.cjs index 09a5192ab..d9bee2c28 100644 --- a/tests/read-injection-scanner.security.test.cjs +++ b/tests/read-injection-scanner.security.test.cjs @@ -24,6 +24,7 @@ const { cleanup } = require('./helpers.cjs'); // #4020: fixture-tree removal const assert = require('node:assert/strict'); const { execFileSync } = require('node:child_process'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { MALFORMED_INPUT_HOOK_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const HOOK_PATH = require('node:path').join(__dirname, '..', 'hooks', 'gsd-read-injection-scanner.js'); @@ -186,7 +187,7 @@ describe('gsd-read-injection-scanner: edge cases', () => { let signal = null; try { stdout = execFileSync(process.execPath, [HOOK_PATH], { - input, encoding: 'utf-8', timeout: 5000, stdio: ['pipe', 'pipe', 'pipe'], + input, encoding: 'utf-8', timeout: MALFORMED_INPUT_HOOK_TIMEOUT_MS, stdio: ['pipe', 'pipe', 'pipe'], }).trim(); } catch (err) { exitCode = err.status ?? 0; diff --git a/tests/secret-scan-lint.security.test.cjs b/tests/secret-scan-lint.security.test.cjs index 49f551567..e3d7b18dd 100644 --- a/tests/secret-scan-lint.security.test.cjs +++ b/tests/secret-scan-lint.security.test.cjs @@ -39,6 +39,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { cleanup } = require('./helpers.cjs'); +const { QUICK_SPAWN_TIMEOUT_MS, SCAN_USAGE_ERROR_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const PROJECT_ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(PROJECT_ROOT, 'scripts', 'secret-scan-lint.sh'); @@ -62,7 +63,7 @@ function runLint(ignoreContent, extraArgs = []) { const args = ['--file', ignoreFile, ...extraArgs]; const result = spawnSync(LINT_SCRIPT, args, { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, }); return { status: result.status !== null ? result.status : 1, @@ -87,7 +88,7 @@ function runSecretScan(fileContent, extraArgs = []) { const args = ['--file', tmpFile, ...extraArgs]; const result = spawnSync(SECRET_SCAN, args, { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, }); return { status: result.status !== null ? result.status : 1, @@ -318,7 +319,7 @@ describe('lint: grandfathered entries (backward compat)', { skip: IS_WINDOWS }, execFileSync(LINT_SCRIPT, [], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], - timeout: 5000, + timeout: SCAN_USAGE_ERROR_TIMEOUT_MS, }); assert.fail('Should have exited non-zero'); } catch (err) { @@ -339,7 +340,7 @@ describe('secret-scan.sh --strict: reduces effective exclusions', { skip: IS_WIN try { const result = spawnSync(SECRET_SCAN, ['--file', tmpFile, '--strict'], { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, cwd: tmpDir, // No .secretscanignore here — clean workspace }); const status = result.status !== null ? result.status : 1; @@ -384,7 +385,7 @@ describe('secret-scan.sh --strict: reduces effective exclusions', { skip: IS_WIN // Default mode: grandfathered entry IS honoured → file is excluded → exit 0 const defaultResult = spawnSync(SECRET_SCAN, ['--file', relFile], { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, cwd: tmpDir, // CWD has .secretscanignore with the grandfathered entry }); const defaultStatus = defaultResult.status !== null ? defaultResult.status : 1; @@ -392,7 +393,7 @@ describe('secret-scan.sh --strict: reduces effective exclusions', { skip: IS_WIN // Strict mode: grandfathered entry NOT honoured → file is scanned → exit 1 const strictResult = spawnSync(SECRET_SCAN, ['--file', relFile, '--strict'], { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, cwd: tmpDir, // Same CWD, same .secretscanignore }); const strictStatus = strictResult.status !== null ? strictResult.status : 0; @@ -426,7 +427,7 @@ describe('secret-scan.sh default mode: regression test', { skip: IS_WINDOWS }, ( // .secretscanignore is found. Excluded → scanned 0 files → exit 0. const result = spawnSync(SECRET_SCAN, ['--file', 'gsd-core/workflows/plan-phase.md'], { encoding: 'utf-8', - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, cwd: PROJECT_ROOT, }); const status = result.status !== null ? result.status : 1; diff --git a/tests/security-prompt-injection.security.test.cjs b/tests/security-prompt-injection.security.test.cjs index f86cdb854..19287448b 100644 --- a/tests/security-prompt-injection.security.test.cjs +++ b/tests/security-prompt-injection.security.test.cjs @@ -85,6 +85,7 @@ const { } = require('./helpers.cjs'); const { runCli } = require('./helpers/cli-negative.cjs'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { MALFORMED_INPUT_HOOK_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const PROMPT_GUARD_HOOK = path.join(REPO_ROOT, 'hooks', 'gsd-prompt-guard.js'); @@ -402,7 +403,7 @@ describe('gsd-prompt-guard: hostile .planning/ writes are advised, not blocked', const r = spawnSync(process.execPath, [PROMPT_GUARD_HOOK], { input: 'this is not json at all', encoding: 'utf-8', - timeout: 5000, + timeout: MALFORMED_INPUT_HOOK_TIMEOUT_MS, }); assert.strictEqual(r.status, 0, 'hook must never propagate parser failure'); }); @@ -493,7 +494,7 @@ describe('gsd-read-injection-scanner: hostile reads are flagged with severity', const r = spawnSync(process.execPath, [READ_SCANNER_HOOK], { input: '{not json', encoding: 'utf-8', - timeout: 5000, + timeout: MALFORMED_INPUT_HOOK_TIMEOUT_MS, }); assert.strictEqual(r.status, 0, 'hook must silent-fail on parser error — never block downstream tool'); diff --git a/tests/security-scan.security.test.cjs b/tests/security-scan.security.test.cjs index cc718afac..370c63b97 100644 --- a/tests/security-scan.security.test.cjs +++ b/tests/security-scan.security.test.cjs @@ -42,7 +42,19 @@ const path = require('path'); const { cleanup, createTempGitProject } = require('./helpers.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); const { gitOrThrow, GIT_FIXTURE_TIMEOUT_MS } = require('./helpers/git-fixture.cjs'); -const { HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const { + HOOK_FANOUT_TIMEOUT_MS, + QUICK_SPAWN_TIMEOUT_MS, + SCAN_USAGE_ERROR_TIMEOUT_MS, +} = require('./helpers/timeouts.cjs'); + +// Bounds base64-scan.sh under the locale-regression scenario (see the +// comment above the describe block below): a dir/file scan with non-UTF8 +// content under a non-C locale must complete cleanly within 30s. Numerically +// coincides with timeouts.cjs's BUILD_TIMEOUT_MS (an unrelated +// hooks-bundle-build operation) but matches neither that nor any other +// existing shared constant, so it stays file-local. +const BASE64_LOCALE_SCAN_TIMEOUT_MS = 30000; const PROJECT_ROOT = path.join(__dirname, '..'); const SCRIPTS = { @@ -68,7 +80,7 @@ function runScript(scriptPath, content, extraArgs) { const result = execFileSync(scriptPath, args, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], - timeout: 10000, + timeout: QUICK_SPAWN_TIMEOUT_MS, }); return { status: 0, stdout: result, stderr: '' }; } catch (err) { @@ -201,7 +213,7 @@ describe('prompt-injection-scan.sh', { skip: IS_WINDOWS }, () => { execFileSync(SCRIPTS.injection, [], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], - timeout: 5000, + timeout: SCAN_USAGE_ERROR_TIMEOUT_MS, }); assert.fail('Should have exited non-zero'); } catch (err) { @@ -218,7 +230,7 @@ describe('prompt-injection-scan.sh', { skip: IS_WINDOWS }, () => { function runScriptOnDir(scriptPath, dirPath, env) { const result = spawnSync('bash', [scriptPath, '--dir', dirPath], { encoding: 'utf-8', - timeout: 30000, + timeout: BASE64_LOCALE_SCAN_TIMEOUT_MS, env: { ...process.env, ...env }, }); return { @@ -233,7 +245,7 @@ function runScriptOnDir(scriptPath, dirPath, env) { function runScriptOnFile(scriptPath, filePath, env) { const result = spawnSync('bash', [scriptPath, '--file', filePath], { encoding: 'utf-8', - timeout: 30000, + timeout: BASE64_LOCALE_SCAN_TIMEOUT_MS, env: { ...process.env, ...env }, }); return { @@ -292,7 +304,7 @@ describe('base64-scan.sh', { skip: IS_WINDOWS }, () => { execFileSync(SCRIPTS.base64, [], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], - timeout: 5000, + timeout: SCAN_USAGE_ERROR_TIMEOUT_MS, }); assert.fail('Should have exited non-zero'); } catch (err) { @@ -504,7 +516,7 @@ describe('secret-scan.sh', { skip: IS_WINDOWS }, () => { execFileSync(SCRIPTS.secret, [], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], - timeout: 5000, + timeout: SCAN_USAGE_ERROR_TIMEOUT_MS, }); assert.fail('Should have exited non-zero'); } catch (err) { diff --git a/tests/security.test.cjs b/tests/security.test.cjs index 7c64c66a5..e75f0944e 100644 --- a/tests/security.test.cjs +++ b/tests/security.test.cjs @@ -654,10 +654,19 @@ describe('validateFieldName', () => { const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +// Bounds a single advisory hook invocation (gsd-context-monitor.js or +// gsd-statusline.js) under a session_id path-traversal security test — +// trivial synchronous work, no subprocess fan-out, expected near-instant +// silent exit. The value (3000ms) is the tightest bound in this migration's +// security-scanners batch and unique to this file/class; preserved exactly, +// not widened to match any other existing norm. No fresh bench data +// justifies a different number. +const SESSION_ID_TRAVERSAL_HOOK_TIMEOUT_MS = 3000; + function runHook(hookPath, inputJson) { const result = runHookSeam(hookPath, [], { input: JSON.stringify(inputJson), - timeoutMs: 3000, + timeoutMs: SESSION_ID_TRAVERSAL_HOOK_TIMEOUT_MS, }); return { exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; }