From 9543dfad4b89baff66579d6ebcae09ee7e07f75a Mon Sep 17 00:00:00 2001 From: Pragati Agrawal Date: Mon, 28 Sep 2026 17:57:27 +0530 Subject: [PATCH 1/2] fix: honor HITL opt-outs without persisting approvals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use request-local Claude hook context for bypass mode and keep run_tool approval in the local plugin. Disable approval lookup and upstream forms when opted out, without saving an Always Allow preference. — sent via Glean Tau --- .../glean/hooks/auto-approve-run-tool.mjs | 125 ++----- overrides/claude/glean/hooks/hooks.json | 2 +- shared/glean/mcp/approval-context.d.mts | 5 + shared/glean/mcp/approval-context.mjs | 116 ++++++ shared/glean/mcp/pluginpack.json | 1 + shared/glean/mcp/src/index.ts | 53 +-- shared/glean/mcp/src/tools/run-tool.ts | 185 ++++------ shared/glean/mcp/src/write-approval.ts | 59 ++++ .../glean/mcp/tests/approval-context.test.ts | 139 ++++++++ .../glean/mcp/tests/approval-handler.test.ts | 333 ++++++++++++++++++ .../glean/mcp/tests/auto-approve-hook.test.ts | 253 +++++-------- shared/glean/mcp/tests/run-tool.test.ts | 22 +- shared/glean/mcp/tests/write-approval.test.ts | 292 +++++++++++++++ 13 files changed, 1175 insertions(+), 410 deletions(-) create mode 100644 shared/glean/mcp/approval-context.d.mts create mode 100644 shared/glean/mcp/approval-context.mjs create mode 100644 shared/glean/mcp/src/write-approval.ts create mode 100644 shared/glean/mcp/tests/approval-context.test.ts create mode 100644 shared/glean/mcp/tests/approval-handler.test.ts create mode 100644 shared/glean/mcp/tests/write-approval.test.ts diff --git a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs b/overrides/claude/glean/hooks/auto-approve-run-tool.mjs index 366fd69..ecb3670 100644 --- a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs +++ b/overrides/claude/glean/hooks/auto-approve-run-tool.mjs @@ -1,101 +1,48 @@ #!/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. -// -// 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. +// PreToolUse: carry request-local bypass context; only run_tool gets native approval. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { pathToFileURL } from "node:url"; -function readStdin() { - try { - return fs.readFileSync(0, "utf-8"); - } catch { - return ""; - } -} - -let input = {}; try { - input = JSON.parse(readStdin()); -} catch { - // Malformed/empty input: do nothing, let the normal permission flow run. -} + const input = JSON.parse(fs.readFileSync(0, "utf8")); + const root = process.env.CLAUDE_PLUGIN_ROOT; + if (!root || typeof input?.tool_name !== "string") process.exit(0); + const manifest = JSON.parse(fs.readFileSync(path.join(root, ".claude-plugin/plugin.json"), "utf8")); + const cfg = JSON.parse(fs.readFileSync(path.join(root, ".mcp.json"), "utf8")); + if (typeof manifest.name !== "string" || !/^[a-zA-Z0-9_-]+$/.test(manifest.name)) process.exit(0); -const toolName = String(input.tool_name ?? ""); -const bareName = toolName.split("__").pop() ?? ""; -// Scope strictly to this plugin's run_tool — the tool name carries the glean -// plugin/server prefix (e.g. mcp__plugin_local-mcp_glean_plugin__run_tool). -if (!toolName.includes("glean") || bareName !== "run_tool") { - process.exit(0); -} - -// The hook process does not inherit the MCP server's env, so read the flag -// from the plugin's own .mcp.json. -let env = {}; -try { - const root = process.env.CLAUDE_PLUGIN_ROOT ?? "."; - const cfg = JSON.parse(fs.readFileSync(path.join(root, ".mcp.json"), "utf-8")); - // The server is named "glean_plugin" in the shipped .mcp.json (namespaced - // under "glean" so it's recognizable alongside a user-connected remote - // Glean server, e.g. "glean_default"). Keep fallbacks to the prior names - // ("glean-local", then the original "glean") for backwards compatibility - // with older installs. - env = - cfg?.mcpServers?.["glean_plugin"]?.env ?? - cfg?.mcpServers?.["glean-local"]?.env ?? - cfg?.mcpServers?.glean?.env ?? - {}; -} catch { - // No readable config: do nothing. -} + for (const [server, config] of Object.entries(cfg.mcpServers ?? {})) { + if (!/^[a-zA-Z0-9_-]+$/.test(server) || !config || + typeof config.command !== "string" || !config.command || config.url || + (config.type && config.type !== "stdio")) continue; + const prefix = `mcp__plugin_${manifest.name}_${server}__`; + if (!input.tool_name.startsWith(prefix)) continue; + const bareName = input.tool_name.slice(prefix.length); + if (!/^[a-zA-Z0-9_-]+$/.test(bareName)) continue; -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. - try { - const permissionMode = String(input.permission_mode ?? ""); - const sessionId = String(input.session_id ?? "") - .replace(/[^a-zA-Z0-9_-]/g, "-") - .slice(0, 64); - if (permissionMode && sessionId) { - const base = - process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".glean"); - const dir = path.join(base, "glean-hitl-mode"); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync( - path.join(dir, `${sessionId}.json`), - JSON.stringify({ permission_mode: permissionMode, ts: Date.now() }), - ); + const validArgs = input.tool_input && typeof input.tool_input === "object" && !Array.isArray(input.tool_input); + // Empty, not omitted: hosts may merge updatedInput with the original input. + const updatedInput = { ...(validArgs ? input.tool_input : {}), _glean_permission_context: "" }; + const hitl = config.env?.ENABLE_HITL === "true"; + if (hitl && validArgs && input.permission_mode === "bypassPermissions") { + try { + const { createBypassReceipt } = await import(pathToFileURL(path.join(root, "mcp/approval-context.mjs")).href); + const base = process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".glean"); + updatedInput._glean_permission_context = createBypassReceipt(base, bareName, updatedInput); + } catch { /* Empty context keeps the plugin's approval gate active. */ } } - } catch { - // Ignore: a failed marker write just means the server keeps prompting. - } - - process.stdout.write( - JSON.stringify({ + 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.", + updatedInput, + ...(hitl && bareName === "run_tool" ? { + 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); + })); + break; + } +} catch { /* Invalid hook input or plugin configuration: retain native permissions. */ } diff --git a/overrides/claude/glean/hooks/hooks.json b/overrides/claude/glean/hooks/hooks.json index f3470f6..deaee09 100644 --- a/overrides/claude/glean/hooks/hooks.json +++ b/overrides/claude/glean/hooks/hooks.json @@ -15,7 +15,7 @@ ], "PreToolUse": [ { - "matcher": "mcp__.*glean.*run_tool", + "matcher": "^mcp__plugin_.*__.*$", "hooks": [ { "type": "command", diff --git a/shared/glean/mcp/approval-context.d.mts b/shared/glean/mcp/approval-context.d.mts new file mode 100644 index 0000000..f5bdbd4 --- /dev/null +++ b/shared/glean/mcp/approval-context.d.mts @@ -0,0 +1,5 @@ +export declare const PERMISSION_CONTEXT_ARG: "_glean_permission_context"; +export declare function stripPermissionContext(args: Record): Record; +export declare function createBypassReceipt(baseDir: string, toolName: string, args: Record): string; +export declare function consumeBypassReceipt(baseDir: string, token: unknown, toolName: string, args: Record): boolean; +export declare function cleanupBypassReceipts(baseDir: string): void; diff --git a/shared/glean/mcp/approval-context.mjs b/shared/glean/mcp/approval-context.mjs new file mode 100644 index 0000000..a968ebb --- /dev/null +++ b/shared/glean/mcp/approval-context.mjs @@ -0,0 +1,116 @@ +import fs from "node:fs"; +import path from "node:path"; +import { createHash, randomUUID } from "node:crypto"; + +export const PERMISSION_CONTEXT_ARG = "_glean_permission_context"; +const DIR = "glean-bypass-receipts"; +const TTL_MS = 5 * 60_000; +const UUID = "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}"; +const TOKEN = new RegExp(`^${UUID}$`, "i"); +const OWN_FILE = new RegExp(`^${UUID}\\.(json|claimed)$`, "i"); + +export function stripPermissionContext(args) { + const clean = { ...args }; + delete clean[PERMISSION_CONTEXT_ARG]; + return clean; +} + +function canonical(value) { + if (Array.isArray(value)) return value.map(canonical); + if (value && typeof value === "object") { + return Object.fromEntries(Object.keys(value).sort().map((k) => [k, canonical(value[k])])); + } + return value; +} + +function fingerprint(args) { + return createHash("sha256").update(JSON.stringify(canonical(stripPermissionContext(args)))).digest("hex"); +} + +function privateStat(stat) { + // Windows uses the user-profile directory's ACLs, not Unix ownership/mode bits. + return typeof process.getuid !== "function" || + ((stat.mode & 0o077) === 0 && stat.uid === process.getuid()); +} + +function receiptDir(baseDir, create = false) { + if (create) fs.mkdirSync(baseDir, { recursive: true, mode: 0o700 }); + if (!fs.lstatSync(baseDir).isDirectory()) throw new Error("Invalid receipt base directory"); + const dir = path.join(baseDir, DIR); + if (create) { + try { fs.mkdirSync(dir, { mode: 0o700 }); } + catch (error) { if (error.code !== "EEXIST") throw error; } + } + const stat = fs.lstatSync(dir); + if (!stat.isDirectory() || !privateStat(stat)) throw new Error("Invalid receipt directory"); + return dir; +} + +export function createBypassReceipt(baseDir, toolName, args) { + const body = JSON.stringify({ toolName, fingerprint: fingerprint(args), ts: Date.now() }); + if (Buffer.byteLength(body) > 4096) throw new Error("Receipt too large"); + const token = randomUUID(); + const file = path.join(receiptDir(baseDir, true), `${token}.json`); + const fd = fs.openSync(file, "wx", 0o600); + try { + fs.writeFileSync(fd, body); + } catch (error) { + fs.unlinkSync(file); + throw error; + } finally { + fs.closeSync(fd); + } + return token; +} + +export function consumeBypassReceipt(baseDir, token, toolName, args) { + if (typeof token !== "string" || !TOKEN.test(token)) return false; + let claimed; + let fd; + try { + const dir = receiptDir(baseDir); + const file = path.join(dir, `${token}.json`); + if (!fs.lstatSync(file).isFile()) return false; + const target = path.join(dir, `${randomUUID()}.claimed`); + // Claim before reading: only one consumer can rename the source file. + fs.renameSync(file, target); + claimed = target; + fd = fs.openSync(claimed, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | fs.constants.O_NONBLOCK); + const stat = fs.fstatSync(fd); + if (!stat.isFile() || !privateStat(stat) || stat.nlink !== 1 || stat.size > 4096) return false; + const buffer = Buffer.alloc(4097); + const length = fs.readSync(fd, buffer, 0, buffer.length, 0); + if (length > 4096) return false; + const receipt = JSON.parse(buffer.toString("utf8", 0, length)); + const now = Date.now(); + return Number.isSafeInteger(receipt?.ts) && receipt.ts > 0 && + receipt.ts <= now && now - receipt.ts < TTL_MS && + receipt.toolName === toolName && receipt.fingerprint === fingerprint(args); + } catch { + return false; + } finally { + if (fd !== undefined) { try { fs.closeSync(fd); } catch { /* Best-effort close. */ } } + if (claimed) { try { fs.unlinkSync(claimed); } catch { /* Already cleaned. */ } } + } +} + +export function cleanupBypassReceipts(baseDir) { + let handle; + try { + const dir = receiptDir(baseDir); + handle = fs.opendirSync(dir); + const cutoff = Date.now() - TTL_MS; + // Bound startup work, including unrelated entries. Never follow links. + for (let i = 0; i < 128; i++) { + const entry = handle.readSync(); + if (!entry) break; + if (!OWN_FILE.test(entry.name) || !entry.isFile()) continue; + try { + const file = path.join(dir, entry.name); + const stat = fs.lstatSync(file); + if (stat.isFile() && privateStat(stat) && stat.nlink === 1 && stat.mtimeMs < cutoff) fs.unlinkSync(file); + } catch { /* A concurrent consumer may have claimed it. */ } + } + } catch { /* Missing or unsafe directories need no cleanup. */ } + finally { if (handle) { try { handle.closeSync(); } catch { /* Best-effort close. */ } } } +} diff --git a/shared/glean/mcp/pluginpack.json b/shared/glean/mcp/pluginpack.json index acb6fdd..42949b0 100644 --- a/shared/glean/mcp/pluginpack.json +++ b/shared/glean/mcp/pluginpack.json @@ -2,6 +2,7 @@ "files": { "mcp/dist/index.js": "dist/index.js", "mcp/start.mjs": "start.mjs", + "mcp/approval-context.mjs": "approval-context.mjs", "mcp/package.json": "package.json", "skills/glean_run/SKILL.md": "skills/glean_run/SKILL.md" } diff --git a/shared/glean/mcp/src/index.ts b/shared/glean/mcp/src/index.ts index 0b8ead2..3c222d1 100644 --- a/shared/glean/mcp/src/index.ts +++ b/shared/glean/mcp/src/index.ts @@ -40,7 +40,13 @@ import { type DispatchContext, } from "./tools/remote-passthrough.js"; import { resolveSessionId } from "./session-id.js"; -import { serverDataDir } from "./data-dir.js"; +import { hostSharedDataDir, serverDataDir } from "./data-dir.js"; +import { cleanupBypassReceipts } from "../approval-context.mjs"; +import { + remoteElicitationOptions, + resolveToolApprovalContext, + withPermissionContext, +} from "./write-approval.js"; import { resolveServerUrlFromEmail } from "./config-search.js"; import { pluginVersionString } from "./version.js"; import { @@ -174,22 +180,12 @@ function getOAuthProvider(): GleanOAuthClientProvider { return oauthProvider; } -function getRemoteClientOpts(): RemoteClientOptions { - const supportsElicitation = !!server.getClientCapabilities()?.elicitation; +function getRemoteClientOpts( + approvalEnabled = process.env.ENABLE_HITL === "true", +): RemoteClientOptions { return { authProvider: getOAuthProvider(), - ...(supportsElicitation - ? { - elicitInput: (params, options) => - server.elicitInput( - { - message: params.message, - requestedSchema: params.requestedSchema, - }, - options, - ), - } - : {}), + ...remoteElicitationOptions(server, approvalEnabled), }; } @@ -351,7 +347,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { features: decision.features, state, }); - return { tools }; + return { tools: tools.map(withPermissionContext) }; }; // Pre-auth gate: tokens() is sync. When unauthenticated (or unconfigured) @@ -445,6 +441,7 @@ function backendErrorResult(label: string, err: unknown): CallToolResult { // reports, so DCR + the authorize request use it directly. async function connectWithSignIn( serverUrl: string, + approvalEnabled: boolean, ): Promise< { ok: true; client: RemoteClient } | { ok: false; result: CallToolResult } > { @@ -455,7 +452,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + getRemoteClientOpts(approvalEnabled), `setup-${process.pid}`, ); return { ok: true, client }; @@ -492,7 +489,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + getRemoteClientOpts(approvalEnabled), `setup-${process.pid}`, ); // Unexpectedly connected without needing auth — done. @@ -534,7 +531,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + getRemoteClientOpts(approvalEnabled), `setup-${process.pid}`, ); return { ok: true, client }; @@ -554,13 +551,13 @@ async function connectWithSignIn( * dynamic tools fetched ✓) or blocked on a user action. Used both by * `setup()` with no args and as the tail of `setup({server_url})`. */ -async function advanceSetup(): Promise { +async function advanceSetup(approvalEnabled: boolean): Promise { const serverUrl = resolveServerUrl(); if (!serverUrl) { return { content: [{ type: "text", text: SETUP_REQUIRED_TEXT }] }; } - const conn = await connectWithSignIn(serverUrl); + const conn = await connectWithSignIn(serverUrl, approvalEnabled); if (!conn.ok) return conn.result; const remoteClient = conn.client; @@ -607,7 +604,8 @@ async function advanceSetup(): Promise { } server.setRequestHandler(CallToolRequestSchema, async (request) => { - const { name, arguments: args = {} } = request.params; + const { name, arguments: input = {} } = request.params; + const { args, approvalEnabled } = resolveToolApprovalContext(name, input); // Advertisement is advisory: a host may retain a stale tool list, so every // policy withdrawal is also enforced at call time. Setup remains the @@ -650,7 +648,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { } const dispatchCtx: DispatchContext = { serverUrl, - remoteClientOpts: getRemoteClientOpts(), + remoteClientOpts: getRemoteClientOpts(approvalEnabled), authRedirectText: AUTH_REDIRECT_TO_SETUP_TEXT, logLine, }; @@ -684,7 +682,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + getRemoteClientOpts(approvalEnabled), sessionId, ); } catch (err) { @@ -746,9 +744,10 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { let remoteClient; try { + // run_tool gates locally; upstream must not add a second approval form. remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + getRemoteClientOpts(false), sessionId, ); } catch (err) { @@ -773,6 +772,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { const skillsBaseDir = resolveSkillsBaseDir(); return await handleRunTool(remoteClient, server, skillsBaseDir, args, { fileArgs: decision.features.fileArgs, + approvalEnabled, }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -885,7 +885,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { // to drive auth + tool fetch in the same call. } - return await advanceSetup(); + return await advanceSetup(approvalEnabled); } default: @@ -897,6 +897,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { }); async function main() { + cleanupBypassReceipts(hostSharedDataDir()); // Run once per session at MCP server startup. const ONE_WEEK_MS = 7 * 24 * 60 * 60 * 1000; try { diff --git a/shared/glean/mcp/src/tools/run-tool.ts b/shared/glean/mcp/src/tools/run-tool.ts index f218d28..461898a 100644 --- a/shared/glean/mcp/src/tools/run-tool.ts +++ b/shared/glean/mcp/src/tools/run-tool.ts @@ -3,12 +3,9 @@ import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; import { EmptyResultSchema } from "@modelcontextprotocol/sdk/types.js"; import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import { callRemoteTool } from "../remote-client.js"; import { FILE_ARGS_DISABLED_TEXT } from "../policy/enforce.js"; -import { resolveSessionId } from "../session-id.js"; -import { hostSharedDataDir } from "../data-dir.js"; const DEFAULT_FILE_ARG_MAX_BYTES = 5 * 1024 * 1024; @@ -287,41 +284,6 @@ 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). -// 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 { - const sessionId = resolveSessionId() - .replace(/[^a-zA-Z0-9_-]/g, "-") - .slice(0, 64); - return path.join(hostSharedDataDir(), "glean-hitl-mode", `${sessionId}.json`); -} - -// Claude Code's live permission mode for THIS session, as captured by the hook -// on the current call. Returns null when the marker is missing, unreadable, or -// malformed — the caller treats null as "unknown" and keeps the approval gate, -// 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 -// 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) -// therefore has its stale bypass marker overwritten with the resumed mode on -// the resumed session's first run_tool call, re-engaging the gate. -async function currentPermissionMode(): Promise { - try { - const raw = await fs.readFile(permissionModeMarkerPath(), "utf-8"); - const parsed = JSON.parse(raw) as { permission_mode?: unknown }; - return typeof parsed.permission_mode === "string" - ? parsed.permission_mode - : null; - } catch { - return null; - } -} - function humanizeMs(ms: number): string { const seconds = Math.round(ms / 1000); if (seconds < 120) return `${seconds}s`; @@ -357,6 +319,8 @@ export function elicitationFailureText( export interface RunToolPolicy { fileArgs: boolean; + // Resolved from trusted host context before opening the upstream connection. + approvalEnabled?: boolean; } class ToolApprovalError extends Error { @@ -482,9 +446,9 @@ export async function handleRunTool( } const remoteArgs = buildRemoteArgs(serverId, toolName, resolvedArgs); - // Read-only tools are exempt from Glean's configurable approval requirements, - // so they do not need an approval lookup. - if (isKnownReadOnlyTool(toolMetadata, serverId, toolName)) { + const approvalEnabled = policy.approvalEnabled ?? (process.env.ENABLE_HITL === "true"); + // A session opt-out is not a saved preference: skip both lookup and elicitation. + if (!approvalEnabled || isKnownReadOnlyTool(toolMetadata, serverId, toolName)) { return callRemoteTool(remoteClient, "run_tool", remoteArgs); } @@ -498,96 +462,65 @@ export async function handleRunTool( ); } - const hitlEnabled = process.env.ENABLE_HITL === "true"; - // Cursor is deliberately not excepted: current Cursor builds can use the same - // local elicitation gate as other capable hosts. Older builds that drop the - // prompt fail closed, and the timeout response explains the upgrade path. - if ( - hitlEnabled && - requiresApproval && - mcpServer.getClientCapabilities()?.elicitation - ) { - // In bypassPermissions mode (`claude --dangerously-skip-permissions`) the - // user has opted out of every approval prompt for the session, so our own - // elicitation gate is just a redundant popup — skip it and execute - // directly. The mode comes from the PreToolUse hook, which writes it keyed - // by session id immediately before this call, so it reflects the current - // call and never leaks across sessions. Any other or unknown mode keeps the - // gate. Only bypassPermissions is skipped (deliberately narrow). - const bypass = (await currentPermissionMode()) === "bypassPermissions"; - if (!bypass) { - const timeout = hitlTimeoutMs(); - - // Make a dummy empty request to burn JSON-RPC request id 0 - primeElicitationCancellation(mcpServer); - - const startedAt = Date.now(); - try { - const result = await mcpServer.elicitInput( - runToolApprovalForm(toolName), - { timeout }, - ); - const decision = approvalDecision(result); - - if (decision === approvalDeny || decision === approvalCancel) { - return { - content: [ - { - type: "text", - text: `Action ${toolName} was ${decision === approvalDeny ? "declined" : "cancelled"} by the user.`, - }, - ], - }; - } - if (decision === null) { - return { - content: [ - { - type: "text", - text: - `Action ${toolName} was not approved — the approval form ` + - `response was invalid. The action was NOT executed.`, - }, - ], - isError: true, - }; - } + // Cursor is deliberately not excepted: capable hosts share the local gate. + if (requiresApproval && mcpServer.getClientCapabilities()?.elicitation) { + const timeout = hitlTimeoutMs(); + // Burn request id 0 so cancellation can address the approval request. + primeElicitationCancellation(mcpServer); - if (decision === approvalAlwaysAllow) { - try { - await callRemoteTool(remoteClient, "set_tool_approval", { - server_id: serverId, - tool_name: toolName, - value: "ALWAYS_ALLOWED", - }); - } catch (err) { - const detail = err instanceof Error ? err.message : String(err); - console.error( - `[set_tool_approval] failed to persist "${toolName}" to Glean: ${detail}`, - ); - } - } - } catch (err) { - // Fail CLOSED. An approval gate that executes the action when the - // prompt times out or errors defeats its own purpose — and the SDK - // rejects elicitInput precisely on request timeout. - const detail = err instanceof Error ? err.message : String(err); + const startedAt = Date.now(); + try { + const result = await mcpServer.elicitInput( + runToolApprovalForm(toolName), + { timeout }, + ); + const decision = approvalDecision(result); + + if (decision === approvalDeny || decision === approvalCancel) { return { - content: [ - { - type: "text", - text: elicitationFailureText( - mcpServer, - toolName, - detail, - Date.now() - startedAt, - timeout, - ), - }, - ], + content: [{ + type: "text", + text: `Action ${toolName} was ${decision === approvalDeny ? "declined" : "cancelled"} by the user.`, + }], + }; + } + if (decision === null) { + return { + content: [{ + type: "text", + text: `Action ${toolName} was not approved — the approval form ` + + `response was invalid. The action was NOT executed.`, + }], isError: true, }; } + + if (decision === approvalAlwaysAllow) { + try { + await callRemoteTool(remoteClient, "set_tool_approval", { + server_id: serverId, + tool_name: toolName, + value: "ALWAYS_ALLOWED", + }); + } catch (err) { + const detail = err instanceof Error ? err.message : String(err); + console.error( + `[set_tool_approval] failed to persist "${toolName}" to Glean: ${detail}`, + ); + } + } + } catch (err) { + // A failed or timed-out approval must never execute the action. + const detail = err instanceof Error ? err.message : String(err); + return { + content: [{ + type: "text", + text: elicitationFailureText( + mcpServer, toolName, detail, Date.now() - startedAt, timeout, + ), + }], + isError: true, + }; } } diff --git a/shared/glean/mcp/src/write-approval.ts b/shared/glean/mcp/src/write-approval.ts new file mode 100644 index 0000000..edac901 --- /dev/null +++ b/shared/glean/mcp/src/write-approval.ts @@ -0,0 +1,59 @@ +import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import { + consumeBypassReceipt, + PERMISSION_CONTEXT_ARG, + stripPermissionContext, +} from "../approval-context.mjs"; +import { hostSharedDataDir } from "./data-dir.js"; +import type { RemoteClientOptions } from "./remote-client.js"; + +export interface ToolApprovalContext { + args: Record; + approvalEnabled: boolean; +} + +// The hook carries a one-use receipt, not a model-supplied permission mode. This +// also works when Claude does not export its session ID to the MCP process. +export function resolveToolApprovalContext( + toolName: string, + input: Record, +): ToolApprovalContext { + const args = stripPermissionContext(input); + const hitlEnabled = process.env.ENABLE_HITL === "true"; + const bypass = hitlEnabled && consumeBypassReceipt( + hostSharedDataDir(), input[PERMISSION_CONTEXT_ARG], toolName, args, + ); + return { args, approvalEnabled: hitlEnabled && !bypass }; +} + +export function withPermissionContext(tool: Tool): Tool { + return { + ...tool, + inputSchema: { + ...tool.inputSchema, + properties: { + ...tool.inputSchema.properties, + [PERMISSION_CONTEXT_ARG]: { + type: "string", + description: "Internal Claude hook context. Leave unset.", + }, + }, + }, + }; +} + +export function remoteElicitationOptions( + server: Server, + approvalEnabled: boolean, +): Pick { + // Without the callback, createRemoteClient advertises no upstream elicitation + // capability. Do not synthesize an approval or persist an Always Allow choice. + if (!approvalEnabled || !server.getClientCapabilities()?.elicitation) return {}; + return { + elicitInput: (params, options) => server.elicitInput( + { message: params.message, requestedSchema: params.requestedSchema }, + options, + ), + }; +} diff --git a/shared/glean/mcp/tests/approval-context.test.ts b/shared/glean/mcp/tests/approval-context.test.ts new file mode 100644 index 0000000..1f81862 --- /dev/null +++ b/shared/glean/mcp/tests/approval-context.test.ts @@ -0,0 +1,139 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { PERMISSION_CONTEXT_ARG as KEY, stripPermissionContext, createBypassReceipt, + consumeBypassReceipt, cleanupBypassReceipts } from "../approval-context.mjs"; + +let base: string; +const args = { server_id: "one", tool_name: "write", arguments: { body: "secret body", list: [1, 2] } }; +const dir = () => path.join(base, "glean-bypass-receipts"); +const receipt = (token: string) => path.join(dir(), `${token}.json`); +const create = () => createBypassReceipt(base, "run_tool", args); +const consume = (token: unknown, input = args, tool = "run_tool") => consumeBypassReceipt(base, token, tool, input); +beforeEach(() => { base = fs.mkdtempSync(path.join(os.tmpdir(), "approval-context-")); }); +afterEach(() => fs.rmSync(base, { recursive: true, force: true })); + +describe("bypass receipts", () => { + it("strips only the top-level context without mutating the caller", () => { + const original = { ...args, [KEY]: "forged" }; + const clean = stripPermissionContext(original); + expect(clean).toEqual(args); + expect(clean).not.toBe(original); + expect(clean.arguments).toBe(args.arguments); + expect(original[KEY]).toBe("forged"); + }); + it("stores only tool identity, a SHA256 fingerprint, and timestamp in private files", () => { + const token = create(); + expect(token).toMatch(/^[\da-f-]{36}$/); + const body = JSON.parse(fs.readFileSync(receipt(token), "utf8")); + expect(Object.keys(body).sort()).toEqual(["fingerprint", "toolName", "ts"]); + expect(body.fingerprint).toMatch(/^[\da-f]{64}$/); + expect(JSON.stringify(body)).not.toContain("secret body"); + expect(fs.statSync(receipt(token)).mode & 0o777).toBe(0o600); + expect(fs.statSync(dir()).mode & 0o777).toBe(0o700); + }); + it("canonicalizes nested object keys, excludes context, and consumes only once", () => { + const token = createBypassReceipt(base, "run_tool", { ...args, [KEY]: "old" }); + const reordered = { arguments: { list: [1, 2], body: "secret body" }, tool_name: "write", server_id: "one", [KEY]: token }; + expect(consume(token, reordered)).toBe(true); + expect(consume(token)).toBe(false); + expect(fs.readdirSync(dir())).toEqual([]); + }); + it.each(["tool", "server", "remote-tool", "extra", "arguments", "array-order"])("rejects and burns a receipt with mismatched %s", (field) => { + const token = create(); + const input = structuredClone(args); + if (field === "server") input.server_id = "two"; + if (field === "remote-tool") input.tool_name = "other"; + if (field === "extra") (input as Record).extra = true; + if (field === "arguments") input.arguments.body = "changed"; + if (field === "array-order") input.arguments.list.reverse(); + expect(consume(token, input, field === "tool" ? "other" : "run_tool")).toBe(false); + expect(consume(token)).toBe(false); + expect(fs.readdirSync(dir())).toEqual([]); + }); + it.each([undefined, null, {}, "", "../escape", "../../outside.json", `${randomUUID()}.json`, randomUUID()])( + "rejects invalid or missing receipt %j", (token) => { + expect(consume(token)).toBe(false); + expect(fs.existsSync(dir())).toBe(false); + }, + ); + it.each(["expired", "future", "string", "negative", "missing"])("rejects %s timestamps", (type) => { + const token = create(); + const body = JSON.parse(fs.readFileSync(receipt(token), "utf8")); + body.ts = { expired: Date.now() - 300_001, future: Date.now() + 60_000, + string: String(Date.now()), negative: -1, missing: undefined }[type]; + fs.writeFileSync(receipt(token), JSON.stringify(body)); + expect(consume(token)).toBe(false); + expect(fs.readdirSync(dir())).toEqual([]); + }); + it.each(["{", "null", "{}", "x".repeat(4097)])("rejects malformed or oversized receipt %#", (body) => { + const token = create(); + fs.writeFileSync(receipt(token), body); + expect(consume(token)).toBe(false); + expect(fs.readdirSync(dir())).toEqual([]); + }); + it("rejects receipt symlinks without touching their targets", () => { + const token = create(); + const target = path.join(base, "target"); + fs.renameSync(receipt(token), target); + fs.symlinkSync(target, receipt(token)); + expect(consume(token)).toBe(false); + cleanupBypassReceipts(base); + expect(fs.lstatSync(receipt(token)).isSymbolicLink()).toBe(true); + expect(fs.existsSync(target)).toBe(true); + }); + it.each(["base", "directory"])("rejects a symlinked %s", (kind) => { + const target = path.join(base, "target"); + fs.mkdirSync(target, { mode: 0o700 }); + const link = kind === "base" ? path.join(base, "linked-base") : dir(); + fs.symlinkSync(target, link); + const location = kind === "base" ? link : base; + expect(() => createBypassReceipt(location, "run_tool", args)).toThrow(); + expect(consumeBypassReceipt(location, randomUUID(), "run_tool", args)).toBe(false); + cleanupBypassReceipts(location); + expect(fs.readdirSync(target)).toEqual([]); + }); + it("rejects non-private receipt files and directories", () => { + const token = create(); + fs.chmodSync(receipt(token), 0o644); + expect(consume(token)).toBe(false); + fs.chmodSync(dir(), 0o755); + expect(() => create()).toThrow(); + }); + it("allows exactly one concurrent process to consume", async () => { + const token = create(); + const script = `import { consumeBypassReceipt } from ${JSON.stringify(new URL("../approval-context.mjs", import.meta.url).href)}; + console.log(consumeBypassReceipt(process.argv[1], process.argv[2], "run_tool", ${JSON.stringify(args)}));`; + const results = await Promise.all(Array.from({ length: 6 }, () => + promisify(execFile)(process.execPath, ["--input-type=module", "-e", script, base, token], { timeout: 5000 }))); + expect(results.filter(({ stdout }) => stdout.trim() === "true")).toHaveLength(1); + expect(fs.readdirSync(dir())).toEqual([]); + }); + it("cleans only its stale UUID receipt/claim files, not fresh or unrelated entries", () => { + const fresh = receipt(create()); + const stale = receipt(create()); + const claimed = path.join(dir(), `${randomUUID()}.claimed`); + const unrelated = path.join(dir(), "preferences.json"); + for (const file of [claimed, unrelated]) fs.writeFileSync(file, "{}", { mode: 0o600 }); + const old = new Date(Date.now() - 600_000); + for (const file of [stale, claimed, unrelated]) fs.utimesSync(file, old, old); + cleanupBypassReceipts(base); + expect(fs.readdirSync(dir()).sort()).toEqual([path.basename(fresh), "preferences.json"].sort()); + }); + it("bounds cleanup work and tolerates missing directories", () => { + cleanupBypassReceipts(base); + create(); + const old = new Date(Date.now() - 600_000); + for (let i = 0; i < 140; i++) { + const file = receipt(randomUUID()); + fs.writeFileSync(file, "{}", { mode: 0o600 }); + fs.utimesSync(file, old, old); + } + cleanupBypassReceipts(base); + expect(fs.readdirSync(dir()).length).toBeGreaterThanOrEqual(13); + }); +}); diff --git a/shared/glean/mcp/tests/approval-handler.test.ts b/shared/glean/mcp/tests/approval-handler.test.ts new file mode 100644 index 0000000..aad31a5 --- /dev/null +++ b/shared/glean/mcp/tests/approval-handler.test.ts @@ -0,0 +1,333 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { UnauthorizedError } from "@modelcontextprotocol/client"; +import type { CallToolRequest, CallToolResult, ListToolsResult } from "@modelcontextprotocol/sdk/types.js"; +import { + acceptedContent, CLIENT_CAPABILITIES_META_KEY, createMcpHandler, inputRequired, + McpServer, type ServerContext, +} from "@modelcontextprotocol/server"; +import * as z from "zod/v4"; + +// Only the host transport and authentication are fake. index.ts registers the +// handlers; its dispatch, remote client, approval policy and receipt reader run unchanged. +const mocks = vi.hoisted(() => { + const handlers = new Map(); + return { + handlers, home: "", + host: { + setRequestHandler: vi.fn((schema, handler) => handlers.set(schema.shape.method.value, handler)), + connect: vi.fn(async () => {}), request: vi.fn(async () => ({})), + getClientCapabilities: vi.fn(() => ({ elicitation: { form: {} } })), + getClientVersion: vi.fn(() => ({ name: "claude-code", version: "1" })), + elicitInput: vi.fn(), sendToolListChanged: vi.fn(async () => {}), + }, + openBrowser: vi.fn(), setPendingAuthCode: vi.fn(), + startCallbackServer: vi.fn(async () => ({ code: Promise.resolve("local-test-code") })), + closeCallbackServer: vi.fn(), + }; +}); +vi.mock("@modelcontextprotocol/sdk/server/index.js", () => ({ Server: class { + constructor() { return mocks.host; } +} })); +vi.mock("node:os", async (original) => { + const actual = await original(); + return { ...actual, homedir: () => mocks.home, + default: { ...actual, homedir: () => mocks.home } }; +}); +vi.mock("../src/auth-provider.js", () => ({ + GleanOAuthClientProvider: class { + authorizationUrl = "https://approval.invalid/local-sign-in"; + tokens() { return { access_token: "local-test-token", token_type: "Bearer" }; } + clientInformation() { return { client_id: "local-test-client" }; } + needsFreshClient() { return false; } + // Complete simulated sign-in immediately; never perform a token exchange. + setPendingAuthCode = mocks.setPendingAuthCode; + }, + openBrowser: mocks.openBrowser, +})); +vi.mock("../src/auth-callback-server.js", () => ({ + startCallbackServer: mocks.startCallbackServer, closeCallbackServer: mocks.closeCallbackServer, +})); + +const repo = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../../.."); +const endpoint = "https://approval.invalid/mcp/gateway/proxy"; +const KEY = "_glean_permission_context"; +const body = { body: "local fake write" }; +const runArgs = { server_id: "fake", tool_name: "fake_write", arguments: body }; +const text = (value: string) => ({ content: [{ type: "text" as const, text: value }] }); +type Mode = "normal" | "off" | "bypass"; +type Config = { mcpServers: Record }> }; +type Seen = { name: string; args: Record; capabilities: unknown; + state: unknown; responses: unknown }; +let buildDir: string, plugin: string, config: Config, serverName: string, namespace: string, hook: string; +let hookMatcher: RegExp; +let root: string, backend: ReturnType; +let calls: Seen[], writes: Record[], denied: string[]; +let wire: { method: string; capabilities: unknown }[]; +let authFailures: number, changeHitlAfterFailure: string | undefined; + +beforeAll(() => { + // CI runs test:bundle before build. Build from source, privately: version.test.ts + // also writes the repository bundle, and prebuild syncs tracked changelogs. + buildDir = fs.mkdtempSync(path.join(os.tmpdir(), "approval-package-")); + for (const entry of ["package.json", "pluginpack.config.ts", "CHANGELOG.md", "LICENSE", "scripts", "shared", "overrides"]) { + fs.cpSync(path.join(repo, entry), path.join(buildDir, entry), { recursive: true, + filter: (source) => !["dist", "node_modules", "tests"].includes(path.basename(source)) }); + } + fs.symlinkSync(path.join(repo, "node_modules"), path.join(buildDir, "node_modules"), "dir"); + const buildHome = path.join(buildDir, "home"); + fs.mkdirSync(buildHome); + const logPath = path.join(buildDir, "build.log"); + const log = fs.openSync(logPath, "w"); + try { + execFileSync("npm", ["run", "build"], { cwd: buildDir, timeout: 60_000, + env: { ...process.env, HOME: buildHome, USERPROFILE: buildHome, + npm_config_offline: "true", npm_config_audit: "false" }, stdio: ["ignore", log, log] }); + } catch (error) { + throw new Error(`${String(error)}\n${fs.readFileSync(logPath, "utf8").slice(-6000)}`); + } finally { fs.closeSync(log); } + plugin = path.join(buildDir, "dist/claude/plugins/glean"); + const manifest = JSON.parse(fs.readFileSync(path.join(plugin, ".claude-plugin/plugin.json"), "utf8")); + config = JSON.parse(fs.readFileSync(path.join(plugin, ".mcp.json"), "utf8")); + expect(Object.keys(config.mcpServers)).toHaveLength(1); + [serverName] = Object.keys(config.mcpServers); + expect(config.mcpServers[serverName].env.ENABLE_HITL).toBe("true"); + namespace = `mcp__plugin_${manifest.name}_${serverName}__`; + const hooks = JSON.parse(fs.readFileSync(path.join(plugin, manifest.hooks ?? "hooks/hooks.json"), "utf8")); + const matched = hooks.hooks.PreToolUse.filter((entry: { matcher: string }) => + new RegExp(entry.matcher).test(`${namespace}run_tool`)); + expect(matched).toHaveLength(1); + hookMatcher = new RegExp(matched[0].matcher); + expect(matched[0].hooks).toHaveLength(1); + expect(matched[0].hooks[0].type).toBe("command"); + const command = /^node "\$\{CLAUDE_PLUGIN_ROOT\}\/([^\"]+)"$/.exec(matched[0].hooks[0].command); + expect(command).not.toBeNull(); + hook = path.join(plugin, command![1]); + for (const file of [hook, path.join(plugin, "mcp/approval-context.mjs"), path.join(plugin, "mcp/dist/index.js")]) { + expect(fs.statSync(file).size).toBeGreaterThan(0); + } +}, 90_000); +afterAll(() => { if (buildDir) fs.rmSync(buildDir, { recursive: true, force: true }); }); + +beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + mocks.handlers.clear(); + root = fs.mkdtempSync(path.join(os.tmpdir(), "approval-handlers-")); + mocks.home = path.join(root, "home"); + fs.mkdirSync(mocks.home); + for (const [key, value] of Object.entries({ + HOME: mocks.home, USERPROFILE: mocks.home, GLEAN_MCP_SERVER_URL: endpoint, + PLUGIN_DATA_DIR: path.join(root, "server-data"), CLAUDE_PLUGIN_DATA: path.join(root, "host-data"), + SKILLS_BASE_DIR: path.join(root, "skills"), GLEAN_SESSION_ID: undefined, + CLAUDE_CODE_SESSION_ID: undefined, ENABLE_HITL: "true", GLEAN_REMOTE_TOOL_TIMEOUT_MS: "2000", + })) vi.stubEnv(key, value); + calls = []; writes = []; denied = []; wire = []; authFailures = 0; changeHitlAfterFailure = undefined; + mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Allow", approved: true } }); + vi.spyOn(console, "error").mockImplementation(() => {}); + function observe(name: string, args: Record, ctx: ServerContext) { + const envelope = ctx.mcpReq.envelope as Record | undefined; + const capabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY] as Record | undefined; + calls.push({ name, args, capabilities, state: ctx.mcpReq.requestState(), responses: ctx.mcpReq.inputResponses }); + return capabilities; + } + function execute(name: string, args: Record, ctx: ServerContext) { + const capabilities = observe(name, args, ctx); + if (capabilities?.elicitation) { + if (!ctx.mcpReq.inputResponses) return inputRequired({ + inputRequests: { approval: inputRequired.elicit({ message: "Upstream approval", + requestedSchema: { type: "object", properties: { approved: { type: "boolean" } }, required: ["approved"] } }) }, + requestState: "pending-write", + }); + if (!acceptedContent<{ approved: boolean }>(ctx.mcpReq.inputResponses, "approval")?.approved) { + return { ...text("declined"), isError: true }; + } + } + writes.push(name === "run_tool" ? args.arguments as Record : args); + return text("executed"); + } + backend = createMcpHandler(() => { + const remote = new McpServer({ name: "approval-handler-fixture", version: "1" }); + const identity = { server_id: z.string(), tool_name: z.string() }; + remote.registerTool("get_tool_approval", { inputSchema: z.strictObject(identity) }, async (args, ctx) => { + observe("get_tool_approval", args, ctx); return text('{"requires_approval":true}'); + }); + remote.registerTool("set_tool_approval", { + inputSchema: z.strictObject({ ...identity, value: z.literal("ALWAYS_ALLOWED") }), + }, async (args, ctx) => { observe("set_tool_approval", args, ctx); return text("saved locally"); }); + const payload = z.strictObject({ body: z.string() }); + remote.registerTool("run_tool", { inputSchema: z.strictObject({ ...identity, arguments: payload }) }, + async (args, ctx) => execute("run_tool", args, ctx)); + remote.registerTool("memory", { inputSchema: payload }, async (args, ctx) => execute("memory", args, ctx)); + return remote; + }); + vi.stubGlobal("fetch", vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + const request = new Request(input, init); + if (request.url !== endpoint) { denied.push(request.url); throw new Error(`Network forbidden: ${request.url}`); } + if (request.method === "POST") { + const frame = await request.clone().json(); + wire.push({ method: frame.method, capabilities: frame.params?._meta?.[CLIENT_CAPABILITIES_META_KEY] }); + if (frame.method === "server/discover" && authFailures > 0) { + authFailures -= 1; + if (changeHitlAfterFailure) vi.stubEnv("ENABLE_HITL", changeHitlAfterFailure); + throw new UnauthorizedError("Simulated expired token; no real OAuth"); + } + } + return backend.fetch(request); + })); + await import("../src/index.js"); + await vi.waitFor(() => expect(mocks.host.connect).toHaveBeenCalledTimes(1)); + expect([...mocks.handlers.keys()].sort()).toEqual(["tools/call", "tools/list"]); +}); +afterEach(async () => { + try { + await backend?.close(); + expect(denied).toEqual([]); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + vi.restoreAllMocks(); vi.unstubAllGlobals(); vi.unstubAllEnvs(); + } +}); + +// No copied dispatch: invoke the functions captured from the real SDK registration. +function call(name: string, args: Record = {}) { + const handler = mocks.handlers.get("tools/call") as (request: CallToolRequest) => Promise; + return handler({ method: "tools/call", params: { name, arguments: args } }); +} +function fromHook(mode: Mode, name = "run_tool", args: Record = runArgs) { + const cfg = structuredClone(config); + const hitl = mode === "off" ? "false" : "true"; + cfg.mcpServers[serverName].env.ENABLE_HITL = hitl; + fs.writeFileSync(path.join(plugin, ".mcp.json"), JSON.stringify(cfg)); + vi.stubEnv("ENABLE_HITL", hitl); + expect(hookMatcher.test(`${namespace}${name}`)).toBe(true); + const output = execFileSync(process.execPath, [hook], { timeout: 5000, encoding: "utf8", + cwd: root, env: { ...process.env, CLAUDE_PLUGIN_ROOT: plugin }, + input: JSON.stringify({ tool_name: `${namespace}${name}`, tool_input: args, + permission_mode: mode === "bypass" ? "bypassPermissions" : "default" }) }); + const result = JSON.parse(output).hookSpecificOutput; + expect(result.hookEventName).toBe("PreToolUse"); + expect(result.permissionDecision).toBe(mode !== "off" && name === "run_tool" ? "allow" : undefined); + expect(result.updatedInput[KEY]).toEqual(mode === "bypass" ? expect.stringMatching(/.+/) : ""); + return result.updatedInput as Record; +} +const names = () => calls.map(({ name }) => name); +function noForms() { + expect(calls.length).toBeGreaterThan(0); + for (const seen of calls) { + expect(seen.capabilities).toBeDefined(); + expect(seen.capabilities).not.toHaveProperty("elicitation"); + expect(seen.responses).toBeUndefined(); + expect(seen.args).not.toHaveProperty(KEY); + } +} + +describe("registered approval handlers with the packaged Claude hook", () => { + it("advertises optional internal context, including a closed promoted schema", async () => { + const list = mocks.handlers.get("tools/list") as () => Promise; + const { tools } = await list(); + expect(tools.map(({ name }) => name)).toEqual(expect.arrayContaining(["setup", "run_tool", "memory"])); + for (const tool of tools) { + expect(tool.inputSchema.properties?.[KEY]).toMatchObject({ type: "string" }); + expect(tool.inputSchema.required ?? []).not.toContain(KEY); + } + expect(tools.find(({ name }) => name === "memory")?.inputSchema).toMatchObject({ + additionalProperties: false, required: ["body"], properties: { body: { type: "string" } }, + }); + expect(wire.map(({ method }) => method)).toEqual(["server/discover", "tools/list"]); + }); + + it.each(["off", "bypass"] as const)("%s skips preference reads/writes and forms without session IDs", async (mode) => { + expect(process.env.GLEAN_SESSION_ID).toBeUndefined(); + expect(process.env.CLAUDE_CODE_SESSION_ID).toBeUndefined(); + const input = Object.freeze(fromHook(mode)); + expect(await call("run_tool", input)).toMatchObject(text("executed")); + expect(names()).toEqual(["run_tool"]); + expect(calls[0].args).toEqual(runArgs); + expect(writes).toEqual([body]); + expect(mocks.host.elicitInput).not.toHaveBeenCalled(); + expect(mocks.openBrowser).not.toHaveBeenCalled(); + noForms(); + }); + + it.each(["Allow", "Deny", "Always Allow"])("normal %s prompts once with no upstream gate", async (choice) => { + mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: choice } }); + const result = await call("run_tool", fromHook("normal")); + expect(mocks.host.elicitInput).toHaveBeenCalledTimes(1); + expect(mocks.host.elicitInput.mock.calls[0][0]).toMatchObject({ mode: "form" }); + expect(names()).toEqual(choice === "Deny" ? ["get_tool_approval"] : choice === "Always Allow" + ? ["get_tool_approval", "set_tool_approval", "run_tool"] : ["get_tool_approval", "run_tool"]); + expect(writes).toEqual(choice === "Deny" ? [] : [body]); + expect(JSON.stringify(result)).toContain(choice === "Deny" ? "declined" : "executed"); + if (choice === "Always Allow") expect(calls[1].args).toEqual({ + server_id: "fake", tool_name: "fake_write", value: "ALWAYS_ALLOWED", + }); + noForms(); + }); + + it("restores normal gating after bypass/off and strips replayed or forged context", async () => { + const bypass = fromHook("bypass"); + await call("run_tool", bypass); + await call("run_tool", fromHook("off")); + mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Deny" } }); + await call("run_tool", fromHook("normal", "run_tool", bypass)); + await call("run_tool", bypass); // One-use receipt cannot authorize a replay. + await call("run_tool", { ...runArgs, [KEY]: "forged" }); + expect(names()).toEqual(["run_tool", "run_tool", "get_tool_approval", "get_tool_approval", "get_tool_approval"]); + expect(mocks.host.elicitInput).toHaveBeenCalledTimes(3); + expect(writes).toEqual([body, body]); + noForms(); + }); + + it.each(["normal", "off", "bypass"] as const)("promoted memory forwards forms only in normal mode: %s", async (mode) => { + expect(await call("memory", fromHook(mode, "memory", body))).toMatchObject(text("executed")); + expect(names()).toEqual(mode === "normal" ? ["memory", "memory"] : ["memory"]); + expect(calls.every(({ args }) => JSON.stringify(args) === JSON.stringify(body))).toBe(true); + expect(writes).toEqual([body]); + expect(mocks.host.elicitInput).toHaveBeenCalledTimes(mode === "normal" ? 1 : 0); + if (mode === "normal") { + expect(calls[0].capabilities).toHaveProperty("elicitation"); + expect(calls.map(({ state }) => state)).toEqual([undefined, "pending-write"]); + expect(calls[1].responses).toMatchObject({ approval: { action: "accept", content: { approved: true } } }); + } else noForms(); + }); + + it("shares ~/.glean with the hook when only PLUGIN_DATA_DIR is set", async () => { + vi.stubEnv("CLAUDE_PLUGIN_DATA", undefined); + const input = fromHook("bypass"); + const receipts = path.join(mocks.home, ".glean/glean-bypass-receipts"); + expect(fs.readdirSync(receipts)).toHaveLength(1); + expect(fs.existsSync(path.join(process.env.PLUGIN_DATA_DIR!, "glean-bypass-receipts"))).toBe(false); + expect(await call("run_tool", input)).toMatchObject(text("executed")); + expect(fs.readdirSync(receipts)).toHaveLength(0); + expect(names()).toEqual(["run_tool"]); + expect(mocks.host.elicitInput).not.toHaveBeenCalled(); + noForms(); + }); + + it.each(["normal", "off", "bypass"] as const)("setup retains %s approval policy through all reconnects", async (mode) => { + const input = fromHook(mode, "setup", {}); + authFailures = 2; // Authenticated attempt, then callback-listener attempt, then post-sign-in reconnect. + changeHitlAfterFailure = mode === "normal" ? "false" : "true"; + expect(JSON.stringify(await call("setup", input))).toContain("Glean setup is complete"); + expect(wire.map(({ method }) => method)).toEqual([ + "server/discover", "server/discover", "server/discover", "tools/list", + ]); + for (const request of wire) { + expect(request.capabilities).toBeDefined(); + if (mode === "normal") expect(request.capabilities).toHaveProperty("elicitation"); + else expect(request.capabilities).not.toHaveProperty("elicitation"); + } + expect(mocks.startCallbackServer).toHaveBeenCalledTimes(1); + expect(mocks.closeCallbackServer).toHaveBeenCalledTimes(1); + expect(mocks.openBrowser).toHaveBeenCalledExactlyOnceWith("https://approval.invalid/local-sign-in"); + expect(mocks.setPendingAuthCode).toHaveBeenCalledExactlyOnceWith("local-test-code"); + expect(mocks.host.elicitInput).not.toHaveBeenCalled(); + expect(calls).toEqual([]); + expect(writes).toEqual([]); + }); +}); diff --git a/shared/glean/mcp/tests/auto-approve-hook.test.ts b/shared/glean/mcp/tests/auto-approve-hook.test.ts index 2b647eb..c63ae2b 100644 --- a/shared/glean/mcp/tests/auto-approve-hook.test.ts +++ b/shared/glean/mcp/tests/auto-approve-hook.test.ts @@ -1,175 +1,114 @@ -import { describe, it, expect } from "vitest"; -import { spawn } from "node:child_process"; -import fs from "node:fs/promises"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; import path from "node:path"; import os from "node:os"; import { fileURLToPath } from "node:url"; +import { PERMISSION_CONTEXT_ARG as KEY, createBypassReceipt, consumeBypassReceipt } from "../approval-context.mjs"; const here = path.dirname(fileURLToPath(import.meta.url)); -const HOOK = path.resolve( - here, - "../../../../overrides/claude/glean/hooks/auto-approve-run-tool.mjs", -); - -interface HookResult { - out: string; - // Parsed contents of the single permission-mode marker the hook wrote, or - // null when none was written. markerFiles lists the filenames present. - marker: { permission_mode?: string; ts?: number } | null; - markerFiles: string[]; -} - -async function runHook( - toolName: string, - env: Record, - extraInput: Record = {}, - seed?: { sessionId: string; mode: string }, -): Promise { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "approve-hook-")); - await fs.writeFile( - path.join(root, ".mcp.json"), - JSON.stringify({ mcpServers: { "glean_plugin": { env } } }), - ); - // Isolate the marker under a throwaway CLAUDE_PLUGIN_DATA so the hook never - // touches the developer's real ~/.glean during tests. - const dataDir = path.join(root, "plugin-data"); - // Optionally pre-seed a leftover marker (e.g. from a prior - // --dangerously-skip-permissions session) to prove the hook overwrites it. - if (seed) { - const dir = path.join(dataDir, "glean-hitl-mode"); - await fs.mkdir(dir, { recursive: true }); - await fs.writeFile( - path.join(dir, `${seed.sessionId}.json`), - JSON.stringify({ permission_mode: seed.mode, ts: 0 }), - ); - } - try { - const out = await new Promise((resolve, reject) => { - const child = spawn("node", [HOOK], { - env: { - ...process.env, - CLAUDE_PLUGIN_ROOT: root, - CLAUDE_PLUGIN_DATA: dataDir, - }, - }); - let o = ""; - child.stdout.on("data", (d) => (o += d.toString())); - child.on("error", reject); - child.on("close", () => resolve(o)); - child.stdin.write(JSON.stringify({ tool_name: toolName, ...extraInput })); - child.stdin.end(); - }); - - let markerFiles: string[] = []; - let marker: HookResult["marker"] = null; - try { - const dir = path.join(dataDir, "glean-hitl-mode"); - markerFiles = await fs.readdir(dir); - if (markerFiles.length) { - marker = JSON.parse( - await fs.readFile(path.join(dir, markerFiles[0]), "utf-8"), - ); - } - } catch { - // No marker directory: nothing was written. - } - return { out, marker, markerFiles }; - } finally { - await fs.rm(root, { recursive: true, force: true }); - } -} - +const HOOK = path.resolve(here, "../../../../overrides/claude/glean/hooks/auto-approve-run-tool.mjs"); const glean = (tool: string) => `mcp__plugin_local-mcp_glean_plugin__${tool}`; -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 () => { - const { out } = await runHook(glean("run_tool"), hitlOn); - expect(JSON.parse(out).hookSpecificOutput.permissionDecision).toBe("allow"); +const args = { server_id: "server", tool_name: "send_message", arguments: { body: "private body" } }; +let root: string, dataDir: string; +function configure(hitl = "true", server = "glean_plugin", extra = {}) { + fs.writeFileSync(path.join(root, ".mcp.json"), JSON.stringify({ + mcpServers: { [server]: { command: "node", env: { ENABLE_HITL: hitl }, ...extra } }, + })); +} +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), "approve-hook-")); + dataDir = path.join(root, "plugin-data"); + fs.mkdirSync(path.join(root, ".claude-plugin")); + fs.writeFileSync(path.join(root, ".claude-plugin/plugin.json"), JSON.stringify({ name: "local-mcp" })); + fs.mkdirSync(path.join(root, "mcp")); + fs.copyFileSync(path.resolve(here, "../approval-context.mjs"), path.join(root, "mcp/approval-context.mjs")); + configure(); +}); +afterEach(() => fs.rmSync(root, { recursive: true, force: true })); +function runHook(tool = glean("run_tool"), extra: Record = {}, raw?: string) { + const env: NodeJS.ProcessEnv = { ...process.env, CLAUDE_PLUGIN_ROOT: root, CLAUDE_PLUGIN_DATA: dataDir }; + delete env.CLAUDE_CODE_SESSION_ID; + const out = execFileSync(process.execPath, [HOOK], { + env, encoding: "utf8", input: raw ?? JSON.stringify({ tool_name: tool, tool_input: args, ...extra }), }); - - it("never allows when HITL is off (safety)", async () => { - const { out } = await runHook(glean("run_tool"), hitlOff); - expect(out.trim()).toBe(""); + return out ? JSON.parse(out).hookSpecificOutput : null; +} +const bypass = { permission_mode: "bypassPermissions" }; + +describe("request-local approval hook", () => { + it("creates a usable receipt without any host session id", () => { + const result = runHook(glean("run_tool"), bypass); + expect(result.permissionDecision).toBe("allow"); + expect(result.hookEventName).toBe("PreToolUse"); + expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(true); + expect(fs.readdirSync(dataDir)).toEqual(["glean-bypass-receipts"]); }); - - it("ignores a non-glean run_tool (scoped to this plugin)", async () => { - const { out } = await runHook("mcp__other-server__run_tool", hitlOn); - expect(out.trim()).toBe(""); + it.each([undefined, null, "", "default", "acceptEdits", "plan", "dontAsk", "bypasspermissions", {}, true])( + "sanitizes forged context for absent, normal, or invalid mode %j", (permission_mode) => { + const result = runHook(glean("run_tool"), { permission_mode, tool_input: { ...args, [KEY]: "forged" } }); + expect(result.updatedInput).toEqual({ ...args, [KEY]: "" }); + expect(result.permissionDecision).toBe("allow"); + expect(fs.existsSync(dataDir)).toBe(false); + }, + ); + it.each(["resumed-session", "other-session"])("sanitizes a valid token in normal %s", (session_id) => { + const token = createBypassReceipt(dataDir, "run_tool", args); + const result = runHook(glean("run_tool"), { + permission_mode: "default", session_id, tool_input: { ...args, [KEY]: token }, + }); + expect(result.updatedInput[KEY]).toBe(""); + expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(false); }); - - it("ignores glean tools other than run_tool (e.g. find_skills_and_tools)", async () => { - const { out } = await runHook(glean("find_skills_and_tools"), hitlOn); - expect(out.trim()).toBe(""); + it("replaces supplied context even in bypass mode", () => { + const token = createBypassReceipt(dataDir, "run_tool", args); + const result = runHook(glean("run_tool"), { ...bypass, tool_input: { ...args, [KEY]: token } }); + expect(result.updatedInput[KEY]).not.toBe(token); + expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(true); }); -}); - -describe("auto-approve-run-tool 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"), - hitlOn, - bypass, - ); - expect(marker).toMatchObject({ permission_mode: "bypassPermissions" }); - expect(typeof marker?.ts).toBe("number"); - expect(markerFiles).toContain("sess-1.json"); + it.each(["find_skills_and_tools", "read_document"])("hands off bypass for %s without native approval", (tool) => { + const result = runHook(glean(tool), bypass); + expect(result.permissionDecision).toBeUndefined(); + expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], tool, args)).toBe(true); }); - - it("keys the marker file by session id (parallel sessions don't collide)", async () => { - const { markerFiles } = await runHook(glean("run_tool"), hitlOn, { - permission_mode: "default", - session_id: "other-session", - }); - expect(markerFiles).toEqual(["other-session.json"]); + it.each(["run_tool", "find_skills_and_tools"])("neither approves nor writes a receipt for HITL-off %s", (tool) => { + configure("false"); + const result = runHook(glean(tool), { ...bypass, tool_input: { ...args, [KEY]: "forged" } }); + expect(result.updatedInput[KEY]).toBe(""); + expect(result.permissionDecision).toBeUndefined(); + expect(fs.existsSync(dataDir)).toBe(false); }); - - it("does not write a marker when HITL is off", async () => { - const { out, marker } = await runHook(glean("run_tool"), hitlOff, bypass); - expect(out.trim()).toBe(""); - expect(marker).toBeNull(); + it.each(["mcp__glean_default__run_tool", "mcp__plugin_other_glean_plugin__run_tool", + "mcp__plugin_local-mcp_glean_plugin_other__run_tool", "mcp__other-server__run_tool"])( + "ignores unrelated server %s", (tool) => { + expect(runHook(tool, bypass)).toBeNull(); + expect(fs.existsSync(dataDir)).toBe(false); + }, + ); + it("derives both namespace components from trusted configuration", () => { + fs.writeFileSync(path.join(root, ".claude-plugin/plugin.json"), '{"name":"renamed-plugin"}'); + configure("true", "local-server"); + expect(runHook("mcp__plugin_renamed-plugin_local-server__run_tool").permissionDecision).toBe("allow"); + expect(runHook()).toBeNull(); }); - - it("does not write a marker for a non-glean run_tool", async () => { - const { marker } = await runHook( - "mcp__other-server__run_tool", - hitlOn, - bypass, - ); - expect(marker).toBeNull(); + it("does not approve a remote server configured in the plugin", () => { + configure("true", "glean_plugin", { type: "http", url: "https://example.test/mcp" }); + expect(runHook()).toBeNull(); }); - - it("writes no marker when permission_mode is absent from the payload", async () => { - 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(marker).toBeNull(); + it("requires the plugin manifest", () => { + fs.unlinkSync(path.join(root, ".claude-plugin/plugin.json")); + expect(runHook(glean("run_tool"), bypass)).toBeNull(); }); - - it("sanitizes the session id used for the marker filename", async () => { - const { markerFiles } = await runHook(glean("run_tool"), hitlOn, { - permission_mode: "default", - session_id: "weird/../id with spaces", - }); - expect(markerFiles).toHaveLength(1); - expect(markerFiles[0]).toMatch(/^[a-zA-Z0-9_-]+\.json$/); + it.each(["unwritable", "missing-helper"])("fails toward the gate when %s", (failure) => { + if (failure === "unwritable") fs.writeFileSync(dataDir, "blocked"); + else fs.unlinkSync(path.join(root, "mcp/approval-context.mjs")); + expect(runHook(glean("run_tool"), bypass).updatedInput[KEY]).toBe(""); }); - - it("overwrites a leftover bypass marker when the session is resumed without the flag", async () => { - // Session was first launched with --dangerously-skip-permissions (leftover - // marker = bypassPermissions), then resumed WITHOUT the flag (current mode - // = default). The hook rewrites the same per-session marker, clearing the - // stale bypass so the server re-engages its gate on this call. - const { marker } = await runHook( - glean("run_tool"), - hitlOn, - { permission_mode: "default", session_id: "sess-1" }, - { sessionId: "sess-1", mode: "bypassPermissions" }, - ); - expect(marker).toMatchObject({ permission_mode: "default" }); + it("ignores malformed JSON", () => expect(runHook(undefined, {}, "{")).toBeNull()); + it("matches all plugin tools but not non-plugin tools", () => { + const config = JSON.parse(fs.readFileSync(path.join(path.dirname(HOOK), "hooks.json"), "utf8")); + const matcher = new RegExp(config.hooks.PreToolUse[0].matcher); + expect(matcher.test(glean("find_skills_and_tools"))).toBe(true); + expect(matcher.test("mcp__glean_default__run_tool")).toBe(false); }); }); diff --git a/shared/glean/mcp/tests/run-tool.test.ts b/shared/glean/mcp/tests/run-tool.test.ts index 17a6339..a2fd483 100644 --- a/shared/glean/mcp/tests/run-tool.test.ts +++ b/shared/glean/mcp/tests/run-tool.test.ts @@ -332,8 +332,7 @@ async function writeToolJson( ); } -// Mirrors the marker the PreToolUse hook writes: /glean-hitl-mode/ -// .json. The server reads it via CLAUDE_PLUGIN_DATA + GLEAN_SESSION_ID. +// Legacy session markers must no longer grant bypass to a current request. async function writeModeMarker( dataDir: string, sessionId: string, @@ -1422,20 +1421,21 @@ describe("handleRunTool (HITL)", () => { ]); }); - it("skips the elicitation gate and executes directly in bypassPermissions mode", async () => { + it("skips approval lookup and persistence for a request-local bypass", async () => { vi.stubEnv("ENABLE_HITL", "true"); - vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); - vi.stubEnv("GLEAN_SESSION_ID", "sess-bypass"); await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); - await writeModeMarker(tmpDir, "sess-bypass", "bypassPermissions"); const remote = makeRemote(); - const elicit = vi.fn().mockResolvedValue({ action: "accept" }); + const elicit = vi.fn().mockResolvedValue(approvalResult("Always Allow")); const server = makeServer({ elicitation: true, elicit }); - await handleRunTool(remote, server, tmpDir, baseArgs, ALL_ON); + await handleRunTool(remote, server, tmpDir, baseArgs, { + ...ALL_ON, + approvalEnabled: false, + }); expect(elicit).not.toHaveBeenCalled(); expect(remote.downstreamCall).toHaveBeenCalledTimes(1); + expect(remote.callTool.mock.calls.map((call: any) => call[0].name)).toEqual(["run_tool"]); }); it("still elicits when the session's permission mode is not bypass", async () => { @@ -1469,13 +1469,13 @@ describe("handleRunTool (HITL)", () => { expect(elicit).toHaveBeenCalledTimes(1); }); - it("ignores a bypass marker written for a different session (no cross-session leak)", async () => { + it("ignores legacy bypass markers, even for the same session", async () => { vi.stubEnv("ENABLE_HITL", "true"); vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); vi.stubEnv("GLEAN_SESSION_ID", "sess-A"); await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); - // Another concurrent session opted into bypass; ours did not. - await writeModeMarker(tmpDir, "sess-B", "bypassPermissions"); + // Old on-disk session state must not grant approval to a new request. + await writeModeMarker(tmpDir, "sess-A", "bypassPermissions"); const remote = makeRemote(); const elicit = allowOnce(); const server = makeServer({ elicitation: true, elicit }); diff --git a/shared/glean/mcp/tests/write-approval.test.ts b/shared/glean/mcp/tests/write-approval.test.ts new file mode 100644 index 0000000..cad924c --- /dev/null +++ b/shared/glean/mcp/tests/write-approval.test.ts @@ -0,0 +1,292 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import type { Client } from "@modelcontextprotocol/client"; +import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import { + acceptedContent, CLIENT_CAPABILITIES_META_KEY, createMcpHandler, inputRequired, + McpServer, type CallToolResult, type InputRequiredResult, type ServerContext, +} from "@modelcontextprotocol/server"; +import * as z from "zod/v4"; +import { createBypassReceipt, PERMISSION_CONTEXT_ARG as KEY } from "../approval-context.mjs"; +import { callRemoteTool, createRemoteClient } from "../src/remote-client.js"; +import { handleRunTool } from "../src/tools/run-tool.js"; +import { + remoteElicitationOptions, resolveToolApprovalContext, withPermissionContext, +} from "../src/write-approval.js"; + +const body = { body: "fake write only" }; +const runArgs = { server_id: "test", tool_name: "fake_write", arguments: body }; +const text = (value: string): CallToolResult => ({ content: [{ type: "text", text: value }] }); +const errors = ["403 Forbidden", "401 [AUTHENTICATION_REQUIRED]"]; +type Mode = "normal" | "off" | "bypass"; +type ObservedCall = { + name: string; + args: Record; + capabilities: Record | undefined; + state: string | undefined; + responses: Record | undefined; +}; + +function makeHost() { + return { + getClientCapabilities: vi.fn(() => ({ elicitation: { form: {} } })), + getClientVersion: vi.fn(() => ({ name: "claude-code", version: "1" })), + elicitInput: vi.fn().mockResolvedValue({ + action: "accept", content: { approval: "Allow", approved: true }, + }), + request: vi.fn().mockResolvedValue({}), + }; +} + +describe("write approval boundary (in-memory modern MCP)", () => { + let base: string; + let host: ReturnType; + let handler: ReturnType; + let clients: Client[]; + let calls: ObservedCall[]; + let writes: Record[]; + const server = () => host as unknown as Server; + const names = () => calls.map((call) => call.name); + const withoutUpstreamForms = () => { + expect(calls.length).toBeGreaterThan(0); + for (const call of calls) { + expect(call.capabilities).toBeDefined(); + expect(call.capabilities).not.toHaveProperty("elicitation"); + expect(call.responses).toBeUndefined(); + expect(call.args).not.toHaveProperty(KEY); + } + }; + + beforeEach(async () => { + base = await fs.mkdtemp(path.join(os.tmpdir(), "write-approval-")); + vi.stubEnv("CLAUDE_PLUGIN_DATA", base); + vi.stubEnv("PLUGIN_DATA_DIR", base); + vi.stubEnv("GLEAN_SESSION_ID", undefined); + vi.stubEnv("CLAUDE_CODE_SESSION_ID", undefined); + vi.stubEnv("ENABLE_HITL", "true"); + vi.stubGlobal("fetch", vi.fn(() => { throw new Error("Unexpected network access"); })); + host = makeHost(); + clients = []; + calls = []; + writes = []; + function observe(name: string, args: Record, ctx: ServerContext) { + // Modern capabilities are per-request envelope metadata, not ctx.clientCapabilities. + const envelope = ctx.mcpReq.envelope as Record | undefined; + const capabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY] as Record | undefined; + calls.push({ name, args, capabilities, state: ctx.mcpReq.requestState(), + responses: ctx.mcpReq.inputResponses }); + return capabilities; + } + function execute(name: string, args: Record, ctx: ServerContext): CallToolResult | InputRequiredResult { + const capabilities = observe(name, args, ctx); + const payload = name === "run_tool" ? args.arguments as Record : args; + if (errors.includes(String(payload.body))) return { ...text(String(payload.body)), isError: true }; + if (capabilities?.elicitation) { + if (!ctx.mcpReq.inputResponses) { + return inputRequired({ + inputRequests: { approval: inputRequired.elicit({ + message: "Remote approval", + requestedSchema: { type: "object", properties: { approved: { type: "boolean" } }, required: ["approved"] }, + }) }, + requestState: "approval-state", + }); + } + if (!acceptedContent<{ approved: boolean }>(ctx.mcpReq.inputResponses, "approval")?.approved) { + return { ...text("Remote approval declined"), isError: true }; + } + } + writes.push(payload); + return text("executed"); + } + handler = createMcpHandler(() => { + const remote = new McpServer({ name: "approval-test", version: "1" }); + const identity = { server_id: z.string(), tool_name: z.string() }; + remote.registerTool("get_tool_approval", { inputSchema: z.strictObject(identity) }, async (args, ctx) => { + observe("get_tool_approval", args, ctx); + return text(JSON.stringify({ requires_approval: true })); + }); + remote.registerTool("set_tool_approval", { + inputSchema: z.strictObject({ ...identity, value: z.literal("ALWAYS_ALLOWED") }), + }, async (args, ctx) => { + observe("set_tool_approval", args, ctx); + return text("saved in fake backend only"); + }); + const payload = z.strictObject({ body: z.string() }); + remote.registerTool("run_tool", { + inputSchema: z.strictObject({ ...identity, arguments: payload }), + }, async (args, ctx) => execute("run_tool", args, ctx)); + remote.registerTool("memory", { inputSchema: payload }, async (args, ctx) => execute("memory", args, ctx)); + return remote; + }); + }); + + afterEach(async () => { + try { + await Promise.all(clients.map((client) => client.close())); + await handler.close(); + expect(globalThis.fetch).not.toHaveBeenCalled(); + } finally { + await fs.rm(base, { recursive: true, force: true }); + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); + } + }); + + function inputFor(mode: Mode, name = "run_tool", args: Record = runArgs) { + vi.stubEnv("ENABLE_HITL", mode === "off" ? "false" : "true"); + return { ...args, [KEY]: mode === "bypass" ? createBypassReceipt(base, name, args) : "invalid-context" }; + } + + // Compose the exported boundaries without importing index.ts and starting stdio/OAuth. + async function dispatch(name: string, input: Record, fileArgs = true) { + const { args, approvalEnabled } = resolveToolApprovalContext(name, input); + const remote = await createRemoteClient("http://approval.test/mcp", { + fetch: (url, init) => handler.fetch(new Request(url, init)), + // run_tool owns one LOCAL gate; only promoted tools forward upstream forms. + ...remoteElicitationOptions(server(), name === "run_tool" ? false : approvalEnabled), + }); + clients.push(remote); + expect(remote.getProtocolEra()).toBe("modern"); + return name === "run_tool" + ? handleRunTool(remote, server(), base, args, { fileArgs, approvalEnabled }) + : callRemoteTool(remote, name, args); + } + + it.each(["off", "bypass"] as const)("%s executes without forms or preference reads/writes, even with no session ID", async (mode) => { + const input = Object.freeze(inputFor(mode)); + expect(process.env.GLEAN_SESSION_ID).toBeUndefined(); + expect(process.env.CLAUDE_CODE_SESSION_ID).toBeUndefined(); + expect(host.getClientCapabilities()).toHaveProperty("elicitation.form"); + const result = await dispatch("run_tool", input); + expect(result.content).toEqual(text("executed").content); + expect(result.isError).not.toBe(true); + expect(names()).toEqual(["run_tool"]); + expect(calls[0].args).toEqual(runArgs); + expect(input).toHaveProperty(KEY); + expect(writes).toEqual([body]); + expect(host.elicitInput).not.toHaveBeenCalled(); + withoutUpstreamForms(); + }); + + it.each(["Allow", "Deny", "Always Allow"])("normal %s uses exactly one local prompt, never a second upstream gate", async (choice) => { + host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: choice } }); + const result = await dispatch("run_tool", inputFor("normal")); + expect(host.elicitInput).toHaveBeenCalledTimes(1); + expect(host.elicitInput.mock.calls[0][0]).toMatchObject({ mode: "form" }); + expect(names()).toEqual(choice === "Deny" ? ["get_tool_approval"] + : choice === "Always Allow" ? ["get_tool_approval", "set_tool_approval", "run_tool"] + : ["get_tool_approval", "run_tool"]); + expect(writes).toEqual(choice === "Deny" ? [] : [body]); + expect(JSON.stringify(result)).toContain(choice === "Deny" ? "declined" : "executed"); + if (choice === "Always Allow") expect(calls[1].args).toEqual({ + server_id: "test", tool_name: "fake_write", value: "ALWAYS_ALLOWED", + }); + withoutUpstreamForms(); + }); + + it("resolves each call afresh across mode changes, new receipts, replays, and sessions", async () => { + const first = inputFor("bypass"); + await dispatch("run_tool", first); + expect(host.elicitInput).not.toHaveBeenCalled(); + vi.stubEnv("GLEAN_SESSION_ID", "session-two"); + vi.stubEnv("CLAUDE_CODE_SESSION_ID", "claude-two"); + await dispatch("run_tool", runArgs); + await dispatch("run_tool", first); // A consumed receipt cannot authorize another call/session. + expect(host.elicitInput).toHaveBeenCalledTimes(2); + const second = inputFor("bypass"); + expect(second[KEY]).not.toBe(first[KEY]); + await dispatch("run_tool", second); + await dispatch("run_tool", inputFor("off")); + expect(host.elicitInput).toHaveBeenCalledTimes(2); + await dispatch("run_tool", inputFor("normal")); + vi.stubEnv("GLEAN_SESSION_ID", undefined); + vi.stubEnv("CLAUDE_CODE_SESSION_ID", undefined); + await dispatch("run_tool", runArgs); + expect(host.elicitInput).toHaveBeenCalledTimes(4); + expect(names().filter((name) => name === "get_tool_approval")).toHaveLength(4); + expect(names()).not.toContain("set_tool_approval"); + expect(writes).toHaveLength(7); + withoutUpstreamForms(); + }); + + it.each([undefined, "bypassPermissions", { permission_mode: "bypassPermissions" }, "00000000-0000-4000-8000-000000000000"])( + "invalid context %j stays in normal approval mode", async (context) => { + host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Deny" } }); + const input = Object.freeze({ ...runArgs, [KEY]: context }); + expect(resolveToolApprovalContext("run_tool", input)).toEqual({ args: runArgs, approvalEnabled: true }); + await dispatch("run_tool", input); + expect(names()).toEqual(["get_tool_approval"]); + expect(host.elicitInput).toHaveBeenCalledTimes(1); + expect(writes).toEqual([]); + expect(input[KEY]).toEqual(context); + withoutUpstreamForms(); + }, + ); + + it.each(["normal", "off", "bypass"] as const)("promoted tool forwards forms only when enabled: %s", async (mode) => { + const input = inputFor(mode, "memory", body); + const result = await dispatch("memory", input); + expect(result.content).toEqual(text("executed").content); + expect(result.isError).not.toBe(true); + expect(host.elicitInput).toHaveBeenCalledTimes(mode === "normal" ? 1 : 0); + expect(names()).toEqual(mode === "normal" ? ["memory", "memory"] : ["memory"]); + expect(calls.every((call) => JSON.stringify(call.args) === JSON.stringify(body))).toBe(true); + expect(writes).toEqual([body]); + if (mode === "normal") { + expect(calls[0].capabilities).toHaveProperty("elicitation"); + expect(calls.map((call) => call.state)).toEqual([undefined, "approval-state"]); + expect(calls[1].responses).toMatchObject({ approval: { action: "accept", content: { approved: true } } }); + expect(host.elicitInput.mock.calls[0][0]).toMatchObject({ message: "Remote approval" }); + } else withoutUpstreamForms(); + }); + + it.each(["off", "bypass"] as const)("%s preserves forbidden/auth errors, without auto-accepting or retrying", async (mode) => { + for (const error of errors) for (const name of ["run_tool", "memory"]) { + const payload = { body: error }; + const args = name === "run_tool" ? { ...runArgs, arguments: payload } : payload; + expect(await dispatch(name, inputFor(mode, name, args))).toMatchObject({ ...text(error), isError: true }); + } + expect(names()).toEqual(["run_tool", "memory", "run_tool", "memory"]); + expect(writes).toEqual([]); + expect(host.elicitInput).not.toHaveBeenCalled(); + withoutUpstreamForms(); + }); + + it.each([ + { fileArgs: false, extra: { file_args: { body: "/never-read" } }, message: "`file_args` is disabled" }, + { fileArgs: true, extra: { file_args: { body: "relative-path" } }, message: "absolute" }, + { fileArgs: true, extra: { server_id: null }, message: "required strings" }, + ])("disabled approval preserves the $message guard", async ({ fileArgs, extra, message }) => { + const result = await dispatch("run_tool", inputFor("off", "run_tool", { ...runArgs, ...extra }), fileArgs); + expect(result.isError).toBe(true); + expect(JSON.stringify(result)).toContain(message); + expect(calls).toEqual([]); + expect(writes).toEqual([]); + expect(host.elicitInput).not.toHaveBeenCalled(); + }); + + it("adds optional context to closed schemas without mutating the original tool", () => { + const tool: Tool = { name: "memory", inputSchema: { + type: "object", properties: { body: { type: "string" } }, required: ["body"], additionalProperties: false, + } }; + const original = structuredClone(tool); + const augmented = withPermissionContext(tool); + expect(tool).toEqual(original); + expect(augmented).not.toBe(tool); + expect(augmented.inputSchema).not.toBe(tool.inputSchema); + expect(augmented.inputSchema.properties).not.toBe(tool.inputSchema.properties); + expect(augmented.inputSchema.additionalProperties).toBe(false); + expect(augmented.inputSchema.required).toEqual(["body"]); + expect(augmented.inputSchema.properties).toEqual({ ...tool.inputSchema.properties, + [KEY]: { type: "string", description: expect.any(String) }, + }); + }); + + it("does not advertise upstream forms when the host has no elicitation support", () => { + host.getClientCapabilities.mockReturnValue({} as ReturnType); + expect(remoteElicitationOptions(server(), true)).toEqual({}); + }); +}); From cdd86fffcd574401af552df73b8f173f4ff59305 Mon Sep 17 00:00:00 2001 From: Pragati Agrawal Date: Mon, 28 Sep 2026 18:51:56 +0530 Subject: [PATCH 2/2] fix: narrow HITL change to the shared approval guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the session-context protocol and broad test harness. Reuse the existing permission-mode lookup to skip local and upstream approval when HITL is off or the reported mode is bypassPermissions. — sent via Glean Tau --- .../glean/hooks/auto-approve-run-tool.mjs | 125 +++++-- overrides/claude/glean/hooks/hooks.json | 2 +- shared/glean/mcp/approval-context.d.mts | 5 - shared/glean/mcp/approval-context.mjs | 116 ------ shared/glean/mcp/pluginpack.json | 1 - shared/glean/mcp/src/index.ts | 59 ++-- shared/glean/mcp/src/tools/run-tool.ts | 192 ++++++---- shared/glean/mcp/src/write-approval.ts | 59 ---- .../glean/mcp/tests/approval-context.test.ts | 139 -------- .../glean/mcp/tests/approval-handler.test.ts | 333 ------------------ .../glean/mcp/tests/auto-approve-hook.test.ts | 253 ++++++++----- shared/glean/mcp/tests/run-tool.test.ts | 45 ++- shared/glean/mcp/tests/write-approval.test.ts | 292 --------------- 13 files changed, 444 insertions(+), 1177 deletions(-) delete mode 100644 shared/glean/mcp/approval-context.d.mts delete mode 100644 shared/glean/mcp/approval-context.mjs delete mode 100644 shared/glean/mcp/src/write-approval.ts delete mode 100644 shared/glean/mcp/tests/approval-context.test.ts delete mode 100644 shared/glean/mcp/tests/approval-handler.test.ts delete mode 100644 shared/glean/mcp/tests/write-approval.test.ts diff --git a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs b/overrides/claude/glean/hooks/auto-approve-run-tool.mjs index ecb3670..366fd69 100644 --- a/overrides/claude/glean/hooks/auto-approve-run-tool.mjs +++ b/overrides/claude/glean/hooks/auto-approve-run-tool.mjs @@ -1,48 +1,101 @@ #!/usr/bin/env node -// PreToolUse: carry request-local bypass context; only run_tool gets native approval. +// 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. +// +// 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. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { pathToFileURL } from "node:url"; +function readStdin() { + try { + return fs.readFileSync(0, "utf-8"); + } catch { + return ""; + } +} + +let input = {}; try { - const input = JSON.parse(fs.readFileSync(0, "utf8")); - const root = process.env.CLAUDE_PLUGIN_ROOT; - if (!root || typeof input?.tool_name !== "string") process.exit(0); - const manifest = JSON.parse(fs.readFileSync(path.join(root, ".claude-plugin/plugin.json"), "utf8")); - const cfg = JSON.parse(fs.readFileSync(path.join(root, ".mcp.json"), "utf8")); - if (typeof manifest.name !== "string" || !/^[a-zA-Z0-9_-]+$/.test(manifest.name)) process.exit(0); + input = JSON.parse(readStdin()); +} catch { + // Malformed/empty input: do nothing, let the normal permission flow run. +} - for (const [server, config] of Object.entries(cfg.mcpServers ?? {})) { - if (!/^[a-zA-Z0-9_-]+$/.test(server) || !config || - typeof config.command !== "string" || !config.command || config.url || - (config.type && config.type !== "stdio")) continue; - const prefix = `mcp__plugin_${manifest.name}_${server}__`; - if (!input.tool_name.startsWith(prefix)) continue; - const bareName = input.tool_name.slice(prefix.length); - if (!/^[a-zA-Z0-9_-]+$/.test(bareName)) continue; +const toolName = String(input.tool_name ?? ""); +const bareName = toolName.split("__").pop() ?? ""; +// Scope strictly to this plugin's run_tool — the tool name carries the glean +// plugin/server prefix (e.g. mcp__plugin_local-mcp_glean_plugin__run_tool). +if (!toolName.includes("glean") || bareName !== "run_tool") { + process.exit(0); +} + +// The hook process does not inherit the MCP server's env, so read the flag +// from the plugin's own .mcp.json. +let env = {}; +try { + const root = process.env.CLAUDE_PLUGIN_ROOT ?? "."; + const cfg = JSON.parse(fs.readFileSync(path.join(root, ".mcp.json"), "utf-8")); + // The server is named "glean_plugin" in the shipped .mcp.json (namespaced + // under "glean" so it's recognizable alongside a user-connected remote + // Glean server, e.g. "glean_default"). Keep fallbacks to the prior names + // ("glean-local", then the original "glean") for backwards compatibility + // with older installs. + env = + cfg?.mcpServers?.["glean_plugin"]?.env ?? + cfg?.mcpServers?.["glean-local"]?.env ?? + cfg?.mcpServers?.glean?.env ?? + {}; +} catch { + // No readable config: do nothing. +} - const validArgs = input.tool_input && typeof input.tool_input === "object" && !Array.isArray(input.tool_input); - // Empty, not omitted: hosts may merge updatedInput with the original input. - const updatedInput = { ...(validArgs ? input.tool_input : {}), _glean_permission_context: "" }; - const hitl = config.env?.ENABLE_HITL === "true"; - if (hitl && validArgs && input.permission_mode === "bypassPermissions") { - try { - const { createBypassReceipt } = await import(pathToFileURL(path.join(root, "mcp/approval-context.mjs")).href); - const base = process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".glean"); - updatedInput._glean_permission_context = createBypassReceipt(base, bareName, updatedInput); - } catch { /* Empty context keeps the plugin's approval gate active. */ } +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. + try { + const permissionMode = String(input.permission_mode ?? ""); + const sessionId = String(input.session_id ?? "") + .replace(/[^a-zA-Z0-9_-]/g, "-") + .slice(0, 64); + if (permissionMode && sessionId) { + const base = + process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".glean"); + const dir = path.join(base, "glean-hitl-mode"); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, `${sessionId}.json`), + JSON.stringify({ permission_mode: permissionMode, ts: Date.now() }), + ); } - process.stdout.write(JSON.stringify({ + } catch { + // Ignore: a failed marker write just means the server keeps prompting. + } + + process.stdout.write( + JSON.stringify({ hookSpecificOutput: { hookEventName: "PreToolUse", - updatedInput, - ...(hitl && bareName === "run_tool" ? { - permissionDecision: "allow", - permissionDecisionReason: "Glean run_tool is gated by its own HITL elicitation prompt; suppressing the redundant native prompt while ENABLE_HITL is on.", - } : {}), + permissionDecision: "allow", + permissionDecisionReason: + "Glean run_tool is gated by its own HITL elicitation prompt; suppressing the redundant native prompt while ENABLE_HITL is on.", }, - })); - break; - } -} catch { /* Invalid hook input or plugin configuration: retain native permissions. */ } + }), + ); +} +process.exit(0); diff --git a/overrides/claude/glean/hooks/hooks.json b/overrides/claude/glean/hooks/hooks.json index deaee09..f3470f6 100644 --- a/overrides/claude/glean/hooks/hooks.json +++ b/overrides/claude/glean/hooks/hooks.json @@ -15,7 +15,7 @@ ], "PreToolUse": [ { - "matcher": "^mcp__plugin_.*__.*$", + "matcher": "mcp__.*glean.*run_tool", "hooks": [ { "type": "command", diff --git a/shared/glean/mcp/approval-context.d.mts b/shared/glean/mcp/approval-context.d.mts deleted file mode 100644 index f5bdbd4..0000000 --- a/shared/glean/mcp/approval-context.d.mts +++ /dev/null @@ -1,5 +0,0 @@ -export declare const PERMISSION_CONTEXT_ARG: "_glean_permission_context"; -export declare function stripPermissionContext(args: Record): Record; -export declare function createBypassReceipt(baseDir: string, toolName: string, args: Record): string; -export declare function consumeBypassReceipt(baseDir: string, token: unknown, toolName: string, args: Record): boolean; -export declare function cleanupBypassReceipts(baseDir: string): void; diff --git a/shared/glean/mcp/approval-context.mjs b/shared/glean/mcp/approval-context.mjs deleted file mode 100644 index a968ebb..0000000 --- a/shared/glean/mcp/approval-context.mjs +++ /dev/null @@ -1,116 +0,0 @@ -import fs from "node:fs"; -import path from "node:path"; -import { createHash, randomUUID } from "node:crypto"; - -export const PERMISSION_CONTEXT_ARG = "_glean_permission_context"; -const DIR = "glean-bypass-receipts"; -const TTL_MS = 5 * 60_000; -const UUID = "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}"; -const TOKEN = new RegExp(`^${UUID}$`, "i"); -const OWN_FILE = new RegExp(`^${UUID}\\.(json|claimed)$`, "i"); - -export function stripPermissionContext(args) { - const clean = { ...args }; - delete clean[PERMISSION_CONTEXT_ARG]; - return clean; -} - -function canonical(value) { - if (Array.isArray(value)) return value.map(canonical); - if (value && typeof value === "object") { - return Object.fromEntries(Object.keys(value).sort().map((k) => [k, canonical(value[k])])); - } - return value; -} - -function fingerprint(args) { - return createHash("sha256").update(JSON.stringify(canonical(stripPermissionContext(args)))).digest("hex"); -} - -function privateStat(stat) { - // Windows uses the user-profile directory's ACLs, not Unix ownership/mode bits. - return typeof process.getuid !== "function" || - ((stat.mode & 0o077) === 0 && stat.uid === process.getuid()); -} - -function receiptDir(baseDir, create = false) { - if (create) fs.mkdirSync(baseDir, { recursive: true, mode: 0o700 }); - if (!fs.lstatSync(baseDir).isDirectory()) throw new Error("Invalid receipt base directory"); - const dir = path.join(baseDir, DIR); - if (create) { - try { fs.mkdirSync(dir, { mode: 0o700 }); } - catch (error) { if (error.code !== "EEXIST") throw error; } - } - const stat = fs.lstatSync(dir); - if (!stat.isDirectory() || !privateStat(stat)) throw new Error("Invalid receipt directory"); - return dir; -} - -export function createBypassReceipt(baseDir, toolName, args) { - const body = JSON.stringify({ toolName, fingerprint: fingerprint(args), ts: Date.now() }); - if (Buffer.byteLength(body) > 4096) throw new Error("Receipt too large"); - const token = randomUUID(); - const file = path.join(receiptDir(baseDir, true), `${token}.json`); - const fd = fs.openSync(file, "wx", 0o600); - try { - fs.writeFileSync(fd, body); - } catch (error) { - fs.unlinkSync(file); - throw error; - } finally { - fs.closeSync(fd); - } - return token; -} - -export function consumeBypassReceipt(baseDir, token, toolName, args) { - if (typeof token !== "string" || !TOKEN.test(token)) return false; - let claimed; - let fd; - try { - const dir = receiptDir(baseDir); - const file = path.join(dir, `${token}.json`); - if (!fs.lstatSync(file).isFile()) return false; - const target = path.join(dir, `${randomUUID()}.claimed`); - // Claim before reading: only one consumer can rename the source file. - fs.renameSync(file, target); - claimed = target; - fd = fs.openSync(claimed, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | fs.constants.O_NONBLOCK); - const stat = fs.fstatSync(fd); - if (!stat.isFile() || !privateStat(stat) || stat.nlink !== 1 || stat.size > 4096) return false; - const buffer = Buffer.alloc(4097); - const length = fs.readSync(fd, buffer, 0, buffer.length, 0); - if (length > 4096) return false; - const receipt = JSON.parse(buffer.toString("utf8", 0, length)); - const now = Date.now(); - return Number.isSafeInteger(receipt?.ts) && receipt.ts > 0 && - receipt.ts <= now && now - receipt.ts < TTL_MS && - receipt.toolName === toolName && receipt.fingerprint === fingerprint(args); - } catch { - return false; - } finally { - if (fd !== undefined) { try { fs.closeSync(fd); } catch { /* Best-effort close. */ } } - if (claimed) { try { fs.unlinkSync(claimed); } catch { /* Already cleaned. */ } } - } -} - -export function cleanupBypassReceipts(baseDir) { - let handle; - try { - const dir = receiptDir(baseDir); - handle = fs.opendirSync(dir); - const cutoff = Date.now() - TTL_MS; - // Bound startup work, including unrelated entries. Never follow links. - for (let i = 0; i < 128; i++) { - const entry = handle.readSync(); - if (!entry) break; - if (!OWN_FILE.test(entry.name) || !entry.isFile()) continue; - try { - const file = path.join(dir, entry.name); - const stat = fs.lstatSync(file); - if (stat.isFile() && privateStat(stat) && stat.nlink === 1 && stat.mtimeMs < cutoff) fs.unlinkSync(file); - } catch { /* A concurrent consumer may have claimed it. */ } - } - } catch { /* Missing or unsafe directories need no cleanup. */ } - finally { if (handle) { try { handle.closeSync(); } catch { /* Best-effort close. */ } } } -} diff --git a/shared/glean/mcp/pluginpack.json b/shared/glean/mcp/pluginpack.json index 42949b0..acb6fdd 100644 --- a/shared/glean/mcp/pluginpack.json +++ b/shared/glean/mcp/pluginpack.json @@ -2,7 +2,6 @@ "files": { "mcp/dist/index.js": "dist/index.js", "mcp/start.mjs": "start.mjs", - "mcp/approval-context.mjs": "approval-context.mjs", "mcp/package.json": "package.json", "skills/glean_run/SKILL.md": "skills/glean_run/SKILL.md" } diff --git a/shared/glean/mcp/src/index.ts b/shared/glean/mcp/src/index.ts index 3c222d1..c0acea4 100644 --- a/shared/glean/mcp/src/index.ts +++ b/shared/glean/mcp/src/index.ts @@ -20,7 +20,7 @@ import { closeCallbackServer, } from "./auth-callback-server.js"; import { handleFindSkills } from "./tools/find-skills.js"; -import { handleRunTool, runToolAnnotations } from "./tools/run-tool.js"; +import { handleRunTool, runToolAnnotations, shouldAskForToolApproval } from "./tools/run-tool.js"; import { evictStaleSkills } from "./skill-writer.js"; import { loadServerUrl, @@ -40,13 +40,7 @@ import { type DispatchContext, } from "./tools/remote-passthrough.js"; import { resolveSessionId } from "./session-id.js"; -import { hostSharedDataDir, serverDataDir } from "./data-dir.js"; -import { cleanupBypassReceipts } from "../approval-context.mjs"; -import { - remoteElicitationOptions, - resolveToolApprovalContext, - withPermissionContext, -} from "./write-approval.js"; +import { serverDataDir } from "./data-dir.js"; import { resolveServerUrlFromEmail } from "./config-search.js"; import { pluginVersionString } from "./version.js"; import { @@ -180,12 +174,24 @@ function getOAuthProvider(): GleanOAuthClientProvider { return oauthProvider; } -function getRemoteClientOpts( - approvalEnabled = process.env.ENABLE_HITL === "true", -): RemoteClientOptions { +async function getRemoteClientOpts(): Promise { + const supportsElicitation = + !!server.getClientCapabilities()?.elicitation && + (await shouldAskForToolApproval()); return { authProvider: getOAuthProvider(), - ...remoteElicitationOptions(server, approvalEnabled), + ...(supportsElicitation + ? { + elicitInput: (params, options) => + server.elicitInput( + { + message: params.message, + requestedSchema: params.requestedSchema, + }, + options, + ), + } + : {}), }; } @@ -347,7 +353,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { features: decision.features, state, }); - return { tools: tools.map(withPermissionContext) }; + return { tools }; }; // Pre-auth gate: tokens() is sync. When unauthenticated (or unconfigured) @@ -369,7 +375,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(), + await getRemoteClientOpts(), `tools-list-${process.pid}`, ); } catch (err) { @@ -441,7 +447,6 @@ function backendErrorResult(label: string, err: unknown): CallToolResult { // reports, so DCR + the authorize request use it directly. async function connectWithSignIn( serverUrl: string, - approvalEnabled: boolean, ): Promise< { ok: true; client: RemoteClient } | { ok: false; result: CallToolResult } > { @@ -452,7 +457,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(approvalEnabled), + await getRemoteClientOpts(), `setup-${process.pid}`, ); return { ok: true, client }; @@ -489,7 +494,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(approvalEnabled), + await getRemoteClientOpts(), `setup-${process.pid}`, ); // Unexpectedly connected without needing auth — done. @@ -531,7 +536,7 @@ async function connectWithSignIn( try { const client = await createRemoteClient( serverUrl, - getRemoteClientOpts(approvalEnabled), + await getRemoteClientOpts(), `setup-${process.pid}`, ); return { ok: true, client }; @@ -551,13 +556,13 @@ async function connectWithSignIn( * dynamic tools fetched ✓) or blocked on a user action. Used both by * `setup()` with no args and as the tail of `setup({server_url})`. */ -async function advanceSetup(approvalEnabled: boolean): Promise { +async function advanceSetup(): Promise { const serverUrl = resolveServerUrl(); if (!serverUrl) { return { content: [{ type: "text", text: SETUP_REQUIRED_TEXT }] }; } - const conn = await connectWithSignIn(serverUrl, approvalEnabled); + const conn = await connectWithSignIn(serverUrl); if (!conn.ok) return conn.result; const remoteClient = conn.client; @@ -604,8 +609,7 @@ async function advanceSetup(approvalEnabled: boolean): Promise { } server.setRequestHandler(CallToolRequestSchema, async (request) => { - const { name, arguments: input = {} } = request.params; - const { args, approvalEnabled } = resolveToolApprovalContext(name, input); + const { name, arguments: args = {} } = request.params; // Advertisement is advisory: a host may retain a stale tool list, so every // policy withdrawal is also enforced at call time. Setup remains the @@ -648,7 +652,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { } const dispatchCtx: DispatchContext = { serverUrl, - remoteClientOpts: getRemoteClientOpts(approvalEnabled), + remoteClientOpts: await getRemoteClientOpts(), authRedirectText: AUTH_REDIRECT_TO_SETUP_TEXT, logLine, }; @@ -682,7 +686,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(approvalEnabled), + await getRemoteClientOpts(), sessionId, ); } catch (err) { @@ -744,10 +748,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { let remoteClient; try { - // run_tool gates locally; upstream must not add a second approval form. remoteClient = await createRemoteClient( serverUrl, - getRemoteClientOpts(false), + await getRemoteClientOpts(), sessionId, ); } catch (err) { @@ -772,7 +775,6 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { const skillsBaseDir = resolveSkillsBaseDir(); return await handleRunTool(remoteClient, server, skillsBaseDir, args, { fileArgs: decision.features.fileArgs, - approvalEnabled, }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -885,7 +887,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { // to drive auth + tool fetch in the same call. } - return await advanceSetup(approvalEnabled); + return await advanceSetup(); } default: @@ -897,7 +899,6 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { }); async function main() { - cleanupBypassReceipts(hostSharedDataDir()); // Run once per session at MCP server startup. const ONE_WEEK_MS = 7 * 24 * 60 * 60 * 1000; try { diff --git a/shared/glean/mcp/src/tools/run-tool.ts b/shared/glean/mcp/src/tools/run-tool.ts index 461898a..c1caeda 100644 --- a/shared/glean/mcp/src/tools/run-tool.ts +++ b/shared/glean/mcp/src/tools/run-tool.ts @@ -3,9 +3,12 @@ import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; import { EmptyResultSchema } from "@modelcontextprotocol/sdk/types.js"; import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { callRemoteTool } from "../remote-client.js"; import { FILE_ARGS_DISABLED_TEXT } from "../policy/enforce.js"; +import { resolveSessionId } from "../session-id.js"; +import { hostSharedDataDir } from "../data-dir.js"; const DEFAULT_FILE_ARG_MAX_BYTES = 5 * 1024 * 1024; @@ -284,6 +287,46 @@ 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). +// 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 { + const sessionId = resolveSessionId() + .replace(/[^a-zA-Z0-9_-]/g, "-") + .slice(0, 64); + return path.join(hostSharedDataDir(), "glean-hitl-mode", `${sessionId}.json`); +} + +// Claude Code's live permission mode for THIS session, as captured by the hook +// on the current call. Returns null when the marker is missing, unreadable, or +// malformed — the caller treats null as "unknown" and keeps the approval gate, +// 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 +// 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) +// therefore has its stale bypass marker overwritten with the resumed mode on +// the resumed session's first run_tool call, re-engaging the gate. +async function currentPermissionMode(): Promise { + try { + const raw = await fs.readFile(permissionModeMarkerPath(), "utf-8"); + const parsed = JSON.parse(raw) as { permission_mode?: unknown }; + return typeof parsed.permission_mode === "string" + ? parsed.permission_mode + : null; + } catch { + return null; + } +} + +export async function shouldAskForToolApproval(): Promise { + return process.env.ENABLE_HITL === "true" && + (await currentPermissionMode()) !== "bypassPermissions"; +} + function humanizeMs(ms: number): string { const seconds = Math.round(ms / 1000); if (seconds < 120) return `${seconds}s`; @@ -319,8 +362,6 @@ export function elicitationFailureText( export interface RunToolPolicy { fileArgs: boolean; - // Resolved from trusted host context before opening the upstream connection. - approvalEnabled?: boolean; } class ToolApprovalError extends Error { @@ -446,9 +487,11 @@ export async function handleRunTool( } const remoteArgs = buildRemoteArgs(serverId, toolName, resolvedArgs); - const approvalEnabled = policy.approvalEnabled ?? (process.env.ENABLE_HITL === "true"); - // A session opt-out is not a saved preference: skip both lookup and elicitation. - if (!approvalEnabled || isKnownReadOnlyTool(toolMetadata, serverId, toolName)) { + // Opt-outs skip the entire approval flow, without reading or saving preferences. + if ( + !(await shouldAskForToolApproval()) || + isKnownReadOnlyTool(toolMetadata, serverId, toolName) + ) { return callRemoteTool(remoteClient, "run_tool", remoteArgs); } @@ -462,65 +505,96 @@ export async function handleRunTool( ); } - // Cursor is deliberately not excepted: capable hosts share the local gate. - if (requiresApproval && mcpServer.getClientCapabilities()?.elicitation) { - const timeout = hitlTimeoutMs(); - // Burn request id 0 so cancellation can address the approval request. - primeElicitationCancellation(mcpServer); - - const startedAt = Date.now(); - try { - const result = await mcpServer.elicitInput( - runToolApprovalForm(toolName), - { timeout }, - ); - const decision = approvalDecision(result); + const hitlEnabled = process.env.ENABLE_HITL === "true"; + // Cursor is deliberately not excepted: current Cursor builds can use the same + // local elicitation gate as other capable hosts. Older builds that drop the + // prompt fail closed, and the timeout response explains the upgrade path. + if ( + hitlEnabled && + requiresApproval && + mcpServer.getClientCapabilities()?.elicitation + ) { + // In bypassPermissions mode (`claude --dangerously-skip-permissions`) the + // user has opted out of every approval prompt for the session, so our own + // elicitation gate is just a redundant popup — skip it and execute + // directly. The mode comes from the PreToolUse hook, which writes it keyed + // by session id immediately before this call, so it reflects the current + // call and never leaks across sessions. Any other or unknown mode keeps the + // gate. Only bypassPermissions is skipped (deliberately narrow). + const bypass = (await currentPermissionMode()) === "bypassPermissions"; + if (!bypass) { + const timeout = hitlTimeoutMs(); + + // Make a dummy empty request to burn JSON-RPC request id 0 + primeElicitationCancellation(mcpServer); + + const startedAt = Date.now(); + try { + const result = await mcpServer.elicitInput( + runToolApprovalForm(toolName), + { timeout }, + ); + const decision = approvalDecision(result); + + if (decision === approvalDeny || decision === approvalCancel) { + return { + content: [ + { + type: "text", + text: `Action ${toolName} was ${decision === approvalDeny ? "declined" : "cancelled"} by the user.`, + }, + ], + }; + } + if (decision === null) { + return { + content: [ + { + type: "text", + text: + `Action ${toolName} was not approved — the approval form ` + + `response was invalid. The action was NOT executed.`, + }, + ], + isError: true, + }; + } - if (decision === approvalDeny || decision === approvalCancel) { - return { - content: [{ - type: "text", - text: `Action ${toolName} was ${decision === approvalDeny ? "declined" : "cancelled"} by the user.`, - }], - }; - } - if (decision === null) { + if (decision === approvalAlwaysAllow) { + try { + await callRemoteTool(remoteClient, "set_tool_approval", { + server_id: serverId, + tool_name: toolName, + value: "ALWAYS_ALLOWED", + }); + } catch (err) { + const detail = err instanceof Error ? err.message : String(err); + console.error( + `[set_tool_approval] failed to persist "${toolName}" to Glean: ${detail}`, + ); + } + } + } catch (err) { + // Fail CLOSED. An approval gate that executes the action when the + // prompt times out or errors defeats its own purpose — and the SDK + // rejects elicitInput precisely on request timeout. + const detail = err instanceof Error ? err.message : String(err); return { - content: [{ - type: "text", - text: `Action ${toolName} was not approved — the approval form ` + - `response was invalid. The action was NOT executed.`, - }], + content: [ + { + type: "text", + text: elicitationFailureText( + mcpServer, + toolName, + detail, + Date.now() - startedAt, + timeout, + ), + }, + ], isError: true, }; } - - if (decision === approvalAlwaysAllow) { - try { - await callRemoteTool(remoteClient, "set_tool_approval", { - server_id: serverId, - tool_name: toolName, - value: "ALWAYS_ALLOWED", - }); - } catch (err) { - const detail = err instanceof Error ? err.message : String(err); - console.error( - `[set_tool_approval] failed to persist "${toolName}" to Glean: ${detail}`, - ); - } - } - } catch (err) { - // A failed or timed-out approval must never execute the action. - const detail = err instanceof Error ? err.message : String(err); - return { - content: [{ - type: "text", - text: elicitationFailureText( - mcpServer, toolName, detail, Date.now() - startedAt, timeout, - ), - }], - isError: true, - }; } } diff --git a/shared/glean/mcp/src/write-approval.ts b/shared/glean/mcp/src/write-approval.ts deleted file mode 100644 index edac901..0000000 --- a/shared/glean/mcp/src/write-approval.ts +++ /dev/null @@ -1,59 +0,0 @@ -import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { - consumeBypassReceipt, - PERMISSION_CONTEXT_ARG, - stripPermissionContext, -} from "../approval-context.mjs"; -import { hostSharedDataDir } from "./data-dir.js"; -import type { RemoteClientOptions } from "./remote-client.js"; - -export interface ToolApprovalContext { - args: Record; - approvalEnabled: boolean; -} - -// The hook carries a one-use receipt, not a model-supplied permission mode. This -// also works when Claude does not export its session ID to the MCP process. -export function resolveToolApprovalContext( - toolName: string, - input: Record, -): ToolApprovalContext { - const args = stripPermissionContext(input); - const hitlEnabled = process.env.ENABLE_HITL === "true"; - const bypass = hitlEnabled && consumeBypassReceipt( - hostSharedDataDir(), input[PERMISSION_CONTEXT_ARG], toolName, args, - ); - return { args, approvalEnabled: hitlEnabled && !bypass }; -} - -export function withPermissionContext(tool: Tool): Tool { - return { - ...tool, - inputSchema: { - ...tool.inputSchema, - properties: { - ...tool.inputSchema.properties, - [PERMISSION_CONTEXT_ARG]: { - type: "string", - description: "Internal Claude hook context. Leave unset.", - }, - }, - }, - }; -} - -export function remoteElicitationOptions( - server: Server, - approvalEnabled: boolean, -): Pick { - // Without the callback, createRemoteClient advertises no upstream elicitation - // capability. Do not synthesize an approval or persist an Always Allow choice. - if (!approvalEnabled || !server.getClientCapabilities()?.elicitation) return {}; - return { - elicitInput: (params, options) => server.elicitInput( - { message: params.message, requestedSchema: params.requestedSchema }, - options, - ), - }; -} diff --git a/shared/glean/mcp/tests/approval-context.test.ts b/shared/glean/mcp/tests/approval-context.test.ts deleted file mode 100644 index 1f81862..0000000 --- a/shared/glean/mcp/tests/approval-context.test.ts +++ /dev/null @@ -1,139 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { randomUUID } from "node:crypto"; -import { execFile } from "node:child_process"; -import { promisify } from "node:util"; -import { PERMISSION_CONTEXT_ARG as KEY, stripPermissionContext, createBypassReceipt, - consumeBypassReceipt, cleanupBypassReceipts } from "../approval-context.mjs"; - -let base: string; -const args = { server_id: "one", tool_name: "write", arguments: { body: "secret body", list: [1, 2] } }; -const dir = () => path.join(base, "glean-bypass-receipts"); -const receipt = (token: string) => path.join(dir(), `${token}.json`); -const create = () => createBypassReceipt(base, "run_tool", args); -const consume = (token: unknown, input = args, tool = "run_tool") => consumeBypassReceipt(base, token, tool, input); -beforeEach(() => { base = fs.mkdtempSync(path.join(os.tmpdir(), "approval-context-")); }); -afterEach(() => fs.rmSync(base, { recursive: true, force: true })); - -describe("bypass receipts", () => { - it("strips only the top-level context without mutating the caller", () => { - const original = { ...args, [KEY]: "forged" }; - const clean = stripPermissionContext(original); - expect(clean).toEqual(args); - expect(clean).not.toBe(original); - expect(clean.arguments).toBe(args.arguments); - expect(original[KEY]).toBe("forged"); - }); - it("stores only tool identity, a SHA256 fingerprint, and timestamp in private files", () => { - const token = create(); - expect(token).toMatch(/^[\da-f-]{36}$/); - const body = JSON.parse(fs.readFileSync(receipt(token), "utf8")); - expect(Object.keys(body).sort()).toEqual(["fingerprint", "toolName", "ts"]); - expect(body.fingerprint).toMatch(/^[\da-f]{64}$/); - expect(JSON.stringify(body)).not.toContain("secret body"); - expect(fs.statSync(receipt(token)).mode & 0o777).toBe(0o600); - expect(fs.statSync(dir()).mode & 0o777).toBe(0o700); - }); - it("canonicalizes nested object keys, excludes context, and consumes only once", () => { - const token = createBypassReceipt(base, "run_tool", { ...args, [KEY]: "old" }); - const reordered = { arguments: { list: [1, 2], body: "secret body" }, tool_name: "write", server_id: "one", [KEY]: token }; - expect(consume(token, reordered)).toBe(true); - expect(consume(token)).toBe(false); - expect(fs.readdirSync(dir())).toEqual([]); - }); - it.each(["tool", "server", "remote-tool", "extra", "arguments", "array-order"])("rejects and burns a receipt with mismatched %s", (field) => { - const token = create(); - const input = structuredClone(args); - if (field === "server") input.server_id = "two"; - if (field === "remote-tool") input.tool_name = "other"; - if (field === "extra") (input as Record).extra = true; - if (field === "arguments") input.arguments.body = "changed"; - if (field === "array-order") input.arguments.list.reverse(); - expect(consume(token, input, field === "tool" ? "other" : "run_tool")).toBe(false); - expect(consume(token)).toBe(false); - expect(fs.readdirSync(dir())).toEqual([]); - }); - it.each([undefined, null, {}, "", "../escape", "../../outside.json", `${randomUUID()}.json`, randomUUID()])( - "rejects invalid or missing receipt %j", (token) => { - expect(consume(token)).toBe(false); - expect(fs.existsSync(dir())).toBe(false); - }, - ); - it.each(["expired", "future", "string", "negative", "missing"])("rejects %s timestamps", (type) => { - const token = create(); - const body = JSON.parse(fs.readFileSync(receipt(token), "utf8")); - body.ts = { expired: Date.now() - 300_001, future: Date.now() + 60_000, - string: String(Date.now()), negative: -1, missing: undefined }[type]; - fs.writeFileSync(receipt(token), JSON.stringify(body)); - expect(consume(token)).toBe(false); - expect(fs.readdirSync(dir())).toEqual([]); - }); - it.each(["{", "null", "{}", "x".repeat(4097)])("rejects malformed or oversized receipt %#", (body) => { - const token = create(); - fs.writeFileSync(receipt(token), body); - expect(consume(token)).toBe(false); - expect(fs.readdirSync(dir())).toEqual([]); - }); - it("rejects receipt symlinks without touching their targets", () => { - const token = create(); - const target = path.join(base, "target"); - fs.renameSync(receipt(token), target); - fs.symlinkSync(target, receipt(token)); - expect(consume(token)).toBe(false); - cleanupBypassReceipts(base); - expect(fs.lstatSync(receipt(token)).isSymbolicLink()).toBe(true); - expect(fs.existsSync(target)).toBe(true); - }); - it.each(["base", "directory"])("rejects a symlinked %s", (kind) => { - const target = path.join(base, "target"); - fs.mkdirSync(target, { mode: 0o700 }); - const link = kind === "base" ? path.join(base, "linked-base") : dir(); - fs.symlinkSync(target, link); - const location = kind === "base" ? link : base; - expect(() => createBypassReceipt(location, "run_tool", args)).toThrow(); - expect(consumeBypassReceipt(location, randomUUID(), "run_tool", args)).toBe(false); - cleanupBypassReceipts(location); - expect(fs.readdirSync(target)).toEqual([]); - }); - it("rejects non-private receipt files and directories", () => { - const token = create(); - fs.chmodSync(receipt(token), 0o644); - expect(consume(token)).toBe(false); - fs.chmodSync(dir(), 0o755); - expect(() => create()).toThrow(); - }); - it("allows exactly one concurrent process to consume", async () => { - const token = create(); - const script = `import { consumeBypassReceipt } from ${JSON.stringify(new URL("../approval-context.mjs", import.meta.url).href)}; - console.log(consumeBypassReceipt(process.argv[1], process.argv[2], "run_tool", ${JSON.stringify(args)}));`; - const results = await Promise.all(Array.from({ length: 6 }, () => - promisify(execFile)(process.execPath, ["--input-type=module", "-e", script, base, token], { timeout: 5000 }))); - expect(results.filter(({ stdout }) => stdout.trim() === "true")).toHaveLength(1); - expect(fs.readdirSync(dir())).toEqual([]); - }); - it("cleans only its stale UUID receipt/claim files, not fresh or unrelated entries", () => { - const fresh = receipt(create()); - const stale = receipt(create()); - const claimed = path.join(dir(), `${randomUUID()}.claimed`); - const unrelated = path.join(dir(), "preferences.json"); - for (const file of [claimed, unrelated]) fs.writeFileSync(file, "{}", { mode: 0o600 }); - const old = new Date(Date.now() - 600_000); - for (const file of [stale, claimed, unrelated]) fs.utimesSync(file, old, old); - cleanupBypassReceipts(base); - expect(fs.readdirSync(dir()).sort()).toEqual([path.basename(fresh), "preferences.json"].sort()); - }); - it("bounds cleanup work and tolerates missing directories", () => { - cleanupBypassReceipts(base); - create(); - const old = new Date(Date.now() - 600_000); - for (let i = 0; i < 140; i++) { - const file = receipt(randomUUID()); - fs.writeFileSync(file, "{}", { mode: 0o600 }); - fs.utimesSync(file, old, old); - } - cleanupBypassReceipts(base); - expect(fs.readdirSync(dir()).length).toBeGreaterThanOrEqual(13); - }); -}); diff --git a/shared/glean/mcp/tests/approval-handler.test.ts b/shared/glean/mcp/tests/approval-handler.test.ts deleted file mode 100644 index aad31a5..0000000 --- a/shared/glean/mcp/tests/approval-handler.test.ts +++ /dev/null @@ -1,333 +0,0 @@ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { execFileSync } from "node:child_process"; -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { fileURLToPath } from "node:url"; -import { UnauthorizedError } from "@modelcontextprotocol/client"; -import type { CallToolRequest, CallToolResult, ListToolsResult } from "@modelcontextprotocol/sdk/types.js"; -import { - acceptedContent, CLIENT_CAPABILITIES_META_KEY, createMcpHandler, inputRequired, - McpServer, type ServerContext, -} from "@modelcontextprotocol/server"; -import * as z from "zod/v4"; - -// Only the host transport and authentication are fake. index.ts registers the -// handlers; its dispatch, remote client, approval policy and receipt reader run unchanged. -const mocks = vi.hoisted(() => { - const handlers = new Map(); - return { - handlers, home: "", - host: { - setRequestHandler: vi.fn((schema, handler) => handlers.set(schema.shape.method.value, handler)), - connect: vi.fn(async () => {}), request: vi.fn(async () => ({})), - getClientCapabilities: vi.fn(() => ({ elicitation: { form: {} } })), - getClientVersion: vi.fn(() => ({ name: "claude-code", version: "1" })), - elicitInput: vi.fn(), sendToolListChanged: vi.fn(async () => {}), - }, - openBrowser: vi.fn(), setPendingAuthCode: vi.fn(), - startCallbackServer: vi.fn(async () => ({ code: Promise.resolve("local-test-code") })), - closeCallbackServer: vi.fn(), - }; -}); -vi.mock("@modelcontextprotocol/sdk/server/index.js", () => ({ Server: class { - constructor() { return mocks.host; } -} })); -vi.mock("node:os", async (original) => { - const actual = await original(); - return { ...actual, homedir: () => mocks.home, - default: { ...actual, homedir: () => mocks.home } }; -}); -vi.mock("../src/auth-provider.js", () => ({ - GleanOAuthClientProvider: class { - authorizationUrl = "https://approval.invalid/local-sign-in"; - tokens() { return { access_token: "local-test-token", token_type: "Bearer" }; } - clientInformation() { return { client_id: "local-test-client" }; } - needsFreshClient() { return false; } - // Complete simulated sign-in immediately; never perform a token exchange. - setPendingAuthCode = mocks.setPendingAuthCode; - }, - openBrowser: mocks.openBrowser, -})); -vi.mock("../src/auth-callback-server.js", () => ({ - startCallbackServer: mocks.startCallbackServer, closeCallbackServer: mocks.closeCallbackServer, -})); - -const repo = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../../.."); -const endpoint = "https://approval.invalid/mcp/gateway/proxy"; -const KEY = "_glean_permission_context"; -const body = { body: "local fake write" }; -const runArgs = { server_id: "fake", tool_name: "fake_write", arguments: body }; -const text = (value: string) => ({ content: [{ type: "text" as const, text: value }] }); -type Mode = "normal" | "off" | "bypass"; -type Config = { mcpServers: Record }> }; -type Seen = { name: string; args: Record; capabilities: unknown; - state: unknown; responses: unknown }; -let buildDir: string, plugin: string, config: Config, serverName: string, namespace: string, hook: string; -let hookMatcher: RegExp; -let root: string, backend: ReturnType; -let calls: Seen[], writes: Record[], denied: string[]; -let wire: { method: string; capabilities: unknown }[]; -let authFailures: number, changeHitlAfterFailure: string | undefined; - -beforeAll(() => { - // CI runs test:bundle before build. Build from source, privately: version.test.ts - // also writes the repository bundle, and prebuild syncs tracked changelogs. - buildDir = fs.mkdtempSync(path.join(os.tmpdir(), "approval-package-")); - for (const entry of ["package.json", "pluginpack.config.ts", "CHANGELOG.md", "LICENSE", "scripts", "shared", "overrides"]) { - fs.cpSync(path.join(repo, entry), path.join(buildDir, entry), { recursive: true, - filter: (source) => !["dist", "node_modules", "tests"].includes(path.basename(source)) }); - } - fs.symlinkSync(path.join(repo, "node_modules"), path.join(buildDir, "node_modules"), "dir"); - const buildHome = path.join(buildDir, "home"); - fs.mkdirSync(buildHome); - const logPath = path.join(buildDir, "build.log"); - const log = fs.openSync(logPath, "w"); - try { - execFileSync("npm", ["run", "build"], { cwd: buildDir, timeout: 60_000, - env: { ...process.env, HOME: buildHome, USERPROFILE: buildHome, - npm_config_offline: "true", npm_config_audit: "false" }, stdio: ["ignore", log, log] }); - } catch (error) { - throw new Error(`${String(error)}\n${fs.readFileSync(logPath, "utf8").slice(-6000)}`); - } finally { fs.closeSync(log); } - plugin = path.join(buildDir, "dist/claude/plugins/glean"); - const manifest = JSON.parse(fs.readFileSync(path.join(plugin, ".claude-plugin/plugin.json"), "utf8")); - config = JSON.parse(fs.readFileSync(path.join(plugin, ".mcp.json"), "utf8")); - expect(Object.keys(config.mcpServers)).toHaveLength(1); - [serverName] = Object.keys(config.mcpServers); - expect(config.mcpServers[serverName].env.ENABLE_HITL).toBe("true"); - namespace = `mcp__plugin_${manifest.name}_${serverName}__`; - const hooks = JSON.parse(fs.readFileSync(path.join(plugin, manifest.hooks ?? "hooks/hooks.json"), "utf8")); - const matched = hooks.hooks.PreToolUse.filter((entry: { matcher: string }) => - new RegExp(entry.matcher).test(`${namespace}run_tool`)); - expect(matched).toHaveLength(1); - hookMatcher = new RegExp(matched[0].matcher); - expect(matched[0].hooks).toHaveLength(1); - expect(matched[0].hooks[0].type).toBe("command"); - const command = /^node "\$\{CLAUDE_PLUGIN_ROOT\}\/([^\"]+)"$/.exec(matched[0].hooks[0].command); - expect(command).not.toBeNull(); - hook = path.join(plugin, command![1]); - for (const file of [hook, path.join(plugin, "mcp/approval-context.mjs"), path.join(plugin, "mcp/dist/index.js")]) { - expect(fs.statSync(file).size).toBeGreaterThan(0); - } -}, 90_000); -afterAll(() => { if (buildDir) fs.rmSync(buildDir, { recursive: true, force: true }); }); - -beforeEach(async () => { - vi.resetModules(); - vi.clearAllMocks(); - mocks.handlers.clear(); - root = fs.mkdtempSync(path.join(os.tmpdir(), "approval-handlers-")); - mocks.home = path.join(root, "home"); - fs.mkdirSync(mocks.home); - for (const [key, value] of Object.entries({ - HOME: mocks.home, USERPROFILE: mocks.home, GLEAN_MCP_SERVER_URL: endpoint, - PLUGIN_DATA_DIR: path.join(root, "server-data"), CLAUDE_PLUGIN_DATA: path.join(root, "host-data"), - SKILLS_BASE_DIR: path.join(root, "skills"), GLEAN_SESSION_ID: undefined, - CLAUDE_CODE_SESSION_ID: undefined, ENABLE_HITL: "true", GLEAN_REMOTE_TOOL_TIMEOUT_MS: "2000", - })) vi.stubEnv(key, value); - calls = []; writes = []; denied = []; wire = []; authFailures = 0; changeHitlAfterFailure = undefined; - mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Allow", approved: true } }); - vi.spyOn(console, "error").mockImplementation(() => {}); - function observe(name: string, args: Record, ctx: ServerContext) { - const envelope = ctx.mcpReq.envelope as Record | undefined; - const capabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY] as Record | undefined; - calls.push({ name, args, capabilities, state: ctx.mcpReq.requestState(), responses: ctx.mcpReq.inputResponses }); - return capabilities; - } - function execute(name: string, args: Record, ctx: ServerContext) { - const capabilities = observe(name, args, ctx); - if (capabilities?.elicitation) { - if (!ctx.mcpReq.inputResponses) return inputRequired({ - inputRequests: { approval: inputRequired.elicit({ message: "Upstream approval", - requestedSchema: { type: "object", properties: { approved: { type: "boolean" } }, required: ["approved"] } }) }, - requestState: "pending-write", - }); - if (!acceptedContent<{ approved: boolean }>(ctx.mcpReq.inputResponses, "approval")?.approved) { - return { ...text("declined"), isError: true }; - } - } - writes.push(name === "run_tool" ? args.arguments as Record : args); - return text("executed"); - } - backend = createMcpHandler(() => { - const remote = new McpServer({ name: "approval-handler-fixture", version: "1" }); - const identity = { server_id: z.string(), tool_name: z.string() }; - remote.registerTool("get_tool_approval", { inputSchema: z.strictObject(identity) }, async (args, ctx) => { - observe("get_tool_approval", args, ctx); return text('{"requires_approval":true}'); - }); - remote.registerTool("set_tool_approval", { - inputSchema: z.strictObject({ ...identity, value: z.literal("ALWAYS_ALLOWED") }), - }, async (args, ctx) => { observe("set_tool_approval", args, ctx); return text("saved locally"); }); - const payload = z.strictObject({ body: z.string() }); - remote.registerTool("run_tool", { inputSchema: z.strictObject({ ...identity, arguments: payload }) }, - async (args, ctx) => execute("run_tool", args, ctx)); - remote.registerTool("memory", { inputSchema: payload }, async (args, ctx) => execute("memory", args, ctx)); - return remote; - }); - vi.stubGlobal("fetch", vi.fn(async (input: string | URL | Request, init?: RequestInit) => { - const request = new Request(input, init); - if (request.url !== endpoint) { denied.push(request.url); throw new Error(`Network forbidden: ${request.url}`); } - if (request.method === "POST") { - const frame = await request.clone().json(); - wire.push({ method: frame.method, capabilities: frame.params?._meta?.[CLIENT_CAPABILITIES_META_KEY] }); - if (frame.method === "server/discover" && authFailures > 0) { - authFailures -= 1; - if (changeHitlAfterFailure) vi.stubEnv("ENABLE_HITL", changeHitlAfterFailure); - throw new UnauthorizedError("Simulated expired token; no real OAuth"); - } - } - return backend.fetch(request); - })); - await import("../src/index.js"); - await vi.waitFor(() => expect(mocks.host.connect).toHaveBeenCalledTimes(1)); - expect([...mocks.handlers.keys()].sort()).toEqual(["tools/call", "tools/list"]); -}); -afterEach(async () => { - try { - await backend?.close(); - expect(denied).toEqual([]); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - vi.restoreAllMocks(); vi.unstubAllGlobals(); vi.unstubAllEnvs(); - } -}); - -// No copied dispatch: invoke the functions captured from the real SDK registration. -function call(name: string, args: Record = {}) { - const handler = mocks.handlers.get("tools/call") as (request: CallToolRequest) => Promise; - return handler({ method: "tools/call", params: { name, arguments: args } }); -} -function fromHook(mode: Mode, name = "run_tool", args: Record = runArgs) { - const cfg = structuredClone(config); - const hitl = mode === "off" ? "false" : "true"; - cfg.mcpServers[serverName].env.ENABLE_HITL = hitl; - fs.writeFileSync(path.join(plugin, ".mcp.json"), JSON.stringify(cfg)); - vi.stubEnv("ENABLE_HITL", hitl); - expect(hookMatcher.test(`${namespace}${name}`)).toBe(true); - const output = execFileSync(process.execPath, [hook], { timeout: 5000, encoding: "utf8", - cwd: root, env: { ...process.env, CLAUDE_PLUGIN_ROOT: plugin }, - input: JSON.stringify({ tool_name: `${namespace}${name}`, tool_input: args, - permission_mode: mode === "bypass" ? "bypassPermissions" : "default" }) }); - const result = JSON.parse(output).hookSpecificOutput; - expect(result.hookEventName).toBe("PreToolUse"); - expect(result.permissionDecision).toBe(mode !== "off" && name === "run_tool" ? "allow" : undefined); - expect(result.updatedInput[KEY]).toEqual(mode === "bypass" ? expect.stringMatching(/.+/) : ""); - return result.updatedInput as Record; -} -const names = () => calls.map(({ name }) => name); -function noForms() { - expect(calls.length).toBeGreaterThan(0); - for (const seen of calls) { - expect(seen.capabilities).toBeDefined(); - expect(seen.capabilities).not.toHaveProperty("elicitation"); - expect(seen.responses).toBeUndefined(); - expect(seen.args).not.toHaveProperty(KEY); - } -} - -describe("registered approval handlers with the packaged Claude hook", () => { - it("advertises optional internal context, including a closed promoted schema", async () => { - const list = mocks.handlers.get("tools/list") as () => Promise; - const { tools } = await list(); - expect(tools.map(({ name }) => name)).toEqual(expect.arrayContaining(["setup", "run_tool", "memory"])); - for (const tool of tools) { - expect(tool.inputSchema.properties?.[KEY]).toMatchObject({ type: "string" }); - expect(tool.inputSchema.required ?? []).not.toContain(KEY); - } - expect(tools.find(({ name }) => name === "memory")?.inputSchema).toMatchObject({ - additionalProperties: false, required: ["body"], properties: { body: { type: "string" } }, - }); - expect(wire.map(({ method }) => method)).toEqual(["server/discover", "tools/list"]); - }); - - it.each(["off", "bypass"] as const)("%s skips preference reads/writes and forms without session IDs", async (mode) => { - expect(process.env.GLEAN_SESSION_ID).toBeUndefined(); - expect(process.env.CLAUDE_CODE_SESSION_ID).toBeUndefined(); - const input = Object.freeze(fromHook(mode)); - expect(await call("run_tool", input)).toMatchObject(text("executed")); - expect(names()).toEqual(["run_tool"]); - expect(calls[0].args).toEqual(runArgs); - expect(writes).toEqual([body]); - expect(mocks.host.elicitInput).not.toHaveBeenCalled(); - expect(mocks.openBrowser).not.toHaveBeenCalled(); - noForms(); - }); - - it.each(["Allow", "Deny", "Always Allow"])("normal %s prompts once with no upstream gate", async (choice) => { - mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: choice } }); - const result = await call("run_tool", fromHook("normal")); - expect(mocks.host.elicitInput).toHaveBeenCalledTimes(1); - expect(mocks.host.elicitInput.mock.calls[0][0]).toMatchObject({ mode: "form" }); - expect(names()).toEqual(choice === "Deny" ? ["get_tool_approval"] : choice === "Always Allow" - ? ["get_tool_approval", "set_tool_approval", "run_tool"] : ["get_tool_approval", "run_tool"]); - expect(writes).toEqual(choice === "Deny" ? [] : [body]); - expect(JSON.stringify(result)).toContain(choice === "Deny" ? "declined" : "executed"); - if (choice === "Always Allow") expect(calls[1].args).toEqual({ - server_id: "fake", tool_name: "fake_write", value: "ALWAYS_ALLOWED", - }); - noForms(); - }); - - it("restores normal gating after bypass/off and strips replayed or forged context", async () => { - const bypass = fromHook("bypass"); - await call("run_tool", bypass); - await call("run_tool", fromHook("off")); - mocks.host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Deny" } }); - await call("run_tool", fromHook("normal", "run_tool", bypass)); - await call("run_tool", bypass); // One-use receipt cannot authorize a replay. - await call("run_tool", { ...runArgs, [KEY]: "forged" }); - expect(names()).toEqual(["run_tool", "run_tool", "get_tool_approval", "get_tool_approval", "get_tool_approval"]); - expect(mocks.host.elicitInput).toHaveBeenCalledTimes(3); - expect(writes).toEqual([body, body]); - noForms(); - }); - - it.each(["normal", "off", "bypass"] as const)("promoted memory forwards forms only in normal mode: %s", async (mode) => { - expect(await call("memory", fromHook(mode, "memory", body))).toMatchObject(text("executed")); - expect(names()).toEqual(mode === "normal" ? ["memory", "memory"] : ["memory"]); - expect(calls.every(({ args }) => JSON.stringify(args) === JSON.stringify(body))).toBe(true); - expect(writes).toEqual([body]); - expect(mocks.host.elicitInput).toHaveBeenCalledTimes(mode === "normal" ? 1 : 0); - if (mode === "normal") { - expect(calls[0].capabilities).toHaveProperty("elicitation"); - expect(calls.map(({ state }) => state)).toEqual([undefined, "pending-write"]); - expect(calls[1].responses).toMatchObject({ approval: { action: "accept", content: { approved: true } } }); - } else noForms(); - }); - - it("shares ~/.glean with the hook when only PLUGIN_DATA_DIR is set", async () => { - vi.stubEnv("CLAUDE_PLUGIN_DATA", undefined); - const input = fromHook("bypass"); - const receipts = path.join(mocks.home, ".glean/glean-bypass-receipts"); - expect(fs.readdirSync(receipts)).toHaveLength(1); - expect(fs.existsSync(path.join(process.env.PLUGIN_DATA_DIR!, "glean-bypass-receipts"))).toBe(false); - expect(await call("run_tool", input)).toMatchObject(text("executed")); - expect(fs.readdirSync(receipts)).toHaveLength(0); - expect(names()).toEqual(["run_tool"]); - expect(mocks.host.elicitInput).not.toHaveBeenCalled(); - noForms(); - }); - - it.each(["normal", "off", "bypass"] as const)("setup retains %s approval policy through all reconnects", async (mode) => { - const input = fromHook(mode, "setup", {}); - authFailures = 2; // Authenticated attempt, then callback-listener attempt, then post-sign-in reconnect. - changeHitlAfterFailure = mode === "normal" ? "false" : "true"; - expect(JSON.stringify(await call("setup", input))).toContain("Glean setup is complete"); - expect(wire.map(({ method }) => method)).toEqual([ - "server/discover", "server/discover", "server/discover", "tools/list", - ]); - for (const request of wire) { - expect(request.capabilities).toBeDefined(); - if (mode === "normal") expect(request.capabilities).toHaveProperty("elicitation"); - else expect(request.capabilities).not.toHaveProperty("elicitation"); - } - expect(mocks.startCallbackServer).toHaveBeenCalledTimes(1); - expect(mocks.closeCallbackServer).toHaveBeenCalledTimes(1); - expect(mocks.openBrowser).toHaveBeenCalledExactlyOnceWith("https://approval.invalid/local-sign-in"); - expect(mocks.setPendingAuthCode).toHaveBeenCalledExactlyOnceWith("local-test-code"); - expect(mocks.host.elicitInput).not.toHaveBeenCalled(); - expect(calls).toEqual([]); - expect(writes).toEqual([]); - }); -}); diff --git a/shared/glean/mcp/tests/auto-approve-hook.test.ts b/shared/glean/mcp/tests/auto-approve-hook.test.ts index c63ae2b..2b647eb 100644 --- a/shared/glean/mcp/tests/auto-approve-hook.test.ts +++ b/shared/glean/mcp/tests/auto-approve-hook.test.ts @@ -1,114 +1,175 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { execFileSync } from "node:child_process"; -import fs from "node:fs"; +import { describe, it, expect } from "vitest"; +import { spawn } from "node:child_process"; +import fs from "node:fs/promises"; import path from "node:path"; import os from "node:os"; import { fileURLToPath } from "node:url"; -import { PERMISSION_CONTEXT_ARG as KEY, createBypassReceipt, consumeBypassReceipt } from "../approval-context.mjs"; const here = path.dirname(fileURLToPath(import.meta.url)); -const HOOK = path.resolve(here, "../../../../overrides/claude/glean/hooks/auto-approve-run-tool.mjs"); -const glean = (tool: string) => `mcp__plugin_local-mcp_glean_plugin__${tool}`; -const args = { server_id: "server", tool_name: "send_message", arguments: { body: "private body" } }; -let root: string, dataDir: string; -function configure(hitl = "true", server = "glean_plugin", extra = {}) { - fs.writeFileSync(path.join(root, ".mcp.json"), JSON.stringify({ - mcpServers: { [server]: { command: "node", env: { ENABLE_HITL: hitl }, ...extra } }, - })); -} -beforeEach(() => { - root = fs.mkdtempSync(path.join(os.tmpdir(), "approve-hook-")); - dataDir = path.join(root, "plugin-data"); - fs.mkdirSync(path.join(root, ".claude-plugin")); - fs.writeFileSync(path.join(root, ".claude-plugin/plugin.json"), JSON.stringify({ name: "local-mcp" })); - fs.mkdirSync(path.join(root, "mcp")); - fs.copyFileSync(path.resolve(here, "../approval-context.mjs"), path.join(root, "mcp/approval-context.mjs")); - configure(); -}); -afterEach(() => fs.rmSync(root, { recursive: true, force: true })); -function runHook(tool = glean("run_tool"), extra: Record = {}, raw?: string) { - const env: NodeJS.ProcessEnv = { ...process.env, CLAUDE_PLUGIN_ROOT: root, CLAUDE_PLUGIN_DATA: dataDir }; - delete env.CLAUDE_CODE_SESSION_ID; - const out = execFileSync(process.execPath, [HOOK], { - env, encoding: "utf8", input: raw ?? JSON.stringify({ tool_name: tool, tool_input: args, ...extra }), - }); - return out ? JSON.parse(out).hookSpecificOutput : null; +const HOOK = path.resolve( + here, + "../../../../overrides/claude/glean/hooks/auto-approve-run-tool.mjs", +); + +interface HookResult { + out: string; + // Parsed contents of the single permission-mode marker the hook wrote, or + // null when none was written. markerFiles lists the filenames present. + marker: { permission_mode?: string; ts?: number } | null; + markerFiles: string[]; } -const bypass = { permission_mode: "bypassPermissions" }; - -describe("request-local approval hook", () => { - it("creates a usable receipt without any host session id", () => { - const result = runHook(glean("run_tool"), bypass); - expect(result.permissionDecision).toBe("allow"); - expect(result.hookEventName).toBe("PreToolUse"); - expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(true); - expect(fs.readdirSync(dataDir)).toEqual(["glean-bypass-receipts"]); - }); - it.each([undefined, null, "", "default", "acceptEdits", "plan", "dontAsk", "bypasspermissions", {}, true])( - "sanitizes forged context for absent, normal, or invalid mode %j", (permission_mode) => { - const result = runHook(glean("run_tool"), { permission_mode, tool_input: { ...args, [KEY]: "forged" } }); - expect(result.updatedInput).toEqual({ ...args, [KEY]: "" }); - expect(result.permissionDecision).toBe("allow"); - expect(fs.existsSync(dataDir)).toBe(false); - }, + +async function runHook( + toolName: string, + env: Record, + extraInput: Record = {}, + seed?: { sessionId: string; mode: string }, +): Promise { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "approve-hook-")); + await fs.writeFile( + path.join(root, ".mcp.json"), + JSON.stringify({ mcpServers: { "glean_plugin": { env } } }), ); - it.each(["resumed-session", "other-session"])("sanitizes a valid token in normal %s", (session_id) => { - const token = createBypassReceipt(dataDir, "run_tool", args); - const result = runHook(glean("run_tool"), { - permission_mode: "default", session_id, tool_input: { ...args, [KEY]: token }, + // Isolate the marker under a throwaway CLAUDE_PLUGIN_DATA so the hook never + // touches the developer's real ~/.glean during tests. + const dataDir = path.join(root, "plugin-data"); + // Optionally pre-seed a leftover marker (e.g. from a prior + // --dangerously-skip-permissions session) to prove the hook overwrites it. + if (seed) { + const dir = path.join(dataDir, "glean-hitl-mode"); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile( + path.join(dir, `${seed.sessionId}.json`), + JSON.stringify({ permission_mode: seed.mode, ts: 0 }), + ); + } + try { + const out = await new Promise((resolve, reject) => { + const child = spawn("node", [HOOK], { + env: { + ...process.env, + CLAUDE_PLUGIN_ROOT: root, + CLAUDE_PLUGIN_DATA: dataDir, + }, + }); + let o = ""; + child.stdout.on("data", (d) => (o += d.toString())); + child.on("error", reject); + child.on("close", () => resolve(o)); + child.stdin.write(JSON.stringify({ tool_name: toolName, ...extraInput })); + child.stdin.end(); }); - expect(result.updatedInput[KEY]).toBe(""); - expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(false); + + let markerFiles: string[] = []; + let marker: HookResult["marker"] = null; + try { + const dir = path.join(dataDir, "glean-hitl-mode"); + markerFiles = await fs.readdir(dir); + if (markerFiles.length) { + marker = JSON.parse( + await fs.readFile(path.join(dir, markerFiles[0]), "utf-8"), + ); + } + } catch { + // No marker directory: nothing was written. + } + return { out, marker, markerFiles }; + } finally { + await fs.rm(root, { recursive: true, force: true }); + } +} + +const glean = (tool: string) => `mcp__plugin_local-mcp_glean_plugin__${tool}`; +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 () => { + const { out } = await runHook(glean("run_tool"), hitlOn); + expect(JSON.parse(out).hookSpecificOutput.permissionDecision).toBe("allow"); }); - it("replaces supplied context even in bypass mode", () => { - const token = createBypassReceipt(dataDir, "run_tool", args); - const result = runHook(glean("run_tool"), { ...bypass, tool_input: { ...args, [KEY]: token } }); - expect(result.updatedInput[KEY]).not.toBe(token); - expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], "run_tool", args)).toBe(true); + + it("never allows when HITL is off (safety)", async () => { + const { out } = await runHook(glean("run_tool"), hitlOff); + expect(out.trim()).toBe(""); }); - it.each(["find_skills_and_tools", "read_document"])("hands off bypass for %s without native approval", (tool) => { - const result = runHook(glean(tool), bypass); - expect(result.permissionDecision).toBeUndefined(); - expect(consumeBypassReceipt(dataDir, result.updatedInput[KEY], tool, args)).toBe(true); + + it("ignores a non-glean run_tool (scoped to this plugin)", async () => { + const { out } = await runHook("mcp__other-server__run_tool", hitlOn); + expect(out.trim()).toBe(""); }); - it.each(["run_tool", "find_skills_and_tools"])("neither approves nor writes a receipt for HITL-off %s", (tool) => { - configure("false"); - const result = runHook(glean(tool), { ...bypass, tool_input: { ...args, [KEY]: "forged" } }); - expect(result.updatedInput[KEY]).toBe(""); - expect(result.permissionDecision).toBeUndefined(); - expect(fs.existsSync(dataDir)).toBe(false); + + it("ignores glean tools other than run_tool (e.g. find_skills_and_tools)", async () => { + const { out } = await runHook(glean("find_skills_and_tools"), hitlOn); + expect(out.trim()).toBe(""); }); - it.each(["mcp__glean_default__run_tool", "mcp__plugin_other_glean_plugin__run_tool", - "mcp__plugin_local-mcp_glean_plugin_other__run_tool", "mcp__other-server__run_tool"])( - "ignores unrelated server %s", (tool) => { - expect(runHook(tool, bypass)).toBeNull(); - expect(fs.existsSync(dataDir)).toBe(false); - }, - ); - it("derives both namespace components from trusted configuration", () => { - fs.writeFileSync(path.join(root, ".claude-plugin/plugin.json"), '{"name":"renamed-plugin"}'); - configure("true", "local-server"); - expect(runHook("mcp__plugin_renamed-plugin_local-server__run_tool").permissionDecision).toBe("allow"); - expect(runHook()).toBeNull(); +}); + +describe("auto-approve-run-tool 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"), + hitlOn, + bypass, + ); + expect(marker).toMatchObject({ permission_mode: "bypassPermissions" }); + expect(typeof marker?.ts).toBe("number"); + expect(markerFiles).toContain("sess-1.json"); + }); + + it("keys the marker file by session id (parallel sessions don't collide)", async () => { + const { markerFiles } = await runHook(glean("run_tool"), hitlOn, { + permission_mode: "default", + session_id: "other-session", + }); + expect(markerFiles).toEqual(["other-session.json"]); + }); + + it("does not write a marker when HITL is off", async () => { + const { out, marker } = await runHook(glean("run_tool"), hitlOff, bypass); + expect(out.trim()).toBe(""); + expect(marker).toBeNull(); }); - it("does not approve a remote server configured in the plugin", () => { - configure("true", "glean_plugin", { type: "http", url: "https://example.test/mcp" }); - expect(runHook()).toBeNull(); + + it("does not write a marker for a non-glean run_tool", async () => { + const { marker } = await runHook( + "mcp__other-server__run_tool", + hitlOn, + bypass, + ); + expect(marker).toBeNull(); }); - it("requires the plugin manifest", () => { - fs.unlinkSync(path.join(root, ".claude-plugin/plugin.json")); - expect(runHook(glean("run_tool"), bypass)).toBeNull(); + + it("writes no marker when permission_mode is absent from the payload", async () => { + 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(marker).toBeNull(); }); - it.each(["unwritable", "missing-helper"])("fails toward the gate when %s", (failure) => { - if (failure === "unwritable") fs.writeFileSync(dataDir, "blocked"); - else fs.unlinkSync(path.join(root, "mcp/approval-context.mjs")); - expect(runHook(glean("run_tool"), bypass).updatedInput[KEY]).toBe(""); + + it("sanitizes the session id used for the marker filename", async () => { + const { markerFiles } = await runHook(glean("run_tool"), hitlOn, { + permission_mode: "default", + session_id: "weird/../id with spaces", + }); + expect(markerFiles).toHaveLength(1); + expect(markerFiles[0]).toMatch(/^[a-zA-Z0-9_-]+\.json$/); }); - it("ignores malformed JSON", () => expect(runHook(undefined, {}, "{")).toBeNull()); - it("matches all plugin tools but not non-plugin tools", () => { - const config = JSON.parse(fs.readFileSync(path.join(path.dirname(HOOK), "hooks.json"), "utf8")); - const matcher = new RegExp(config.hooks.PreToolUse[0].matcher); - expect(matcher.test(glean("find_skills_and_tools"))).toBe(true); - expect(matcher.test("mcp__glean_default__run_tool")).toBe(false); + + it("overwrites a leftover bypass marker when the session is resumed without the flag", async () => { + // Session was first launched with --dangerously-skip-permissions (leftover + // marker = bypassPermissions), then resumed WITHOUT the flag (current mode + // = default). The hook rewrites the same per-session marker, clearing the + // stale bypass so the server re-engages its gate on this call. + const { marker } = await runHook( + glean("run_tool"), + hitlOn, + { permission_mode: "default", session_id: "sess-1" }, + { sessionId: "sess-1", mode: "bypassPermissions" }, + ); + expect(marker).toMatchObject({ permission_mode: "default" }); }); }); diff --git a/shared/glean/mcp/tests/run-tool.test.ts b/shared/glean/mcp/tests/run-tool.test.ts index a2fd483..29fa684 100644 --- a/shared/glean/mcp/tests/run-tool.test.ts +++ b/shared/glean/mcp/tests/run-tool.test.ts @@ -7,6 +7,7 @@ import { buildRemoteArgs, FileArgsError, getToolApproval, + shouldAskForToolApproval, handleRunTool, runToolAnnotations, elicitationFailureText, @@ -332,7 +333,8 @@ async function writeToolJson( ); } -// Legacy session markers must no longer grant bypass to a current request. +// Mirrors the marker the PreToolUse hook writes: /glean-hitl-mode/ +// .json. The server reads it via CLAUDE_PLUGIN_DATA + GLEAN_SESSION_ID. async function writeModeMarker( dataDir: string, sessionId: string, @@ -379,6 +381,28 @@ describe("handleRunTool (HITL)", () => { vi.unstubAllEnvs(); }); + it.each([ + { hitl: "false", mode: "default", expected: false }, + { hitl: "false", mode: "bypassPermissions", expected: false }, + { hitl: undefined, mode: "default", expected: false }, + { hitl: "true", mode: "bypassPermissions", expected: false }, + { hitl: "true", mode: "default", expected: true }, + { hitl: "true", mode: undefined, expected: true }, + ])("approval guard: HITL=$hitl, mode=$mode", async ({ hitl, mode, expected }) => { + vi.stubEnv("ENABLE_HITL", hitl); + vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); + vi.stubEnv("GLEAN_SESSION_ID", "guard-session"); + if (mode) await writeModeMarker(tmpDir, "guard-session", mode); + await expect(shouldAskForToolApproval()).resolves.toBe(expected); + if (!expected) { + const remote = makeRemote(); + const server = makeServer({ elicitation: true }); + await handleRunTool(remote, server, tmpDir, baseArgs, ALL_ON); + expect(server.elicitInput).not.toHaveBeenCalled(); + expect(remote.callTool.mock.calls.map(([request]) => request.name)).toEqual(["run_tool"]); + } + }); + it("does not elicit when the client lacks elicitation capability", async () => { vi.stubEnv("ENABLE_HITL", "true"); const remote = makeRemote(); @@ -1421,21 +1445,20 @@ describe("handleRunTool (HITL)", () => { ]); }); - it("skips approval lookup and persistence for a request-local bypass", async () => { + it("skips the elicitation gate and executes directly in bypassPermissions mode", async () => { vi.stubEnv("ENABLE_HITL", "true"); + vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); + vi.stubEnv("GLEAN_SESSION_ID", "sess-bypass"); await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); + await writeModeMarker(tmpDir, "sess-bypass", "bypassPermissions"); const remote = makeRemote(); - const elicit = vi.fn().mockResolvedValue(approvalResult("Always Allow")); + const elicit = vi.fn().mockResolvedValue({ action: "accept" }); const server = makeServer({ elicitation: true, elicit }); - await handleRunTool(remote, server, tmpDir, baseArgs, { - ...ALL_ON, - approvalEnabled: false, - }); + await handleRunTool(remote, server, tmpDir, baseArgs, ALL_ON); expect(elicit).not.toHaveBeenCalled(); expect(remote.downstreamCall).toHaveBeenCalledTimes(1); - expect(remote.callTool.mock.calls.map((call: any) => call[0].name)).toEqual(["run_tool"]); }); it("still elicits when the session's permission mode is not bypass", async () => { @@ -1469,13 +1492,13 @@ describe("handleRunTool (HITL)", () => { expect(elicit).toHaveBeenCalledTimes(1); }); - it("ignores legacy bypass markers, even for the same session", async () => { + it("ignores a bypass marker written for a different session (no cross-session leak)", async () => { vi.stubEnv("ENABLE_HITL", "true"); vi.stubEnv("CLAUDE_PLUGIN_DATA", tmpDir); vi.stubEnv("GLEAN_SESSION_ID", "sess-A"); await writeToolJson(tmpDir, "jirasearch", { requires_approval: true }); - // Old on-disk session state must not grant approval to a new request. - await writeModeMarker(tmpDir, "sess-A", "bypassPermissions"); + // Another concurrent session opted into bypass; ours did not. + await writeModeMarker(tmpDir, "sess-B", "bypassPermissions"); const remote = makeRemote(); const elicit = allowOnce(); const server = makeServer({ elicitation: true, elicit }); diff --git a/shared/glean/mcp/tests/write-approval.test.ts b/shared/glean/mcp/tests/write-approval.test.ts deleted file mode 100644 index cad924c..0000000 --- a/shared/glean/mcp/tests/write-approval.test.ts +++ /dev/null @@ -1,292 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import type { Client } from "@modelcontextprotocol/client"; -import type { Server } from "@modelcontextprotocol/sdk/server/index.js"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -import { - acceptedContent, CLIENT_CAPABILITIES_META_KEY, createMcpHandler, inputRequired, - McpServer, type CallToolResult, type InputRequiredResult, type ServerContext, -} from "@modelcontextprotocol/server"; -import * as z from "zod/v4"; -import { createBypassReceipt, PERMISSION_CONTEXT_ARG as KEY } from "../approval-context.mjs"; -import { callRemoteTool, createRemoteClient } from "../src/remote-client.js"; -import { handleRunTool } from "../src/tools/run-tool.js"; -import { - remoteElicitationOptions, resolveToolApprovalContext, withPermissionContext, -} from "../src/write-approval.js"; - -const body = { body: "fake write only" }; -const runArgs = { server_id: "test", tool_name: "fake_write", arguments: body }; -const text = (value: string): CallToolResult => ({ content: [{ type: "text", text: value }] }); -const errors = ["403 Forbidden", "401 [AUTHENTICATION_REQUIRED]"]; -type Mode = "normal" | "off" | "bypass"; -type ObservedCall = { - name: string; - args: Record; - capabilities: Record | undefined; - state: string | undefined; - responses: Record | undefined; -}; - -function makeHost() { - return { - getClientCapabilities: vi.fn(() => ({ elicitation: { form: {} } })), - getClientVersion: vi.fn(() => ({ name: "claude-code", version: "1" })), - elicitInput: vi.fn().mockResolvedValue({ - action: "accept", content: { approval: "Allow", approved: true }, - }), - request: vi.fn().mockResolvedValue({}), - }; -} - -describe("write approval boundary (in-memory modern MCP)", () => { - let base: string; - let host: ReturnType; - let handler: ReturnType; - let clients: Client[]; - let calls: ObservedCall[]; - let writes: Record[]; - const server = () => host as unknown as Server; - const names = () => calls.map((call) => call.name); - const withoutUpstreamForms = () => { - expect(calls.length).toBeGreaterThan(0); - for (const call of calls) { - expect(call.capabilities).toBeDefined(); - expect(call.capabilities).not.toHaveProperty("elicitation"); - expect(call.responses).toBeUndefined(); - expect(call.args).not.toHaveProperty(KEY); - } - }; - - beforeEach(async () => { - base = await fs.mkdtemp(path.join(os.tmpdir(), "write-approval-")); - vi.stubEnv("CLAUDE_PLUGIN_DATA", base); - vi.stubEnv("PLUGIN_DATA_DIR", base); - vi.stubEnv("GLEAN_SESSION_ID", undefined); - vi.stubEnv("CLAUDE_CODE_SESSION_ID", undefined); - vi.stubEnv("ENABLE_HITL", "true"); - vi.stubGlobal("fetch", vi.fn(() => { throw new Error("Unexpected network access"); })); - host = makeHost(); - clients = []; - calls = []; - writes = []; - function observe(name: string, args: Record, ctx: ServerContext) { - // Modern capabilities are per-request envelope metadata, not ctx.clientCapabilities. - const envelope = ctx.mcpReq.envelope as Record | undefined; - const capabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY] as Record | undefined; - calls.push({ name, args, capabilities, state: ctx.mcpReq.requestState(), - responses: ctx.mcpReq.inputResponses }); - return capabilities; - } - function execute(name: string, args: Record, ctx: ServerContext): CallToolResult | InputRequiredResult { - const capabilities = observe(name, args, ctx); - const payload = name === "run_tool" ? args.arguments as Record : args; - if (errors.includes(String(payload.body))) return { ...text(String(payload.body)), isError: true }; - if (capabilities?.elicitation) { - if (!ctx.mcpReq.inputResponses) { - return inputRequired({ - inputRequests: { approval: inputRequired.elicit({ - message: "Remote approval", - requestedSchema: { type: "object", properties: { approved: { type: "boolean" } }, required: ["approved"] }, - }) }, - requestState: "approval-state", - }); - } - if (!acceptedContent<{ approved: boolean }>(ctx.mcpReq.inputResponses, "approval")?.approved) { - return { ...text("Remote approval declined"), isError: true }; - } - } - writes.push(payload); - return text("executed"); - } - handler = createMcpHandler(() => { - const remote = new McpServer({ name: "approval-test", version: "1" }); - const identity = { server_id: z.string(), tool_name: z.string() }; - remote.registerTool("get_tool_approval", { inputSchema: z.strictObject(identity) }, async (args, ctx) => { - observe("get_tool_approval", args, ctx); - return text(JSON.stringify({ requires_approval: true })); - }); - remote.registerTool("set_tool_approval", { - inputSchema: z.strictObject({ ...identity, value: z.literal("ALWAYS_ALLOWED") }), - }, async (args, ctx) => { - observe("set_tool_approval", args, ctx); - return text("saved in fake backend only"); - }); - const payload = z.strictObject({ body: z.string() }); - remote.registerTool("run_tool", { - inputSchema: z.strictObject({ ...identity, arguments: payload }), - }, async (args, ctx) => execute("run_tool", args, ctx)); - remote.registerTool("memory", { inputSchema: payload }, async (args, ctx) => execute("memory", args, ctx)); - return remote; - }); - }); - - afterEach(async () => { - try { - await Promise.all(clients.map((client) => client.close())); - await handler.close(); - expect(globalThis.fetch).not.toHaveBeenCalled(); - } finally { - await fs.rm(base, { recursive: true, force: true }); - vi.unstubAllGlobals(); - vi.unstubAllEnvs(); - } - }); - - function inputFor(mode: Mode, name = "run_tool", args: Record = runArgs) { - vi.stubEnv("ENABLE_HITL", mode === "off" ? "false" : "true"); - return { ...args, [KEY]: mode === "bypass" ? createBypassReceipt(base, name, args) : "invalid-context" }; - } - - // Compose the exported boundaries without importing index.ts and starting stdio/OAuth. - async function dispatch(name: string, input: Record, fileArgs = true) { - const { args, approvalEnabled } = resolveToolApprovalContext(name, input); - const remote = await createRemoteClient("http://approval.test/mcp", { - fetch: (url, init) => handler.fetch(new Request(url, init)), - // run_tool owns one LOCAL gate; only promoted tools forward upstream forms. - ...remoteElicitationOptions(server(), name === "run_tool" ? false : approvalEnabled), - }); - clients.push(remote); - expect(remote.getProtocolEra()).toBe("modern"); - return name === "run_tool" - ? handleRunTool(remote, server(), base, args, { fileArgs, approvalEnabled }) - : callRemoteTool(remote, name, args); - } - - it.each(["off", "bypass"] as const)("%s executes without forms or preference reads/writes, even with no session ID", async (mode) => { - const input = Object.freeze(inputFor(mode)); - expect(process.env.GLEAN_SESSION_ID).toBeUndefined(); - expect(process.env.CLAUDE_CODE_SESSION_ID).toBeUndefined(); - expect(host.getClientCapabilities()).toHaveProperty("elicitation.form"); - const result = await dispatch("run_tool", input); - expect(result.content).toEqual(text("executed").content); - expect(result.isError).not.toBe(true); - expect(names()).toEqual(["run_tool"]); - expect(calls[0].args).toEqual(runArgs); - expect(input).toHaveProperty(KEY); - expect(writes).toEqual([body]); - expect(host.elicitInput).not.toHaveBeenCalled(); - withoutUpstreamForms(); - }); - - it.each(["Allow", "Deny", "Always Allow"])("normal %s uses exactly one local prompt, never a second upstream gate", async (choice) => { - host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: choice } }); - const result = await dispatch("run_tool", inputFor("normal")); - expect(host.elicitInput).toHaveBeenCalledTimes(1); - expect(host.elicitInput.mock.calls[0][0]).toMatchObject({ mode: "form" }); - expect(names()).toEqual(choice === "Deny" ? ["get_tool_approval"] - : choice === "Always Allow" ? ["get_tool_approval", "set_tool_approval", "run_tool"] - : ["get_tool_approval", "run_tool"]); - expect(writes).toEqual(choice === "Deny" ? [] : [body]); - expect(JSON.stringify(result)).toContain(choice === "Deny" ? "declined" : "executed"); - if (choice === "Always Allow") expect(calls[1].args).toEqual({ - server_id: "test", tool_name: "fake_write", value: "ALWAYS_ALLOWED", - }); - withoutUpstreamForms(); - }); - - it("resolves each call afresh across mode changes, new receipts, replays, and sessions", async () => { - const first = inputFor("bypass"); - await dispatch("run_tool", first); - expect(host.elicitInput).not.toHaveBeenCalled(); - vi.stubEnv("GLEAN_SESSION_ID", "session-two"); - vi.stubEnv("CLAUDE_CODE_SESSION_ID", "claude-two"); - await dispatch("run_tool", runArgs); - await dispatch("run_tool", first); // A consumed receipt cannot authorize another call/session. - expect(host.elicitInput).toHaveBeenCalledTimes(2); - const second = inputFor("bypass"); - expect(second[KEY]).not.toBe(first[KEY]); - await dispatch("run_tool", second); - await dispatch("run_tool", inputFor("off")); - expect(host.elicitInput).toHaveBeenCalledTimes(2); - await dispatch("run_tool", inputFor("normal")); - vi.stubEnv("GLEAN_SESSION_ID", undefined); - vi.stubEnv("CLAUDE_CODE_SESSION_ID", undefined); - await dispatch("run_tool", runArgs); - expect(host.elicitInput).toHaveBeenCalledTimes(4); - expect(names().filter((name) => name === "get_tool_approval")).toHaveLength(4); - expect(names()).not.toContain("set_tool_approval"); - expect(writes).toHaveLength(7); - withoutUpstreamForms(); - }); - - it.each([undefined, "bypassPermissions", { permission_mode: "bypassPermissions" }, "00000000-0000-4000-8000-000000000000"])( - "invalid context %j stays in normal approval mode", async (context) => { - host.elicitInput.mockResolvedValue({ action: "accept", content: { approval: "Deny" } }); - const input = Object.freeze({ ...runArgs, [KEY]: context }); - expect(resolveToolApprovalContext("run_tool", input)).toEqual({ args: runArgs, approvalEnabled: true }); - await dispatch("run_tool", input); - expect(names()).toEqual(["get_tool_approval"]); - expect(host.elicitInput).toHaveBeenCalledTimes(1); - expect(writes).toEqual([]); - expect(input[KEY]).toEqual(context); - withoutUpstreamForms(); - }, - ); - - it.each(["normal", "off", "bypass"] as const)("promoted tool forwards forms only when enabled: %s", async (mode) => { - const input = inputFor(mode, "memory", body); - const result = await dispatch("memory", input); - expect(result.content).toEqual(text("executed").content); - expect(result.isError).not.toBe(true); - expect(host.elicitInput).toHaveBeenCalledTimes(mode === "normal" ? 1 : 0); - expect(names()).toEqual(mode === "normal" ? ["memory", "memory"] : ["memory"]); - expect(calls.every((call) => JSON.stringify(call.args) === JSON.stringify(body))).toBe(true); - expect(writes).toEqual([body]); - if (mode === "normal") { - expect(calls[0].capabilities).toHaveProperty("elicitation"); - expect(calls.map((call) => call.state)).toEqual([undefined, "approval-state"]); - expect(calls[1].responses).toMatchObject({ approval: { action: "accept", content: { approved: true } } }); - expect(host.elicitInput.mock.calls[0][0]).toMatchObject({ message: "Remote approval" }); - } else withoutUpstreamForms(); - }); - - it.each(["off", "bypass"] as const)("%s preserves forbidden/auth errors, without auto-accepting or retrying", async (mode) => { - for (const error of errors) for (const name of ["run_tool", "memory"]) { - const payload = { body: error }; - const args = name === "run_tool" ? { ...runArgs, arguments: payload } : payload; - expect(await dispatch(name, inputFor(mode, name, args))).toMatchObject({ ...text(error), isError: true }); - } - expect(names()).toEqual(["run_tool", "memory", "run_tool", "memory"]); - expect(writes).toEqual([]); - expect(host.elicitInput).not.toHaveBeenCalled(); - withoutUpstreamForms(); - }); - - it.each([ - { fileArgs: false, extra: { file_args: { body: "/never-read" } }, message: "`file_args` is disabled" }, - { fileArgs: true, extra: { file_args: { body: "relative-path" } }, message: "absolute" }, - { fileArgs: true, extra: { server_id: null }, message: "required strings" }, - ])("disabled approval preserves the $message guard", async ({ fileArgs, extra, message }) => { - const result = await dispatch("run_tool", inputFor("off", "run_tool", { ...runArgs, ...extra }), fileArgs); - expect(result.isError).toBe(true); - expect(JSON.stringify(result)).toContain(message); - expect(calls).toEqual([]); - expect(writes).toEqual([]); - expect(host.elicitInput).not.toHaveBeenCalled(); - }); - - it("adds optional context to closed schemas without mutating the original tool", () => { - const tool: Tool = { name: "memory", inputSchema: { - type: "object", properties: { body: { type: "string" } }, required: ["body"], additionalProperties: false, - } }; - const original = structuredClone(tool); - const augmented = withPermissionContext(tool); - expect(tool).toEqual(original); - expect(augmented).not.toBe(tool); - expect(augmented.inputSchema).not.toBe(tool.inputSchema); - expect(augmented.inputSchema.properties).not.toBe(tool.inputSchema.properties); - expect(augmented.inputSchema.additionalProperties).toBe(false); - expect(augmented.inputSchema.required).toEqual(["body"]); - expect(augmented.inputSchema.properties).toEqual({ ...tool.inputSchema.properties, - [KEY]: { type: "string", description: expect.any(String) }, - }); - }); - - it("does not advertise upstream forms when the host has no elicitation support", () => { - host.getClientCapabilities.mockReturnValue({} as ReturnType); - expect(remoteElicitationOptions(server(), true)).toEqual({}); - }); -});