mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
fix: honor host opt-out for allowDangerouslySkipPermissions (#1129)
## Summary Closes #1119. Supersedes #1120 by @amansk (their commits are included on this branch; the fork does not allow maintainer edits, so review fixes landed here instead). Hosts that never use `bypassPermissions` can opt a session out by sending `_meta.claudeCode.options.allowDangerouslySkipPermissions: false` on `session/new` (and again on `session/load`). When omitted or `true`, behavior is unchanged: bypass is available on non-root sessions. ## Design Bypass availability is decided once per session in `createSession`: ```ts const allowBypass = ALLOW_BYPASS && sessionMeta?.claudeCode?.options?.allowDangerouslySkipPermissions !== false; ``` and threaded to the three places that must agree: - the SDK `allowDangerouslySkipPermissions` flag (the actual enforcement; the CLI refuses bypass without it) - the spawn-time mode clamp in `resolvePermissionMode`, so a `permissions.defaultMode` of `bypassPermissions` (or a mode carried across a clear-context restart) becomes `default` before `query()` rather than failing at spawn - the session mode catalog in `SessionModeManager.initialize`, so the client is never offered a mode the CLI would reject ## Changes from #1120 - Fold the host opt-out into `resolvePermissionMode` via an `allowBypass` parameter instead of a second inline clamp; drop the separate `resolveAllowDangerouslySkipPermissions` helper - Make `allowBypass` a required `initialize` parameter and remove the module-global `ALLOW_BYPASS` fallback from `session-mode.ts` - Fix `tsc` errors in the new tests (`modes` is optional on `NewSessionResponse`), which broke the CI build job - Move the settings-driven clamp test onto the shared `acp-agent-settings` fixture (isolated cwd, no double module reload, passes as root) - Document the opt-out in `docs/permission-extension.md` ## Testing - `npm run check`, `npm run build` — pass - `npm run test:run` — pass (the branch is based before #1123, so the SDK version guard in `model-presentation.test.ts` only passes with the pinned 0.3.257; it clears on rebase) Co-authored-by: Cursor Agent <cursoragent@cursor.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amandeep Khurana <amansk@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -164,6 +164,11 @@ The session exposes the Claude modes available for the current model using Claud
|
||||
internal `dontAsk` SDK mode is accepted from settings for compatibility but is not advertised as a
|
||||
user-selectable mode.
|
||||
|
||||
Hosts can remove `Bypass permissions` from a session by sending
|
||||
`_meta.claudeCode.options.allowDangerouslySkipPermissions: false` on `session/new` (and again on
|
||||
`session/load`); the mode leaves the catalog and a `bypassPermissions` settings default clamps to
|
||||
`default`.
|
||||
|
||||
The ACP mode ids are the Claude SDK wire ids: `default`, `acceptEdits`, `plan`, `auto`, and
|
||||
`bypassPermissions`. `Manual` deliberately retains the SDK id `default`; `manual` is only a settings
|
||||
input alias. The adapter does not invent parallel ids for presentation labels.
|
||||
|
||||
@@ -1167,7 +1167,6 @@ export type NewSessionMeta = {
|
||||
* Those parameters will not be forwarded because they are managed by ACP:
|
||||
* - cwd
|
||||
* - includePartialMessages
|
||||
* - allowDangerouslySkipPermissions
|
||||
* - permissionMode
|
||||
* - canUseTool
|
||||
* - executable
|
||||
@@ -1178,6 +1177,8 @@ export type NewSessionMeta = {
|
||||
* - mcpServers (merged with ACP's mcpServers)
|
||||
* - disallowedTools (merged with ACP's disallowedTools)
|
||||
* - tools (passed through; defaults to claude_code preset if not provided)
|
||||
* - allowDangerouslySkipPermissions (set to `false` to remove bypassPermissions
|
||||
* from this session; repeat it on session/load. `true` cannot override root)
|
||||
*/
|
||||
options?: Options;
|
||||
/**
|
||||
@@ -7926,14 +7927,20 @@ export class ClaudeAcpAgent {
|
||||
}
|
||||
}
|
||||
|
||||
const permissionMode = resolvePermissionMode(
|
||||
settingsManager.getSettings().permissions?.defaultMode,
|
||||
this.logger,
|
||||
);
|
||||
const initialPermissionMode = creationOpts.permissionMode ?? permissionMode;
|
||||
|
||||
// Extract options from _meta if provided
|
||||
const sessionMeta = params._meta as NewSessionMeta | undefined;
|
||||
// Bypass is off for root outside a sandbox, and hosts may opt a session out.
|
||||
// Decided once here: it gates the SDK flag, the spawn-time mode (the SDK
|
||||
// rejects bypassPermissions without the flag), and the mode catalog.
|
||||
const allowBypass =
|
||||
ALLOW_BYPASS && sessionMeta?.claudeCode?.options?.allowDangerouslySkipPermissions !== false;
|
||||
|
||||
const initialPermissionMode = resolvePermissionMode(
|
||||
creationOpts.permissionMode ?? settingsManager.getSettings().permissions?.defaultMode,
|
||||
this.logger,
|
||||
allowBypass,
|
||||
);
|
||||
|
||||
const userProvidedOptions = sessionMeta?.claudeCode?.options
|
||||
? { ...sessionMeta.claudeCode.options }
|
||||
: undefined;
|
||||
@@ -8079,9 +8086,7 @@ export class ClaudeAcpAgent {
|
||||
? { [FILE_CHANGE_AUDIT_SERVER_NAME]: fileChangeAuditSupport.mcpServer }
|
||||
: {}),
|
||||
},
|
||||
// If we want bypassPermissions to be an option, we have to allow it here.
|
||||
// But it doesn't work in root mode, so we only activate it if it will work.
|
||||
allowDangerouslySkipPermissions: ALLOW_BYPASS,
|
||||
allowDangerouslySkipPermissions: allowBypass,
|
||||
permissionMode: initialPermissionMode,
|
||||
canUseTool: this.canUseTool(sessionId),
|
||||
// Forward MCP elicitation requests onto ACP elicitation. Only attached
|
||||
@@ -8343,6 +8348,7 @@ export class ClaudeAcpAgent {
|
||||
requestedMode: initialPermissionMode,
|
||||
currentModelInfo,
|
||||
currentModelId: models.currentModelId,
|
||||
allowBypass,
|
||||
});
|
||||
timing.phase("modes");
|
||||
|
||||
|
||||
@@ -23,9 +23,12 @@ const PERMISSION_MODE_ALIASES: Record<string, PermissionMode> = {
|
||||
bypass: "bypassPermissions",
|
||||
};
|
||||
|
||||
/** When `allowBypass` is false, `bypassPermissions` clamps to `default` so the SDK
|
||||
* is never spawned in a mode it would reject. */
|
||||
export function resolvePermissionMode(
|
||||
defaultMode?: unknown,
|
||||
logger: PermissionModeLogger = console,
|
||||
allowBypass: boolean = ALLOW_BYPASS,
|
||||
): PermissionMode {
|
||||
if (defaultMode === undefined) {
|
||||
return "default";
|
||||
@@ -48,10 +51,8 @@ export function resolvePermissionMode(
|
||||
return "default";
|
||||
}
|
||||
|
||||
if (mapped === "bypassPermissions" && !ALLOW_BYPASS) {
|
||||
logger.error(
|
||||
"Ignoring permissions.defaultMode from settings: bypassPermissions is not available when running as root.",
|
||||
);
|
||||
if (mapped === "bypassPermissions" && !allowBypass) {
|
||||
logger.error("Ignoring permission mode bypassPermissions: not available in this session.");
|
||||
return "default";
|
||||
}
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ import type {
|
||||
PermissionResult,
|
||||
Query,
|
||||
} from "@anthropic-ai/claude-agent-sdk";
|
||||
import { ALLOW_BYPASS } from "./permissions/modes.js";
|
||||
|
||||
export const MODE_CONFIG_ID = "mode";
|
||||
export const AUTO_MODE_FALLBACK: PermissionMode = "acceptEdits";
|
||||
@@ -49,6 +48,8 @@ type InitializeSessionModeParams = {
|
||||
requestedMode: PermissionMode;
|
||||
currentModelInfo?: ModelInfo;
|
||||
currentModelId: string;
|
||||
/** When false, bypassPermissions is omitted from the mode catalog. */
|
||||
allowBypass: boolean;
|
||||
};
|
||||
|
||||
/** Owns session-mode policy and the ACP/SDK synchronization it requires. */
|
||||
@@ -60,11 +61,12 @@ export class SessionModeManager<S extends SessionMode> {
|
||||
requestedMode,
|
||||
currentModelInfo,
|
||||
currentModelId,
|
||||
allowBypass,
|
||||
}: InitializeSessionModeParams): Promise<{
|
||||
modes: SessionModeState;
|
||||
autoModeFallbackWarningPending: boolean;
|
||||
}> {
|
||||
const availableModes = this.buildAvailableModes();
|
||||
const availableModes = this.buildAvailableModes(allowBypass);
|
||||
let effectiveMode = requestedMode;
|
||||
let autoModeFallbackWarningPending = false;
|
||||
|
||||
@@ -281,7 +283,7 @@ export class SessionModeManager<S extends SessionMode> {
|
||||
}
|
||||
}
|
||||
|
||||
private buildAvailableModes(): SessionModeState["availableModes"] {
|
||||
private buildAvailableModes(allowBypass: boolean): SessionModeState["availableModes"] {
|
||||
const modes: SessionModeState["availableModes"] = [
|
||||
{
|
||||
id: "default",
|
||||
@@ -308,7 +310,7 @@ export class SessionModeManager<S extends SessionMode> {
|
||||
_meta: { kind: "auto_review" },
|
||||
},
|
||||
];
|
||||
if (ALLOW_BYPASS) {
|
||||
if (allowBypass) {
|
||||
modes.push({
|
||||
id: "bypassPermissions",
|
||||
name: "Bypass permissions",
|
||||
|
||||
@@ -262,6 +262,37 @@ describe("ClaudeAcpAgent settings", () => {
|
||||
expect(response.modes.currentModeId).toBe("default");
|
||||
});
|
||||
|
||||
it("honors the host opt-out from bypassPermissions", async () => {
|
||||
await fs.promises.writeFile(
|
||||
path.join(tempDir, "settings.json"),
|
||||
JSON.stringify({ permissions: { defaultMode: "bypassPermissions" } }),
|
||||
);
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
const { getCapturedOptions } = mockQuery();
|
||||
|
||||
const { ClaudeAcpAgent } = await import("../acp-agent.js");
|
||||
const agent: ClaudeAcpAgentType = new ClaudeAcpAgent(createMockClient());
|
||||
(agent as any).logger = { log: () => {}, error: () => {} };
|
||||
|
||||
const response = await (agent as any).createSession({
|
||||
cwd: projectDir,
|
||||
mcpServers: [],
|
||||
_meta: {
|
||||
disableBuiltInTools: true,
|
||||
claudeCode: { options: { allowDangerouslySkipPermissions: false } },
|
||||
},
|
||||
});
|
||||
|
||||
expect(getCapturedOptions().allowDangerouslySkipPermissions).toBe(false);
|
||||
expect(getCapturedOptions().permissionMode).toBe("default");
|
||||
expect(response.modes.currentModeId).toBe("default");
|
||||
expect(response.modes.availableModes.map((mode: { id: string }) => mode.id)).not.toContain(
|
||||
"bypassPermissions",
|
||||
);
|
||||
});
|
||||
|
||||
it("defaults to 'default' when no permissions.defaultMode is set", async () => {
|
||||
const projectDir = path.join(tempDir, "project");
|
||||
await fs.promises.mkdir(projectDir, { recursive: true });
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import type { ClientCapabilities } from "@agentclientprotocol/sdk";
|
||||
import { getSessionMessages, type Options } from "@anthropic-ai/claude-agent-sdk";
|
||||
import type { AcpClient, ClaudeAcpAgent as ClaudeAcpAgentType } from "../acp-agent.js";
|
||||
import { ALLOW_BYPASS } from "../permissions/modes.js";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
@@ -96,6 +97,17 @@ describe("createSession options merging", () => {
|
||||
agent = new ClaudeAcpAgent(createMockClient());
|
||||
});
|
||||
|
||||
describe("allowDangerouslySkipPermissions", () => {
|
||||
it("requests bypass capability by default and mirrors it in the mode catalog", async () => {
|
||||
const response = await agent.newSession({ cwd: process.cwd(), mcpServers: [] });
|
||||
|
||||
expect(capturedOptions!.allowDangerouslySkipPermissions).toBe(ALLOW_BYPASS);
|
||||
expect(response.modes!.availableModes.some((mode) => mode.id === "bypassPermissions")).toBe(
|
||||
ALLOW_BYPASS,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("merges user-provided disallowedTools with ACP internal list", async () => {
|
||||
await agent.newSession({
|
||||
cwd: process.cwd(),
|
||||
|
||||
@@ -20,14 +20,20 @@ describe("resolvePermissionMode", () => {
|
||||
expect(resolvePermissionMode("acceptEdits")).toBe("acceptEdits");
|
||||
expect(resolvePermissionMode("dontAsk")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("plan")).toBe("plan");
|
||||
expect(resolvePermissionMode("bypassPermissions")).toBe("bypassPermissions");
|
||||
expect(resolvePermissionMode("bypassPermissions", console, true)).toBe("bypassPermissions");
|
||||
});
|
||||
|
||||
it("resolves case-insensitive aliases", () => {
|
||||
expect(resolvePermissionMode("DontAsk")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("DONTASK")).toBe("dontAsk");
|
||||
expect(resolvePermissionMode("AcceptEdits")).toBe("acceptEdits");
|
||||
expect(resolvePermissionMode("bypass")).toBe("bypassPermissions");
|
||||
expect(resolvePermissionMode("bypass", console, true)).toBe("bypassPermissions");
|
||||
});
|
||||
|
||||
it("clamps bypassPermissions to 'default' and logs when bypass is not allowed", () => {
|
||||
const { logger, error } = mockLogger();
|
||||
expect(resolvePermissionMode("bypassPermissions", logger, false)).toBe("default");
|
||||
expect(error).toHaveBeenCalledWith(expect.stringContaining("bypassPermissions"));
|
||||
});
|
||||
|
||||
it("resolves 'manual' as an alias for 'default'", () => {
|
||||
|
||||
@@ -122,6 +122,19 @@ describe("session mode", () => {
|
||||
).rejects.toThrow("start a new session");
|
||||
});
|
||||
|
||||
it("omits bypassPermissions from the catalog when bypass capability is disabled", async () => {
|
||||
const setPermissionMode = vi.fn();
|
||||
const { manager } = createManager(undefined);
|
||||
const result = await manager.initialize({
|
||||
query: { setPermissionMode },
|
||||
requestedMode: "default",
|
||||
currentModelId: "opus",
|
||||
allowBypass: false,
|
||||
});
|
||||
|
||||
expect(result.modes.availableModes.map((mode) => mode.id)).not.toContain("bypassPermissions");
|
||||
});
|
||||
|
||||
it("initializes the stable mode catalog and falls unsupported Auto back", async () => {
|
||||
const setPermissionMode = vi.fn();
|
||||
const { manager, notifications } = createManager(undefined);
|
||||
@@ -130,6 +143,7 @@ describe("session mode", () => {
|
||||
requestedMode: "auto",
|
||||
currentModelInfo: { value: "haiku", displayName: "Haiku", description: "" },
|
||||
currentModelId: "haiku",
|
||||
allowBypass: true,
|
||||
});
|
||||
|
||||
expect(result.modes.currentModeId).toBe("acceptEdits");
|
||||
|
||||
Reference in New Issue
Block a user