diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index f5757a51c7..408e6848b9 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -54,6 +54,17 @@ export const DEFAULT_COLLAPSE_COMMAND_OUTPUT_BY_DEFAULT = false export const DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED = false +/** + * Whether workspace-scoped command-list layers (`zoo-code.allowedCommands` / + * `zoo-code.deniedCommands` values set in workspace settings) participate in + * command auto-approval at all. Opt-in: while off, workspace-layer entries + * contribute nothing to enforcement or display. The manifest declares the key + * with `scope: "application"`, so only user settings can change it; this + * constant is the single default shared by every consumer reading the + * setting. + */ +export const DEFAULT_WORKSPACE_COMMAND_LISTS_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 @@ -180,6 +191,14 @@ export const globalSettingsSchema = z.object({ followupAutoApproveTimeoutMs: z.number().optional(), allowedCommands: z.array(z.string()).optional(), deniedCommands: z.array(z.string()).optional(), + /** + * Gate for the workspace-scoped command-list layers. While false (the + * default), workspace-set `allowedCommands`/`deniedCommands` entries + * contribute nothing; when true they are composed under the global layer, + * which outranks them wherever it matches a command. + * @default false + */ + workspaceCommandListsEnabled: z.boolean().optional(), commandExecutionTimeout: z.number().optional(), commandTimeoutAllowlist: z.array(z.string()).optional(), preventCompletionWithOpenTodos: z.boolean().optional(), diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 8a006c0639..c1cfebdc82 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -304,6 +304,7 @@ export type ExtensionState = Pick< | "followupAutoApproveTimeoutMs" | "allowedCommands" | "deniedCommands" + | "workspaceCommandListsEnabled" | "allowedMaxRequests" | "allowedMaxCost" | "ttsEnabled" @@ -387,6 +388,18 @@ export type ExtensionState = Pick< toolRequirements?: Record // Map of tool names to their requirements (e.g. {"apply_diff": true}) cwd?: string // Current working directory + + /** + * The user-scope command lists as persisted in extension global state, + * before any workspace-scoped composition. Auto-approval composes the + * pair it enforces from these raw layers plus the folder the task runs + * in, because the merged `allowedCommands`/`deniedCommands` pair follows + * the window's first folder and would judge a task by another root's + * lists. Not user settings: runtime state, written by the provider's + * state bake, never by the settings dialog. + */ + globalAllowedCommands?: string[] + globalDeniedCommands?: string[] telemetrySetting: TelemetrySetting telemetryKey?: string machineId?: string @@ -666,9 +679,7 @@ export interface WebviewMessage { | "getDismissedUpsells" | "openMarkdownPreview" | "updateSettings" - | "allowedCommands" | "getTaskWithAggregatedCosts" - | "deniedCommands" | "openDebugApiHistory" | "openDebugUiHistory" | "downloadErrorDiagnostics" @@ -726,7 +737,6 @@ export interface WebviewMessage { stepIndex?: number isLaunchAction?: boolean forceShow?: boolean - commands?: string[] audioType?: AudioType serverName?: string toolName?: string @@ -815,6 +825,14 @@ export interface WebviewMessage { codebaseIndexOpenRouterApiKey?: string } updatedSettings?: RooCodeSettings + /** + * The command lists as the settings dialog last displayed them, sent alongside the + * save that rewrites them. The settings file can gain entries while the dialog is + * open, and without this baseline the save path cannot tell those additions apart + * from list entries the user deleted: both look like "in the file, absent from the + * submitted list". Senders that do not display the lists omit it. + */ + commandListBaseline?: { allowedCommands?: string[]; deniedCommands?: string[] } /** Task configuration applied via `createTask()`. */ taskConfiguration?: RooCodeSettings // Worktree properties diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index abcd3c28a0..bfa6821201 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -22,6 +22,7 @@ vi.mock("vscode", () => ({ registerTextDocumentContentProvider: vi.fn(), getConfiguration: vi.fn().mockReturnValue({ get: vi.fn().mockReturnValue([]), + inspect: vi.fn(), }), createFileSystemWatcher: vi.fn().mockReturnValue({ onDidCreate: vi.fn(), @@ -228,15 +229,17 @@ vi.mock("../api/providers/fetchers/modelCache", () => ({ describe("extension.ts", () => { let mockContext: vscode.ExtensionContext + let globalStateValues: Map let settingsUpdatedHandler: ((data: Record) => void | Promise) | undefined beforeEach(() => { vi.clearAllMocks() + globalStateValues = new Map() mockContext = { extensionPath: "/test/path", globalState: { - get: vi.fn().mockReturnValue(undefined), + get: vi.fn((key: string) => globalStateValues.get(key)), update: vi.fn(), }, subscriptions: [], @@ -536,6 +539,148 @@ describe("extension.ts", () => { expect(dotenv.config).toHaveBeenCalledTimes(1) }) + describe("command-list activation seeding", () => { + function setConfigScope( + vscode: typeof import("vscode"), + scope: { globalValue?: string[]; workspaceValue?: string[]; deniedGlobalValue?: string[]; gate?: boolean }, + ) { + const { globalValue, workspaceValue, deniedGlobalValue, gate = false } = scope + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + // Key-scoped so proxy/other activation reads still see no value. + get: vi.fn((key: string) => { + if (key === "workspaceCommandListsEnabled") { + return gate + } + return key === "allowedCommands" ? [...(globalValue ?? []), ...(workspaceValue ?? [])] : undefined + }), + // Key-scoped inspect: the deny seed must not read the allow scope's + // values through the same fixture object. + inspect: vi.fn().mockImplementation((key: string) => + key === "deniedCommands" + ? { key, defaultValue: [], globalValue: deniedGlobalValue } + : { + key, + // Real inspect() always reports the declared package default; + // including it here lets the workspace-override test prove the + // default stays unseeded beside an override, not just absent. + defaultValue: ["git log", "git diff", "git show"], + globalValue, + workspaceValue, + }, + ), + update: vi.fn(), + // The double scripts key-scoped get() and per-scope inspect() + // shapes the shared helper cannot express without obscuring them. + } as unknown as vscode.WorkspaceConfiguration) + } + + beforeEach(() => { + vi.resetModules() + }) + + afterEach(async () => { + const vscode = await import("vscode") + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue([]), + inspect: vi.fn(), + // Restores the neutral default after the scope fixtures above; + // update is intentionally absent because no test runs after this. + } as unknown as vscode.WorkspaceConfiguration) + }) + + test("seeds both command lists before initializing the terminal registry", async () => { + // Invariant: a seed rejection must fail activation before the terminal + // registry is initialized, so no initialized-but-half-activated state is + // reachable (the registry's listeners are released only via deactivate, + // which the host does not guarantee after a failed activation). + const order: string[] = [] + const seedCommandList = vi.fn(async (_globalState: vscode.Memento, configKey: string): Promise => { + order.push(`seed:${configKey}`) + }) + vi.doMock("../utils/seedAllowedCommands", () => ({ seedCommandList })) + try { + const { TerminalRegistry } = await import("../integrations/terminal/TerminalRegistry") + vi.mocked(TerminalRegistry.initialize).mockImplementation(() => void order.push("initialize")) + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(order).toEqual(["seed:allowedCommands", "seed:deniedCommands", "initialize"]) + expect(TerminalRegistry.initialize).toHaveBeenCalledTimes(1) + } finally { + vi.doUnmock("../utils/seedAllowedCommands") + } + }) + + test("aborts activation before the terminal registry is initialized when seeding rejects", async () => { + vi.doMock("../utils/seedAllowedCommands", () => ({ + seedCommandList: vi.fn().mockRejectedValue(new Error("seed state write failed")), + })) + try { + const { TerminalRegistry } = await import("../integrations/terminal/TerminalRegistry") + const { ClineProvider } = await import("../core/webview/ClineProvider") + + const { activate } = await import("../extension") + await expect(activate(mockContext)).rejects.toThrow("seed state write failed") + + // The seed rejection must reach the host unchanged, with the registry + // never initialized and no downstream activation (sidebar provider) reached. + expect(TerminalRegistry.initialize).not.toHaveBeenCalled() + expect(ClineProvider).not.toHaveBeenCalled() + } finally { + vi.doUnmock("../utils/seedAllowedCommands") + } + }) + + test("does not seed global state from an enforced workspace-scoped override", async () => { + const vscode = await import("vscode") + // The gate is on: an enforced workspace list replaces the default across + // scopes, so the seed must not re-grant the approvals it removed. (With + // the gate off the workspace layer is inert and the default is seeded — + // pinned in the seed module's own spec.) + setConfigScope(vscode, { workspaceValue: ["docker"], gate: true }) + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(mockContext.globalState.update).not.toHaveBeenCalledWith("allowedCommands", expect.anything()) + expect(globalStateValues.get("allowedCommands")).toBeUndefined() + }) + + test("seeds global state from the user-scoped value when it is empty", async () => { + const vscode = await import("vscode") + setConfigScope(vscode, { globalValue: ["git"] }) + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", ["git"]) + }) + + test("does not overwrite global state that is already populated", async () => { + const vscode = await import("vscode") + setConfigScope(vscode, { globalValue: ["git"] }) + globalStateValues.set("allowedCommands", ["npm"]) + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(mockContext.globalState.update).not.toHaveBeenCalledWith("allowedCommands", expect.anything()) + expect(globalStateValues.get("allowedCommands")).toEqual(["npm"]) + }) + + test("seeds both command-list keys on a fresh profile", async () => { + const vscode = await import("vscode") + setConfigScope(vscode, { globalValue: ["git"], deniedGlobalValue: ["docker"] }) + + const { activate } = await import("../extension") + await activate(mockContext) + + expect(mockContext.globalState.update).toHaveBeenCalledWith("allowedCommands", ["git"]) + expect(mockContext.globalState.update).toHaveBeenCalledWith("deniedCommands", ["docker"]) + }) + }) + describe("cloud organization settings handling", () => { beforeEach(() => { vi.resetModules() diff --git a/src/core/auto-approval/__tests__/commandScope.spec.ts b/src/core/auto-approval/__tests__/commandScope.spec.ts new file mode 100644 index 0000000000..ee7a6e4104 --- /dev/null +++ b/src/core/auto-approval/__tests__/commandScope.spec.ts @@ -0,0 +1,990 @@ +import * as fs from "fs" +import * as os from "os" +import * as path from "path" + +import { DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED, globalSettingsSchema } from "@roo-code/types" +import * as vscode from "vscode" + +import { Package } from "../../../shared/package" +import { makeUri, makeWorkspaceConfiguration } from "../../../test-utils/vscode" +import { + commandListFolderForResource, + readCommandListScopes, + resolveEffectiveCommandLists, + resolveWorkspaceWriteTarget, + splitCommandListScopes, + workspaceCommandListsEnabled, +} from "../commandScope" +import { checkAutoApproval } from "../index" +import { baseState, type State } from "./fixtures" + +// Scriptable stand-in for `vscode.workspace.workspaceFolders`: the module +// mock's getter reads through this object, so a test can change the open +// folders without re-mocking the module. `foldersByPath` backs +// `getWorkspaceFolder`: keys are folder paths, values the folder objects, so +// a test controls which cwds resolve to a root. `throwsOnFoldersRead` makes +// the getter itself fail once, standing in for a workspace that cannot answer +// the lookup at all. +const workspaceState = vi.hoisted(() => ({ + workspaceFolders: undefined as { uri: unknown }[] | undefined, + foldersByPath: new Map(), + throwsOnFoldersRead: false, +})) + +// Scriptable stand-in for `fs.realpathSync`, which the root-side containment +// fallback consults. Every other fs binding stays real — the manifest reads +// and the real-filesystem tests run through `importOriginal` — and the double +// resolves paths to themselves by default: the mock roots are fake paths that +// exist on no filesystem, and an unresolvable root is compared by its +// registered path, so identity reproduces exactly what the real call does to +// them. Tests that need other behavior script it explicitly. +const fsState = vi.hoisted(() => ({ + // The unmocked implementation, captured by the module mock so a test can + // delegate through the double to real filesystem behavior. + realRealpathSync: null as null | ((target: string) => string), +})) + +vi.mock("fs", async (importOriginal) => { + const actual = await importOriginal() + fsState.realRealpathSync = (target: string) => String(actual.realpathSync(target)) + const realpathSync = vi.fn((target: string): string => String(target)) + // Interop shape: production code reaches `realpathSync` through the + // default export and this spec through the named one; both must see the + // same double. + const actualDefault = (actual as { default?: typeof import("fs") }).default ?? actual + return { + ...actual, + default: { ...actualDefault, realpathSync }, + realpathSync, + } +}) + +/** The unmocked `fs.realpathSync`, for tests that pin real-filesystem behavior. */ +function realPath(target: string): string { + if (!fsState.realRealpathSync) { + throw new Error("the fs module mock did not capture the unmocked realpathSync") + } + return fsState.realRealpathSync(target) +} + +vi.mock("vscode", () => ({ + ConfigurationTarget: { + Global: 1, + Workspace: 2, + WorkspaceFolder: 3, + }, + Uri: { + // Structural local stand-in: the resolver only carries the path + // through to `getWorkspaceFolder`. + file: (fsPath: string) => ({ scheme: "file", authority: "", path: fsPath, fsPath, query: "", fragment: "" }), + }, + workspace: { + get workspaceFolders() { + if (workspaceState.throwsOnFoldersRead) { + // One-shot, like the throwing-lookup test it serves: a leaked + // throw would fail every later read, including the + // first-folder default reads the composition still performs. + workspaceState.throwsOnFoldersRead = false + throw new Error("workspace unavailable") + } + return workspaceState.workspaceFolders + }, + getConfiguration: vi.fn(), + // Mirrors VS Code's lexical containment: a prefix test against the + // registered paths, with the platform separator as the guard between + // root and remainder. The folder lookup does not consult this double — + // it compares roots by their real paths — but the double stays + // faithful so a test can register an outer root that lexically + // contains a symlinked root's target, the containment that must not + // decide the folder match. + getWorkspaceFolder: vi.fn((resource?: { fsPath?: string }) => { + const fsPath = resource?.fsPath ?? "" + for (const [folderPath, uri] of workspaceState.foldersByPath) { + if (fsPath === folderPath || fsPath.startsWith(folderPath + path.sep)) { + return { uri } + } + } + return undefined + }), + }, +})) + +interface ConfigDouble { + inspect?: { + key: string + defaultValue?: string[] + globalValue?: string[] + workspaceValue?: string[] + workspaceFolderValue?: string[] + } + effective?: string[] +} + +function mockConfiguration({ inspect, effective = [] }: ConfigDouble = {}) { + const config = makeWorkspaceConfiguration() + // The shared double is scripted after creation so the per-test fixture + // values need no type assertion at the construction site. + vi.mocked(config.get).mockReturnValue(effective) + vi.mocked(config.inspect).mockReturnValue(inspect) + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) +} + +describe("readCommandListScopes", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("reads the workspace layer from the workspace-scoped configuration value", () => { + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker"] } }) + + const scopes = readCommandListScopes("deniedCommands", []) + + expect(scopes.workspace).toEqual(["docker"]) + expect(scopes.merged).toEqual(["docker"]) + }) + + it("prefers the workspace-folder value over the workspace value", () => { + mockConfiguration({ + inspect: { + key: "deniedCommands", + workspaceValue: ["docker"], + workspaceFolderValue: ["pnpm docker"], + }, + }) + + const scopes = readCommandListScopes("deniedCommands", []) + + expect(scopes.workspace).toEqual(["pnpm docker"]) + expect(scopes.merged).toEqual(["pnpm docker"]) + }) + + it("ignores the effective get() value so global-scoped entries are not double-counted", () => { + // A real VS Code get() folds globalValue into the effective result, so + // reading the workspace layer from get() would leak "npm" (a + // user-settings-only entry) into the workspace layer. + mockConfiguration({ + inspect: { key: "allowedCommands", globalValue: ["npm"], workspaceValue: ["docker"] }, + effective: ["npm", "docker"], + }) + + const scopes = readCommandListScopes("allowedCommands", []) + + expect(scopes.workspace).toEqual(["docker"]) + expect(scopes.merged).toEqual(["docker"]) + }) + + it("returns an empty workspace layer when only globalValue is set", () => { + mockConfiguration({ + inspect: { key: "allowedCommands", globalValue: ["npm"] }, + effective: ["npm"], + }) + + const scopes = readCommandListScopes("allowedCommands", []) + + expect(scopes.workspace).toEqual([]) + expect(scopes.merged).toEqual([]) + }) + + it("unions global and workspace layers with deduplication and stable order", () => { + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker", "rm"] } }) + + const scopes = readCommandListScopes("deniedCommands", ["git", "docker"]) + + expect(scopes.global).toEqual(["git", "docker"]) + expect(scopes.workspace).toEqual(["docker", "rm"]) + expect(scopes.merged).toEqual(["git", "docker", "rm"]) + }) + + it("drops blank and non-string entries from both layers", () => { + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker", " "] } }) + + const scopes = readCommandListScopes("deniedCommands", ["git", "", " "] as string[]) + + expect(scopes.global).toEqual(["git"]) + expect(scopes.workspace).toEqual(["docker"]) + expect(scopes.merged).toEqual(["git", "docker"]) + }) + + it("treats a missing inspect result as an empty workspace layer", () => { + mockConfiguration({ inspect: undefined }) + + const scopes = readCommandListScopes("deniedCommands", ["git"]) + + expect(scopes.workspace).toEqual([]) + expect(scopes.merged).toEqual(["git"]) + }) + + it("falls back to an empty workspace layer when the configuration lookup throws", () => { + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { + throw new Error("configuration unavailable") + }) + + const scopes = readCommandListScopes("deniedCommands", ["git"]) + + expect(scopes.workspace).toEqual([]) + expect(scopes.merged).toEqual(["git"]) + }) + + it("takes the workspace layer from a caller-supplied snapshot without reading the configuration", () => { + // The save path threads one captured inspection through every + // derivation, so the snapshot is authoritative: the configuration's own + // value must never be consulted a second time. + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["stale"] } }) + + const scopes = readCommandListScopes("deniedCommands", [], { workspaceValue: ["docker"] }) + + expect(scopes.workspace).toEqual(["docker"]) + expect(scopes.merged).toEqual(["docker"]) + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalled() + }) +}) + +describe("splitCommandListScopes", () => { + it("keeps workspace entries in the workspace layer and routes new entries to global", () => { + // The issue #1939 leak: the dialog sends the merged list back, and the + // workspace-only entry "docker" must not end up in the global write. + const result = splitCommandListScopes(["docker", "aws"], ["docker"], []) + + expect(result.workspaceWrite).toEqual(["docker"]) + expect(result.globalWrite).toEqual(["aws"]) + }) + + it("keeps a global entry in the global layer", () => { + const result = splitCommandListScopes(["git"], [], ["git"]) + + expect(result.workspaceWrite).toEqual([]) + expect(result.globalWrite).toEqual(["git"]) + }) + + it("keeps an entry present in both layers in both layers when retained", () => { + // Semantics: do not silently delete the user's global entry just + // because the current folder happens to define it too; the entry was + // deliberately authored in both scopes and the user kept it. + const result = splitCommandListScopes(["docker"], ["docker"], ["docker"]) + + expect(result.workspaceWrite).toEqual(["docker"]) + expect(result.globalWrite).toEqual(["docker"]) + }) + + it("removes a deleted entry from the workspace layer without touching global", () => { + // The user kept "git" and deleted workspace-only "docker"; "git" must + // survive at its own (global) layer. + const result = splitCommandListScopes(["git"], ["docker"], ["git"]) + + expect(result.workspaceWrite).toEqual([]) + expect(result.globalWrite).toEqual(["git"]) + }) + + it("removes a deleted entry from every layer that contained it", () => { + // "docker" lived in both layers and the user deleted it; kept "rm" + // stays at the workspace layer only. + const result = splitCommandListScopes(["rm"], ["docker", "rm"], ["docker"]) + + expect(result.workspaceWrite).toEqual(["rm"]) + expect(result.globalWrite).toEqual([]) + }) + + it("routes entries the dialog seeded to the layer they came from", () => { + const result = splitCommandListScopes( + ["git", "docker", "pnpm docker", "aws"], + ["docker", "pnpm docker"], + ["git"], + ) + + expect(result.workspaceWrite).toEqual(["docker", "pnpm docker"]) + expect(result.globalWrite).toEqual(["git", "aws"]) + }) + + it("orders the global write as existing global entries then new entries in dialog order", () => { + const result = splitCommandListScopes(["aws", "git", "npm"], ["docker"], ["git"]) + + expect(result.globalWrite).toEqual(["git", "aws", "npm"]) + }) + + it("drops blank and non-string entries from the incoming list", () => { + // Deliberate wrong runtime type: configuration lists arrive as unknown, + // so the sanitizer must drop non-strings instead of throwing on them. + const result = splitCommandListScopes(["git", "", " ", 42 as unknown as string], [], ["git"]) + + expect(result.globalWrite).toEqual(["git"]) + expect(result.workspaceWrite).toEqual([]) + }) + + it("deduplicates layers so re-writing cannot introduce duplicates", () => { + const result = splitCommandListScopes(["docker", "docker"], ["docker", "docker"], ["docker"]) + + expect(result.workspaceWrite).toEqual(["docker"]) + expect(result.globalWrite).toEqual(["docker"]) + }) +}) + +describe("resolveWorkspaceWriteTarget", () => { + beforeEach(() => { + vi.clearAllMocks() + workspaceState.workspaceFolders = undefined + }) + + it("returns the workspace-folder target when a folder value is set", () => { + mockConfiguration({ + inspect: { key: "deniedCommands", workspaceValue: ["docker"], workspaceFolderValue: ["pnpm docker"] }, + }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBe(vscode.ConfigurationTarget.WorkspaceFolder) + }) + + it("returns the workspace target when only a workspace value is set", () => { + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker"] } }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBe(vscode.ConfigurationTarget.Workspace) + }) + + it("returns undefined when only a global value is set, so no workspace layer is created", () => { + mockConfiguration({ inspect: { key: "deniedCommands", globalValue: ["npm"] } }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBeUndefined() + }) + + it("returns undefined when there is no inspect result", () => { + mockConfiguration({ inspect: undefined }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBeUndefined() + }) + + it("returns undefined when the configuration lookup throws", () => { + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { + throw new Error("configuration unavailable") + }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBeUndefined() + }) + + it("resolves the target from a caller-supplied snapshot without reading the configuration", () => { + // The snapshot alone decides the target, so a folder value the live + // configuration does not report still selects the folder target — + // keeping split, write, and rollback on one captured state. + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker"] } }) + + expect(resolveWorkspaceWriteTarget("deniedCommands", { workspaceFolderValue: ["pnpm docker"] })).toBe( + vscode.ConfigurationTarget.WorkspaceFolder, + ) + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalled() + }) + + it("resolves the target from the configuration scoped to the first workspace folder", () => { + const folderA = makeUri("/mock/folder-a") + const folderB = makeUri("/mock/folder-b") + workspaceState.workspaceFolders = [{ uri: folderA }, { uri: folderB }] + mockConfiguration({ inspect: { key: "deniedCommands", workspaceFolderValue: ["pnpm docker"] } }) + + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBe(vscode.ConfigurationTarget.WorkspaceFolder) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderA) + // Only the first folder is ever in scope: reading any other folder's + // configuration would silently target the wrong folder's settings file. + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalledWith(Package.name, folderB) + }) + + it.each([undefined, [] as { uri: unknown }[]])( + "falls back to an unscoped configuration when no workspace folder is open (%s)", + (workspaceFolders) => { + workspaceState.workspaceFolders = workspaceFolders + mockConfiguration({ inspect: { key: "deniedCommands", workspaceValue: ["docker"] } }) + + // With no resource to resolve against, no folder value can be + // reported, so the folder target stays unreachable. + expect(resolveWorkspaceWriteTarget("deniedCommands")).toBe(vscode.ConfigurationTarget.Workspace) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, undefined) + }, + ) +}) + +function mockGateConfiguration(gate: unknown) { + const config = makeWorkspaceConfiguration() + vi.mocked(config.get).mockReturnValue(gate) + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) + return config +} + +// Raw per-key layers in the shape readCommandListScopes returns them. +const listScopes = (globalList: string[], workspaceList: string[]) => ({ + global: globalList, + workspace: workspaceList, + merged: [...new Set([...globalList, ...workspaceList])], +}) + +it("workspaceCommandListsEnabled: enables the workspace layers only when the gate configuration reads true", () => { + vi.clearAllMocks() + const config = mockGateConfiguration(true) + + expect(workspaceCommandListsEnabled()).toBe(true) + expect(config.get).toHaveBeenCalledWith("workspaceCommandListsEnabled") +}) + +it("workspaceCommandListsEnabled: is off when the gate is unset", () => { + vi.clearAllMocks() + // Unset is the state of every installation that has never opted in, so it + // must read as disabled: treating unset as enabled would widen + // auto-approval by default. The equality against the declared default also + // keeps this expectation attached to the constant that documents the + // shipped default, not to an independently hard-coded boolean. + const config = mockGateConfiguration(undefined) + + expect(workspaceCommandListsEnabled()).toBe(false) + expect(workspaceCommandListsEnabled()).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + expect(config.get).toHaveBeenCalledWith("workspaceCommandListsEnabled") +}) + +it("workspaceCommandListsEnabled: keeps the workspace layers disabled when the gate lookup throws", () => { + vi.clearAllMocks() + // Fail-closed: a broken configuration read must never widen what the + // workspace layer can auto-approve. + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { + throw new Error("configuration unavailable") + }) + + expect(workspaceCommandListsEnabled()).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + expect(workspaceCommandListsEnabled()).toBe(false) +}) + +// The manifest declaration is what makes the gate workspace-unsettable; +// VS Code itself refusing a workspace-layer value is platform behavior. +it("manifest: declares the gate as a default-off, application-scoped boolean", () => { + const manifest = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../package.json"), "utf8")) + const gate = manifest.contributes.configuration.properties["zoo-code.workspaceCommandListsEnabled"] + + expect(gate.type).toBe("boolean") + expect(gate.default).toBe(false) + expect(gate.scope).toBe("application") +}) + +it("manifest: keeps the command-list keys workspace-settable so the gated layer can exist", () => { + // The gate scopes the workspace layers' effect; scoping the list keys + // themselves would remove the workspace layer entirely. + const manifest = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../package.json"), "utf8")) + const properties = manifest.contributes.configuration.properties + + expect(properties["zoo-code.allowedCommands"].scope).toBeUndefined() + expect(properties["zoo-code.deniedCommands"].scope).toBeUndefined() +}) + +it("persisted-settings schema: defaults the gate to disabled", () => { + expect(globalSettingsSchema.parse({}).workspaceCommandListsEnabled).toBeUndefined() + expect(globalSettingsSchema.parse({ workspaceCommandListsEnabled: true }).workspaceCommandListsEnabled).toBe(true) + expect(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED).toBe(false) +}) + +it("resolveEffectiveCommandLists: hides every workspace entry, allow and deny alike, while the gate is disabled", () => { + const effective = resolveEffectiveCommandLists(listScopes([], ["docker"]), listScopes([], ["rm"]), false) + + expect(effective.allowed).toEqual([]) + expect(effective.denied).toEqual([]) + expect(effective.hiddenWorkspaceAllow).toEqual(["docker"]) + expect(effective.hiddenWorkspaceDeny).toEqual(["rm"]) +}) + +it("resolveEffectiveCommandLists: treats unshadowed workspace entries as the sole layer when the global layer is empty", () => { + const effective = resolveEffectiveCommandLists(listScopes(["git"], ["docker", "aws"]), listScopes([], ["rm"]), true) + + expect(effective.allowed).toEqual(["git", "docker", "aws"]) + expect(effective.denied).toEqual(["rm"]) + expect(effective.hiddenWorkspaceAllow).toEqual([]) + expect(effective.hiddenWorkspaceDeny).toEqual([]) +}) + +it("resolveEffectiveCommandLists: lets a global allow entry decide a command a longer workspace deny entry also matches", () => { + // Cross-directional shadowing: the workspace deny entry cannot decide + // anything the global allow layer already covers, so it never reaches + // the matcher. + const effective = resolveEffectiveCommandLists(listScopes(["git"], []), listScopes([], ["git push"]), true) + + expect(effective.allowed).toEqual(["git"]) + expect(effective.denied).toEqual([]) + expect(effective.hiddenWorkspaceDeny).toEqual(["git push"]) +}) + +it("resolveEffectiveCommandLists: lets a global deny entry veto a longer workspace allow entry", () => { + const effective = resolveEffectiveCommandLists(listScopes([], ["git push"]), listScopes(["git"], []), true) + + expect(effective.allowed).toEqual([]) + expect(effective.denied).toEqual(["git"]) + expect(effective.hiddenWorkspaceAllow).toEqual(["git push"]) +}) + +it("resolveEffectiveCommandLists: silences every workspace deny entry behind a global wildcard allow entry", () => { + const effective = resolveEffectiveCommandLists(listScopes(["*"], []), listScopes([], ["git push", "rm -rf"]), true) + + expect(effective.allowed).toEqual(["*"]) + expect(effective.denied).toEqual([]) + expect(effective.hiddenWorkspaceDeny).toEqual(["git push", "rm -rf"]) +}) + +it("resolveEffectiveCommandLists: shadows workspace entries case-insensitively, the way the matcher matches", () => { + const effective = resolveEffectiveCommandLists( + listScopes(["Git"], ["git status"]), + listScopes([], ["DOCKER"]), + true, + ) + + expect(effective.allowed).toEqual(["Git"]) + expect(effective.denied).toEqual(["DOCKER"]) + expect(effective.hiddenWorkspaceAllow).toEqual(["git status"]) + expect(effective.hiddenWorkspaceDeny).toEqual([]) +}) + +// Command-branch enforcement composes its allow/deny pair from the raw +// global layers at the folder the task's cwd resolves to, so a task in +// another root is judged by that root's lists rather than the window's +// first folder. The window-scoped pair on the state stays what the dialog +// displays. + +function openMultiRootWindow() { + const folderA = makeUri("/mock/folder-a") + const folderB = makeUri("/mock/folder-b") + workspaceState.workspaceFolders = [{ uri: folderA }, { uri: folderB }] + workspaceState.foldersByPath.set(folderA.fsPath, folderA) + workspaceState.foldersByPath.set(folderB.fsPath, folderB) + return { folderA, folderB } +} + +// Scripts each folder's configuration independently so the test can tell +// which folder's settings the enforcement path actually read. +function mockFolderConfigurations(scripts: Record) { + vi.mocked(vscode.workspace.getConfiguration).mockImplementation( + (_section?: string, scope?: vscode.ConfigurationScope | null) => { + const entry = scripts[scope && "fsPath" in scope ? scope.fsPath : ""] + const config = makeWorkspaceConfiguration({ workspaceCommandListsEnabled: entry?.gate ?? false }) + vi.mocked(config.inspect).mockImplementation((key: string) => { + const layer = + key === "deniedCommands" ? entry?.deny : key === "allowedCommands" ? entry?.allow : undefined + return layer ? { key, workspaceFolderValue: layer } : undefined + }) + return config + }, + ) +} + +// Raw global layers empty: every enforcement decision then comes from the +// folder layer, so a decision can only be right for one folder. The baked +// pair on the state is set to values no folder composes, so a check that +// consumed it instead of re-composing decides differently. +const taskFolderState = (bakedAllowed: string[]): State => ({ + ...baseState, + alwaysAllowExecute: true, + allowedCommands: bakedAllowed, + deniedCommands: [], + globalAllowedCommands: [], + globalDeniedCommands: [], +}) + +it("checkAutoApproval command: enforces the deny entries of the folder the task's cwd resolves to", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + // Folder A denies "echo", folder B denies "rm"; the task runs in B. + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: ["echo"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + // The baked pair would approve exactly what folder B's layer denies. + const result = await checkAutoApproval({ + state: taskFolderState(["rm"]), + cwd: folderB.fsPath, + ask: "command", + text: "rm file", + }) + + expect(result).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "rm file", pattern: "rm" } }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderB) +}) + +it("checkAutoApproval command: does not apply another root's deny entries to a task outside it", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: ["echo"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + // "echo" is denied only in the first folder; judging this B-rooted task + // there would auto-deny it. Nothing allows it either, so the correct + // multi-root decision is the plain user prompt. + const result = await checkAutoApproval({ + state: taskFolderState(["echo"]), + cwd: folderB.fsPath, + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "ask" }) +}) + +it("checkAutoApproval command: does not apply the first folder's allow list to a cwd naming no workspace root", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + // Folder A's layer allows "echo"; the cwd names a removed or relocated + // folder, so no folder's layer may decide the command. Consuming folder A's + // allow entry here would approve a command executing outside every + // workspace folder: a cwd naming no workspace root must not be approvable + // by any folder's allow list — the composition is judged by the (empty) + // global layers only. + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: [], allow: ["echo"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + const result = await checkAutoApproval({ + state: taskFolderState([]), + cwd: "/mock/removed-folder", + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "ask" }) +}) + +it("checkAutoApproval command: composes a command without a cwd from the global entries and the first folder's list when both raw layers are present", async () => { + vi.clearAllMocks() + // With both raw layers set, the command branch re-composes instead of + // consuming the baked pair. An absent cwd keeps the first-folder + // configuration, so two independent witnesses apply: the raw global allow + // entry decides "echo hi" (a check that consumed the baked pair instead + // would find no match and fall to the plain prompt), and folder A's deny + // entry — about a command the global layers say nothing of — decides + // "date -u" (a composition that ignored the folder's configuration would + // have only the raw global layers to enforce and would fall to the plain + // prompt). The scope assertions pin which folder's configuration the + // composition read. + const { folderA, folderB } = openMultiRootWindow() + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: ["date"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + const state = { + ...taskFolderState(["baked"]), + globalAllowedCommands: ["echo"], + globalDeniedCommands: ["rm"], + } + + const globalWitness = await checkAutoApproval({ state, ask: "command", text: "echo hi" }) + expect(globalWitness).toEqual({ decision: "approve" }) + + // "date" is shadowed by neither global layer, so only folder A's list can + // deny it: the outcome names the folder whose configuration decided. + const folderWitness = await checkAutoApproval({ state, ask: "command", text: "date -u" }) + expect(folderWitness).toEqual({ + decision: "deny", + autoDeny: { kind: "denylist", command: "date -u", pattern: "date" }, + }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderA) + // Reading folder B instead would have judged the command against a folder + // the window never defaults to. + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalledWith(Package.name, folderB) +}) + +it("checkAutoApproval command: still lets a global deny entry veto a command whose cwd names no workspace root", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + // The global-only composition keeps the global layers authoritative: the + // unmatched cwd drops only the workspace layer, never the global deny. + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: ["rm"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + const result = await checkAutoApproval({ + state: { ...taskFolderState([]), globalDeniedCommands: ["echo"] }, + cwd: "/mock/removed-folder", + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "echo hi", pattern: "echo" } }) +}) + +it("checkAutoApproval command: keeps the first-folder composition for a blank cwd", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + // A blank cwd is the cwd-absent case, not the unmatched case: it stays on + // the first-folder configuration it used before per-folder scoping existed. + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: ["echo"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + const result = await checkAutoApproval({ + state: taskFolderState([]), + cwd: "", + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "echo hi", pattern: "echo" } }) +}) + +it("checkAutoApproval command: treats a throwing workspace-folder lookup as a cwd naming no folder", async () => { + vi.clearAllMocks() + const { folderA, folderB } = openMultiRootWindow() + // The lookup guard exists so a broken lookup cannot fail the decision; the + // degradation it produces must be the global-only composition, not a + // first-folder guess — the same fail-closed direction as an unmatched cwd. + // The stand-in throws exactly once, so a leak cannot change later tests' + // folder resolution. + workspaceState.throwsOnFoldersRead = true + mockFolderConfigurations({ + [folderA.fsPath]: { gate: true, deny: [], allow: ["echo"] }, + [folderB.fsPath]: { gate: true, deny: ["rm"] }, + }) + + const result = await checkAutoApproval({ + state: taskFolderState([]), + cwd: folderA.fsPath, + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "ask" }) +}) + +it("checkAutoApproval command: in a single-folder window reads exactly the same configuration as before", async () => { + vi.clearAllMocks() + const { folderA } = openMultiRootWindow() + workspaceState.workspaceFolders = [{ uri: folderA }] + workspaceState.foldersByPath.delete(makeUri("/mock/folder-b").fsPath) + // The one open folder is both the task folder and the first folder, so + // the resolved scope and the no-arg default are the same resource. + mockFolderConfigurations({ [folderA.fsPath]: { gate: true, deny: ["echo"] } }) + + const result = await checkAutoApproval({ + state: taskFolderState(["echo"]), + cwd: folderA.fsPath, + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "echo hi", pattern: "echo" } }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderA) +}) + +// Creating a directory symlink on Windows needs developer mode or elevation; a +// junction needs neither and is resolved by realpath like a POSIX link, so the +// fixture tries a symlink first and falls back to a junction. Where neither can +// be created the real-filesystem test is skipped; the scripted tests below pin +// the same resolution logic on every platform. +function createDirectoryLink(target: string, linkPath: string): void { + try { + fs.symlinkSync(target, linkPath, "dir") + } catch { + fs.symlinkSync(target, linkPath, "junction") + } +} + +function canCreateDirectoryLink(): boolean { + const probeDir = fs.mkdtempSync(path.join(os.tmpdir(), "command-scope-link-probe-")) + try { + const target = path.join(probeDir, "target") + fs.mkdirSync(target) + createDirectoryLink(target, path.join(probeDir, "link")) + return true + } catch { + return false + } finally { + fs.rmSync(probeDir, { recursive: true, force: true }) + } +} +const directoryLinksAvailable = canCreateDirectoryLink() + +// The root-side fallback runs after the registered paths matched nothing, so +// these tests control resolution through the double (the mock header explains +// the identity default) and keep the registry lists they expect to be read +// independent of resolution. Resetting here also keeps the double's scripted +// behavior from leaking into the manifest reads above and the file's other +// lookup tests. +afterEach(() => { + workspaceState.workspaceFolders = undefined + workspaceState.foldersByPath.clear() + vi.mocked(fs.realpathSync).mockImplementation((target) => String(target)) +}) + +it.skipIf(!directoryLinksAvailable)( + "checkAutoApproval command: applies a symlinked workspace root's folder layer to a cwd in the root's real path", + async () => { + vi.clearAllMocks() + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "command-scope-link-root-")) + try { + const realRoot = path.join(baseDir, "real-root") + const taskDir = path.join(realRoot, "sub") + fs.mkdirSync(realRoot) + fs.mkdirSync(taskDir) + const link = path.join(baseDir, "link-to-root") + createDirectoryLink(realRoot, link) + const linkUri = makeUri(link) + // The registry holds the link: the folder was opened through it, so + // the registered paths match nothing for a cwd the filesystem has + // already resolved, and only the root's real path connects the two. + workspaceState.workspaceFolders = [{ uri: linkUri }] + workspaceState.foldersByPath.set(link, linkUri) + // One delegate covers every call the lookup makes against this link. + vi.mocked(fs.realpathSync).mockImplementation((target) => realPath(String(target))) + // The configuration is scripted for the registry entry, not the + // real path: folder-scoped settings resolve against what VS Code + // opened. + mockFolderConfigurations({ [link]: { gate: true, allow: ["echo"] } }) + + const result = await checkAutoApproval({ + // Empty raw global layers: only the link folder's layer can + // decide. + state: taskFolderState([]), + cwd: realPath(taskDir), + ask: "command", + text: "echo hi", + }) + + expect(result).toEqual({ decision: "approve" }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, linkUri) + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }) + } + }, +) + +it("commandListFolderForResource: falls back to the registered path for a root whose real path cannot be resolved", () => { + vi.clearAllMocks() + const rootPath = path.join(path.sep + "mock", "broken-root") + const rootUri = makeUri(rootPath) + // A relocated or deleted root has no real path; comparing it by the path the + // registry holds keeps that folder's layer applied instead of silently + // dropping it. + workspaceState.workspaceFolders = [{ uri: rootUri }] + vi.mocked(fs.realpathSync).mockImplementation((target) => { + if (String(target) === rootPath) { + throw new Error("ENOENT: no such file or directory") + } + return String(target) + }) + + expect(commandListFolderForResource(path.join(rootPath, "sub"))).toBe(rootUri) +}) + +it("commandListFolderForResource: a path inside no root stays unmatched after the real-path retry", () => { + vi.clearAllMocks() + const rootPath = path.join(path.sep + "mock", "root") + workspaceState.workspaceFolders = [{ uri: makeUri(rootPath) }] + + // The sibling "root-two" shares "root" as a string prefix; only the + // separator guard between root and remainder keeps it outside the root. + expect(commandListFolderForResource(path.join(path.sep + "mock", "root-two", "x"))).toBeUndefined() + expect(commandListFolderForResource(path.join(path.sep + "mock", "elsewhere"))).toBeUndefined() +}) + +it("commandListFolderForResource: matches a cwd equal to the root's real path", () => { + vi.clearAllMocks() + workspaceState.foldersByPath.clear() + const rootPath = path.join(path.sep + "mock", "root") + const registeredPath = path.join(path.sep + "mock", "root-link") + const rootUri = makeUri(registeredPath) + workspaceState.workspaceFolders = [{ uri: rootUri }] + vi.mocked(fs.realpathSync).mockImplementation((target) => + String(target) === registeredPath ? rootPath : String(target), + ) + + expect(commandListFolderForResource(rootPath)).toBe(rootUri) +}) + +it("commandListFolderForResource: returns undefined when no workspace folder is open", () => { + vi.clearAllMocks() + workspaceState.foldersByPath.clear() + workspaceState.workspaceFolders = undefined + + expect(commandListFolderForResource(path.join(path.sep + "mock", "root"))).toBeUndefined() +}) + +it("commandListFolderForResource: compares the cwd to the root's real path case-sensitively", () => { + vi.clearAllMocks() + workspaceState.workspaceFolders = [{ uri: makeUri(path.join(path.sep + "mock", "Root")) }] + // Both sides arrive canonical, so their case agrees wherever the + // comparison applies; folding case here could match a root the filesystem + // treats as distinct. + expect(commandListFolderForResource(path.join(path.sep + "mock", "root", "sub"))).toBeUndefined() +}) + +it("commandListFolderForResource: returns the registry Uri of the deepest matching root", () => { + vi.clearAllMocks() + const outerRoot = path.sep + "mock" + const innerRegistered = path.join(outerRoot, "inner-real") + const innerReal = path.join(outerRoot, "inner") + const outerUri = makeUri(outerRoot) + const innerUri = makeUri(innerRegistered) + // Order matters: the outer root is seen first, so a lookup that returned + // the first match instead of the deepest one decides differently here. + workspaceState.workspaceFolders = [{ uri: outerUri }, { uri: innerUri }] + vi.mocked(fs.realpathSync).mockImplementation((target) => + String(target) === innerRegistered ? innerReal : String(target), + ) + + const result = commandListFolderForResource(path.join(innerReal, "sub")) + + // Object identity against the registry entry pins both halves: the deepest + // root won, and the value returned is the registered Uri (folder-scoped + // configuration resolves against the registry, not the real path). + expect(result).toBe(innerUri) + expect(result?.fsPath).toBe(innerRegistered) +}) + +it("checkAutoApproval command: judges a cwd inside a symlinked root by that root's layer when an outer root also contains it lexically", async () => { + vi.clearAllMocks() + const outerRoot = path.sep + "mock" + const outerUri = makeUri(outerRoot) + const linkPath = path.join(outerRoot, "link") + const innerReal = path.join(outerRoot, "inner") + const linkUri = makeUri(linkPath) + // The outer root is registered first, and the registry holds the inner + // folder as the link: the command's canonical cwd sits inside the link's + // target. The outer root is also a lexical prefix of the cwd, so + // VS Code-style lexical containment resolves it there — a lookup that + // trusted that first lexical hit would enforce the outer folder's list + // and never consult the folder the command physically runs in. + workspaceState.workspaceFolders = [{ uri: outerUri }, { uri: linkUri }] + workspaceState.foldersByPath.set(outerRoot, outerUri) + vi.mocked(fs.realpathSync).mockImplementation((target) => + String(target) === linkPath ? innerReal : String(target), + ) + // The two folders' lists are deliberately opposite on "echo": the outer + // root allows what the inner root denies, so the decision names the + // folder whose configuration enforcement read. + mockFolderConfigurations({ + [outerRoot]: { gate: true, allow: ["echo"] }, + [linkPath]: { gate: true, deny: ["echo"] }, + }) + + const result = await checkAutoApproval({ + // Empty raw global layers: only the winning folder's layer can decide. + state: taskFolderState([]), + cwd: path.join(innerReal, "sub"), + ask: "command", + text: "echo hi", + }) + + // The deepest canonical match wins over the shallower lexical containment, + // so the symlinked root's deny entry decides the command running inside it. + expect(result).toEqual({ decision: "deny", autoDeny: { kind: "denylist", command: "echo hi", pattern: "echo" } }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, linkUri) +}) + +it("realpathSync preserves the on-disk case of an existing directory", () => { + vi.clearAllMocks() + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "command-scope-case-")) + try { + const dirName = "MixedCase-Dir" + const dir = path.join(baseDir, dirName) + fs.mkdirSync(dir) + // The comparison trusts realpath output case, so it is pinned against + // the real filesystem rather than assumed: the final component keeps + // the case it was created with, and a differently-cased spelling of the + // same directory does not sit inside it. + const realBase = realPath(baseDir) + expect(realPath(dir)).toBe(path.join(realBase, dirName)) + expect(path.join(realBase, dirName.toLowerCase()).startsWith(`${realPath(dir)}${path.sep}`)).toBe(false) + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }) + } +}) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index ade53c2b20..87f6f44f5b 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -1,4 +1,10 @@ -import { containsDangerousSubstitution, getCommandDecision, getCommandDecisionDetailed } from "../commands" +import { resolveEffectiveCommandLists } from "../commandScope" +import { + containsDangerousSubstitution, + getCommandDecision, + getCommandDecisionDetailed, + isShadowedByAnyEntry, +} from "../commands" describe("containsDangerousSubstitution", () => { describe("zsh array assignments (should NOT be flagged)", () => { @@ -215,3 +221,82 @@ describe("getCommandDecisionDetailed", () => { } }) }) + +it("isShadowedByAnyEntry: shadows an entry whose every match a prefixing entry also matches", () => { + expect(isShadowedByAnyEntry("git push", ["git"])).toBe(true) + expect(isShadowedByAnyEntry("git push", ["git push"])).toBe(true) +}) + +it("isShadowedByAnyEntry: keeps an entry that decides matches no shadowing entry covers", () => { + expect(isShadowedByAnyEntry("git", ["git push"])).toBe(false) +}) + +it("isShadowedByAnyEntry: shadows every entry behind the universal wildcard", () => { + expect(isShadowedByAnyEntry("git push", ["*"])).toBe(true) + expect(isShadowedByAnyEntry("anything at all", ["*"])).toBe(true) +}) + +it("isShadowedByAnyEntry: compares case-insensitively, the way the matcher matches", () => { + expect(isShadowedByAnyEntry("Git Push", ["git"])).toBe(true) + expect(isShadowedByAnyEntry("git push", ["GIT"])).toBe(true) +}) + +it("isShadowedByAnyEntry: compares entries as stored, the way the matcher compares them", () => { + // The matcher trims commands, never entries, and shadowing mirrors that: + // both sides compare as stored. A trimmed comparison would let an inert + // padded entry erase a decidable one (and the reverse), deciding with a + // rule enforcement never applies. + expect(isShadowedByAnyEntry(" git push ", ["git"])).toBe(false) + expect(isShadowedByAnyEntry("git push", [" git "])).toBe(false) +}) + +it("layered decisions: a padded global deny entry cannot erase a workspace deny entry", () => { + // The padded global entry is inert at enforcement, so it must not shadow + // the workspace entry the matcher does honor: the deny survives. + const scopes = (globalList: string[], workspaceList: string[]) => ({ + global: globalList, + workspace: workspaceList, + merged: [...new Set([...globalList, ...workspaceList])], + }) + const effective = resolveEffectiveCommandLists(scopes([" git"], []), scopes([], ["git push"]), true) + + expect(getCommandDecision("git push origin", effective.allowed, effective.denied)).toBe("auto_deny") +}) + +it("layered decisions: a padded global allow entry cannot erase a workspace allow entry", () => { + // Mirror of the deny side: the inert padded entry leaves the workspace + // allow entry effective, so the command is approved rather than downgraded + // to a prompt for a match no entry can produce. + const scopes = (globalList: string[], workspaceList: string[]) => ({ + global: globalList, + workspace: workspaceList, + merged: [...new Set([...globalList, ...workspaceList])], + }) + const effective = resolveEffectiveCommandLists(scopes([" git"], ["git push"]), scopes([], []), true) + + expect(getCommandDecision("git push origin", effective.allowed, effective.denied)).toBe("auto_approve") +}) + +it("isShadowedByAnyEntry: never treats an empty entry as covered", () => { + // Compared naively, an empty entry prefix-matches every command, so + // against an empty shadow entry it would report itself covered. The + // empty-entry guard keeps such an entry visible instead. Command-list + // layers sanitize blanks before shadowing, so this pins the function's + // contract directly. + expect(isShadowedByAnyEntry("", [""])).toBe(false) + expect(isShadowedByAnyEntry("", ["git"])).toBe(false) +}) + +it("layered decisions: reproduces the global-decides precedence through the scope-blind matcher", () => { + // Global deny "git" vs workspace allow "git push": the composed lists + // drop the shadowed allow entry, so the scope-blind matcher — never told + // about layers — returns the global layer's deny. + const scopes = (globalList: string[], workspaceList: string[]) => ({ + global: globalList, + workspace: workspaceList, + merged: [...new Set([...globalList, ...workspaceList])], + }) + const effective = resolveEffectiveCommandLists(scopes([], ["git push"]), scopes(["git"], []), true) + + expect(getCommandDecision("git push origin", effective.allowed, effective.denied)).toBe("auto_deny") +}) diff --git a/src/core/auto-approval/commandScope.ts b/src/core/auto-approval/commandScope.ts new file mode 100644 index 0000000000..365756454c --- /dev/null +++ b/src/core/auto-approval/commandScope.ts @@ -0,0 +1,354 @@ +import fs from "fs" +import path from "path" + +import { DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED } from "@roo-code/types" +import * as vscode from "vscode" + +import { Package } from "../../shared/package" +import { isShadowedByAnyEntry } from "./commands" + +export type CommandListConfigKey = "allowedCommands" | "deniedCommands" + +export interface CommandListScopes { + global: string[] + workspace: string[] + merged: string[] +} + +/** + * The workspace-scoped layers of a command-list key as reported by + * `WorkspaceConfiguration.inspect()`. Capturing it once lets a save derive + * the read layers, the write target, and a rollback value from a single + * configuration state instead of three independent reads that could disagree. + */ +export interface CommandListInspection { + workspaceValue?: string[] + workspaceFolderValue?: string[] +} + +/** + * The configuration object for every command-list read and write. Scoped to + * `folderResource` when the caller resolved one — per-task enforcement reads + * the folder that task runs in — and to the first workspace folder otherwise, + * the default the display, save, and seeding callers keep. Sharing one scoped + * access path keeps the two sides symmetric: `inspect()` can report + * `workspaceFolderValue`, and `ConfigurationTarget.WorkspaceFolder` updates + * are legal (the API contract throws for that target when the configuration + * is not scoped to a resource). With no folder open — or when a supplied + * resource resolved to nothing, see {@link commandListFolderForResource} — + * the configuration is unscoped or defaults to the first folder and reports + * only what that lookup can see. + */ +export function commandListConfiguration(folderResource?: vscode.Uri): vscode.WorkspaceConfiguration { + return vscode.workspace.getConfiguration( + Package.name, + folderResource ?? vscode.workspace.workspaceFolders?.[0]?.uri, + ) +} + +/** + * Resolves the workspace folder a task's `cwd` lives in, so command-list + * enforcement scopes its configuration reads to the folder the task belongs + * to instead of the window's first folder. A child task inherits the working + * directory of the task it came from, so it resolves to the same folder + * without separate plumbing. + * + * The command path hands this lookup a cwd the filesystem has already + * resolved to its real path, while the workspace registry holds each root as + * it was opened. Containment is therefore tested against every root's real + * path, so a root that is itself a symlink (or a Windows junction) keeps + * matching its own real directory. Comparing against the registered paths + * alone could not decide the folder: that comparison is lexical, so an outer + * root lexically contains a directory that physically belongs to a deeper + * root, and enforcing the outer root's lists there would let the deeper + * folder's deny entries be skipped. Among several matches the deepest root + * wins, mirroring how VS Code's own containment picks the innermost folder. + * A root whose real path cannot be resolved is compared by its registered + * path: the cwd is already canonical, so an unresolvable root can only + * narrow which folder layer applies, never widen what a folder's allow list + * may approve — and dropping the layer outright would silently change + * enforcement for a folder the user has open. + * + * @param cwd - The task's working directory. An empty or absent cwd yields + * `undefined` and keeps the first-folder configuration. A nonempty cwd with + * no matching root also yields `undefined`. When both raw global lists are + * available, enforcement then excludes the workspace layer; otherwise, it + * uses the pre-composed state lists. + * @returns The containing folder's registry URI — never a canonicalized one, + * because folder-scoped configuration resolves against the registry — or + * `undefined` when cwd is empty, resolves to no root, or lookup fails. + */ +export function commandListFolderForResource(cwd?: string): vscode.Uri | undefined { + if (!cwd) { + return undefined + } + + // A failed lookup returns undefined, like a cwd naming no root. + // With both raw global lists available, enforcement excludes the workspace + // layer; otherwise, it uses the pre-composed state lists. + try { + // Comparing by root-path length makes the deepest match win: a root + // registered lexically above a symlinked root must not claim a command + // running in the deeper root's real directory. The separator guard + // keeps a sibling like `/root/other` from matching a root `/root`. + let match: vscode.Uri | undefined + let matchedLength = 0 + for (const candidate of vscode.workspace.workspaceFolders ?? []) { + const rootPath = realPathOrRegistered(candidate.uri.fsPath) + if ((cwd === rootPath || cwd.startsWith(rootPath + path.sep)) && rootPath.length > matchedLength) { + match = candidate.uri + matchedLength = rootPath.length + } + } + return match + } catch { + return undefined + } +} + +/** + * The path a workspace root is compared by: its real filesystem path, or — + * when that cannot be resolved (a relocated or deleted root) — the path the + * registry holds. Root-side lookups stay best-effort because the cwd being + * matched is already canonical; failing the comparison for one unresolvable + * root would drop that folder's command-list layer without widening any + * approval. + */ +function realPathOrRegistered(fsPath: string): string { + try { + return fs.realpathSync(fsPath) + } catch { + return fsPath + } +} + +/** + * Reads an auto-approval command list from both configuration layers and + * composes the list that should be enforced and displayed. + * + * The workspace layer is read via `inspect()` rather than `get()` on purpose: + * `get()` returns the effective value, which already folds `globalValue` into + * the result. Unioning that with the global layer would double-count entries + * and make removals from global settings unable to shrink the enforced set. + * + * @param configKey - The extension configuration key to read. + * @param globalList - The list persisted in extension global state. + * @param inspectionOverride - A snapshot captured by the caller before any + * write; when present it is used as-is and no fresh lookup happens, so a + * save's reads, target resolution, and rollback stay derived from one + * configuration state. Without it the lookup runs guarded (lenient): this is + * the enforcement/display path, where an unavailable workspace layer must + * degrade to empty rather than fail. + * @param folderResource - Workspace folder the configuration is scoped to; + * `undefined` keeps the first-folder default. + * @returns The per-layer lists and their deduplicated set union. + */ +export function readCommandListScopes( + configKey: CommandListConfigKey, + globalList: string[] | undefined, + inspectionOverride?: CommandListInspection, + folderResource?: vscode.Uri, +): CommandListScopes { + const global = sanitizeCommandList(globalList) + + let inspected: CommandListInspection | undefined = inspectionOverride + if (inspected === undefined) { + // Only the configuration lookup itself is guarded: it can fail (for + // example before the workspace is ready), and an absent workspace layer + // must not break enforcement. The value reads below have no failure mode + // of their own, so the catch covers exactly the failure it exists to + // absorb. + try { + inspected = commandListConfiguration(folderResource).inspect(configKey) + } catch { + inspected = undefined + } + } + + // The folder value wins over the workspace value, matching VS Code's + // folder-over-workspace precedence. An absent inspect result composes to + // an empty workspace layer: enforcement must not break. + const workspace = sanitizeCommandList(inspected?.workspaceFolderValue ?? inspected?.workspaceValue) + + // Set union: an entry present in both layers collapses to one, so + // composition cannot change prefix-match outcomes. + const merged = [...new Set([...global, ...workspace])] + + return { global, workspace, merged } +} + +export interface CommandListWriteScopes { + globalWrite: string[] + workspaceWrite: string[] +} + +/** + * Splits the merged command list the settings dialog saves back into the two + * configuration layers it was composed from, so saving the dialog cannot + * flatten workspace-scoped entries into the user's global settings. + * + * Semantics: + * - An entry the user kept that exists in the workspace layer stays in the + * workspace layer. If the same entry also exists in the global layer it + * stays there too: dropping it from the global write would silently delete + * a user's global entry just because the current folder happens to define + * it as well. + * - An entry the user removed is dropped from every layer that contained it, + * which makes workspace-scoped entries deletable from the dialog. + * - A brand-new entry (in neither layer) lands in the global layer, matching + * the pre-existing behavior of routing dialog edits to + * `ConfigurationTarget.Global`. + * + * @param incoming - The full merged list sent by the webview on save. + * @param workspaceLayer - The current workspace-scope configuration value. + * @param globalLayer - The list currently persisted in extension global state. + * @returns The per-layer values to write back. + */ +export function splitCommandListScopes( + incoming: string[], + workspaceLayer: string[], + globalLayer: string[], +): CommandListWriteScopes { + const kept = new Set(sanitizeCommandList(incoming)) + const workspace = sanitizeCommandList(workspaceLayer) + const global = sanitizeCommandList(globalLayer) + const known = new Set([...workspace, ...global]) + + // Deduplicated so re-writing a layer cannot introduce duplicates. + const workspaceWrite = [...new Set(workspace.filter((command) => kept.has(command)))] + const globalWrite = [ + ...new Set(global.filter((command) => kept.has(command))), + // Preserve the dialog's ordering for entries the user just added. + ...[...kept].filter((command) => !known.has(command)), + ] + + return { globalWrite, workspaceWrite } +} + +/** + * Picks the configuration target that owns the current workspace-scope layer + * for `configKey`. Reading the layers runs through the same + * `commandListConfiguration()` object `readCommandListScopes` uses; because + * that configuration is scoped to the first workspace folder, folder values + * resolve against that folder's settings; with no folder open the + * configuration is unscoped and reports no folder value, so no folder target + * can be selected. + * + * - `workspaceFolderValue` set -> write at `WorkspaceFolder` scope. + * - Otherwise `workspaceValue` set -> write at `Workspace` scope. + * - Neither set -> `undefined`: no workspace layer exists, so none should be + * created (an empty workspace layer must not materialize in settings.json). + * + * @param configKey - The extension configuration key to inspect. + * @param inspectionOverride - A snapshot captured by the caller; when present + * it alone decides the target and no fresh lookup happens, keeping a save's + * split, write, and rollback on one configuration state. Without it the + * lookup is guarded leniently: a failure means no workspace layer exists. + * @returns The workspace-scoped target to write, or `undefined` when no + * workspace layer exists. + */ +export function resolveWorkspaceWriteTarget( + configKey: CommandListConfigKey, + inspectionOverride?: CommandListInspection, +): vscode.ConfigurationTarget.WorkspaceFolder | vscode.ConfigurationTarget.Workspace | undefined { + let inspected: CommandListInspection | undefined = inspectionOverride + if (inspected === undefined) { + // Same defensive posture as readCommandListScopes: only the + // configuration lookup itself may fail, and a failure means we create no + // workspace layer. + try { + inspected = commandListConfiguration().inspect(configKey) + } catch { + inspected = undefined + } + } + + if (inspected?.workspaceFolderValue !== undefined) { + return vscode.ConfigurationTarget.WorkspaceFolder + } + if (inspected?.workspaceValue !== undefined) { + return vscode.ConfigurationTarget.Workspace + } + return undefined +} + +/** + * Reads the `zoo-code.workspaceCommandListsEnabled` gate that decides whether + * the workspace layers of the command-list keys take part in enforcement and + * display at all. The configuration declares the key with `scope: + * "application"`, so only user settings can flip it. The lookup is guarded + * leniently like the command-list reads: a configuration failure keeps the + * workspace layers inert, which cannot widen what auto-approval allows. + */ +export function workspaceCommandListsEnabled(): boolean { + try { + return commandListConfiguration().get("workspaceCommandListsEnabled") === true + } catch { + return DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED + } +} + +export interface EffectiveCommandLists { + allowed: string[] + denied: string[] + /** + * Workspace allow entries absent from the effective allowlist — hidden by + * the gate or shadowed by a global entry — which a dialog save must not be + * able to delete by omission. + */ + hiddenWorkspaceAllow: string[] + /** Deny-side counterpart of {@link hiddenWorkspaceAllow}. */ + hiddenWorkspaceDeny: string[] +} + +/** + * Composes the raw per-key layers into the pair of lists to enforce and + * display, keeping display byte-equal to enforcement. + * + * Gate off: the workspace layers contribute nothing and the global layers are + * effective verbatim. Gate on: a workspace entry survives only when no entry + * of either global list shadows it, so a global entry decides every command it + * matches while workspace entries apply where the global layer is silent. + * Shadowing is cross-directional because a global allow entry silences a + * workspace deny entry (and vice versa) just as much as a same-list shadow. + * + * Composing here — not inside {@link readCommandListScopes} — keeps the raw + * layers intact for the save path, which derives the write split from them and + * must never see entries filtered out of the workspace layer. + * + * @param allowScopes - Raw `allowedCommands` layers from readCommandListScopes. + * @param denyScopes - Raw `deniedCommands` layers from readCommandListScopes. + * @param enabled - The value of the `workspaceCommandListsEnabled` gate. + * @returns The effective allow/deny lists and the workspace entries hidden + * from each. + */ +export function resolveEffectiveCommandLists( + allowScopes: CommandListScopes, + denyScopes: CommandListScopes, + enabled: boolean, +): EffectiveCommandLists { + const globalEntries = [...allowScopes.global, ...denyScopes.global] + + const keptAllow = enabled + ? allowScopes.workspace.filter((entry) => !isShadowedByAnyEntry(entry, globalEntries)) + : [] + const keptDeny = enabled ? denyScopes.workspace.filter((entry) => !isShadowedByAnyEntry(entry, globalEntries)) : [] + + const allowed = [...new Set([...allowScopes.global, ...keptAllow])] + const denied = [...new Set([...denyScopes.global, ...keptDeny])] + + return { + allowed, + denied, + hiddenWorkspaceAllow: allowScopes.workspace.filter((entry) => !allowed.includes(entry)), + hiddenWorkspaceDeny: denyScopes.workspace.filter((entry) => !denied.includes(entry)), + } +} + +function sanitizeCommandList(list: unknown): string[] { + if (!Array.isArray(list)) { + return [] + } + + return list.filter((command): command is string => typeof command === "string" && command.trim().length > 0) +} diff --git a/src/core/auto-approval/commands.ts b/src/core/auto-approval/commands.ts index 4d28e0fe1b..d772bffb12 100644 --- a/src/core/auto-approval/commands.ts +++ b/src/core/auto-approval/commands.ts @@ -113,6 +113,34 @@ export function findLongestPrefixMatch(command: string, prefixes: string[]): str return longestMatch } +/** + * Whether one command-list entry is covered by any entry of `shadowing`. + * + * "Covered" applies the matcher's comparison — case-insensitive `startsWith` + * with `"*"` as universal prefix — directly to the two stored entries: only + * commands reach the matcher trimmed, entries never are, so an entry whose + * stored form carries leading whitespace matches no command and can shadow + * only other entries that match nothing either. An entry whose every possible + * match is also matched by some `shadowing` entry cannot decide anything on + * its own: that layer's decision stands. Dropping covered entries when + * composing layers reproduces a global-decides precedence without the matcher + * knowing about layers. + * + * @param entry - The entry that may be covered. + * @param shadowing - The entries whose match coverage decides. + * @returns true if some `shadowing` entry covers `entry`. An empty `entry` + * counts as covered only by `"*"`: the guard that stops it prefix-matching + * every command keeps a blank entry visible, as it cannot be enforced. + */ +export function isShadowedByAnyEntry(entry: string, shadowing: string[]): boolean { + const lowerEntry = entry.toLowerCase() + + return shadowing.some((shadow) => { + const lowerShadow = shadow.toLowerCase() + return lowerShadow === "*" || (lowerEntry !== "" && lowerEntry.startsWith(lowerShadow)) + }) +} + /** * Command approval decision types */ diff --git a/src/core/auto-approval/index.ts b/src/core/auto-approval/index.ts index 3539eef24d..de9bdcede1 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -15,6 +15,12 @@ import { ClineAskResponse } from "../../shared/WebviewMessage" import { isWriteToolAction, isReadOnlyToolAction } from "./tools" import { isMcpToolAlwaysAllowed } from "./mcp" import { containsDangerousSubstitution, getCommandDecisionDetailed } from "./commands" +import { + commandListFolderForResource, + readCommandListScopes, + resolveEffectiveCommandLists, + workspaceCommandListsEnabled, +} from "./commandScope" import { isFileMatchedByPatterns } from "./filePatterns" import { type AutoDenyDetail } from "./autoDenyReason" @@ -40,6 +46,8 @@ export type AutoApprovalStateOptions = | "mcpServers" // For `alwaysAllowMcp`. | "allowedCommands" // For `alwaysAllowExecute`. | "deniedCommands" + | "globalAllowedCommands" // Raw global layer feeding the per-folder command-list composition. + | "globalDeniedCommands" | "destructiveCommandGuardEnabled" | "alwaysDenyUnapprovedCommands" // For `alwaysAllowExecute` (blanket auto-deny). @@ -170,8 +178,11 @@ export async function checkAutoApproval({ * Workspace root the allowlist patterns and the checked path are resolved * against. * - * Must be the `cwd` of the task this ask belongs to, which is the root the - * path in `text` was made relative to. It is not read from `state`, because + * For every ask but a command, this is the `cwd` of the task the ask belongs + * to — the root the path in `text` was made relative to. A command ask is + * judged against the command's own resolved working directory when the + * caller forwards `commandCwd`, which a custom working directory moves off + * the task folder. It is not read from `state`, because * the provider's `cwd` follows the window (the focused editor in a multi-root * workspace, or a `refreshWorkspace()` while the task runs) and a resumed or * child task can run against another root entirely. Resolving against the @@ -304,10 +315,44 @@ export async function checkAutoApproval({ return { decision: "approve" } } + // The pair on `state` was composed against the window's first + // folder; a command executing in another folder must be judged by + // that folder's lists, so enforcement re-composes from the raw + // global layers at the folder the command's `cwd` resolves to. A + // non-blank `cwd` that resolves to no workspace root names a + // directory outside every folder, and no folder's layer may decide + // it: the workspace layer is dropped (`enabled=false`) and only the + // global layers apply — fail-closed, so nothing on a folder's + // allow list can approve a command in a directory no folder + // claims. A blank or absent `cwd` keeps the first-folder + // configuration. Without the raw layers the pre-composed pair is + // all the state carries, and that pair is what enforcement + // consumes. + const commandFolder = commandListFolderForResource(cwd) + const cwdNamesNoFolder = Boolean(cwd) && commandFolder === undefined + const effectiveLists = + state.globalAllowedCommands !== undefined && state.globalDeniedCommands !== undefined + ? resolveEffectiveCommandLists( + readCommandListScopes( + "allowedCommands", + state.globalAllowedCommands, + undefined, + commandFolder, + ), + readCommandListScopes( + "deniedCommands", + state.globalDeniedCommands, + undefined, + commandFolder, + ), + workspaceCommandListsEnabled() && !cwdNamesNoFolder, + ) + : { allowed: state.allowedCommands || [], denied: state.deniedCommands || [] } + const { decision, offendingCommand, matchedPattern, parseError } = getCommandDecisionDetailed( text, - state.allowedCommands || [], - state.deniedCommands || [], + effectiveLists.allowed, + effectiveLists.denied, ) if (decision === "auto_approve") { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index b911479bc6..ab71f40053 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1287,10 +1287,12 @@ export class Task extends EventEmitter implements TaskLike { text, isProtected, dcgDecision, + commandCwd, }: { text?: string isProtected?: boolean dcgDecision?: AutoApprovalContext["dcgDecision"] + commandCwd?: AutoApprovalContext["commandCwd"] }, abortSignal?: AbortSignal, ): Promise { @@ -1319,11 +1321,12 @@ export class Task extends EventEmitter implements TaskLike { // 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. + // Engaged: the full policy decides, with the fresh state. `cwd`, the + // forwarded DCG verdict, and the command's working directory match the + // ask-time `checkAutoApproval` call. const approval = await checkAutoApproval({ state: freshState, - cwd: this.cwd, + cwd: commandCwd ?? this.cwd, ask: "command", text, isProtected, @@ -1984,12 +1987,15 @@ export class Task extends EventEmitter implements TaskLike { // `this.cwd`, not `provider.cwd`: // The path inside `text` was made relative to this task's workspace, // which for a resumed or child task need not be the one the provider - // currently reports. + // currently reports. A tool that forwards a `commandCwd` overrides that + // for policy purposes: the command lists must judge the directory the + // command will actually run in, which a custom tool `cwd` moves off the + // task folder. const approval = queuedAskResolution ? ({ decision: "ask" } as const) : await checkAutoApproval({ state, - cwd: this.cwd, + cwd: autoApprovalContext?.commandCwd ?? this.cwd, ask: type, text, isProtected, @@ -2262,6 +2268,7 @@ export class Task extends EventEmitter implements TaskLike { text, isProtected, dcgDecision: autoApprovalContext?.dcgDecision, + commandCwd: autoApprovalContext?.commandCwd, }, recheckAbort.signal, ) @@ -2335,6 +2342,7 @@ export class Task extends EventEmitter implements TaskLike { text, isProtected, dcgDecision: autoApprovalContext?.dcgDecision, + commandCwd: autoApprovalContext?.commandCwd, }, recheckAbort.signal, ) diff --git a/src/core/task/__tests__/ask-allowlist-cwd.spec.ts b/src/core/task/__tests__/ask-allowlist-cwd.spec.ts index 3b3682f969..be3f855616 100644 --- a/src/core/task/__tests__/ask-allowlist-cwd.spec.ts +++ b/src/core/task/__tests__/ask-allowlist-cwd.spec.ts @@ -4,6 +4,55 @@ import type { ExtensionState } from "@roo-code/types" import { Task } from "../Task" +// Command-list enforcement resolves the command's cwd to a workspace folder and +// reads that folder's configuration; the shared module mock resolves every +// lookup to nothing, so these scriptable stand-ins (wrapping the real mock) are +// what let a command ask be judged by a named folder's allow/deny entries. +const workspaceState = vi.hoisted(() => ({ + workspaceFolders: [] as { fsPath: string; uri: { fsPath: string } }[], + foldersByPath: new Map(), + configScripts: {} as Record, +})) + +vi.mock("vscode", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + workspace: { + ...actual.workspace, + get workspaceFolders() { + return workspaceState.workspaceFolders + }, + getConfiguration: (_section?: string, scope?: { fsPath?: string } | null) => { + const entry = workspaceState.configScripts[scope?.fsPath ?? ""] + return { + get: (key: string, defaultValue?: unknown) => + key === "workspaceCommandListsEnabled" ? (entry?.gate ?? false) : defaultValue, + inspect: (key: string) => { + const layer = + key === "deniedCommands" + ? entry?.deny + : key === "allowedCommands" + ? entry?.allow + : undefined + return layer ? { key, workspaceFolderValue: layer } : undefined + }, + update: () => Promise.resolve(), + } + }, + getWorkspaceFolder: (resource?: { fsPath?: string }) => { + const fsPath = resource?.fsPath ?? "" + for (const [folderPath, uri] of workspaceState.foldersByPath) { + if (fsPath === folderPath || fsPath.startsWith(`${folderPath}/`)) { + return { uri } + } + } + return undefined + }, + }, + } +}) + // The allowlist patterns are resolved against a workspace root, and the path in // the tool message was made relative by `getReadablePath(task.cwd, ...)`. So the // root used for matching has to be the task's own `cwd`. @@ -103,3 +152,188 @@ describe("Task.ask resolves allowlists against the task's workspace", () => { expect(await askToWriteRelativePath([`${PROVIDER_CWD}/notes.md`])).toBe("ask") }) }) + +// Command asks: the per-folder command lists must judge the directory the +// command will actually run in (`commandCwd` from the tool's approval +// context), not the task folder — a task in folder A must not be able to ride +// folder A's allow entry to run a command in folder B. The raw global layers +// are set empty on the state so every decision below comes from the folder the +// command's cwd resolves to; the baked pair carries values no folder composes, +// so a check that consumed it instead of re-composing decides differently. + +const FOLDER_A = "/mock/folder-a" +const FOLDER_B = "/mock/folder-b" + +function openTwoFolders() { + const folderA = { fsPath: FOLDER_A, uri: { fsPath: FOLDER_A } } + const folderB = { fsPath: FOLDER_B, uri: { fsPath: FOLDER_B } } + workspaceState.workspaceFolders = [folderA, folderB] + workspaceState.foldersByPath.set(FOLDER_A, folderA.uri) + workspaceState.foldersByPath.set(FOLDER_B, folderB.uri) + // Folder A allows "echo" and folder B denies it, so a decision names the + // folder that was read. + workspaceState.configScripts = { + [FOLDER_A]: { gate: true, allow: ["echo"] }, + [FOLDER_B]: { gate: true, deny: ["echo"] }, + } +} + +const commandState = (extra: Partial = {}): Partial => ({ + autoApprovalEnabled: true, + alwaysAllowExecute: true, + allowedCommands: ["echo"], + deniedCommands: [], + globalAllowedCommands: [], + globalDeniedCommands: [], + ...extra, +}) + +/** Ask to run a command with the given approval context and report the policy decision. */ +const askToRunCommand = async (commandCwd: string | undefined, state: Partial) => { + const provider: ProviderStub = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + cwd: PROVIDER_CWD, + getState: async () => state, + } + // The task runs in folder A (whose allow entry must NOT approve a command + // executing in folder B) — the task cwd equals folder A here. + const task = buildTask(provider, FOLDER_A) + await attachQueue(task) + + const asked = task.ask("command", "echo hi", false, undefined, false, { commandCwd }) + + const addToClineMessages = task["addToClineMessages"] as ReturnType + await vi.waitUntil(() => addToClineMessages.mock.calls.length > 0) + const message = addToClineMessages.mock.calls[0][0] + + task.approveAsk() + await asked + + return message.autoApprovalDecision ?? ("ask" as const) +} + +describe("Task.ask judges command asks by the command's working directory", () => { + beforeEach(() => { + workspaceState.workspaceFolders = [] + workspaceState.foldersByPath.clear() + workspaceState.configScripts = {} + }) + + it("approves a command whose custom cwd resolves to the folder that allows it", async () => { + openTwoFolders() + // Only folder B carries the entry, so approval can only come from the + // folder the command runs in. + workspaceState.configScripts[FOLDER_A] = { gate: true } + workspaceState.configScripts[FOLDER_B] = { gate: true, allow: ["echo"] } + + expect(await askToRunCommand(FOLDER_B, commandState())).toBe("approve") + }) + + it("does not approve a command executing where the same entry is denied, although the task folder allows it", async () => { + openTwoFolders() + + // The task folder (A) allows "echo"; the command runs in B, which denies + // it. The deny layer must decide. + expect(await askToRunCommand(FOLDER_B, commandState())).toBe("deny") + }) + + it("judges a custom cwd that names no workspace folder by the global layers only", async () => { + openTwoFolders() + + // The baked pair would approve, but with the raw layers present the + // command branch re-composes: an unmatched cwd drops the workspace + // layer entirely, so nothing allows or denies "echo" and the user decides. + expect(await askToRunCommand("/mock/removed-folder", commandState())).toBe("ask") + }) + + it("keeps judging a command without a custom cwd by the task folder's policy", async () => { + openTwoFolders() + + // No approval context at all: the task folder's own entry still decides. + expect(await askToRunCommand(undefined, commandState())).toBe("approve") + }) + + it("re-checks a drained queued command against the command's working directory", async () => { + openTwoFolders() + const provider: ProviderStub = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + cwd: PROVIDER_CWD, + // First read (ask time): blanket deny off, execute auto-approval off, + // so the ask stays pending. Second read (drain re-check): blanket + // deny engaged, so the queued message must re-earn its approval. + getState: vi + .fn() + .mockResolvedValueOnce(commandState({ alwaysAllowExecute: false, alwaysDenyUnapprovedCommands: false })) + .mockResolvedValue(commandState({ alwaysDenyUnapprovedCommands: true })), + } + // The task is rooted in folder B (which denies "echo") while the command + // runs in folder A (which allows it): a re-check that fell back to the + // task folder would deny the queued approval the command's own folder + // grants. + const task = buildTask(provider, FOLDER_B) + await attachQueue(task) + + const asked = task.ask("command", "echo hi", false, undefined, false, { commandCwd: FOLDER_A }) + await vi.waitUntil(() => (task["addToClineMessages"] as ReturnType).mock.calls.length > 0) + task.messageQueueService.addMessage("run it") + + // Folder A allows "echo"; had the re-check judged the task-independent + // defaults or the baked pair, the blanket deny would reject the queued + // approval. The folder the command runs in approves it. + const result = await asked + + expect(result.response).toBe("yesButtonClicked") + expect(result.autoDenyDetail).toBeUndefined() + }) + + it("re-checks a queued command drained at ask entry with no approval context", async () => { + openTwoFolders() + // A message already queued when the ask opens, with the claim gate open + // (blanket deny off): the ENTRY drain (not the later pWaitFor one) + // performs the policy re-check, and folder A's entry approves it. + const provider: ProviderStub = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + cwd: PROVIDER_CWD, + getState: async () => commandState({ alwaysDenyUnapprovedCommands: false }), + } + const task = buildTask(provider, FOLDER_A) + await attachQueue(task) + task.messageQueueService.addMessage("run it") + + // No approval context at all: the entry re-check must run cleanly + // against the task folder and approve via folder A's entry. A throw + // there is caught and logged (then the pWaitFor drain would paper over + // it), so the log is the witness. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const result = await task.ask("command", "echo hi", false) + + expect(result.response).toBe("yesButtonClicked") + expect(result.autoDenyDetail).toBeUndefined() + expect(errorSpy).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it("denies a drained queued command whose working directory the policy denies, although the blanket path would accept it", async () => { + openTwoFolders() + const provider: ProviderStub = { + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + cwd: PROVIDER_CWD, + getState: vi + .fn() + .mockResolvedValueOnce(commandState({ alwaysAllowExecute: false, alwaysDenyUnapprovedCommands: false })) + .mockResolvedValue(commandState({ alwaysDenyUnapprovedCommands: true })), + } + // The task folder (A) allows; the command runs in B, which denies. + const task = buildTask(provider, FOLDER_A) + await attachQueue(task) + + const asked = task.ask("command", "echo hi", false, undefined, false, { commandCwd: FOLDER_B }) + await vi.waitUntil(() => (task["addToClineMessages"] as ReturnType).mock.calls.length > 0) + task.messageQueueService.addMessage("run it") + + const result = await asked + + expect(result.response).toBe("noButtonClicked") + expect(result.autoDenyDetail?.kind).toBe("denylist") + }) +}) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 9855e6e417..98c6c0a128 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -156,13 +156,45 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { return } const provider = await task.providerRef.deref() + // One canonical working directory, resolved before any approval + // decision: the command-list policy, the DCG run, and the terminal + // spawn must all see the same directory, so a custom `cwd` can never + // be approved against one folder and executed in another. + const lexicalWorkingDirectory = customCwd + ? path.isAbsolute(customCwd) + ? customCwd + : path.resolve(task.cwd, customCwd) + : task.cwd + // A folder's command lists apply to a command by containment: the + // working directory is matched against the open roots, and against + // each root's real path when the registered paths match nothing, so + // a root that is itself a link still claims its own directory. + // Canonicalizing here keeps both sides of that match physical, so + // the judged folder is the one containing the directory the process + // actually starts in — a symlinked directory would otherwise be + // judged by the folder its path sits in while the process starts in + // the link's target, the same drift in filesystem form (precedent: + // `RooIgnoreController.validateAccess` also resolves symlinks with + // realpath before a policy check). Unlike that file-access posture, + // an unresolvable link here names no directory any policy could + // judge, so the command fails closed through the working-directory + // error channel rather than falling back to the lexical path. A link + // re-pointed between this call and the spawn cannot be closed from + // here; it rides the same accepted race class as a settings flip + // during a pending prompt. + let workingDirectory: string + try { + workingDirectory = await fs.realpath(lexicalWorkingDirectory) + } catch (error) { + const resolutionError = + (await commandWorkingDirectoryError(lexicalWorkingDirectory)) ?? + `Unable to resolve working directory '${lexicalWorkingDirectory}': ${error instanceof Error ? error.message : String(error)}` + task.didToolFailInCurrentTurn = true + pushToolResult(formatResponse.toolError(resolutionError)) + return + } let dcgDecision: DcgDecision | undefined if (provider?.contextProxy.getValue("destructiveCommandGuardEnabled") === true) { - const workingDirectory = customCwd - ? path.isAbsolute(customCwd) - ? customCwd - : path.resolve(task.cwd, customCwd) - : task.cwd const workingDirectoryError = await commandWorkingDirectoryError(workingDirectory) if (workingDirectoryError) { task.didToolFailInCurrentTurn = true @@ -214,13 +246,23 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { // choose whether to execute it. let didApprove: boolean if (dcgDecision === undefined) { - didApprove = await askApproval("command", canonicalCommand) + didApprove = await askApproval("command", canonicalCommand, undefined, false, { + commandCwd: workingDirectory, + }) } else if (dcgDecision.decision === "allow") { - didApprove = await askApproval("command", canonicalCommand, undefined, false, { dcgDecision }) + didApprove = await askApproval("command", canonicalCommand, undefined, false, { + dcgDecision, + commandCwd: workingDirectory, + }) } else if (blanketAutoDeny) { - didApprove = await askApproval("command", canonicalCommand, undefined, false, { dcgDecision }) + didApprove = await askApproval("command", canonicalCommand, undefined, false, { + dcgDecision, + commandCwd: workingDirectory, + }) } else { - didApprove = await askApproval("command", canonicalCommand, undefined, isProtectedAsk) + didApprove = await askApproval("command", canonicalCommand, undefined, isProtectedAsk, { + commandCwd: workingDirectory, + }) } if (!didApprove) { @@ -257,7 +299,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { // whose own approval was legal, inverting approval semantics. const recheck = await checkAutoApproval({ state: freshState, - cwd: task.cwd, + cwd: workingDirectory, ask: "command", text: canonicalCommand, isProtected: false, @@ -330,6 +372,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { executionId, command: canonicalCommand, customCwd, + workingDirectory, terminalShellIntegrationDisabled, commandExecutionTimeout, agentTimeout, @@ -394,6 +437,14 @@ export type ExecuteCommandOptions = { executionId: string command: string customCwd?: string + /** + * The directory the approval was judged against, resolved once by the + * tool. When present the command spawns there directly, so the executed + * directory cannot drift from the one the command policy approved; + * callers that build options without it fall back to resolving + * `customCwd` against the task's own `cwd` here. + */ + workingDirectory?: string terminalShellIntegrationDisabled?: boolean commandExecutionTimeout?: number agentTimeout?: number @@ -406,6 +457,7 @@ export async function executeCommandInTerminal( executionId, command, customCwd, + workingDirectory, terminalShellIntegrationDisabled = true, commandExecutionTimeout = 0, agentTimeout = 0, @@ -415,7 +467,9 @@ export async function executeCommandInTerminal( // Convert milliseconds back to seconds for display purposes. const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000 let workingDir: string - if (!customCwd) { + if (workingDirectory !== undefined) { + workingDir = workingDirectory + } else if (!customCwd) { workingDir = task.cwd } else if (path.isAbsolute(customCwd)) { workingDir = customCwd diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index c580b55102..f14019dcc3 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -2,7 +2,9 @@ import type { ToolUsage } from "@roo-code/types" import fs from "fs/promises" -import { constants as fsConstants, type Stats } from "fs" +import { constants as fsConstants, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, type Stats } from "fs" +import * as os from "os" +import * as path from "path" import * as vscode from "vscode" import { Task } from "../../task/Task" @@ -23,6 +25,10 @@ vitest.mock("fs/promises", () => ({ default: { access: vitest.fn().mockResolvedValue(undefined), stat: vitest.fn().mockResolvedValue({ isDirectory: () => true }), + // The tool canonicalizes the resolved working directory before any + // approval decision; the default double resolves every path to itself, + // so a test only sees symlink behavior when it scripts this mock. + realpath: vitest.fn().mockImplementation((p: string) => Promise.resolve(p)), }, })) @@ -78,6 +84,34 @@ const { executeCommandTool } = executeCommandModule // capturing inside a test can capture another test's leaked spy and recurse. const realCheckAutoApproval = autoApprovalModule.checkAutoApproval +// Creating a directory symlink on Windows needs developer mode or elevation; a +// junction needs neither and is resolved by `realpath` like a POSIX link, so the +// fixture tries a symlink first and falls back to a junction. Where neither can +// be created the fixture is skipped: the real-filesystem behavior it pins cannot +// be produced by the file-mock module this suite otherwise runs on. +function createDirectoryLink(target: string, linkPath: string): void { + try { + symlinkSync(target, linkPath, "dir") + } catch { + symlinkSync(target, linkPath, "junction") + } +} + +function canCreateDirectoryLink(): boolean { + const probeDir = mkdtempSync(path.join(os.tmpdir(), "execute-command-symlink-probe-")) + try { + const target = path.join(probeDir, "target") + mkdirSync(target) + createDirectoryLink(target, path.join(probeDir, "link")) + return true + } catch { + return false + } finally { + rmSync(probeDir, { recursive: true, force: true }) + } +} +const directoryLinksAvailable = canCreateDirectoryLink() + describe("executeCommandTool", () => { // Setup common test variables let mockCline: any & { consecutiveMistakeCount: number; didRejectTool: boolean } @@ -206,7 +240,9 @@ describe("executeCommandTool", () => { }) // Verify - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test") + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + commandCwd: "/test/workspace", + }) expect(mockPushToolResult).toHaveBeenCalled() // The exact message depends on the terminal mock's behavior const result = mockPushToolResult.mock.calls[0][0] @@ -228,12 +264,185 @@ describe("executeCommandTool", () => { }) // Verify - command approved, result pushed, and custom cwd passed to terminal - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test") + // The approval context carries the same directory the terminal will + // spawn in, so the folder-scoped command policy judges that folder. + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + commandCwd: "/custom/path", + }) expect(mockPushToolResult).toHaveBeenCalled() const { TerminalRegistry } = await import("../../../integrations/terminal/TerminalRegistry") const firstArg = (TerminalRegistry.getOrCreateTerminal as ReturnType).mock.calls[0][0] expect(firstArg).toBe("/custom/path") }) + + it("reports a relative custom cwd resolved against the task's workspace", async () => { + // A relative custom cwd runs the command in a subdirectory of the task + // folder, so the approval must name the resolved absolute path: the + // policy lookup resolves against workspace folders, which a relative + // path never names. + mockToolUse.params.command = "echo test" + mockToolUse.params.cwd = "sub/dir" + mockToolUse.nativeArgs = { command: "echo test", cwd: "sub/dir" } + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + expect(mockAskApproval).toHaveBeenCalledWith( + "command", + "echo test", + undefined, + false, + expect.objectContaining({ + commandCwd: path.resolve("/test/workspace", "sub/dir"), + }), + ) + }) + }) + + // The folder-scoped command policy resolves a directory by string + // containment, so a link would be judged by the folder it sits in while the + // process starts in the link's target. The tool canonicalizes the resolved + // working directory once, before any approval decision, so every consumer + // (ask, DCG, re-checks, spawn) sees the physical directory. + describe("Working directory canonicalization", () => { + it("forwards the physical directory behind a symlinked custom cwd to the approval ask", async () => { + mockToolUse.params.command = "echo test" + mockToolUse.params.cwd = "/mock/link" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/mock/link" } + vi.mocked(fs.realpath).mockResolvedValueOnce("/physical/target") + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + expect(fs.realpath).toHaveBeenCalledWith("/mock/link") + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + commandCwd: "/physical/target", + }) + }) + + it("runs the destructive-command guard in the physical directory behind a symlinked custom cwd", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + mockToolUse.params.cwd = "/mock/link" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/mock/link" } + vi.mocked(fs.realpath).mockResolvedValueOnce("/physical/target") + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + expect(mockRunDcg).toHaveBeenCalledWith("/test/storage/dcg", "echo test", "/physical/target") + }) + + it("fails closed on a broken symlink without reaching approval", async () => { + mockToolUse.params.cwd = "/mock/broken-link" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/mock/broken-link" } + vi.mocked(fs.realpath).mockRejectedValueOnce(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + vi.mocked(fs.stat).mockRejectedValueOnce(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + vi.mocked(formatResponse.toolError).mockReturnValueOnce("formatted broken-link error") + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + // The realpath failure is reported through the working-directory + // error channel, which names the directory by its validated state. + expect(formatResponse.toolError).toHaveBeenCalledExactlyOnceWith( + "Working directory '/mock/broken-link' does not exist.", + ) + expect(mockPushToolResult).toHaveBeenCalledExactlyOnceWith("formatted broken-link error") + expect(mockAskApproval).not.toHaveBeenCalled() + expect(mockCline.didToolFailInCurrentTurn).toBe(true) + }) + + it.each([ + { thrown: Object.assign(new Error("ELOOP: too many symbolic links"), { code: "ELOOP" }) }, + { thrown: "ELOOP: too many symbolic links" }, + ])( + "fails closed when canonicalization fails although the lexical directory validates (%s)", + async ({ thrown }) => { + mockToolUse.params.cwd = "/mock/loop" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/mock/loop" } + vi.mocked(fs.realpath).mockRejectedValueOnce(thrown) + + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + // A link that cannot be resolved names no directory any policy + // could judge, even when the lexical `stat` check would pass: the + // fallback message is the witness that the error came from + // canonicalization, and it carries the failure reason whatever + // shape the rejection has. + expect(formatResponse.toolError).toHaveBeenCalledExactlyOnceWith( + expect.stringContaining("Unable to resolve working directory '/mock/loop': ELOOP"), + ) + expect(mockAskApproval).not.toHaveBeenCalled() + expect(mockCline.didToolFailInCurrentTurn).toBe(true) + }, + ) + + // The scripted-realpath tests above pin the tool's contract; this one + // pins it against a real link: the folder policy resolves a path by + // string containment (already covered against the canonical value), so + // the physical path reaching `commandCwd` is what makes a link inside one + // folder judged by the folder of its target. + it.skipIf(!directoryLinksAvailable)( + "forwards the real target of a directory link created on disk", + async () => { + const baseDir = mkdtempSync(path.join(os.tmpdir(), "execute-command-canonicalize-")) + try { + const folderA = path.join(baseDir, "folder-a") + const folderB = path.join(baseDir, "folder-b") + mkdirSync(folderA) + mkdirSync(folderB) + const link = path.join(folderA, "link") + createDirectoryLink(folderB, link) + // The module mock resolves paths through this delegate, so the + // tool sees the same physical path the filesystem reports. + // Once-only: the tool canonicalizes exactly one path per call, + // and a leaked delegate would resolve later tests' fake paths + // against the real filesystem. + vi.mocked(fs.realpath).mockImplementationOnce(async (target) => realpathSync(String(target))) + + mockToolUse.params.cwd = link + mockToolUse.nativeArgs = { command: "echo test", cwd: link } + await executeCommandTool.handle(mockCline as Task, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + + // Compared against the independently resolved target rather + // than a literal: temporary-directory paths themselves contain + // links on some platforms. + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + commandCwd: realpathSync(folderB), + }) + } finally { + rmSync(baseDir, { recursive: true, force: true }) + } + }, + ) }) describe("Error handling", () => { @@ -342,7 +551,9 @@ describe("executeCommandTool", () => { "error", "executeCommand.destructiveCommandGuard.blockedWithReasonAndRule", ) - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/test/workspace", + }) }) it("requests normal approval when DCG allows the command", async () => { @@ -366,6 +577,7 @@ describe("executeCommandTool", () => { // the verdict itself rather than inferring it from settings alone. expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { dcgDecision: { decision: "allow" }, + commandCwd: "/test/workspace", }) expect(mockPushToolResult).toHaveBeenCalled() }) @@ -395,6 +607,7 @@ describe("executeCommandTool", () => { // 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" }, + commandCwd: "/test/workspace", }) }) @@ -419,7 +632,9 @@ describe("executeCommandTool", () => { toolCallId: "test-tool-call-id", }) - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/test/workspace", + }) }) it("keeps the protected prompt when blanket auto-deny is on but command auto-approval is off", async () => { @@ -443,7 +658,9 @@ describe("executeCommandTool", () => { toolCallId: "test-tool-call-id", }) - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/test/workspace", + }) }) it("keeps the protected prompt when blanket auto-deny is on but execute auto-approval is off", async () => { @@ -471,7 +688,9 @@ describe("executeCommandTool", () => { toolCallId: "test-tool-call-id", }) - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/test/workspace", + }) }) it("denies an approved command at execute time when blanket deny engages during the approval dwell", async () => { @@ -621,7 +840,9 @@ describe("executeCommandTool", () => { // 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) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/test/workspace", + }) // The blanket denial wins over the affirmative approval: the command // never reaches the terminal. The provider-level assertion is the @@ -661,6 +882,53 @@ describe("executeCommandTool", () => { expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() }) + it("re-checks the command policy against the custom working directory", async () => { + // The folder-scoped command policy must judge the directory the command + // actually runs in. Forwarding `task.cwd` here would let folder A's + // allow entry approve a command executing in folder B, so the same + // resolved directory the terminal will spawn in rides the re-check. + await setupProtectedDwellFlip() + mockToolUse.params.cwd = "/custom/path" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/custom/path" } + // Delegates to the import-time real check: re-reading the module + // property here would capture this file's own leaked spy and recurse. + const recheckSpy = vitest + .spyOn(autoApprovalModule, "checkAutoApproval") + .mockImplementation((args) => realCheckAutoApproval(args)) + + await executeCommandTool.handle(mockCline, mockToolUse, { + askApproval: mockAskApproval, + handleError: mockHandleError, + pushToolResult: mockPushToolResult, + toolCallId: "test-tool-call-id", + }) + expect(recheckSpy).toHaveBeenCalledTimes(1) + expect(recheckSpy).toHaveBeenCalledWith(expect.objectContaining({ cwd: "/custom/path" })) + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, true, { + commandCwd: "/custom/path", + }) + }) + + it("spawns in the approval's working directory rather than re-deriving from the raw custom cwd", async () => { + // The executor must honor the directory the tool resolved for + // approval verbatim, even though a different raw `customCwd` rides + // the options. Re-deriving from `customCwd` would spawn the command + // somewhere other than the directory the command policy approved. + vitest.spyOn(executeCommandModule, "executeCommandInTerminal").mockRestore() + + const [rejected] = await executeCommandModule.executeCommandInTerminal(mockCline, { + executionId: "test-exec", + command: "echo test", + customCwd: "sub/dir", + workingDirectory: "/approved/folder", + toolCallId: "test-tool-call-id", + }) + + expect(rejected).toBe(false) + const firstArg = (TerminalRegistry.getOrCreateTerminal as ReturnType).mock.calls[0][0] + expect(firstArg).toBe("/approved/folder") + }) + 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 @@ -961,7 +1229,9 @@ describe("executeCommandTool", () => { }) // Verify - expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test") + expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test", undefined, false, { + commandCwd: "/test/workspace", + }) // executeCommandInTerminal should not be called since approval was denied expect(mockPushToolResult).not.toHaveBeenCalled() }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8fff156425..af76926aa1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -70,6 +70,7 @@ import { ORGANIZATION_ALLOW_ALL, DEFAULT_MODES, DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, + DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED, getModelId, isRetiredProvider, modelIdKeys, @@ -134,6 +135,11 @@ import { ContextProxy } from "../config/ContextProxy" import { withoutInvalidDisplaySettings } from "../config/displaySettings" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" +import { + readCommandListScopes, + resolveEffectiveCommandLists, + workspaceCommandListsEnabled, +} from "../auto-approval/commandScope" import { PendingActionSettlementError, Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" @@ -3040,62 +3046,6 @@ export class ClineProvider } } - /** - * Merges allowed commands from global state and workspace configuration - * with proper validation and deduplication - */ - private mergeAllowedCommands(globalStateCommands?: string[]): string[] { - return this.mergeCommandLists("allowedCommands", "allowed", globalStateCommands) - } - - /** - * Merges denied commands from global state and workspace configuration - * with proper validation and deduplication - */ - private mergeDeniedCommands(globalStateCommands?: string[]): string[] { - return this.mergeCommandLists("deniedCommands", "denied", globalStateCommands) - } - - /** - * Common utility for merging command lists from global state and workspace configuration. - * Implements the Command Denylist feature's merging strategy with proper validation. - * - * @param configKey - VSCode workspace configuration key - * @param commandType - Type of commands for error logging - * @param globalStateCommands - Commands from global state - * @returns Merged and deduplicated command list - */ - private mergeCommandLists( - configKey: "allowedCommands" | "deniedCommands", - commandType: "allowed" | "denied", - globalStateCommands?: string[], - ): string[] { - try { - // Validate and sanitize global state commands - const validGlobalCommands = Array.isArray(globalStateCommands) - ? globalStateCommands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) - : [] - - // Get workspace configuration commands - const workspaceCommands = vscode.workspace.getConfiguration(Package.name).get(configKey) || [] - - // Validate and sanitize workspace commands - const validWorkspaceCommands = Array.isArray(workspaceCommands) - ? workspaceCommands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) - : [] - - // Combine and deduplicate commands - // Global state takes precedence over workspace configuration - const mergedCommands = [...new Set([...validGlobalCommands, ...validWorkspaceCommands])] - - return mergedCommands - } catch (error) { - console.error(`Error merging ${commandType} commands:`, error) - // Return empty array as fallback to prevent crashes - return [] - } - } - async getStateToPostToWebview({ includeTaskHistory = true }: GetStateOptions = {}): Promise { // Ensure the store is initialized before reading task history await this.taskHistoryStore.initialized @@ -3116,6 +3066,7 @@ export class ClineProvider alwaysDenyUnapprovedCommands, allowedCommands, deniedCommands, + workspaceCommandListsEnabled: gateEnabled, alwaysAllowMcp, alwaysAllowModeSwitch, alwaysAllowSubtasks, @@ -3221,8 +3172,6 @@ export class ClineProvider const telemetryKey = process.env.POSTHOG_API_KEY const machineId = vscode.env.machineId const vscodeTelemetryEnabled = vscode.env.isTelemetryEnabled - const mergedAllowedCommands = this.mergeAllowedCommands(allowedCommands) - const mergedDeniedCommands = this.mergeDeniedCommands(deniedCommands) const cwd = this.cwd const currentTask = this.getCurrentTask() let currentTaskMode: string | undefined @@ -3302,8 +3251,11 @@ export class ClineProvider checkpointTimeout: checkpointTimeout ?? DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, shouldShowAnnouncement: telemetrySetting !== "unset" && lastShownAnnouncementId !== this.latestAnnouncementId, - allowedCommands: mergedAllowedCommands, - deniedCommands: mergedDeniedCommands, + // getState() always resolves these to the effective command lists + // (never undefined), so no fallback is needed here. + allowedCommands, + deniedCommands, + workspaceCommandListsEnabled: gateEnabled ?? DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED, soundVolume: soundVolume ?? 0.5, writeDelayMs: writeDelayMs ?? DEFAULT_WRITE_DELAY_MS, diffFuzzyThreshold: diffFuzzyThreshold ?? DEFAULT_DIFF_FUZZY_THRESHOLD, @@ -3508,6 +3460,14 @@ export class ClineProvider const taskSyncEnabled: boolean = false + // One gate read and one resolver pass per getState() call, so the + // effective lists and the gate value posted together describe a single + // configuration state. + const gateEnabled = workspaceCommandListsEnabled() + const allowScopes = readCommandListScopes("allowedCommands", stateValues.allowedCommands) + const denyScopes = readCommandListScopes("deniedCommands", stateValues.deniedCommands) + const effectiveCommandLists = resolveEffectiveCommandLists(allowScopes, denyScopes, gateEnabled) + // Return the same structure as before. return { apiConfiguration: providerSettings, @@ -3537,8 +3497,22 @@ export class ClineProvider autoCondenseContext: stateValues.autoCondenseContext ?? true, autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100, taskHistory: includeTaskHistory ? this.taskHistoryStore.getAll() : [], - allowedCommands: stateValues.allowedCommands, - deniedCommands: stateValues.deniedCommands, + // The effective lists composed from the global-state and + // workspace-scope configuration layers, scoped to the window's + // first folder; getStateToPostToWebview shows exactly these values, + // so the dialog cannot drift from what this state posts. In a + // multi-root window a task in another folder is judged by that + // folder's lists instead — enforcement re-composes the pair from + // the raw global layers below, so the displayed pair describes the + // window, not the task. + allowedCommands: effectiveCommandLists.allowed, + deniedCommands: effectiveCommandLists.denied, + workspaceCommandListsEnabled: gateEnabled, + // The global layers before workspace composition, so command + // enforcement can re-compose the pair against each task's own + // folder; the window-scoped pair above stays what display shows. + globalAllowedCommands: allowScopes.global, + globalDeniedCommands: denyScopes.global, soundEnabled: stateValues.soundEnabled ?? false, ttsEnabled: stateValues.ttsEnabled ?? false, ttsSpeed: stateValues.ttsSpeed ?? 1.0, @@ -4274,6 +4248,9 @@ export class ClineProvider if (configuration) { await this.setValues(configuration) + // Deliberately Global-targeted: these writes carry "start this task with + // exactly this auto-approval policy" semantics (e.g. from handoff/delegation + // payloads), unlike the scope-split persistence in the settings dialog. if (configuration.allowedCommands) { await vscode.workspace .getConfiguration(Package.name) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 97a25a7038..cc45f4d370 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -20,6 +20,7 @@ import { ORGANIZATION_ALLOW_ALL, DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, DEFAULT_DIFF_FUZZY_THRESHOLD, + DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED, DEFAULT_WRITE_DELAY_MS, providerIdentifiers, openAiModelInfoSaneDefaults, @@ -29,6 +30,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { defaultModeSlug } from "../../../shared/modes" import { experimentDefault } from "../../../shared/experiments" import { setTtsEnabled } from "../../../utils/tts" +import { makeUri, makeWorkspaceConfiguration } from "../../../test-utils/vscode" import { ContextProxy } from "../../config/ContextProxy" import { WorkspaceIndexingEnablementManager } from "../../../services/code-index/workspace-indexing-enablement-manager" import { Task, TaskOptions } from "../../task/Task" @@ -4095,6 +4097,250 @@ describe("ClineProvider", () => { expect(state.alwaysDenyUnapprovedCommands).toBe(false) }) + describe("command list scope merging", () => { + // getConfiguration overrides must not leak past these tests: + // clearAllMocks does not remove mockReturnValue defaults. + afterEach(() => { + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue([]), + update: vi.fn(), + } as unknown as vscode.WorkspaceConfiguration) + Reflect.deleteProperty(vscode.workspace, "workspaceFolders") + }) + + // The gate is a synchronous configuration read, so the doubles make the + // `get` lookup key-aware: only the gate key reports true here. + test("getState enforces workspace-scope denied commands even when global state is empty", async () => { + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockImplementation((key: string) => key === "workspaceCommandListsEnabled"), + inspect: vi + .fn() + .mockImplementation((key: string) => + key === "deniedCommands" ? { key, workspaceValue: ["docker"] } : undefined, + ), + update: vi.fn(), + } as unknown as vscode.WorkspaceConfiguration) + + const state = await provider.getState() + + expect(state.deniedCommands).toEqual(["docker"]) + }) + + test("getStateToPostToWebview shows the same effective command lists as getState", async () => { + await provider.resolveWebviewView(mockWebviewView) + await provider.contextProxy.setValue("deniedCommands", ["git"]) + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockImplementation((key: string) => key === "workspaceCommandListsEnabled"), + inspect: vi + .fn() + .mockImplementation((key: string) => + key === "deniedCommands" ? { key, workspaceValue: ["docker"] } : undefined, + ), + update: vi.fn(), + } as unknown as vscode.WorkspaceConfiguration) + + const state = await provider.getStateToPostToWebview() + + // The workspace entry survives because no global entry shadows it, so + // display is the global layer plus that entry — exactly what + // getState() enforces. + expect(state.deniedCommands).toEqual(["git", "docker"]) + expect(state.allowedCommands).toEqual([]) + }) + + test("getState returns the raw global command lists when the workspace layer differs", async () => { + await provider.contextProxy.setValue("allowedCommands", ["git"]) + await provider.contextProxy.setValue("deniedCommands", ["curl"]) + const config = makeWorkspaceConfiguration({ workspaceCommandListsEnabled: true }) + const inspect = vi.fn() + inspect.mockImplementation((key: string) => ({ + key, + workspaceValue: key === "allowedCommands" ? ["docker"] : ["pnpm"], + })) + config.inspect = inspect + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) + + const state = await provider.getState() + + // The raw global layers are the feed per-folder enforcement + // re-composes from, so they must stay distinguishable from the + // window-composed pair: with the layers diverging here, a dropped + // field or a composed value would silently reroute enforcement to + // the first-folder lists. + expect(state.globalAllowedCommands).toEqual(["git"]) + expect(state.globalDeniedCommands).toEqual(["curl"]) + expect(state.allowedCommands).toEqual(["git", "docker"]) + expect(state.deniedCommands).toEqual(["curl", "pnpm"]) + }) + + test("getState prefers the workspace-folder value for multi-root workspaces", async () => { + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockImplementation((key: string) => key === "workspaceCommandListsEnabled"), + inspect: vi + .fn() + .mockImplementation((key: string) => + key === "deniedCommands" + ? { key, workspaceValue: ["docker"], workspaceFolderValue: ["pnpm docker"] } + : undefined, + ), + update: vi.fn(), + } as unknown as vscode.WorkspaceConfiguration) + + const state = await provider.getState() + + // The folder value replaces (not unions with) the workspace value, and + // the effective get() value is ignored so global-scoped entries do not + // leak into the workspace layer. + expect(state.deniedCommands).toEqual(["pnpm docker"]) + }) + + test("getState scopes its command-list lookups to the first workspace folder", async () => { + // Each folder carries its own distinct configuration, keyed by the + // resource the lookup was scoped with: any lookup that resolves to + // the second folder — or to no resource at all — reads the second + // folder's values, so the first folder's list can only come from a + // configuration scoped to the first folder's uri. + const firstFolder = { uri: makeUri("/root/one"), name: "first", index: 0 } + const secondFolder = { uri: makeUri("/root/two"), name: "second", index: 1 } + // The mocked module's workspaceFolders is readonly-typed, so the test + // installs it through defineProperty; the afterEach removes it again. + Reflect.defineProperty(vscode.workspace, "workspaceFolders", { + configurable: true, + value: [firstFolder, secondFolder], + }) + + const firstFolderConfig = makeWorkspaceConfiguration({ workspaceCommandListsEnabled: true }) + firstFolderConfig.inspect = vi + .fn() + .mockImplementation((key: string) => + key === "deniedCommands" + ? { key, workspaceValue: ["workspace docker"], workspaceFolderValue: ["pnpm docker"] } + : undefined, + ) + const secondFolderConfig = makeWorkspaceConfiguration({ workspaceCommandListsEnabled: false }) + secondFolderConfig.inspect = vi + .fn() + .mockImplementation((key: string) => + key === "deniedCommands" ? { key, workspaceFolderValue: ["curl"] } : undefined, + ) + vi.mocked(vscode.workspace.getConfiguration).mockImplementation((_section?: string, scope?: unknown) => + scope === firstFolder.uri ? firstFolderConfig : secondFolderConfig, + ) + + const state = await provider.getState() + + expect(state.deniedCommands).toEqual(["pnpm docker"]) + expect(state.workspaceCommandListsEnabled).toBe(true) + }) + + // A workspace-only allow entry is exactly what the gate decides: enabled + // it joins the effective lists enforcement uses; disabled enforcement ignores it. + function mockGateConfiguration(enabled: boolean) { + // The shared double reports scripted values through `get`, which + // is how the gate reader sees its value; the per-key `inspect` + // override supplies the allow-side workspace layer. + const config = makeWorkspaceConfiguration({ workspaceCommandListsEnabled: enabled }) + const inspect = vi.fn() + inspect.mockImplementation((key: string) => + key === "allowedCommands" ? { key, workspaceValue: ["docker"] } : undefined, + ) + config.inspect = inspect + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) + } + + // Counterpart for a profile that never set the gate: the configuration + // reports nothing for the gate key, so the provider paths must apply the + // shared default themselves. The allow-side workspace layer is present so + // the asserted default direction (workspace entries stay inert) is + // observable, not just the boolean. + function mockUnsetGateConfiguration() { + const config = makeWorkspaceConfiguration() + const inspect = vi.fn() + inspect.mockImplementation((key: string) => + key === "allowedCommands" ? { key, workspaceValue: ["docker"] } : undefined, + ) + config.inspect = inspect + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) + } + + test("a workspace-only allow entry surfaces through getState and getStateToPostToWebview when the gate is on", async () => { + await provider.resolveWebviewView(mockWebviewView) + mockGateConfiguration(true) + + const state = await provider.getState() + const postedState = await provider.getStateToPostToWebview() + + expect(state.allowedCommands).toEqual(["docker"]) + expect(postedState.allowedCommands).toEqual(["docker"]) + }) + + test("a workspace-only allow entry is absent from getState and getStateToPostToWebview when the gate is off", async () => { + await provider.resolveWebviewView(mockWebviewView) + mockGateConfiguration(false) + + const state = await provider.getState() + const postedState = await provider.getStateToPostToWebview() + + expect(state.allowedCommands).toEqual([]) + expect(postedState.allowedCommands).toEqual([]) + }) + + test("getState returns the saved workspace command-list gate value", async () => { + mockGateConfiguration(true) + + const state = await provider.getState() + + expect(state.workspaceCommandListsEnabled).toBe(true) + }) + + test("getState defaults the workspace command-list gate to the shared constant", async () => { + mockGateConfiguration(false) + + const state = await provider.getState() + + expect(state.workspaceCommandListsEnabled).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + }) + + test("getStateToPostToWebview returns the saved workspace command-list gate value", async () => { + await provider.resolveWebviewView(mockWebviewView) + mockGateConfiguration(true) + + const state = await provider.getStateToPostToWebview() + + expect(state.workspaceCommandListsEnabled).toBe(true) + }) + + test("getStateToPostToWebview defaults the workspace command-list gate to the shared constant", async () => { + await provider.resolveWebviewView(mockWebviewView) + mockGateConfiguration(false) + + const state = await provider.getStateToPostToWebview() + + expect(state.workspaceCommandListsEnabled).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + }) + + test("getState defaults the workspace command-list gate when the configuration is unset", async () => { + mockUnsetGateConfiguration() + + const state = await provider.getState() + + expect(state.workspaceCommandListsEnabled).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + // The default's direction, not just its value: an unset gate keeps the + // workspace entry out of the effective list. + expect(state.allowedCommands).toEqual([]) + }) + + test("getStateToPostToWebview defaults the workspace command-list gate when the configuration is unset", async () => { + await provider.resolveWebviewView(mockWebviewView) + mockUnsetGateConfiguration() + + const state = await provider.getStateToPostToWebview() + + expect(state.workspaceCommandListsEnabled).toBe(DEFAULT_WORKSPACE_COMMAND_LISTS_ENABLED) + expect(state.allowedCommands).toEqual([]) + }) + }) + test("getState returns the saved allowed read files", async () => { await provider.contextProxy.setValue("allowedReadFiles", ["notes.md"]) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index bf50f08786..2a34c42dfc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -77,10 +77,12 @@ vi.mock("@roo-code/telemetry", () => ({ }, })) -import type { ModelRecord } from "@roo-code/types" +import type { ModelRecord, WebviewMessage } from "@roo-code/types" import * as vsCodeLmProvider from "../../../api/providers/vscode-lm" import * as copilotProvider from "../../../api/providers/github-copilot" +import { Package } from "../../../shared/package" +import { makeWorkspaceConfiguration } from "../../../test-utils/vscode" import { webviewMessageHandler } from "../webviewMessageHandler" import { registerCopilotModelBroadcast } from "../../../services/language-models/copilotModelBroadcast" import type { ClineProvider } from "../ClineProvider" @@ -774,20 +776,35 @@ describe("webviewMessageHandler - theme fixture probes", () => { import { changeLanguage, t } from "../../../i18n" +// Mutable so a test can script a multi-root workspace; every other test sees the +// single folder the command-list configuration resolves against by default. +const mockWorkspaceFolders = vi.hoisted(() => ({ + folders: [{ uri: { fsPath: "/mock/workspace" } }] as { uri: { fsPath: string } }[], +})) + vi.mock("vscode", () => { const showInformationMessage = vi.fn() const showErrorMessage = vi.fn() + const showWarningMessage = vi.fn() const openTextDocument = vi.fn().mockResolvedValue({}) const showTextDocument = vi.fn().mockResolvedValue(undefined) return { + ConfigurationTarget: { + Global: 1, + Workspace: 2, + WorkspaceFolder: 3, + }, window: { showInformationMessage, showErrorMessage, + showWarningMessage, showTextDocument, }, workspace: { - workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + get workspaceFolders() { + return mockWorkspaceFolders.folders + }, openTextDocument, getConfiguration: vi.fn(() => ({ get: vi.fn() })), }, @@ -2095,6 +2112,903 @@ describe("webviewMessageHandler - allowlists together", () => { }) }) +describe("webviewMessageHandler - command list scope-aware persistence", () => { + beforeEach(() => { + vi.clearAllMocks() + mockWorkspaceFolders.folders = [{ uri: { fsPath: "/mock/workspace" } }] + }) + + // The file-level vscode mock opens one workspace folder, so the command + // list configuration resolves to its uri. + const mockWorkspaceFolderUri = { fsPath: "/mock/workspace" } + + // Mirrors the real VS Code configuration API: `inspect()` exposes the + // per-scope values, and `update()` records the target it was written to. + // Extension global state is left unset (getValue returns undefined), i.e. + // the global layer starts empty. + // + // `updateSequence` / `inspectSequence` let a failure test script what + // successive `update()` / `inspect()` calls return (each `update()` return + // is awaited by the handler, so a rejected promise fails that write), in + // call order; a plain value resolves like the default mock, and an Error + // element makes that call throw. + type InspectedCommandList = { + globalValue?: string[] + workspaceValue?: string[] + workspaceFolderValue?: string[] + } + + // `inspected` may be a function of the requested key so a two-key payload + // test can give each list its own layers. `gateEnabled` scripts the + // `workspaceCommandListsEnabled` configuration read; leaving it unset + // keeps the gate off, matching the extension default. + function mockCommandConfiguration( + inspected: InspectedCommandList | ((key: string) => InspectedCommandList), + updateSequence?: unknown[], + inspectSequence?: unknown[], + gateEnabled?: boolean, + ) { + const update = vi.fn() + if (updateSequence) { + update.mockImplementation(() => updateSequence.shift()) + } + const inspect = vi.fn() + if (inspectSequence) { + inspect.mockImplementation(() => { + const next = inspectSequence.shift() + if (next instanceof Error) { + throw next + } + return next + }) + } else { + inspect.mockImplementation((key: string) => (typeof inspected === "function" ? inspected(key) : inspected)) + } + // The shared structurally complete double keeps the configuration type + // correct; the scripted members are installed per test. + const config = makeWorkspaceConfiguration( + gateEnabled === undefined ? {} : { workspaceCommandListsEnabled: gateEnabled }, + ) + config.update = update + config.inspect = inspect + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(config) + return { update, inspect } + } + + it("keeps workspace-scoped denied commands out of the global write when saving the merged list", async () => { + // Gate on: the workspace entry is visible in the dialog, so keeping it + // is not a re-add. (With the gate off every submitted workspace entry + // is a re-add and is promoted to the global layer instead.) + const { update, inspect } = mockCommandConfiguration({ workspaceValue: ["docker"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + // "docker" stays at the workspace scope instead of leaking into the + // user's global settings (issue #1939: persist without scope leakage). + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Global) + // Extension global state holds only the global layer, so the next save + // cannot double-home the workspace entry. + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", ["aws"]) + // One snapshot per command-list key: this key's scope read, write + // target, and rollback all derive from the same inspect(), and the + // shadowing check reads the opposite key through its own guarded + // snapshot, so the count is two, never more. + expect(inspect).toHaveBeenCalledTimes(2) + }) + + it("writes the workspace-folder value at the folder target in multi-root workspaces", async () => { + // Gate on for the same reason as the merged-list split test: the kept + // folder entry is visible in the dialog, so it stays at its scope. + const { update } = mockCommandConfiguration( + { workspaceValue: ["npm"], workspaceFolderValue: ["docker"] }, + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + // Read, target resolution, and write all run on the configuration + // scoped to the first workspace folder — the folder-target update is + // only legal on a resource-scoped configuration. + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, mockWorkspaceFolderUri) + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker"], vscode.ConfigurationTarget.WorkspaceFolder) + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", ["aws"]) + }) + + it("clears a deleted workspace entry at the workspace scope without adding it to global", async () => { + // Gate on: the entry is visible in the dialog, and only visible entries + // are deletable there (a hidden entry is re-added by the preservation + // rule instead). + const { update } = mockCommandConfiguration({ workspaceValue: ["docker"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: [] }, + }) + + // Deleting a project entry must clear it where it lives... + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Workspace) + // ...and must not copy it into global settings. + expect(update).not.toHaveBeenCalledWith("deniedCommands", ["docker"], vscode.ConfigurationTarget.Global) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", []) + }) + + it("does not create a workspace layer when none exists", async () => { + const { update } = mockCommandConfiguration({}) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { allowedCommands: ["git"] }, + }) + + // Only the Global write happens; saving must not materialize an empty + // workspace-scoped value in settings.json. + expect(update).toHaveBeenCalledTimes(1) + expect(update).toHaveBeenCalledWith("allowedCommands", ["git"], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("allowedCommands", ["git"]) + }) + + it("restores the raw workspace value when a workspace-target global write fails", async () => { + // With no folder layer the rewind must read the workspace value, not + // the absent folder value. The gate is on so the kept workspace entry is + // visible and the workspace write (`["docker"]`) differs from the raw + // rollback value (which still holds the entry this save dropped); the + // gate-off alternative would make the first write re-add the hidden + // entries and duplicate the rollback value, letting a call-agnostic + // assertion match a write that is not the rewind. + const { update } = mockCommandConfiguration( + { workspaceValue: ["npm docker", "docker"] }, + [undefined, Promise.reject(new Error("disk on fire"))], + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + // Call order: workspace write, failed global write, rewind — so the + // rewind is pinned by position, not just by matching some call. + expect(update).toHaveBeenNthCalledWith( + 3, + "deniedCommands", + ["npm docker", "docker"], + vscode.ConfigurationTarget.Workspace, + ) + expect(update).toHaveBeenCalledTimes(3) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to save deniedCommands: disk on fire") + }) + + it("restores the previous workspace value when the global write fails", async () => { + // update() call order: workspace write succeeds, global write rejects. + // The raw folder-layer value differs from the sanitized post-split layer + // (and from the workspace-layer value), so the restore call is uniquely + // identifiable and provably reads the folder value for a folder target. + // The gate is on so the workspace write keeps only the visible entry and + // cannot match the raw rollback value; the rewind is asserted by call + // position. + const { update } = mockCommandConfiguration( + { workspaceValue: ["npm"], workspaceFolderValue: ["pnpm docker", "docker"] }, + [undefined, Promise.reject(new Error("disk on fire"))], + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // A second key pins that only the failed command-list key is skipped. + updatedSettings: { deniedCommands: ["docker", "aws"], alwaysDenyUnapprovedCommands: true }, + }) + + // The capture, the failed write, and the restore all ran on the + // configuration scoped to the first workspace folder. + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, mockWorkspaceFolderUri) + // The raw previous folder value (entries this save dropped included) is + // written back at the target the failed save chose, as the third write. + expect(update).toHaveBeenNthCalledWith( + 3, + "deniedCommands", + ["pnpm docker", "docker"], + vscode.ConfigurationTarget.WorkspaceFolder, + ) + expect(update).toHaveBeenCalledTimes(3) + // The failed key's extension global-state write is skipped... + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalledWith("deniedCommands", expect.anything()) + // ...while the other key of the same payload still persists. + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to save deniedCommands: disk on fire") + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("does not write global or extension state when the workspace write fails", async () => { + // The first update() call is the workspace write; it rejects, so the + // global write must never be attempted for this key. The rejection is a + // plain string to pin the non-Error rendering of the failure message. + const { update } = mockCommandConfiguration({ workspaceValue: ["docker"] }, [ + Promise.reject("workspace locked"), + ]) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + // Nothing was changed yet, so there is nothing to restore: only the + // failed workspace write happened, and no compensation write followed. + expect(update).toHaveBeenCalledTimes(1) + expect(update).not.toHaveBeenCalledWith("deniedCommands", expect.anything(), vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to save deniedCommands: workspace locked") + }) + + it("aborts the key without writing when the configuration inspection has no result", async () => { + // The save derives scopes, write target, and rollback from one inspect() + // snapshot, so a missing snapshot means the workspace layer is unknown — + // proceeding would classify the retained workspace entry "docker" as new + // and re-copy it into global settings (issue #1939's leak). The key is + // aborted instead: no configuration write, no extension global state. + const { update } = mockCommandConfiguration({ workspaceValue: ["docker"] }, undefined, [undefined]) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + expect(update).not.toHaveBeenCalled() + // Leak pin: the workspace entry must not land in the global layer. + expect(update).not.toHaveBeenCalledWith("deniedCommands", ["docker", "aws"], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to save deniedCommands: the configuration value could not be read", + ) + // No exception escaped the handler: the state post still ran. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("aborts the command-list key when the inspection throws, while other keys of the payload still save", async () => { + const { update } = mockCommandConfiguration({ workspaceValue: ["docker"] }, undefined, [ + new Error("configuration unavailable"), + ]) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // A second key pins that only the aborted command-list key is skipped. + updatedSettings: { deniedCommands: ["docker", "aws"], alwaysDenyUnapprovedCommands: true }, + }) + + // The aborted key wrote nothing at any scope... + expect(update).not.toHaveBeenCalled() + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalledWith("deniedCommands", expect.anything()) + // ...while the other key of the same payload still persists. + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to save deniedCommands: configuration unavailable", + ) + }) + + it("propagates a configuration lookup failure instead of degrading it to a per-key abort", async () => { + // The commandListConfiguration() lookup sits outside the guarded + // snapshot read on purpose: a lookup that cannot return a configuration + // object at all is a different failure class than an unreadable value, + // so it aborts the entire message rather than the one key. + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => { + throw new Error("workspace not ready") + }) + + await expect( + webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // A second key pins that the whole message aborts, not just the + // command-list key: with the guarded per-key failure paths this + // key would still persist. + updatedSettings: { deniedCommands: ["docker", "aws"], alwaysDenyUnapprovedCommands: true }, + }), + ).rejects.toThrow("workspace not ready") + + // Nothing persisted for any key and no per-key error handling ran; the + // rejection surfaced to the caller. The webview still gets a state + // refresh: a failed save now always re-posts before re-throwing. + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("reports partial failure when the compensating restore also fails", async () => { + // update() call order: workspace write succeeds, global write rejects, + // restore write rejects too. The raw workspace value differs from the + // post-split layer so the restore call is uniquely identifiable. The + // gate is on so the workspace write (`["docker"]`) stays distinct from + // the raw rollback value, and the rewind is pinned by call position. + const { update } = mockCommandConfiguration( + { workspaceValue: ["docker", "old"] }, + [undefined, Promise.reject(new Error("disk on fire")), Promise.reject(new Error("restore also failed"))], + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + // The restore was attempted at the owning target even though it failed. + expect(update).toHaveBeenNthCalledWith( + 3, + "deniedCommands", + ["docker", "old"], + vscode.ConfigurationTarget.Workspace, + ) + expect(update).toHaveBeenCalledTimes(3) + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to save deniedCommands: disk on fire. The workspace-scoped restore also failed: Error: restore also failed. Review the command lists in your user and workspace settings.", + ) + // No exception escaped the handler: the state post still ran. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + // Entries the dialog never displayed — hidden while the gate is off, or + // shadowed by a global entry with it on — cannot be deleted by omission + // from a list the user could not see, so a save re-adds them to the + // workspace layer it rewrites. + it("re-adds gate-hidden workspace entries to the workspace write instead of deleting them", async () => { + const { update } = mockCommandConfiguration({ workspaceValue: ["docker", "npm"] }, undefined, undefined, false) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // The gate is off, so the dialog displayed nothing for this key. + updatedSettings: { deniedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker", "npm"], vscode.ConfigurationTarget.Workspace) + expect(update).not.toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Workspace) + }) + + it("keeps hidden workspace entries out of the global write", async () => { + const { update } = mockCommandConfiguration({ workspaceValue: ["docker", "npm"] }, undefined, undefined, false) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: [] }, + }) + + // Preservation is workspace-side only: re-adding hidden entries must + // not leak them into the user's global settings. + expect(update).not.toHaveBeenCalledWith("deniedCommands", ["docker", "npm"], vscode.ConfigurationTarget.Global) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", []) + }) + + it("re-adds a workspace deny entry shadowed by a global allow entry when the gate is on", async () => { + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "allowedCommands" ? ["git"] : undefined, + ) + const { update } = mockCommandConfiguration( + (key) => (key === "deniedCommands" ? { workspaceValue: ["git push"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // "git push" is shadowed by the global allow "git", so the dialog + // displayed nothing deletable. + updatedSettings: { deniedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["git push"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + }) + + it("promotes a gate-hidden workspace entry the dialog re-adds to the global write", async () => { + // With the gate off the dialog shows only the global layer, so a + // submitted entry the workspace layer already holds hidden is one the + // user just added: it must reach the enforced global layer instead of + // being treated as existing knowledge, while the re-add below still + // keeps it in the workspace file. + const { update } = mockCommandConfiguration({ workspaceValue: ["docker"] }, undefined, undefined, false) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker", "aws"], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", ["docker", "aws"]) + }) + + it("promotes a shadowed workspace entry the dialog re-adds to the global write", async () => { + // The global allow "git" shadows the workspace deny "git push", so the + // dialog never displayed the deny entry; a user who re-adds it submits a + // list containing it, and the entry must land in the global layer where + // the gate makes it effective, while the re-add preserves the workspace + // copy. + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "allowedCommands" ? ["git"] : undefined, + ) + const { update } = mockCommandConfiguration( + (key) => (key === "deniedCommands" ? { workspaceValue: ["git push"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["git push"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["git push"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", ["git push"], vscode.ConfigurationTarget.Global) + }) + + it("re-adds a shadowed workspace allow entry alongside the user's visible deletions", async () => { + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "deniedCommands" ? ["git"] : undefined, + ) + const { update } = mockCommandConfiguration( + (key) => (key === "allowedCommands" ? { workspaceValue: ["git", "docker"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // "docker" stayed visible and kept; "git" was hidden by the global + // deny, and a workspace entry the user never saw is not deletable. + updatedSettings: { allowedCommands: ["docker"] }, + }) + + expect(update).toHaveBeenCalledWith("allowedCommands", ["docker", "git"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("allowedCommands", [], vscode.ConfigurationTarget.Global) + }) + + it("does not persist a webview-supplied workspaceCommandListsEnabled value", async () => { + mockCommandConfiguration({}) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // A settings payload is not a second source for the gate: the + // synchronous user-scope configuration read is. + updatedSettings: { workspaceCommandListsEnabled: true, alwaysDenyUnapprovedCommands: true }, + }) + + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalledWith( + "workspaceCommandListsEnabled", + expect.anything(), + ) + // Sibling keys of the same payload are unaffected by the refusal. + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + }) + + it("abandons a payload carrying only the gate key without writing configuration", async () => { + const { update } = mockCommandConfiguration({}) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { workspaceCommandListsEnabled: true }, + }) + + expect(update).not.toHaveBeenCalled() + expect(mockClineProvider.contextProxy.setValue).not.toHaveBeenCalled() + // The state post still runs, so the dialog re-reads the effective value + // instead of keeping the refused one. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + // A payload can carry both command-list keys; the second-processed key's + // hidden-entry check must judge shadowing against the state as it was when + // the save started, not as the same loop's earlier key left it. The double + // below mirrors ContextProxy: setValue publishes the in-memory copy before + // resolving, so a fresh read after the first key's awaited write would + // observe it and let the never-displayed entry be deleted. + function mockCommandListStateStore(initial: Partial>) { + const store: Partial> = { ...initial } + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "allowedCommands" || key === "deniedCommands" ? store[key] : undefined, + ) + vi.mocked(mockClineProvider.contextProxy.setValue).mockImplementation(async (key: string, value: unknown) => { + if (key === "allowedCommands" || key === "deniedCommands") { + store[key] = value as string[] | undefined + } + }) + return store + } + + it("preserves a workspace deny entry shadowed by a global allow the same save removes", async () => { + mockCommandListStateStore({ allowedCommands: ["git"] }) + const { update } = mockCommandConfiguration( + (key) => (key === "deniedCommands" ? { workspaceValue: ["git push"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // "allowedCommands" is processed first and drops the shadowing + // global "git"; "git push" was hidden from the dialog, so an empty + // deny list must not delete it. + updatedSettings: { allowedCommands: [], deniedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["git push"], vscode.ConfigurationTarget.Workspace) + expect(update).not.toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Workspace) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("preserves a workspace allow entry shadowed by a global deny the same save removes", async () => { + mockCommandListStateStore({ deniedCommands: ["git"] }) + const { update } = mockCommandConfiguration( + (key) => (key === "allowedCommands" ? { workspaceValue: ["git push"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + // The mirror order: the deny key is processed first, so this pins + // that preservation does not depend on which key arrives first. + updatedSettings: { deniedCommands: [], allowedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("allowedCommands", ["git push"], vscode.ConfigurationTarget.Workspace) + expect(update).not.toHaveBeenCalledWith("allowedCommands", [], vscode.ConfigurationTarget.Workspace) + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("reports a rejected extension-state write without aborting the save", async () => { + // The configuration writes succeeded, so silently dropping the state + // write would leave the settings files and the stored list disagreeing + // with nothing shown to the user. The rejection is queued (once) so it + // consumes exactly this key's write and the sibling key still persists. + vi.mocked(mockClineProvider.contextProxy.setValue).mockRejectedValueOnce(new Error("state store unavailable")) + const { update } = mockCommandConfiguration({}) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["aws"], alwaysDenyUnapprovedCommands: true }, + }) + + // The configuration leg did persist... + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Global) + // ...so the message names the leg that failed... + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to save deniedCommands: state store unavailable. The configuration was updated, but the extension's stored copy of the list was not, so the two disagree until a later save succeeds.", + ) + // ...the save continued to the next key... + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("alwaysDenyUnapprovedCommands", true) + // ...and the dialog was re-synced instead of keeping an unsaved value. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("still propagates a rejected state write for non-command-list keys", async () => { + // The partial-failure report is scoped to the command-list keys, whose + // configuration legs succeeded first; other keys have no half-completed + // save to explain, so their failures keep propagating as before. The + // rejection is queued (once) so it stays scoped to this test. + vi.mocked(mockClineProvider.contextProxy.setValue).mockRejectedValueOnce(new Error("state store unavailable")) + + await expect( + webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { alwaysDenyUnapprovedCommands: true }, + }), + ).rejects.toThrow("state store unavailable") + + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() + // The webview still gets a state refresh: a failed save now always + // re-posts before the rejection propagates. + expect(mockClineProvider.postStateToWebview).toHaveBeenCalled() + }) + + it("keeps a settings-file entry the dialog never displayed and warns that it was preserved", async () => { + // An entry the settings file gained while the dialog was open has the same + // shape as a user deletion (in the file, absent from the submitted list); + // the baseline tells them apart, so the new entry is kept in the layer the + // snapshot found it in and the user is told instead of losing it silently. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["aws", "rm -rf"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["aws"] }, + commandListBaseline: { deniedCommands: ["aws"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws", "rm -rf"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", []) + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("rm -rf")) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("1 entry was")) + }) + + it("applies a user deletion of a displayed entry without warning", async () => { + // The counterpart classification: an entry the baseline shows the dialog + // displayed and the submitted list omits is a deliberate deletion and must + // be dropped from every layer that held it, with nothing disclosed. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["aws", "rm -rf"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["aws"] }, + commandListBaseline: { deniedCommands: ["aws", "rm -rf"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(update).not.toHaveBeenCalledWith("deniedCommands", ["rm -rf"], vscode.ConfigurationTarget.Global) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + }) + + it("keeps a file-added entry while applying a displayed deletion in one save, warning once", async () => { + // Both divergence classes in one payload: "git" was displayed and the user + // deleted it (deletion applies), "curl" arrived after the dialog opened + // (preserved). The warning counts only the preserved entry, once per save. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["git", "curl"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: [] }, + commandListBaseline: { deniedCommands: ["git"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["curl"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("curl")) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalledWith(expect.stringContaining("git")) + }) + + it("saves command lists unchanged when the message carries no baseline", async () => { + // Backward-compatibility witness: senders that omit the baseline keep the + // exact pre-existing behavior - the omitted entry is dropped as a deletion + // and no warning is produced. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["aws", "rm -rf"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["aws"] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", []) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + }) + + it("routes a preserved entry to the layer the save-start snapshot found it in", async () => { + // A preserved entry must land where it lives, not be flattened into the + // global layer: "rm -rf" is in the workspace snapshot, "npm" in the + // extension's stored global layer, and neither was ever displayed. + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "deniedCommands" ? ["npm"] : undefined, + ) + const { update } = mockCommandConfiguration({ workspaceValue: ["rm -rf"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: [] }, + commandListBaseline: { deniedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["rm -rf"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", ["npm"], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", ["npm"]) + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("2 entries were")) + }) + + it("treats a malformed command-list baseline exactly like a missing one", async () => { + // The save message crosses a JSON boundary, so a baseline whose key holds a + // non-array is reachable at runtime even though the declared type forbids it. + // The classification guard treats it as no baseline at all: the submitted list + // is written unclassified and no entry the dialog never displayed is claimed + // as user-deleted, matching the no-baseline witness exactly. + const wireMessage: WebviewMessage = JSON.parse( + JSON.stringify({ + type: "updateSettings", + updatedSettings: { deniedCommands: ["aws"] }, + commandListBaseline: { deniedCommands: "aws" }, + }), + ) + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["aws", "rm -rf"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, wireMessage) + + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("deniedCommands", []) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + }) + + it("composes and writes only the first workspace folder's layer in a multi-root save", async () => { + // The dialog composes against the first folder, so a save must never read or + // write the second folder's per-folder layer: folder B's entries survive a + // folder A save untouched. + const secondFolderUri = { fsPath: "/mock/second-folder" } + mockWorkspaceFolders.folders = [{ uri: mockWorkspaceFolderUri }, { uri: secondFolderUri }] + const { update } = mockCommandConfiguration( + { workspaceValue: ["npm"], workspaceFolderValue: ["docker"] }, + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["docker", "aws"] }, + }) + + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, mockWorkspaceFolderUri) + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalledWith(Package.name, secondFolderUri) + // The folder-target write the save does make goes to the first folder's layer. + expect(update).toHaveBeenCalledWith("deniedCommands", ["docker"], vscode.ConfigurationTarget.WorkspaceFolder) + expect(update).toHaveBeenCalledWith("deniedCommands", ["aws"], vscode.ConfigurationTarget.Global) + }) + + it("aggregates preserved entries from both command-list keys into a single save warning", async () => { + // A payload carrying both list keys classifies per key, so two file entries the + // dialog never displayed - one per key - must survive in their own layers and be + // disclosed together in exactly one warning for the save. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration( + (key) => + key === "allowedCommands" + ? { workspaceValue: ["git", "curl"] } + : { workspaceValue: ["rm -rf", "docker"] }, + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { allowedCommands: ["git"], deniedCommands: ["rm -rf"] }, + commandListBaseline: { allowedCommands: ["git"], deniedCommands: ["rm -rf"] }, + }) + + // "curl" stays in the workspace allow layer, "docker" in the workspace deny layer. + expect(update).toHaveBeenCalledWith("allowedCommands", ["git", "curl"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith( + "deniedCommands", + ["rm -rf", "docker"], + vscode.ConfigurationTarget.Workspace, + ) + expect(update).toHaveBeenCalledWith("allowedCommands", [], vscode.ConfigurationTarget.Global) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + // One aggregated warning naming both preserved entries and counting them together. + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("2 entries were")) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("curl")) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("docker")) + }) + + // A file-side extra the hidden-entry re-add already preserves was never + // displayed, so naming it in the changed-settings warning would repeat that + // warning on every save while the file stays unchanged. + it.each([ + { + name: "hidden while the workspace gate is off", + gateEnabled: false, + hiddenEntry: "docker", + prepare: () => { + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + }, + }, + { + name: "shadowed by a global allow while the gate is on", + gateEnabled: true, + hiddenEntry: "git push", + prepare: () => { + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "allowedCommands" ? ["git"] : undefined, + ) + }, + }, + ])( + "re-adds a $name deny entry without warning that the settings file changed", + async ({ gateEnabled, hiddenEntry, prepare }) => { + prepare() + const { update } = mockCommandConfiguration( + { workspaceValue: [hiddenEntry] }, + undefined, + undefined, + gateEnabled, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: [] }, + // The dialog displayed the effective deny list, which never + // contained the hidden entry, so the baseline omits it. + commandListBaseline: { deniedCommands: [] }, + }) + + expect(update).toHaveBeenCalledWith("deniedCommands", [hiddenEntry], vscode.ConfigurationTarget.Workspace) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + }, + ) + + it("warns only about the genuine file addition when a hidden entry is also present", async () => { + // Mixed divergence in one save: "git push" is hidden by the global allow + // "git" and preserved by the re-add, while "wget" arrived in the visible + // part of the file after the dialog opened. The warning must name only + // the genuine addition. + vi.mocked(mockClineProvider.contextProxy.getValue).mockImplementation((key: string) => + key === "allowedCommands" ? ["git"] : undefined, + ) + const { update } = mockCommandConfiguration( + (key) => (key === "deniedCommands" ? { workspaceValue: ["git push", "curl", "wget"] } : {}), + undefined, + undefined, + true, + ) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { deniedCommands: ["curl"] }, + // The dialog displayed the shadow-filtered deny list: "curl" was + // visible and kept, "git push" was never displayed, and "wget" + // arrived in the settings file after the dialog opened. + commandListBaseline: { deniedCommands: ["curl"] }, + }) + + // "git push" survives through the hidden-entry re-add, "wget" through + // the external-addition re-admission; the global layer stays empty. + expect(update).toHaveBeenCalledWith( + "deniedCommands", + ["curl", "wget", "git push"], + vscode.ConfigurationTarget.Workspace, + ) + expect(update).toHaveBeenCalledWith("deniedCommands", [], vscode.ConfigurationTarget.Global) + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("1 entry was")) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("wget")) + expect(vscode.window.showWarningMessage).not.toHaveBeenCalledWith(expect.stringContaining("git push")) + }) + + it("still warns about a visible workspace addition alongside the hidden-entry exclusion", async () => { + // Regression witness for the hidden-entry exclusion: an entry that + // arrived in the displayed part of the file is a genuine external + // addition, is kept in the layer the snapshot found it in, and is named + // in the warning. + vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined) + const { update } = mockCommandConfiguration({ workspaceValue: ["git", "curl"] }, undefined, undefined, true) + + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { allowedCommands: ["git"] }, + commandListBaseline: { allowedCommands: ["git"] }, + }) + + expect(update).toHaveBeenCalledWith("allowedCommands", ["git", "curl"], vscode.ConfigurationTarget.Workspace) + expect(update).toHaveBeenCalledWith("allowedCommands", [], vscode.ConfigurationTarget.Global) + expect(vscode.window.showWarningMessage).toHaveBeenCalledTimes(1) + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(expect.stringContaining("curl")) + }) +}) + describe("webviewMessageHandler - terminalProfile", () => { beforeEach(() => { vi.clearAllMocks() diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4adb29264b..e75df27a16 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -62,6 +62,15 @@ import { } from "./rulesMessageHandler" import { changeLanguage, t } from "../../i18n" import { Package } from "../../shared/package" +import { + type CommandListInspection, + commandListConfiguration, + readCommandListScopes, + resolveEffectiveCommandLists, + resolveWorkspaceWriteTarget, + splitCommandListScopes, + workspaceCommandListsEnabled, +} from "../auto-approval/commandScope" import { type RouterName, toRouterName } from "../../shared/api" import { MessageEnhancer } from "./messageEnhancer" @@ -773,6 +782,17 @@ export const webviewMessageHandler = async ( } } + // Shadowing is cross-directional, and a payload can carry both + // command-list keys, so the second-processed key's hidden-entry + // check would otherwise read the layer the first key's iteration + // just wrote. Both global layers are captured before any write so + // a hidden entry's preservation cannot depend on payload key order. + const commandListGlobalsAtSaveStart = { + allowedCommands: getGlobalState("allowedCommands"), + deniedCommands: getGlobalState("deniedCommands"), + } + const externallyAddedCommandEntries: string[] = [] + for (const [key, value] of Object.entries(message.updatedSettings)) { // An invalid display value is ignored, before anything is done for it. if (isInvalidDisplayValue(key, value)) { @@ -785,26 +805,174 @@ export const webviewMessageHandler = async ( if (key === "language") { newValue = value ?? "en" changeLanguage(newValue as Language) - } else if (key === "allowedCommands") { + } else if (key === "workspaceCommandListsEnabled") { + // The gate's effective value is the synchronous + // user-scope configuration read that enforcement and + // display already perform; a value arriving in a + // settings payload is not a second source that can flip + // it, so it is never persisted here. + continue + } else if (key === "allowedCommands" || key === "deniedCommands") { const commands = value ?? [] - newValue = Array.isArray(commands) + const incoming = Array.isArray(commands) ? commands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) : [] - await vscode.workspace - .getConfiguration(Package.name) - .update("allowedCommands", newValue, vscode.ConfigurationTarget.Global) - } else if (key === "deniedCommands") { - const commands = value ?? [] + // The webview always sends the full merged list (it displays + // the union of the global-state and workspace-scope layers). + // Splitting that list by origin scope needs the workspace + // layer, the write target, and the rollback value to describe + // the SAME configuration state, so they are all derived from + // one inspect() snapshot taken before any write. Without a + // snapshot the key's save is aborted rather than guessed at: + // reading the workspace layer as empty would classify a + // workspace-scoped entry as brand new and re-copy it into + // global settings. + // Only the snapshot read is guarded: a failing + // commandListConfiguration() lookup propagates instead of being + // downgraded to a per-key save failure. + const config = commandListConfiguration() + let inspection: CommandListInspection | undefined + let inspectionFailure: string | undefined + try { + inspection = config.inspect(key) + } catch (error) { + inspectionFailure = error instanceof Error ? error.message : String(error) + } + if (inspection === undefined) { + // No snapshot: the workspace layer could only be guessed + // at, and a guessed-empty layer re-copies workspace entries + // into global settings, so this key's save is abandoned + // while the remaining keys of the payload persist normally. + vscode.window.showErrorMessage( + `Failed to save ${key}: ${inspectionFailure ?? "the configuration value could not be read"}`, + ) + continue + } - newValue = Array.isArray(commands) - ? commands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) + const currentScopes = readCommandListScopes(key, getGlobalState(key), inspection) + + // Shadowing is cross-directional, so the hidden-entry set + // needs the opposite key's global layer as input. It comes + // from the save-start snapshot, not a fresh read: an + // earlier iteration of this loop may already have written + // that key. The layer only feeds the hidden-entry mechanism + // below — additive preservation, external-addition + // classification, and the split's promotion filter — + // and never this key's target or rollback, which stay + // on the per-key configuration snapshot. + const otherKey = key === "allowedCommands" ? "deniedCommands" : "allowedCommands" + const otherScopes = readCommandListScopes(otherKey, commandListGlobalsAtSaveStart[otherKey]) + const { hiddenWorkspaceAllow, hiddenWorkspaceDeny } = resolveEffectiveCommandLists( + key === "allowedCommands" ? currentScopes : otherScopes, + key === "deniedCommands" ? currentScopes : otherScopes, + workspaceCommandListsEnabled(), + ) + // Entries hidden from the dialog — while the gate is off, + // or shadowed by a global entry with it on — cannot be + // deleted through a list that never displayed them, so a + // save re-adds them to the workspace layer it rewrites. + // Users remove those directly in the workspace settings + // file. + const hiddenWorkspaceEntries = + key === "allowedCommands" ? hiddenWorkspaceAllow : hiddenWorkspaceDeny + + // An entry the settings file gained while the dialog was open has + // the same shape as an entry the user deleted: present in the file, + // absent from the submitted list. Only the dialog's last-displayed + // baseline separates the two, so file entries outside that baseline + // are re-admitted before the split (deletions of baseline entries + // still apply). Hidden entries never appear in that baseline, yet + // the workspace-layer re-add below preserves them, so classifying + // them as external additions would repeat a false "settings file + // changed" warning on every save. A sender that shows no lists + // sends no baseline, and its submitted list is written + // unclassified. + const baseline = message.commandListBaseline?.[key] + const externallyAdded = Array.isArray(baseline) + ? [...new Set([...currentScopes.workspace, ...currentScopes.global])].filter( + (entry) => + !baseline.includes(entry) && + !incoming.includes(entry) && + !hiddenWorkspaceEntries.includes(entry), + ) : [] + const { globalWrite, workspaceWrite: keptWorkspaceWrite } = splitCommandListScopes( + [...incoming, ...externallyAdded], + // Hidden entries are not existing knowledge for promotion + // purposes: the dialog never displayed them, so an entry the + // user just added is new even when the workspace layer + // already holds it hidden. The re-add below restores the + // hidden entries this reduced layer drops. + currentScopes.workspace.filter((entry) => !hiddenWorkspaceEntries.includes(entry)), + currentScopes.global, + ) + + const workspaceWrite = [...new Set([...keptWorkspaceWrite, ...hiddenWorkspaceEntries])] + const workspaceTarget = resolveWorkspaceWriteTarget(key, inspection) + + // The two configuration writes below are not atomic, so the raw + // previous workspace value comes from the same snapshot the split + // used, taken from the target that owns the workspace layer: a + // failed global write rewinds that target to exactly what it held + // beforehand, including the entries this save drops. The raw + // inspected value is used rather than the sanitized layer so the + // rewind reproduces the file contents instead of a cleaned-up + // approximation of them. + const previousWorkspaceValue = + workspaceTarget === vscode.ConfigurationTarget.WorkspaceFolder + ? inspection.workspaceFolderValue + : inspection.workspaceValue + let workspaceWriteApplied = false + try { + // Write the workspace layer first, at the target that owns it. + // When no workspace layer exists the write is skipped entirely + // so saving never materializes one; when entries were deleted + // the `[]` write clears them at that scope. + if (workspaceTarget !== undefined) { + await config.update(key, workspaceWrite, workspaceTarget) + workspaceWriteApplied = true + } + + await config.update(key, globalWrite, vscode.ConfigurationTarget.Global) + } catch (error) { + const failure = error instanceof Error ? error.message : String(error) + + // A failed workspace write leaves nothing changed, so only a + // global-write failure has a changed workspace layer to rewind, + // and the rewind goes to the same target the write used. + if (workspaceWriteApplied) { + try { + await config.update(key, previousWorkspaceValue, workspaceTarget) + } catch (restoreError) { + // The rewind itself failed: the configuration keeps the + // partially written values and the extension global-state + // write is skipped, so stored state never reflects a save + // that did not complete. The user hears about both failures. + vscode.window.showErrorMessage( + `Failed to save ${key}: ${failure}. The workspace-scoped restore also failed: ${restoreError}. Review the command lists in your user and workspace settings.`, + ) + continue + } + } + + // Only this key's save is abandoned; the remaining keys of the + // payload persist normally. + vscode.window.showErrorMessage(`Failed to save ${key}: ${failure}`) + continue + } - await vscode.workspace - .getConfiguration(Package.name) - .update("deniedCommands", newValue, vscode.ConfigurationTarget.Global) + // Extension global state keeps only the global layer: + // enforcement re-adds the workspace layer through + // readCommandListScopes, so storing the merged list here + // would double-home workspace entries and re-leak them on the + // next save. + newValue = globalWrite + // Accumulated only after both configuration writes succeeded: the + // disclosure must not claim preservation for a key whose save was + // abandoned by a failed write. + externallyAddedCommandEntries.push(...externallyAdded) } else if (key === "allowedReadFiles" || key === "allowedWriteFiles") { const patterns = value ?? [] @@ -889,7 +1057,32 @@ export const webviewMessageHandler = async ( } } - await provider.contextProxy.setValue(key as keyof RooCodeSettings, newValue) + if (key === "allowedCommands" || key === "deniedCommands") { + // The configuration writes above already succeeded, so a + // rejected state write leaves the configuration and the + // extension's stored list disagreeing until a later save + // overwrites it. Letting the rejection escape would also + // skip the state post that re-syncs the dialog, so the key + // reports which leg failed and the save continues. + try { + await provider.contextProxy.setValue(key as keyof RooCodeSettings, newValue) + } catch (error) { + const failure = error instanceof Error ? error.message : String(error) + vscode.window.showErrorMessage( + `Failed to save ${key}: ${failure}. The configuration was updated, but the extension's stored copy of the list was not, so the two disagree until a later save succeeds.`, + ) + continue + } + } else { + await provider.contextProxy.setValue(key as keyof RooCodeSettings, newValue) + } + } + + if (externallyAddedCommandEntries.length > 0) { + const count = externallyAddedCommandEntries.length + vscode.window.showWarningMessage( + `Your settings file changed while the settings dialog was open. ${count} ${count === 1 ? "entry was" : "entries were"} added there during this session and kept by this save: ${externallyAddedCommandEntries.join(", ")}. Reopen the settings dialog to see the current command lists.`, + ) } } catch (error) { failed = true @@ -1918,38 +2111,6 @@ export const webviewMessageHandler = async ( // Cancel any pending auto-approval timeout for the current task provider.getCurrentTask()?.cancelAutoApprovalTimeout() break - case "allowedCommands": { - // Validate and sanitize the commands array - const commands = message.commands ?? [] - const validCommands = Array.isArray(commands) - ? commands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) - : [] - - await updateGlobalState("allowedCommands", validCommands) - - // Also update workspace settings. - await vscode.workspace - .getConfiguration(Package.name) - .update("allowedCommands", validCommands, vscode.ConfigurationTarget.Global) - - break - } - case "deniedCommands": { - // Validate and sanitize the commands array - const commands = message.commands ?? [] - const validCommands = Array.isArray(commands) - ? commands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0) - : [] - - await updateGlobalState("deniedCommands", validCommands) - - // Also update workspace settings. - await vscode.workspace - .getConfiguration(Package.name) - .update("deniedCommands", validCommands, vscode.ConfigurationTarget.Global) - - break - } case "openCustomModesSettings": { const customModesFilePath = await provider.customModesManager.getCustomModesFilePath() diff --git a/src/extension.ts b/src/extension.ts index 34865230c4..68742a62dc 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -42,6 +42,7 @@ import { CodeIndexManagerRegistry } from "./services/code-index/code-index-manag import { CodeIndexScope } from "./services/code-index/code-index-scope" import { MdmService } from "./services/mdm/MdmService" import { migrateSettings } from "./utils/migrateSettings" +import { seedCommandList } from "./utils/seedAllowedCommands" import { autoImportSettings } from "./utils/autoImportSettings" import { API } from "./extension/api" @@ -161,9 +162,6 @@ export async function activate(context: vscode.ExtensionContext) { // Initialize i18n for internationalization support. initializeI18n(context.globalState.get("language") ?? formatLanguage(vscode.env.language)) - // Initialize terminal shell execution handlers. - TerminalRegistry.initialize() - // Initialize OpenAI Codex OAuth manager for ChatGPT subscription-based access. openAiCodexOAuthManager.initialize(context, (message) => outputChannel.appendLine(message)) // Kimi Code OAuth tokens live only in VS Code SecretStorage, outside provider profile JSON/cloud sync. @@ -172,13 +170,18 @@ export async function activate(context: vscode.ExtensionContext) { // Initialize Zoo Code auth service for extension session token management. await initZooCodeAuth(context) - // Get default commands from configuration. - const defaultCommands = vscode.workspace.getConfiguration(Package.name).get("allowedCommands") || [] + // Seed before the terminal registry is initialized: a rejected seed must + // abort activation while the registry holds no resources, because its + // listeners are released only by `deactivate()`, which the host does not + // guarantee after a failed activation. + // Each list key carries its own first-run guard, so a profile whose + // allowlist was seeded by an earlier version still gets the deny-list + // migration on the next activation. + await seedCommandList(context.globalState, "allowedCommands") + await seedCommandList(context.globalState, "deniedCommands") - // Initialize global state if not already set. - if (!context.globalState.get("allowedCommands")) { - context.globalState.update("allowedCommands", defaultCommands) - } + // Initialize terminal shell execution handlers. + TerminalRegistry.initialize() const contextProxy = await ContextProxy.getInstance(context) const updateTelemetryState = () => { diff --git a/src/package.json b/src/package.json index 8a30d614e1..62c9e3371d 100644 --- a/src/package.json +++ b/src/package.json @@ -327,6 +327,13 @@ "default": [], "description": "%commands.deniedCommands.description%" }, + "zoo-code.workspaceCommandListsEnabled": { + "type": "boolean", + "default": false, + "scope": "application", + "description": "%settings.workspaceCommandListsEnabled.description%", + "markdownDescription": "%settings.workspaceCommandListsEnabled.description%" + }, "zoo-code.commandExecutionTimeout": { "type": "number", "default": 0, diff --git a/src/package.nls.ca.json b/src/package.nls.ca.json index d6b40efba3..77f5d530fe 100644 --- a/src/package.nls.ca.json +++ b/src/package.nls.ca.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Accepta certificats auto-signats del proxy. **Requerit per a la inspecció MITM.** ⚠️ Insegur — utilitza-ho només per a debugging local.", "settings.workspace.rootResolution.description": "Com resol Zoo l'arrel de l'espai de treball en un espai de treball multi-arrel. L'arrel s'utilitza per localitzar `.roomodes`, `.roo/mcp.json`, `.roo/rules/` i altra configuració d'àmbit de projecte. Canviar aquest paràmetre només afecta les cerques futures; les tasques en execució conserven la seva arrel original.", "settings.workspace.rootResolution.activeEditor.description": "Utilitza la carpeta de l'espai de treball que conté l'editor actiu; recorre a la primera carpeta de l'espai de treball. (Per defecte — conserva el comportament heretat.)", - "settings.workspace.rootResolution.firstFolder.description": "Utilitza sempre la primera carpeta de l'espai de treball (workspaceFolders[0]). Determinista — independent del fitxer que tingui el focus en aquest moment." + "settings.workspace.rootResolution.firstFolder.description": "Utilitza sempre la primera carpeta de l'espai de treball (workspaceFolders[0]). Determinista — independent del fitxer que tingui el focus en aquest moment.", + "settings.workspaceCommandListsEnabled.description": "Activa les llistes de comandes aportades pels ajustos de l'espai de treball (els valors de `zoo-code.allowedCommands` i `zoo-code.deniedCommands` als fitxers d'ajustos de l'espai de treball). Mentre estigui desactivada, les llistes de comandes de l'espai de treball no tenen cap efecte. Quan estigui activada, les entrades de nivell d'usuari tenen preferència: una entrada de nivell d'usuari decideix qualsevol comanda que coincideixi, i les entrades de l'espai de treball només s'apliquen a les comandes amb què cap entrada de nivell d'usuari no coincideix. Aquest paràmetre només es pot canviar als [paràmetres d'usuari](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.de.json b/src/package.nls.de.json index a8bbadbf15..d74eed6998 100644 --- a/src/package.nls.de.json +++ b/src/package.nls.de.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Akzeptiere selbstsignierte Zertifikate vom Proxy. **Erforderlich für MITM-Inspektion.** ⚠️ Unsicher – verwende das nur für lokales Debugging.", "settings.workspace.rootResolution.description": "Wie Zoo das Workspace-Stammverzeichnis in einem Multi-Root-Workspace auflöst. Das Stammverzeichnis wird verwendet, um `.roomodes`, `.roo/mcp.json`, `.roo/rules/` und andere projektbezogene Konfiguration zu finden. Eine Änderung dieser Einstellung wirkt sich nur auf zukünftige Suchen aus; laufende Aufgaben behalten ihr ursprüngliches Stammverzeichnis.", "settings.workspace.rootResolution.activeEditor.description": "Verwende den Workspace-Ordner, der den aktiven Editor enthält; greife auf den ersten Workspace-Ordner zurück. (Standard – behält das bisherige Verhalten bei.)", - "settings.workspace.rootResolution.firstFolder.description": "Verwende immer den ersten Workspace-Ordner (workspaceFolders[0]). Deterministisch – unabhängig davon, welche Datei gerade fokussiert ist." + "settings.workspace.rootResolution.firstFolder.description": "Verwende immer den ersten Workspace-Ordner (workspaceFolders[0]). Deterministisch – unabhängig davon, welche Datei gerade fokussiert ist.", + "settings.workspaceCommandListsEnabled.description": "Aktiviert die von Workspace-Einstellungen beigesteuerten Befehlslisten (die Werte von `zoo-code.allowedCommands` und `zoo-code.deniedCommands` in Workspace-Einstellungsdateien). Solange deaktiviert, haben Workspace-Befehlslisten keine Auswirkung. Wenn aktiviert, haben Einträge auf Benutzerebene Vorrang: Ein Eintrag auf Benutzerebene entscheidet über jeden Befehl, mit dem er übereinstimmt, und Workspace-Einträge gelten nur für Befehle, mit denen kein Eintrag auf Benutzerebene übereinstimmt. Diese Einstellung kann nur in den [Benutzereinstellungen](https://code.visualstudio.com/docs/getstarted/settings) geändert werden." } diff --git a/src/package.nls.es.json b/src/package.nls.es.json index 26d9f87e9d..e2762d26be 100644 --- a/src/package.nls.es.json +++ b/src/package.nls.es.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Aceptar certificados autofirmados del proxy. **Necesario para la inspección MITM.** ⚠️ Inseguro: úsalo solo para depuración local.", "settings.workspace.rootResolution.description": "Cómo resuelve Zoo la raíz del espacio de trabajo en un espacio de trabajo multi-raíz. La raíz se usa para localizar `.roomodes`, `.roo/mcp.json`, `.roo/rules/` y otra configuración del ámbito del proyecto. Cambiar este ajuste solo afecta a las búsquedas futuras; las tareas en ejecución conservan su raíz original.", "settings.workspace.rootResolution.activeEditor.description": "Usa la carpeta del espacio de trabajo que contiene el editor activo; recurre a la primera carpeta del espacio de trabajo. (Predeterminado: conserva el comportamiento heredado.)", - "settings.workspace.rootResolution.firstFolder.description": "Usa siempre la primera carpeta del espacio de trabajo (workspaceFolders[0]). Determinista: independiente del archivo que tenga el foco en ese momento." + "settings.workspace.rootResolution.firstFolder.description": "Usa siempre la primera carpeta del espacio de trabajo (workspaceFolders[0]). Determinista: independiente del archivo que tenga el foco en ese momento.", + "settings.workspaceCommandListsEnabled.description": "Habilita las listas de comandos aportadas por la configuración del espacio de trabajo (los valores de `zoo-code.allowedCommands` y `zoo-code.deniedCommands` en los archivos de configuración del espacio de trabajo). Mientras esté deshabilitada, las listas de comandos del espacio de trabajo no tienen ningún efecto. Cuando esté habilitada, las entradas de nivel de usuario tienen prioridad: una entrada de nivel de usuario decide cualquier comando que coincida, y las entradas del espacio de trabajo solo se aplican a los comandos con los que ninguna entrada de nivel de usuario coincide. Este ajuste solo se puede cambiar en la [configuración de usuario](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.fr.json b/src/package.nls.fr.json index 364a8878c4..e3d28cfcd3 100644 --- a/src/package.nls.fr.json +++ b/src/package.nls.fr.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Accepter les certificats auto-signés du proxy. **Requis pour l'inspection MITM.** ⚠️ Non sécurisé — à utiliser uniquement pour le debug local.", "settings.workspace.rootResolution.description": "Comment Zoo résout la racine de l'espace de travail dans un espace de travail multi-racine. La racine sert à localiser `.roomodes`, `.roo/mcp.json`, `.roo/rules/` et d'autres configurations propres au projet. Modifier ce paramètre n'affecte que les recherches futures ; les tâches en cours conservent leur racine d'origine.", "settings.workspace.rootResolution.activeEditor.description": "Utilise le dossier de l'espace de travail contenant l'éditeur actif ; revient au premier dossier de l'espace de travail. (Par défaut — conserve le comportement hérité.)", - "settings.workspace.rootResolution.firstFolder.description": "Utilise toujours le premier dossier de l'espace de travail (workspaceFolders[0]). Déterministe — indépendant du fichier actuellement ciblé." + "settings.workspace.rootResolution.firstFolder.description": "Utilise toujours le premier dossier de l'espace de travail (workspaceFolders[0]). Déterministe — indépendant du fichier actuellement ciblé.", + "settings.workspaceCommandListsEnabled.description": "Active les listes de commandes issues des paramètres de l'espace de travail (les valeurs de `zoo-code.allowedCommands` et `zoo-code.deniedCommands` dans les fichiers de paramètres de l'espace de travail). Lorsqu'elle est désactivée, les listes de commandes de l'espace de travail n'ont aucun effet. Lorsqu'elle est activée, les entrées de niveau utilisateur ont la priorité : une entrée de niveau utilisateur décide de toute commande à laquelle elle correspond, et les entrées de l'espace de travail ne s'appliquent qu'aux commandes pour lesquelles aucune entrée de niveau utilisateur ne correspond. Ce paramètre ne peut être modifié que dans les [paramètres utilisateur](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.hi.json b/src/package.nls.hi.json index db70bc8417..74ce2fd37a 100644 --- a/src/package.nls.hi.json +++ b/src/package.nls.hi.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Proxy से आने वाले self-signed certificates accept करो। **MITM inspection के लिए ज़रूरी।** ⚠️ Insecure — सिर्फ local debugging के लिए इस्तेमाल करो।", "settings.workspace.rootResolution.description": "Multi-root workspace में Zoo workspace root को कैसे resolve करता है। Root का इस्तेमाल `.roomodes`, `.roo/mcp.json`, `.roo/rules/` और दूसरी project-scoped configuration ढूंढने के लिए होता है। इस setting को बदलने का असर सिर्फ आगे की lookups पर पड़ता है; चल रहे tasks अपना original root बनाए रखते हैं।", "settings.workspace.rootResolution.activeEditor.description": "उस workspace folder का इस्तेमाल करो जिसमें active editor है; पहले workspace folder पर fall back करो। (Default — legacy behavior बनाए रखता है।)", - "settings.workspace.rootResolution.firstFolder.description": "हमेशा पहले workspace folder (workspaceFolders[0]) का इस्तेमाल करो। Deterministic — इस बात से स्वतंत्र कि अभी कौन सी file focused है।" + "settings.workspace.rootResolution.firstFolder.description": "हमेशा पहले workspace folder (workspaceFolders[0]) का इस्तेमाल करो। Deterministic — इस बात से स्वतंत्र कि अभी कौन सी file focused है।", + "settings.workspaceCommandListsEnabled.description": "Workspace settings द्वारा दी गई command lists को सक्रिय करো (workspace settings files में `zoo-code.allowedCommands` और `zoo-code.deniedCommands` के मान)। जब तक इह निष्क्रिय है, workspace command lists का कौनो प्रभाव नाहीं होतै। सक्रिय करतै पर user-level entries कौ प्राथमिकत मिलतै है: हर user-level entry उन सब्ह commands कौ तय करतै है जिनसै वह मेल खातै है, अऊर workspace entries केवल उन commands पर लागू होतै हैं जिनसै कौनो user-level entry मेल नाहीं खातै। इस setting कौ केवल [user settings](https://code.visualstudio.com/docs/getstarted/settings) में बदल जा सकतै है।" } diff --git a/src/package.nls.id.json b/src/package.nls.id.json index b7e150ee53..2e5d084ae8 100644 --- a/src/package.nls.id.json +++ b/src/package.nls.id.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Terima sertifikat self-signed dari proxy. **Diperlukan untuk inspeksi MITM.** ⚠️ Tidak aman — gunakan hanya untuk debugging lokal.", "settings.workspace.rootResolution.description": "Bagaimana Zoo menentukan root workspace di workspace multi-root. Root digunakan untuk menemukan `.roomodes`, `.roo/mcp.json`, `.roo/rules/`, dan konfigurasi lain yang bersifat project-scoped. Mengubah pengaturan ini hanya memengaruhi pencarian berikutnya; task yang sedang berjalan tetap memakai root aslinya.", "settings.workspace.rootResolution.activeEditor.description": "Gunakan folder workspace yang berisi editor aktif; kembali ke folder workspace pertama. (Default — mempertahankan perilaku lama.)", - "settings.workspace.rootResolution.firstFolder.description": "Selalu gunakan folder workspace pertama (workspaceFolders[0]). Deterministik — tidak bergantung pada file mana yang sedang difokuskan." + "settings.workspace.rootResolution.firstFolder.description": "Selalu gunakan folder workspace pertama (workspaceFolders[0]). Deterministik — tidak bergantung pada file mana yang sedang difokuskan.", + "settings.workspaceCommandListsEnabled.description": "Aktifkan daftar perintah yang disediakan oleh pengaturan workspace (nilai `zoo-code.allowedCommands` dan `zoo-code.deniedCommands` di file pengaturan workspace). Saat dinonaktifkan, daftar perintah workspace tidak memiliki efek apa pun. Saat diaktifkan, entri level pengguna lebih diutamakan: entri level pengguna menentukan perintah mana pun yang cocok dengannya, dan entri workspace hanya berlaku untuk perintah yang tidak cocok dengan entri level pengguna mana pun. Pengaturan ini hanya dapat diubah di [pengaturan pengguna](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.it.json b/src/package.nls.it.json index 6d50b96bc4..6ec6801fc4 100644 --- a/src/package.nls.it.json +++ b/src/package.nls.it.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Accetta certificati autofirmati dal proxy. **Necessario per l'ispezione MITM.** ⚠️ Non sicuro — usalo solo per il debugging locale.", "settings.workspace.rootResolution.description": "Come Zoo determina la radice del workspace in un workspace multi-radice. La radice viene usata per individuare `.roomodes`, `.roo/mcp.json`, `.roo/rules/` e altra configurazione a livello di progetto. Modificare questa impostazione influisce solo sulle ricerche future; i task in esecuzione mantengono la loro radice originale.", "settings.workspace.rootResolution.activeEditor.description": "Usa la cartella del workspace che contiene l'editor attivo; ripiega sulla prima cartella del workspace. (Predefinito — mantiene il comportamento legacy.)", - "settings.workspace.rootResolution.firstFolder.description": "Usa sempre la prima cartella del workspace (workspaceFolders[0]). Deterministico — indipendente dal file attualmente attivo." + "settings.workspace.rootResolution.firstFolder.description": "Usa sempre la prima cartella del workspace (workspaceFolders[0]). Deterministico — indipendente dal file attualmente attivo.", + "settings.workspaceCommandListsEnabled.description": "Abilita gli elenchi di comandi forniti dalle impostazioni del workspace (i valori di `zoo-code.allowedCommands` e `zoo-code.deniedCommands` nei file delle impostazioni del workspace). Finché è disabilitata, gli elenchi di comandi del workspace non hanno alcun effetto. Quando è abilitata, le voci a livello utente hanno la precedenza: una voce a livello utente decide qualsiasi comando con cui corrisponde, e le voci del workspace si applicano solo ai comandi con cui nessuna voce a livello utente corrisponde. Questa impostazione può essere modificata solo nelle [impostazioni utente](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.ja.json b/src/package.nls.ja.json index 22f4f0dfce..8c48c277e0 100644 --- a/src/package.nls.ja.json +++ b/src/package.nls.ja.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "プロキシからの自己署名証明書を許可します。**MITM インスペクションに必須です。** ⚠️ 危険な設定なので、ローカルでのデバッグにだけ使用してください。", "settings.workspace.rootResolution.description": "マルチルートワークスペースで Zoo がワークスペースルートをどのように解決するか。ルートは `.roomodes`、`.roo/mcp.json`、`.roo/rules/` などのプロジェクト単位の設定を見つけるために使われます。この設定の変更は今後の検索にのみ影響し、実行中のタスクは元のルートを保持します。", "settings.workspace.rootResolution.activeEditor.description": "アクティブなエディタを含むワークスペースフォルダを使用し、最初のワークスペースフォルダにフォールバックします。（デフォルト — 従来の動作を維持します。）", - "settings.workspace.rootResolution.firstFolder.description": "常に最初のワークスペースフォルダ（workspaceFolders[0]）を使用します。決定的で、現在フォーカスされているファイルに依存しません。" + "settings.workspace.rootResolution.firstFolder.description": "常に最初のワークスペースフォルダ（workspaceFolders[0]）を使用します。決定的で、現在フォーカスされているファイルに依存しません。", + "settings.workspaceCommandListsEnabled.description": "ワークスペース設定が提供するコマンドリスト（ワークスペースの設定ファイル内の `zoo-code.allowedCommands` と `zoo-code.deniedCommands` の値）を有効にします。無効の間は、ワークスペースのコマンドリストは効果を持ちません。有効にすると、ユーザーレベルのエントリが優先されます。ユーザーレベルのエントリは、一致するコマンドをすべて決定し、ワークスペースのエントリはユーザーレベルのエントリが一致しないコマンドにのみ適用されます。この設定は[ユーザー設定](https://code.visualstudio.com/docs/getstarted/settings)でのみ変更できます。" } diff --git a/src/package.nls.json b/src/package.nls.json index d1f9afbc39..3d5a7caa33 100644 --- a/src/package.nls.json +++ b/src/package.nls.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Accept self-signed certificates from the proxy. **Required for MITM inspection.** ⚠️ Insecure — only use for local debugging.", "settings.workspace.rootResolution.description": "How Zoo resolves the workspace root in a multi-root workspace. The root is used to locate `.roomodes`, `.roo/mcp.json`, `.roo/rules/`, and other project-scoped configuration. Changing this setting only affects future lookups; running tasks keep their original root.", "settings.workspace.rootResolution.activeEditor.description": "Use the workspace folder containing the active editor; fall back to the first workspace folder. (Default — preserves legacy behavior.)", - "settings.workspace.rootResolution.firstFolder.description": "Always use the first workspace folder (workspaceFolders[0]). Deterministic — independent of which file is currently focused." + "settings.workspace.rootResolution.firstFolder.description": "Always use the first workspace folder (workspaceFolders[0]). Deterministic — independent of which file is currently focused.", + "settings.workspaceCommandListsEnabled.description": "Enable command lists contributed by workspace settings (`zoo-code.allowedCommands` and `zoo-code.deniedCommands` values in workspace settings files). While disabled, workspace command lists have no effect. When enabled, user-level entries take precedence: a user-level entry decides any command it matches, and workspace entries only apply to commands no user-level entry matches. This setting can only be changed in [user settings](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.ko.json b/src/package.nls.ko.json index 48d8695a1d..7546304047 100644 --- a/src/package.nls.ko.json +++ b/src/package.nls.ko.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "프록시의 self-signed 인증서를 허용합니다. **MITM 검사에 필요합니다.** ⚠️ 안전하지 않으므로 로컬 디버깅에만 사용하세요.", "settings.workspace.rootResolution.description": "멀티 루트 워크스페이스에서 Zoo가 워크스페이스 루트를 어떻게 결정하는지 설정합니다. 루트는 `.roomodes`, `.roo/mcp.json`, `.roo/rules/` 및 기타 프로젝트 범위 구성을 찾는 데 사용됩니다. 이 설정을 변경해도 이후의 조회에만 영향을 주며, 실행 중인 작업은 원래 루트를 유지합니다.", "settings.workspace.rootResolution.activeEditor.description": "활성 편집기가 있는 워크스페이스 폴더를 사용하고, 첫 번째 워크스페이스 폴더로 폴백합니다. (기본값 — 기존 동작을 유지합니다.)", - "settings.workspace.rootResolution.firstFolder.description": "항상 첫 번째 워크스페이스 폴더(workspaceFolders[0])를 사용합니다. 결정적이며, 현재 포커스된 파일과 무관합니다." + "settings.workspace.rootResolution.firstFolder.description": "항상 첫 번째 워크스페이스 폴더(workspaceFolders[0])를 사용합니다. 결정적이며, 현재 포커스된 파일과 무관합니다.", + "settings.workspaceCommandListsEnabled.description": "워크스페이스 설정이 제공하는 명령 목록을 활성화합니다(워크스페이스 설정 파일의 `zoo-code.allowedCommands` 및 `zoo-code.deniedCommands` 값). 비활성화된 동안에는 워크스페이스 명령 목록이 효과가 없습니다. 활성화하면 사용자 수준 항목이 우선합니다: 사용자 수준 항목은 자신이 일치시키는 모든 명령을 결정하며, 워크스페이스 항목은 사용자 수준 항목이 일치시키지 않는 명령에만 적용됩니다. 이 설정은 [사용자 설정](https://code.visualstudio.com/docs/getstarted/settings)에서만 변경할 수 있습니다." } diff --git a/src/package.nls.nl.json b/src/package.nls.nl.json index 81113c4b5a..46f5eab9bd 100644 --- a/src/package.nls.nl.json +++ b/src/package.nls.nl.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Accepteer zelfondertekende certificaten van de proxy. **Vereist voor MITM-inspectie.** ⚠️ Onveilig — gebruik dit alleen voor lokale debugging.", "settings.workspace.rootResolution.description": "Hoe Zoo de workspace-root bepaalt in een multi-root workspace. De root wordt gebruikt om `.roomodes`, `.roo/mcp.json`, `.roo/rules/` en andere project-scoped configuratie te vinden. Deze instelling wijzigen heeft alleen invloed op toekomstige zoekopdrachten; lopende taken behouden hun oorspronkelijke root.", "settings.workspace.rootResolution.activeEditor.description": "Gebruik de workspace-map die de actieve editor bevat; val terug op de eerste workspace-map. (Standaard — behoudt het oude gedrag.)", - "settings.workspace.rootResolution.firstFolder.description": "Gebruik altijd de eerste workspace-map (workspaceFolders[0]). Deterministisch — onafhankelijk van welk bestand op dit moment de focus heeft." + "settings.workspace.rootResolution.firstFolder.description": "Gebruik altijd de eerste workspace-map (workspaceFolders[0]). Deterministisch — onafhankelijk van welk bestand op dit moment de focus heeft.", + "settings.workspaceCommandListsEnabled.description": "Schakel commandolijsten in die worden bijgedragen door werkruimte-instellingen (de waarden van `zoo-code.allowedCommands` en `zoo-code.deniedCommands` in instellingenbestanden van de werkruimte). Terwijl uitgeschakeld hebben commandolijsten van de werkruimte geen effect. Wanneer ingeschakeld hebben items op gebruikersniveau voorrang: een item op gebruikersniveau beslist elk commando dat ermee overeenkomt, en werkruimte-items zijn alleen van toepassing op commando's waar geen item op gebruikersniveau mee overeenkomt. Deze instelling kan alleen worden gewijzigd in de [gebruikersinstellingen](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.pl.json b/src/package.nls.pl.json index 267ddff277..a6378085f1 100644 --- a/src/package.nls.pl.json +++ b/src/package.nls.pl.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Akceptuj certyfikaty self-signed z proxy. **Wymagane do inspekcji MITM.** ⚠️ Niezabezpieczone — używaj tylko do lokalnego debugowania.", "settings.workspace.rootResolution.description": "Jak Zoo ustala katalog główny workspace w workspace z wieloma katalogami głównymi. Katalog główny służy do lokalizowania `.roomodes`, `.roo/mcp.json`, `.roo/rules/` i innej konfiguracji na poziomie projektu. Zmiana tego ustawienia wpływa tylko na przyszłe wyszukiwania; uruchomione zadania zachowują swój pierwotny katalog główny.", "settings.workspace.rootResolution.activeEditor.description": "Użyj folderu workspace zawierającego aktywny edytor; w razie potrzeby wróć do pierwszego folderu workspace. (Domyślnie — zachowuje dotychczasowe zachowanie.)", - "settings.workspace.rootResolution.firstFolder.description": "Zawsze używaj pierwszego folderu workspace (workspaceFolders[0]). Deterministyczne — niezależne od tego, który plik jest aktualnie aktywny." + "settings.workspace.rootResolution.firstFolder.description": "Zawsze używaj pierwszego folderu workspace (workspaceFolders[0]). Deterministyczne — niezależne od tego, który plik jest aktualnie aktywny.", + "settings.workspaceCommandListsEnabled.description": "Włącza listy poleceń dostarczane przez ustawienia workspace (wartości `zoo-code.allowedCommands` i `zoo-code.deniedCommands` w plikach ustawień workspace). Gdy jest wyłączona, listy poleceń workspace nie mają żadnego efektu. Gdy jest włączona, wpisy na poziomie użytkownika mają pierwszeństwo: wpis na poziomie użytkownika rozstrzyga każde polecenie, z którym się dopasuje, a wpisy workspace dotyczą tylko poleceń, z którymi nie dopasuje się żaden wpis na poziomie użytkownika. To ustawienie można zmienić tylko w [ustawieniach użytkownika](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.pt-BR.json b/src/package.nls.pt-BR.json index 853832b699..9824a9fd67 100644 --- a/src/package.nls.pt-BR.json +++ b/src/package.nls.pt-BR.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Aceitar certificados self-signed do proxy. **Necessário para inspeção MITM.** ⚠️ Inseguro — use apenas para depuração local.", "settings.workspace.rootResolution.description": "Como o Zoo resolve a raiz do workspace em um workspace multi-raiz. A raiz é usada para localizar `.roomodes`, `.roo/mcp.json`, `.roo/rules/` e outras configurações no escopo do projeto. Alterar esta configuração afeta apenas as buscas futuras; as tarefas em execução mantêm sua raiz original.", "settings.workspace.rootResolution.activeEditor.description": "Usa a pasta do workspace que contém o editor ativo; recorre à primeira pasta do workspace. (Padrão — preserva o comportamento legado.)", - "settings.workspace.rootResolution.firstFolder.description": "Usa sempre a primeira pasta do workspace (workspaceFolders[0]). Determinístico — independente de qual arquivo está em foco no momento." + "settings.workspace.rootResolution.firstFolder.description": "Usa sempre a primeira pasta do workspace (workspaceFolders[0]). Determinístico — independente de qual arquivo está em foco no momento.", + "settings.workspaceCommandListsEnabled.description": "Ativa as listas de comandos fornecidas pelas configurações do workspace (os valores de `zoo-code.allowedCommands` e `zoo-code.deniedCommands` nos arquivos de configurações do workspace). Enquanto desativada, as listas de comandos do workspace não têm efeito. Quando ativada, as entradas em nível de usuário têm precedência: uma entrada em nível de usuário decide qualquer comando com o qual corresponda, e as entradas do workspace se aplicam apenas a comandos com os quais nenhuma entrada em nível de usuário corresponde. Essa configuração só pode ser alterada nas [configurações de usuário](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.ru.json b/src/package.nls.ru.json index d46ea14245..e3231a3bcf 100644 --- a/src/package.nls.ru.json +++ b/src/package.nls.ru.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Принимать self-signed сертификаты от прокси. **Требуется для MITM-инспекции.** ⚠️ Небезопасно — используй только для локальной отладки.", "settings.workspace.rootResolution.description": "Как Zoo определяет корень рабочей области в рабочей области с несколькими корнями. Корень используется для поиска `.roomodes`, `.roo/mcp.json`, `.roo/rules/` и другой конфигурации уровня проекта. Изменение этой настройки влияет только на последующие поиски; выполняющиеся задачи сохраняют свой исходный корень.", "settings.workspace.rootResolution.activeEditor.description": "Использовать папку рабочей области, содержащую активный редактор; при необходимости вернуться к первой папке рабочей области. (По умолчанию — сохраняет прежнее поведение.)", - "settings.workspace.rootResolution.firstFolder.description": "Всегда использовать первую папку рабочей области (workspaceFolders[0]). Детерминированно — не зависит от того, какой файл сейчас в фокусе." + "settings.workspace.rootResolution.firstFolder.description": "Всегда использовать первую папку рабочей области (workspaceFolders[0]). Детерминированно — не зависит от того, какой файл сейчас в фокусе.", + "settings.workspaceCommandListsEnabled.description": "Включает списки команд, задаваемые в настройках рабочей области (значения `zoo-code.allowedCommands` и `zoo-code.deniedCommands` в файлах настроек рабочей области). Пока отключено, списки команд рабочей области ни на что не влияют. Когда включено, записи уровня пользователя имеют приоритет: запись уровня пользователя решает любую подошедшую ей команду, а записи рабочей области применяются только к командам, которые не совпали ни с одной записью уровня пользователя. Этот параметр можно изменить только в [настройках пользователя](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.tr.json b/src/package.nls.tr.json index cbc81d959f..25f1df2fc8 100644 --- a/src/package.nls.tr.json +++ b/src/package.nls.tr.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Proxy'den gelen self-signed sertifikaları kabul et. **MITM incelemesi için gerekli.** ⚠️ Güvensiz — yalnızca lokal debugging için kullan.", "settings.workspace.rootResolution.description": "Zoo'nun çok köklü bir çalışma alanında çalışma alanı kökünü nasıl belirlediği. Kök; `.roomodes`, `.roo/mcp.json`, `.roo/rules/` ve diğer proje kapsamındaki yapılandırmayı bulmak için kullanılır. Bu ayarı değiştirmek yalnızca sonraki aramaları etkiler; çalışan görevler özgün köklerini korur.", "settings.workspace.rootResolution.activeEditor.description": "Aktif düzenleyiciyi içeren çalışma alanı klasörünü kullan; ilk çalışma alanı klasörüne geri dön. (Varsayılan — eski davranışı korur.)", - "settings.workspace.rootResolution.firstFolder.description": "Her zaman ilk çalışma alanı klasörünü (workspaceFolders[0]) kullan. Deterministik — o anda hangi dosyanın odakta olduğundan bağımsız." + "settings.workspace.rootResolution.firstFolder.description": "Her zaman ilk çalışma alanı klasörünü (workspaceFolders[0]) kullan. Deterministik — o anda hangi dosyanın odakta olduğundan bağımsız.", + "settings.workspaceCommandListsEnabled.description": "Çalışma alanı ayarlarının sağladığı komut listelerini etkinleştirir (çalışma alanı ayar dosyalarındaki `zoo-code.allowedCommands` ve `zoo-code.deniedCommands` değerleri). Devre dışıyken çalışma alanı komut listelerinin hiçbir etkisi olmaz. Etkinleştirildiğinde kullanıcı düzeyi girdiler önceliklidir: bir kullanıcı düzeyi girdi eşleştiği her komutu kararlaştırır ve çalışma alanı girdileri yalnızca hiçbir kullanıcı düzeyi girdinin eşleşmediği komutlara uygulanır. Bu ayar yalnızca [kullanıcı ayarlarında](https://code.visualstudio.com/docs/getstarted/settings) değiştirilebilir." } diff --git a/src/package.nls.vi.json b/src/package.nls.vi.json index 1ee4eabd70..f8913118c1 100644 --- a/src/package.nls.vi.json +++ b/src/package.nls.vi.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "Chấp nhận chứng chỉ self-signed từ proxy. **Bắt buộc cho việc kiểm tra MITM.** ⚠️ Không an toàn — chỉ dùng cho debug cục bộ.", "settings.workspace.rootResolution.description": "Cách Zoo xác định gốc workspace trong một workspace nhiều gốc. Gốc được dùng để tìm `.roomodes`, `.roo/mcp.json`, `.roo/rules/` và các cấu hình khác ở phạm vi dự án. Thay đổi cài đặt này chỉ ảnh hưởng đến các lần tra cứu về sau; các tác vụ đang chạy vẫn giữ gốc ban đầu của chúng.", "settings.workspace.rootResolution.activeEditor.description": "Dùng thư mục workspace chứa trình soạn thảo đang hoạt động; quay lại thư mục workspace đầu tiên. (Mặc định — giữ nguyên hành vi cũ.)", - "settings.workspace.rootResolution.firstFolder.description": "Luôn dùng thư mục workspace đầu tiên (workspaceFolders[0]). Mang tính xác định — không phụ thuộc vào tệp nào đang được focus." + "settings.workspace.rootResolution.firstFolder.description": "Luôn dùng thư mục workspace đầu tiên (workspaceFolders[0]). Mang tính xác định — không phụ thuộc vào tệp nào đang được focus.", + "settings.workspaceCommandListsEnabled.description": "Bật các danh sách lệnh do cài đặt workspace đóng góp (giá trị `zoo-code.allowedCommands` và `zoo-code.deniedCommands` trong các tệp cài đặt workspace). Khi bị tắt, các danh sách lệnh của workspace không có tác dụng. Khi được bật, các mục cấp người dùng được ưu tiên: một mục cấp người dùng quyết định bất kỳ lệnh nào mà nó khớp, và các mục của workspace chỉ áp dụng cho các lệnh không có mục cấp người dùng nào khớp. Cài đặt này chỉ có thể thay đổi trong [cài đặt người dùng](https://code.visualstudio.com/docs/getstarted/settings)." } diff --git a/src/package.nls.zh-CN.json b/src/package.nls.zh-CN.json index 85dafc3bc7..fe384c7efa 100644 --- a/src/package.nls.zh-CN.json +++ b/src/package.nls.zh-CN.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "接受来自代理的 self-signed 证书。**MITM 检查所必需。** ⚠️ 不安全——只在本地调试时使用。", "settings.workspace.rootResolution.description": "Zoo 在多根工作区中如何解析工作区根目录。根目录用于定位 `.roomodes`、`.roo/mcp.json`、`.roo/rules/` 以及其他项目范围的配置。更改此设置仅影响后续查找；正在运行的任务会保留其原始根目录。", "settings.workspace.rootResolution.activeEditor.description": "使用包含活动编辑器的工作区文件夹；回退到第一个工作区文件夹。（默认——保留旧行为。）", - "settings.workspace.rootResolution.firstFolder.description": "始终使用第一个工作区文件夹（workspaceFolders[0]）。具有确定性——与当前聚焦的文件无关。" + "settings.workspace.rootResolution.firstFolder.description": "始终使用第一个工作区文件夹（workspaceFolders[0]）。具有确定性——与当前聚焦的文件无关。", + "settings.workspaceCommandListsEnabled.description": "启用由工作区设置提供的命令列表（工作区设置文件中的 `zoo-code.allowedCommands` 和 `zoo-code.deniedCommands` 值）。禁用期间，工作区命令列表不产生任何影响。启用后，用户级条目具有优先权：用户级条目决定其匹配的任何命令，工作区条目仅适用于没有任何用户级条目匹配的命令。此设置只能在[用户设置](https://code.visualstudio.com/docs/getstarted/settings)中更改。" } diff --git a/src/package.nls.zh-TW.json b/src/package.nls.zh-TW.json index cda075e306..084e620258 100644 --- a/src/package.nls.zh-TW.json +++ b/src/package.nls.zh-TW.json @@ -50,5 +50,6 @@ "settings.debugProxy.tlsInsecure.description": "接受來自代理的 self-signed 憑證。**MITM 檢查所必需。** ⚠️ 不安全——只在本機偵錯時使用。", "settings.workspace.rootResolution.description": "Zoo 在多根工作區中如何解析工作區根目錄。根目錄用於定位 `.roomodes`、`.roo/mcp.json`、`.roo/rules/` 以及其他專案範圍的設定。變更此設定僅影響後續查找；正在執行的工作會保留其原始根目錄。", "settings.workspace.rootResolution.activeEditor.description": "使用包含使用中編輯器的工作區資料夾；回退到第一個工作區資料夾。（預設——保留舊行為。）", - "settings.workspace.rootResolution.firstFolder.description": "一律使用第一個工作區資料夾（workspaceFolders[0]）。具決定性——與目前聚焦的檔案無關。" + "settings.workspace.rootResolution.firstFolder.description": "一律使用第一個工作區資料夾（workspaceFolders[0]）。具決定性——與目前聚焦的檔案無關。", + "settings.workspaceCommandListsEnabled.description": "啟用由工作區設定所提供的工作區指令清單（工作區設定檔案中的 `zoo-code.allowedCommands` 與 `zoo-code.deniedCommands` 值）。停用時，工作區指令清單不會產生任何影響。啟用後，使用者層級的條目具有優先權：使用者層級的條目會決定其符合的任何指令，工作區條目僅適用於沒有任何使用者層級條目符合的指令。此設定只能在[使用者設定](https://code.visualstudio.com/docs/getstarted/settings)中變更。" } diff --git a/src/shared/tools.ts b/src/shared/tools.ts index 6d2e9422ee..e10f24c97c 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -19,6 +19,15 @@ export type AutoApprovalContext = { * produce a verdict — they surface as a retryable tool error beforehand. */ dcgDecision?: DcgDecision + /** + * Directory the command will actually run in (`execute_command` only): + * the tool's resolved working directory, which differs from the task's + * own `cwd` whenever the call carries a custom `cwd`. Command-list + * enforcement judges this directory so a command executing in another + * workspace folder is judged by that folder's policy, never the task + * folder's. + */ + commandCwd?: string } export type AskApproval = ( diff --git a/src/utils/__tests__/seedAllowedCommands.spec.ts b/src/utils/__tests__/seedAllowedCommands.spec.ts new file mode 100644 index 0000000000..9a60682952 --- /dev/null +++ b/src/utils/__tests__/seedAllowedCommands.spec.ts @@ -0,0 +1,443 @@ +// npx vitest run utils/__tests__/seedAllowedCommands.spec.ts + +import * as vscode from "vscode" + +import { readCommandListScopes } from "../../core/auto-approval/commandScope" +import { Package } from "../../shared/package" +import { makeUri, makeWorkspaceConfiguration } from "../../test-utils/vscode" +import { seedCommandList } from "../seedAllowedCommands" + +// Scriptable stand-in for `vscode.workspace.workspaceFolders`: the module +// mock's getter reads through this object, so a test can change the open +// folders without re-mocking the module. +const workspaceState = vi.hoisted(() => ({ + workspaceFolders: undefined as { uri: unknown }[] | undefined, +})) + +vi.mock("vscode", () => ({ + workspace: { + get workspaceFolders() { + return workspaceState.workspaceFolders + }, + getConfiguration: vi.fn(), + }, +})) + +interface ConfigDouble { + inspect?: { + key: string + defaultValue?: string[] + globalValue?: string[] + workspaceValue?: string[] + workspaceFolderValue?: string[] + } + effective?: string[] + inspectThrows?: boolean + gate?: boolean +} + +function mockConfiguration({ inspect, effective = [], inspectThrows = false, gate = false }: ConfigDouble = {}) { + // Real VS Code reports `workspaceFolderValue` only for a configuration + // looked up against a resource; an unscoped lookup reports no folder + // layer no matter what any folder's settings contain. Scripting that + // split per call is what lets these tests tell a scoped lookup apart + // from an unscoped one. + vi.mocked(vscode.workspace.getConfiguration).mockImplementation( + (_section?: string, scope?: vscode.ConfigurationScope | null) => { + const config = makeWorkspaceConfiguration() + // The shared double is scripted per call so the per-test fixture + // values need no type assertion at the construction site. The gate + // section reads independently of the list values: its shipped + // default is off, so a test only gets an enforced workspace layer + // by asking for one. + vi.mocked(config.get).mockImplementation((section: string) => + section === "workspaceCommandListsEnabled" ? gate : effective, + ) + vi.mocked(config.inspect).mockImplementation(() => { + if (inspectThrows) { + throw new Error("configuration access failed") + } + if (inspect === undefined) { + return undefined + } + // No resource to resolve against => no folder layer, matching + // the real contract for unscoped (and explicit-null) lookups. + return scope === undefined || scope === null ? { ...inspect, workspaceFolderValue: undefined } : inspect + }) + return config + }, + ) +} + +function mockMemento(existing: unknown = undefined, deniedExisting: unknown = undefined) { + const store = new Map() + if (existing !== undefined) { + store.set("allowedCommands", existing) + } + if (deniedExisting !== undefined) { + store.set("deniedCommands", deniedExisting) + } + + // Structurally complete Memento double (keys/get/update), so the call + // sites need no type assertion. + return { + keys: vi.fn((): readonly string[] => [...store.keys()]), + get: vi.fn().mockImplementation((key: string) => store.get(key)), + update: vi.fn().mockImplementation(async (key: string, value: unknown) => { + store.set(key, value) + }), + } +} + +describe("seedCommandList", () => { + beforeEach(() => { + vi.clearAllMocks() + workspaceState.workspaceFolders = undefined + }) + + it("leaves global state unset when only a workspace-scoped value exists", async () => { + // A real VS Code get() folds the workspace override into the effective + // value, so this is the leak scenario: reading get() would seed + // ["docker"] (from .vscode/settings.json) into global state. The + // defaultValue mirrors real VS Code, where the declared default is + // always present: the workspace override replaced it deliberately, so + // re-granting it from global state is also forbidden. + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + globalValue: undefined, + workspaceValue: ["docker"], + }, + effective: ["docker"], + // Suppression only applies while the gate lets the workspace layer + // take effect in enforcement. + gate: true, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("allowedCommands")).toBeUndefined() + }) + + it("leaves global state unset when only a workspace-folder-scoped value exists", async () => { + // A folder override is visible only to a lookup resolved against that + // folder's resource, so this fixture opens a folder and scripts the + // folder layer (which the resource-aware double reports only for the + // scoped call). An unscoped seed would read "no value anywhere" here + // and re-grant the default the folder removed — this test fails on + // that regression. + const folderA = makeUri("/mock/folder-a") + workspaceState.workspaceFolders = [{ uri: folderA }] + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + globalValue: undefined, + workspaceFolderValue: ["docker"], + }, + gate: true, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + // The folder layer only reaches the seed through a resource-scoped + // lookup, so the seed must request one (same pin as the save path). + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderA) + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("allowedCommands")).toBeUndefined() + }) + + it("resolves the folder layer against the first workspace folder only", async () => { + // Reading any other folder's configuration would silently decide the + // seed from the wrong folder's settings file. + const folderA = makeUri("/mock/folder-a") + const folderB = makeUri("/mock/folder-b") + workspaceState.workspaceFolders = [{ uri: folderA }, { uri: folderB }] + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + workspaceFolderValue: ["docker"], + }, + gate: true, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, folderA) + expect(vscode.workspace.getConfiguration).not.toHaveBeenCalledWith(Package.name, folderB) + expect(globalState.update).not.toHaveBeenCalled() + }) + + it("treats an explicitly empty workspace value as an override that suppresses the default", async () => { + // Presence is checked with `undefined`, not truthiness: a workspace that + // set the list to [] opted out of the default as deliberately as one + // that set ["docker"]. + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + globalValue: undefined, + workspaceValue: [], + }, + gate: true, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("allowedCommands")).toBeUndefined() + }) + + it("seeds the declared default on a fresh install with no value set at any scope", async () => { + const defaultValue = ["git log", "git diff", "git show"] + mockConfiguration({ + inspect: { key: "allowedCommands", defaultValue }, + effective: defaultValue, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", defaultValue) + // Copy, like the global-value path: global state must not share a + // reference with the configuration object. + expect(globalState.update.mock.calls[0][1]).not.toBe(defaultValue) + + // A later activation with the gate on but still no workspace-scoped + // value: only an actual override suppresses the default, so the gate + // alone must not withhold it. + mockConfiguration({ + inspect: { key: "allowedCommands", defaultValue }, + effective: defaultValue, + gate: true, + }) + const secondGlobalState = mockMemento() + + await seedCommandList(secondGlobalState, "allowedCommands") + + expect(secondGlobalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", defaultValue) + }) + + it("seeds from the global-scope value when global state is empty", async () => { + mockConfiguration({ inspect: { key: "allowedCommands", globalValue: ["git"] }, effective: ["git"] }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", ["git"]) + expect(globalState.get("allowedCommands")).toEqual(["git"]) + }) + + it("seeds an explicitly empty global value instead of the populated default", async () => { + // An explicitly empty global list is a deliberate opt-out: the seed + // persists [] so global state counts as seeded, rather than treating + // the empty allowlist as unset and re-granting the declared defaults. + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + globalValue: [], + }, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", []) + expect(globalState.get("allowedCommands")).toEqual([]) + }) + + it("does not overwrite global state that is already populated", async () => { + mockConfiguration({ inspect: { key: "allowedCommands", globalValue: ["git"] }, effective: ["git"] }) + const globalState = mockMemento(["npm"]) + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("allowedCommands")).toEqual(["npm"]) + }) + + it("leaves an explicitly empty global-state list untouched", async () => { + // The guard treats presence, not emptiness: saving [] is a deliberate + // choice to auto-approve nothing, and re-seeding the package default + // would silently re-grant the approvals the user just revoked. + mockConfiguration({ inspect: { key: "allowedCommands", defaultValue: ["git log", "git diff", "git show"] } }) + const globalState = mockMemento([]) + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("allowedCommands")).toEqual([]) + }) + + it("composes a later workspace override as a union with the seeded defaults", async () => { + // Full schedule: a fresh activation seeds the declared default, then a + // workspace override appears in a later activation. The seed + // early-returns because global state already holds a list, so the + // override is never persisted — and the enforcement read composes the + // two layers as a set union. Neither list replaces the other; the + // longest-prefix matcher decides which entry governs a given command. + const defaultValue = ["git log", "git diff", "git show"] + mockConfiguration({ + inspect: { key: "allowedCommands", defaultValue }, + effective: defaultValue, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", defaultValue) + + // A later activation in a workspace whose settings file sets the list: + // the guard early-returns before any inspection, so no second write. + mockConfiguration({ + inspect: { key: "allowedCommands", defaultValue, workspaceValue: ["docker ps"] }, + effective: ["docker ps"], + }) + + await seedCommandList(globalState, "allowedCommands") + expect(globalState.update).toHaveBeenCalledTimes(1) + expect(globalState.get("allowedCommands")).toEqual(defaultValue) + + // The enforcement view (ClineProvider.getState) keeps both layers and + // merges them: the override adds entries, it does not replace the + // seeded defaults, and vice versa. + const scopes = readCommandListScopes("allowedCommands", globalState.get("allowedCommands")) + expect(scopes.global).toEqual(defaultValue) + expect(scopes.workspace).toEqual(["docker ps"]) + expect(scopes.merged).toEqual([...defaultValue, "docker ps"]) + }) + + it("inspects the global value under the extension's own section key", async () => { + // The seed must read the extension's own section; asking VS Code about + // a different (or empty) section key would silently never find the + // user's global list. With no folder open there is no resource to + // scope against, so the explicit unscoped call is (section, undefined). + mockConfiguration({ inspect: { key: "allowedCommands", globalValue: ["git"] } }) + // Structurally complete Memento double (keys/get/update), so the call + // site needs no type assertion. + const globalState: vscode.Memento = { + keys: () => [], + get: vi.fn(), + update: vi.fn(), + } + + await seedCommandList(globalState, "allowedCommands") + + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith(Package.name, undefined) + const config = vi.mocked(vscode.workspace.getConfiguration).mock.results[0].value + expect(config.inspect).toHaveBeenCalledWith("allowedCommands") + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", ["git"]) + }) + + it("leaves global state untouched when there is no inspect result at all", async () => { + mockConfiguration({ inspect: undefined, effective: ["git"] }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + }) + + it("skips the seed without throwing when configuration access fails", async () => { + mockConfiguration({ inspectThrows: true }) + const globalState = mockMemento() + + await expect(seedCommandList(globalState, "allowedCommands")).resolves.toBeUndefined() + + expect(globalState.update).not.toHaveBeenCalled() + }) + + it("does not share a reference with the inspected configuration value", async () => { + const globalValue = ["git"] + mockConfiguration({ inspect: { key: "allowedCommands", globalValue } }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).toHaveBeenCalledWith("allowedCommands", expect.arrayContaining(["git"])) + expect(globalState.update.mock.calls[0][1]).not.toBe(globalValue) + }) + + it("seeds the deny list from its settings-file global value on a fresh profile", async () => { + // Symmetric with the allow side: a user-level `deniedCommands` edit in + // settings.json has a one-time migration role, and the enforcement + // global layer reads extension state only, so an unseeded deny list + // silently drops the user's denials. + mockConfiguration({ inspect: { key: "deniedCommands", globalValue: ["docker"] }, effective: ["docker"] }) + const globalState = mockMemento() + + await seedCommandList(globalState, "deniedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("deniedCommands", ["docker"]) + expect(globalState.get("deniedCommands")).toEqual(["docker"]) + }) + + it("does not overwrite an existing deniedCommands global state", async () => { + mockConfiguration({ inspect: { key: "deniedCommands", globalValue: ["docker"] }, effective: ["docker"] }) + const globalState = mockMemento(undefined, ["rm"]) + + await seedCommandList(globalState, "deniedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + expect(globalState.get("deniedCommands")).toEqual(["rm"]) + }) + + it("seeds the deny list when only the allow list was already seeded", async () => { + // Per-key guards: an install whose allowlist was seeded by an earlier + // version must still get the deny migration on the next activation. + mockConfiguration({ inspect: { key: "deniedCommands", globalValue: ["docker"] }, effective: ["docker"] }) + const globalState = mockMemento(["npm"]) + + await seedCommandList(globalState, "deniedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("deniedCommands", ["docker"]) + expect(globalState.get("allowedCommands")).toEqual(["npm"]) + }) + + it("seeds the declared default when a workspace value exists but the gate is off", async () => { + // With the gate off the workspace layer contributes nothing to + // enforcement, so withholding the default would leave commands + // unapproved that the override never actually governed. + const defaultValue = ["git log", "git diff", "git show"] + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue, + workspaceValue: ["docker"], + }, + effective: ["docker"], + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).toHaveBeenCalledExactlyOnceWith("allowedCommands", defaultValue) + }) + + it("suppresses the declared default for a workspace value while the gate is on", async () => { + // Pins the explicit-override semantics: an enforced workspace list + // replaces the default across scopes, so the seed must not re-grant + // the approvals the override removed. + mockConfiguration({ + inspect: { + key: "allowedCommands", + defaultValue: ["git log", "git diff", "git show"], + workspaceValue: ["docker"], + }, + effective: ["docker"], + gate: true, + }) + const globalState = mockMemento() + + await seedCommandList(globalState, "allowedCommands") + + expect(globalState.update).not.toHaveBeenCalled() + }) +}) diff --git a/src/utils/seedAllowedCommands.ts b/src/utils/seedAllowedCommands.ts new file mode 100644 index 0000000000..be4b930ada --- /dev/null +++ b/src/utils/seedAllowedCommands.ts @@ -0,0 +1,78 @@ +import * as vscode from "vscode" + +import { commandListConfiguration, workspaceCommandListsEnabled } from "../core/auto-approval/commandScope" +import type { CommandListConfigKey } from "../core/auto-approval/commandScope" + +/** + * Seeds one auto-approval command-list key (`allowedCommands` or + * `deniedCommands`) from its settings-file value into the extension's + * global state, but only when global state holds no list for that key yet + * (first run). Each key carries its own guard, so a profile whose allowlist + * was already seeded still gets an unseeded deny list migrated on the next + * activation. + * + * The seed reads scoped values via `inspect()` rather than the effective + * `get()` on purpose: `get()` folds in workspace-scoped overrides, so a + * project's `.vscode/settings.json` would otherwise be copied permanently + * into global state the first time the extension activates inside that + * project — a leak that survives closing the project. The lookup goes + * through `commandListConfiguration()`, the folder-scoped accessor shared + * with the command-list read and write paths: only a configuration resolved + * against a resource reports `workspaceFolderValue` at all, so the folder + * layer is visible here exactly where enforcement can see it. In a + * multi-root workspace only the first folder is in scope; with no folder + * open the configuration is unscoped and reports no folder value. + * + * Selection: the global-scope value wins. With nothing set at any scope (a + * fresh install) the declared package default is seeded, because the + * enforcement path reads only global state plus workspace scopes and never + * falls back to that default on its own. A workspace-scoped value is an + * explicit choice and suppresses the default — but only while the + * `workspaceCommandListsEnabled` gate lets that value take effect: with the + * gate off the workspace layer contributes nothing to enforcement, so + * withholding the default would leave commands unapproved that the override + * never actually governed. Array settings replace across scopes, so seeding + * past an enforced override would re-grant the approvals it removed. + * + * @param globalState - The extension global-state memento to seed. + * @param configKey - The command-list configuration key to seed. + */ +export async function seedCommandList(globalState: vscode.Memento, configKey: CommandListConfigKey): Promise { + if (globalState.get(configKey)) { + return + } + + // Only the configuration lookup itself is guarded: it can fail (for example + // before the workspace is ready), and skipping the seed then only defers it + // to a later activation. The `inspected` reads below stay outside the + // guarded region so a genuine lookup failure stays distinguishable from an + // absent value rather than being swallowed as one. + let inspected: + | { + globalValue?: string[] + defaultValue?: string[] + workspaceValue?: string[] + workspaceFolderValue?: string[] + } + | undefined + try { + inspected = commandListConfiguration().inspect(configKey) + } catch { + return + } + + // The `undefined` checks, not truthiness: an explicitly empty array still + // counts as an override. + const workspaceOverride = inspected?.workspaceValue !== undefined || inspected?.workspaceFolderValue !== undefined + + const seedValue = + inspected?.globalValue ?? + (workspaceOverride && workspaceCommandListsEnabled() ? undefined : inspected?.defaultValue) + + if (!Array.isArray(seedValue)) { + return + } + + // Copy so global state never shares a reference with the configuration object. + await globalState.update(configKey, [...seedValue]) +} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 26955663ee..a0fea12123 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -103,6 +103,25 @@ export const settingsTabTrigger = "whitespace-nowrap overflow-hidden min-w-0 h-12 px-4 py-3 box-border flex items-center border-l-2 border-transparent text-vscode-foreground opacity-70 hover:bg-vscode-list-hoverBackground data-[compact=true]:w-12 data-[compact=true]:p-4" export const settingsTabTriggerActive = "opacity-100 border-vscode-focusBorder bg-vscode-list-activeSelectionBackground" +// Command-list snapshot of a settings state object. Entries are compared by value because every +// state post delivers freshly deserialized arrays; reference identity carries no meaning here. +interface CommandListSnapshot { + allowedCommands?: string[] + deniedCommands?: string[] + workspaceCommandListsEnabled?: boolean +} + +const commandListSnapshot = (state: CommandListSnapshot): string => + JSON.stringify([ + state.allowedCommands ?? [], + state.deniedCommands ?? [], + state.workspaceCommandListsEnabled ?? false, + ]) + +const commandListsEquivalent = (left?: string[], right?: string[]) => + left === right || + (!!left && !!right && left.length === right.length && left.every((entry, index) => entry === right[index])) + export interface SettingsViewRef { checkUnsaveChanges: (then: () => void) => void } @@ -167,6 +186,13 @@ const SettingsView = forwardRef(({ onDone, t const latestCachedState = useRef(cachedState) latestCachedState.current = cachedState + // Command-list values the dialog last displayed, so a later state post can be recognized as + // an external change rather than a restatement of what is already shown. + const lastDisplayedCommandSnapshot = useRef(commandListSnapshot(extensionState)) + // An in-flight user edit of either list claims ownership of the lists until Save, discard, or + // a full reseed; external list updates must not silently overwrite it. + const commandListsTouched = useRef(false) + const { alwaysAllowReadOnly, alwaysAllowReadOnlyOutsideWorkspace, @@ -252,6 +278,8 @@ const SettingsView = forwardRef(({ onDone, t setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState })) prevApiConfigName.current = currentApiConfigName prevMode.current = mode + lastDisplayedCommandSnapshot.current = commandListSnapshot(extensionState) + commandListsTouched.current = false setChangeDetected(false) }, [currentApiConfigName, mode, extensionState]) @@ -263,15 +291,79 @@ const SettingsView = forwardRef(({ onDone, t handledSettingsImportedAt.current = settingsImportedAt setCachedState((prevCachedState) => ({ ...prevCachedState, ...extensionState })) + lastDisplayedCommandSnapshot.current = commandListSnapshot(extensionState) + commandListsTouched.current = false setChangeDetected(false) }, [settingsImportedAt, extensionState]) + // The command lists can change in the extension host while this dialog is open (for example, + // a settings-file edit landing mid-session). Fold exactly those fields into the edit buffer on + // every state post that changes them, so a later Save cannot rewrite the configuration layer + // from a list that never showed the new entry. List fields are held back while the user has an + // in-flight list edit (in-flight edits win); every other buffered field is left untouched and + // the dirty flag is never cleared here, keeping the buffer's Save-gated semantics. + useEffect(() => { + const snapshot = commandListSnapshot(extensionState) + if (snapshot === lastDisplayedCommandSnapshot.current) { + return + } + + setCachedState((prevState) => { + const merged: CommandListSnapshot = {} + let changedFields = 0 + + if (!commandListsTouched.current) { + if (!commandListsEquivalent(prevState.allowedCommands, extensionState.allowedCommands)) { + merged.allowedCommands = extensionState.allowedCommands + changedFields++ + } + if (!commandListsEquivalent(prevState.deniedCommands, extensionState.deniedCommands)) { + merged.deniedCommands = extensionState.deniedCommands + changedFields++ + } + } + if (prevState.workspaceCommandListsEnabled !== extensionState.workspaceCommandListsEnabled) { + merged.workspaceCommandListsEnabled = extensionState.workspaceCommandListsEnabled + changedFields++ + } + + // The stamp records the lists this merge leaves on screen, never the posted + // state outright: an entry held back by an in-flight edit was never displayed, + // and a baseline claiming it had been lets the save erase it as a user + // deletion without warning. While the lists are held back the buffered lists + // are themselves unposted, so the stamp keeps its prior lists instead - a + // deletion inside the buffered edit must stay in the baseline for the save to + // apply it rather than resurrect the deleted entry as an external addition. + const [stampAllowed, stampDenied] = commandListsTouched.current + ? (JSON.parse(lastDisplayedCommandSnapshot.current) as [string[], string[], boolean]) + : [ + merged.allowedCommands ?? prevState.allowedCommands, + merged.deniedCommands ?? prevState.deniedCommands, + ] + lastDisplayedCommandSnapshot.current = commandListSnapshot({ + allowedCommands: stampAllowed, + deniedCommands: stampDenied, + workspaceCommandListsEnabled: + merged.workspaceCommandListsEnabled ?? prevState.workspaceCommandListsEnabled, + }) + + if (changedFields === 0) { + return prevState + } + return { ...prevState, ...merged } + }) + }, [extensionState]) + const setCachedStateField: SetCachedStateField = useCallback((field, value) => { setCachedState((prevState) => { if (prevState[field] === value) { return prevState } + if (field === "allowedCommands" || field === "deniedCommands") { + commandListsTouched.current = true + } + setChangeDetected(true) return { ...prevState, [field]: value } }) @@ -406,6 +498,12 @@ const SettingsView = forwardRef(({ onDone, t const handleSubmit = () => { if (isSettingValid) { + const [lastDisplayedAllowed, lastDisplayedDenied] = JSON.parse(lastDisplayedCommandSnapshot.current) as [ + string[], + string[], + boolean, + ] + const saveId = displayChanged ? nextSaveRequestId() : undefined if (saveId) { sentState.current = cachedState @@ -415,6 +513,10 @@ const SettingsView = forwardRef(({ onDone, t vscode.postMessage({ type: "updateSettings", requestId: saveId, + commandListBaseline: { + allowedCommands: lastDisplayedAllowed, + deniedCommands: lastDisplayedDenied, + }, updatedSettings: { language, alwaysAllowReadOnly: alwaysAllowReadOnly ?? undefined, @@ -499,9 +601,20 @@ const SettingsView = forwardRef(({ onDone, t vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting }) vscode.postMessage({ type: "debugSetting", bool: cachedState.debug }) - // With a time or date edit the flag is cleared when the host reports the save (see below). A failed save - // leaves the user's edits in place to retry. - if (!saveId) setChangeDetected(false) + lastDisplayedCommandSnapshot.current = commandListSnapshot({ + allowedCommands, + deniedCommands, + workspaceCommandListsEnabled: cachedState.workspaceCommandListsEnabled, + }) + // The stamp records what is on screen, so it moves at submit regardless of the outcome; the lists' + // ownership of the buffer does not. A save the host answers commits the buffered lists only once the + // host says the write landed (see the settle below) - committing at submit would let the state refresh + // that follows a rejected save overwrite the edit the user is still looking at. A save the host never + // answers has no result to wait for, so it clears the hold at submit. + if (!saveId) { + commandListsTouched.current = false + setChangeDetected(false) + } } } @@ -510,8 +623,13 @@ const SettingsView = forwardRef(({ onDone, t const settle = (success: boolean) => { setPendingSaveId(undefined) - // An edit made while the save was in flight is not in what was sent: it stays unsaved. - if (success && latestCachedState.current === sentState.current) setChangeDetected(false) + // An edit made while the save was in flight is not in what was sent: it stays unsaved. A failed save + // keeps the buffered lists holding off external updates too, so the host's refresh after a rejected + // write cannot replace the edit the user is looking at. + if (success && latestCachedState.current === sentState.current) { + commandListsTouched.current = false + setChangeDetected(false) + } } const onResult = (event: MessageEvent) => { @@ -547,6 +665,8 @@ const SettingsView = forwardRef(({ onDone, t if (confirm) { // Discard changes: Reset state and flag setCachedState(extensionState) // Revert to original state + lastDisplayedCommandSnapshot.current = commandListSnapshot(extensionState) + commandListsTouched.current = false // Buffered edits are gone, so live lists are not held off setChangeDetected(false) // Reset change flag confirmDialogHandler.current?.() // Execute the pending action (e.g., tab switch) } 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 6704aa1cf0..f8e4b95737 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 @@ -472,7 +472,7 @@ describe("SettingsView - Change Detection Fix", () => { it("should not show unsaved changes when no changes are made", async () => { const onDone = vi.fn() - ;(useExtensionState as any).mockReturnValue(createExtensionState()) + ;(useExtensionState as ReturnType).mockReturnValue(createExtensionState()) renderWithExtensionState(, { queryClient }) @@ -497,7 +497,7 @@ describe("SettingsView - Change Detection Fix", () => { }, 10000) it("persists a normalized reasoning default through Save", async () => { - ;(useExtensionState as any).mockReturnValue(createExtensionState()) + ;(useExtensionState as ReturnType).mockReturnValue(createExtensionState()) renderWithExtensionState(, { queryClient }) await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) @@ -544,7 +544,7 @@ describe("SettingsView - Change Detection Fix", () => { }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) const { rerender } = renderWithExtensionState(, { queryClient }) @@ -580,7 +580,7 @@ describe("SettingsView - Change Detection Fix", () => { basetenApiKey: "test-baseten-key", }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) rerender() }) @@ -615,7 +615,7 @@ describe("SettingsView - Change Detection Fix", () => { }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) const { rerender } = renderWithExtensionState(, { queryClient }) @@ -635,7 +635,7 @@ describe("SettingsView - Change Detection Fix", () => { basetenApiKey: "imported-baseten-key", }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) rerender() }) @@ -660,7 +660,7 @@ describe("SettingsView - Change Detection Fix", () => { }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) const { rerender } = renderWithExtensionState(, { queryClient }) @@ -720,7 +720,7 @@ describe("SettingsView - Change Detection Fix", () => { apiModelId: "friendli-model", }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) rerender() }) @@ -743,7 +743,7 @@ describe("SettingsView - Change Detection Fix", () => { }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) const { rerender } = renderWithExtensionState(, { queryClient }) @@ -766,7 +766,7 @@ describe("SettingsView - Change Detection Fix", () => { apiModelId: "gpt-4.1", }, }) - ;(useExtensionState as any).mockImplementation(() => extensionState) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) rerender() }) @@ -775,4 +775,109 @@ describe("SettingsView - Change Detection Fix", () => { expect(screen.getByTestId("provider-value")).toHaveTextContent("baseten") }, 20000) }) + + describe("command-list refresh while the dialog is open", () => { + it("merges an externally added deny entry into the buffer before a later save", async () => { + let extensionState = createExtensionState({ deniedCommands: [] }) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + + const { rerender } = renderWithExtensionState(, { queryClient }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) + + // Buffer an unrelated edit so a Save happens later in this dialog session. + fireEvent.click(screen.getByTestId("set-provider-baseten")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // A deny entry lands externally while the dialog is open. + await act(async () => { + extensionState = createExtensionState({ deniedCommands: ["rm -rf"] }) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + rerender() + }) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + // Save must submit the list that includes the entry the dialog now displays, not the + // stale one captured when the dialog opened. + fireEvent.click(screen.getByTestId("save-button")) + expect(mockPostMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ deniedCommands: ["rm -rf"] }), + }), + ) + }, 10000) + + it("baselines the save on the post-merge lists, not the lists shown when the dialog opened", async () => { + let extensionState = createExtensionState({ deniedCommands: [] }) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + + const { rerender } = renderWithExtensionState(, { queryClient }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) + + // Buffer an unrelated edit so a Save happens later in this dialog session. + fireEvent.click(screen.getByTestId("set-provider-baseten")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // A deny entry lands externally and the merge puts it into the displayed lists. + await act(async () => { + extensionState = createExtensionState({ deniedCommands: ["rm -rf"] }) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + rerender() + }) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + // The baseline must be the list the dialog NOW displays (post-merge), so the + // merge's entry counts as shown - not the empty list present when the dialog + // opened, which would make the merge's own entry look externally added. + fireEvent.click(screen.getByTestId("save-button")) + expect(mockPostMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + commandListBaseline: { allowedCommands: [], deniedCommands: ["rm -rf"] }, + }), + ) + }, 10000) + + it("merges without clearing unsaved changes when the command-list state flips externally", async () => { + let extensionState = createExtensionState() + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + + const { rerender } = renderWithExtensionState(, { queryClient }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) + + // Buffer an edit that touches neither command list, so the dialog is dirty. + fireEvent.click(screen.getByTestId("set-provider-baseten")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // One state post flips the workspace command-list gate and adds a deny entry, which is + // what enabling the gate does: workspace entries become part of the displayed lists. + await act(async () => { + extensionState = createExtensionState({ + deniedCommands: ["git push"], + workspaceCommandListsEnabled: true, + }) + ;(useExtensionState as ReturnType).mockImplementation(() => extensionState) + rerender() + }) + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + // The merge must not clear the dirty flag or discard the buffered provider edit... + expect(screen.getByTestId("provider-value")).toHaveTextContent("baseten") + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + // ...while the flip's list change still reaches the buffer and the next Save. + fireEvent.click(screen.getByTestId("save-button")) + expect(mockPostMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ deniedCommands: ["git push"] }), + }), + ) + }, 10000) + }) }) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx index d8958f87c6..30bc2016cf 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx @@ -767,6 +767,32 @@ describe("SettingsView - Allowed Commands", () => { ) }) + it("sends the lists displayed at open as the save baseline, unmoved by an in-flight list edit", () => { + // The save path classifies divergence between "user deleted this" and "the + // settings file gained this" from the baseline, so the payload must carry what + // the dialog displayed BEFORE this save - not the edited list it is submitting, + // and not the committed value the post-save bookkeeping writes back. + renderWithExtensionState(, { + state: { allowedCommands: ["git"], alwaysAllowExecute: true }, + }) + + const content = screen.getByTestId("settings-content") + // An in-flight addition lands in the edit buffer only. + const input = within(content).getByTestId("command-input") + fireEvent.change(input, { target: { value: "npm test" } }) + fireEvent.click(within(content).getByTestId("add-command-button")) + + fireEvent.click(screen.getByTestId("save-button")) + + expect(vscode.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + commandListBaseline: { allowedCommands: ["git"], deniedCommands: [] }, + updatedSettings: expect.objectContaining({ allowedCommands: ["git", "npm test"] }), + }), + ) + }) + it("removes command from the list", () => { // Render once and get the activateTab helper const { activateTab, getSettingsContent } = renderSettingsView() 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 1bc4ab1afb..eec8fc01a6 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 @@ -9,6 +9,8 @@ import React from "react" import SettingsView from "../SettingsView" import { vscode } from "@src/utils/vscode" +import { answerSave, lastSaveRequestId } from "./saveAcknowledgement" + const postMessage = vi.spyOn(vscode, "postMessage").mockImplementation(() => {}) // Mock the extension state context @@ -249,6 +251,7 @@ vi.mock("../SettingsSearch", () => ({ import { useExtensionState } from "@src/context/ExtensionStateContext" import ApiOptions from "../ApiOptions" +import { AutoApproveSettings } from "../AutoApproveSettings" describe("SettingsView - Unsaved Changes Detection", () => { let queryClient: QueryClient @@ -811,6 +814,236 @@ describe("SettingsView - Unsaved Changes Detection", () => { }) }) + it("keeps an in-flight deny-list edit when an external list update arrives while the dialog is open", async () => { + // Drive the real cached-state setter through a stand-in auto-approve section so the + // buffered list edit is indistinguishable from a user typing in the deny list. + vi.mocked(AutoApproveSettings).mockImplementation( + ({ setCachedStateField }: React.ComponentProps) => ( + + ), + ) + const view = renderWithExtensionState(, { + queryClient, + }) + + fireEvent.click(await screen.findByTestId("buffer-deny-edit")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // The deny list changes in the extension host while the dialog is open. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf"], + }) + view.rerender() + + // The user's unsaved list edit wins over the external update, and Save submits it. + fireEvent.click(screen.getByTestId("save-button")) + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ deniedCommands: ["git status"] }), + }), + ) + expect(postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ deniedCommands: ["rm -rf"] }), + }), + ) + }) + + it("baselines the save on the pre-edit displayed lists when a state post lands during an in-flight list edit", async () => { + // The baseline feeds the save's classification of every entry the submitted + // list omits, so it must hold the lists the dialog displayed BEFORE the + // in-flight edit. Folding the posted state in would claim the never-displayed + // entry had been shown, letting the save erase it as a user deletion; folding + // the edited buffer in would drop an in-flight deletion from the baseline, + // letting the save resurrect it as an external addition. + vi.mocked(AutoApproveSettings).mockImplementation( + ({ setCachedStateField }: React.ComponentProps) => ( + + ), + ) + const view = renderWithExtensionState(, { + queryClient, + }) + + fireEvent.click(await screen.findByTestId("buffer-deny-edit")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // A deny entry lands externally while the list edit holds the merge back. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf"], + }) + view.rerender() + + fireEvent.click(screen.getByTestId("save-button")) + + // The submit keeps the in-flight edit, and the baseline stays at the lists + // displayed before the edit - neither the held-back post's list nor the + // edited buffer's. + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + commandListBaseline: { allowedCommands: [], deniedCommands: [] }, + updatedSettings: expect.objectContaining({ deniedCommands: ["git status"] }), + }), + ) + }) + + it("keeps a deleted displayed entry in the save baseline when a state post lands during the deletion", async () => { + // The deletion counterpart: an entry the dialog displayed and the user then + // deleted must stay in the baseline, so the save classifies it as a user + // deletion and applies it. A baseline folded from the deletion-carrying + // buffer would make the entry look externally added and re-admit it. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf"], + }) + vi.mocked(AutoApproveSettings).mockImplementation( + ({ setCachedStateField }: React.ComponentProps) => ( + + ), + ) + const view = renderWithExtensionState(, { + queryClient, + }) + + fireEvent.click(await screen.findByTestId("clear-deny-list")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // A coincidental post re-bakes the deny list and adds an entry the dialog + // never displayed; the held merge keeps the buffered deletion. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf", "docker"], + }) + view.rerender() + + fireEvent.click(screen.getByTestId("save-button")) + + // The baseline retains the deleted (displayed) entry so the save applies the + // deletion, and excludes the never-displayed "docker" so the save keeps that + // entry and warns about it instead. + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + commandListBaseline: { allowedCommands: [], deniedCommands: ["rm -rf"] }, + updatedSettings: expect.objectContaining({ deniedCommands: [] }), + }), + ) + }) + + it("keeps a buffered list edit and its hold on external updates when the save is reported failed", async () => { + // A failed save must leave the dialog exactly as the user left it: still dirty, and with the + // in-flight list still holding back external refreshes. Clearing the hold at submit - before + // the host's answer - would let the post-failure state refresh overwrite the edit on screen. + vi.mocked(AutoApproveSettings).mockImplementation( + ({ setCachedStateField, deniedCommands }: React.ComponentProps) => ( +
+ {JSON.stringify(deniedCommands)} + +
+ ), + ) + const view = renderWithExtensionState(, { + queryClient, + }) + + fireEvent.click(await screen.findByTestId("buffer-deny-edit")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + // A display-setting difference between the buffer and the live state puts this save on the + // path that waits for the host's answer, without touching the buffered command lists. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + showRelativeTimes: false, + }) + view.rerender() + + fireEvent.click(screen.getByTestId("save-button")) + + const saveId = lastSaveRequestId(postMessage) + expect(saveId).toBeDefined() + expect(screen.getByTestId("save-button")).toBeDisabled() // waiting for the host's answer + answerSave(saveId, { success: false, error: "storage unavailable" }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) // the failure keeps it dirty + + // The host refreshes state after the rejected write; the edit the user is looking at wins. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf"], + }) + view.rerender() + expect(screen.getByTestId("current-deny-list")).toHaveTextContent(JSON.stringify(["git status"])) + + // Retrying submits the buffered edit, not the refresh that tried to replace it. + fireEvent.click(screen.getByTestId("save-button")) + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ deniedCommands: ["git status"] }), + }), + ) + }) + + it("releases the buffered list to external updates once the host confirms the save", async () => { + vi.mocked(AutoApproveSettings).mockImplementation( + ({ setCachedStateField, deniedCommands }: React.ComponentProps) => ( +
+ {JSON.stringify(deniedCommands)} + +
+ ), + ) + const view = renderWithExtensionState(, { + queryClient, + }) + + fireEvent.click(await screen.findByTestId("buffer-deny-edit")) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + showRelativeTimes: false, + }) + view.rerender() + + fireEvent.click(screen.getByTestId("save-button")) + expect(lastSaveRequestId(postMessage)).toBeDefined() // this save waits for the host's answer + answerSave(lastSaveRequestId(postMessage), { success: true }) + await waitFor(() => expect(screen.getByTestId("save-button")).toBeDisabled()) + + // The lists committed, so a later host update is no longer an in-flight edit being stomped. + ;(useExtensionState as ReturnType).mockReturnValue({ + ...defaultExtensionState, + deniedCommands: ["rm -rf"], + }) + view.rerender() + await waitFor(() => + expect(screen.getByTestId("current-deny-list")).toHaveTextContent(JSON.stringify(["rm -rf"])), + ) + }) + it("discards IO Intelligence cached edits and restores the extension values", async () => { const onDone = vi.fn() ;(useExtensionState as ReturnType).mockReturnValue({