diff --git a/packages/types/src/__tests__/global-settings.test.ts b/packages/types/src/__tests__/global-settings.test.ts index 8107053333..c2040383e8 100644 --- a/packages/types/src/__tests__/global-settings.test.ts +++ b/packages/types/src/__tests__/global-settings.test.ts @@ -1,4 +1,5 @@ import { + DEFAULT_ALWAYS_DENY_UNAPPROVED_COMMANDS, DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED, GLOBAL_SETTINGS_KEYS, globalSettingsSchema, @@ -20,3 +21,20 @@ describe("destructive command guard global setting", () => { expect(() => globalSettingsSchema.parse({ destructiveCommandGuardEnabled: "true" })).toThrow() }) }) + +describe("alwaysDenyUnapprovedCommands global setting", () => { + it("is opt-in by default", () => { + expect(DEFAULT_ALWAYS_DENY_UNAPPROVED_COMMANDS).toBe(false) + }) + + it("accepts and exposes the persisted setting", () => { + expect(globalSettingsSchema.parse({ alwaysDenyUnapprovedCommands: true })).toEqual({ + alwaysDenyUnapprovedCommands: true, + }) + expect(GLOBAL_SETTINGS_KEYS).toContain("alwaysDenyUnapprovedCommands") + }) + + it("rejects non-boolean setting values", () => { + expect(() => globalSettingsSchema.parse({ alwaysDenyUnapprovedCommands: "true" })).toThrow() + }) +}) diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 692798d00d..54ae7af089 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -49,6 +49,15 @@ export const DEFAULT_DIFF_FUZZY_THRESHOLD = 1.0 export const DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED = false +/** + * Whether commands that are not explicitly auto-approved are automatically + * denied (with a structured reason sent to the model) instead of prompting the + * user. Opt-in: by default, unapproved commands still ask for confirmation. + * Only engages when command auto-approval (`autoApprovalEnabled` + + * `alwaysAllowExecute`) is on. + */ +export const DEFAULT_ALWAYS_DENY_UNAPPROVED_COMMANDS = false + /** * Terminal output preview size options for persisted command output. * @@ -154,6 +163,14 @@ export const globalSettingsSchema = z.object({ alwaysAllowSubtasks: z.boolean().optional(), alwaysAllowExecute: z.boolean().optional(), destructiveCommandGuardEnabled: z.boolean().optional(), + /** + * Blanket auto-deny for unapproved commands. When true (and command + * auto-approval is engaged), every command that is not explicitly + * auto-approved is automatically denied with a structured reason delivered + * to the model, instead of prompting the user. + * @default false + */ + alwaysDenyUnapprovedCommands: z.boolean().optional(), alwaysAllowFollowupQuestions: z.boolean().optional(), followupAutoApproveTimeoutMs: z.number().optional(), allowedCommands: z.array(z.string()).optional(), diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 5f6b579779..2f1222f8ee 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -283,6 +283,7 @@ export type ExtensionState = Pick< | "alwaysAllowFollowupQuestions" | "alwaysAllowExecute" | "destructiveCommandGuardEnabled" + | "alwaysDenyUnapprovedCommands" | "followupAutoApproveTimeoutMs" | "allowedCommands" | "deniedCommands" diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts new file mode 100644 index 0000000000..1a3cbcefd8 --- /dev/null +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts @@ -0,0 +1,719 @@ +// npx vitest run core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts + +import type { Anthropic } from "@anthropic-ai/sdk" +import { describe, it, expect, beforeEach, vi } from "vitest" +import { presentAssistantMessage } from "../presentAssistantMessage" +import { validateToolUse } from "../../tools/validateToolUse" +import type { Task } from "../../task/Task" +import type { AskApproval } from "../../../shared/tools" + +vi.mock("../../task/Task") +vi.mock("../../tools/validateToolUse", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + validateToolUse: vi.fn(), + } +}) + +vi.mock("@roo-code/core", () => ({ + customToolRegistry: { + has: vi.fn(() => false), + get: vi.fn(), + }, +})) + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureToolUsage: vi.fn(), + captureConsecutiveMistakeError: vi.fn(), + captureException: vi.fn(), + captureEvent: vi.fn(), + }, + }, +})) + +// Mock the tool handlers so each test controls exactly what askApproval/pushToolResult +// callbacks do inside a tool execution, isolating the approval-flow behavior of +// presentAssistantMessage itself. +const { executeCommandHandle, listFilesHandle, useMcpToolHandle } = vi.hoisted(() => ({ + executeCommandHandle: vi.fn(), + listFilesHandle: vi.fn(), + useMcpToolHandle: vi.fn(), +})) + +vi.mock("../../tools/ExecuteCommandTool", () => ({ + executeCommandTool: { handle: executeCommandHandle }, +})) + +vi.mock("../../tools/ListFilesTool", () => ({ + listFilesTool: { handle: listFilesHandle }, +})) + +vi.mock("../../tools/UseMcpToolTool", () => ({ + useMcpToolTool: { handle: useMcpToolHandle }, +})) + +interface MockTask { + taskId: string + instanceId: string + abort: boolean + presentAssistantMessageLocked: boolean + presentAssistantMessageHasPendingUpdates: boolean + currentStreamingContentIndex: number + assistantMessageContent: unknown[] + userMessageContent: Anthropic.ToolResultBlockParam[] + didCompleteReadingStream: boolean + didRejectTool: boolean + didAlreadyUseTool: boolean + consecutiveMistakeCount: number + clineMessages: unknown[] + api: { getModel: () => { id: string; info: Record } } + apiConfiguration: { apiProvider: string } + recordToolUsage: ReturnType + recordToolError: ReturnType + toolRepetitionDetector: { check: ReturnType } + providerRef: { + deref: () => { + getState: ReturnType + getMcpHub?: () => { findServerNameBySanitizedName: (name: string) => string | undefined } + } + } + say: ReturnType + ask: ReturnType + pushToolResultToUserContent: ReturnType + getTaskMode: ReturnType +} + +function buildMockTask(): MockTask { + const mockTask: MockTask = { + taskId: "test-task-id", + instanceId: "test-instance", + abort: false, + presentAssistantMessageLocked: false, + presentAssistantMessageHasPendingUpdates: false, + currentStreamingContentIndex: 0, + assistantMessageContent: [], + userMessageContent: [], + didCompleteReadingStream: true, + didRejectTool: false, + didAlreadyUseTool: false, + consecutiveMistakeCount: 0, + clineMessages: [], + api: { + getModel: () => ({ id: "test-model", info: {} }), + }, + apiConfiguration: { apiProvider: "test" }, + recordToolUsage: vi.fn(), + recordToolError: vi.fn(), + toolRepetitionDetector: { + check: vi.fn().mockReturnValue({ allowExecution: true }), + }, + providerRef: { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + }), + }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + pushToolResultToUserContent: vi.fn(), + getTaskMode: vi.fn().mockResolvedValue("code"), + } + + // Mirror the real Task: collect tool results into userMessageContent, one per + // tool_use_id, so assertions can inspect the exact payloads the model receives. + mockTask.pushToolResultToUserContent = vi.fn().mockImplementation((toolResult: Anthropic.ToolResultBlockParam) => { + const existingResult = mockTask.userMessageContent.find( + (block) => block.type === "tool_result" && block.tool_use_id === toolResult.tool_use_id, + ) + if (existingResult) { + return false + } + mockTask.userMessageContent.push(toolResult) + return true + }) + + return mockTask +} + +const executeCommandBlock = { + type: "tool_use", + id: "call_exec", + name: "execute_command", + params: { command: "rm x && npm test" }, + nativeArgs: { command: "rm x && npm test" }, + partial: false, +} + +const listFilesBlock = { + type: "tool_use", + id: "call_ls", + name: "list_files", + params: { path: "src" }, + nativeArgs: { path: "src" }, + partial: false, +} + +// Structural double — the mock Task implements only the fields this path reads; no typed alternative exists. +const asTask = (task: MockTask): Task => task as unknown as Task + +// checkAutoApproval emits dcgRuleId only on `dcg` denials, so a non-DCG detail +// carries no rule id for the askApproval copies to forward. +const NOT_ALLOWLISTED_DETAIL = { + kind: "not_allowlisted" as const, + command: "rm x && npm test", +} + +// Mirrors checkAutoApproval's `dcg` branch — the only denial shape that carries +// a rule id. +const DCG_DENY_DETAIL = { + kind: "dcg" as const, + command: "rm -rf /", + dcgReason: "matches a destructive pattern", + dcgRuleId: "R-1", +} + +// Mirrors checkAutoApproval's guard-state branch: a verdictless ask under an +// enabled Destructive Command Guard denies with kind "guard_unavailable", a +// guard-state inconsistency that must reach the model as a retryable error. +const GUARD_UNAVAILABLE_DETAIL = { + kind: "guard_unavailable" as const, + command: "npm test", +} + +// Mirrors checkAutoApproval's dangerous-substitution branch: under blanket deny +// a command with shell expansions is never auto-approved; the detail carries a +// kind-fixed reason and no rule id or parse error. +const DANGEROUS_SUBSTITUTION_DETAIL = { + kind: "dangerous_substitution" as const, + command: 'echo "${var@P}"', +} + +// Mirrors checkAutoApproval's malformed_command branch: an unterminated quote +// denies with the parser's syntax error forwarded verbatim. +const MALFORMED_COMMAND_DETAIL = { + kind: "malformed_command" as const, + command: "sh -c 'echo a", + parseError: "unexpected EOF while looking for matching quote", +} + +// Captures the boolean a tool receives back from askApproval; vi.clearAllMocks() +// does not reset closures, so beforeEach must clear this explicitly. +let execApproval: boolean | undefined + +const DCG_ALLOW = { decision: "allow" } as const +const AUTO_APPROVAL_CONTEXT = { dcgDecision: DCG_ALLOW } + +describe("presentAssistantMessage - automatic (policy) denials", () => { + let mockTask: MockTask + + beforeEach(() => { + vi.clearAllMocks() + execApproval = undefined + vi.mocked(validateToolUse).mockImplementation(() => undefined) + mockTask = buildMockTask() + + // The mocked execute_command handler only runs the approval step: an + // automatic denial means the tool returns without executing anything. + executeCommandHandle.mockImplementation( + async ( + _task: unknown, + _block: unknown, + { askApproval }: { askApproval: (t: string, m?: string) => Promise }, + ) => { + execApproval = await askApproval("command", "rm x && npm test") + }, + ) + + // The mocked list_files handler records whether it ever ran, and pushes a + // recognizable result when its own approval succeeds. + listFilesHandle.mockImplementation( + async ( + _task: unknown, + _block: unknown, + { + askApproval, + pushToolResult, + }: { askApproval: (t: string, m?: string) => Promise; pushToolResult: (c: string) => void }, + ) => { + const approved = await askApproval("tool", JSON.stringify({ tool: "listFilesTopLevel", path: "src" })) + if (approved) { + pushToolResult("second tool executed") + } + }, + ) + }) + + it("pushes a structured auto_deny tool_result, keeps didRejectTool false, and lets the next tool execute", async () => { + mockTask.assistantMessageContent = [executeCommandBlock, listFilesBlock] + + // First ask: automatic denial with structured detail. Second ask: approval. + mockTask.ask + .mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: NOT_ALLOWLISTED_DETAIL }) + .mockResolvedValueOnce({ response: "yesButtonClicked" }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(2) + + // The denied command gets the structured auto_deny payload naming the reason. + const denialResult = mockTask.userMessageContent[0] + expect(denialResult.type).toBe("tool_result") + expect(denialResult.tool_use_id).toBe("call_exec") + const denialPayload = JSON.parse(denialResult.content as string) + expect(denialPayload.status).toBe("denied") + expect(denialPayload.type).toBe("auto_deny") + expect(denialPayload.reason).toContain("not on the command allowlist") + expect(denialPayload.offending_command).toBe("rm x && npm test") + // A detail without a dcgRuleId must not gain a rule_id key in the payload. + expect(denialPayload).not.toHaveProperty("rule_id") + + // The tool must treat an automatic denial as a refusal: proceeding on a + // `true` return would execute a policy-denied command. + expect(execApproval).toBe(false) + + // An automatic denial is scoped to its own tool call: it must NOT abort the + // turn the way a user rejection does. + expect(mockTask.didRejectTool).toBe(false) + expect(mockTask.didAlreadyUseTool).toBe(false) + + // The reason is system-generated: it must not surface as user feedback. + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything()) + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything()) + + // The second tool in the same turn still executes normally. + expect(listFilesHandle).toHaveBeenCalledTimes(1) + const secondResult = mockTask.userMessageContent[1] + expect(secondResult.tool_use_id).toBe("call_ls") + expect(secondResult.content).toBe("second tool executed") + expect(secondResult.is_error).toBeUndefined() + }) + + it("routes guard_unavailable to a retryable toolError while a policy kind keeps toolAutoDenied (command askApproval copy)", async () => { + mockTask.assistantMessageContent = [executeCommandBlock, listFilesBlock] + + // First ask: guard-state denial (retryable). Second ask: policy denial + // (sibling kind through the same harness) — must stay toolAutoDenied. + mockTask.ask + .mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: GUARD_UNAVAILABLE_DETAIL }) + .mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: NOT_ALLOWLISTED_DETAIL }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(2) + + // guard_unavailable is a guard-state inconsistency, not a policy denial: + // the model must receive the retryable toolError payload, not the + // auto_deny denial that advises switching to approved commands. + const guardPayload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(guardPayload.status).toBe("error") + expect(guardPayload.message).toBe("The tool execution failed") + expect(guardPayload.error).toContain("Command `npm test` was not executed") + expect(guardPayload.error).toContain("internal guard-state inconsistency") + expect(guardPayload.error).toContain("You may retry the same command") + expect(guardPayload).not.toHaveProperty("type") + expect(guardPayload).not.toHaveProperty("note") + expect(guardPayload).not.toHaveProperty("suggestion") + + // The tool treats it as a refusal (return false) without aborting the turn. + expect(execApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + + // Sibling policy kind is unchanged: structured auto_deny with the + // denial advice. + const policyPayload = JSON.parse(mockTask.userMessageContent[1].content as string) + expect(policyPayload.status).toBe("denied") + expect(policyPayload.type).toBe("auto_deny") + expect(policyPayload.reason).toContain("not on the command allowlist") + }) + + it("routes guard_unavailable to a retryable toolError while a policy kind keeps toolAutoDenied (MCP askApproval copy)", async () => { + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_mcp_a", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + { + type: "mcp_tool_use", + id: "call_mcp_b", + name: "mcp_my_server_other_thing", + serverName: "my_server", + toolName: "other_thing", + arguments: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }), + getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }), + }), + } + + const approvals: boolean[] = [] + useMcpToolHandle.mockImplementation( + async ( + _task: unknown, + _block: unknown, + { askApproval }: { askApproval: (t: string, m?: string) => Promise }, + ) => { + approvals.push(await askApproval("use_mcp_server", "{}")) + }, + ) + + // Same split on the MCP closure: guard-state first (retryable error), + // denylist second (policy auto_deny). + mockTask.ask + .mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: GUARD_UNAVAILABLE_DETAIL }) + .mockResolvedValueOnce({ + response: "noButtonClicked", + autoDenyDetail: { kind: "denylist", command: "rm x", pattern: "rm" }, + }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(2) + + const guardPayload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(guardPayload.status).toBe("error") + expect(guardPayload.error).toContain("internal guard-state inconsistency") + expect(guardPayload.error).toContain("You may retry the same command") + expect(guardPayload).not.toHaveProperty("type") + expect(guardPayload).not.toHaveProperty("suggestion") + + const policyPayload = JSON.parse(mockTask.userMessageContent[1].content as string) + expect(policyPayload.status).toBe("denied") + expect(policyPayload.type).toBe("auto_deny") + expect(policyPayload.reason).toContain("matches denied prefix `rm`") + + expect(approvals).toEqual([false, false]) + expect(mockTask.didRejectTool).toBe(false) + }) + + it("routes an auto-deny through the MCP askApproval copy without aborting the turn", async () => { + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_mcp", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }), + getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }), + }), + } + + let mcpApproval: boolean | undefined + useMcpToolHandle.mockImplementation( + async ( + _task: unknown, + _block: unknown, + { askApproval }: { askApproval: (t: string, m?: string) => Promise }, + ) => { + mcpApproval = await askApproval("use_mcp_server", "{}") + }, + ) + + // A denylist denial is policy-emitted without a rule id: kind, command, pattern only. + mockTask.ask.mockResolvedValueOnce({ + response: "noButtonClicked", + autoDenyDetail: { kind: "denylist", command: "rm x", pattern: "rm" }, + }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(1) + const payload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(payload.type).toBe("auto_deny") + expect(payload.reason).toContain("matches denied prefix `rm`") + expect(payload.offending_command).toBe("rm x") + expect(payload).not.toHaveProperty("rule_id") + // The MCP tool must also see the denial as a refusal to execute. + expect(mcpApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything()) + }) + + it("keeps the legacy user-rejection behavior when the rejection carries no autoDenyDetail", async () => { + mockTask.assistantMessageContent = [executeCommandBlock, listFilesBlock] + + // Real user click: noButtonClicked with no structured detail. + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked" }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(2) + + // Rejection wording stays the user-denial payload. + const denialResult = mockTask.userMessageContent[0] + expect(denialResult.tool_use_id).toBe("call_exec") + const denialPayload = JSON.parse(denialResult.content as string) + expect(denialPayload.message).toBe("The user denied this operation.") + expect(denialPayload.type).toBeUndefined() + + // A real user rejection still aborts the rest of the turn. + expect(mockTask.didRejectTool).toBe(true) + + // The remaining tool is skipped with the "user rejecting a previous tool" + // message and never executes. + expect(listFilesHandle).not.toHaveBeenCalled() + const skippedResult = mockTask.userMessageContent[1] + expect(skippedResult.tool_use_id).toBe("call_ls") + expect(skippedResult.is_error).toBe(true) + expect(skippedResult.content).toContain("due to user rejecting a previous tool") + expect(skippedResult.content).not.toContain("auto_deny") + }) + + it("persists user feedback as a user_feedback say row on a text-carrying rejection (no detail)", async () => { + mockTask.assistantMessageContent = [executeCommandBlock] + + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked", text: "do not run that" }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.say).toHaveBeenCalledWith("user_feedback", "do not run that", undefined) + expect(mockTask.didRejectTool).toBe(true) + + const payload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(payload.status).toBe("denied") + expect(payload.feedback).toBe("do not run that") + expect(payload.type).toBeUndefined() + }) + + it("forwards autoApprovalContext from the tool askApproval copy into cline.ask", async () => { + mockTask.assistantMessageContent = [executeCommandBlock] + + // The seam: whatever a tool hands to askApproval must reach Task.ask + // positionally — dropping the argument re-opens the DCG-context bypass. + executeCommandHandle.mockImplementation( + async (_task: unknown, _block: unknown, { askApproval }: { askApproval: AskApproval }) => { + await askApproval("command", "rm x", undefined, false, AUTO_APPROVAL_CONTEXT) + }, + ) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.ask).toHaveBeenCalledWith("command", "rm x", false, undefined, false, AUTO_APPROVAL_CONTEXT) + }) + + it("forwards autoApprovalContext from the MCP askApproval copy into cline.ask", async () => { + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_mcp", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }), + getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }), + }), + } + + // The same seam on the mcp_tool_use copy of the closure: the context + // must survive the positional forwarding to cline.ask here too. + useMcpToolHandle.mockImplementation( + async (_task: unknown, _block: unknown, { askApproval }: { askApproval: AskApproval }) => { + await askApproval("use_mcp_server", "{}", undefined, false, AUTO_APPROVAL_CONTEXT) + }, + ) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.ask).toHaveBeenCalledWith( + "use_mcp_server", + "{}", + false, + undefined, + false, + AUTO_APPROVAL_CONTEXT, + ) + }) + + it("keeps the user-rejection payload on the MCP copy when the denial carries no autoDenyDetail", async () => { + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_mcp", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }), + getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }), + }), + } + + useMcpToolHandle.mockImplementation( + async (_task: unknown, _block: unknown, { askApproval }: { askApproval: AskApproval }) => { + await askApproval("use_mcp_server", "{}") + }, + ) + + // A rejection without structured detail is a real user click: legacy + // wording, and the rest of the turn aborts. + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked" }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(1) + const payload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(payload.message).toBe("The user denied this operation.") + expect(payload.type).toBeUndefined() + expect(mockTask.didRejectTool).toBe(true) + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything()) + }) + + it("forwards the DCG rule id into the payload rule_id (command askApproval copy)", async () => { + mockTask.assistantMessageContent = [executeCommandBlock] + + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: DCG_DENY_DETAIL }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(1) + const payload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(payload.type).toBe("auto_deny") + // The DCG reason and rule id both reach the model: the reason string is + // built from the detail's own fields at this layer. + expect(payload.reason).toContain("matches a destructive pattern") + expect(payload.reason).toContain("(Rule: R-1)") + expect(payload.offending_command).toBe("rm -rf /") + expect(payload.rule_id).toBe("R-1") + expect(execApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + }) + + it("forwards the DCG rule id into the payload rule_id (MCP askApproval copy)", async () => { + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_mcp", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ mode: "code", customModes: [] }), + getMcpHub: () => ({ findServerNameBySanitizedName: () => undefined }), + }), + } + + let mcpApproval: boolean | undefined + useMcpToolHandle.mockImplementation( + async ( + _task: unknown, + _block: unknown, + { askApproval }: { askApproval: (t: string, m?: string) => Promise }, + ) => { + mcpApproval = await askApproval("use_mcp_server", "{}") + }, + ) + + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: DCG_DENY_DETAIL }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(1) + const payload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(payload.type).toBe("auto_deny") + expect(payload.rule_id).toBe("R-1") + expect(mcpApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + }) + + it("denies dangerous_substitution with the fixed shell-expansion reason without aborting the turn", async () => { + mockTask.assistantMessageContent = [executeCommandBlock, listFilesBlock] + + // First ask: blanket denial of a shell-expansion command. Second ask: + // approval — the denial must stay scoped to its own tool call. + mockTask.ask + .mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: DANGEROUS_SUBSTITUTION_DETAIL }) + .mockResolvedValueOnce({ response: "yesButtonClicked" }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(2) + + const denialPayload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(denialPayload.status).toBe("denied") + expect(denialPayload.type).toBe("auto_deny") + // The reason is kind-fixed and carries no rule id: expansion forms are + // never auto-approved under blanket deny. + expect(denialPayload.reason).toContain("shell expansions") + expect(denialPayload.reason).toContain("never auto-approved") + expect(denialPayload.offending_command).toBe('echo "${var@P}"') + expect(denialPayload).not.toHaveProperty("rule_id") + + // The boundary assertions of the dcg/not_allowlisted pins: the tool sees a + // refusal — it must not execute the command — and the denial does not + // abort the turn the way a user rejection does. + expect(execApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + + // The reason is system-generated: it must not surface as user feedback. + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything()) + + // The denial is scoped to its own tool call: the next tool still executes. + expect(listFilesHandle).toHaveBeenCalledTimes(1) + expect(mockTask.userMessageContent[1].content).toBe("second tool executed") + }) + + it("denies malformed_command with the forwarded parse error without aborting the turn", async () => { + mockTask.assistantMessageContent = [executeCommandBlock] + + mockTask.ask.mockResolvedValueOnce({ response: "noButtonClicked", autoDenyDetail: MALFORMED_COMMAND_DETAIL }) + + await presentAssistantMessage(asTask(mockTask)) + + expect(mockTask.userMessageContent).toHaveLength(1) + const denialPayload = JSON.parse(mockTask.userMessageContent[0].content as string) + expect(denialPayload.status).toBe("denied") + expect(denialPayload.type).toBe("auto_deny") + // A malformed_command denial carries its parse error through to the payload + // verbatim — the boundary must not flatten it to the generic reason. + expect(denialPayload.reason).toBe("unexpected EOF while looking for matching quote") + expect(denialPayload.offending_command).toBe("sh -c 'echo a") + expect(denialPayload).not.toHaveProperty("rule_id") + + // Same boundary assertions as the dcg/not_allowlisted pins: refusal + // without execution, no turn abort, no user_feedback row. + expect(execApproval).toBe(false) + expect(mockTask.didRejectTool).toBe(false) + expect(mockTask.say).not.toHaveBeenCalledWith("user_feedback", expect.anything(), expect.anything()) + }) +}) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 546c43c06c..1321e6a800 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -9,7 +9,9 @@ import { customToolRegistry } from "@roo-code/core" import { t } from "../../i18n" import { defaultModeSlug, getModeBySlug } from "../../shared/modes" -import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../shared/tools" +import type { AutoApprovalContext, ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../shared/tools" + +import { buildAutoDenyReason } from "../auto-approval" import { AskIgnoredError } from "../task/AskIgnoredError" import { Task } from "../task/Task" @@ -214,16 +216,40 @@ export async function presentAssistantMessage(cline: Task) { partialMessage?: string, progressStatus?: ToolProgressStatus, isProtected?: boolean, + autoApprovalContext?: AutoApprovalContext, ) => { - const { response, text, images } = await cline.ask( + const { response, text, images, autoDenyDetail } = await cline.ask( type, partialMessage, false, progressStatus, isProtected || false, + autoApprovalContext, ) if (response !== "yesButtonClicked") { + // Automatic (policy) denial: scoped to this tool call, so no + // `didRejectTool` (remaining tool calls in the turn proceed) + // and no `user_feedback` say (the reason is system-generated). + if (autoDenyDetail) { + // `guard_unavailable` marks a guard-state inconsistency, not a + // policy denial: the command never ran and a re-issue re-reads + // the guard setting, so the payload must stay a retryable error + // instead of carrying policy-denial advice. + if (autoDenyDetail.kind === "guard_unavailable") { + pushToolResult(formatResponse.toolError(buildAutoDenyReason(autoDenyDetail))) + } else { + pushToolResult( + formatResponse.toolAutoDenied({ + reason: buildAutoDenyReason(autoDenyDetail), + offendingCommand: autoDenyDetail.command, + ruleId: autoDenyDetail.dcgRuleId, + }), + ) + } + return false + } + if (text) { await cline.say("user_feedback", text, images) pushToolResult(formatResponse.toolResult(formatResponse.toolDeniedWithFeedback(text), images)) @@ -522,16 +548,42 @@ export async function presentAssistantMessage(cline: Task) { partialMessage?: string, progressStatus?: ToolProgressStatus, isProtected?: boolean, + autoApprovalContext?: AutoApprovalContext, ) => { - const { response, text, images, queuedMessageId } = await cline.ask( + const { response, text, images, queuedMessageId, autoDenyDetail } = await cline.ask( type, partialMessage, false, progressStatus, isProtected || false, + autoApprovalContext, ) if (response !== "yesButtonClicked") { + // Automatic (policy) denial: scoped to this tool call, so no + // `didRejectTool` (remaining tool calls in the turn proceed) + // and no `user_feedback` say (the reason is system-generated). + // Automatic denials never carry queued feedback — a queued + // message forces a real ask. + if (autoDenyDetail) { + // `guard_unavailable` marks a guard-state inconsistency, not a + // policy denial: the command never ran and a re-issue re-reads + // the guard setting, so the payload must stay a retryable error + // instead of carrying policy-denial advice. + if (autoDenyDetail.kind === "guard_unavailable") { + pushToolResult(formatResponse.toolError(buildAutoDenyReason(autoDenyDetail))) + } else { + pushToolResult( + formatResponse.toolAutoDenied({ + reason: buildAutoDenyReason(autoDenyDetail), + offendingCommand: autoDenyDetail.command, + ruleId: autoDenyDetail.dcgRuleId, + }), + ) + } + return false + } + // Handle both messageResponse and noButtonClicked with text. if (queuedMessageId) { const persisted = await cline.persistQueuedFeedbackAndAcknowledge(queuedMessageId, text, images) diff --git a/src/core/auto-approval/__tests__/autoApprovalEdgeCases.spec.ts b/src/core/auto-approval/__tests__/autoApprovalEdgeCases.spec.ts new file mode 100644 index 0000000000..b0882e3da1 --- /dev/null +++ b/src/core/auto-approval/__tests__/autoApprovalEdgeCases.spec.ts @@ -0,0 +1,50 @@ +import type { ExtensionState } from "@roo-code/types" +import { checkAutoApproval, type AutoApprovalState, type AutoApprovalStateOptions } from ".." +import { getCommandDecisionDetailed } from "../commands" + +type State = Pick + +// Edge cases around the command auto-approval path that the blanket-deny +// matrix does not cover: command lists absent from state entirely, a +// DCG-enabled ask arriving without a verdict, and the original-cased denied +// pattern reported back for the model-facing denial detail. +describe("command auto-approval edge cases", () => { + const stateWithoutCommandLists: State = { + autoApprovalEnabled: true, + alwaysAllowExecute: true, + } + + it("asks for an unlisted command when the state omits both command lists", async () => { + expect(await checkAutoApproval({ state: stateWithoutCommandLists, ask: "command", text: "some-tool" })).toEqual( + { decision: "ask" }, + ) + }) + + it("auto-denies an unlisted command with blanket on when the state omits both command lists", async () => { + expect( + await checkAutoApproval({ + state: { ...stateWithoutCommandLists, alwaysDenyUnapprovedCommands: true }, + ask: "command", + text: "some-tool", + }), + ).toEqual({ decision: "deny", autoDeny: { kind: "not_allowlisted", command: "some-tool" } }) + }) + + it("denies a DCG-enabled command ask that carries no verdict with a retryable guard-state detail", async () => { + expect( + await checkAutoApproval({ + state: { ...stateWithoutCommandLists, destructiveCommandGuardEnabled: true }, + ask: "command", + text: "rm file", + }), + ).toEqual({ decision: "deny", autoDeny: { kind: "guard_unavailable", command: "rm file" } }) + }) + + it("reports the denied prefix in its original casing", () => { + expect(getCommandDecisionDetailed("rm x", ["git"], ["RM"])).toEqual({ + decision: "auto_deny", + offendingCommand: "rm x", + matchedPattern: "RM", + }) + }) +}) diff --git a/src/core/auto-approval/__tests__/autoDenyReason.spec.ts b/src/core/auto-approval/__tests__/autoDenyReason.spec.ts new file mode 100644 index 0000000000..30b0d0a8ec --- /dev/null +++ b/src/core/auto-approval/__tests__/autoDenyReason.spec.ts @@ -0,0 +1,66 @@ +import { buildAutoDenyReason } from "../autoDenyReason" + +// Strings are asserted against buildAutoDenyReason's own templates: these are +// model-facing payloads, so exact wording (backticks, placeholders, rule +// suffix) is the contract. +describe("buildAutoDenyReason", () => { + it("names the command and matched prefix for a complete denylist detail", () => { + expect(buildAutoDenyReason({ kind: "denylist", command: "rm -rf /", pattern: "rm" })).toBe( + "Command `rm -rf /` matches denied prefix `rm`.", + ) + }) + + it("falls back to (unknown) placeholders when a denylist detail omits command and pattern", () => { + expect(buildAutoDenyReason({ kind: "denylist" })).toBe("Command `(unknown)` matches denied prefix `(unknown)`.") + }) + + it("names the command for a complete not_allowlisted detail", () => { + expect(buildAutoDenyReason({ kind: "not_allowlisted", command: "unknown-tool" })).toBe( + "Command `unknown-tool` is not on the command allowlist.", + ) + }) + + it("falls back to an (unknown) command when a not_allowlisted detail omits it", () => { + expect(buildAutoDenyReason({ kind: "not_allowlisted" })).toBe( + "Command `(unknown)` is not on the command allowlist.", + ) + }) + + it("returns the fixed shell-expansion warning for dangerous_substitution", () => { + expect(buildAutoDenyReason({ kind: "dangerous_substitution", command: 'echo "${var@P}"' })).toBe( + "Command contains shell expansions (${...} forms, process substitution, and similar) that are never auto-approved. Choose an approved command without shell expansions.", + ) + }) + + it("returns the honest retryable detail for guard_unavailable", () => { + const reason = buildAutoDenyReason({ kind: "guard_unavailable", command: "npm test" }) + expect(reason).toBe( + "Command `npm test` was not executed: the Destructive Command Guard is enabled but supplied no verdict, which is an internal guard-state inconsistency, not a policy denial. You may retry the same command.", + ) + // The never-ask posture: the reason must name the state and invite a + // retry without ever pointing at user approval. + expect(reason).not.toMatch(/approv|ask the user/i) + expect(buildAutoDenyReason({ kind: "guard_unavailable" })).toBe( + "Command `(unknown)` was not executed: the Destructive Command Guard is enabled but supplied no verdict, which is an internal guard-state inconsistency, not a policy denial. You may retry the same command.", + ) + }) + + it("forwards the parse error for malformed_command and defaults when it is absent", () => { + expect(buildAutoDenyReason({ kind: "malformed_command", parseError: "boom" })).toBe("boom") + expect(buildAutoDenyReason({ kind: "malformed_command" })).toBe("Command contains a shell syntax error.") + }) + + it("appends the rule id for a dcg detail and omits the suffix without one", () => { + expect( + buildAutoDenyReason({ + kind: "dcg", + command: "rm -rf /", + dcgReason: "matches a destructive pattern", + dcgRuleId: "recursive-delete", + }), + ).toBe("Destructive Command Guard denied the command: matches a destructive pattern (Rule: recursive-delete)") + expect(buildAutoDenyReason({ kind: "dcg" })).toBe( + "Destructive Command Guard denied the command: no reason provided", + ) + }) +}) diff --git a/src/core/auto-approval/__tests__/blanket-deny.spec.ts b/src/core/auto-approval/__tests__/blanket-deny.spec.ts new file mode 100644 index 0000000000..f60380b8cf --- /dev/null +++ b/src/core/auto-approval/__tests__/blanket-deny.spec.ts @@ -0,0 +1,263 @@ +import { checkAutoApproval } from ".." +import { baseState as sharedBaseState, type State } from "./fixtures" + +// Matrix over the blanket auto-deny feature (`alwaysDenyUnapprovedCommands`): +// DCG on/off × blanket on/off × command shapes. The blanket setting only +// engages while command auto-approval (`autoApprovalEnabled` + +// `alwaysAllowExecute`) is on, so the engagement cases keep both gates on; the +// disengagement cases turn them off to prove the setting is inert. +describe("blanket auto-deny for unapproved commands", () => { + const baseState = { + ...sharedBaseState, + alwaysAllowExecute: true, + allowedCommands: ["git"], + deniedCommands: ["rm"], + } + + const commandCase = (text: string, overrides: Partial = {}, extra: object = {}) => + checkAutoApproval({ state: { ...baseState, ...overrides }, ask: "command", text, ...extra }) + + describe("DCG disabled", () => { + it("auto-approves allowlist matches regardless of the blanket setting", async () => { + expect(await commandCase("git status")).toEqual({ decision: "approve" }) + expect(await commandCase("git status", { alwaysDenyUnapprovedCommands: true })).toEqual({ + decision: "approve", + }) + }) + + it("auto-denies denylist matches with structured detail, even with blanket off", async () => { + expect(await commandCase("rm file")).toEqual({ + decision: "deny", + autoDeny: { kind: "denylist", command: "rm file", pattern: "rm" }, + }) + expect(await commandCase("rm file", { alwaysDenyUnapprovedCommands: true })).toEqual({ + decision: "deny", + autoDeny: { kind: "denylist", command: "rm file", pattern: "rm" }, + }) + }) + + it("asks for unlisted commands with blanket off", async () => { + expect(await commandCase("some-unknown-command")).toEqual({ decision: "ask" }) + }) + + it("auto-denies unlisted commands with blanket on", async () => { + expect(await commandCase("some-unknown-command", { alwaysDenyUnapprovedCommands: true })).toEqual({ + decision: "deny", + autoDeny: { kind: "not_allowlisted", command: "some-unknown-command" }, + }) + }) + + it("names the first sub-command lacking an allowlist match in a chain", async () => { + const result = await commandCase("git status && unknown-tool", { + alwaysDenyUnapprovedCommands: true, + }) + + expect(result).toEqual({ + decision: "deny", + autoDeny: { kind: "not_allowlisted", command: "unknown-tool" }, + }) + }) + + it("asks for dangerous substitutions with blanket off", async () => { + expect(await commandCase('echo "${var@P}"', { allowedCommands: ["echo"] })).toEqual({ decision: "ask" }) + }) + + it("auto-denies dangerous substitutions with blanket on", async () => { + expect( + await commandCase('echo "${var@P}"', { + allowedCommands: ["echo"], + alwaysDenyUnapprovedCommands: true, + }), + ).toEqual({ + decision: "deny", + autoDeny: { kind: "dangerous_substitution", command: 'echo "${var@P}"' }, + }) + }) + + it("asks for malformed commands with blanket off (normally blocked earlier by the tool)", async () => { + expect(await commandCase("sh -c 'echo a")).toEqual({ decision: "ask" }) + }) + + it("auto-denies malformed commands with blanket on (defense in depth)", async () => { + const result = await commandCase("sh -c 'echo a", { alwaysDenyUnapprovedCommands: true }) + + expect(result).toEqual({ + decision: "deny", + autoDeny: expect.objectContaining({ + kind: "malformed_command", + command: "sh -c 'echo a", + parseError: expect.stringContaining("unterminated"), + }), + }) + }) + + it("never returns ask for a command ask with all gates on and blanket on", async () => { + const commands = [ + "git status", // allowlisted + "rm file", // denylisted + "unknown-command", // unlisted + 'echo "${var@P}"', // dangerous substitution + "sh -c 'echo a", // malformed + "git status && rm x && npm test", // chain with a denied part + "git status && unknown-tool", // chain with an unlisted part + ] + + for (const command of commands) { + const result = await commandCase(command, { + alwaysDenyUnapprovedCommands: true, + allowedCommands: ["echo", "git", "npm", "sh"], + }) + expect(result.decision, `command: ${command}`).not.toBe("ask") + } + }) + + it("leaves protected asks prompting even with blanket on", async () => { + expect( + await checkAutoApproval({ + state: { ...baseState, alwaysDenyUnapprovedCommands: true }, + ask: "command", + text: "some-unknown-command", + isProtected: true, + }), + ).toEqual({ decision: "ask" }) + }) + }) + + describe("gate: blanket only engages while command auto-approval is on", () => { + it("asks when the master auto-approval switch is off", async () => { + expect( + await commandCase("unknown-command", { + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: false, + }), + ).toEqual({ decision: "ask" }) + }) + + it("asks when execute auto-approval is off", async () => { + expect( + await commandCase("unknown-command", { + alwaysDenyUnapprovedCommands: true, + alwaysAllowExecute: false, + }), + ).toEqual({ decision: "ask" }) + }) + }) + + describe("DCG enabled", () => { + const dcgState = { destructiveCommandGuardEnabled: true } + + it("approves a DCG-allowed verdict in every blanket mode", async () => { + expect(await commandCase("rm file", dcgState, { dcgDecision: { decision: "allow" } })).toEqual({ + decision: "approve", + }) + expect( + await commandCase( + "rm file", + { ...dcgState, alwaysDenyUnapprovedCommands: true }, + { + dcgDecision: { decision: "allow" }, + }, + ), + ).toEqual({ decision: "approve" }) + }) + + it("auto-denies a verdictless command ask with the retryable guard-state detail in both blanket modes", async () => { + // Verdictless + guard-on is an inconsistent guard state, not a guard + // decision, so the denial is the same in both blanket modes. + expect(await commandCase("rm file", { ...dcgState })).toEqual({ + decision: "deny", + autoDeny: { kind: "guard_unavailable", command: "rm file" }, + }) + expect(await commandCase("rm file", { ...dcgState, alwaysDenyUnapprovedCommands: true })).toEqual({ + decision: "deny", + autoDeny: { kind: "guard_unavailable", command: "rm file" }, + }) + }) + + it("auto-denies with the DCG reason when blanket is on", async () => { + expect( + await commandCase( + "rm -rf /", + { ...dcgState, alwaysDenyUnapprovedCommands: true }, + { + dcgDecision: { + decision: "deny", + reason: "matches a destructive pattern", + ruleId: "recursive-delete", + }, + }, + ), + ).toEqual({ + decision: "deny", + autoDeny: { + kind: "dcg", + command: "rm -rf /", + dcgReason: "matches a destructive pattern", + dcgRuleId: "recursive-delete", + }, + }) + }) + + it("prompts (protected ask) when blanket is off", async () => { + expect( + await commandCase( + "rm -rf /", + { ...dcgState, alwaysDenyUnapprovedCommands: false }, + { dcgDecision: { decision: "deny", reason: "matches a destructive pattern" } }, + ), + ).toEqual({ decision: "ask" }) + }) + + it("does not let an allowlist match rescue a DCG denial", async () => { + const result = await commandCase( + "rm file", + { ...dcgState, alwaysDenyUnapprovedCommands: true, allowedCommands: ["rm"] }, + { dcgDecision: { decision: "deny", reason: "matches a destructive pattern" } }, + ) + + expect(result).toEqual({ + decision: "deny", + autoDeny: { + kind: "dcg", + command: "rm file", + dcgReason: "matches a destructive pattern", + dcgRuleId: undefined, + }, + }) + }) + + it("never returns ask for a command ask with all gates on, blanket on, and a verdict", async () => { + for (const dcgDecision of [{ decision: "allow" }, { decision: "deny", reason: "nope" }] as const) { + const result = await commandCase( + "anything", + { ...dcgState, alwaysDenyUnapprovedCommands: true }, + { dcgDecision }, + ) + expect(result.decision, `verdict: ${dcgDecision.decision}`).not.toBe("ask") + } + }) + }) + + it("does not affect non-command asks", async () => { + const state = { ...baseState, alwaysDenyUnapprovedCommands: true, alwaysAllowWrite: true } + + expect( + await checkAutoApproval({ + state, + cwd: "/repo", + ask: "tool", + text: JSON.stringify({ tool: "editedExistingFile", path: "a.ts" }), + }), + ).toEqual({ decision: "approve" }) + + // A write that is not allowed still asks — the blanket setting is command-only. + expect( + await checkAutoApproval({ + state: { ...state, alwaysAllowWrite: false }, + cwd: "/repo", + ask: "tool", + text: JSON.stringify({ tool: "editedExistingFile", path: "a.ts" }), + }), + ).toEqual({ decision: "ask" }) + }) +}) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index fa5762cb56..ade53c2b20 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -1,4 +1,4 @@ -import { containsDangerousSubstitution, getCommandDecision } from "../commands" +import { containsDangerousSubstitution, getCommandDecision, getCommandDecisionDetailed } from "../commands" describe("containsDangerousSubstitution", () => { describe("zsh array assignments (should NOT be flagged)", () => { @@ -159,3 +159,59 @@ describe("getCommandDecision — multi-line script wrapped in a quoted argument" expect(getCommandDecision(malformed, [malformed])).toBe("malformed_command") }) }) + +describe("getCommandDecisionDetailed", () => { + it("names the offending sub-command and matched denied prefix on chained commands", () => { + const result = getCommandDecisionDetailed("git status && rm x && npm test", ["git", "npm"], ["rm"]) + + expect(result.decision).toBe("auto_deny") + expect(result.offendingCommand).toBe("rm x") + expect(result.matchedPattern).toBe("rm") + }) + + it("preserves the original casing of the matched denied prefix", () => { + const result = getCommandDecisionDetailed("RM -rf /tmp/x", [], ["RM -rf"]) + + expect(result.decision).toBe("auto_deny") + expect(result.matchedPattern).toBe("RM -rf") + }) + + it("names the first sub-command lacking an allowlist match for ask_user decisions", () => { + const result = getCommandDecisionDetailed("git status && unknown-tool", ["git"]) + + expect(result.decision).toBe("ask_user") + expect(result.offendingCommand).toBe("unknown-tool") + expect(result.matchedPattern).toBeUndefined() + }) + + it("reports the parse error message for malformed commands", () => { + const result = getCommandDecisionDetailed("sh -c 'echo a", ["sh"]) + + expect(result.decision).toBe("malformed_command") + expect(result.offendingCommand).toBe("sh -c 'echo a") + expect(result.parseError).toContain("unterminated") + }) + + it("returns plain approval details for auto-approved commands", () => { + expect(getCommandDecisionDetailed("git status", ["git"])).toEqual({ decision: "auto_approve" }) + expect(getCommandDecisionDetailed(" ", ["git"])).toEqual({ decision: "auto_approve" }) + }) + + it("returns the same decision as getCommandDecision across representative inputs", () => { + const cases: Array<[string, string[], string[] | undefined]> = [ + ["git status", ["git"], []], + ["git push origin", ["git"], ["git push"]], + ["git status && rm file", ["git"], ["rm"]], + ["unknown command", ["git"], ["rm"]], + ['echo "${var@P}"', ["echo"], []], + ["sh -c 'echo a", ["sh"], []], + ["", ["git"], []], + ] + + for (const [command, allowed, denied] of cases) { + expect(getCommandDecisionDetailed(command, allowed, denied).decision).toBe( + getCommandDecision(command, allowed, denied), + ) + } + }) +}) diff --git a/src/core/auto-approval/__tests__/dcg.spec.ts b/src/core/auto-approval/__tests__/dcg.spec.ts index fb4f0ac9a7..cfaffe007f 100644 --- a/src/core/auto-approval/__tests__/dcg.spec.ts +++ b/src/core/auto-approval/__tests__/dcg.spec.ts @@ -16,12 +16,14 @@ describe("Destructive Command Guard auto-approval precedence", () => { allowedCommands: ["echo"], deniedCommands: ["rm"], destructiveCommandGuardEnabled: true, + alwaysDenyUnapprovedCommands: false, mcpServers: [], } - it("auto-approves commands allowed by DCG without consulting Zoo's deny list", async () => { + it("denies with the retryable guard-state detail when no verdict is supplied", async () => { expect(await checkAutoApproval({ state: baseState, ask: "command", text: "rm file" })).toEqual({ - decision: "approve", + decision: "deny", + autoDeny: { kind: "guard_unavailable", command: "rm file" }, }) }) @@ -31,15 +33,16 @@ describe("Destructive Command Guard auto-approval precedence", () => { expect(await checkAutoApproval({ state, ask: "command", text: "rm file" })).toEqual({ decision: "ask" }) }) - it("requires explicit approval for a DCG-protected command", async () => { + it("requires explicit approval for a DCG-protected command when blanket auto-deny is off", async () => { expect( await checkAutoApproval({ state: baseState, ask: "command", text: "echo safe", isProtected: true }), ).toEqual({ decision: "ask" }) }) - it("auto-approves DCG-allowed commands without consulting Zoo's allowlist", async () => { + it("denies with the retryable guard-state detail without consulting Zoo's allowlist when no verdict is supplied", async () => { expect(await checkAutoApproval({ state: baseState, ask: "command", text: "unlisted-command" })).toEqual({ - decision: "approve", + decision: "deny", + autoDeny: { kind: "guard_unavailable", command: "unlisted-command" }, }) }) @@ -60,8 +63,11 @@ describe("Destructive Command Guard auto-approval precedence", () => { it("keeps ordinary denylist behavior when DCG is disabled", async () => { const state = { ...baseState, destructiveCommandGuardEnabled: false } + // Denylist denials are automatic denials and carry their structured + // detail even when the blanket setting is off. expect(await checkAutoApproval({ state, ask: "command", text: "rm file" })).toEqual({ decision: "deny", + autoDeny: { kind: "denylist", command: "rm file", pattern: "rm" }, }) }) @@ -72,4 +78,94 @@ describe("Destructive Command Guard auto-approval precedence", () => { decision: "ask", }) }) + + describe("with an explicit DCG verdict", () => { + it("auto-approves when the verdict allows the command, bypassing Zoo's deny list", async () => { + expect( + await checkAutoApproval({ + state: baseState, + ask: "command", + text: "rm file", + dcgDecision: { decision: "allow" }, + }), + ).toEqual({ decision: "approve" }) + }) + + it("falls back to the (protected) user prompt when DCG denies and blanket auto-deny is off", async () => { + expect( + await checkAutoApproval({ + state: baseState, + ask: "command", + text: "echo test", + isProtected: true, + dcgDecision: { + decision: "deny", + reason: "matches a destructive pattern", + ruleId: "recursive-delete", + }, + }), + ).toEqual({ decision: "ask" }) + }) + + it("auto-denies with the DCG reason and rule when blanket auto-deny is on", async () => { + const state = { ...baseState, alwaysDenyUnapprovedCommands: true } + + expect( + await checkAutoApproval({ + state, + ask: "command", + text: "rm -rf /", + dcgDecision: { + decision: "deny", + reason: "matches a destructive pattern", + ruleId: "recursive-delete", + }, + }), + ).toEqual({ + decision: "deny", + autoDeny: { + kind: "dcg", + command: "rm -rf /", + dcgReason: "matches a destructive pattern", + dcgRuleId: "recursive-delete", + }, + }) + }) + + it("does not let an allowlist match rescue a DCG denial under blanket auto-deny", async () => { + const state = { ...baseState, alwaysDenyUnapprovedCommands: true, allowedCommands: ["rm"] } + + const result = await checkAutoApproval({ + state, + ask: "command", + text: "rm file", + dcgDecision: { decision: "deny", reason: "matches a destructive pattern" }, + }) + + expect(result).toEqual({ + decision: "deny", + autoDeny: { + kind: "dcg", + command: "rm file", + dcgReason: "matches a destructive pattern", + dcgRuleId: undefined, + }, + }) + }) + + it("ignores the verdict when DCG is disabled in settings", async () => { + const state = { ...baseState, destructiveCommandGuardEnabled: false } + + // The verdict is only consulted while DCG is enabled; with it off, + // the ordinary denylist still denies. + expect( + await checkAutoApproval({ + state, + ask: "command", + text: "rm file", + dcgDecision: { decision: "allow" }, + }), + ).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "rm file", pattern: "rm" } }) + }) + }) }) diff --git a/src/core/auto-approval/__tests__/fixtures.ts b/src/core/auto-approval/__tests__/fixtures.ts index de19c95986..6ac092de43 100644 --- a/src/core/auto-approval/__tests__/fixtures.ts +++ b/src/core/auto-approval/__tests__/fixtures.ts @@ -45,6 +45,7 @@ export const baseState: State = { alwaysAllowExecute: false, alwaysAllowFollowupQuestions: false, destructiveCommandGuardEnabled: false, + alwaysDenyUnapprovedCommands: false, allowedCommands: [], deniedCommands: [], } diff --git a/src/core/auto-approval/autoDenyReason.ts b/src/core/auto-approval/autoDenyReason.ts new file mode 100644 index 0000000000..b13d438b46 --- /dev/null +++ b/src/core/auto-approval/autoDenyReason.ts @@ -0,0 +1,52 @@ +/** + * Structured detail attached to automatic command denials. + * + * An automatic denial is one produced by the system rather than by a user + * clicking "reject": from policy (denylist, blanket auto-deny, DCG block) or + * from a guard-state inconsistency (`guard_unavailable`), which is not itself + * a policy denial. The detail travels from `checkAutoApproval` through + * `Task.ask` to `presentAssistantMessage`, where it selects the structured + * `formatResponse.toolAutoDenied` payload instead of the user-rejection + * wording — and marks the denial as scoped to its own tool call, so it never + * aborts the rest of the turn. + */ +export type AutoDenyDetail = { + kind: "dcg" | "denylist" | "not_allowlisted" | "dangerous_substitution" | "malformed_command" | "guard_unavailable" + /** Offending sub-command text (or the full command when no single offending sub-command applies). */ + command?: string + /** Matched denied prefix, for `denylist` denials. */ + pattern?: string + /** Raw DCG reason string, for `dcg` denials (not the i18n'd chat string). */ + dcgReason?: string + /** Raw DCG rule id, for `dcg` denials. */ + dcgRuleId?: string + /** Parse-error message, for `malformed_command` denials. */ + parseError?: string +} + +/** + * Build the model-facing reason string for an automatic denial. + * + * Hardcoded English, matching the existing `formatResponse` precedent: these + * strings go to the model, not the chat UI (chat rows stay i18n'd). The text + * deliberately never suggests asking the user — in hands-free mode that would + * invite `ask_followup_question` and defeat the purpose of the feature. + */ +export function buildAutoDenyReason(detail: AutoDenyDetail): string { + switch (detail.kind) { + case "denylist": + return `Command \`${detail.command ?? "(unknown)"}\` matches denied prefix \`${detail.pattern ?? "(unknown)"}\`.` + case "not_allowlisted": + return `Command \`${detail.command ?? "(unknown)"}\` is not on the command allowlist.` + case "dangerous_substitution": + return "Command contains shell expansions (${...} forms, process substitution, and similar) that are never auto-approved. Choose an approved command without shell expansions." + case "malformed_command": + return detail.parseError ?? "Command contains a shell syntax error." + case "dcg": { + const base = `Destructive Command Guard denied the command: ${detail.dcgReason ?? "no reason provided"}` + return detail.dcgRuleId ? `${base} (Rule: ${detail.dcgRuleId})` : base + } + case "guard_unavailable": + return `Command \`${detail.command ?? "(unknown)"}\` was not executed: the Destructive Command Guard is enabled but supplied no verdict, which is an internal guard-state inconsistency, not a policy denial. You may retry the same command.` + } +} diff --git a/src/core/auto-approval/commands.ts b/src/core/auto-approval/commands.ts index 82b937e66f..9d7289c265 100644 --- a/src/core/auto-approval/commands.ts +++ b/src/core/auto-approval/commands.ts @@ -259,8 +259,46 @@ export function getCommandDecision( allowedCommands: string[], deniedCommands?: string[], ): CommandDecision { + return getCommandDecisionDetailed(command, allowedCommands, deniedCommands).decision +} + +/** + * Result of {@link getCommandDecisionDetailed}: {@link getCommandDecision}'s + * plain decision plus the offending sub-command and the matched pattern that + * produced it, so automatic denials can tell the model exactly what went + * wrong. + */ +export interface CommandDecisionDetail { + decision: CommandDecision + /** + * For `auto_deny`: the first sub-command matched by the denylist. + * For `ask_user`: the first sub-command lacking an allowlist match. + * For `malformed_command`: the raw (unsplit) command string. + */ + offendingCommand?: string + /** The matched denied prefix, for `auto_deny` decisions. */ + matchedPattern?: string + /** Human-readable shell syntax error, for `malformed_command` decisions. */ + parseError?: string +} + +/** + * Same decision logic as {@link getCommandDecision}, additionally naming the + * offending sub-command (and matched denied prefix) that produced the + * decision, so the auto-deny feedback can point the model at the specific + * part of a chained command that caused the rejection. + * + * The chain-level aggregation mirrors `getCommandDecision` exactly: + * "any denial blocks all", dangerous substitutions force `ask_user`, and a + * parse error yields `malformed_command`. + */ +export function getCommandDecisionDetailed( + command: string, + allowedCommands: string[], + deniedCommands?: string[], +): CommandDecisionDetail { if (!command?.trim()) { - return "auto_approve" + return { decision: "auto_approve" } } // Parse into sub-commands (split by &&, ||, ;, |). parseCommand also @@ -274,34 +312,44 @@ export function getCommandDecision( // distinct decision lets callers surface a useful message to the agent // rather than silently presenting the command for user approval. if (parseError !== null) { - return "malformed_command" + return { decision: "malformed_command", offendingCommand: command, parseError: parseError.message } } - // Check each sub-command and collect decisions - const decisions: CommandDecision[] = subCommands.map((cmd) => { - // Remove simple PowerShell-like redirections (e.g. 2>&1) before checking - const cmdWithoutRedirection = cmd.replace(/\d*>&\d*/, "").trim() + // Remove simple PowerShell-like redirections (e.g. 2>&1) before checking + const sanitizedCommands = subCommands.map((cmd) => cmd.replace(/\d*>&\d*/, "").trim()) - return getSingleCommandDecision(cmdWithoutRedirection, allowedCommands, deniedCommands) - }) + const decisions: CommandDecision[] = sanitizedCommands.map((cmd) => + getSingleCommandDecision(cmd, allowedCommands, deniedCommands), + ) - // If any sub-command is denied, deny the whole command - if (decisions.includes("auto_deny")) { - return "auto_deny" + // If any sub-command is denied, deny the whole command; name the first + // denied sub-command and the denied prefix it matched. + const denyIndex = decisions.indexOf("auto_deny") + if (denyIndex !== -1) { + const offendingCommand = sanitizedCommands[denyIndex] + const lowerMatch = findLongestPrefixMatch(offendingCommand, deniedCommands || []) + // Prefer the original-cased pattern the user typed, falling back to the + // case-insensitive match itself. + const matchedPattern = + (deniedCommands || []).find((pattern) => pattern.toLowerCase() === lowerMatch) ?? lowerMatch ?? undefined + + return { decision: "auto_deny", offendingCommand, matchedPattern } } // Require explicit user approval for dangerous patterns if (containsDangerousSubstitution(command)) { - return "ask_user" + return { decision: "ask_user" } } // If all sub-commands are approved, approve the whole command if (decisions.every((decision) => decision === "auto_approve")) { - return "auto_approve" + return { decision: "auto_approve" } } - // Otherwise, ask user - return "ask_user" + // Otherwise, ask user; name the first sub-command with no allowlist match. + const askIndex = decisions.indexOf("ask_user") + + return { decision: "ask_user", offendingCommand: askIndex !== -1 ? sanitizedCommands[askIndex] : undefined } } /** diff --git a/src/core/auto-approval/index.ts b/src/core/auto-approval/index.ts index 2fc4d1d45e..2aa5008fc8 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -8,12 +8,15 @@ import { isNonBlockingAsk, } from "@roo-code/types" +import type { DcgDecision } from "../../services/destructive-command-guard/runner" + import { ClineAskResponse } from "../../shared/WebviewMessage" import { isWriteToolAction, isReadOnlyToolAction } from "./tools" import { isMcpToolAlwaysAllowed } from "./mcp" -import { getCommandDecision } from "./commands" +import { containsDangerousSubstitution, getCommandDecisionDetailed } from "./commands" import { isFileMatchedByPatterns } from "./filePatterns" +import { type AutoDenyDetail } from "./autoDenyReason" // We have auto-approval actions for different categories. export type AutoApprovalState = @@ -38,6 +41,7 @@ export type AutoApprovalStateOptions = | "allowedCommands" // For `alwaysAllowExecute`. | "deniedCommands" | "destructiveCommandGuardEnabled" + | "alwaysDenyUnapprovedCommands" // For `alwaysAllowExecute` (blanket auto-deny). /** * Every file a tool action names, as far as the allowlists are concerned. @@ -136,7 +140,16 @@ function isWriteAllowedByPatterns( export type CheckAutoApprovalResult = | { decision: "approve" } - | { decision: "deny" } + /** + * Automatic denial. `autoDeny` carries the structured reason and the + * offending sub-command when the denial came from command policy (denylist + * match, blanket auto-deny, or a DCG block under blanket mode) or from the + * guard-state inconsistency the command branch denies as + * `guard_unavailable`. It marks the denial as policy-scoped — the model + * receives an explanatory `auto_deny` result, and unlike a user rejection + * the denial does not abort the remaining tool calls of the turn. + */ + | { decision: "deny"; autoDeny?: AutoDenyDetail } | { decision: "ask" } | { decision: "timeout" @@ -150,6 +163,7 @@ export async function checkAutoApproval({ ask, text, isProtected, + dcgDecision, }: { state?: Pick /** @@ -168,6 +182,14 @@ export async function checkAutoApproval({ ask: ClineAsk text?: string isProtected?: boolean + /** + * The verdict from a Destructive Command Guard run that the caller + * (ExecuteCommandTool) already performed for this exact command. Only + * provided for `ask: "command"` when DCG is enabled; undefined otherwise. + * Infra failures in DCG never produce a verdict — they surface as a tool + * error before this check, so a verdict here is authoritative. + */ + dcgDecision?: DcgDecision }): Promise { if (isNonBlockingAsk(ask)) { return { decision: "approve" } @@ -238,23 +260,95 @@ export async function checkAutoApproval({ } if (state.alwaysAllowExecute === true) { + const blanketDeny = state.alwaysDenyUnapprovedCommands === true + // Execute commands immediately when DCG allows them. ExecuteCommandTool - // marks commands blocked by DCG as protected before reaching this check, - // which keeps the explicit user approval prompt for those commands. When - // enabled, DCG is the authoritative command policy, so Zoo's allow and deny - // lists are intentionally bypassed for commands that DCG allows. + // passes the guard's verdict through so this single decision point can + // act on it. When enabled, DCG is the authoritative command policy, so + // Zoo's allow and deny lists are intentionally bypassed for commands + // DCG rules on — and an allowlist match cannot rescue a DCG denial. if (state.destructiveCommandGuardEnabled === true) { + if (dcgDecision?.decision === "deny") { + // Blanket on: auto-deny, forwarding the guard's reason and + // rule to the model. Blanket off: this ask falls through + // to the normal user prompt. + return blanketDeny + ? { + decision: "deny", + autoDeny: { + kind: "dcg", + command: text, + dcgReason: dcgDecision.reason, + dcgRuleId: dcgDecision.ruleId, + }, + } + : { decision: "ask" } + } + + // A verdictless ask under an enabled guard is an inconsistent + // guard state, not a guard decision: ExecuteCommandTool computes + // and forwards its verdict on one straight-line path, so an ask + // arriving without one means the setting flipped on mid-flight or + // the caller never ran the guard. Deny with an explicitly + // retryable detail — the command did not run, the turn is not + // aborted, and a re-issue re-reads the setting. + if (dcgDecision === undefined) { + return { + decision: "deny", + autoDeny: { kind: "guard_unavailable", command: text }, + } + } + + // The guard returned an explicit allow verdict: DCG is the + // authoritative policy — approve. return { decision: "approve" } } - const decision = getCommandDecision(text, state.allowedCommands || [], state.deniedCommands || []) + const { decision, offendingCommand, matchedPattern, parseError } = getCommandDecisionDetailed( + text, + state.allowedCommands || [], + state.deniedCommands || [], + ) if (decision === "auto_approve") { return { decision: "approve" } } else if (decision === "auto_deny") { - return { decision: "deny" } + // Denylist denials are automatic denials and carry their structured + // detail even when the blanket setting is off: they were never user + // rejections, so the model gets the precise reason and the denial + // stays scoped to this tool call. + return { + decision: "deny", + autoDeny: { kind: "denylist", command: offendingCommand, pattern: matchedPattern }, + } + } else if (decision === "malformed_command") { + // Defense in depth: ExecuteCommandTool blocks shell syntax errors as + // a retryable tool_error before any ask is created, so this is + // normally unreachable. Under blanket mode deny rather than ask. + return blanketDeny + ? { + decision: "deny", + autoDeny: { kind: "malformed_command", command: offendingCommand, parseError }, + } + : { decision: "ask" } } else { - return { decision: "ask" } + // ask_user: with the blanket setting on, unapproved commands are + // auto-denied instead of interrupting a hands-free session. + if (!blanketDeny) { + return { decision: "ask" } + } + + if (containsDangerousSubstitution(text)) { + // The substitution check runs chain-wide, so the full command + // is the offending one; the list classifier reports no single + // offending sub-command when it defers to this branch. + return { + decision: "deny", + autoDeny: { kind: "dangerous_substitution", command: offendingCommand ?? text }, + } + } + + return { decision: "deny", autoDeny: { kind: "not_allowlisted", command: offendingCommand } } } } } @@ -334,3 +428,4 @@ export async function checkAutoApproval({ } export { AutoApprovalHandler } from "./AutoApprovalHandler" +export { type AutoDenyDetail, buildAutoDenyReason } from "./autoDenyReason" diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index a547dbe474..85a2192217 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -113,6 +113,19 @@ export class MessageQueueService extends EventEmitter { return this._messages.length === 0 } + /** + * Whether at least one queued message is still available to be claimed. + * + * `isEmpty()` measures the queue's length and is blind to claims, so a + * message some consumer is holding (claim is not a removal) still makes it + * report a non-empty queue. Consumers that are about to take a message need + * this instead: it answers "is there anything I may take", which is false + * when every remaining message is already spoken for. + */ + public hasUnclaimed(): boolean { + return this._messages.some((message) => !this.claimedMessageIds.has(message.id)) + } + public dispose(): void { this._messages = [] this.claimedMessageIds.clear() diff --git a/src/core/message-queue/__tests__/MessageQueueService.spec.ts b/src/core/message-queue/__tests__/MessageQueueService.spec.ts index d0abe86bf2..567dca314b 100644 --- a/src/core/message-queue/__tests__/MessageQueueService.spec.ts +++ b/src/core/message-queue/__tests__/MessageQueueService.spec.ts @@ -37,4 +37,34 @@ describe("MessageQueueService claims", () => { expect(queue.messages).toEqual([message]) expect(queue.claimNextMessage()).toEqual(message) }) + + it("reports no unclaimed message only when every queued message is spoken for", () => { + const queue = new MessageQueueService() + queue.addMessage("first") + queue.addMessage("second") + expect(queue.hasUnclaimed()).toBe(true) + + expect(queue.claimNextMessage()).toBeDefined() + // One message held, one still available: a new consumer can still claim. + expect(queue.hasUnclaimed()).toBe(true) + + expect(queue.claimNextMessage()).toBeDefined() + // Both held: the queue is not empty, but nothing remains claimable. + expect(queue.hasUnclaimed()).toBe(false) + }) + + it("pins isEmpty() to queue length so an outstanding claim does not empty it", () => { + const queue = new MessageQueueService() + const message = queue.addMessage("held")! + expect(queue.claimNextMessage()).toEqual(message) + + // A claim is not a removal, so length-based emptiness still counts the + // held message; consumers deciding whether they may take a message must + // use hasUnclaimed() instead. + expect(queue.isEmpty()).toBe(false) + expect(queue.hasUnclaimed()).toBe(false) + + queue.releaseMessage(message.id) + expect(queue.hasUnclaimed()).toBe(true) + }) }) diff --git a/src/core/prompts/__tests__/responses-tool-auto-denied.spec.ts b/src/core/prompts/__tests__/responses-tool-auto-denied.spec.ts new file mode 100644 index 0000000000..5ee03bfcd1 --- /dev/null +++ b/src/core/prompts/__tests__/responses-tool-auto-denied.spec.ts @@ -0,0 +1,51 @@ +// npx vitest run core/prompts/__tests__/responses-tool-auto-denied.spec.ts + +import { formatResponse } from "../responses" + +// `note`/`suggestion` are hardcoded model-facing copy: the payload must never +// advise asking the user, and wording edits must be deliberate, so they are +// pinned verbatim here rather than matched loosely. +const NOTE = "The command chain was rejected in its entirety; none of the chained commands were executed." +const SUGGESTION = + "Re-run the remaining commands as separate execute_command calls using approved commands only, or choose an approved alternative." + +describe("formatResponse.toolAutoDenied", () => { + it("emits the full auto_deny payload when every field is supplied", () => { + const parsed = JSON.parse( + formatResponse.toolAutoDenied({ + reason: "Command `rm x` is not on the command allowlist.", + offendingCommand: "rm x", + ruleId: "R-1", + }), + ) + + expect(parsed.status).toBe("denied") + expect(parsed.type).toBe("auto_deny") + expect(parsed.reason).toBe("Command `rm x` is not on the command allowlist.") + expect(parsed.offending_command).toBe("rm x") + expect(parsed.rule_id).toBe("R-1") + expect(parsed.note).toBe(NOTE) + expect(parsed.suggestion).toBe(SUGGESTION) + }) + + it("omits offending_command and rule_id keys when the detail carries neither", () => { + const parsed = JSON.parse(formatResponse.toolAutoDenied({ reason: "Denied by policy." })) + + expect(parsed.status).toBe("denied") + expect(parsed.type).toBe("auto_deny") + expect(parsed.reason).toBe("Denied by policy.") + expect(parsed).not.toHaveProperty("offending_command") + expect(parsed).not.toHaveProperty("rule_id") + }) + + it("pins the note and suggestion copy verbatim, including the no-ask-the-user wording", () => { + const parsed = JSON.parse(formatResponse.toolAutoDenied({ reason: "Denied by policy." })) + + expect(parsed.note).toBe(NOTE) + expect(parsed.suggestion).toBe(SUGGESTION) + // Asking the user would stall a hands-free session; the suggestion must + // route the model to re-issue approved commands instead. + expect(parsed.note).not.toMatch(/ask the user/i) + expect(parsed.suggestion).not.toMatch(/ask the user/i) + }) +}) diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index 60b5b4123a..7714438431 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -11,6 +11,28 @@ export const formatResponse = { message: "The user denied this operation.", }), + /** + * Structured result for an automatic (policy) denial of a command — a + * denylist match, a blanket auto-deny, or a Destructive Command Guard + * block under blanket mode. Distinct from `toolDenied` (a real user + * rejection): the reason names the offending part of the chain, states + * that nothing in it executed, and — deliberately — never suggests asking + * the user, which would invite `ask_followup_question` and stall a + * hands-free session. Hardcoded English like the rest of this module: + * model-facing strings are not i18n'd. + */ + toolAutoDenied: (detail: { reason: string; offendingCommand?: string; ruleId?: string }) => + JSON.stringify({ + status: "denied", + type: "auto_deny", + reason: detail.reason, + offending_command: detail.offendingCommand, + rule_id: detail.ruleId, + note: "The command chain was rejected in its entirety; none of the chained commands were executed.", + suggestion: + "Re-run the remaining commands as separate execute_command calls using approved commands only, or choose an approved alternative.", + }), + toolDeniedWithFeedback: (feedback?: string) => JSON.stringify({ status: "denied", diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index d5313f68cf..0807a5b539 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -30,6 +30,7 @@ import { type ClineMessage, type ClineSay, type ClineAsk, + type ExtensionState, type ToolProgressStatus, type HistoryItem, type PendingTaskAction, @@ -74,7 +75,13 @@ import { t } from "../../i18n" import { getApiMetrics, hasTokenUsageChanged, hasToolUsageChanged } from "../../shared/getApiMetrics" import { ClineAskResponse } from "../../shared/WebviewMessage" import { defaultModeSlug, getModeBySlug } from "../../shared/modes" -import { DiffStrategy, type ToolUse, type ToolParamName, toolParamNames } from "../../shared/tools" +import { + DiffStrategy, + type AutoApprovalContext, + type ToolUse, + type ToolParamName, + toolParamNames, +} from "../../shared/tools" import { getModelMaxOutputTokens } from "../../shared/api" // services @@ -133,7 +140,7 @@ import { import { processUserContentMentions } from "../mentions/processUserContentMentions" import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" -import { AutoApprovalHandler, checkAutoApproval } from "../auto-approval" +import { type AutoDenyDetail, AutoApprovalHandler, checkAutoApproval } from "../auto-approval" import { MessageManager } from "../message-manager" import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" @@ -158,6 +165,51 @@ const QUEUED_FEEDBACK_SAVE_RETRY_DELAYS_MS = [250, 1_000, 4_000] as const type QueuedAskResolution = { response: ClineAskResponse; requiresDurableAck: boolean } +/** + * Denial kinds the command policy produces only while blanket auto-deny is on + * (`checkAutoApproval` returns `ask` for these when the setting is off — see + * the command branch of `src/core/auto-approval/index.ts`), so a denial of one + * of these kinds is blanket-caused. `denylist` and `guard_unavailable` are + * excluded: they deny independently of the blanket setting. + */ +export const BLANKET_DENY_AUTO_DENY_KINDS: ReadonlySet = new Set([ + "dcg", + "not_allowlisted", + "dangerous_substitution", + "malformed_command", +]) + +/** + * What to do with a claimed queued message about to answer a command ask, + * decided against the auto-approval policy as it stands NOW. + */ +type QueuedCommandPolicyAction = + | { action: "consume" } + | { action: "approve" } + | { action: "deny"; detail?: AutoDenyDetail } + | { action: "release" } + +/** + * Whether blanket auto-deny currently engages. The three settings act as a + * conjunction: blanket deny only means anything while auto-approval is on and + * command auto-approval is on — without those two an unapproved command is + * prompted rather than auto-approved, so there is nothing to deny. + * + * Single source of truth for the derivation: the ask-time snapshot, the + * consume-site re-reads, and the tool-level execute-time re-check must not + * drift apart, or a flip landing between two of them decides execution or + * consumption on stale policy. + */ +export function isBlanketDenyEngaged( + state?: Pick, +): boolean { + return ( + state?.alwaysDenyUnapprovedCommands === true && + state?.autoApprovalEnabled === true && + state?.alwaysAllowExecute === true + ) +} + function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolution | undefined { if (type === "command_output") { return undefined @@ -370,6 +422,23 @@ export class Task extends EventEmitter implements TaskLike { private askResponse?: ClineAskResponse private askResponseText?: string private askResponseImages?: string[] + /** + * Structured detail of the automatic denial that resolved the current ask, + * set when `checkAutoApproval` denies via policy (denylist, blanket + * auto-deny, or a DCG block in blanket mode) and consumed (cleared) by the + * `ask()` result. System-generated, unlike `askResponseText` which carries + * user feedback and triggers a `user_feedback` say row. + */ + private pendingAutoDenyDetail?: AutoDenyDetail + /** + * Set while the current turn has blanket-denied a command ask. The queued + * messages such a denial deliberately leaves in place were typed in response + * to that denial, not as approval — so a later ask in the same turn must not + * consume one as `yesButtonClicked` (which would silently approve it with the + * interactive prompt suppressed). Read at the queued-message consume sites; + * cleared by the per-turn reset beside `didToolFailInCurrentTurn`. + */ + private blanketDeniedCommandThisTurn = false public lastMessageTs?: number private autoApprovalTimeoutRef?: NodeJS.Timeout @@ -1004,6 +1073,146 @@ export class Task extends EventEmitter implements TaskLike { return undefined } + /** + * Re-read the auto-approval policy immediately before a queued message would + * stand in for a command ask's approval, and consult the full command policy + * under the fresh state if blanket deny now engages. + * + * The queued-answer shortcut skips `checkAutoApproval` and the ask's decision + * otherwise rides on a single settings snapshot taken before the prompt was + * shown. A blanket-deny flip landing between the snapshot and the consume is + * invisible to that frozen gate, and the queued message would auto-approve an + * unallowlisted command — fail-OPEN through a window as wide as the prompt + * dwell. Re-reading here closes it: while blanket deny engages, the message + * is never consumed as approval and the ask gets the structured denial policy + * would have produced without a queued message. + * + * With `abortSignal`, the re-check settles on the abort itself instead of + * merely losing a race against it: the fresh-state read ends in uncancellable + * provider work, so a post-await check alone would leave this promise pending + * whenever the read never lands, retaining the task and running policy work + * after the abort. An aborted re-check resolves to the `release` no-op, + * leaving the claim and the pending ask to the caller's release path. + */ + private async recheckQueuedCommandPolicy( + { + text, + isProtected, + dcgDecision, + }: { + text?: string + isProtected?: boolean + dcgDecision?: AutoApprovalContext["dcgDecision"] + }, + abortSignal?: AbortSignal, + ): Promise { + // Resolving a sentinel rather than rejecting keeps a late rejection from + // the uncancellable read unhandled once the abort has won the race, and + // routes the abort through the same release early-return as the checks. + const signal = abortSignal + const abortPromise = signal + ? new Promise<"aborted">((resolve) => { + if (signal.aborted) { + resolve("aborted") + } else { + signal.addEventListener("abort", () => resolve("aborted"), { once: true }) + } + }) + : undefined + const freshState = abortPromise + ? await Promise.race([this.providerRef.deref()?.getState(), abortPromise]) + : await this.providerRef.deref()?.getState() + // The abort either won the race (sentinel) or landed while the read was + // still resolving; both settle the re-check before any policy work. + if (freshState === "aborted" || signal?.aborted) { + return { action: "release" } + } + if (!isBlanketDenyEngaged(freshState)) { + // Disengaged: the queued answer is a legitimate approval, as before. + return { action: "consume" } + } + // Engaged: the full policy decides, with the fresh state. `cwd` and the + // forwarded DCG verdict match the ask-time `checkAutoApproval` call. + const approval = await checkAutoApproval({ + state: freshState, + cwd: this.cwd, + ask: "command", + text, + isProtected, + dcgDecision, + }) + if (signal?.aborted) { + return { action: "release" } + } + if (approval.decision === "deny") { + return { action: "deny", detail: approval.autoDeny } + } + if (approval.decision === "approve") { + return { action: "approve" } + } + return { action: "release" } + } + + /** + * Apply a queued-command policy re-check outcome to a claimed message: the + * claim is released on every path except the legitimate consume, and an + * engaged-policy outcome resolves the ask the same way the no-queued-message + * path would (structured denial, approval, or — for a plain `ask` decision — + * left pending for the user). + */ + private applyQueuedCommandPolicyAction( + action: QueuedCommandPolicyAction, + message: QueuedMessage, + resolution: QueuedAskResolution, + ): string | undefined { + if (action.action !== "consume") { + this.messageQueueService.releaseMessage(message.id) + } + switch (action.action) { + case "consume": + return this.handleQueuedAskResponse(message, resolution) + case "deny": + if (action.detail && BLANKET_DENY_AUTO_DENY_KINDS.has(action.detail.kind)) { + this.blanketDeniedCommandThisTurn = true + } + this.pendingAutoDenyDetail = action.detail + this.denyAsk() + return undefined + case "approve": + this.approveAsk() + return undefined + case "release": + return undefined + } + } + + /** + * Shared claim-gate for the two queued-message consume sites. + * + * The latch blocks a message left in the queue by a command this turn already + * blanket-denied (it answers that denial, not the current ask), and + * `hasUnclaimed()` replaces the length-only `isEmpty()`, which reports a + * queue containing nothing but claims as available for a new consumer. + * `isMessageQueued`/`isStatusMutable` keep `isEmpty()` semantics on purpose: + * flipping those would re-enable interactive prompt timers whenever a claim + * is outstanding. + */ + private mayDrainQueuedMessageForAsk(): boolean { + return !this.blanketDeniedCommandThisTurn && this.messageQueueService.hasUnclaimed() + } + + /** + * Latch the turn after a blanket-caused command denial detected outside + * the ask path (the tool's execute-time policy re-check). A message left + * in the queue by that denial answers the denial, not the next ask, so + * without the latch a later non-command ask would consume it as an + * approval. Deliberately not cleared mid-turn: the per-turn reset owns + * clearing. + */ + public recordBlanketCommandDenial(): void { + this.blanketDeniedCommandThisTurn = true + } + static create(options: TaskOptions): [Task, Promise] { const instance = new Task({ ...options, startTask: false }) const { images, task, historyItem } = options @@ -1436,7 +1645,19 @@ export class Task extends EventEmitter implements TaskLike { partial?: boolean, progressStatus?: ToolProgressStatus, isProtected?: boolean, - ): Promise<{ response: ClineAskResponse; text?: string; images?: string[]; queuedMessageId?: string }> { + autoApprovalContext?: AutoApprovalContext, + ): Promise<{ + response: ClineAskResponse + text?: string + images?: string[] + queuedMessageId?: string + /** + * Present when the ask was resolved by an automatic (policy) denial + * rather than a user click. Consumers must not treat this as a user + * rejection: the denial is scoped to its own tool call. + */ + autoDenyDetail?: AutoDenyDetail + }> { // If this Cline instance was aborted by the provider, then the only // thing keeping us alive is a promise still running in the background, // in which case we don't want to send its result to the webview as it @@ -1459,8 +1680,28 @@ export class Task extends EventEmitter implements TaskLike { // rendered, leaving them stuck on-screen). const provider = this.providerRef.deref() const state = provider ? await provider.getState() : undefined + // The blanket auto-deny setting only engages while command auto-approval + // is on; while it is disengaged, the queued-message shortcut below is + // unaffected. + const blanketDenyEngaged = isBlanketDenyEngaged(state) + // A queued message normally answers the pending ask, which for command asks + // means an unconditional auto-approve. That shortcut must never bypass + // blanket deny: while it is engaged, a command ask keeps its policy + // decision and the queued message is left in place for a later turn + // instead of being consumed as approval. The shared claim gate adds the + // per-turn latch: a message a blanket denial left behind answers that + // denial, not this ask, and a claim would force the `ask` decision below + // while the policy would have auto-answered it — holding a prompt no user + // has to see and stalling a hands-free session. Leaving the message + // unclaimed lets the policy decision stand and keeps the message queued. + const queueMayAnswerThisAsk = !(blanketDenyEngaged && type === "command") const queuedMessage = - partial === true || type === "command_output" ? undefined : this.messageQueueService.claimNextMessage() + partial === true || + type === "command_output" || + !queueMayAnswerThisAsk || + !this.mayDrainQueuedMessageForAsk() + ? undefined + : this.messageQueueService.claimNextMessage() const queuedAskResolution = queuedMessage ? queuedResponseForAsk(type, text) : undefined // `this.cwd`, not `provider.cwd`: // The path inside `text` was made relative to this task's workspace, @@ -1468,7 +1709,14 @@ export class Task extends EventEmitter implements TaskLike { // currently reports. const approval = queuedAskResolution ? ({ decision: "ask" } as const) - : await checkAutoApproval({ state, cwd: this.cwd, ask: type, text, isProtected }) + : await checkAutoApproval({ + state, + cwd: this.cwd, + ask: type, + text, + isProtected, + dcgDecision: autoApprovalContext?.dcgDecision, + }) const isAutoAnswered = approval.decision === "approve" || approval.decision === "deny" const autoApprovalDecision = isAutoAnswered ? approval.decision : undefined @@ -1580,6 +1828,28 @@ export class Task extends EventEmitter implements TaskLike { const timeouts: NodeJS.Timeout[] = [] + // Record the structured detail of an automatic (policy) denial so the + // `ask()` result can hand it to the caller. Assigned unconditionally so + // a stale detail from a previous ask can never leak into this result. + // Deliberately not routed through `askResponseText`: that field means + // *user feedback* and triggers a `user_feedback` say row, while this + // reason is system-generated. + this.pendingAutoDenyDetail = approval.decision === "deny" ? approval.autoDeny : undefined + // A blanket-caused denial leaves the queued messages this turn for later + // turns; latch so no ask in this turn consumes one as approval. Set only on + // blanket-caused denials — here and at the consume-site re-check + // (`applyQueuedCommandPolicyAction`) — and never cleared mid-turn: the + // per-turn reset owns clearing, so a later non-denied ask cannot drop the + // latch prematurely. + if ( + type === "command" && + approval.decision === "deny" && + approval.autoDeny && + BLANKET_DENY_AUTO_DENY_KINDS.has(approval.autoDeny.kind) + ) { + this.blanketDeniedCommandThisTurn = true + } + if (approval.decision === "approve") { this.approveAsk() } else if (approval.decision === "deny") { @@ -1604,12 +1874,45 @@ export class Task extends EventEmitter implements TaskLike { const isStatusMutable = !partial && isBlocking && !isMessageQueued && approval.decision === "ask" let queuedMessageId: string | undefined - if (isStatusMutable) { + // Arm the interactive/resumable/idle status timers for this ask: the + // single source of that arm, shared between the queue-free case and the + // claim-gated and queued-release paths below. A gated or released claim + // keeps the message in the queue, so `isMessageQueued` stays true and + // `isStatusMutable` — which requires an empty queue — stays false while + // the ask waits for the user; arming only from `isStatusMutable` would + // leave hands-free/API consumers seeing `Running` with no + // `TaskInteractive`/`interactionRequired` for a prompt that is in fact + // pending. Idempotent: several arm sites can fire for one ask (e.g. the + // queue-free arm, then a drain-site release), and a second arm would + // double-emit the event. The `timeouts` array is the arm ledger, and a + // `finally` around the wait owns the `clearTimeout` teardown on every + // settle path, including the abort and supersession throws. The + // callbacks still re-check liveness at fire time: an already-due timer + // can outrun that teardown, and a status transition published after + // abort would describe a task that no longer runs. + const armAskStatusTimers = (): void => { + const askStillPending = this.askResponse === undefined && this.lastMessageTs === askTs + if (!askStillPending || this.abort || partial || approval.decision !== "ask" || timeouts.length > 0) { + return + } + const statusMutationTimeout = 2_000 + // Fire-time liveness check for the timers armed below. Clearing a due + // timer does not retract it: when abort lands between the wait's last + // poll tick and the timer's due time, the callback runs before + // `ask()`'s continuation reaches the teardown sweep, so liveness has + // to be re-checked here. Read-only: the `timeouts` ledger is never + // touched from the callbacks. + const statusTimerStillLive = (): boolean => + !this.abort && this.askResponse === undefined && this.lastMessageTs === askTs + if (isInteractiveAsk(type)) { timeouts.push( setTimeout(() => { + if (!statusTimerStillLive()) { + return + } const message = this.findMessageByTimestamp(askTs) if (message) { @@ -1625,6 +1928,9 @@ export class Task extends EventEmitter implements TaskLike { } else if (isResumableAsk(type)) { timeouts.push( setTimeout(() => { + if (!statusTimerStillLive()) { + return + } const message = this.findMessageByTimestamp(askTs) if (message) { @@ -1636,6 +1942,9 @@ export class Task extends EventEmitter implements TaskLike { } else if (isIdleAsk(type)) { timeouts.push( setTimeout(() => { + if (!statusTimerStillLive()) { + return + } const message = this.findMessageByTimestamp(askTs) if (message) { @@ -1645,74 +1954,236 @@ export class Task extends EventEmitter implements TaskLike { }, statusMutationTimeout), ) } - } else if (isMessageQueued && shouldDrainQueuedMessageForAsk && queuedMessage && queuedAskResolution) { - queuedMessageId = this.handleQueuedAskResponse(queuedMessage, queuedAskResolution) } - // Wait for askResponse to be set - await pWaitFor( - () => { - if (this.abort || this.askResponse !== undefined || this.lastMessageTs !== askTs) { - return true + if (isStatusMutable) { + armAskStatusTimers() + } else if (queuedMessage && queuedAskResolution) { + if (type === "command") { + // The snapshot gate is frozen; blanket deny may have engaged since the + // ask began. Re-read policy before the message stands in for approval. + // Drain-site parity for cancellation and cleanup: the fresh policy + // read ends in an uncancellable provider read, so an abort poller + // aborts a signal the re-check itself awaits — settling it on the + // abort instead of leaving a pending promise that retains this task + // and runs policy post-abort — and one `finally` releases the claim + // on the abort, throw, supersession, and "release" outcomes alike. + const recheckAbort = new AbortController() + const checkAbort = () => { + if (this.abort) { + recheckAbort.abort() + } } - - // If a queued message arrives while we're blocked on an ask (e.g. a follow-up - // suggestion click that was incorrectly queued due to UI state), consume it - // immediately so the task doesn't hang. - if (shouldDrainQueuedMessageForAsk && !this.messageQueueService.isEmpty()) { - const message = this.messageQueueService.claimNextMessage() - const resolution = message ? queuedResponseForAsk(type, text) : undefined - if (message && resolution) { - queuedMessageId = this.handleQueuedAskResponse(message, resolution) + checkAbort() + const abortWatcher = setInterval(checkAbort, 100) + try { + const action = await this.recheckQueuedCommandPolicy( + { + text, + isProtected, + dcgDecision: autoApprovalContext?.dcgDecision, + }, + recheckAbort.signal, + ) + if (!this.abort && this.askResponse === undefined && this.lastMessageTs === askTs) { + // Any outcome on a still-live ask — the user answered or the + // ask was superseded — leaves the message for the next + // consumer; the `finally` below releases the claim and the ask + // resolves with the user's own response via the pWaitFor. + queuedMessageId = this.applyQueuedCommandPolicyAction( + action, + queuedMessage, + queuedAskResolution, + ) + // A "release" outcome leaves the ask pending; consume/deny/approve + // resolved it, and the arm's pending check declines to arm then. + armAskStatusTimers() + } + } catch (error) { + // Drain-site parity: a failed re-check must not reject ask() nor + // strand the claim; the prompt stays pending for the user. + console.error("[Task#ask] queued command policy re-check failed:", error) + armAskStatusTimers() + } finally { + clearInterval(abortWatcher) + // One release path for abort, throw, supersession, and "release". + // `releaseMessage` is idempotent, so it stays safe beside the + // helper-internal release. The durable consume is the one outcome + // that must keep its claim until persistence removes the message — + // it is the only path that assigned `queuedMessageId` here. + if (queuedMessageId !== queuedMessage.id) { + this.messageQueueService.releaseMessage(queuedMessage.id) } } + } else { + queuedMessageId = this.handleQueuedAskResponse(queuedMessage, queuedAskResolution) + } + } else if (shouldDrainQueuedMessageForAsk && isMessageQueued) { + // The claim gate (per-turn latch, or blanket deny engaged for a command + // ask) left the queued message untouched. If the policy still leaves the + // prompt pending, the non-empty queue keeps `isStatusMutable` false, so + // the interactive arm must run from here — the same reason a release + // re-arms. For an auto-answered ask the arm's pending check declines. + armAskStatusTimers() + } + + // At most one drain-site policy re-check is in flight per ask; the + // pWaitFor predicate is synchronous, so its await runs out here. + let queuedCommandPolicyCheck: Promise | undefined + const verifyDrainedCommandMessage = async ( + message: QueuedMessage, + resolution: QueuedAskResolution, + ): Promise => { + // The fresh policy read ends in `provider.getState()`, which awaits + // custom-mode file work that cannot be cancelled from here. An abort + // poller aborts a signal the re-check itself awaits, settling it within + // one poll of the abort instead of leaving `ask()` blocked on the pending + // read or a lost race retaining this task. The re-check's own race + // attaches handlers to the read, so it rejecting after the abort settles + // is already considered handled — no extra `.catch` needed. + const recheckAbort = new AbortController() + const checkAbort = () => { + if (this.abort) { + recheckAbort.abort() + } + } + checkAbort() + const abortWatcher = setInterval(checkAbort, 100) + try { + const action = await this.recheckQueuedCommandPolicy( + { + text, + isProtected, + dcgDecision: autoApprovalContext?.dcgDecision, + }, + recheckAbort.signal, + ) + if (!this.abort && this.askResponse === undefined && this.lastMessageTs === askTs) { + queuedMessageId = this.applyQueuedCommandPolicyAction(action, message, resolution) + // A "release" outcome leaves the ask pending while the queue stays + // non-empty, so the arm that `isStatusMutable` gates — computed + // once, before the claim — must run here. + armAskStatusTimers() + } + } finally { + clearInterval(abortWatcher) + // One release path for abort, throw, supersession, and "release". + // `releaseMessage` is idempotent, so it stays safe beside the + // branch-local releases. The durable consume is the one outcome + // that must keep its claim until persistence removes the message — + // it is the only path that assigned `queuedMessageId`. + if (queuedMessageId !== message.id) { + this.messageQueueService.releaseMessage(message.id) + } + } + } - return false - }, - { interval: 100 }, - ) + // Wait for askResponse to be set + try { + await pWaitFor( + () => { + if (this.abort || this.askResponse !== undefined || this.lastMessageTs !== askTs) { + return true + } - /* v8 ignore next 3 -- abort-while-waiting path; covered by e2e standalone-resume test */ - if (this.abort) { - if (queuedMessageId) { - this.messageQueueService.releaseMessage(queuedMessageId) + // If a queued message arrives while we're blocked on an ask (e.g. a follow-up + // suggestion click that was incorrectly queued due to UI state), consume it + // immediately so the task doesn't hang. Command asks under blanket deny are + // excluded (`queueMayAnswerThisAsk`): a queued message must never stand in + // for the explicit approval the policy withheld. + if ( + queueMayAnswerThisAsk && + shouldDrainQueuedMessageForAsk && + !queuedCommandPolicyCheck && + this.mayDrainQueuedMessageForAsk() + ) { + const message = this.messageQueueService.claimNextMessage() + const resolution = message ? queuedResponseForAsk(type, text) : undefined + if (message && resolution) { + if (type === "command") { + // Claim first, then verify the policy off-predicate: a + // blanket-deny flip landing during the prompt dwell is + // invisible to the frozen snapshot gate, so the claim is + // provisional until the fresh check clears it. + queuedCommandPolicyCheck = verifyDrainedCommandMessage(message, resolution).catch( + (error) => { + // The background check must never reject unhandled; + // on failure the claim is released so the message + // stays available to a later consumer. + console.error("[Task#ask] queued command policy re-check failed:", error) + this.messageQueueService.releaseMessage(message.id) + armAskStatusTimers() + }, + ) + } else { + queuedMessageId = this.handleQueuedAskResponse(message, resolution) + } + } + } + + return false + }, + { interval: 100 }, + ) + + // Let a policy re-check that was in flight when the wait resolved run to + // completion: its bail-out path releases the claim, and leaving it + // unresolved would let the consume race the result below. On abort, detach + // instead: the re-check settles on the abort and its `finally` releases the + // claim either way, while awaiting past the abort could stall on an + // uncancellable read instead of letting the throw below settle the ask. + if (queuedCommandPolicyCheck && !this.abort) { + await queuedCommandPolicyCheck } - throw new Error(`[ZooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) - } - if (this.lastMessageTs !== askTs) { - // Could happen if we send multiple asks in a row i.e. with - // command_output. It's important that when we know an ask could - // fail, it is handled gracefully. - if (queuedMessageId) { - this.messageQueueService.releaseMessage(queuedMessageId) + /* v8 ignore next 3 -- abort-while-waiting path; covered by e2e standalone-resume test */ + if (this.abort) { + if (queuedMessageId) { + this.messageQueueService.releaseMessage(queuedMessageId) + } + throw new Error(`[ZooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) } - throw new AskIgnoredError("superseded") - } - const result = { - response: this.askResponse!, - text: this.askResponseText, - images: this.askResponseImages, - queuedMessageId, - } - this.askResponse = undefined - this.askResponseText = undefined - this.askResponseImages = undefined + if (this.lastMessageTs !== askTs) { + // Could happen if we send multiple asks in a row i.e. with + // command_output. It's important that when we know an ask could + // fail, it is handled gracefully. + if (queuedMessageId) { + this.messageQueueService.releaseMessage(queuedMessageId) + } + throw new AskIgnoredError("superseded") + } - // Cancel the timeouts if they are still running. - timeouts.forEach((timeout) => clearTimeout(timeout)) + const result = { + response: this.askResponse!, + text: this.askResponseText, + images: this.askResponseImages, + queuedMessageId, + autoDenyDetail: this.pendingAutoDenyDetail, + } + this.askResponse = undefined + this.askResponseText = undefined + this.askResponseImages = undefined + this.pendingAutoDenyDetail = undefined + + // Switch back to an active state. + if (this.idleAsk || this.resumableAsk || this.interactiveAsk) { + this.idleAsk = undefined + this.resumableAsk = undefined + this.interactiveAsk = undefined + this.emit(RooCodeEventName.TaskActive, this.taskId) + } - // Switch back to an active state. - if (this.idleAsk || this.resumableAsk || this.interactiveAsk) { - this.idleAsk = undefined - this.resumableAsk = undefined - this.interactiveAsk = undefined - this.emit(RooCodeEventName.TaskActive, this.taskId) + this.emit(RooCodeEventName.TaskAskResponded) + return result + } finally { + // Teardown for every settle path: the normal resolve and the + // abort/supersession throws that never reach the result handling + // above. The fire-time guard in the armed callbacks covers the + // sub-tick window where an already-due timer fires before this + // sweep runs. + timeouts.forEach((timeout) => clearTimeout(timeout)) } - - this.emit(RooCodeEventName.TaskAskResponded) - return result } handleWebviewAskResponse(askResponse: ClineAskResponse, text?: string, images?: string[]) { @@ -3242,6 +3713,10 @@ export class Task extends EventEmitter implements TaskLike { // only prevent attempt_completion within the same assistant message, not across turns // (e.g., if a tool fails, then user sends a message saying "just complete anyway") this.didToolFailInCurrentTurn = false + // A blanket command denial only suppresses queued-message approval + // for the turn that earned it: a message the user queues afterward + // (a new turn) is fair game for the next ask to answer. + this.blanketDeniedCommandThisTurn = false this.presentAssistantMessageLocked = false this.presentAssistantMessageHasPendingUpdates = false // No legacy text-stream tool parser. diff --git a/src/core/task/__tests__/ask-auto-deny.spec.ts b/src/core/task/__tests__/ask-auto-deny.spec.ts new file mode 100644 index 0000000000..4254791025 --- /dev/null +++ b/src/core/task/__tests__/ask-auto-deny.spec.ts @@ -0,0 +1,1022 @@ +// npx vitest run core/task/__tests__/ask-auto-deny.spec.ts + +import { type ClineMessage, type ExtensionState, RooCodeEventName } from "@roo-code/types" + +import * as autoApprovalModule from "../../auto-approval" +import { createRateLimitClock } from "../RateLimitClock" +import { Task } from "../Task" + +// The streaming-loop drive below never asserts on environment details; the +// real collector reaches into the VS Code window API, which this file's +// lightweight task stub does not model. +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) + +// Blanket auto-deny (`alwaysDenyUnapprovedCommands`) at the Task level: a +// command ask that policy denies must resolve immediately with the structured +// `autoDenyDetail` (so presentAssistantMessage can distinguish it from a user +// rejection), and the chat row must carry the auto-deny chip +// (`autoApprovalDecision: "deny"` + `isAnswered`). A subsequent ask must never +// see a stale detail from a previous denial. + +/** The parts of the provider that `Task.ask` and the streaming-loop drive reach for. */ +type ProviderStub = { + getState: () => Promise> + postMessageToWebview: ReturnType + postStateToWebviewWithoutTaskHistory: ReturnType + getSkillsManager: () => undefined + cwd: string +} + +function buildTask(provider: ProviderStub, taskCwd: string) { + const task = Object.create(Task.prototype) as Task + task["abort"] = false + task["clineMessages"] = [] + task["askResponse"] = undefined + task["askResponseText"] = undefined + task["askResponseImages"] = undefined + task["lastMessageTs"] = undefined + task["addToClineMessages"] = vi.fn(async () => {}) + task["saveClineMessages"] = vi.fn(async () => true) + task["updateClineMessage"] = vi.fn(async () => {}) + task["cancelAutoApprovalTimeout"] = vi.fn(() => {}) + task["checkpointSave"] = vi.fn(async () => {}) + task["emit"] = vi.fn() + // A double assertion is unavoidable here: `providerRef` is a `WeakRef`, + // and the stub is neither a `WeakRef` nor a whole `ClineProvider`. Constructing + // either would drag in the extension host, when `Task.ask` only ever calls + // `deref()`, `getState()` and `postMessageToWebview()` on it. + task["providerRef"] = { deref: () => provider } as unknown as Task["providerRef"] + Object.defineProperty(task, "workspacePath", { value: taskCwd }) + + return task +} + +async function attachQueue(task: Task) { + const { MessageQueueService } = await import("../../message-queue/MessageQueueService") + const queue = new MessageQueueService() + Object.defineProperty(task, "messageQueueService", { value: queue }) + return queue +} + +/** + * Adds the fields `recursivelyMakeClineRequests` touches before its per-turn + * reset, so a test can drive one assistant turn without the full provider + * harness. The stubbed `attemptApiRequest` streams a single text chunk; + * `abandoned` ends the loop right after the stream via the loop's + * abort/abandoned exit, so the drive stops before any post-stream tool + * presentation and never reaches the retry/backoff paths. + */ +function attachTurnHarness(task: Task) { + task.messageCounts = { user: 0, assistant: 0 } + task.apiConversationHistory = [] + task["abandoned"] = true + Object.defineProperty(task, "api", { + value: { getModel: () => ({ id: "gpt-4.1", info: {} }) }, + }) + Object.defineProperty(task, "apiConfiguration", { value: { apiProvider: undefined } }) + Object.defineProperty(task, "rateLimitClock", { value: createRateLimitClock() }) + Object.defineProperty(task, "diffViewProvider", { value: { isEditing: false, reset: async () => {} } }) + Object.defineProperty(task, "streamingToolCallIndices", { value: new Map() }) + task["saveApiConversationHistory"] = vi.fn(async () => true) + task["say"] = vi.fn(async () => undefined) + // The loop rewrites the newest api_req_started row with cost data; the + // no-op `say` stub above never adds one itself. + task["clineMessages"].push({ type: "say", say: "api_req_started", text: "{}", ts: Date.now() }) + task["attemptApiRequest"] = vi.fn().mockImplementation(() => + (async function* () { + yield { type: "text" as const, text: "next turn reply" } + })(), + ) +} + +const TASK_CWD = "/path/to/task-workspace" + +/** + * Swaps the no-op `addToClineMessages` stub for a recording one, so the 2 s + * status timers' `findMessageByTimestamp(askTs)` lookup finds the pending ask + * row and the interactive emit can actually run. + */ +function recordClineMessages(task: Task) { + const addToClineMessages = vi.fn(async (message: ClineMessage) => { + task["clineMessages"].push(message) + }) + task["addToClineMessages"] = addToClineMessages + return addToClineMessages +} + +/** + * Installs a fresh `emit` recorder (replacing `buildTask`'s no-op stub) and + * returns a counter of this task's `TaskInteractive` emissions. + */ +function installInteractiveEmitRecorder(task: Task) { + const emit = vi.fn() + task["emit"] = emit + return () => emit.mock.calls.filter(([event]) => event === RooCodeEventName.TaskInteractive).length +} + +describe("Task.ask resolves blanket command denials with structured detail", () => { + // Mutable state so a test can flip the policy between consecutive asks on + // the same task (mirrors the live per-ask `provider.getState()` read). + let state: Partial + let provider: ProviderStub + + beforeEach(() => { + state = { + autoApprovalEnabled: true, + alwaysAllowExecute: true, + alwaysDenyUnapprovedCommands: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: false, + } + provider = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + getSkillsManager: () => undefined, + cwd: TASK_CWD, + getState: async () => state, + } + }) + + it("auto-denies an unallowlisted command and stamps the deny chip on the chat row", async () => { + const task = buildTask(provider, TASK_CWD) + await attachQueue(task) + + const result = await task.ask("command", "rm x", false) + + // Policy denial: resolves without user interaction, carrying the reason. + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail).toBeDefined() + expect(result.autoDenyDetail?.kind).toBe("not_allowlisted") + expect(result.autoDenyDetail?.command).toBe("rm x") + + // Chat row: the existing auto-deny chip (answered + deny decision), so no + // approval buttons ever appear. + const addToClineMessages = task["addToClineMessages"] as ReturnType + expect(addToClineMessages).toHaveBeenCalledTimes(1) + const message = addToClineMessages.mock.calls[0][0] + expect(message.type).toBe("ask") + expect(message.ask).toBe("command") + expect(message.isAnswered).toBe(true) + expect(message.autoApprovalDecision).toBe("deny") + }) + + it("a following approved ask does not leak the previous denial's detail", async () => { + const task = buildTask(provider, TASK_CWD) + await attachQueue(task) + + const denied = await task.ask("command", "rm x", false) + expect(denied.autoDenyDetail?.kind).toBe("not_allowlisted") + + // Approve the next command via the allowlist: the denial detail from the + // previous ask must not ride along into this result. + state.allowedCommands = ["git"] + const approved = await task.ask("command", "git status", false) + + expect(approved.response).toBe("yesButtonClicked") + expect(approved.autoDenyDetail).toBeUndefined() + + const addToClineMessages = task["addToClineMessages"] as ReturnType + expect(addToClineMessages).toHaveBeenCalledTimes(2) + expect(addToClineMessages.mock.calls[1][0].autoApprovalDecision).toBe("approve") + }) + + it("carries a denylist denial's detail even with the blanket setting off", async () => { + // Denylist denials were never user rejections: they carry structured + // detail regardless of the blanket flag (unified vocabulary). + state.alwaysDenyUnapprovedCommands = false + state.deniedCommands = ["rm"] + + const task = buildTask(provider, TASK_CWD) + await attachQueue(task) + + const result = await task.ask("command", "rm -rf build", false) + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail?.kind).toBe("denylist") + expect(result.autoDenyDetail?.command).toBe("rm -rf build") + expect(result.autoDenyDetail?.pattern).toBe("rm") + + const addToClineMessages = task["addToClineMessages"] as ReturnType + expect(addToClineMessages.mock.calls[0][0].autoApprovalDecision).toBe("deny") + }) + + it("routes a forwarded DCG verdict into the policy decision", async () => { + // The seam: Task.ask must hand the tool-supplied verdict to + // checkAutoApproval. Without the forwarding this DCG-enabled ask + // (verdict-less from checkAutoApproval's view) approves instead of + // carrying the guard's structured denial. + state.destructiveCommandGuardEnabled = true + const task = buildTask(provider, TASK_CWD) + await attachQueue(task) + + const result = await task.ask("command", "rm x", false, undefined, false, { + dcgDecision: { decision: "deny", reason: "matches a destructive pattern" }, + }) + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail?.kind).toBe("dcg") + expect(result.autoDenyDetail?.command).toBe("rm x") + expect(result.autoDenyDetail?.dcgReason).toBe("matches a destructive pattern") + }) + + it("flips to the retryable guard-state deny mid-session and heals once the retried ask carries a verdict", async () => { + // Simulates the guard setting flipping on between consecutive asks at + // the decision-bearing boundary (Task.ask reads provider state per ask, + // then checkAutoApproval sees no verdict for the newly-enabled guard). + // The sub-microsecond tool-vs-setting window itself is only reachable + // end-to-end; what is provable here is that the verdictless arrival + // denies with the retryable detail and that a re-issue carrying a + // verdict is approved again. + const task = buildTask(provider, TASK_CWD) + await attachQueue(task) + + // ask 1: guard off — the ordinary blanket path. + const before = await task.ask("command", "echo hi", false) + expect(before.response).toBe("noButtonClicked") + expect(before.autoDenyDetail?.kind).toBe("not_allowlisted") + + // The flip: the guard setting turns on, but this ask arrives without a + // verdict — an inconsistent guard state, denied retryably, not approved. + state.destructiveCommandGuardEnabled = true + const flipped = await task.ask("command", "rm x", false) + expect(flipped.response).toBe("noButtonClicked") + expect(flipped.autoDenyDetail?.kind).toBe("guard_unavailable") + expect(flipped.autoDenyDetail?.command).toBe("rm x") + + // The retry heals: the re-issued ask carries the guard's allow verdict + // and approves. + const healed = await task.ask("command", "rm x", false, undefined, false, { + dcgDecision: { decision: "allow" }, + }) + expect(healed.response).toBe("yesButtonClicked") + expect(healed.autoDenyDetail).toBeUndefined() + }) +}) + +describe("Task.ask queue path cannot bypass blanket deny", () => { + let state: Partial + let provider: ProviderStub + + beforeEach(() => { + state = { + autoApprovalEnabled: true, + alwaysAllowExecute: true, + alwaysDenyUnapprovedCommands: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: false, + } + provider = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + getSkillsManager: () => undefined, + cwd: TASK_CWD, + getState: async () => state, + } + }) + + it("denies a blanket-denied command ask even when a queued message would auto-approve it", async () => { + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + // A queued message answers command asks with an unconditional + // yesButtonClicked — exactly the shortcut that must never stand in for + // approval while blanket deny is engaged. The policy denial must win, + // and it must carry the same structured detail as the main path. + queue.addMessage("queued feedback arriving while blanket deny is engaged") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail).toBeDefined() + expect(result.autoDenyDetail?.kind).toBe("not_allowlisted") + expect(result.autoDenyDetail?.command).toBe("rm x") + // The queued message was not consumed as a fake approval: it stays in the + // queue for a later conversational turn. + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(1) + }) + + it("still lets a queued message answer a command ask when blanket deny is off", async () => { + // Behavior unchanged while the blanket configuration is disengaged: the + // queued-message auto-approval shortcut keeps working. + state.alwaysDenyUnapprovedCommands = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued feedback with blanket deny off") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("yesButtonClicked") + expect(result.autoDenyDetail).toBeUndefined() + // Non-durable resolution consumed the queued message. + expect(queue.messages).toHaveLength(0) + }) + + it("keeps the queue gate open (queued message answers the command ask) while autoApprovalEnabled is false", async () => { + // The blanket gate is a conjunction of three settings; each false member + // alone must disengage it, so the queued shortcut stays live. + state.autoApprovalEnabled = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued feedback with the conjunction incomplete") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBe("queued feedback with the conjunction incomplete") + expect(queue.messages).toHaveLength(0) + }) + + it("keeps the queue gate open (queued message answers the command ask) while alwaysAllowExecute is false", async () => { + state.alwaysAllowExecute = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued feedback with the conjunction incomplete") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBe("queued feedback with the conjunction incomplete") + expect(queue.messages).toHaveLength(0) + }) + + it("denies the command ask when blanket deny engages between the snapshot and the immediate queued consume", async () => { + // The ask-time snapshot must not be the last word: the queued shortcut + // skips checkAutoApproval, so a blanket-deny engagement landing after the + // snapshot is invisible to it and would auto-approve an unallowlisted + // command. Interleaving here: first getState = snapshot (deny OFF, so the + // message is claimed); second getState = the consume-site re-check, at + // which the settings save has landed (deny ON). + state.alwaysDenyUnapprovedCommands = false + let getStateCalls = 0 + provider.getState = async () => { + getStateCalls++ + if (getStateCalls >= 2) { + state.alwaysDenyUnapprovedCommands = true + } + return state + } + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued feedback arriving during the flip window") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail?.kind).toBe("not_allowlisted") + // The claimed message was released, not consumed as a fake approval. + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("the user's Deny during the immediate re-check await is not overwritten", async () => { + // The Deny lands while the fresh policy read is pending (queued behind + // the backlog). The re-check itself still resolves `consume` — applying + // that outcome would answer the ask with the queued message's + // yesButtonClicked over the user's decision. + state.alwaysDenyUnapprovedCommands = false + const task = buildTask(provider, TASK_CWD) + let getStateCalls = 0 + provider.getState = async () => { + getStateCalls++ + if (getStateCalls === 2) { + task.handleWebviewAskResponse("noButtonClicked") + } + return state + } + const queue = await attachQueue(task) + queue.addMessage("queued feedback arriving behind the user's Deny") + + const result = await task.ask("command", "rm x", false) + + expect(result.response).toBe("noButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + // The claim was released, not consumed: the message survives for the + // next consumer. + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("a throwing re-check at the immediate site releases the claim and leaves the prompt pending", async () => { + // A rejected policy read must neither reject ask() nor strand the claim: + // the prompt stays pending for the user and the message survives for a + // later consumer. + state.alwaysDenyUnapprovedCommands = false + let getStateCalls = 0 + // Sticky: every read after the snapshot fails, so the drain site's + // re-check also fails and the released claim survives for the assertion + // poll instead of being re-claimed and legitimately consumed. + provider.getState = async () => { + getStateCalls++ + if (getStateCalls >= 2) { + throw new Error("policy read failed") + } + return state + } + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued feedback with a failing policy read") + const addToClineMessages = task["addToClineMessages"] as ReturnType + + const askPromise = task.ask("command", "rm x", false) + // Past the prompt post, into the re-check await; then poll for the + // catch-arm's claim release (no fixed sleep). + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(queue.hasUnclaimed()).toBe(true)) + + task.approveAsk() + const result = await askPromise + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(1) + }) + + it("denies the command ask when blanket deny engages during the prompt dwell and the drain claims a message", async () => { + // The seconds-wide fail-open window: the flip and the queue arrival both + // land during the pWaitFor dwell, after the frozen snapshot gate opened. + state.alwaysDenyUnapprovedCommands = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + const addToClineMessages = task["addToClineMessages"] as ReturnType + + const askPromise = task.ask("command", "rm x", false) + // Past the snapshot read, into the prompt dwell. + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(1)) + state.alwaysDenyUnapprovedCommands = true + queue.addMessage("queued during the dwell") + + const result = await askPromise + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail?.kind).toBe("not_allowlisted") + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("the user's Deny landing while the drain-site re-check awaits is not overwritten", async () => { + // Distinct from the immediate-site supersede case: the message arrives + // during the prompt dwell, so the claim and the policy re-check run + // inside the pWaitFor predicate's drain closure. The Deny lands while + // that re-check awaits; without the closure's bail-out guard the drained + // message would answer the ask over the user's decision. + state.alwaysDenyUnapprovedCommands = false + const task = buildTask(provider, TASK_CWD) + let getStateCalls = 0 + provider.getState = async () => { + getStateCalls++ + if (getStateCalls === 2) { + // The Deny lands while the drain closure's fresh policy read is + // pending (first call is the ask's snapshot). + task.handleWebviewAskResponse("noButtonClicked") + } + return state + } + const queue = await attachQueue(task) + const addToClineMessages = task["addToClineMessages"] as ReturnType + + const askPromise = task.ask("command", "rm x", false) + // Past the snapshot read, into the dwell; the message arriving now + // forces the drain site rather than the immediate consume path. + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(1)) + queue.addMessage("queued during the dwell, superseded by the user's Deny") + + const result = await askPromise + + expect(result.response).toBe("noButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + // The provisional claim was released, not consumed: the message + // survives for the next consumer. + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("latches the turn when the denial fires at the drain site and the latch blocks the follow-up ask", async () => { + // The drain-site denial takes the same latch path as the immediate site + // (the deny branch of `applyQueuedCommandPolicyAction`); the follow-up + // ask proves the latch — not just this ask's denial result — keeps + // queue messages from standing in as approval for the rest of the turn. + state.alwaysDenyUnapprovedCommands = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + const addToClineMessages = task["addToClineMessages"] as ReturnType + + const askPromise = task.ask("command", "rm x", false) + // Past the snapshot read, into the prompt dwell: the flip and the + // message arrival both land during the dwell, so the denial is produced + // by the drain site's re-check rather than the frozen snapshot gate. + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(1)) + state.alwaysDenyUnapprovedCommands = true + queue.addMessage("queued during the dwell") + + const denied = await askPromise + expect(denied.response).toBe("noButtonClicked") + expect(denied.autoDenyDetail?.kind).toBe("not_allowlisted") + // The latch set is the drain-site path's, not the main ask path's: the + // snapshot saw the blanket setting off, so `checkAutoApproval` asked. + expect(task["blanketDeniedCommandThisTurn"]).toBe(true) + + // Same turn: with the latch set, `mayDrainQueuedMessageForAsk` gates the + // claim itself at both consume sites, so the follow-up ask never claims + // either queued message — no message may answer it as approval. + let settled: Awaited> | undefined + const toolAsk = task + .ask("tool", JSON.stringify({ tool: "write_to_file", path: "a.txt" }), false) + .then((result) => { + settled = result + return result + }) + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(2)) + queue.addMessage("second message during the tool-ask dwell") + // Poll with a deadline for the failure mode (ask self-resolving via the + // queue); a correctly latched ask stays pending for the user. + await vi.waitFor(() => expect(settled).toBeDefined(), { timeout: 350, interval: 25 }).catch(() => undefined) + expect(settled).toBeUndefined() + expect(queue.messages).toHaveLength(2) + + // The user answers the prompt themselves; neither queued message rides along. + task.approveAsk() + const result = await toolAsk + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(2) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("a tool ask after a blanket-denied command leaves both queued messages for the user instead of self-approving", async () => { + // A message left in the queue by a blanket denial answers that denial, not + // whatever ask runs next in the same turn; consuming it as + // yesButtonClicked would silently approve (and suppress the prompt for) + // the next ask. + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + const addToClineMessages = task["addToClineMessages"] as ReturnType + queue.addMessage("feedback on the denied command") + + const denied = await task.ask("command", "rm x", false) + expect(denied.response).toBe("noButtonClicked") + expect(task["blanketDeniedCommandThisTurn"]).toBe(true) + + // Same turn: a non-command ask may claim the surviving message; the latch + // must stop it from being consumed as approval. + let settled: Awaited> | undefined + const toolAsk = task + .ask("tool", JSON.stringify({ tool: "write_to_file", path: "a.txt" }), false) + .then((result) => { + settled = result + return result + }) + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(2)) + // A second message arriving during the dwell exercises the drain site's + // latch guard, not just the immediate-consume guard. + queue.addMessage("second message during the tool-ask dwell") + // Poll with a deadline for the failure mode (ask self-resolving via the + // queue); a correctly latched ask stays pending for the user. + await vi.waitFor(() => expect(settled).toBeDefined(), { timeout: 350, interval: 25 }).catch(() => undefined) + expect(settled).toBeUndefined() + expect(queue.messages).toHaveLength(2) + + // The user answers the prompt themselves; neither queued message rides along. + task.approveAsk() + const result = await toolAsk + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(2) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("arms the interactive status timer when the latch gate leaves the ask pending", async () => { + // A gated claim keeps the queue non-empty, so `isStatusMutable` is false + // for the whole dwell and the 2 s interactive arm is skipped unless the + // gate branch arms it: without that, hands-free/API consumers see + // `Running` with no `TaskInteractive` for a prompt that is waiting on + // the user. + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + recordClineMessages(task) + const interactive = installInteractiveEmitRecorder(task) + queue.addMessage("feedback on the denied command") + + const denied = await task.ask("command", "rm x", false) + expect(denied.response).toBe("noButtonClicked") + // The denial resolved on its own: no interactive state was ever entered. + expect(interactive()).toBe(0) + + const toolAsk = task.ask("tool", JSON.stringify({ tool: "write_to_file", path: "a.txt" }), false) + await vi.waitFor(() => expect(interactive()).toBe(1), { timeout: 6_000 }) + expect(provider.postMessageToWebview).toHaveBeenCalledWith({ type: "interactionRequired" }) + + task.approveAsk() + const result = await toolAsk + expect(result.response).toBe("yesButtonClicked") + expect(queue.messages).toHaveLength(1) + }) + + it("arms the interactive status timer when a throwing re-check releases the prompt pending", async () => { + // A rejected policy read leaves the prompt pending for the user, so the + // catch's claim release must arm the 2 s interactive timer: with the + // queue non-empty, `isStatusMutable` — computed once, before the claim — never armed. + state.alwaysDenyUnapprovedCommands = false + let getStateCalls = 0 + provider.getState = async () => { + getStateCalls++ + if (getStateCalls >= 2) { + throw new Error("policy read failed") + } + return state + } + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + recordClineMessages(task) + const interactive = installInteractiveEmitRecorder(task) + queue.addMessage("queued feedback with a failing policy read") + + const askPromise = task.ask("command", "rm x", false) + await vi.waitFor(() => expect(queue.hasUnclaimed()).toBe(true)) + await vi.waitFor(() => expect(interactive()).toBe(1), { timeout: 6_000 }) + + task.approveAsk() + const result = await askPromise + expect(result.response).toBe("yesButtonClicked") + expect(queue.messages).toHaveLength(1) + }) + + it("emits exactly one TaskInteractive when back-to-back policy releases leave the prompt pending", async () => { + // Two release branches run for one prompt: the immediate-site fresh + // read answers `release` (blanket engages at the re-read, and the + // protected prompt survives it), which arms the interactive timer even + // though the non-empty queue skipped the `isStatusMutable`-gated arm + // (computed once, before the claim); the predicate then re-claims the + // released message and the drain + // release re-arms. The idempotence guard must make that second (and + // every later) arm a no-op: without it, one prompt emits + // `TaskInteractive`/`interactionRequired` more than once. + state.alwaysDenyUnapprovedCommands = false + let getStateCalls = 0 + provider.getState = async () => { + getStateCalls++ + if (getStateCalls === 2) { + // The flip lands at the immediate-site fresh read: the frozen + // snapshot let the message be claimed, the fresh check must now + // neither consume it nor let it answer the ask. + state.alwaysDenyUnapprovedCommands = true + } + return state + } + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + recordClineMessages(task) + const interactive = installInteractiveEmitRecorder(task) + queue.addMessage("queued feedback facing the engaged blanket deny") + + const askPromise = task.ask("command", "some-unknown-command", false, undefined, true) + + const armedAt = Date.now() + await vi.waitFor(() => expect(interactive()).toBe(1), { timeout: 6_000 }) + // Wait past the window in which a second (drain-site) arm would also + // fire, then assert the total: the first timer can show up alone + // briefly before a hypothetical second one, so the count is only + // meaningful after both would have expired. + await vi.waitFor(() => expect(Date.now() - armedAt).toBeGreaterThan(2_600), { timeout: 5_000 }) + expect(interactive()).toBe(1) + expect( + provider.postMessageToWebview.mock.calls.filter(([message]) => message?.type === "interactionRequired"), + ).toHaveLength(1) + + task.approveAsk() + const result = await askPromise + expect(result.response).toBe("yesButtonClicked") + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("settles ask() and releases the claim when the task aborts while the drain re-check is pending", async () => { + // The drain's fresh policy read ends in an uncancellable + // `provider.getState()`. With that read pending, an abort must not + // leave `ask()` riding it: the abort race settles the re-check, the + // re-check's finally releases the claim, and `ask()` rejects at the + // post-abort throw. + state.alwaysDenyUnapprovedCommands = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + const addToClineMessages = recordClineMessages(task) + + let getStateCalls = 0 + provider.getState = () => { + getStateCalls++ + if (getStateCalls >= 2) { + // The drain-site re-check never settles: nothing resolves it, + // only the abort race ends the ask's dependence on it. + return new Promise>(() => {}) + } + return Promise.resolve(state) + } + + const setIntervalSpy = vi.spyOn(globalThis, "setInterval") + const clearIntervalSpy = vi.spyOn(globalThis, "clearInterval") + + const askPromise = task.ask("command", "rm x", false) + await vi.waitFor(() => expect(addToClineMessages).toHaveBeenCalledTimes(1)) + // The message arriving during the dwell routes the re-check through the + // drain site, where the provisional claim and the pending read live. + queue.addMessage("queued during the dwell") + await vi.waitFor(() => expect(getStateCalls).toBe(2)) + expect(queue.hasUnclaimed()).toBe(false) + + task["abort"] = true + + // `ask()` must reject on abort; racing a deadline distinguishes a wrong + // settle from a hang on the pending read. + const outcome = await Promise.race([ + askPromise.then( + () => "resolved", + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ), + new Promise((resolve) => setTimeout(() => resolve("deadline-exceeded"), 3_000)), + ]) + expect(outcome).toContain("aborted") + + // The claim releases from the re-check's finally once the abort race + // settles it, so a later consumer can take the message. + await vi.waitFor(() => expect(queue.hasUnclaimed()).toBe(true)) + + // The abort watcher must not keep polling past the settle. + const intervals = setIntervalSpy.mock.results.map((result) => result.value) + expect(intervals.length).toBeGreaterThan(0) + for (const interval of intervals) { + expect(clearIntervalSpy.mock.calls.some(([cleared]) => cleared === interval)).toBe(true) + } + setIntervalSpy.mockRestore() + clearIntervalSpy.mockRestore() + }) + + it("the next turn's tool ask consumes the queued message once the latch reset clears it", async () => { + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + attachTurnHarness(task) + queue.addMessage("queued feedback") + + const denied = await task.ask("command", "rm x", false) + expect(denied.response).toBe("noButtonClicked") + expect(task["blanketDeniedCommandThisTurn"]).toBe(true) + + // The only production clear is the per-turn reset inside the streaming + // loop (beside didToolFailInCurrentTurn), so the latch is released by + // driving a real assistant turn — a hand-set flag would not pin it. + await task.recursivelyMakeClineRequests([{ type: "text", text: "next turn" }], false) + expect(task["blanketDeniedCommandThisTurn"]).toBe(false) + + const result = await task.ask("tool", JSON.stringify({ tool: "readFile", path: "a.txt" }), false) + + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBe("queued feedback") + expect(queue.messages).toHaveLength(0) + }) + + it("does not emit TaskInteractive or retain the armed timer when the task aborts during the 2 s window", async () => { + // A status timer armed for 2 s can outlive an abort that lands inside + // its window: the ask then throws without ever reaching the response + // handling, and a live webview would still receive + // `interactionRequired` for a task that no longer runs. Two independent + // defenses are pinned: the callbacks' fire-time liveness check, exercised + // by invoking the captured callback after the rejection (the sub-tick + // race where an already-due timer fires before the teardown sweep cannot + // be interleaved deterministically on the real clock), and the + // `finally`-sweep, exercised by the clearTimeout handle inventory. + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + recordClineMessages(task) + const interactive = installInteractiveEmitRecorder(task) + queue.addMessage("feedback on the denied command") + + const denied = await task.ask("command", "rm x", false) + expect(denied.response).toBe("noButtonClicked") + + const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout") + const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout") + + const toolAsk = task.ask("tool", JSON.stringify({ tool: "write_to_file", path: "a.txt" }), false) + // The latch-release arm is the only >= 2 s timer this ask can create + // (the wait's own poller schedules 100 ms handles), so the exact-delay + // filter isolates the status-timer handle without fake timers. + const armedArms = () => + setTimeoutSpy.mock.calls + .map(([callback, delay], i) => ({ callback, delay, handle: setTimeoutSpy.mock.results[i].value })) + .filter((entry) => entry.delay === 2_000) + await vi.waitFor(() => expect(armedArms().length).toBe(1), { timeout: 6_000 }) + const armedAt = Date.now() + + // Abort while the window is still open, shortly after the arm: the + // longer this wait runs, the less event-loop-stall slack is left before + // the 2 s timer could legitimately fire while the task is still live. + await vi.waitFor(() => expect(Date.now() - armedAt).toBeGreaterThanOrEqual(300), { timeout: 2_000 }) + task["abort"] = true + + const outcome = await toolAsk.then( + () => "resolved", + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ) + expect(outcome).toContain("aborted") + + // Fire-time guard pin: the captured armed callback, invoked synchronously + // after the rejection, must decline on its own — clearing covers only + // timers that had not fired yet when the sweep ran. + const [armed] = armedArms() + armed.callback() + expect(interactive()).toBe(0) + expect( + provider.postMessageToWebview.mock.calls.filter(([message]) => message?.type === "interactionRequired"), + ).toHaveLength(0) + + // Behavior pin: past the window the timer would have needed, nothing + // fires on its own either. + await vi.waitFor(() => expect(Date.now() - armedAt).toBeGreaterThan(2_500), { timeout: 6_000 }) + expect(interactive()).toBe(0) + expect( + provider.postMessageToWebview.mock.calls.filter(([message]) => message?.type === "interactionRequired"), + ).toHaveLength(0) + + // Ledger pin: every status-timer handle this ask armed was cleared, on + // the throw path included. + const clearedHandles = clearTimeoutSpy.mock.calls.map(([cleared]) => cleared) + for (const { handle } of armedArms()) { + expect(clearedHandles).toContain(handle) + } + + setTimeoutSpy.mockRestore() + clearTimeoutSpy.mockRestore() + }) + + it("auto-approves a policy-approved tool ask while the latch leaves the queue for later", async () => { + // The latch is a reason not to claim, not a reason to claim-and-release: + // a claim forces the `ask` decision before the policy runs, so a + // `read_file` that `alwaysAllowReadOnly` auto-approves would sit waiting + // for a user who has nothing to decide, stalling a hands-free session. + // The policy must decide as if the queue were empty, and the message the + // denial left behind must stay unclaimed for a later turn. + state.alwaysAllowReadOnly = true + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("feedback on the denied command") + + const denied = await task.ask("command", "rm x", false) + expect(denied.response).toBe("noButtonClicked") + expect(task["blanketDeniedCommandThisTurn"]).toBe(true) + + // Race a deadline against the ask: with a claim-forced prompt the ask + // stays pending forever, and a hang must read as a failure, not a pass. + const outcome = await Promise.race([ + task.ask("tool", JSON.stringify({ tool: "readFile", path: "a.txt" }), false), + new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 1_500)), + ]) + expect(outcome).not.toBe("pending") + const result = outcome as Awaited> + // Approved through policy, not through the queued message: the result + // carries no feedback text, and the message survives unclaimed. + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect(result.queuedMessageId).toBeUndefined() + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + }) + + it("settles ask() and releases the claim when the task aborts while the immediate re-check is pending", async () => { + // The immediate site's fresh policy read ends in the same uncancellable + // `provider.getState()` as the drain site's. With the claim held and the + // read pending before the wait even starts, an abort must settle the + // re-check through the abort race, release the claim from its `finally`, + // tear the watcher down, and reject `ask()`. + state.alwaysDenyUnapprovedCommands = false + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued before the ask begins") + + let getStateCalls = 0 + provider.getState = () => { + getStateCalls++ + if (getStateCalls >= 2) { + // The immediate-site re-check never settles: nothing resolves it, + // only the abort race ends the ask's dependence on it. + return new Promise>(() => {}) + } + return Promise.resolve(state) + } + + const setIntervalSpy = vi.spyOn(globalThis, "setInterval") + const clearIntervalSpy = vi.spyOn(globalThis, "clearInterval") + + const askPromise = task.ask("command", "rm x", false) + await vi.waitFor(() => expect(getStateCalls).toBe(2)) + expect(queue.hasUnclaimed()).toBe(false) + + task["abort"] = true + + // `ask()` must reject on abort; racing a deadline distinguishes a wrong + // settle from a hang on the pending read. + const outcome = await Promise.race([ + askPromise.then( + () => "resolved", + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ), + new Promise((resolve) => setTimeout(() => resolve("deadline-exceeded"), 3_000)), + ]) + expect(outcome).toContain("aborted") + + // The claim releases from the re-check's finally once the abort race + // settles it, so a later consumer can take the message. + await vi.waitFor(() => expect(queue.hasUnclaimed()).toBe(true)) + + // The abort watcher must not keep polling past the settle. + const intervals = setIntervalSpy.mock.results.map((result) => result.value) + expect(intervals.length).toBeGreaterThan(0) + for (const interval of intervals) { + expect(clearIntervalSpy.mock.calls.some(([cleared]) => cleared === interval)).toBe(true) + } + setIntervalSpy.mockRestore() + clearIntervalSpy.mockRestore() + }) + + it("never consults the policy when the fresh-state read lands after the abort", async () => { + // An abort must settle the re-check itself, not merely win a race against + // it: a re-check left pending on the uncancellable read retains the task + // and, when the read finally lands with the blanket deny engaged, runs + // `checkAutoApproval` post-abort. Here the read resolves only after the + // abort, with deny now ON — the settled re-check must not take it + // through the policy, and the claimed message must stay unconsumed. + state.alwaysDenyUnapprovedCommands = false + const checkAutoApprovalSpy = vi.spyOn(autoApprovalModule, "checkAutoApproval") + + let resolveLateState: (() => void) | undefined + let getStateCalls = 0 + provider.getState = () => { + getStateCalls++ + if (getStateCalls >= 2) { + // The immediate-site re-check's fresh read stays pending until the + // test releases it, past the abort, with the policy engaged. + return new Promise>((resolve) => { + resolveLateState = () => { + state.alwaysDenyUnapprovedCommands = true + resolve(state) + } + }) + } + return Promise.resolve(state) + } + + const task = buildTask(provider, TASK_CWD) + const queue = await attachQueue(task) + queue.addMessage("queued before the ask begins") + + const askPromise = task.ask("command", "rm x", false) + await vi.waitFor(() => expect(getStateCalls).toBe(2)) + expect(queue.hasUnclaimed()).toBe(false) + + task["abort"] = true + + // `ask()` must reject on abort; racing a deadline distinguishes a wrong + // settle from a hang on the pending read. + const outcome = await Promise.race([ + askPromise.then( + () => "resolved", + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ), + new Promise((resolve) => setTimeout(() => resolve("deadline-exceeded"), 3_000)), + ]) + expect(outcome).toContain("aborted") + + // The uncancellable read lands only now — the settled re-check must have + // detached from it, so its continuation never reaches the policy. + resolveLateState!() + await vi.waitFor(() => expect(queue.hasUnclaimed()).toBe(true)) + // Past a full abort-watcher poll: enough for any retained continuation + // of the late read to have run and been caught by the spy. + await new Promise((resolve) => setTimeout(resolve, 150)) + expect(checkAutoApprovalSpy).not.toHaveBeenCalled() + // No post-abort consume: the message survives, unclaimed, for a later + // consumer. + expect(queue.messages).toHaveLength(1) + expect(queue.hasUnclaimed()).toBe(true) + checkAutoApprovalSpy.mockRestore() + }) +}) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 8383d9a4e1..a6034e4f4d 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -7,9 +7,11 @@ import delay from "delay" import { CommandExecutionStatus, DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, PersistedCommandOutput } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" -import { Task } from "../task/Task" +import { buildAutoDenyReason, checkAutoApproval } from "../auto-approval" +import { BLANKET_DENY_AUTO_DENY_KINDS, isBlanketDenyEngaged, Task } from "../task/Task" import type { ClineProvider } from "../webview/ClineProvider" +import type { DcgDecision } from "../../services/destructive-command-guard" import { ToolUse, ToolResponse } from "../../shared/tools" import { formatResponse } from "../prompts/responses" import { unescapeHtmlEntities } from "../../utils/text-normalization" @@ -129,7 +131,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { } const provider = await task.providerRef.deref() - let dcgBlocked = false + let dcgDecision: DcgDecision | undefined if (provider?.contextProxy.getValue("destructiveCommandGuardEnabled") === true) { const { ensureDcgInstalled, runDcg } = await import("../../services/destructive-command-guard") // Resolve through the managed installer on use so an extension update @@ -143,27 +145,132 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { ? customCwd : path.resolve(task.cwd, customCwd) : task.cwd - const dcgResult = await runDcg(binaryPath, canonicalCommand, workingDirectory) - dcgBlocked = dcgResult.decision === "deny" - if (dcgResult.decision === "deny") { - await task.say("error", formatDcgBlockedMessage(dcgResult.reason, dcgResult.ruleId)) + // Infra failures (spawn/parse/timeout) reject here and surface as a + // retryable tool_error via the outer catch — never as a policy denial. + dcgDecision = await runDcg(binaryPath, canonicalCommand, workingDirectory) + if (dcgDecision.decision === "deny") { + await task.say("error", formatDcgBlockedMessage(dcgDecision.reason, dcgDecision.ruleId)) } } - // DCG-approved commands are auto-approved by checkAutoApproval. A DCG - // block is presented as Zoo's normal command prompt, with isProtected - // forcing the user to explicitly choose whether to execute it. - const didApprove = dcgBlocked - ? await askApproval("command", canonicalCommand, undefined, true) - : await askApproval("command", canonicalCommand) + // The blanket auto-deny setting only engages while command auto-approval + // is on. A DCG block keeps its protected user prompt unless the blanket + // setting is fully engaged, in which case it is auto-denied. This + // snapshot governs only how the ask is presented; after approval, the + // engagement, the command policy, and terminal behavior are re-derived + // from a fresh read (below), so a settings flip during a pending prompt + // takes effect — engaging blanket deny denies the command instead of + // executing it. A blanket off-flip landing between this snapshot and + // Task.ask's own re-read routes a DCG block to the normal prompt instead + // of the protected one; a user still decides either way, so the snapshot + // stays. + const providerState = await provider?.getState() + const blanketAutoDeny = isBlanketDenyEngaged(providerState) + + // Outside blanket mode a DCG block is presented as the protected + // prompt. This flag governs prompt presentation only: protection is a + // property of the policy that produced the prompt, so the execute-time + // re-check re-derives it from the fresh state rather than reusing this + // value (below). + const isProtectedAsk = dcgDecision !== undefined && dcgDecision.decision === "deny" && !blanketAutoDeny + + // DCG-approved commands are auto-approved by checkAutoApproval (from the + // passed verdict). A DCG block is either auto-denied with the guard's + // reason delivered to the model (blanket mode), or presented as Zoo's + // normal command prompt, with isProtected forcing the user to explicitly + // choose whether to execute it. + let didApprove: boolean + if (dcgDecision === undefined) { + didApprove = await askApproval("command", canonicalCommand) + } else if (dcgDecision.decision === "allow") { + didApprove = await askApproval("command", canonicalCommand, undefined, false, { dcgDecision }) + } else if (blanketAutoDeny) { + didApprove = await askApproval("command", canonicalCommand, undefined, false, { dcgDecision }) + } else { + didApprove = await askApproval("command", canonicalCommand, undefined, isProtectedAsk) + } if (!didApprove) { return } const executionId = task.lastMessageTs?.toString() ?? Date.now().toString() - const providerState = await provider?.getState() - const { terminalShellIntegrationDisabled = true } = providerState ?? {} + // Re-read after approval so a settings flip while the approval prompt + // was pending is honored for terminal behavior and policy. + const freshState = await provider?.getState() + const { terminalShellIntegrationDisabled = true } = freshState ?? {} + + if (isBlanketDenyEngaged(freshState)) { + // Execute-time re-validate: the approval rode on the pre-ask + // snapshot, so a blanket-deny engagement that landed between the + // approval and this point would otherwise execute a command the + // fresh policy denies. Parity call with the drain-site re-check, + // forwarding the verdict already computed above — re-running the + // guard would respawn the process for no new information. A fresh + // policy that still approves executes normally. Protection is + // re-derived from the fresh state, not latched from the prompt: + // `isProtected` short-circuits the command policy to an ask before + // any denial is evaluated, so forwarding the ask-time flag would + // let a DCG block approved during the dwell execute even though + // the blanket setting is now on. Inside this branch blanket deny + // is engaged, so the protection rule (`DCG block && blanket off`) + // cannot hold under the current policy — the fresh derivation is + // `false`, and the blanket denial is evaluated. Routing keeps the + // ask-path distinction: `guard_unavailable` marks a guard-state + // inconsistency (retryable error, not a policy denial, and no + // latch); blanket kinds latch the turn so a queue message left by + // this denial cannot be consumed as approval by a later ask. + // Consulting the latch instead of the policy would deny commands + // whose own approval was legal, inverting approval semantics. + const recheck = await checkAutoApproval({ + state: freshState, + cwd: task.cwd, + ask: "command", + text: canonicalCommand, + isProtected: false, + dcgDecision, + }) + if (recheck.decision === "deny") { + const detail = recheck.autoDeny + // Fail closed: a deny without structured detail (permitted by the + // result type, produced by no command-policy branch today) must + // still block execution — it rides the retryable-error channel + // like `guard_unavailable` instead of falling through to the terminal. + if (!detail || detail.kind === "guard_unavailable") { + pushToolResult( + formatResponse.toolError( + detail + ? buildAutoDenyReason(detail) + : `Command \`${canonicalCommand}\` was not executed: the command policy denied it without a reason. You may retry the same command.`, + ), + ) + return + } + if (BLANKET_DENY_AUTO_DENY_KINDS.has(detail.kind)) { + task.recordBlanketCommandDenial() + } + pushToolResult( + formatResponse.toolAutoDenied({ + reason: buildAutoDenyReason(detail), + offendingCommand: detail.command, + ruleId: detail.dcgRuleId, + }), + ) + return + } else if (recheck.decision !== "approve") { + // Fail closed on every non-approval, not just `deny`: with + // blanket engaged the command policy should never answer + // `ask`/`timeout` here, but the result union allows it, and an + // unexpected non-approval must not reach the terminal. Retry + // is safe — nothing ran and nothing latched. + pushToolResult( + formatResponse.toolError( + `Command \`${canonicalCommand}\` was not executed: the fresh command policy re-check returned "${recheck.decision}" instead of an approval. You may retry the same command.`, + ), + ) + return + } + } // Get command execution timeout from VSCode configuration (in seconds) const commandExecutionTimeoutSeconds = vscode.workspace diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index a856b180ca..2797897f46 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -4,6 +4,7 @@ import type { ToolUsage } from "@roo-code/types" import * as vscode from "vscode" import { Task } from "../../task/Task" +import * as autoApprovalModule from "../../auto-approval" import { formatResponse } from "../../prompts/responses" import { ToolUse, AskApproval, HandleError, PushToolResult } from "../../../shared/tools" import { unescapeHtmlEntities } from "../../../utils/text-normalization" @@ -26,6 +27,11 @@ vitest.mock("vscode", () => ({ workspace: { getConfiguration: vitest.fn(), }, + // The Task module's real blanket-policy helpers load the editor decoration + // controller, which builds a decoration type at import time. + window: { + createTextEditorDecorationType: vitest.fn().mockReturnValue({ dispose: vitest.fn() }), + }, })) vitest.mock("../../../integrations/terminal/TerminalRegistry", () => ({ @@ -43,7 +49,13 @@ vitest.mock("../../../integrations/terminal/TerminalRegistry", () => ({ }, })) -vitest.mock("../../task/Task") +// The handler calls `isBlanketDenyEngaged` and reads +// `BLANKET_DENY_AUTO_DENY_KINDS` from the Task module, so the blanket-policy +// helpers must stay real while the Task class itself stays a stub. +vitest.mock("../../task/Task", async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, Task: vitest.fn() } +}) vitest.mock("../../prompts/responses") const mockRunDcg = vitest.fn() @@ -88,6 +100,7 @@ describe("executeCommandTool", () => { recordToolUsage: vitest.fn().mockReturnValue({} as ToolUsage), recordToolError: vitest.fn(), supersedePendingAsk: vitest.fn(), + recordBlanketCommandDenial: vitest.fn(), providerRef: { deref: vitest.fn().mockResolvedValue({ contextProxy: { @@ -334,10 +347,319 @@ describe("executeCommandTool", () => { pushToolResult: mockPushToolResult as unknown as PushToolResult, }) - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test") + // The DCG verdict is forwarded so checkAutoApproval auto-approves from + // the verdict itself rather than inferring it from settings alone. + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + dcgDecision: { decision: "allow" }, + }) expect(mockPushToolResult).toHaveBeenCalled() }) + it("passes the DCG deny verdict unprotected when blanket auto-deny is engaged", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + provider.getState.mockResolvedValue({ + destructiveCommandGuardEnabled: true, + terminalShellIntegrationDisabled: true, + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + }) + mockRunDcg.mockResolvedValue({ decision: "deny", reason: "matches a destructive pattern" }) + mockAskApproval.mockResolvedValue(false) + + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + // Blanket mode: not protected, so checkAutoApproval resolves the ask as + // an automatic denial carrying the DCG reason instead of prompting. + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + dcgDecision: { decision: "deny", reason: "matches a destructive pattern" }, + }) + }) + + it("keeps the protected prompt when DCG denies and blanket auto-deny is off", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + provider.getState.mockResolvedValue({ + destructiveCommandGuardEnabled: true, + terminalShellIntegrationDisabled: true, + alwaysDenyUnapprovedCommands: false, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + }) + mockRunDcg.mockResolvedValue({ decision: "deny", reason: "matches a destructive pattern" }) + mockAskApproval.mockResolvedValue(false) + + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + }) + + it("keeps the protected prompt when blanket auto-deny is on but command auto-approval is off", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + provider.getState.mockResolvedValue({ + destructiveCommandGuardEnabled: true, + terminalShellIntegrationDisabled: true, + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: false, + alwaysAllowExecute: true, + }) + mockRunDcg.mockResolvedValue({ decision: "deny", reason: "matches a destructive pattern" }) + mockAskApproval.mockResolvedValue(false) + + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + }) + + it("keeps the protected prompt when blanket auto-deny is on but execute auto-approval is off", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + provider.getState.mockResolvedValue({ + destructiveCommandGuardEnabled: true, + terminalShellIntegrationDisabled: true, + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: false, + }) + mockRunDcg.mockResolvedValue({ decision: "deny", reason: "matches a destructive pattern" }) + mockAskApproval.mockResolvedValue(false) + + // Structural harness double — mockCline carries only the fields the handler reads; a typed Task is impractical. + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + // vi.fn stands in for the AskApproval signature; every test in this block uses this identical cast. + askApproval: mockAskApproval as unknown as AskApproval, + // vi.fn stands in for the HandleError signature; every test in this block uses this identical cast. + handleError: mockHandleError as unknown as HandleError, + // vi.fn stands in for the PushToolResult signature; every test in this block uses this identical cast. + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + }) + + it("denies an approved command at execute time when blanket deny engages during the approval dwell", async () => { + // The seconds-wide window: the approval rode on the pre-ask snapshot + // taken with blanket deny off, and the engagement save lands before + // the command reaches the terminal. The execute-time re-check must + // deny instead of executing, and the blanket-kind denial must latch + // the turn so the queue message this denial leaves cannot stand in + // as approval for a later ask. + const provider = await mockCline.providerRef.deref() + provider.getState + .mockResolvedValueOnce({ + alwaysDenyUnapprovedCommands: false, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: false, + terminalShellIntegrationDisabled: true, + }) + .mockResolvedValue({ + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: false, + terminalShellIntegrationDisabled: true, + }) + mockAskApproval.mockResolvedValue(true) + + // The `as Task` cast is documentary — the harness double is `any`-typed, + // so it and the `vi.fn` callbacks already satisfy the parameter types. + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + }) + + // Position pin: the denial must arrive after an ask actually happened — + // a pre-approval gate would deny without ever asking. + expect(mockAskApproval).toHaveBeenCalledTimes(1) + + // Denied, not executed: the command never reaches the terminal. + expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled() + expect(mockCline.recordBlanketCommandDenial).toHaveBeenCalledTimes(1) + expect(formatResponse.toolAutoDenied).toHaveBeenCalledWith({ + reason: expect.stringContaining("not on the command allowlist"), + offendingCommand: "echo test", + ruleId: undefined, + }) + expect(formatResponse.toolError).not.toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalledTimes(1) + }) + + it("routes a guard flip during the approval dwell to a retryable error without latching", async () => { + // The DCG setting flips on between approval and execution: the tool + // holds no verdict for this command, so the fresh read denies with the + // guard-state inconsistency. That is not a policy denial: the payload + // stays a retryable tool error, and nothing latches — a re-issue + // carrying a verdict may still execute. + const provider = await mockCline.providerRef.deref() + provider.getState + .mockResolvedValueOnce({ + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: false, + terminalShellIntegrationDisabled: true, + }) + .mockResolvedValue({ + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + destructiveCommandGuardEnabled: true, + terminalShellIntegrationDisabled: true, + }) + mockAskApproval.mockResolvedValue(true) + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + }) + + // Same position pin as the sibling test: asked first, denied by the re-check. + expect(mockAskApproval).toHaveBeenCalledTimes(1) + expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled() + expect(mockCline.recordBlanketCommandDenial).not.toHaveBeenCalled() + expect(formatResponse.toolAutoDenied).not.toHaveBeenCalled() + expect(formatResponse.toolError).toHaveBeenCalledWith(expect.stringContaining("not a policy denial")) + expect(mockPushToolResult).toHaveBeenCalledTimes(1) + }) + + // Shared scenario for the protected-prompt transition: the command is + // DCG-denied with blanket auto-deny OFF, so the user sees the protected + // prompt, explicitly approves it, and the blanket setting engages before + // the command reaches the terminal. + const setupProtectedDwellFlip = async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + provider.getState + .mockResolvedValueOnce({ + destructiveCommandGuardEnabled: true, + alwaysDenyUnapprovedCommands: false, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + terminalShellIntegrationDisabled: true, + }) + .mockResolvedValue({ + destructiveCommandGuardEnabled: true, + alwaysDenyUnapprovedCommands: true, + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: [], + deniedCommands: [], + terminalShellIntegrationDisabled: true, + }) + mockRunDcg.mockResolvedValue({ + decision: "deny", + reason: "matches a destructive pattern", + ruleId: "recursive-delete", + }) + // The user affirmatively approves the protected prompt during the dwell. + mockAskApproval.mockResolvedValue(true) + return provider + } + + it("auto-denies a DCG-denied protected command when blanket deny engages during the approval dwell", async () => { + await setupProtectedDwellFlip() + + await executeCommandTool.handle(mockCline, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + }) + + // The prompt shown was the protected one — the blanket setting was off + // when the ask was built. + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + + // The blanket denial wins over the affirmative approval: the command + // never reaches the terminal. The provider-level assertion is the + // load-bearing one — it is the real execution boundary. + expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() + expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled() + expect(mockCline.recordBlanketCommandDenial).toHaveBeenCalledTimes(1) + expect(formatResponse.toolAutoDenied).toHaveBeenCalledWith({ + reason: expect.stringContaining("matches a destructive pattern"), + offendingCommand: "echo test", + ruleId: "recursive-delete", + }) + expect(formatResponse.toolError).not.toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalledTimes(1) + }) + + it("re-derives the recheck's protected flag from the fresh state instead of forwarding the latched prompt flag", async () => { + await setupProtectedDwellFlip() + const originalCheckAutoApproval = autoApprovalModule.checkAutoApproval + const recheckSpy = vitest + .spyOn(autoApprovalModule, "checkAutoApproval") + .mockImplementation((args) => originalCheckAutoApproval(args)) + + await executeCommandTool.handle(mockCline, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + }) + + expect(recheckSpy).toHaveBeenCalledTimes(1) + // The ask the user answered was protected, but protection is a property + // of the policy that produced the prompt. Forwarding the latched flag + // would make `checkAutoApproval` short-circuit to `ask` before ever + // evaluating the now-engaged blanket denial. + expect(recheckSpy).toHaveBeenCalledWith(expect.objectContaining({ ask: "command", isProtected: false })) + expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() + }) + + it("fails closed when the execute-time recheck returns a non-approval other than deny", async () => { + await setupProtectedDwellFlip() + // The blanket-engaged command policy never answers `ask` today; force a + // result from the permitted-but-unexpected tail of the union to pin that + // any non-approval blocks execution instead of falling through. + vitest.spyOn(autoApprovalModule, "checkAutoApproval").mockResolvedValue({ decision: "ask" }) + + await executeCommandTool.handle(mockCline, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + }) + + expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() + expect(executeCommandModule.executeCommandInTerminal).not.toHaveBeenCalled() + // Not a policy denial: no latch, and the retryable-error channel instead. + expect(mockCline.recordBlanketCommandDenial).not.toHaveBeenCalled() + expect(formatResponse.toolAutoDenied).not.toHaveBeenCalled() + expect(formatResponse.toolError).toHaveBeenCalledWith(expect.stringContaining("instead of an approval")) + expect(mockPushToolResult).toHaveBeenCalledTimes(1) + }) + it("installs or updates DCG before evaluating an enabled command", async () => { const provider = await mockCline.providerRef.deref() provider.context = { globalStorageUri: { fsPath: "/test/storage" } } @@ -673,6 +995,38 @@ describe("executeCommandTool", () => { expect(mockPushToolResult.mock.calls[0][0]).toContain("Exit code: 0") }) + it("honors a terminal shell integration flip made during a pending approval", async () => { + vitest.useFakeTimers() + const provider = await mockCline.providerRef.deref() + // The pre-ask snapshot must not pin terminal behavior: execution + // reads provider state again after approval and must honor the + // fresher value. + provider.getState + .mockResolvedValueOnce({ terminalShellIntegrationDisabled: true }) + .mockResolvedValueOnce({ terminalShellIntegrationDisabled: false }) + vitest.spyOn(Terminal, "isActiveShellCmdExe").mockReturnValue(false) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("Write-Output hello") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + // A stale pre-ask snapshot would have selected "execa"; the post-approval + // re-read sees shell integration enabled again and selects "vscode". + expect(terminal.provider).toBe("vscode") + + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("hello\n", proc) + await callbacks.onCompleted!("hello\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(100) + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + }) + it("allows an explicit agent timeout to move a command to the background", async () => { vitest.useFakeTimers() const terminal = await setupControllableTerminal() diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 718d6430c1..3a38512e21 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -47,6 +47,7 @@ import { DEFAULT_WRITE_DELAY_MS, DEFAULT_DIFF_FUZZY_THRESHOLD, DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED, + DEFAULT_ALWAYS_DENY_UNAPPROVED_COMMANDS, DEFAULT_AUTO_CLOSE_ZOO_OPENED_FILES, DEFAULT_AUTO_CLOSE_ZOO_OPENED_FILES_AFTER_USER_EDITED, DEFAULT_AUTO_CLOSE_ZOO_OPENED_NEW_FILES, @@ -2562,6 +2563,7 @@ export class ClineProvider allowedWriteFiles, alwaysAllowExecute, destructiveCommandGuardEnabled, + alwaysDenyUnapprovedCommands, allowedCommands, deniedCommands, alwaysAllowMcp, @@ -2722,6 +2724,7 @@ export class ClineProvider allowedWriteFiles: allowedWriteFiles ?? [], alwaysAllowExecute: alwaysAllowExecute ?? false, destructiveCommandGuardEnabled, + alwaysDenyUnapprovedCommands: alwaysDenyUnapprovedCommands ?? false, alwaysAllowMcp: alwaysAllowMcp ?? false, alwaysAllowModeSwitch: alwaysAllowModeSwitch ?? false, alwaysAllowSubtasks: alwaysAllowSubtasks ?? false, @@ -2962,6 +2965,8 @@ export class ClineProvider alwaysAllowExecute: stateValues.alwaysAllowExecute ?? false, destructiveCommandGuardEnabled: stateValues.destructiveCommandGuardEnabled ?? DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED, + alwaysDenyUnapprovedCommands: + stateValues.alwaysDenyUnapprovedCommands ?? DEFAULT_ALWAYS_DENY_UNAPPROVED_COMMANDS, alwaysAllowMcp: stateValues.alwaysAllowMcp ?? false, alwaysAllowModeSwitch: stateValues.alwaysAllowModeSwitch ?? false, alwaysAllowSubtasks: stateValues.alwaysAllowSubtasks ?? false, diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 29a8ed53f6..ad98302335 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1518,6 +1518,20 @@ describe("ClineProvider", () => { expect(state.destructiveCommandGuardEnabled).toBe(true) }) + test("getState returns the saved blanket auto-deny setting", async () => { + await provider.contextProxy.setValue("alwaysDenyUnapprovedCommands", true) + + const state = await provider.getState() + + expect(state.alwaysDenyUnapprovedCommands).toBe(true) + }) + + test("getState defaults blanket auto-deny to false", async () => { + const state = await provider.getState() + + expect(state.alwaysDenyUnapprovedCommands).toBe(false) + }) + test("getState returns the saved allowed read files", async () => { await provider.contextProxy.setValue("allowedReadFiles", ["notes.md"]) @@ -1597,6 +1611,23 @@ describe("ClineProvider", () => { expect(state.destructiveCommandGuardEnabled).toBe(false) }) + test("getStateToPostToWebview returns the saved blanket auto-deny setting", async () => { + await provider.resolveWebviewView(mockWebviewView) + await provider.contextProxy.setValue("alwaysDenyUnapprovedCommands", true) + + const state = await provider.getStateToPostToWebview() + + expect(state.alwaysDenyUnapprovedCommands).toBe(true) + }) + + test("getStateToPostToWebview disables blanket auto-deny by default", async () => { + await provider.resolveWebviewView(mockWebviewView) + + const state = await provider.getStateToPostToWebview() + + expect(state.alwaysDenyUnapprovedCommands).toBe(false) + }) + test("language is set to VSCode language", async () => { // Mock VSCode language as Spanish ;(vscode.env as any).language = "pt-BR" diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 4c2a301965..dcd70f92f1 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -1310,6 +1310,40 @@ describe("webviewMessageHandler - destructiveCommandGuardEnabled", () => { }) }) +// A plain boolean needs no normalization branch in the updateSettings loop, +// so this pins the generic persistence path: every polarity, including an +// explicit unset (undefined), must reach ContextProxy verbatim rather than +// being defaulted away. +it("persists alwaysDenyUnapprovedCommands through the generic updateSettings loop", async () => { + vi.clearAllMocks() + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { alwaysDenyUnapprovedCommands: true }, + }) + + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalledTimes(1) + + vi.clearAllMocks() + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { alwaysDenyUnapprovedCommands: false }, + }) + + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", false) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalledTimes(1) + + vi.clearAllMocks() + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { alwaysDenyUnapprovedCommands: undefined }, + }) + + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", undefined) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalledTimes(1) +}) + // Both allowlists are normalized by the same branch, so both are held to the // same contract. describe.each(["allowedReadFiles", "allowedWriteFiles"] as const)("webviewMessageHandler - %s", (key) => { diff --git a/src/scripts/__tests__/merge-lcov.spec.mjs b/src/scripts/__tests__/merge-lcov.spec.mjs index 64706eaf02..4b231fe54c 100644 --- a/src/scripts/__tests__/merge-lcov.spec.mjs +++ b/src/scripts/__tests__/merge-lcov.spec.mjs @@ -42,6 +42,24 @@ describe("mergeLcov", () => { expect(merged).not.toContain("DA:3,1") }) + it("treats a negative v8 branch delta as uncovered instead of failing the merge", () => { + // The v8 provider emits negative BRDA counts for short-circuit conditions + // (e.g. `freshState === "aborted" || signal?.aborted`), which previously + // aborted the CI coverage merge with "Invalid BRDA ... -3". + const merged = mergeLcov([ + ["core", report(new Set([1])).replace("BRDA:2,0,0,-", "BRDA:2,0,0,-3")], + ["api", report(new Set([1, 2]))], + ]) + + // The negative-delta branch merges to uncovered, and the positive + // duplicate from the other lane still wins the union. + expect(merged).toContain("BRDA:2,0,0,1") + + const solo = mergeLcov([["core", report(new Set([1])).replace("BRDA:2,0,0,-", "BRDA:2,0,0,-3")]]) + expect(solo).toContain("BRDA:2,0,0,-") + expect(solo).not.toContain("-3") + }) + it("merges disjoint source records without changing their paths", () => { const merged = mergeLcov([ ["api", report(new Set([1])).replaceAll("src/example.ts", "src/api.ts")], diff --git a/src/scripts/merge-lcov.mjs b/src/scripts/merge-lcov.mjs index cfcbd46cb6..8720697076 100644 --- a/src/scripts/merge-lcov.mjs +++ b/src/scripts/merge-lcov.mjs @@ -7,6 +7,16 @@ const parseCount = (value, description) => { return count } +// The v8 provider emits negative BRDA counts for short-circuit conditions, where +// an uncovered inner branch range is subtracted from a covered outer one +// (e.g. `a === X || b?.flag`). A negative delta means the branch was never taken +// on its own, so clamp it to uncovered instead of failing the merge. +const parseBranchCount = (value, description) => { + const count = Number(value) + if (!Number.isSafeInteger(count)) throw new Error(`Invalid ${description}: ${value}`) + return count > 0 ? count : 0 +} + const mergeCount = (records, key, count) => records.set(key, Math.max(records.get(key) ?? 0, count)) const parseLcov = (lcov, label) => { @@ -48,7 +58,7 @@ const parseLcov = (lcov, label) => { } else if (record && line.startsWith("BRDA:")) { const [lineNumber, block, branch, taken] = line.slice(5).split(",") const key = `${lineNumber},${block},${branch}` - const count = taken === "-" ? 0 : parseCount(taken, `BRDA for ${record.source}`) + const count = taken === "-" ? 0 : parseBranchCount(taken, `BRDA for ${record.source}`) mergeCount(record.branches, key, count) } else if (record && line.startsWith("DA:")) { const [lineNumber, count, checksum] = line.slice(3).split(",") diff --git a/src/shared/tools.ts b/src/shared/tools.ts index 1a1fb03200..a1daf27d08 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -2,13 +2,31 @@ import { Anthropic } from "@anthropic-ai/sdk" import type { ClineAsk, ToolProgressStatus, ToolGroup, ToolName, GenerateImageParams } from "@roo-code/types" +import type { DcgDecision } from "../services/destructive-command-guard/runner" + export type ToolResponse = string | Array +/** + * Extra inputs that only some ask types consult when resolving auto-approval. + * Carried from the tool through `askApproval` into `Task.ask` and onward to + * `checkAutoApproval`. All fields are optional, so tools that pass nothing + * keep the existing behavior. + */ +export type AutoApprovalContext = { + /** + * Verdict of a Destructive Command Guard run the tool already performed + * for this exact command (`execute_command` only). Infra failures never + * produce a verdict — they surface as a retryable tool error beforehand. + */ + dcgDecision?: DcgDecision +} + export type AskApproval = ( type: ClineAsk, partialMessage?: string, progressStatus?: ToolProgressStatus, forceApproval?: boolean, + autoApprovalContext?: AutoApprovalContext, ) => Promise export type HandleError = (action: string, error: Error) => Promise diff --git a/webview-ui/playwright/gallery/stories.tsx b/webview-ui/playwright/gallery/stories.tsx index 7b42e9780d..97c91d1423 100644 --- a/webview-ui/playwright/gallery/stories.tsx +++ b/webview-ui/playwright/gallery/stories.tsx @@ -88,6 +88,11 @@ export const stories: Record = { ) }, + "auto-approve-settings": async () => { + const { AutoApproveSettingsStory } = + await import("@/components/settings/__tests__/AutoApproveSettings.visual.fixture") + return + }, "chat-text-area": async () => { const { ChatTextAreaStory } = await import("@/components/chat/__tests__/ChatTextArea.visual.fixture") return diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx index 6276ea6514..58034960fb 100644 --- a/webview-ui/src/components/settings/AutoApproveSettings.tsx +++ b/webview-ui/src/components/settings/AutoApproveSettings.tsx @@ -1,4 +1,4 @@ -import { HTMLAttributes, useState } from "react" +import { FormEvent, HTMLAttributes, useState } from "react" import { X } from "lucide-react" import { Trans } from "react-i18next" import { Package } from "@roo/package" @@ -32,6 +32,7 @@ type AutoApproveSettingsProps = HTMLAttributes & { alwaysAllowSubtasks?: boolean alwaysAllowExecute?: boolean destructiveCommandGuardEnabled?: boolean + alwaysDenyUnapprovedCommands?: boolean alwaysAllowFollowupQuestions?: boolean followupAutoApproveTimeoutMs?: number allowedCommands?: string[] @@ -51,6 +52,7 @@ type AutoApproveSettingsProps = HTMLAttributes & { | "alwaysAllowSubtasks" | "alwaysAllowExecute" | "destructiveCommandGuardEnabled" + | "alwaysDenyUnapprovedCommands" | "alwaysAllowFollowupQuestions" | "followupAutoApproveTimeoutMs" | "allowedCommands" @@ -60,6 +62,13 @@ type AutoApproveSettingsProps = HTMLAttributes & { > } +// The toolkit checkbox delivers change events retargeted to its custom-element host, which mirrors +// `checked` without being an HTMLInputElement; a directly rendered input is one. Match on the +// property so both target shapes update the setting, and decline targets that carry no boolean +// `checked` rather than writing `undefined` through the guard. +const isCheckboxTarget = (target: EventTarget | null): target is EventTarget & { checked: boolean } => + target !== null && "checked" in target && typeof target.checked === "boolean" + export const AutoApproveSettings = ({ alwaysAllowReadOnly, alwaysAllowReadOnlyOutsideWorkspace, @@ -73,6 +82,7 @@ export const AutoApproveSettings = ({ alwaysAllowSubtasks, alwaysAllowExecute, destructiveCommandGuardEnabled, + alwaysDenyUnapprovedCommands, alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs = 60000, allowedCommands, @@ -340,6 +350,28 @@ export const AutoApproveSettings = ({ + {/* Visible in both DCG modes: it replaces the hidden command + lists as the fail-closed policy in hands-free setups. */} + + ) => { + if (!isCheckboxTarget(e.target)) { + return + } + setCachedStateField("alwaysDenyUnapprovedCommands", e.target.checked) + }} + data-testid="auto-deny-unapproved-checkbox"> + {t("settings:autoApprove.execute.autoDeny.label")} + +
+ {t("settings:autoApprove.execute.autoDeny.description")} +
+ + {!destructiveCommandGuardEnabled && ( <> (({ onDone, t language, alwaysAllowExecute, destructiveCommandGuardEnabled, + alwaysDenyUnapprovedCommands, alwaysAllowMcp, alwaysAllowModeSwitch, alwaysAllowSubtasks, @@ -393,6 +394,7 @@ const SettingsView = forwardRef(({ onDone, t allowedWriteFiles: allowedWriteFiles ?? [], alwaysAllowExecute: alwaysAllowExecute ?? undefined, destructiveCommandGuardEnabled: destructiveCommandGuardEnabled ?? false, + alwaysDenyUnapprovedCommands: alwaysDenyUnapprovedCommands ?? false, alwaysAllowMcp, alwaysAllowModeSwitch, allowedCommands: allowedCommands ?? [], @@ -823,6 +825,7 @@ const SettingsView = forwardRef(({ onDone, t alwaysAllowSubtasks={alwaysAllowSubtasks} alwaysAllowExecute={alwaysAllowExecute} destructiveCommandGuardEnabled={destructiveCommandGuardEnabled} + alwaysDenyUnapprovedCommands={alwaysDenyUnapprovedCommands} alwaysAllowFollowupQuestions={alwaysAllowFollowupQuestions} followupAutoApproveTimeoutMs={followupAutoApproveTimeoutMs} allowedCommands={allowedCommands} diff --git a/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx index c5c0e30024..12090b42b5 100644 --- a/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx @@ -1,5 +1,7 @@ // npx vitest src/components/settings/__tests__/AutoApproveSettings.spec.tsx +import React from "react" + import { render, screen, fireEvent } from "@/utils/test-utils" import { AutoApproveSettings } from "../AutoApproveSettings" @@ -11,6 +13,54 @@ vi.mock("@/utils/vscode", () => ({ }, })) +// The toolkit checkbox is a shadow-DOM custom element, so its change events reach the wrapper +// retargeted to the host element, which exposes `checked` itself and is not an HTMLInputElement. +// The global JSX mock renders a plain , hiding that shape from every test in this file; +// this override renders the host tag so the suite exercises the event shape the real webview sends. +type CheckboxHost = HTMLElement & { checked: boolean } + +type CheckboxHostProps = { + children?: React.ReactNode + checked?: boolean + "data-testid"?: string + onChange?: (event: Event) => void +} + +vi.mock("@vscode/webview-ui-toolkit/react", async () => { + const toolkitMock = await import("@/__mocks__/@vscode/webview-ui-toolkit/react") + + const VSCodeCheckboxHost = ({ children, onChange, checked, "data-testid": dataTestId }: CheckboxHostProps) => + React.createElement( + "vscode-checkbox", + { + "data-testid": dataTestId, + role: "checkbox", + "aria-checked": String(checked ?? false), + // Mirror the controlled prop onto the host like the real component's internal state, + // and flip it on click so the dispatched change event carries the post-click value. + // An unset prop leaves the host without `checked` at all — the degenerate shape the + // handler must decline instead of writing `undefined`. + ref: (el: CheckboxHost | null) => { + if (!el) { + return + } + if (checked !== undefined) { + el.checked = checked + } + el.onclick = () => { + el.checked = !el.checked + el.setAttribute("aria-checked", String(el.checked)) + el.dispatchEvent(new CustomEvent("change", { bubbles: true, composed: true })) + } + el.onchange = (e: Event) => onChange?.(e) + }, + }, + children, + ) + + return { ...toolkitMock, VSCodeCheckbox: VSCodeCheckboxHost } +}) + vi.mock("@/i18n/TranslationContext", () => ({ useAppTranslation: () => ({ t: (key: string) => key }), })) @@ -226,4 +276,81 @@ describe("AutoApproveSettings - Save/Discard contract", () => { expect(screen.getByTestId("allowed-commands-heading")).toBeInTheDocument() expect(screen.getByTestId("denied-commands-heading")).toBeInTheDocument() }) + + // The blanket auto-deny toggle replaces the hidden command lists as the + // fail-closed policy in hands-free setups, so it must stay reachable in + // BOTH DCG modes — unlike the list editors above. + it.each([ + ["disabled", false], + ["enabled", true], + ])("shows the blanket auto-deny toggle while destructive command guard is %s", (_label, dcgEnabled) => { + renderSettings({ destructiveCommandGuardEnabled: dcgEnabled }) + + expect(screen.getByTestId("auto-deny-unapproved-checkbox")).toBeInTheDocument() + }) + + it("renders blanket auto-deny disabled by default", () => { + renderSettings() + + expect(screen.getByTestId("auto-deny-unapproved-checkbox")).not.toBeChecked() + }) + + it("renders blanket auto-deny enabled from cached settings", () => { + renderSettings({ alwaysDenyUnapprovedCommands: true }) + + expect(screen.getByTestId("auto-deny-unapproved-checkbox")).toBeChecked() + }) + + it("buffers the blanket auto-deny setting", () => { + const { setCachedStateField } = renderSettings() + + fireEvent.click(screen.getByTestId("auto-deny-unapproved-checkbox")) + + expect(setCachedStateField).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + expectNoImmediateUpdateSettings() + }) + + it("buffers disabling blanket auto-deny", () => { + const { setCachedStateField } = renderSettings({ alwaysDenyUnapprovedCommands: true }) + + fireEvent.click(screen.getByTestId("auto-deny-unapproved-checkbox")) + + expect(setCachedStateField).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", false) + expectNoImmediateUpdateSettings() + }) + + it("hides the blanket auto-deny toggle while command auto-approval is off", () => { + // With alwaysAllowExecute off, the whole Execute section (and its + // toggles, including the blanket auto-deny toggle) is hidden. + renderSettings({ alwaysAllowExecute: false }) + + expect(screen.queryByTestId("auto-deny-unapproved-checkbox")).not.toBeInTheDocument() + }) + + // A change event from the real toolkit arrives with the custom-element host as + // e.target, which is not an HTMLInputElement; an instanceof-style guard would + // silently swallow it, leaving the toggle dead in the real webview. + it("writes the blanket auto-deny value from a change event retargeted to the custom-element host", () => { + const { setCachedStateField } = renderSettings() + const host = screen.getByTestId("auto-deny-unapproved-checkbox") + + expect(host).not.toBeInstanceOf(HTMLInputElement) + fireEvent.click(host) + + expect(setCachedStateField).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + expectNoImmediateUpdateSettings() + }) + + it("declines a blanket auto-deny change whose target carries no checked value", () => { + const { setCachedStateField } = renderSettings() + const host = screen.getByTestId("auto-deny-unapproved-checkbox") + + // The degenerate shape: a change event whose target exposes no boolean + // `checked`. The handler must decline it entirely rather than buffer + // `undefined` through the guard. + fireEvent.change(host) + + expect(setCachedStateField).not.toHaveBeenCalled() + expectNoImmediateUpdateSettings() + }) }) diff --git a/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.fixture.tsx b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.fixture.tsx new file mode 100644 index 0000000000..d7fa363fdc --- /dev/null +++ b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.fixture.tsx @@ -0,0 +1,81 @@ +import React, { useState } from "react" + +import type { ExtensionStateContextType } from "@/context/ExtensionStateContext" +import type { SetCachedStateField } from "../types" +import { AutoApproveSettings } from "../AutoApproveSettings" +import { AppProviders } from "../../../../playwright/AppProviders" + +interface AutoApproveState { + alwaysAllowReadOnly: boolean + alwaysAllowWrite: boolean + alwaysAllowMcp: boolean + alwaysAllowModeSwitch: boolean + alwaysAllowSubtasks: boolean + alwaysAllowExecute: boolean + alwaysAllowFollowupQuestions: boolean + destructiveCommandGuardEnabled: boolean + alwaysDenyUnapprovedCommands: boolean + allowedCommands: string[] + deniedCommands: string[] + allowedReadFiles: string[] + allowedWriteFiles: string[] + allowedMaxRequests?: number + allowedMaxCost?: number +} + +export function AutoApproveSettingsStory() { + // The blanket auto-deny checkbox is pinned here in its operative mode: with + // the destructive command guard on it is the fail-closed policy, and the + // guard's hidden command-list editors would otherwise vary the snapshot. + const [state, setState] = useState({ + alwaysAllowReadOnly: false, + alwaysAllowWrite: false, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: true, + alwaysAllowFollowupQuestions: false, + destructiveCommandGuardEnabled: true, + alwaysDenyUnapprovedCommands: true, + allowedCommands: [], + deniedCommands: [], + allowedReadFiles: [], + allowedWriteFiles: [], + }) + const setCachedStateField: SetCachedStateField = (field, value) => { + setState((current) => { + switch (field) { + case "alwaysAllowReadOnly": + case "alwaysAllowWrite": + case "alwaysAllowMcp": + case "alwaysAllowModeSwitch": + case "alwaysAllowSubtasks": + case "alwaysAllowExecute": + case "alwaysAllowFollowupQuestions": + case "destructiveCommandGuardEnabled": + case "alwaysDenyUnapprovedCommands": + return { ...current, [field]: Boolean(value) } + case "allowedCommands": + case "deniedCommands": + case "allowedReadFiles": + case "allowedWriteFiles": + return { ...current, [field]: Array.isArray(value) ? [...value] : [] } + case "allowedMaxRequests": + case "allowedMaxCost": + return { ...current, [field]: typeof value === "number" ? value : undefined } + default: + return current + } + }) + } + + return ( + +
+ +
+ + ) +} diff --git a/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.tsx b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.tsx new file mode 100644 index 0000000000..049330e4cc --- /dev/null +++ b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.visual.tsx @@ -0,0 +1,20 @@ +import { expect, test } from "../../../../playwright/coverage-fixture" +import { mountedStory } from "../../../../playwright/mounted-story" +import { applyVisualTheme, visualThemes } from "../../../../playwright/themes" + +// Declared in webview-ui/src/i18n/locales/en/settings.json under +// settings:autoApprove.execute.autoDeny.label; the checkbox takes its accessible +// name from that label text, so the two must stay in sync. +const autoDenyLabel = "Auto-deny unapproved commands (never ask)" + +for (const theme of visualThemes) { + test(`renders the blanket auto-deny checkbox in the VS Code ${theme.name} theme`, async ({ mount, page }) => { + const component = mountedStory(await mount("auto-approve-settings")) + await applyVisualTheme(page, theme) + const story = component.getByTestId("auto-approve-settings-story") + const checkbox = story.getByRole("checkbox", { name: autoDenyLabel }) + await expect(checkbox).toBeVisible() + await expect(checkbox).toBeChecked() + await expect(story).toHaveScreenshot(`auto-approve-settings-${theme.name}.png`) + }) +} diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx index 07a4c3a018..6704aa1cf0 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx @@ -400,6 +400,8 @@ describe("SettingsView - Change Detection Fix", () => { allowedMaxCost: undefined, language: "en", alwaysAllowExecute: false, + alwaysDenyUnapprovedCommands: false, + destructiveCommandGuardEnabled: false, alwaysAllowMcp: false, alwaysAllowModeSwitch: false, alwaysAllowSubtasks: false, diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx index a3aa131902..528cbda497 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx @@ -276,6 +276,7 @@ const mockPostMessage = (state: any) => { shouldShowAnnouncement: false, allowedCommands: [], alwaysAllowExecute: false, + alwaysDenyUnapprovedCommands: false, ttsEnabled: false, ttsSpeed: 1, soundEnabled: false, @@ -804,3 +805,110 @@ describe("SettingsView - Duplicate Commands", () => { expect(onDone).toHaveBeenCalledTimes(1) }) }) + +describe("SettingsView - Blanket Auto-Deny", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + // Completes the persisted-setting round trip required by the repo's + // AGENTS.md "Persisted Setting Checklist" for the blanket auto-deny + // toggle: UI binding buffers in cachedState, and only Save persists the + // value to the extension host. + it("saves the blanket auto-deny toggle when clicking Save", () => { + const { activateTab, getSettingsContent } = renderSettingsView() + + // Activate the autoApprove tab + activateTab("autoApprove") + + const content = getSettingsContent() + // Enable always allow execute to reveal the execute section + const executeCheckbox = within(content).getByTestId("always-allow-execute-toggle") + fireEvent.click(executeCheckbox) + + // Enable blanket auto-deny + const autoDenyCheckbox = within(content).getByTestId("auto-deny-unapproved-checkbox") + fireEvent.click(autoDenyCheckbox) + expect(autoDenyCheckbox).toBeChecked() + + // Click Save to save settings + const saveButton = screen.getByTestId("save-button") + fireEvent.click(saveButton) + + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ + alwaysDenyUnapprovedCommands: true, + }), + }), + ) + }) + + it("posts blanket auto-deny as false when the setting is unset", () => { + // The submit path coerces an omitted setting to false + // (`alwaysDenyUnapprovedCommands ?? false`) rather than omitting the + // key, so the host always receives an explicit boolean. + const { activateTab, getSettingsContent } = renderSettingsView({ + alwaysDenyUnapprovedCommands: undefined, + }) + + // Activate the autoApprove tab + activateTab("autoApprove") + + const content = getSettingsContent() + // Enable always allow execute to reveal the execute section; the + // auto-deny toggle stays un-checked. + const executeCheckbox = within(content).getByTestId("always-allow-execute-toggle") + fireEvent.click(executeCheckbox) + expect(within(content).getByTestId("auto-deny-unapproved-checkbox")).not.toBeChecked() + + // Click Save to save settings + const saveButton = screen.getByTestId("save-button") + fireEvent.click(saveButton) + + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ + alwaysDenyUnapprovedCommands: false, + }), + }), + ) + }) + + it("buffers the blanket auto-deny toggle until Save", () => { + const { activateTab, getSettingsContent } = renderSettingsView() + + // Activate the autoApprove tab + activateTab("autoApprove") + + const content = getSettingsContent() + // Enable always allow execute to reveal the execute section + const executeCheckbox = within(content).getByTestId("always-allow-execute-toggle") + fireEvent.click(executeCheckbox) + + // Toggle blanket auto-deny on + const autoDenyCheckbox = within(content).getByTestId("auto-deny-unapproved-checkbox") + fireEvent.click(autoDenyCheckbox) + expect(autoDenyCheckbox).toBeChecked() + + // Toggling must NOT persist before Save; it only buffers in cachedState. + expect(vscode.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ alwaysDenyUnapprovedCommands: true }), + }), + ) + + // Save now persists the buffered value. + fireEvent.click(screen.getByTestId("save-button")) + + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ alwaysDenyUnapprovedCommands: true }), + }), + ) + }) +}) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx index 72e1cc7c5d..575da5a833 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx @@ -270,6 +270,8 @@ describe("SettingsView - Unsaved Changes Detection", () => { allowedMaxCost: undefined, language: "en" as const, alwaysAllowExecute: false, + alwaysDenyUnapprovedCommands: false, + destructiveCommandGuardEnabled: false, alwaysAllowMcp: false, alwaysAllowModeSwitch: false, alwaysAllowSubtasks: false, diff --git a/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-dark.png b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-dark.png new file mode 100644 index 0000000000..d2c31eaa59 Binary files /dev/null and b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-dark.png differ diff --git a/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast-light.png b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast-light.png new file mode 100644 index 0000000000..7516643165 Binary files /dev/null and b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast-light.png differ diff --git a/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast.png b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast.png new file mode 100644 index 0000000000..077f4efbca Binary files /dev/null and b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-high-contrast.png differ diff --git a/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-light.png b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-light.png new file mode 100644 index 0000000000..7b70bae540 Binary files /dev/null and b/webview-ui/src/components/settings/__tests__/__screenshots__/auto-approve-settings-light.png differ diff --git a/webview-ui/src/i18n/locales/ca/settings.json b/webview-ui/src/i18n/locales/ca/settings.json index 762a1290af..39b585a6b1 100644 --- a/webview-ui/src/i18n/locales/ca/settings.json +++ b/webview-ui/src/i18n/locales/ca/settings.json @@ -346,6 +346,10 @@ "label": "Activa la protecció contra ordres destructives", "description": "Baixa i utilitza Destructive Command Guard (DCG) per a aquesta plataforma. Les ordres permeses per DCG s'executen automàticament. Les ordres bloquejades per DCG requereixen la teva aprovació. Les llistes d'ordres de Zoo es desactiven mentre aquesta opció està activa. L'executable baixat es conserva si la desactives." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Comandes d'auto-execució permeses", "allowedCommandsDescription": "Prefixos de comandes que poden ser executats automàticament quan \"Aprovar sempre operacions d'execució\" està habilitat. Afegeix * per permetre totes les comandes (usar amb precaució).", "deniedCommands": "Comandes denegades", diff --git a/webview-ui/src/i18n/locales/de/settings.json b/webview-ui/src/i18n/locales/de/settings.json index f56984bc73..cf6106a6dd 100644 --- a/webview-ui/src/i18n/locales/de/settings.json +++ b/webview-ui/src/i18n/locales/de/settings.json @@ -346,6 +346,10 @@ "label": "Schutz vor destruktiven Befehlen aktivieren", "description": "Lädt Destructive Command Guard (DCG) für diese Plattform herunter und verwendet es. Von DCG erlaubte Befehle werden automatisch ausgeführt. Von DCG blockierte Befehle benötigen deine Zustimmung. Die Befehlslisten von Zoo sind währenddessen deaktiviert. Die heruntergeladene ausführbare Datei bleibt erhalten, wenn du die Option ausschaltest." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Erlaubte Auto-Ausführungsbefehle", "allowedCommandsDescription": "Befehlspräfixe, die automatisch ausgeführt werden können, wenn 'Ausführungsoperationen immer genehmigen' aktiviert ist. Fügen Sie * hinzu, um alle Befehle zu erlauben (mit Vorsicht verwenden).", "deniedCommands": "Verweigerte Befehle", diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 77f8a4f86d..3ada77ccc3 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -424,6 +424,10 @@ "label": "Enable destructive command guard", "description": "Download and use Destructive Command Guard (DCG) for this platform. Commands allowed by DCG run automatically. Commands blocked by DCG require your approval. Zoo's command lists are disabled while this is enabled. The downloaded executable is retained if you turn this off." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Allowed Auto-Execute Commands", "allowedCommandsDescription": "Command prefixes that can be auto-executed when \"Always approve execute operations\" is enabled. Add * to allow all commands (use with caution).", "deniedCommands": "Denied Commands", diff --git a/webview-ui/src/i18n/locales/es/settings.json b/webview-ui/src/i18n/locales/es/settings.json index e730483fa0..edd35629b9 100644 --- a/webview-ui/src/i18n/locales/es/settings.json +++ b/webview-ui/src/i18n/locales/es/settings.json @@ -346,6 +346,10 @@ "label": "Activar la protección contra comandos destructivos", "description": "Descarga y usa Destructive Command Guard (DCG) para esta plataforma. Los comandos permitidos por DCG se ejecutan automáticamente. Los comandos bloqueados por DCG requieren tu aprobación. Las listas de comandos de Zoo se desactivan mientras esta opción está habilitada. El ejecutable descargado se conserva si la desactivas." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Comandos de auto-ejecución permitidos", "allowedCommandsDescription": "Prefijos de comandos que pueden ser ejecutados automáticamente cuando \"Aprobar siempre operaciones de ejecución\" está habilitado. Añade * para permitir todos los comandos (usar con precaución).", "deniedCommands": "Comandos denegados", diff --git a/webview-ui/src/i18n/locales/fr/settings.json b/webview-ui/src/i18n/locales/fr/settings.json index 78770da21d..fbd2512e42 100644 --- a/webview-ui/src/i18n/locales/fr/settings.json +++ b/webview-ui/src/i18n/locales/fr/settings.json @@ -347,6 +347,10 @@ "label": "Activer la protection contre les commandes destructrices", "description": "Télécharge et utilise Destructive Command Guard (DCG) pour cette plateforme. Les commandes autorisées par DCG s'exécutent automatiquement. Les commandes bloquées par DCG nécessitent votre approbation. Les listes de commandes de Zoo sont désactivées tant que cette option est active. L'exécutable téléchargé est conservé si vous la désactivez." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Commandes auto-exécutables autorisées", "allowedCommandsDescription": "Préfixes de commandes qui peuvent être auto-exécutés lorsque \"Toujours approuver les opérations d'exécution\" est activé. Ajoutez * pour autoriser toutes les commandes (à utiliser avec précaution).", "deniedCommands": "Commandes refusées", diff --git a/webview-ui/src/i18n/locales/hi/settings.json b/webview-ui/src/i18n/locales/hi/settings.json index 413d3515bc..c6dffef9a3 100644 --- a/webview-ui/src/i18n/locales/hi/settings.json +++ b/webview-ui/src/i18n/locales/hi/settings.json @@ -346,6 +346,10 @@ "label": "विनाशकारी कमांड सुरक्षा सक्षम करें", "description": "इस प्लेटफ़ॉर्म के लिए Destructive Command Guard (DCG) डाउनलोड करके उपयोग करें। DCG द्वारा अनुमत कमांड अपने आप चलते हैं। DCG द्वारा रोके गए कमांड चलाने के लिए आपकी मंज़ूरी आवश्यक होगी। इसके सक्षम रहने पर Zoo की कमांड सूचियाँ बंद रहेंगी। इसे बंद करने पर डाउनलोड किया गया executable रखा जाएगा।" }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "अनुमत स्वतः-निष्पादन कमांड", "allowedCommandsDescription": "कमांड प्रीफिक्स जो स्वचालित रूप से निष्पादित किए जा सकते हैं जब \"निष्पादन ऑपरेशन हमेशा अनुमोदित करें\" सक्षम है। सभी कमांड की अनुमति देने के लिए * जोड़ें (सावधानी से उपयोग करें)।", "deniedCommands": "अस्वीकृत कमांड", diff --git a/webview-ui/src/i18n/locales/id/settings.json b/webview-ui/src/i18n/locales/id/settings.json index 9b9928da64..1ca1c4035b 100644 --- a/webview-ui/src/i18n/locales/id/settings.json +++ b/webview-ui/src/i18n/locales/id/settings.json @@ -346,6 +346,10 @@ "label": "Aktifkan perlindungan perintah destruktif", "description": "Unduh dan gunakan Destructive Command Guard (DCG) untuk platform ini. Perintah yang diizinkan DCG dijalankan secara otomatis. Perintah yang diblokir DCG memerlukan persetujuanmu. Daftar perintah Zoo dinonaktifkan selama opsi ini aktif. File executable yang diunduh tetap disimpan jika kamu menonaktifkannya." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Perintah Auto-Execute yang Diizinkan", "allowedCommandsDescription": "Prefix perintah yang dapat di-auto-execute ketika \"Selalu setujui operasi eksekusi\" diaktifkan. Tambahkan * untuk mengizinkan semua perintah (gunakan dengan hati-hati).", "deniedCommands": "Perintah yang ditolak", diff --git a/webview-ui/src/i18n/locales/it/settings.json b/webview-ui/src/i18n/locales/it/settings.json index 8a43ec3d35..12e801aa49 100644 --- a/webview-ui/src/i18n/locales/it/settings.json +++ b/webview-ui/src/i18n/locales/it/settings.json @@ -346,6 +346,10 @@ "label": "Abilita la protezione dai comandi distruttivi", "description": "Scarica e usa Destructive Command Guard (DCG) per questa piattaforma. I comandi consentiti da DCG vengono eseguiti automaticamente. I comandi bloccati da DCG richiedono la tua approvazione. Gli elenchi dei comandi di Zoo vengono disabilitati mentre questa opzione è attiva. L'eseguibile scaricato viene conservato se la disattivi." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Comandi di auto-esecuzione consentiti", "allowedCommandsDescription": "Prefissi di comando che possono essere auto-eseguiti quando \"Approva sempre operazioni di esecuzione\" è abilitato. Aggiungi * per consentire tutti i comandi (usare con cautela).", "deniedCommands": "Comandi negati", diff --git a/webview-ui/src/i18n/locales/ja/settings.json b/webview-ui/src/i18n/locales/ja/settings.json index b2cfbe977e..72f0b4e54f 100644 --- a/webview-ui/src/i18n/locales/ja/settings.json +++ b/webview-ui/src/i18n/locales/ja/settings.json @@ -346,6 +346,10 @@ "label": "破壊的コマンドガードを有効にする", "description": "このプラットフォーム用の Destructive Command Guard（DCG）をダウンロードして使用します。DCG が許可したコマンドは自動的に実行されます。DCG がブロックしたコマンドの実行には承認が必要です。有効な間は Zoo のコマンドリストが無効になります。無効にしても、ダウンロードした実行ファイルは保持されます。" }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "許可された自動実行コマンド", "allowedCommandsDescription": "「実行操作を常に承認」が有効な場合に自動実行できるコマンドプレフィックス。すべてのコマンドを許可するには * を追加します（注意して使用してください）。", "deniedCommands": "拒否されたコマンド", diff --git a/webview-ui/src/i18n/locales/ko/settings.json b/webview-ui/src/i18n/locales/ko/settings.json index 1ff9addeb4..8f7b1dbbcb 100644 --- a/webview-ui/src/i18n/locales/ko/settings.json +++ b/webview-ui/src/i18n/locales/ko/settings.json @@ -346,6 +346,10 @@ "label": "파괴적 명령어 보호 활성화", "description": "이 플랫폼용 Destructive Command Guard(DCG)를 다운로드하여 사용합니다. DCG가 허용한 명령어는 자동으로 실행됩니다. DCG가 차단한 명령어는 실행 전 승인이 필요합니다. 이 옵션을 사용하는 동안 Zoo의 명령어 목록은 비활성화됩니다. 옵션을 꺼도 다운로드한 실행 파일은 유지됩니다." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "허용된 자동 실행 명령", "allowedCommandsDescription": "\"실행 작업 항상 승인\"이 활성화되었을 때 자동 실행될 수 있는 명령 접두사. 모든 명령을 허용하려면 * 추가(주의해서 사용)", "deniedCommands": "거부된 명령", diff --git a/webview-ui/src/i18n/locales/nl/settings.json b/webview-ui/src/i18n/locales/nl/settings.json index 4361d091a1..9751728c80 100644 --- a/webview-ui/src/i18n/locales/nl/settings.json +++ b/webview-ui/src/i18n/locales/nl/settings.json @@ -346,6 +346,10 @@ "label": "Beveiliging tegen destructieve opdrachten inschakelen", "description": "Download en gebruik Destructive Command Guard (DCG) voor dit platform. Opdrachten die DCG toestaat, worden automatisch uitgevoerd. Voor opdrachten die DCG blokkeert, is jouw goedkeuring nodig. De opdrachtenlijsten van Zoo zijn uitgeschakeld zolang deze optie actief is. Het gedownloade uitvoerbare bestand blijft bewaard als je de optie uitschakelt." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Toegestane automatisch uit te voeren commando's", "allowedCommandsDescription": "Commando-prefixen die automatisch kunnen worden uitgevoerd als 'Altijd goedkeuren voor uitvoeren' is ingeschakeld. Voeg * toe om alle commando's toe te staan (gebruik met voorzichtigheid).", "deniedCommands": "Geweigerde commando's", diff --git a/webview-ui/src/i18n/locales/pl/settings.json b/webview-ui/src/i18n/locales/pl/settings.json index 277bcaa470..5fd8bdfe32 100644 --- a/webview-ui/src/i18n/locales/pl/settings.json +++ b/webview-ui/src/i18n/locales/pl/settings.json @@ -346,6 +346,10 @@ "label": "Włącz ochronę przed destrukcyjnymi poleceniami", "description": "Pobiera i używa Destructive Command Guard (DCG) dla tej platformy. Polecenia dozwolone przez DCG są wykonywane automatycznie. Polecenia zablokowane przez DCG wymagają twojej zgody. Listy poleceń Zoo są wyłączone, gdy ta opcja jest aktywna. Pobrany plik wykonywalny pozostaje na dysku po wyłączeniu opcji." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Dozwolone polecenia auto-wykonania", "allowedCommandsDescription": "Prefiksy poleceń, które mogą być automatycznie wykonywane, gdy \"Zawsze zatwierdzaj operacje wykonania\" jest włączone. Dodaj * aby zezwolić na wszystkie polecenia (używaj z ostrożnością).", "deniedCommands": "Odrzucone polecenia", diff --git a/webview-ui/src/i18n/locales/pt-BR/settings.json b/webview-ui/src/i18n/locales/pt-BR/settings.json index 8ce67bcd48..2b23f147b0 100644 --- a/webview-ui/src/i18n/locales/pt-BR/settings.json +++ b/webview-ui/src/i18n/locales/pt-BR/settings.json @@ -346,6 +346,10 @@ "label": "Ativar a proteção contra comandos destrutivos", "description": "Baixa e usa o Destructive Command Guard (DCG) para esta plataforma. Comandos permitidos pelo DCG são executados automaticamente. Comandos bloqueados pelo DCG precisam da sua aprovação. As listas de comandos do Zoo ficam desativadas enquanto esta opção estiver ativa. O executável baixado é mantido se você desativá-la." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Comandos de auto-execução permitidos", "allowedCommandsDescription": "Prefixos de comando que podem ser auto-executados quando \"Aprovar sempre operações de execução\" está ativado. Adicione * para permitir todos os comandos (use com cautela).", "deniedCommands": "Comandos negados", diff --git a/webview-ui/src/i18n/locales/ru/settings.json b/webview-ui/src/i18n/locales/ru/settings.json index 0ac516c190..c684696317 100644 --- a/webview-ui/src/i18n/locales/ru/settings.json +++ b/webview-ui/src/i18n/locales/ru/settings.json @@ -346,6 +346,10 @@ "label": "Включить защиту от разрушительных команд", "description": "Скачивает и использует Destructive Command Guard (DCG) для этой платформы. Разрешённые DCG команды выполняются автоматически. Команды, заблокированные DCG, требуют твоего подтверждения. Пока эта настройка включена, списки команд Zoo отключены. Скачанный исполняемый файл сохраняется после отключения настройки." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Разрешённые авто-выполняемые команды", "allowedCommandsDescription": "Префиксы команд, которые могут быть автоматически выполнены при включённом параметре \"Всегда одобрять выполнение операций\". Добавьте * для разрешения всех команд (используйте с осторожностью).", "deniedCommands": "Запрещенные команды", diff --git a/webview-ui/src/i18n/locales/tr/settings.json b/webview-ui/src/i18n/locales/tr/settings.json index 5d3a5cb89a..cd4556c7db 100644 --- a/webview-ui/src/i18n/locales/tr/settings.json +++ b/webview-ui/src/i18n/locales/tr/settings.json @@ -346,6 +346,10 @@ "label": "Yıkıcı komut korumasını etkinleştir", "description": "Bu platform için Destructive Command Guard'ı (DCG) indirir ve kullanır. DCG'nin izin verdiği komutlar otomatik olarak çalıştırılır. DCG tarafından engellenen komutlar onayını gerektirir. Bu seçenek etkinken Zoo'nun komut listeleri devre dışı bırakılır. Seçeneği kapatsan da indirilen çalıştırılabilir dosya korunur." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "İzin Verilen Otomatik Yürütme Komutları", "allowedCommandsDescription": "\"Yürütme işlemlerini her zaman onayla\" etkinleştirildiğinde otomatik olarak yürütülebilen komut önekleri. Tüm komutlara izin vermek için * ekleyin (dikkatli kullanın).", "deniedCommands": "Reddedilen komutlar", diff --git a/webview-ui/src/i18n/locales/vi/settings.json b/webview-ui/src/i18n/locales/vi/settings.json index adb1be64e3..b81ad24a2f 100644 --- a/webview-ui/src/i18n/locales/vi/settings.json +++ b/webview-ui/src/i18n/locales/vi/settings.json @@ -346,6 +346,10 @@ "label": "Bật bảo vệ khỏi lệnh phá hoại", "description": "Tải xuống và sử dụng Destructive Command Guard (DCG) cho nền tảng này. Các lệnh được DCG cho phép sẽ tự động chạy. Các lệnh bị DCG chặn cần bạn phê duyệt. Danh sách lệnh của Zoo sẽ bị vô hiệu hóa khi tùy chọn này được bật. Tệp thực thi đã tải xuống vẫn được giữ lại nếu bạn tắt tùy chọn." }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "Các lệnh tự động thực thi được phép", "allowedCommandsDescription": "Tiền tố lệnh có thể được tự động thực thi khi \"Luôn phê duyệt các hoạt động thực thi\" được bật. Thêm * để cho phép tất cả các lệnh (sử dụng cẩn thận).", "deniedCommands": "Lệnh bị từ chối", diff --git a/webview-ui/src/i18n/locales/zh-CN/settings.json b/webview-ui/src/i18n/locales/zh-CN/settings.json index 916f629efe..53ec69eb80 100644 --- a/webview-ui/src/i18n/locales/zh-CN/settings.json +++ b/webview-ui/src/i18n/locales/zh-CN/settings.json @@ -346,6 +346,10 @@ "label": "启用破坏性命令防护", "description": "下载并使用适用于此平台的 Destructive Command Guard（DCG）。DCG 允许的命令会自动执行。被 DCG 拦截的命令需要你批准后才能执行。启用后，Zoo 的命令列表将被禁用。关闭此选项时，已下载的可执行文件会保留。" }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "命令白名单", "allowedCommandsDescription": "当\"自动批准命令行操作\"启用时可以自动执行的命令前缀。添加 * 以允许所有命令（谨慎使用）。", "deniedCommands": "拒绝的命令", diff --git a/webview-ui/src/i18n/locales/zh-TW/settings.json b/webview-ui/src/i18n/locales/zh-TW/settings.json index d1f9258dcf..312b005843 100644 --- a/webview-ui/src/i18n/locales/zh-TW/settings.json +++ b/webview-ui/src/i18n/locales/zh-TW/settings.json @@ -371,6 +371,10 @@ "label": "啟用破壞性命令防護", "description": "下載並使用適用於此平台的 Destructive Command Guard（DCG）。DCG 允許的命令會自動執行。被 DCG 阻擋的命令需要你核准後才能執行。啟用後，Zoo 的命令清單將停用。關閉此選項時，已下載的執行檔會保留。" }, + "autoDeny": { + "label": "Auto-deny unapproved commands (never ask)", + "description": "When auto-approval for command execution is on, no confirmation prompts appear for commands. DCG off: everything not on the allowlist is auto-denied. DCG on: everything DCG does not approve is auto-denied, with the DCG reason sent to the model. No confirmation prompts." + }, "allowedCommands": "允許自動執行的命令", "allowedCommandsDescription": "啟用「始終核准執行」時，可自動執行的命令前綴。新增 * 可允許所有命令（請謹慎使用）。", "deniedCommands": "拒絕的命令",