fix(security): address review on workspace-boundary enforcement (#169)

- GenerateImageTool: apply the workspace-boundary check to the *input* image
  path too, not just the output path. A symlink inside the workspace pointing
  outside could otherwise be read and base64-encoded/forwarded upstream.
- BaseTool.resolveIsOutsideWorkspace: wrap getState() in try/catch defaulting
  to false, so a provider torn down mid-operation no longer aborts the tool.
- ReadFileTool.requestApproval: read provider state once and pass the flag down
  to each per-file boundary check in a batch, instead of a getState() per file.
- pathUtils.isPathOutsideWorkspace: normalize case on macOS/Windows before
  comparing, so realpath casing differences don't cause a path inside the
  workspace to be reported as outside (false negative). +regression test.

Author: proyectoauraorg

src/core/tools/BaseTool.ts

@@ -114,10 +114,17 @@ export abstract class BaseTool<TName extends ToolName> {
 		absolutePath: string,
 		allowSymlinksOutsideWorkspace?: boolean,
 	): Promise<boolean> {
-		const allow =
-			allowSymlinksOutsideWorkspace ??
-			(await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ??
-			false
+		let allow = allowSymlinksOutsideWorkspace
+		if (allow === undefined) {
+			try {
+				allow = (await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? false
+			} catch {
+				// The provider may have been torn down mid-operation, in which case
+				// `getState()` rejects. Don't abort the tool call — default to the safe
+				// (symlink-resolving, fail-closed) behavior instead.
+				allow = false
+			}
+		}
 		return isPathOutsideWorkspace(absolutePath, { allowSymlinksOutsideWorkspace: allow })
 	}
 

src/core/tools/GenerateImageTool.ts

@@ -82,6 +82,22 @@ export class GenerateImageTool extends BaseTool<"generate_image"> {
 				return
 			}
 
+			// Apply the workspace-boundary check to the input image too (#169): a symlink
+			// living inside the workspace but pointing outside it could otherwise be read
+			// and base64-encoded/forwarded upstream, bypassing the boundary.
+			const inputIsOutsideWorkspace = await this.resolveIsOutsideWorkspace(
+				task,
+				inputImageFullPath,
+				state?.allowSymlinksOutsideWorkspace,
+			)
+			if (inputIsOutsideWorkspace) {
+				const message = `Input image is outside the workspace: ${getReadablePath(task.cwd, inputImagePath)}`
+				await task.say("error", message)
+				task.didToolFailInCurrentTurn = true
+				pushToolResult(formatResponse.toolError(message))
+				return
+			}
+
 			try {
 				const imageBuffer = await fs.readFile(inputImageFullPath)
 				const imageExtension = path.extname(inputImageFullPath).toLowerCase().replace(".", "")
@@ -162,7 +178,11 @@ export class GenerateImageTool extends BaseTool<"generate_image"> {
 		}
 
 		const fullPath = path.resolve(task.cwd, relPath)
-		const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath)
+		const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(
+			task,
+			fullPath,
+			state?.allowSymlinksOutsideWorkspace,
+		)
 
 		const sharedMessageProps = {
 			tool: "generateImage" as const,

src/core/tools/ReadFileTool.ts

@@ -430,13 +430,22 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 	): Promise<void> {
 		if (filesToApprove.length === 0) return
 
+		// Read provider state once and pass the flag down, rather than firing a separate
+		// `getState()` lookup per file in the batch below (mirrors ListFilesTool).
+		let allowSymlinks = false
+		try {
+			allowSymlinks = (await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? false
+		} catch {
+			allowSymlinks = false
+		}
+
 		if (filesToApprove.length > 1) {
 			// Batch approval
 			const batchFiles = await Promise.all(
 				filesToApprove.map(async (fileResult) => {
 					const relPath = fileResult.path
 					const fullPath = path.resolve(task.cwd, relPath)
-					const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath)
+					const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath, allowSymlinks)
 					const readablePath = getReadablePath(task.cwd, relPath)
 
 					const lineSnippet = this.getLineSnippet(fileResult.entry!)
@@ -502,7 +511,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 			const fileResult = filesToApprove[0]
 			const relPath = fileResult.path
 			const fullPath = path.resolve(task.cwd, relPath)
-			const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath)
+			const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath, allowSymlinks)
 			const lineSnippet = this.getLineSnippet(fileResult.entry!)
 
 			const startLine = this.getStartLine(fileResult.entry!)

src/utils/__tests__/pathUtils.spec.ts

@@ -165,4 +165,31 @@ describe("isPathOutsideWorkspace", () => {
 		mockWorkspace.folders = []
 		expect(isPathOutsideWorkspace(path.join(workspaceDir, "file.ts"))).toBe(true)
 	})
+
+	it("normalizes case on case-insensitive platforms so a differently-cased inside path is still inside (#241)", () => {
+		const originalPlatform = process.platform
+		Object.defineProperty(process, "platform", { value: "darwin", configurable: true })
+
+		const inside = path.join(workspaceDir, "File.ts")
+		fs.writeFileSync(inside, "x")
+
+		// On case-insensitive macOS/Windows, realpath can return a different case for the
+		// resolved file than VS Code registered for the workspace folder. Simulate that by
+		// upper-casing the "workspace" segment only for the target file's resolution.
+		const realNative = fs.realpathSync.native
+		const spy = vi.spyOn(fs.realpathSync, "native").mockImplementation(((p: string) => {
+			const resolved = realNative(p)
+			return p === inside
+				? resolved.replace(`${path.sep}workspace${path.sep}`, `${path.sep}WORKSPACE${path.sep}`)
+				: resolved
+		}) as typeof fs.realpathSync.native)
+
+		try {
+			// Without case normalization the case mismatch would wrongly report "outside".
+			expect(isPathOutsideWorkspace(inside)).toBe(false)
+		} finally {
+			spy.mockRestore()
+			Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true })
+		}
+	})
 })

src/utils/pathUtils.ts

@@ -82,6 +82,14 @@ export function isPathOutsideWorkspace(
 		return true
 	}
 
+	// On case-insensitive filesystems (macOS APFS/HFS+, Windows) `realpath` may return a
+	// different case than VS Code registered for the workspace folder, which would make a
+	// path that is actually inside the workspace compare as "outside" (a false negative on
+	// the security boundary). Normalize case before comparing on those platforms only.
+	const caseInsensitive = process.platform === "darwin" || process.platform === "win32"
+	const normalize = (p: string) => (caseInsensitive ? p.toLowerCase() : p)
+	const target = normalize(absolutePath)
+
 	// Check if the path is within any workspace folder
 	return !vscode.workspace.workspaceFolders.some((folder) => {
 		// Resolve the workspace folder too, in case it is itself reached via a symlink.
@@ -93,6 +101,7 @@ export function isPathOutsideWorkspace(
 			return false
 		}
 		// Path is inside a workspace if it equals the workspace path or is a subfolder
-		return absolutePath === folderPath || absolutePath.startsWith(folderPath + path.sep)
+		const base = normalize(folderPath)
+		return target === base || target.startsWith(base + path.sep)
 	})
 }