fix(amazonq): require workspace containment before opening chat card files

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.

server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts

@@ -3,9 +3,9 @@
  * Will be deleted or merged.
  */
 
 import * as crypto from 'crypto'
 import * as path from 'path'
 import * as os from 'os'
 import {
     ChatTriggerType,
     Origin,
@@ -54,6 +54,7 @@
     ActiveEditorChangedParams,
     PinnedContextParams,
     ChatUpdateParams,
+    MessageActionItem,
     MessageType,
     ExecuteCommandParams,
     FollowUpClickParams,
@@ -169,6 +170,8 @@
     CommandValidation,
     ExplanatoryParams,
     InvokeOutput,
+    requiresPathAcceptance,
+    resolveCanonicalPath,
     resolveSymlinkAwarePath,
     ToolApprovalException,
 } from './tools/toolShared'
@@ -4023,18 +4026,65 @@
                     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) {
             this.#features.logging.error(`Error opening file: ${e.message}`)
         }
     }
 
+    /**
+     * 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<void> {
+        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)}`)
 

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,

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))
 }