diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts index 4ba552b98f..1b14c2f406 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts @@ -4,8 +4,11 @@ */ import * as crypto from 'crypto' +import * as fs from 'fs' +import * as os from 'os' import * as path from 'path' import * as chokidar from 'chokidar' +import { URI } from 'vscode-uri' import { ChatResponseStream, CodeWhispererStreaming, @@ -35,7 +38,7 @@ import { ChatUpdateParams, ConnectionMetadata, } from '@aws/language-server-runtimes/server-interface' -import { Model } from '@aws/language-server-runtimes/protocol' +import { MessageType, Model } from '@aws/language-server-runtimes/protocol' import { TestFeatures } from '@aws/language-server-runtimes/testing' import * as assert from 'assert' import { createIterableResponse, setCredentialsForAmazonQTokenServiceManagerFactory } from '../../shared/testUtils' @@ -3636,6 +3639,182 @@ ${' '.repeat(8)}} assert.strictEqual(toolInput.ruleArtifacts[1].path, '/test/rule2.json') }) }) + + describe('onFileClicked workspace containment', () => { + // Chat file-list cards carry paths that originate in model tool results, + // so a click must not open arbitrary files. A real temp directory stands + // in for the workspace because the containment check canonicalizes + // folders through the filesystem. + let workspaceDir: string + let outsideDir: string + let insideFile: string + let outsideFile: string + let showDocumentStub: sinon.SinonStub + let showMessageRequestStub: sinon.SinonStub + + const OPEN = { title: 'Open' } + const CANCEL = { title: 'Cancel' } + + beforeEach(async () => { + workspaceDir = await fs.promises.realpath(await fs.promises.mkdtemp(path.join(os.tmpdir(), 'ws-'))) + outsideDir = await fs.promises.realpath(await fs.promises.mkdtemp(path.join(os.tmpdir(), 'outside-'))) + insideFile = path.join(workspaceDir, 'inside.txt') + outsideFile = path.join(outsideDir, 'secret.txt') + await fs.promises.writeFile(insideFile, 'inside') + await fs.promises.writeFile(outsideFile, 'outside') + ;(testFeatures.workspace.getAllWorkspaceFolders as sinon.SinonStub).returns([ + { uri: URI.file(workspaceDir).toString(), name: 'ws' }, + ]) + showDocumentStub = testFeatures.lsp.window.showDocument as sinon.SinonStub + showDocumentStub.resetHistory() + // An out-of-workspace path prompts the user. Default to declining so + // each "refuses" case below means "refused when the user declines". + showMessageRequestStub = sinon.stub().resolves(CANCEL) + testFeatures.lsp.window.showMessageRequest = showMessageRequestStub + chatController.onTabAdd({ tabId: mockTabId }) + }) + + afterEach(async () => { + await fs.promises.rm(workspaceDir, { recursive: true, force: true }) + await fs.promises.rm(outsideDir, { recursive: true, force: true }) + }) + + function openedUris(): string[] { + return showDocumentStub.getCalls().map(c => c.args[0].uri) + } + + it('opens a file inside the workspace from a fullPath card', async () => { + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'inside.txt', fullPath: insideFile }) + assert.deepStrictEqual(openedUris(), [URI.file(insideFile).toString()]) + }) + + it('refuses a fullPath outside the workspace', async () => { + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'secret.txt', fullPath: outsideFile }) + sinon.assert.notCalled(showDocumentStub) + }) + + it('refuses a relative filePath that escapes the workspace', async () => { + const escaping = path.relative(workspaceDir, outsideFile) + assert.ok(escaping.startsWith('..'), 'test precondition: path must traverse upward') + await chatController.onFileClicked({ tabId: mockTabId, filePath: escaping }) + sinon.assert.notCalled(showDocumentStub) + }) + + it('refuses an fsRead card path outside the workspace when it was never approved', async () => { + const session = chatSessionManagementService.getSession(mockTabId).data! + session.toolUseLookup.set('read-1', { + toolUseId: 'read-1', + name: 'fsRead', + input: { paths: [outsideFile] }, + }) + await chatController.onFileClicked({ tabId: mockTabId, messageId: 'read-1', filePath: outsideFile }) + sinon.assert.notCalled(showDocumentStub) + }) + + it('opens an fsRead card path outside the workspace when the user approved it in this session', async () => { + const session = chatSessionManagementService.getSession(mockTabId).data! + session.toolUseLookup.set('read-2', { + toolUseId: 'read-2', + name: 'fsRead', + input: { paths: [outsideFile] }, + }) + session.addApprovedPath(outsideFile, 'fsRead') + await chatController.onFileClicked({ tabId: mockTabId, messageId: 'read-2', filePath: outsideFile }) + assert.deepStrictEqual(openedUris(), [URI.file(outsideFile).toString()]) + // Already approved for this tool, so no dialog is shown. + sinon.assert.notCalled(showMessageRequestStub) + }) + + it('does not treat an fsWrite approval as permission to open via an fsRead card', async () => { + const session = chatSessionManagementService.getSession(mockTabId).data! + session.toolUseLookup.set('read-3', { + toolUseId: 'read-3', + name: 'fsRead', + input: { paths: [outsideFile] }, + }) + session.addApprovedPath(outsideFile, 'fsWrite') + await chatController.onFileClicked({ tabId: mockTabId, messageId: 'read-3', filePath: outsideFile }) + sinon.assert.notCalled(showDocumentStub) + // Not approved for fsRead, so the user is asked rather than silently opened. + sinon.assert.calledOnce(showMessageRequestStub) + }) + + it('prompts with a warning for an out-of-workspace path and opens when the user chooses Open', async () => { + showMessageRequestStub.resolves(OPEN) + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'secret.txt', fullPath: outsideFile }) + + sinon.assert.calledOnce(showMessageRequestStub) + const request = showMessageRequestStub.firstCall.args[0] + assert.strictEqual(request.type, MessageType.Warning) + assert.ok(request.message.includes(outsideFile), 'dialog names the exact path being opened') + assert.deepStrictEqual( + request.actions.map((a: { title: string }) => a.title), + ['Open', 'Cancel'] + ) + assert.deepStrictEqual(openedUris(), [URI.file(outsideFile).toString()]) + }) + + it('does not open when the user dismisses the dialog without choosing', async () => { + showMessageRequestStub.resolves(null) + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'secret.txt', fullPath: outsideFile }) + sinon.assert.notCalled(showDocumentStub) + }) + + it('remembers an Open choice so the same path does not prompt again', async () => { + showMessageRequestStub.resolves(OPEN) + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'secret.txt', fullPath: outsideFile }) + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'secret.txt', fullPath: outsideFile }) + + sinon.assert.calledOnce(showMessageRequestStub) + assert.strictEqual(openedUris().length, 2) + }) + + it('surfaces the sensitive-path reason in the dialog for a sensitive out-of-workspace file', async () => { + const sensitiveDir = path.join(outsideDir, '.ssh') + await fs.promises.mkdir(sensitiveDir, { recursive: true }) + const sensitiveFile = path.join(sensitiveDir, 'authorized_keys') + await fs.promises.writeFile(sensitiveFile, 'k') + + await chatController.onFileClicked({ + tabId: mockTabId, + filePath: 'authorized_keys', + fullPath: sensitiveFile, + }) + + sinon.assert.calledOnce(showMessageRequestStub) + const request = showMessageRequestStub.firstCall.args[0] + assert.ok(/sensitive/i.test(request.message), `dialog explains why: ${request.message}`) + sinon.assert.notCalled(showDocumentStub) + }) + + it('opens a prompt file from the user prompts directory outside the workspace', async () => { + const promptsDir = path.join(outsideDir, '.aws', 'amazonq', 'prompts') + await fs.promises.mkdir(promptsDir, { recursive: true }) + const promptFile = path.join(promptsDir, 'my.prompt.md') + await fs.promises.writeFile(promptFile, '# prompt') + const homeStub = sinon.stub(os, 'homedir').returns(outsideDir) + try { + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'my.prompt.md', fullPath: promptFile }) + assert.deepStrictEqual(openedUris(), [URI.file(promptFile).toString()]) + } finally { + homeStub.restore() + } + }) + + it('refuses a non-prompt file placed in the prompts directory', async () => { + const promptsDir = path.join(outsideDir, '.aws', 'amazonq', 'prompts') + await fs.promises.mkdir(promptsDir, { recursive: true }) + const stray = path.join(promptsDir, 'credentials') + await fs.promises.writeFile(stray, 'x') + const homeStub = sinon.stub(os, 'homedir').returns(outsideDir) + try { + await chatController.onFileClicked({ tabId: mockTabId, filePath: 'credentials', fullPath: stray }) + sinon.assert.notCalled(showDocumentStub) + } finally { + homeStub.restore() + } + }) + }) }) // The body may include text-based progress updates from tool invocations. diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts index a4f42c7d33..171dc73624 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts @@ -54,6 +54,7 @@ import { ActiveEditorChangedParams, PinnedContextParams, ChatUpdateParams, + MessageActionItem, MessageType, ExecuteCommandParams, FollowUpClickParams, @@ -169,6 +170,8 @@ import { CommandValidation, ExplanatoryParams, InvokeOutput, + requiresPathAcceptance, + resolveCanonicalPath, resolveSymlinkAwarePath, ToolApprovalException, } from './tools/toolShared' @@ -4023,11 +4026,11 @@ export class AgenticChatController implements ChatHandlers { fileContent: toolUse.fileChange?.after, }) } else if (toolUse?.name === FS_READ) { - await this.#features.lsp.window.showDocument({ uri: URI.file(params.filePath).toString() }) + await this.#openIfAllowed(params.filePath, FS_READ, session.data) } else { const absolutePath = params.fullPath ?? (await this.#resolveAbsolutePath(params.filePath)) if (absolutePath) { - await this.#features.lsp.window.showDocument({ uri: URI.file(absolutePath).toString() }) + await this.#openIfAllowed(absolutePath, toolUse?.name ?? FS_READ, session.data) } } } catch (e: any) { @@ -4035,6 +4038,53 @@ export class AgenticChatController implements ChatHandlers { } } + /** + * Open a file from a chat file-list card only when the user could have + * read it through the tools: inside a workspace folder, or a path the user + * already approved for `toolName` in this session. Card paths come from + * model tool results and service findings, so an unchecked path plus one + * click would open any readable file and feed it to completion context. + * + * The user prompts directory is allowed as well: `#resolveAbsolutePath` + * deliberately resolves `.prompt.md` files there, and they are user-authored + * context files rather than arbitrary disk contents. + */ + async #openIfAllowed(filePath: string, toolName: string, session: ChatSessionService | undefined): Promise { + const canonicalPath = await resolveCanonicalPath(filePath) + const promptsDirectory = await resolveCanonicalPath(getUserPromptsDirectory()) + const isPromptFile = + canonicalPath.endsWith(promptFileExtension) && + workspaceUtils.isParentFolder(promptsDirectory, canonicalPath) + if (!isPromptFile) { + const { requiresAcceptance, warning } = await requiresPathAcceptance( + canonicalPath, + toolName, + this.#features.workspace, + this.#features.logging, + session?.approvedPaths + ) + if (requiresAcceptance) { + // The path on a chat card comes from a tool result, not the + // user, so a path outside the workspace is opened only after an + // explicit confirmation. Approving records the path for this + // tool so a later click does not prompt again. + const open: MessageActionItem = { title: 'Open' } + const cancel: MessageActionItem = { title: 'Cancel' } + const choice = await this.#features.lsp.window.showMessageRequest({ + type: MessageType.Warning, + message: `${warning ?? 'This file is outside your workspace.'}\n\n` + `Open ${canonicalPath}?`, + actions: [open, cancel], + }) + if (choice?.title !== open.title) { + this.#features.logging.info(`User declined to open out-of-workspace file from a chat card`) + return + } + session?.addApprovedPath(canonicalPath, toolName) + } + } + await this.#features.lsp.window.showDocument({ uri: URI.file(canonicalPath).toString() }) + } + async onFollowUpClicked(params: FollowUpClickParams) { this.#log(`onFollowUpClicked: ${JSON.stringify(params)}`) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.test.ts index 2186df2794..e7081346a0 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.test.ts @@ -6,6 +6,7 @@ import { workspaceUtils } from '@aws/lsp-core' import { Features } from '@aws/language-server-runtimes/server-interface/server' import * as workspaceUtilsModule from '@aws/lsp-core/out/util/workspaceUtils' import { TestFeatures } from '@aws/language-server-runtimes/testing' +import { ChatSessionService } from '../../chat/chatSessionService' import { Context } from 'mocha' // Re-export isSensitivePath for testing via the module's internal function @@ -28,6 +29,24 @@ describe('toolShared', () => { assert.strictEqual(isPathApproved(filePath, 'testTool', approvedPaths), true) }) + it('matches an exact Windows file approval stored by the session', () => { + const session = new ChatSessionService() + const filePath = 'C:\\workspace\\notes.txt' + session.addApprovedPath(filePath, 'fsRead') + assert.ok(session.approvedPaths.get('fsRead')?.has('C:/workspace/notes.txt')) + assert.strictEqual(isPathApproved(filePath, 'fsRead', session.approvedPaths), true) + assert.strictEqual(isPathApproved(filePath, 'fsWrite', session.approvedPaths), false) + assert.strictEqual(isPathApproved('C:\\workspace\\other.txt', 'fsRead', session.approvedPaths), false) + }) + + it('matches an exact UNC file approval stored by the session', () => { + const session = new ChatSessionService() + const filePath = '\\\\server\\share\\notes.txt' + session.addApprovedPath(filePath, 'fsRead') + assert.ok(session.approvedPaths.get('fsRead')?.has('//server/share/notes.txt')) + assert.strictEqual(isPathApproved(filePath, 'fsRead', session.approvedPaths), true) + }) + it('should return true if a path is a parent folder', () => { const approvedPaths = new Map([['testTool', new Set(['/test'])]]) const filePath = '/test/path/file.js' @@ -341,7 +360,7 @@ describe('toolShared', () => { }) it('should require acceptance for sensitive paths', async () => { - const filePath = '/home/user/.ssh/id_rsa' + const filePath = path.join(path.parse(process.cwd()).root, 'fixture-home', '.ssh', 'id_rsa') const result = await requiresPathAcceptance( filePath, diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts index 22ce189b31..f4f0de358f 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts @@ -203,7 +203,7 @@ export function isPathApproved(filePath: string, toolName: string, approvedPaths } // Normalize path separators for consistent comparison - const normalizedFilePath = filePath.replace(/\\\\/g, '/') + const normalizedFilePath = filePath.replace(/\\/g, '/') // Check if the exact path is approved for this tool if (toolPaths.has(filePath) || toolPaths.has(normalizedFilePath)) { @@ -215,7 +215,7 @@ export function isPathApproved(filePath: string, toolName: string, approvedPaths // Check if any approved path is a parent of the file path using isParentFolder for (const approvedPath of toolPaths) { - const normalizedApprovedPath = approvedPath.replace(/\\\\/g, '/') + const normalizedApprovedPath = approvedPath.replace(/\\/g, '/') // Check using the isParentFolder utility if (workspaceUtils.isParentFolder(normalizedApprovedPath, normalizedFilePath)) { @@ -346,5 +346,6 @@ function isSensitivePath(filePath: string): boolean { /\/dev\//, ] - return sensitivePatterns.some(pattern => pattern.test(filePath)) + const normalizedPath = process.platform === 'win32' ? filePath.replace(/\\/g, '/') : filePath + return sensitivePatterns.some(pattern => pattern.test(normalizedPath)) }