From dcab56fda53ed4e318ff68cf77dd664b405e24e0 Mon Sep 17 00:00:00 2001 From: Mert Koseoglu Date: Sun, 21 Jun 2026 18:19:44 +0300 Subject: [PATCH] fix(hooks): add size threshold so small Bash/WebFetch calls skip interception (#817) --- hooks/core/routing.mjs | 42 +++++++++++++++++++++++ tests/core/routing.test.ts | 68 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/hooks/core/routing.mjs b/hooks/core/routing.mjs index bfb40f4f..e92ab46b 100644 --- a/hooks/core/routing.mjs +++ b/hooks/core/routing.mjs @@ -76,6 +76,37 @@ function getExternalMcpNudgeEvery() { return parsed; } +// #817: size threshold so small Bash calls skip the routing nudge. +// +// PreToolUse fires BEFORE the command runs, so the actual output size is +// unknowable here. The only deterministic pre-execution signal is the command +// string itself. The Gemini CLI adapter solves the same over-interception +// problem with a matcher that only fires on large-output tools — "avoids +// unnecessary hook overhead on lightweight tools" (README). We mirror that at +// the routing layer: when CONTEXT_MODE_BASH_NUDGE_MIN_COMMAND_BYTES is set to +// N>0, an unbounded Bash command whose UTF-8 byte length is below N is treated +// as expected-lightweight and the generic routing nudge is suppressed. +// +// Default is 0 (unset) → CURRENT BEHAVIOR: every unbounded command is nudged. +// This preserves the context-saving guarantee for large outputs by default — +// the threshold is strictly opt-in. Bounds [0, 100000]; invalid/zero/negative +// values fall back to 0 (disabled). The threshold gates ONLY the generic Bash +// nudge — curl/wget, inline-HTTP, and build-tool redirects run earlier and are +// never relaxed, because those are deterministic floods regardless of command +// length. +const BASH_NUDGE_MIN_BYTES_ENV = "CONTEXT_MODE_BASH_NUDGE_MIN_COMMAND_BYTES"; +const BASH_NUDGE_MIN_BYTES_MAX = 100_000; + +function getBashNudgeMinCommandBytes() { + const raw = process.env[BASH_NUDGE_MIN_BYTES_ENV]; + if (raw == null || raw === "") return 0; + const parsed = Number.parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed <= 0 || parsed > BASH_NUDGE_MIN_BYTES_MAX) { + return 0; + } + return parsed; +} + function defaultGuidanceId() { return process.env.VITEST_WORKER_ID ? `${process.ppid}-w${process.env.VITEST_WORKER_ID}` @@ -794,6 +825,17 @@ export function routePreToolUse(toolName, toolInput, projectDir, platform, sessi return null; } + // #817: opt-in size threshold. When the operator configures + // CONTEXT_MODE_BASH_NUDGE_MIN_COMMAND_BYTES, a short unbounded command is + // treated as expected-lightweight and passes through untouched — reserving + // the nudge for commands large/complex enough to plausibly flood context. + // Default (0) preserves current behavior, so large-output savings are not + // weakened unless the operator explicitly opts in. + const minCommandBytes = getBashNudgeMinCommandBytes(); + if (minCommandBytes > 0 && Buffer.byteLength(command, "utf8") < minCommandBytes) { + return null; + } + // allow all other Bash commands, but inject routing nudge (once per session) return guidanceOnce("bash", bashGuidance, sessionId); } diff --git a/tests/core/routing.test.ts b/tests/core/routing.test.ts index 98deb7c2..71ddfbaf 100644 --- a/tests/core/routing.test.ts +++ b/tests/core/routing.test.ts @@ -425,3 +425,71 @@ describe("Bash structurally-bounded allowlist: newline injection (#470)", () => expect(isStructurallyBounded("git status\rfind /")).toBe(false); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// #817: size threshold so small Bash/WebFetch calls skip interception. +// +// PreToolUse cannot observe a command's ACTUAL output size (the command has +// not run yet). The only deterministic pre-execution signal is the command +// string itself. The Gemini CLI adapter solves the same problem with a matcher +// that only fires on large-output tools — "avoids unnecessary hook overhead on +// lightweight tools" (README L193). We mirror that at the routing layer with an +// env-configurable command-length threshold: when CONTEXT_MODE_BASH_NUDGE_MIN_COMMAND_BYTES +// is set to N>0, an unbounded Bash command whose string is shorter than N bytes +// is treated as expected-lightweight and the routing nudge is skipped. +// +// Sane default: UNSET / 0 → current behavior (every unbounded command nudged), +// so the context-saving guarantee for large outputs is NOT silently weakened. +// Opt-in only — the operator chooses the threshold. +// ───────────────────────────────────────────────────────────────────────── +describe("Bash nudge size threshold (#817)", () => { + const SID = "threshold-817"; + const ENV = "CONTEXT_MODE_BASH_NUDGE_MIN_COMMAND_BYTES"; + + beforeEach(() => { + resetGuidanceThrottle(SID); + delete process.env[ENV]; + }); + + it("default (unset): short unbounded command STILL nudges — no behavior change", () => { + // Regression guard: without the env var, nothing changes. `ps` is short and + // unbounded — it must keep getting the nudge so the default stays safe. + const decision = routePreToolUse("Bash", { command: "ps" }, "/test", "claude-code", SID); + expect(decision?.action).toBe("context"); + }); + + it("threshold set: short unbounded command below threshold SKIPS the nudge", () => { + process.env[ENV] = "64"; + // "ps aux" is 6 bytes — below 64 → expected-lightweight → pass through. + const decision = routePreToolUse("Bash", { command: "ps aux" }, "/test", "claude-code", SID); + expect(decision, "short command below threshold should pass through untouched").toBeNull(); + }); + + it("threshold set: long unbounded command at/above threshold STILL nudges", () => { + process.env[ENV] = "16"; + // A long pipeline (> 16 bytes) can flood — must still intercept. + const long = "find / -type f -name '*.log' -exec cat {} +"; + const decision = routePreToolUse("Bash", { command: long }, "/test", "claude-code", SID); + expect(decision?.action, "long command must still be nudged").toBe("context"); + }); + + it("threshold does NOT relax curl/wget redirects (those stay deterministic)", () => { + process.env[ENV] = "4096"; // generous threshold — would otherwise mark this short cmd lightweight + // The threshold gates ONLY the generic Bash routing nudge. The curl/wget + // branch runs earlier and returns a `modify` redirect (or null only when + // MCP is unavailable) — it must NEVER be turned into a "pass-through-because-short". + // Assert the decision is NOT the generic "context" nudge: the threshold must + // not reclassify a curl flood as a lightweight bounded command. + const curl = routePreToolUse("Bash", { command: "curl https://x.io" }, "/test", "claude-code", SID); + expect(curl?.action ?? "modify-or-passthrough", "curl path must not become the generic nudge").not.toBe("context"); + }); + + it("invalid / zero env value falls back to default (every unbounded cmd nudged)", () => { + for (const bad of ["0", "-5", "abc", ""]) { + resetGuidanceThrottle(SID); + process.env[ENV] = bad; + const decision = routePreToolUse("Bash", { command: "ps" }, "/test", "claude-code", SID); + expect(decision?.action, `env="${bad}" should behave as default`).toBe("context"); + } + }); +});