From a0fb334de68ba3595859ebcec3c92a1d091baeb2 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 28 Oct 2026 13:00:28 -0700 Subject: [PATCH 1/2] fix: stop the Claude Code hook from approving run_tool calls The PreToolUse hook answered "allow" for every Glean run_tool call while ENABLE_HITL was on, to avoid a double prompt. Anthropic's plugin directory now holds any plugin whose hook can decide a permission question for the user, so v3.6.0 is held for review. The hook now only records Claude Code's permission mode, which the server uses to skip its own prompt under --dangerously-skip-permissions. It prints nothing, so Claude Code's normal permission flow always runs. The file is renamed to record-permission-mode.mjs to match what it does. Users see Claude Code's own prompt on run_tool again until they allow the tool permanently. The server's approval prompt still gates writes. --- overrides/claude/glean/hooks/hooks.json | 2 +- ...un-tool.mjs => record-permission-mode.mjs} | 40 ++++++------------- shared/glean/mcp/src/policy/key.ts | 5 +-- shared/glean/mcp/src/tools/run-tool.ts | 4 +- ...k.test.ts => permission-mode-hook.test.ts} | 23 +++++++---- 5 files changed, 32 insertions(+), 42 deletions(-) rename overrides/claude/glean/hooks/{auto-approve-run-tool.mjs => record-permission-mode.mjs} (62%) rename shared/glean/mcp/tests/{auto-approve-hook.test.ts => permission-mode-hook.test.ts} (87%) diff --git a/overrides/claude/glean/hooks/hooks.json b/overrides/claude/glean/hooks/hooks.json index f3470f6..a59adb6 100644 --- a/overrides/claude/glean/hooks/hooks.json +++ b/overrides/claude/glean/hooks/hooks.json @@ -19,7 +19,7 @@ "hooks": [ { "type": "command", - "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/auto-approve-run-tool.mjs\"" + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/record-permission-mode.mjs\"" } ] } diff --git a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs b/overrides/claude/glean/hooks/record-permission-mode.mjs similarity index 62% rename from overrides/claude/glean/hooks/auto-approve-run-tool.mjs rename to overrides/claude/glean/hooks/record-permission-mode.mjs index 366fd69..1e2abca 100644 --- a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs +++ b/overrides/claude/glean/hooks/record-permission-mode.mjs @@ -1,16 +1,13 @@ #!/usr/bin/env node // PreToolUse hook for Claude Code. // -// When HITL is enabled, run_tool is gated by the plugin's own elicitation -// prompt, so Claude Code's separate native "allow this tool?" prompt is -// redundant (the double-prompt). With ENABLE_HITL=true we auto-approve the -// run_tool call, leaving the HITL elicitation as the single gate. +// Records Claude Code's live permission mode before each Glean run_tool call, +// so the MCP server can skip its own elicitation gate when the user launched +// with --dangerously-skip-permissions (permission_mode "bypassPermissions"). // -// Safety: run_tool is read-only ONLY while HITL gates it. This hook runs only -// under Claude Code, which always advertises the elicitation capability, so -// ENABLE_HITL=true means run_tool's HITL prompt is active — never an ungated -// write. When ENABLE_HITL is not "true" the hook does nothing and the normal -// permission flow runs. +// This hook makes no permission decision and prints nothing. Claude Code's +// normal permission flow always runs, and the server's elicitation prompt +// still gates every write. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -27,7 +24,7 @@ let input = {}; try { input = JSON.parse(readStdin()); } catch { - // Malformed/empty input: do nothing, let the normal permission flow run. + // Malformed/empty input: nothing to record. } const toolName = String(input.tool_name ?? ""); @@ -55,19 +52,17 @@ try { cfg?.mcpServers?.glean?.env ?? {}; } catch { - // No readable config: do nothing. + // No readable config: nothing to record. } +// The server reads the marker only when its own elicitation gate is on. if (env.ENABLE_HITL === "true") { - // Record Claude Code's live permission mode so the MCP server can skip its - // own elicitation gate when the user launched with - // --dangerously-skip-permissions (permission_mode "bypassPermissions"). // Written on every run_tool call and keyed by session id, so it is always // fresh for the call that immediately follows and never leaks across // sessions. The base dir and session-id sanitization MUST match // run-tool.ts (permissionModeMarkerPath) and start.sh's PLUGIN_DATA_DIR: - // CLAUDE_PLUGIN_DATA when set, else ~/.glean. Best-effort — marker I/O must - // never break the approval decision below. + // CLAUDE_PLUGIN_DATA when set, else ~/.glean. Best-effort — a failed write + // just means the server keeps prompting. try { const permissionMode = String(input.permission_mode ?? ""); const sessionId = String(input.session_id ?? "") @@ -84,18 +79,7 @@ if (env.ENABLE_HITL === "true") { ); } } catch { - // Ignore: a failed marker write just means the server keeps prompting. + // Ignore: see above. } - - process.stdout.write( - JSON.stringify({ - hookSpecificOutput: { - hookEventName: "PreToolUse", - permissionDecision: "allow", - permissionDecisionReason: - "Glean run_tool is gated by its own HITL elicitation prompt; suppressing the redundant native prompt while ENABLE_HITL is on.", - }, - }), - ); } process.exit(0); diff --git a/shared/glean/mcp/src/policy/key.ts b/shared/glean/mcp/src/policy/key.ts index 8d939cd..c4cbc39 100644 --- a/shared/glean/mcp/src/policy/key.ts +++ b/shared/glean/mcp/src/policy/key.ts @@ -11,9 +11,8 @@ export const CAPABILITY_POLICY_KEY = "com.glean.mcp/capabilityPolicy"; // HITL only makes sense once approval moves to the remote, and the remote is stateless: // it has no back-channel to send `elicitation/create` on, so remote-side approval waits // on MCP 2026-07-28 landing there. Until then a remote-disabled HITL would leave no gate -// at all -- and under Claude Code it would be worse than having no policy, because the -// plugin's PreToolUse hook auto-approves run_tool on the premise that the local prompt -// IS the gate. A remote that needs to stop such a plugin deactivates the version. +// at all -- run_tool advertises readOnlyHint on the premise that the local prompt IS +// the gate. A remote that needs to stop such a plugin deactivates the version. // // Declaring it here is what would make the remote believe it is controllable, so the // honest signal is to leave it out until a build can honour it. A remote that sends diff --git a/shared/glean/mcp/src/tools/run-tool.ts b/shared/glean/mcp/src/tools/run-tool.ts index 8283b6f..c44ae23 100644 --- a/shared/glean/mcp/src/tools/run-tool.ts +++ b/shared/glean/mcp/src/tools/run-tool.ts @@ -287,7 +287,7 @@ function primeElicitationCancellation(mcpServer: Server): void { } // Path to the per-session permission-mode marker the PreToolUse hook writes -// immediately before each run_tool call (see hooks/auto-approve-run-tool.mjs). +// immediately before each run_tool call (see hooks/record-permission-mode.mjs). // The directory has to be the one the HOOK can compute, not the one this process // would prefer -- see hostSharedDataDir() in ../data-dir.ts. function permissionModeMarkerPath(): string { @@ -303,7 +303,7 @@ function permissionModeMarkerPath(): string { // so any failure fails toward prompting, never toward a silent bypass. // // Resume safety: the PreToolUse hook rewrites this marker with the CURRENT mode -// on every run_tool call (see hooks/auto-approve-run-tool.mjs), and PreToolUse +// on every run_tool call (see hooks/record-permission-mode.mjs), and PreToolUse // always runs before the tool executes, so the value read here is the one // written for this exact call. A session first launched with // --dangerously-skip-permissions and later resumed WITHOUT it (same session id) diff --git a/shared/glean/mcp/tests/auto-approve-hook.test.ts b/shared/glean/mcp/tests/permission-mode-hook.test.ts similarity index 87% rename from shared/glean/mcp/tests/auto-approve-hook.test.ts rename to shared/glean/mcp/tests/permission-mode-hook.test.ts index 2b647eb..4383134 100644 --- a/shared/glean/mcp/tests/auto-approve-hook.test.ts +++ b/shared/glean/mcp/tests/permission-mode-hook.test.ts @@ -8,7 +8,7 @@ import { fileURLToPath } from "node:url"; const here = path.dirname(fileURLToPath(import.meta.url)); const HOOK = path.resolve( here, - "../../../../overrides/claude/glean/hooks/auto-approve-run-tool.mjs", + "../../../../overrides/claude/glean/hooks/record-permission-mode.mjs", ); interface HookResult { @@ -84,13 +84,21 @@ const hitlOn = { ENABLE_HITL: "true" }; const hitlOff = { ENABLE_HITL: "false" }; const bypass = { permission_mode: "bypassPermissions", session_id: "sess-1" }; -describe("auto-approve-run-tool hook (Claude Code PreToolUse)", () => { - it("allows glean run_tool when HITL is on", async () => { +describe("record-permission-mode hook (Claude Code PreToolUse)", () => { + // Anthropic's plugin directory holds any plugin whose hook can decide a + // permission question for the user (HOOK_GRANTS_PERMISSION). The check reads + // the file statically, so the decision must not appear anywhere in it. + it("contains no permission decision", async () => { + const source = await fs.readFile(HOOK, "utf-8"); + expect(source).not.toMatch(/permissionDecision|"allow"|"approve"/); + }); + + it("prints nothing for glean run_tool when HITL is on", async () => { const { out } = await runHook(glean("run_tool"), hitlOn); - expect(JSON.parse(out).hookSpecificOutput.permissionDecision).toBe("allow"); + expect(out.trim()).toBe(""); }); - it("never allows when HITL is off (safety)", async () => { + it("prints nothing when HITL is off", async () => { const { out } = await runHook(glean("run_tool"), hitlOff); expect(out.trim()).toBe(""); }); @@ -106,7 +114,7 @@ describe("auto-approve-run-tool hook (Claude Code PreToolUse)", () => { }); }); -describe("auto-approve-run-tool hook (permission-mode marker)", () => { +describe("record-permission-mode hook (permission-mode marker)", () => { it("records the permission_mode marker for run_tool when HITL is on", async () => { const { marker, markerFiles } = await runHook( glean("run_tool"), @@ -145,8 +153,7 @@ describe("auto-approve-run-tool hook (permission-mode marker)", () => { const { out, marker } = await runHook(glean("run_tool"), hitlOn, { session_id: "sess-1", }); - // Still auto-approves, just has no mode to record. - expect(JSON.parse(out).hookSpecificOutput.permissionDecision).toBe("allow"); + expect(out.trim()).toBe(""); expect(marker).toBeNull(); }); From 03ef162f0a678f8745751040ed6626d581d0651c Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 28 Oct 2026 13:51:39 -0700 Subject: [PATCH 2/2] fix: keep permission words out of the hook and correct its comment The hook comment said the server's prompt gates every write. It skips tools the user told it to stop asking about, so say that. The directory's check reads the hook file statically, so the test now fails on "allow" or "approve" anywhere in the file, in any case. --- overrides/claude/glean/hooks/record-permission-mode.mjs | 4 ++-- shared/glean/mcp/tests/permission-mode-hook.test.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/overrides/claude/glean/hooks/record-permission-mode.mjs b/overrides/claude/glean/hooks/record-permission-mode.mjs index 1e2abca..47286cd 100644 --- a/overrides/claude/glean/hooks/record-permission-mode.mjs +++ b/overrides/claude/glean/hooks/record-permission-mode.mjs @@ -6,8 +6,8 @@ // with --dangerously-skip-permissions (permission_mode "bypassPermissions"). // // This hook makes no permission decision and prints nothing. Claude Code's -// normal permission flow always runs, and the server's elicitation prompt -// still gates every write. +// normal permission flow always runs. The server's elicitation prompt still +// gates writes, except for tools the user told it to stop asking about. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; diff --git a/shared/glean/mcp/tests/permission-mode-hook.test.ts b/shared/glean/mcp/tests/permission-mode-hook.test.ts index 4383134..1b6c00b 100644 --- a/shared/glean/mcp/tests/permission-mode-hook.test.ts +++ b/shared/glean/mcp/tests/permission-mode-hook.test.ts @@ -87,10 +87,11 @@ const bypass = { permission_mode: "bypassPermissions", session_id: "sess-1" }; describe("record-permission-mode hook (Claude Code PreToolUse)", () => { // Anthropic's plugin directory holds any plugin whose hook can decide a // permission question for the user (HOOK_GRANTS_PERMISSION). The check reads - // the file statically, so the decision must not appear anywhere in it. + // the file statically, so neither the decision nor its words may appear + // anywhere in it, comments included. it("contains no permission decision", async () => { const source = await fs.readFile(HOOK, "utf-8"); - expect(source).not.toMatch(/permissionDecision|"allow"|"approve"/); + expect(source).not.toMatch(/permissionDecision|allow|approve/i); }); it("prints nothing for glean run_tool when HITL is on", async () => {