Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/glean/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "glean-vnext",
"version": "0.2.36",
"version": "0.2.37",
"description": "Glean plugin for discovering skills and running tools.",
"author": {
"name": "Glean"
Expand Down
2 changes: 1 addition & 1 deletion plugins/glean/.codex-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "glean-vnext",
"version": "0.2.36",
"version": "0.2.37",
"description": "Glean Codex plugin for discovering skills and running tools.",
"author": {
"name": "Glean"
Expand Down
2 changes: 1 addition & 1 deletion plugins/glean/.cursor-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "glean-vnext",
"displayName": "Glean vNext",
"version": "0.2.36",
"version": "0.2.37",
"description": "Search and act across your company's apps — Jira, Slack, Salesforce, Google Workspace, and more — without leaving Cursor.",
"author": {
"name": "Glean"
Expand Down
20 changes: 18 additions & 2 deletions plugins/glean/dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions plugins/glean/hooks/auto-approve-run-tool.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// 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";

function readStdin() {
Expand Down Expand Up @@ -49,6 +50,34 @@ try {
}

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() }),
Comment thread
eshwar-sundar-glean marked this conversation as resolved.
);
}
} catch {
// Ignore: a failed marker write just means the server keeps prompting.
}

process.stdout.write(
JSON.stringify({
hookSpecificOutput: {
Expand Down
50 changes: 48 additions & 2 deletions src/tools/run-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ 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 { buildCompactArgs, writeApprovalArgsFile } from "./approval-args.js";
import { resolveSessionId } from "../session-id.js";

const DEFAULT_FILE_ARG_MAX_BYTES = 1 * 1024 * 1024;

Expand Down Expand Up @@ -240,6 +242,38 @@ 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 base dir and session-id sanitization MUST stay in lockstep with the hook
// and with start.sh's PLUGIN_DATA_DIR export, or the server reads a path the
// hook never wrote and silently falls back to prompting.
function permissionModeMarkerPath(): string {
const base =
process.env.PLUGIN_DATA_DIR ||
Comment thread
eshwar-sundar-glean marked this conversation as resolved.
Outdated
process.env.CLAUDE_PLUGIN_DATA ||
os.tmpdir();
Comment thread
eshwar-sundar-glean marked this conversation as resolved.
Outdated
const sessionId = resolveSessionId()
.replace(/[^a-zA-Z0-9_-]/g, "-")
.slice(0, 64);
return path.join(base, "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.
async function currentPermissionMode(): Promise<string | null> {
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 handleRunTool(
remoteClient: Client,
mcpServer: Server,
Expand Down Expand Up @@ -288,8 +322,20 @@ export async function handleRunTool(
}

const hitlEnabled = process.env.ENABLE_HITL === "true";
if (hitlEnabled && mcpServer.getClientCapabilities()?.elicitation) {
if (toolMeta?.requires_approval) {
if (
hitlEnabled &&
toolMeta?.requires_approval &&
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 message = await buildApprovalMessage(
mcpServer,
toolName,
Expand Down
108 changes: 97 additions & 11 deletions tests/auto-approve-hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,58 @@ const HOOK = path.resolve(
"../plugins/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<string, string>,
): Promise<string> {
extraInput: Record<string, unknown> = {},
): Promise<HookResult> {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "approve-hook-"));
await fs.writeFile(
path.join(root, ".mcp.json"),
JSON.stringify({ mcpServers: { glean: { 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");
try {
return await new Promise<string>((resolve, reject) => {
const out = await new Promise<string>((resolve, reject) => {
const child = spawn("node", [HOOK], {
env: { ...process.env, CLAUDE_PLUGIN_ROOT: root },
env: {
...process.env,
CLAUDE_PLUGIN_ROOT: root,
CLAUDE_PLUGIN_DATA: dataDir,
},
});
let out = "";
child.stdout.on("data", (d) => (out += d.toString()));
let o = "";
child.stdout.on("data", (d) => (o += d.toString()));
child.on("error", reject);
child.on("close", () => resolve(out));
child.stdin.write(JSON.stringify({ tool_name: toolName }));
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 });
}
Expand All @@ -40,25 +71,80 @@ async function runHook(
const glean = (tool: string) => `mcp__plugin_glean-vnext_glean__${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);
const { out } = await runHook(glean("run_tool"), hitlOn);
expect(JSON.parse(out).hookSpecificOutput.permissionDecision).toBe("allow");
});

it("never allows when HITL is off (safety)", async () => {
const out = await runHook(glean("run_tool"), hitlOff);
const { out } = await runHook(glean("run_tool"), hitlOff);
expect(out.trim()).toBe("");
});

it("ignores a non-glean run_tool (scoped to this plugin)", async () => {
const out = await runHook("mcp__other-server__run_tool", hitlOn);
const { out } = await runHook("mcp__other-server__run_tool", hitlOn);
expect(out.trim()).toBe("");
});

it("ignores glean tools other than run_tool (e.g. find_skills)", async () => {
const out = await runHook(glean("find_skills"), hitlOn);
const { out } = await runHook(glean("find_skills"), hitlOn);
expect(out.trim()).toBe("");
});
});

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 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("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("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$/);
});
});
Loading
Loading