Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -3516,7 +3516,7 @@ export class CopilotAgentSession extends Disposable {
}

const isNewFile = edits?.items.some(edit => !edit.before && !!edit.after);
const { confirmationTitle, invocationMessage, toolInput, permissionKind, permissionPath } = getPermissionDisplay(request, this._workingDirectory, isNewFile);
const { confirmationTitle, invocationMessage, toolInput, permissionKind, permissionPath } = getPermissionDisplay(request, this._workingDirectory, isNewFile, this._appliedAdditionalDirectories);

// Fire a pending_confirmation signal to transition the tool to PendingConfirmation
const toolName = request.kind === 'mcp' || request.kind === 'custom-tool' || request.kind === 'hook'
Expand Down
48 changes: 48 additions & 0 deletions src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,14 @@ export class CopilotSessionLauncher implements ICopilotSessionLauncher {

private async _finalizeSession(raw: CopilotSessionWrapper['session'], sandboxConfig: SandboxConfig | undefined, sessionId: string, modelId: string | undefined): Promise<CopilotSessionWrapper> {
await this._applySandboxConfig(raw, sandboxConfig, sessionId);
try {
await this._applyScriptSafety(raw, sessionId);
} catch (err) {
// Nothing owns `raw` until it is wrapped below, so a fail-closed launch has
// to disconnect it here or the runtime keeps an orphaned session alive.
await raw.disconnect().catch(() => { /* best-effort teardown */ });
throw err;
}
// TODO: Remove these post-launch updates once the SDK exposes verbosity and
// reasoningSummary in SessionConfig, alongside launch options such as reasoningEffort.
if (isGpt56Model(modelId)) {
Expand All @@ -637,6 +645,46 @@ export class CopilotSessionLauncher implements ICopilotSessionLauncher {
return new CopilotSessionWrapper(raw);
}

/**
* Enables the runtime's shell-script safety classifier, which managed permissions
* depend on to govern shell operations.
*
* Without it the runtime short-circuits the classifier, so a shell command reaches
* the permission layer with an empty `possiblePaths` and `hasWriteFileRedirection:
* false`. Managed `Read(...)`/`Edit(...)` rules then cannot match a redirect target,
* letting `echo ... >> denied/path` bypass a managed deny. The Copilot CLI opts in at
* session creation; the SDK exposes it to hosts only through `options.update`, so it
* is applied here to cover both created and resumed sessions.
*
* This fails the launch closed unconditionally. The host cannot tell whether a
* session is policy-bearing: `IAgentHostManagedSettingsService` only carries the
* legacy VS Code settings bridge, which is itself behind a false-by-default
* compatibility setting, while server and MDM policy is discovered by the runtime
* itself under `enableManagedSettings`. Gating a security control on that signal
* would leave exactly the enterprise sessions it protects unprotected, so the
* option is treated as required for every session.
*
* The client-level `managedSettings.read` is not a usable substitute: it discovers
* only device sources (MDM and managed-file), so a session governed solely by
* GitHub org policy would still read as unmanaged. Approximating the boundary is
* worse than not drawing one.
*/
private async _applyScriptSafety(session: CopilotSessionWrapper['session'], sessionId: string): Promise<void> {
try {
const result = await session.rpc.options.update({ enableScriptSafety: true });
if (!result.success) {
throw new Error('SDK rejected enabling script safety');
}
} catch (err) {
// The runtime reports success for any patch it accepts and signals real
// problems by failing the request, so this is the path a genuine failure
// takes. Log the reason before it propagates: the launch is aborted below
// and the raw RPC error alone would not say which option was refused.
this._logService.error(`[Copilot:${sessionId}] Could not enable script safety; managed permissions cannot govern shell paths`, err);
throw err;
}
}

/** Applies the post-launch session options used by GPT-5.6 models. */
private async _applyGpt56Customizations(session: CopilotSessionWrapper['session'], sessionId: string): Promise<void> {
await this._applyVerbosity(session, 'medium', sessionId);
Expand Down
50 changes: 47 additions & 3 deletions src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import { hasKey, isObject } from '../../../../base/common/types.js';
import { URI } from '../../../../base/common/uri.js';
import { appendEscapedMarkdownInlineCode, escapeMarkdownLinkLabel, MarkdownString } from '../../../../base/common/htmlContent.js';
import { hash } from '../../../../base/common/hash.js';
import { isAbsolute } from '../../../../base/common/path.js';
import { localize } from '../../../../nls.js';
import type { IAgentToolPendingConfirmationSignal } from '../../common/agent.js';
import type { ToolKind } from '../../common/meta/agentToolCallMeta.js';
import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js';
import { parsePartialToolInput } from '../../common/partialToolInput.js';
import { StringOrMarkdown } from '../../common/state/protocol/state.js';
import { basename } from '../../../../base/common/resources.js';
import { basename, extUriBiasedIgnorePathCase } from '../../../../base/common/resources.js';
import { getStreamingCreateMessage, getStreamingInsertMessage, getStreamingPatchMessage, getStreamingReplaceMessage, streamingToolTextLineCount, type ToolPathResolver } from '../../common/streamingToolCallDisplay.js';
import { getServerToolDisplay } from '../shared/serverToolGroups.js';

Expand Down Expand Up @@ -1092,10 +1093,53 @@ function str(value: unknown): string | undefined {
return typeof value === 'string' ? value : undefined;
}

/**
* True when a request came from the runtime's unauthorized-path gate.
*
* That gate runs ahead of the per-kind gates and fires only for paths outside
* the allowed directories, so it *is* the out-of-workspace case by
* construction. It is not a distinct request kind: it reuses the access kind
* (`read`/`write`/`shell`) and carries `paths` instead of the per-kind `path`,
* which the SDK's `PermissionRequestRead` type does not model — hence the
* structural check.
*/
function isUnauthorizedPathGateRequest(request: PermissionRequest): boolean {
return isObject(request) && Array.isArray((request as { paths?: unknown }).paths);
}

/**
* Chooses the confirmation title for a read request based on why approval is
* actually needed.
*
* A read is gated for several reasons — the path lies outside the allowed
* directories, a managed or scoped rule matched it, or the model asked to
* escape the sandbox. Only the first is about location, so the title claims it
* either when the unauthorized-path gate raised the request or when the path is
* absolute and contained by none of the session's workspace roots. A relative
* path, an unknown path, or an unknown workspace falls back to the neutral
* title rather than asserting a location the request does not establish.
*/
function readConfirmationTitle(request: PermissionRequest, path: string | undefined, workspaceRoots: readonly URI[], requestSandboxBypass: boolean | undefined): string {
if (requestSandboxBypass) {
return localize('copilot.permission.read.bypass.title', "Read file outside the sandbox?");
}
const outsideWorkspace = isUnauthorizedPathGateRequest(request)
|| (path !== undefined
&& isAbsolute(path)
&& workspaceRoots.length > 0
&& !workspaceRoots.some(root => extUriBiasedIgnorePathCase.isEqualOrParent(URI.file(path), root)));
return outsideWorkspace
? localize('copilot.permission.read.title', "Allow reading file outside of workspace?")
: localize('copilot.permission.read.generic.title', "Allow reading file?");
}

/**
* Derives display fields from a permission request for the tool confirmation UI.
*
* `additionalDirectories` carries the peer roots of a multi-root session, so a
* read under any root is recognized as inside the workspace.
*/
export function getPermissionDisplay(request: PermissionRequest, workingDirectory?: URI, isNewFile?: boolean): {
export function getPermissionDisplay(request: PermissionRequest, workingDirectory?: URI, isNewFile?: boolean, additionalDirectories?: readonly URI[]): {
confirmationTitle: string;
invocationMessage: StringOrMarkdown;
toolInput?: string;
Expand Down Expand Up @@ -1186,7 +1230,7 @@ export function getPermissionDisplay(request: PermissionRequest, workingDirector
}
case 'read':
return {
confirmationTitle: localize('copilot.permission.read.title', "Allow reading file outside of workspace?"),
confirmationTitle: readConfirmationTitle(request, path, workingDirectory ? [workingDirectory, ...(additionalDirectories ?? [])] : [], requestSandboxBypass),
invocationMessage: getInvocationMessage(CopilotToolName.View, getToolDisplayName(CopilotToolName.View), path ? { path } : undefined),
permissionKind: 'read',
permissionPath: path,
Expand Down
103 changes: 98 additions & 5 deletions src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ suite('CopilotSessionLauncher shared session config', () => {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { },
rpc: { options: { update: async () => ({ success: true }) } },
} as unknown as CopilotSession;
const client = {
createSession: async (config: Parameters<CopilotClient['createSession']>[0]) => {
Expand Down Expand Up @@ -651,6 +652,7 @@ suite('CopilotSessionLauncher resume fallback', () => {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { },
rpc: { options: { update: async () => ({ success: true }) } },
} as unknown as CopilotSession;
const client = {
createSession: async () => {
Expand Down Expand Up @@ -786,7 +788,7 @@ suite('CopilotSessionLauncher verbosity', () => {
const session = {
rpc: {
options: {
update: async (options: unknown) => updates.push(options),
update: async (options: unknown) => { updates.push(options); return { success: true }; },
},
},
} as unknown as CopilotSession;
Expand Down Expand Up @@ -815,7 +817,7 @@ suite('CopilotSessionLauncher reasoning summary', () => {
const session = {
rpc: {
options: {
update: async (options: unknown) => updates.push(options),
update: async (options: unknown) => { updates.push(options); return { success: true }; },
},
},
} as unknown as CopilotSession;
Expand Down Expand Up @@ -845,7 +847,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => {
const session = {
rpc: {
options: {
update: async (options: unknown) => updates.push(options),
update: async (options: unknown) => { updates.push(options); return { success: true }; },
},
},
} as unknown as CopilotSession;
Expand All @@ -865,7 +867,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => {
_applyGpt56Customizations(session: CopilotSession, sessionId: string): Promise<void>;
};
const session = {
rpc: { options: { update: async (options: unknown) => updates.push(options) } },
rpc: { options: { update: async (options: unknown) => { updates.push(options); return { success: true }; } } },
} as unknown as CopilotSession;

await launcher._applyGpt56Customizations(session, 'session-1');
Expand All @@ -874,13 +876,103 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => {
}
});

test('enables script safety on a non-GPT-5.6 created session so managed permissions govern shell paths', async () => {
const updates: unknown[] = [];
const session = {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { },
rpc: { options: { update: async (options: unknown) => { updates.push(options); return { success: true }; } } },
} as unknown as CopilotSession;
// Enablement is required for every session, so anything short of success would
// fail the launch — the success path is what is asserted here.
const launcher = createTestLauncher({ deny: ['Shell(rm -rf *)'] });
const plan: CopilotSessionLaunchPlan = {
kind: 'create',
client: { createSession: async () => session } as unknown as CopilotClient,
sessionId: 'session-1',
workingDirectory: testWorkingDirectory,
resolvedAgentName: undefined,
snapshot: { tools: [], plugins: [], mcpServers: {} },
activeClientToolSet: new ActiveClientToolSet(),
shellManager: undefined,
githubToken: undefined,
model: { id: 'claude-sonnet-4.5', config: {} },
};

const wrapper = await launcher.launch(plan, testRuntime);
try {
assert.deepStrictEqual(updates, [{ enableScriptSafety: true }]);
} finally {
wrapper.dispose();
await launcher.disposeByokProxyHandle();
}
});

test('fails the launch closed and disconnects when script safety cannot be enabled', async () => {
let disconnected = false;
const session = {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { disconnected = true; },
rpc: { options: { update: async () => ({ success: false }) } },
} as unknown as CopilotSession;
const launcher = createTestLauncher({ deny: ['Shell(rm -rf *)'] });
const plan: CopilotSessionLaunchPlan = {
kind: 'create',
client: { createSession: async () => session } as unknown as CopilotClient,
sessionId: 'session-1',
workingDirectory: testWorkingDirectory,
resolvedAgentName: undefined,
snapshot: { tools: [], plugins: [], mcpServers: {} },
activeClientToolSet: new ActiveClientToolSet(),
shellManager: undefined,
githubToken: undefined,
model: { id: 'claude-sonnet-4.5', config: {} },
};

await assert.rejects(() => launcher.launch(plan, testRuntime), /script safety/);
assert.strictEqual(disconnected, true, 'expected the orphaned session to be disconnected');
await launcher.disposeByokProxyHandle();
});

test('fails closed even when the host sees no managed rules, since server and MDM policy is invisible to it', async () => {
let disconnected = false;
const session = {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { disconnected = true; },
rpc: { options: { update: async () => ({ success: false }) } },
} as unknown as CopilotSession;
// No client-bridged rules. `IAgentHostManagedSettingsService` only carries the
// legacy VS Code settings bridge, so an enterprise session governed solely by
// GitHub or MDM policy looks exactly like this one from the host's side.
const launcher = createTestLauncher();
const plan: CopilotSessionLaunchPlan = {
kind: 'create',
client: { createSession: async () => session } as unknown as CopilotClient,
sessionId: 'session-1',
workingDirectory: testWorkingDirectory,
resolvedAgentName: undefined,
snapshot: { tools: [], plugins: [], mcpServers: {} },
activeClientToolSet: new ActiveClientToolSet(),
shellManager: undefined,
githubToken: undefined,
model: { id: 'claude-sonnet-4.5', config: {} },
};

await assert.rejects(() => launcher.launch(plan, testRuntime), /script safety/);
assert.strictEqual(disconnected, true, 'expected the orphaned session to be disconnected');
await launcher.disposeByokProxyHandle();
});

test('applies GPT-5.6 customizations when resuming an existing session', async () => {
const updates: unknown[] = [];
const session = {
sessionId: 'session-1',
on: () => () => { },
disconnect: async () => { },
rpc: { options: { update: async (options: unknown) => updates.push(options) } },
rpc: { options: { update: async (options: unknown) => { updates.push(options); return { success: true }; } } },
} as unknown as CopilotSession;
const launcher = createTestLauncher(undefined, { [CopilotCliConfigKey.ReasoningSummary]: true });
const plan: CopilotSessionLaunchPlan = {
Expand All @@ -899,6 +991,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => {
const wrapper = await launcher.launch(plan, testRuntime);
try {
assert.deepStrictEqual(updates, [
{ enableScriptSafety: true },
{ verbosity: 'medium' },
{ reasoningSummary: 'concise' },
]);
Expand Down
43 changes: 43 additions & 0 deletions src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,49 @@ suite('copilotToolDisplay — markdown-rendered tools', () => {
});
});

suite('getPermissionDisplay — read confirmation title', () => {

ensureNoDisposablesAreLeakedInTestSuite();

const wd = URI.file('/repo/project');

function readRequest(path: string, requestSandboxBypass?: boolean): PermissionRequest {
return { kind: 'read', intention: `Read file: ${path}`, path, ...(requestSandboxBypass ? { requestSandboxBypass } : {}) } as PermissionRequest;
}

/**
* The runtime's unauthorized-path gate reuses the access kind and carries
* `paths` rather than the per-kind `path`.
*/
function unauthorizedPathGateRequest(...paths: string[]): PermissionRequest {
return { kind: 'read', intention: 'Read files', paths } as unknown as PermissionRequest;
}

test('claims "outside of workspace" only when the path really is outside', () => {
assert.deepStrictEqual({
inside: getPermissionDisplay(readRequest('/repo/project/src/app.ts'), wd).confirmationTitle,
insideDirectory: getPermissionDisplay(readRequest('/repo/project/src'), wd).confirmationTitle,
outside: getPermissionDisplay(readRequest('/etc/hosts'), wd).confirmationTitle,
secondRoot: getPermissionDisplay(readRequest('/repo/other/lib.ts'), wd, undefined, [URI.file('/repo/other')]).confirmationTitle,
outsideEveryRoot: getPermissionDisplay(readRequest('/etc/hosts'), wd, undefined, [URI.file('/repo/other')]).confirmationTitle,
pathGate: getPermissionDisplay(unauthorizedPathGateRequest('/etc/hosts'), wd).confirmationTitle,
relative: getPermissionDisplay(readRequest('README.md'), wd).confirmationTitle,
unknownWorkspace: getPermissionDisplay(readRequest('/repo/project/src/app.ts'), undefined).confirmationTitle,
sandboxBypass: getPermissionDisplay(readRequest('/repo/project/src/app.ts', true), wd).confirmationTitle,
}, {
inside: 'Allow reading file?',
insideDirectory: 'Allow reading file?',
outside: 'Allow reading file outside of workspace?',
secondRoot: 'Allow reading file?',
outsideEveryRoot: 'Allow reading file outside of workspace?',
pathGate: 'Allow reading file outside of workspace?',
relative: 'Allow reading file?',
unknownWorkspace: 'Allow reading file?',
sandboxBypass: 'Read file outside the sandbox?',
});
});
});

suite('getPermissionDisplay — cd-prefix stripping', () => {

ensureNoDisposablesAreLeakedInTestSuite();
Expand Down
Loading
Loading