Skip to content
Merged
23 changes: 23 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,7 @@ 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);
await this._applyScriptSafety(raw, sessionId);
// 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 +638,28 @@ 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.
*/
private async _applyScriptSafety(session: CopilotSessionWrapper['session'], sessionId: string): Promise<void> {
try {
const result = await session.rpc.options.update({ enableScriptSafety: true });
if (!result.success) {
this._logService.error(`[Copilot:${sessionId}] SDK rejected enabling script safety; managed permissions cannot govern shell paths`);
}
} catch (err) {
this._logService.error(err, `[Copilot:${sessionId}] Failed to enable script safety; managed permissions cannot govern shell paths`);
Comment thread
Copilot marked this conversation as resolved.
Outdated
}
}

/** 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
29 changes: 27 additions & 2 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,6 +1093,30 @@ function str(value: unknown): string | undefined {
return typeof value === 'string' ? value : undefined;
}

/**
* 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 workspace, a
* managed or scoped rule matched it, or the model asked to escape the sandbox.
* Only the first is about location, so the title is claimed only when the path
* is absolute and genuinely outside `workingDirectory`. 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(path: string | undefined, workingDirectory: URI | undefined, requestSandboxBypass: boolean | undefined): string {
if (requestSandboxBypass) {
return localize('copilot.permission.read.bypass.title', "Read file outside the sandbox?");
}
const outsideWorkspace = path !== undefined
&& isAbsolute(path)
&& workingDirectory !== undefined
&& !extUriBiasedIgnorePathCase.isEqualOrParent(URI.file(path), workingDirectory);
Comment thread
Copilot marked this conversation as resolved.
Outdated
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.
*/
Expand Down Expand Up @@ -1186,7 +1211,7 @@ export function getPermissionDisplay(request: PermissionRequest, workingDirector
}
case 'read':
return {
confirmationTitle: localize('copilot.permission.read.title', "Allow reading file outside of workspace?"),
confirmationTitle: readConfirmationTitle(path, workingDirectory, requestSandboxBypass),
invocationMessage: getInvocationMessage(CopilotToolName.View, getToolDisplayName(CopilotToolName.View), path ? { path } : undefined),
permissionKind: 'read',
permissionPath: path,
Expand Down
32 changes: 32 additions & 0 deletions src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,37 @@ 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) } },
Comment thread
joshspicer marked this conversation as resolved.
Outdated
} as unknown as CopilotSession;
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: {} },
};

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

test('applies GPT-5.6 customizations when resuming an existing session', async () => {
const updates: unknown[] = [];
const session = {
Expand All @@ -899,6 +930,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
29 changes: 29 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,35 @@ 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;
}

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,
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?',
relative: 'Allow reading file?',
unknownWorkspace: 'Allow reading file?',
sandboxBypass: 'Read file outside the sandbox?',
});
});
});

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

ensureNoDisposablesAreLeakedInTestSuite();
Expand Down
Loading