diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/grepSearch.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/grepSearch.ts index 9ae48ccae3..6a8cbf3b3b 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/grepSearch.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/grepSearch.ts @@ -79,6 +79,12 @@ export class GrepSearch { await closeWriter(writer) } + // TODO (workspace-boundary hardening): this acceptance check performs a + // string-only, non-symlink-aware workspace containment test on the raw + // search path. For parity with the file tools, route it through the + // symlink-aware requiresPathAcceptance helper (as fsRead/fsWrite/ + // listDirectory do) before this tool is enabled. Note this tool is not + // currently registered as an active tool (see toolServer.ts). public async requiresAcceptance(params: GrepSearchParams): Promise { const path = this.getSearchDirectory(params.path) return { requiresAcceptance: !workspaceUtils.isInWorkspace(getWorkspaceFolderPaths(this.workspace), path) } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/lspApplyWorkspaceEdit.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/lspApplyWorkspaceEdit.ts index 3cc4040535..870d37ecc6 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/lspApplyWorkspaceEdit.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/lspApplyWorkspaceEdit.ts @@ -28,6 +28,12 @@ export class LspApplyWorkspaceEdit { this.lsp = features.lsp } + // TODO (workspace-boundary hardening): this tool applies edits without a + // workspace-boundary or acceptance check. For parity with the file tools, + // add symlink-aware boundary validation and approval gating that mirror the + // fs* write tools (resolve each edited URI via requiresPathAcceptance) + // before this tool is enabled. Note this tool is not currently registered + // in any runtime (LspToolsServer is not enabled). public async invoke(params: LspApplyWorkspaceEditParams): Promise { try { if (!params.edit || !params.edit.changes) { diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/symlinkBoundary.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/symlinkBoundary.test.ts new file mode 100644 index 0000000000..d07cf7e89c --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/symlinkBoundary.test.ts @@ -0,0 +1,174 @@ +import * as assert from 'assert' +import * as fs from 'fs' +import * as os from 'os' +import * as path from 'path' +import { URI } from 'vscode-uri' +import { Features } from '@aws/language-server-runtimes/server-interface/server' +import { resolveSymlinkAwarePath, requiresPathAcceptance } from './toolShared' + +/** + * Real-filesystem tests for the symlink-aware workspace-boundary guard used by + * the file-writing/reading tools (fsWrite, fsRead, fsReplace, listDirectory, + * fileSearch) via requiresPathAcceptance. These exercise symlink resolution + * against the actual filesystem, since the guard's whole purpose is to reason + * about where a path physically lands, so they intentionally do not stub path + * utilities. + * + * The tests are OS-agnostic: paths are built with `path.join`, and both sides + * of every path assertion are normalized through the same fs.promises.realpath + * the production code uses (see `canon`). This matters on Windows, where the + * temp dir can surface as an 8.3 short name (e.g. RUNNER~1) that fs.realpath + * expands to the long name (runneradmin); normalizing both sides identically + * keeps the assertions stable. Any test that needs a symlink skips itself where + * the platform or user cannot create one (e.g. Windows without the privilege). + */ +describe('workspace boundary symlink handling', () => { + let root: string + let ws: string + let outside: string + + const noopLogging = { + info: () => {}, + warn: () => {}, + error: () => {}, + log: () => {}, + debug: () => {}, + } as unknown as Features['logging'] + + const makeWorkspace = (folder: string): Features['workspace'] => + ({ + getAllWorkspaceFolders: () => [{ uri: URI.file(folder).toString(), name: 'ws' }], + }) as unknown as Features['workspace'] + + /** + * Canonicalize a path the same way the production guard does + * (fs.promises.realpath), resolving the deepest existing ancestor for paths + * that do not exist yet. Used on BOTH sides of path assertions so a + * Windows 8.3 short name never causes a spurious mismatch. + */ + const canon = async (p: string): Promise => { + try { + return await fs.promises.realpath(p) + } catch { + return path.join(await fs.promises.realpath(path.dirname(p)), path.basename(p)) + } + } + + /** Create a symlink, returning false if the platform/user cannot. */ + const trySymlink = (target: string, linkPath: string): boolean => { + try { + fs.symlinkSync(target, linkPath) + return true + } catch { + return false + } + } + + beforeEach(() => { + const realTmp = fs.realpathSync(os.tmpdir()) + root = fs.mkdtempSync(path.join(realTmp, 'wsb-')) + ws = path.join(root, 'workspace') + outside = path.join(root, 'outside') + fs.mkdirSync(ws) + fs.mkdirSync(outside) + }) + + afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }) + }) + + describe('resolveSymlinkAwarePath', () => { + it('resolves a dangling leaf symlink to its (outside) target', async function () { + const target = path.join(outside, 'authorized_keys') // does not exist yet + const link = path.join(ws, 'setup-notes.md') + if (!trySymlink(target, link)) { + return this.skip() + } + const resolved = await resolveSymlinkAwarePath(link) + assert.strictEqual(await canon(resolved), await canon(target)) + }) + + it('resolves a nonexistent leaf under a symlinked ancestor to the real (outside) location', async function () { + const linkDir = path.join(ws, 'sub') // -> outside + if (!trySymlink(outside, linkDir)) { + return this.skip() + } + const resolved = await resolveSymlinkAwarePath(path.join(linkDir, 'newfile.txt')) + assert.strictEqual(await canon(resolved), await canon(path.join(outside, 'newfile.txt'))) + }) + + it('leaves a plain not-yet-created in-workspace path inside the workspace', async () => { + const resolved = await resolveSymlinkAwarePath(path.join(ws, 'brand-new.txt')) + assert.strictEqual(await canon(resolved), await canon(path.join(ws, 'brand-new.txt'))) + }) + + it('canonicalizes an existing regular file', async () => { + const file = path.join(ws, 'exists.txt') + fs.writeFileSync(file, 'x') + const resolved = await resolveSymlinkAwarePath(file) + assert.strictEqual(await canon(resolved), await canon(file)) + }) + + it('follows a chain of symlinks to the final (outside) target', async function () { + const finalTarget = path.join(outside, 'chained.txt') + const mid = path.join(ws, 'mid') + const head = path.join(ws, 'head') + if (!trySymlink(finalTarget, mid) || !trySymlink(mid, head)) { + return this.skip() + } + const resolved = await resolveSymlinkAwarePath(head) + assert.strictEqual(await canon(resolved), await canon(finalTarget)) + }) + }) + + describe('requiresPathAcceptance (write/read guard)', () => { + it('requires acceptance for a dangling in-workspace symlink that points outside', async function () { + const target = path.join(outside, 'authorized_keys') // dangling: parent exists, file does not + const link = path.join(ws, 'setup-notes.md') + if (!trySymlink(target, link)) { + return this.skip() + } + const result = await requiresPathAcceptance(link, 'fsWrite', makeWorkspace(ws), noopLogging) + assert.strictEqual(result.requiresAcceptance, true) + }) + + it('does not require acceptance for a normal new in-workspace file', async () => { + const result = await requiresPathAcceptance( + path.join(ws, 'notes.md'), + 'fsWrite', + makeWorkspace(ws), + noopLogging + ) + assert.strictEqual(result.requiresAcceptance, false) + }) + + it('still requires acceptance for a symlink to an existing outside file (regression)', async function () { + const target = path.join(outside, 'existing.txt') + fs.writeFileSync(target, 'secret') + const link = path.join(ws, 'link.txt') + if (!trySymlink(target, link)) { + return this.skip() + } + const result = await requiresPathAcceptance(link, 'fsWrite', makeWorkspace(ws), noopLogging) + assert.strictEqual(result.requiresAcceptance, true) + }) + + it('does not raise a false prompt when the workspace itself lives under a symlinked directory', async function () { + // Simulate macOS-style /tmp -> /private/tmp: workspace reached via a symlink. + const realWs = path.join(root, 'realws') + fs.mkdirSync(realWs) + const symWs = path.join(root, 'linkws') + if (!trySymlink(realWs, symWs)) { + return this.skip() + } + // A new file created through the symlinked workspace path is genuinely in-workspace. + const result = await requiresPathAcceptance( + path.join(symWs, 'inside.txt'), + 'fsWrite', + makeWorkspace(symWs), + noopLogging + ) + assert.strictEqual(result.requiresAcceptance, false) + }) + }) +}) 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 dfea83d502..92476196e1 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 @@ -258,13 +258,12 @@ describe('toolShared', () => { ) assert.strictEqual(result.requiresAcceptance, false) - // requiresPathAcceptance canonicalizes the path with path.resolve() before - // passing to isInWorkspace, so verify with the resolved path - const expectedPath = path.resolve(filePath) - assert.strictEqual( - isInWorkspaceStub.calledWith(['/workspace/folder1', '/workspace/folder2'], expectedPath), - true - ) + // requiresPathAcceptance canonicalizes the path (symlink-aware) before + // passing it to isInWorkspace. The exact canonical form is platform + // specific (and is covered by symlinkBoundary.test.ts), so here just + // verify isInWorkspace was consulted with an absolute, resolved path. + assert.strictEqual(isInWorkspaceStub.called, true) + assert.strictEqual(path.isAbsolute(isInWorkspaceStub.firstCall.args[1]), true) }) it('should return requiresAcceptance=true if path is not in workspace', async () => { @@ -284,11 +283,8 @@ describe('toolShared', () => { ) assert.strictEqual(result.requiresAcceptance, true) - const expectedPath = path.resolve(filePath) - assert.strictEqual( - isInWorkspaceStub.calledWith(['/workspace/folder1', '/workspace/folder2'], expectedPath), - true - ) + assert.strictEqual(isInWorkspaceStub.called, true) + assert.strictEqual(path.isAbsolute(isInWorkspaceStub.firstCall.args[1]), true) }) it('should return requiresAcceptance=true if an error occurs', async () => { 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 5bee9dacf2..0776a90819 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 @@ -5,6 +5,82 @@ import * as fs from 'fs' import * as path from 'path' import { CommandCategory } from './executeBash' +/** + * Resolve a path to its canonical on-disk location in a symlink-aware way, + * including when the final path segment is a symlink whose target does not + * yet exist (a "dangling" symlink), or when an ancestor directory is a + * symlink. + * + * Unlike fs.realpath, this does not throw for paths that do not exist yet. + * It follows a symlink at the leaf (even a dangling one), resolves the + * longest existing ancestor through the filesystem, then re-appends any + * remaining not-yet-created segments. The returned path therefore reflects + * where a subsequent read or write would actually land, so a workspace + * boundary check cannot be fooled by an in-workspace link name whose target + * points outside the workspace. A `visited` set bounds symlink-cycle + * traversal. + */ +export async function resolveSymlinkAwarePath(inputPath: string): Promise { + let current = path.resolve(inputPath) + const visited = new Set() + + // eslint-disable-next-line no-constant-condition + while (true) { + let stats: fs.Stats + try { + stats = await fs.promises.lstat(current) + } catch { + // `current` does not exist even as a symlink: resolve its existing + // ancestor chain (which may traverse symlinked directories) and + // re-append the final, not-yet-created segment. + const parent = path.dirname(current) + if (parent === current) { + return current + } + const resolvedParent = await resolveSymlinkAwarePath(parent) + return path.join(resolvedParent, path.basename(current)) + } + + if (stats.isSymbolicLink()) { + if (visited.has(current)) { + // Cyclic symlink: stop resolving and return what we have. + return current + } + visited.add(current) + const linkTarget = await fs.promises.readlink(current) + current = path.resolve(path.dirname(current), linkTarget) + continue + } + + // `current` exists and is not a symlink: canonicalize it, which also + // resolves any symlinked ancestor directories. + try { + return await fs.promises.realpath(current) + } catch { + return current + } + } +} + +/** + * Canonicalize workspace folder paths through the filesystem so boundary + * comparisons stay accurate even when a workspace lives under a symlinked + * directory (for example, macOS exposes temp directories under /var and /tmp + * which are symlinks into /private). Falls back to a lexical resolve for any + * folder that cannot be resolved. + */ +export async function canonicalizeWorkspaceFolders(workspaceFolderPaths: string[]): Promise { + return Promise.all( + workspaceFolderPaths.map(async folder => { + try { + return await fs.promises.realpath(folder) + } catch { + return path.resolve(folder) + } + }) + ) +} + interface Output { kind: Kind content: Content @@ -126,28 +202,15 @@ export async function requiresPathAcceptance( approvedPaths?: Map> ): Promise { try { - // Canonicalize the path through the filesystem so symlinks are resolved - // before the workspace-boundary check. path.resolve is string-only and - // would treat a symlink as in-workspace based on its name alone, even - // when its target lies outside the workspace. For paths that don't - // exist yet (e.g., a new file the agent is about to create), fall back - // to realpath of the parent directory joined with the basename. - let canonicalPath: string - try { - canonicalPath = await fs.promises.realpath(inputPath) - } catch (err: any) { - if (err && err.code === 'ENOENT') { - const parent = path.dirname(inputPath) - try { - const realParent = await fs.promises.realpath(parent) - canonicalPath = path.join(realParent, path.basename(inputPath)) - } catch { - canonicalPath = path.resolve(inputPath) - } - } else { - canonicalPath = path.resolve(inputPath) - } - } + // Canonicalize the path in a symlink-aware way before the + // workspace-boundary check. This resolves symlinks at every segment, + // including a symlink at the leaf whose target does not exist yet + // (a "dangling" symlink). A string-only resolve, or an fs.realpath + // that silently falls back to the literal link name when the target + // is missing, would treat such a link as in-workspace based on its + // name alone even though a write or read through it would land + // outside the workspace. + const canonicalPath = await resolveSymlinkAwarePath(inputPath) // Then check if the path is already approved for this specific tool if (isPathApproved(canonicalPath, toolName, approvedPaths)) { @@ -166,7 +229,10 @@ export async function requiresPathAcceptance( // This is the primary security check — files genuinely inside the workspace // are trusted regardless of their filename (e.g. "PasswordService.java", // "credentials/auth.ts", or paths under a "/dev/" folder). - const isInWs = workspaceUtils.isInWorkspace(workspaceFolders, canonicalPath) + // Workspace folders are canonicalized too so the comparison holds even + // when the workspace itself lives under a symlinked directory. + const canonicalWorkspaceFolders = await canonicalizeWorkspaceFolders(workspaceFolders) + const isInWs = workspaceUtils.isInWorkspace(canonicalWorkspaceFolders, canonicalPath) if (isInWs) { return { requiresAcceptance: false } }