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..47286cd 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. 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"; @@ -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..1b6c00b 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,22 @@ 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 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/i); + }); + + 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 +115,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 +154,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(); });