Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
57 changes: 55 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,45 @@ 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).
// This resolution MUST match the hook's exactly. The hook cannot see the
// server-only PLUGIN_DATA_DIR that start.sh derives, so both sides key off
// CLAUDE_PLUGIN_DATA (falling back to ~/.glean) — the one anchor available to
// both processes. Under start.sh, PLUGIN_DATA_DIR resolves to this same path.
function permissionModeMarkerPath(): string {
const base =
process.env.CLAUDE_PLUGIN_DATA || path.join(os.homedir(), ".glean");

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now this is clean because this is only applicable to claude and cursor is not regressed because of this

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.
//
// 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<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 +329,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
133 changes: 122 additions & 11 deletions tests/auto-approve-hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,69 @@ 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> = {},
seed?: { sessionId: string; mode: string },
): 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");
// 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 {
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 +82,94 @@ 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$/);
});

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