From 20e45b9d52c790a25f594afb1fdded9976d6bfa9 Mon Sep 17 00:00:00 2001 From: Dung Dong Date: Mon, 5 Oct 2026 23:18:18 +0000 Subject: [PATCH 1/3] fix(amazonq): require workspace containment before opening chat card files onFileClicked passed card paths to showDocument with no containment check. The fsRead branch opened params.filePath directly, the default branch opened params.fullPath directly, and resolveAbsolutePath joined a relative filePath onto the workspace root, so a traversing relative path escaped as well. Card paths originate in model tool results and service findings, so a manipulated response plus one click opened any readable file and fed it to completion context. All three open paths now go through openIfAllowed, which canonicalizes the path and reuses requiresPathAcceptance: a file opens only when it is inside a workspace folder or the user already approved it for the card's tool in this session. The user prompts directory is allowed for .prompt.md files, which resolveAbsolutePath deliberately resolves there. Refused clicks are logged and otherwise ignored. Tests cover an inside file, an outside fullPath, an escaping relative path, an fsRead card for an unapproved and an approved outside path, an fsWrite approval not unlocking an fsRead card, a prompt file in the prompts directory, and a non-prompt file in that directory. --- .../agenticChat/agenticChatController.test.ts | 119 ++++++++++++++++++ .../agenticChat/agenticChatController.ts | 41 +++++- 2 files changed, 158 insertions(+), 2 deletions(-) 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..5e975f998c 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, @@ -3636,6 +3639,122 @@ ${' '.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 + + 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() + 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()]) + }) + + 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) + }) + + 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..49755d9e58 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts @@ -169,6 +169,8 @@ import { CommandValidation, ExplanatoryParams, InvokeOutput, + requiresPathAcceptance, + resolveCanonicalPath, resolveSymlinkAwarePath, ToolApprovalException, } from './tools/toolShared' @@ -4023,11 +4025,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 +4037,41 @@ 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 } = await requiresPathAcceptance( + canonicalPath, + toolName, + this.#features.workspace, + this.#features.logging, + session?.approvedPaths + ) + if (requiresAcceptance) { + this.#features.logging.warn( + `Refusing to open file outside the workspace from a chat card: ${canonicalPath}` + ) + return + } + } + await this.#features.lsp.window.showDocument({ uri: URI.file(canonicalPath).toString() }) + } + async onFollowUpClicked(params: FollowUpClickParams) { this.#log(`onFollowUpClicked: ${JSON.stringify(params)}`) From ec39a474dc0818a7509d6306e7530ee26599224f Mon Sep 17 00:00:00 2001 From: Dung Dong Date: Tue, 6 Oct 2026 16:50:39 +0000 Subject: [PATCH 2/3] fix(amazonq): prompt before opening an out-of-workspace chat card file The previous commit refused such opens outright and only logged. A card can legitimately point outside the workspace, for example a findings path the user wants to inspect, so the open is now gated by a warning dialog instead of dropped. openIfAllowed shows a showMessageRequest warning with Open and Cancel when requiresPathAcceptance requires acceptance. The message leads with the specific reason the check returned, such as the sensitive-file warning, and names the canonical path. Choosing Open records the path for the card's tool in the session's approved paths, so a repeat click or a later tool use on the same path does not prompt again. Cancel or dismissal aborts the open. In-workspace, already-approved, and prompt directory paths open without a dialog as before. MessageType and MessageActionItem are imported from the protocol module, which already supplied MessageType to this file; a second import from server-interface produced a duplicate identifier. Tests cover the dialog contents and action order, Open opening the file, Cancel and dismissal refusing, the choice being remembered across clicks, the sensitive-path reason surfacing in the dialog, and existing approvals suppressing the dialog. --- .../agenticChat/agenticChatController.test.ts | 62 ++++++++++++++++++- .../agenticChat/agenticChatController.ts | 23 +++++-- 2 files changed, 79 insertions(+), 6 deletions(-) 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 5e975f998c..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 @@ -38,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' @@ -3650,6 +3650,10 @@ ${' '.repeat(8)}} 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-'))) @@ -3663,6 +3667,10 @@ ${' '.repeat(8)}} ]) 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 }) }) @@ -3713,6 +3721,8 @@ ${' '.repeat(8)}} 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 () => { @@ -3725,6 +3735,56 @@ ${' '.repeat(8)}} 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 () => { 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 49755d9e58..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, @@ -4055,7 +4056,7 @@ export class AgenticChatController implements ChatHandlers { canonicalPath.endsWith(promptFileExtension) && workspaceUtils.isParentFolder(promptsDirectory, canonicalPath) if (!isPromptFile) { - const { requiresAcceptance } = await requiresPathAcceptance( + const { requiresAcceptance, warning } = await requiresPathAcceptance( canonicalPath, toolName, this.#features.workspace, @@ -4063,10 +4064,22 @@ export class AgenticChatController implements ChatHandlers { session?.approvedPaths ) if (requiresAcceptance) { - this.#features.logging.warn( - `Refusing to open file outside the workspace from a chat card: ${canonicalPath}` - ) - return + // 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() }) From ac44c787a9956435740b9ed53f7a1f38cf65ade4 Mon Sep 17 00:00:00 2001 From: Dung Dong Date: Tue, 6 Oct 2026 18:07:20 +0000 Subject: [PATCH 3/3] fix(amazonq): normalize Windows paths in approval lookup Match single backslash separators consistently with session approval storage. Normalize Windows paths before sensitive-location pattern checks. Add session round-trip tests for drive and UNC paths and use native separators in the sensitive-path fixture. --- .../agenticChat/tools/toolShared.test.ts | 21 ++++++++++++++++++- .../agenticChat/tools/toolShared.ts | 7 ++++--- 2 files changed, 24 insertions(+), 4 deletions(-) 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)) }