Skip to content

Commit 1f63695

Browse files
DonJayamanneDonJayamanne
authored
copilotcli: Enhance permission handling in Copilot CLI (#310359)
* Enhance permission handling in Copilot CLI * Updates * updates * Fixes * Fix issues * add tests
1 parent 55c1881 commit 1f63695

3 files changed

Lines changed: 830 additions & 465 deletions

File tree

‎extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSession.ts‎

Lines changed: 73 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { CancellationToken } from '../../../../util/vs/base/common/cancellation'
2020
import { Codicon } from '../../../../util/vs/base/common/codicons';
2121
import { Emitter } from '../../../../util/vs/base/common/event';
2222
import { DisposableStore, IDisposable, toDisposable } from '../../../../util/vs/base/common/lifecycle';
23-
import { extUriBiasedIgnorePathCase, isEqual } from '../../../../util/vs/base/common/resources';
2423
import { truncate } from '../../../../util/vs/base/common/strings';
2524
import { ThemeIcon } from '../../../../util/vs/base/common/themables';
2625
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
@@ -29,12 +28,11 @@ import { IToolsService } from '../../../tools/common/toolsService';
2928
import { IChatSessionMetadataStore } from '../../common/chatSessionMetadataStore';
3029
import { ExternalEditTracker } from '../../common/externalEditTracker';
3130
import { getWorkingDirectory, isIsolationEnabled, IWorkspaceInfo } from '../../common/workspaceInfo';
32-
import { enrichToolInvocationWithSubagentMetadata, getAffectedUrisForEditTool, isCopilotCliEditToolCall, isCopilotCLIToolThatCouldRequirePermissions, processToolExecutionComplete, processToolExecutionStart, ToolCall, updateTodoList } from '../common/copilotCLITools';
33-
import { getCopilotCLISessionStateDir } from './cliHelpers';
31+
import { enrichToolInvocationWithSubagentMetadata, isCopilotCliEditToolCall, isCopilotCLIToolThatCouldRequirePermissions, processToolExecutionComplete, processToolExecutionStart, ToolCall, updateTodoList } from '../common/copilotCLITools';
3432
import type { CopilotCliBridgeSpanProcessor } from './copilotCliBridgeSpanProcessor';
3533
import { ICopilotCLIImageSupport } from './copilotCLIImageSupport';
3634
import { handleExitPlanMode } from './exitPlanModeHandler';
37-
import { PermissionRequest, requestPermission, requiresFileEditconfirmation } from './permissionHelpers';
35+
import { handleMcpPermission, handleReadPermission, handleShellPermission, handleWritePermission, type PermissionRequest, type PermissionRequestResult, showInteractivePermissionPrompt } from './permissionHelpers';
3836
import { IQuestion, IUserQuestionHandler } from './userInputHelpers';
3937

4038
/**
@@ -429,32 +427,80 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
429427
disposables.add(toDisposable(this._sdkSession.on('permission.requested', async (event) => {
430428
const permissionRequest = event.data.permissionRequest;
431429
const requestId = event.data.requestId;
432-
const response = await this.requestPermission(permissionRequest, editTracker,
433-
(toolCallId: string) => {
434-
const toolData = toolCalls.get(toolCallId);
435-
if (!toolData) {
436-
return undefined;
437-
}
438-
const data = pendingToolInvocations.get(toolCallId);
439-
if (data) {
440-
return [toolData, data[2]] as const;
430+
431+
// Auto-approve all requests when the permission level allows it.
432+
if (this._permissionLevel === 'autoApprove' || this._permissionLevel === 'autopilot') {
433+
this.logService.trace(`[CopilotCLISession] Auto Approving ${permissionRequest.kind} request (permission level: ${this._permissionLevel})`);
434+
this._sdkSession.respondToPermission(requestId, { kind: 'approved' });
435+
return;
436+
}
437+
438+
// Resolve tool call data for the permission request.
439+
const toolData = permissionRequest.toolCallId ? toolCalls.get(permissionRequest.toolCallId) : undefined;
440+
const pendingData = permissionRequest.toolCallId ? pendingToolInvocations.get(permissionRequest.toolCallId) : undefined;
441+
const toolParentCallId = pendingData ? pendingData[2] : undefined;
442+
const toolInvocationToken = this._toolInvocationToken as unknown as never;
443+
444+
try {
445+
let response: PermissionRequestResult;
446+
if (this._permissionLevel === 'autoApprove' || this._permissionLevel === 'autopilot') {
447+
this.logService.trace(`[CopilotCLISession] Auto Approving ${permissionRequest.kind} request (permission level: ${this._permissionLevel})`);
448+
response = { kind: 'approved' };
449+
} else {
450+
switch (permissionRequest.kind) {
451+
case 'read':
452+
response = await handleReadPermission(
453+
this.sessionId, permissionRequest, toolParentCallId,
454+
this.attachments, this._imageSupport, this.workspace, this.workspaceService,
455+
this._toolsService, toolInvocationToken, this.logService, token,
456+
);
457+
break;
458+
case 'write':
459+
response = await handleWritePermission(
460+
this.sessionId, permissionRequest, toolData, toolParentCallId,
461+
this._stream, editTracker, this.workspace, this.workspaceService,
462+
this.instantiationService, this._toolsService, toolInvocationToken, this.logService, token,
463+
);
464+
break;
465+
case 'shell':
466+
response = await handleShellPermission(
467+
permissionRequest, toolParentCallId,
468+
this.workspace, this._toolsService, toolInvocationToken, this.logService, token,
469+
);
470+
break;
471+
case 'mcp':
472+
response = await handleMcpPermission(
473+
permissionRequest, toolParentCallId,
474+
this._toolsService, toolInvocationToken, this.logService, token,
475+
);
476+
break;
477+
default:
478+
response = await showInteractivePermissionPrompt(
479+
permissionRequest, toolParentCallId,
480+
this._toolsService, toolInvocationToken, this.logService, token,
481+
);
482+
break;
441483
}
442-
return [toolData, undefined] as const;
443-
},
444-
token
445-
);
446-
flushPendingInvocationMessageForToolCallId(permissionRequest.toolCallId);
484+
}
447485

448-
this._requestLogger.addEntry({
449-
type: LoggedRequestKind.MarkdownContentRequest,
450-
debugName: `Permission Request`,
451-
startTimeMs: Date.now(),
452-
icon: Codicon.question,
453-
markdownContent: this._renderPermissionToMarkdown(permissionRequest, response.kind),
454-
isConversationRequest: true
455-
});
486+
flushPendingInvocationMessageForToolCallId(permissionRequest.toolCallId);
456487

457-
this._sdkSession.respondToPermission(requestId, response);
488+
this._requestLogger.addEntry({
489+
type: LoggedRequestKind.MarkdownContentRequest,
490+
debugName: `Permission Request`,
491+
startTimeMs: Date.now(),
492+
icon: Codicon.question,
493+
markdownContent: this._renderPermissionToMarkdown(permissionRequest, response.kind),
494+
isConversationRequest: true
495+
});
496+
497+
this._sdkSession.respondToPermission(requestId, response);
498+
}
499+
catch (error) {
500+
this.logService.error(error, `[CopilotCLISession] Error handling permission request of kind ${permissionRequest.kind}`);
501+
flushPendingInvocationMessageForToolCallId(permissionRequest.toolCallId);
502+
this._sdkSession.respondToPermission(requestId, { kind: 'denied-interactively-by-user' });
503+
}
458504
})));
459505
if (shouldHandleExitPlanModeRequests) {
460506
disposables.add(toDisposable(this._sdkSession.on('exit_plan_mode.requested', async (event) => {
@@ -868,132 +914,6 @@ export class CopilotCLISession extends DisposableStore implements ICopilotCLISes
868914
return this._sdkSession.getSelectedModel();
869915
}
870916

871-
private isFileFromSessionWorkspace(file: Uri): boolean {
872-
const workingDirectory = getWorkingDirectory(this.workspace);
873-
if (workingDirectory && extUriBiasedIgnorePathCase.isEqualOrParent(file, workingDirectory)) {
874-
return true;
875-
}
876-
if (this.workspace.folder && extUriBiasedIgnorePathCase.isEqualOrParent(file, this.workspace.folder)) {
877-
return true;
878-
}
879-
// Only if we have a worktree should we check the repository.
880-
// As this means the user created a worktree and we have a repository.
881-
// & if the worktree is automatically trusted, then so is the repository as we created the worktree from that.
882-
if (this.workspace.worktree && this.workspace.repository && extUriBiasedIgnorePathCase.isEqualOrParent(file, this.workspace.repository)) {
883-
return true;
884-
}
885-
886-
return false;
887-
}
888-
private async requestPermission(
889-
permissionRequest: PermissionRequest,
890-
editTracker: ExternalEditTracker,
891-
getToolCall: (toolCallId: string) => undefined | [ToolCall, parentToolCallId: string | undefined],
892-
token: vscode.CancellationToken
893-
): Promise<{ kind: 'approved' } | { kind: 'denied-interactively-by-user' }> {
894-
if (this._permissionLevel === 'autoApprove' || this._permissionLevel === 'autopilot') {
895-
this.logService.trace(`[CopilotCLISession] Auto Approving ${permissionRequest.kind} request (permission level: ${this._permissionLevel})`);
896-
return { kind: 'approved' };
897-
}
898-
899-
const workingDirectory = getWorkingDirectory(this.workspace);
900-
901-
if (permissionRequest.kind === 'read') {
902-
// If user is reading a file in the working directory or workspace, auto-approve
903-
// read requests. Outside workspace reads (e.g., /etc/passwd) will still require
904-
// approval.
905-
const data = Uri.file(permissionRequest.path);
906-
907-
if (this._imageSupport.isTrustedImage(data)) {
908-
return { kind: 'approved' };
909-
}
910-
911-
if (this.isFileFromSessionWorkspace(data)) {
912-
this.logService.trace(`[CopilotCLISession] Auto Approving request to read file in session workspace ${permissionRequest.path}`);
913-
return { kind: 'approved' };
914-
}
915-
916-
if (this.workspaceService.getWorkspaceFolder(data)) {
917-
this.logService.trace(`[CopilotCLISession] Auto Approving request to read workspace file ${permissionRequest.path}`);
918-
return { kind: 'approved' };
919-
}
920-
921-
// If reading a file from session directory, e.g. plan.md, then auto approve it, this is internal file to Cli.
922-
const sessionDir = Uri.joinPath(Uri.file(getCopilotCLISessionStateDir()), this.sessionId);
923-
if (extUriBiasedIgnorePathCase.isEqualOrParent(data, sessionDir)) {
924-
this.logService.trace(`[CopilotCLISession] Auto Approving request to read Copilot CLI session resource ${permissionRequest.path}`);
925-
return { kind: 'approved' };
926-
}
927-
928-
// If model is trying to read the contents of a file thats attached, then auto-approve it, as this is an explicit action by the user to share the file with the model.
929-
if (this.attachments.some(attachment => attachment.type === 'file' && isEqual(Uri.file(attachment.path), data))) {
930-
this.logService.trace(`[CopilotCLISession] Auto Approving request to read attached file ${permissionRequest.path}`);
931-
return { kind: 'approved' };
932-
}
933-
}
934-
935-
// Get hold of file thats being edited if this is a edit tool call (requiring write permissions).
936-
const toolData = permissionRequest.toolCallId ? getToolCall(permissionRequest.toolCallId) : undefined;
937-
const toolCall = toolData ? toolData[0] : undefined;
938-
const toolParentCallId = toolData ? toolData[1] : undefined;
939-
const editFiles = toolCall ? getAffectedUrisForEditTool(toolCall) : undefined;
940-
// Sometimes we don't get a tool call id for the edit permission request
941-
const editFile = permissionRequest.kind === 'write' ? (editFiles && editFiles.length ? editFiles[0] : (permissionRequest.fileName ? Uri.file(permissionRequest.fileName) : undefined)) : undefined;
942-
if (workingDirectory && permissionRequest.kind === 'write' && editFile) {
943-
const isWorkspaceFile = this.workspaceService.getWorkspaceFolder(editFile);
944-
const isWorkingDirectoryFile = !this.workspaceService.getWorkspaceFolder(workingDirectory) && extUriBiasedIgnorePathCase.isEqualOrParent(editFile, workingDirectory);
945-
946-
let autoApprove = false;
947-
// If isolation is enabled, we only auto-approve writes within the working directory.
948-
if (isIsolationEnabled(this.workspace) && isWorkingDirectoryFile) {
949-
autoApprove = true;
950-
}
951-
// If its a workspace file, and not editing protected files, we auto-approve.
952-
if (!autoApprove && isWorkspaceFile && !(await requiresFileEditconfirmation(this.instantiationService, permissionRequest, toolCall))) {
953-
autoApprove = true;
954-
}
955-
// If we're working in the working directory (non-isolation), and not editing protected files, we auto-approve.
956-
if (!autoApprove && isWorkingDirectoryFile && !(await requiresFileEditconfirmation(this.instantiationService, permissionRequest, toolCall, workingDirectory))) {
957-
autoApprove = true;
958-
}
959-
960-
if (autoApprove) {
961-
this.logService.trace(`[CopilotCLISession] Auto Approving request ${editFile.fsPath}`);
962-
963-
// If we're editing a file, start tracking the edit & wait for core to acknowledge it.
964-
if (toolCall && this._stream) {
965-
this.logService.trace(`[CopilotCLISession] Starting to track edit for toolCallId ${toolCall.toolCallId} & file ${editFile.fsPath}`);
966-
await editTracker.trackEdit(toolCall.toolCallId, [editFile], this._stream);
967-
}
968-
969-
return { kind: 'approved' };
970-
}
971-
}
972-
// If reading a file from session directory, e.g. plan.md, then auto approve it, this is internal file to Cli.
973-
const sessionDir = Uri.joinPath(Uri.file(getCopilotCLISessionStateDir()), this.sessionId);
974-
if (permissionRequest.kind === 'write' && editFile && extUriBiasedIgnorePathCase.isEqualOrParent(editFile, sessionDir)) {
975-
this.logService.trace(`[CopilotCLISession] Auto Approving request to write to Copilot CLI session resource ${editFile.fsPath}`);
976-
return { kind: 'approved' };
977-
}
978-
979-
try {
980-
if (await requestPermission(this.instantiationService, permissionRequest, toolCall, getWorkingDirectory(this.workspace), this._toolsService, this._toolInvocationToken as unknown as never, toolParentCallId, token)) {
981-
// If we're editing a file, start tracking the edit & wait for core to acknowledge it.
982-
if (editFile && toolCall && this._stream) {
983-
this.logService.trace(`[CopilotCLISession] Starting to track edit for toolCallId ${toolCall.toolCallId} & file ${editFile.fsPath}`);
984-
await editTracker.trackEdit(toolCall.toolCallId, [editFile], this._stream);
985-
}
986-
return { kind: 'approved' };
987-
}
988-
} catch (error) {
989-
this.logService.error(`[CopilotCLISession] Permission request error: ${error}`);
990-
} finally {
991-
this._permissionRequested = undefined;
992-
}
993-
994-
return { kind: 'denied-interactively-by-user' };
995-
}
996-
997917
private _logRequest(userPrompt: string, modelId: string, attachments: Attachment[], startTimeMs: number): void {
998918
const markdownContent = this._renderRequestToMarkdown(userPrompt, modelId, attachments, startTimeMs);
999919
this._requestLogger.addEntry({

0 commit comments

Comments
 (0)