Skip to content

Commit ede259f

Browse files
refactor(tools): accept optional allowSymlinks in resolveIsOutsideWorkspace; consolidate ListFiles state read; cover symlinked-ancestor ENOENT case (#169)
1 parent 1bf6c29 commit ede259f

4 files changed

Lines changed: 44 additions & 8 deletions

File tree

src/core/tools/BaseTool.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,21 @@ export abstract class BaseTool<TName extends ToolName> {
104104
* `allowSymlinksOutsideWorkspace` setting (#169 / #241). When that setting is enabled,
105105
* a symlink resolving outside the workspace is treated by its lexical path rather than
106106
* being blocked; otherwise symlink targets are resolved and the check fails closed.
107+
*
108+
* Callers that already read provider state in the same execution path can pass
109+
* `allowSymlinksOutsideWorkspace` to avoid a redundant `getState()` call; when omitted
110+
* it is read here.
107111
*/
108-
protected async resolveIsOutsideWorkspace(task: Task, absolutePath: string): Promise<boolean> {
109-
const allowSymlinksOutsideWorkspace =
110-
(await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? false
111-
return isPathOutsideWorkspace(absolutePath, { allowSymlinksOutsideWorkspace })
112+
protected async resolveIsOutsideWorkspace(
113+
task: Task,
114+
absolutePath: string,
115+
allowSymlinksOutsideWorkspace?: boolean,
116+
): Promise<boolean> {
117+
const allow =
118+
allowSymlinksOutsideWorkspace ??
119+
(await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ??
120+
false
121+
return isPathOutsideWorkspace(absolutePath, { allowSymlinksOutsideWorkspace: allow })
112122
}
113123

114124
/**

src/core/tools/ListFilesTool.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,18 @@ export class ListFilesTool extends BaseTool<"list_files"> {
3434
task.consecutiveMistakeCount = 0
3535

3636
const absolutePath = path.resolve(task.cwd, relDirPath)
37-
const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, absolutePath)
37+
38+
// Read provider state once and reuse it for both the workspace-boundary check
39+
// and the rooignore display preference.
40+
const { allowSymlinksOutsideWorkspace = false, showRooIgnoredFiles = false } =
41+
(await task.providerRef.deref()?.getState()) ?? {}
42+
const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(
43+
task,
44+
absolutePath,
45+
allowSymlinksOutsideWorkspace,
46+
)
3847

3948
const [files, didHitLimit] = await listFiles(absolutePath, recursive || false, 200)
40-
const { showRooIgnoredFiles = false } = (await task.providerRef.deref()?.getState()) ?? {}
4149

4250
const result = formatResponse.formatFilesList(
4351
absolutePath,

src/core/webview/webviewMessageHandler.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,8 +1208,12 @@ export const webviewMessageHandler = async (
12081208
// Workspace-boundary validation: prevent path traversal attacks.
12091209
// Honor the `allowSymlinksOutsideWorkspace` setting (#169 / #241) so symlink
12101210
// targets are resolved (fail-closed) unless the user opted in.
1211-
const { allowSymlinksOutsideWorkspace } = await provider.getState()
1212-
if (isPathOutsideWorkspace(absPath, { allowSymlinksOutsideWorkspace })) {
1211+
const state = await provider.getState()
1212+
if (
1213+
isPathOutsideWorkspace(absPath, {
1214+
allowSymlinksOutsideWorkspace: state.allowSymlinksOutsideWorkspace,
1215+
})
1216+
) {
12131217
provider.postMessageToWebview({
12141218
type: "fileContent",
12151219
fileContent: { path: relPath, content: null, error: "Path is outside workspace" },

src/utils/__tests__/pathUtils.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,20 @@ describe("isPathOutsideWorkspace", () => {
7272
expect(isPathOutsideWorkspace(path.join(linkDir, "deep.txt"))).toBe(true)
7373
})
7474

75+
it("treats a not-yet-existing file under a symlinked ancestor directory as OUTSIDE (#169)", () => {
76+
// Intersection of the ENOENT walk-up and symlink resolution: linked-dir is a
77+
// symlink to outsideDir and the target file doesn't exist yet. The walk-up resolves
78+
// the symlinked ancestor and re-appends the basename, landing outside the workspace.
79+
const linkDir = path.join(workspaceDir, "linked-dir")
80+
fs.symlinkSync(outsideDir, linkDir)
81+
82+
expect(isPathOutsideWorkspace(path.join(linkDir, "new-file.ts"))).toBe(true)
83+
// Opt-in (#246) keeps the lexical (inside) location and does not resolve the symlink.
84+
expect(isPathOutsideWorkspace(path.join(linkDir, "new-file.ts"), { allowSymlinksOutsideWorkspace: true })).toBe(
85+
false,
86+
)
87+
})
88+
7589
it("allows a symlink pointing outside when allowSymlinksOutsideWorkspace is enabled (#246)", () => {
7690
const secret = path.join(outsideDir, "secret.txt")
7791
fs.writeFileSync(secret, "secret")

0 commit comments

Comments
 (0)