fix(security): follow symlinks in workspace boundary check (#169)#241
fix(security): follow symlinks in workspace boundary check (#169)#241proyectoauraorg wants to merge 16 commits into
Conversation
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughResolves symlink bypass by canonicalizing paths and updating workspace containment checks (fail-closed). Adds tests, a new ChangesSymlink-aware path containment validation
Sequence Diagram(s)sequenceDiagram
participant Webview
participant webviewMessageHandler
participant BaseTool.resolveIsOutsideWorkspace
participant isPathOutsideWorkspace
participant fs_realpath_native
Webview->>webviewMessageHandler: readFileContent(file request)
webviewMessageHandler->>BaseTool.resolveIsOutsideWorkspace: resolve check (provider state)
BaseTool.resolveIsOutsideWorkspace->>isPathOutsideWorkspace: isPathOutsideWorkspace(path, {allowSymlinksOutsideWorkspace})
isPathOutsideWorkspace->>fs_realpath_native: fs.realpathSync.native (walk up on ENOENT)
fs_realpath_native-->>isPathOutsideWorkspace: resolved path or error
isPathOutsideWorkspace-->>BaseTool.resolveIsOutsideWorkspace: containment decision
BaseTool.resolveIsOutsideWorkspace-->>webviewMessageHandler: result
webviewMessageHandler-->>Webview: respond (file or outside error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@proyectoauraorg Thank you! I think this needs to be optional via the
setting suggested in #169. There will be inevitably be users whose workflow will be broken without such a setting. |
| try { | ||
| const resolved = fs.realpathSync.native(current) | ||
| return trailing.length > 0 ? path.join(resolved, ...trailing.reverse()) : resolved | ||
| } catch { |
There was a problem hiding this comment.
What happens if realpathSync.native throws something other than ENOENT — e.g. EACCES on a symlink whose target has restricted permissions? The walk-up would skip the symlink entirely and return the lexical path, so isPathOutsideWorkspace would report "inside workspace" even though the real target might be outside.
For a security boundary, would it be safer to catch only ENOENT and let other errors propagate (or treat them as "outside")? Something like:
| } catch { | |
| } catch (err: unknown) { | |
| if (!(err instanceof Error && 'code' in err && (err as NodeJS.ErrnoException).code === 'ENOENT')) { | |
| // Non-ENOENT error (e.g. EACCES) — fail closed for safety. | |
| return path.resolve(target) | |
| } |
There was a problem hiding this comment.
Great catch — thanks @edelauna 🙏
Fixed in 49a70d9: the walk-up now only happens on ENOENT. Any other error (e.g. EACCES on a symlink whose target has restricted permissions) propagates, and isPathOutsideWorkspace catches it and fails closed — treating the path as outside rather than falling back to the lexical path.
I went with propagate-and-fail-closed instead of returning path.resolve(target) directly, because returning the lexical path would still report a workspace-internal symlink as "inside" in exactly the case you flagged. Added a regression test that stubs realpath to throw EACCES.
Happy to adjust if you'd prefer a different shape.
There was a problem hiding this comment.
Addressed in commit `49a70d9` + follow-up `ede259f2d`.
The `realPathOrNearest()` helper now catches only `ENOENT` — any other error (EACCES, ELOOP, etc.) falls through to fail-closed: the function treats the path as outside the workspace, which is the safe default for a security boundary.
} catch (err: unknown) {
if (isNodeError(err) && err.code === "ENOENT") {
return fallback
}
// Non-ENOENT errors → fail-closed
throw err
}Verified: `tsc --noEmit` clean, 160 tests pass.
|
Thanks @nh2 — that's a fair point. Symlinks pointing outside the workspace are a legitimate workflow for some users, and a hard block shouldn't break them silently. The opt-in setting from #169 ("Include symlinks resolving outside of workspace") is the right way to cover that. My suggestion would be to land this PR as the default-safe security fix and add the setting as a focused follow-up, so the config + UI + i18n changes get their own review — but I'm equally happy to add it here if a single PR is preferred. @edelauna any preference on scope? |
|
@proyectoauraorg lets track the ui/setting as a sub-issue. I'll wait to approve this PR until we also have the settings so that we can push both in the same release. |
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
|
Added the opt-in setting (#246) to this PR so the fix and the escape hatch ship together, as discussed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webview-ui/src/components/settings/SettingsView.tsx (1)
406-407: ⚡ Quick winAdd local webview tests for the new symlink setting wiring.
Please add/extend Vitest coverage for this setting’s dirty-state and save payload path (including init vs user-edit behavior).
As per coding guidelines,
webview-ui/src/**/*.{ts,tsx}: “Prefer localwebview-uitests… Add or update Vitest coverage underwebview-ui/src/**/__tests__”, andwebview-ui/src/**/SettingsView.{ts,tsx}: “tests should distinguish automatic initialization from real user edits.”Also applies to: 840-840
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/SettingsView.tsx` around lines 406 - 407, Add Vitest unit tests under webview-ui/src/**/__tests__ that cover the new allowSymlinksOutsideWorkspace wiring in the SettingsView component: verify initial initialization (when SettingsView reads default or persisted settings) does not mark the form as dirty and does not include unintended values in the save payload, and verify a real user edit to the allowSymlinksOutsideWorkspace toggle marks the view as dirty and produces the expected save payload (including allowSymlinksOutsideWorkspace and maxImageFileSize fields). Target the SettingsView component (and any local helpers used to build the save payload) and write assertions for initial vs user-edited behavior, dirty-state changes, and that the saved payload contains the correct boolean/number values for allowSymlinksOutsideWorkspace and maxImageFileSize.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 406-407: Add Vitest unit tests under webview-ui/src/**/__tests__
that cover the new allowSymlinksOutsideWorkspace wiring in the SettingsView
component: verify initial initialization (when SettingsView reads default or
persisted settings) does not mark the form as dirty and does not include
unintended values in the save payload, and verify a real user edit to the
allowSymlinksOutsideWorkspace toggle marks the view as dirty and produces the
expected save payload (including allowSymlinksOutsideWorkspace and
maxImageFileSize fields). Target the SettingsView component (and any local
helpers used to build the save payload) and write assertions for initial vs
user-edited behavior, dirty-state changes, and that the saved payload contains
the correct boolean/number values for allowSymlinksOutsideWorkspace and
maxImageFileSize.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ff706651-a819-4254-9e23-9915d643fbf3
📒 Files selected for processing (29)
.changeset/symlink-workspace-boundary.mdpackages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tssrc/core/tools/BaseTool.tssrc/core/tools/EditFileTool.tssrc/core/tools/ListFilesTool.tssrc/core/tools/SearchReplaceTool.tssrc/utils/__tests__/pathUtils.spec.tssrc/utils/pathUtils.tswebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
proyectoauraorg
force-pushed
the
fix/169-symlink-workspace-boundary
branch
from
7652419 to
19cc696
Compare
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
proyectoauraorg
force-pushed
the
fix/169-symlink-workspace-boundary
branch
from
19cc696 to
f71b6d6
Compare
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Nice thanks for addressing this.
| protected async resolveIsOutsideWorkspace(task: Task, absolutePath: string): Promise<boolean> { | ||
| const allowSymlinksOutsideWorkspace = | ||
| (await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? false | ||
| return isPathOutsideWorkspace(absolutePath, { allowSymlinksOutsideWorkspace }) | ||
| } |
There was a problem hiding this comment.
Nice helper — but 6 other BaseTool subclasses still call isPathOutsideWorkspace() directly, bypassing symlink resolution. They already inherit this method; they just need the same await this.resolveIsOutsideWorkspace(task, absolutePath) swap applied here:
ReadFileTool.ts(lines 439, 504, 654, 701) — primary read vector from [BUG] vscode out-of-workspace read protection trivially circumvented by symlinks #169WriteToFileTool.ts(lines 89, 234)ApplyPatchTool.ts(lines 163, 253, 313, 399, 472)EditTool.ts(lines 177, 256)SearchFilesTool.ts(lines 48, 80)GenerateImageTool.ts(line 166)
webviewMessageHandler.ts:1209 also needs updating — it's not a BaseTool subclass, so the setting would need to be fetched from the provider and passed as { allowSymlinksOutsideWorkspace } directly.
There was a problem hiding this comment.
Resolved in commits `09d4ab586` + `ede259f2d`.
All 8 BaseTool subclasses now use `this.resolveIsOutsideWorkspace()` instead of calling `isPathOutsideWorkspace()` directly:
| Tool | Sites migrated |
|---|---|
| ReadFileTool.ts | 4 (lines 439, 505, 655, 702) |
| WriteToFileTool.ts | 2 (lines 88, 233) |
| ApplyPatchTool.ts | 5 (lines 162, 252, 312, 398, 471) |
| EditTool.ts | 2 (lines 176, 255) |
| EditFileTool.ts | 3 (lines 159, 401, 514) |
| SearchFilesTool.ts | 2 (lines 47, 79) |
| SearchReplaceTool.ts | 2 (lines 172, 264) |
| ListFilesTool.ts | 2 (lines 42, 84) |
| GenerateImageTool.ts | 1 (line 165) |
Grep residual confirms 0 direct `isPathOutsideWorkspace` calls outside BaseTool/_tests_.
|
Thanks for the thorough review @edelauna. Addressed all three points in A) Remaining direct
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/tools/BaseTool.ts`:
- Around line 108-112: The workspace-boundary check in resolveIsOutsideWorkspace
should fail closed when provider state lookup fails: wrap the call to
task.providerRef.deref()?.getState() in a try/catch (or otherwise handle
rejected Promise) and if it throws or returns undefined, set
allowSymlinksOutsideWorkspace = false instead of propagating the error; then
call isPathOutsideWorkspace(absolutePath, { allowSymlinksOutsideWorkspace }).
This ensures resolveIsOutsideWorkspace, task.providerRef.deref()?.getState(),
allowSymlinksOutsideWorkspace, and isPathOutsideWorkspace continue
deterministically even on transient provider-state failures.
In `@src/core/tools/GenerateImageTool.ts`:
- Around line 164-165: The workspace-boundary check is only applied to the
output file path; add the same restriction for image inputs by resolving and
validating inputImageFullPath before using it. After you compute
inputImageFullPath (the resolved path for inputImagePath/inputImageFullPath),
call this.resolveIsOutsideWorkspace(task, inputImageFullPath) and handle the
boolean result the same way you handle isOutsideWorkspace for outputs (e.g.,
reject/throw or return an error) so symlinked images that resolve outside the
workspace are blocked; reference the resolveIsOutsideWorkspace method, the
inputImagePath/inputImageFullPath variables, and the task context when adding
this check.
In `@src/utils/pathUtils.ts`:
- Around line 24-49: realPathOrNearest currently treats any ENOENT as a missing
segment and walks up, which hides dangling symlink ancestors; change the ENOENT
branch in realPathOrNearest to call fs.lstatSync(current) and if lstat succeeds
and reports isSymbolicLink(), re-throw the ENOENT (propagate) so callers (e.g.,
isPathOutsideWorkspace) fail closed by default; otherwise continue the existing
walk-up behavior for true missing segments. Add regression tests in
src/utils/__tests__/pathUtils.spec.ts: create a workspace/link pointing to a
non-existent outside target and assert that isPathOutsideWorkspace(...) returns
true by default, and assert that with { allowSymlinksOutsideWorkspace: true }
the lexical classification remains false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6404ebec-38e8-4cd1-a785-967983ef18ed
📒 Files selected for processing (40)
.changeset/symlink-workspace-boundary.mdpackages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tssrc/core/tools/ApplyPatchTool.tssrc/core/tools/BaseTool.tssrc/core/tools/EditFileTool.tssrc/core/tools/EditTool.tssrc/core/tools/GenerateImageTool.tssrc/core/tools/ListFilesTool.tssrc/core/tools/ReadFileTool.tssrc/core/tools/SearchFilesTool.tssrc/core/tools/SearchReplaceTool.tssrc/core/tools/WriteToFileTool.tssrc/core/tools/__tests__/readFileTool.spec.tssrc/core/webview/__tests__/webviewMessageHandler.readFileContent.spec.tssrc/core/webview/webviewMessageHandler.tssrc/utils/__tests__/pathUtils.spec.tssrc/utils/pathUtils.tswebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsxwebview-ui/src/components/settings/__tests__/SettingsView.spec.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
✅ Files skipped from review due to trivial changes (11)
- webview-ui/src/i18n/locales/nl/settings.json
- webview-ui/src/i18n/locales/pl/settings.json
- webview-ui/src/i18n/locales/hi/settings.json
- .changeset/symlink-workspace-boundary.md
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
- webview-ui/src/i18n/locales/id/settings.json
- webview-ui/src/i18n/locales/es/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/en/settings.json
| function realPathOrNearest(target: string): string { | ||
| let current = path.resolve(target) | ||
| const trailing: string[] = [] | ||
|
|
||
| // Walk up until an existing path can be resolved, bounded by the filesystem root. | ||
| while (true) { | ||
| try { | ||
| const resolved = fs.realpathSync.native(current) | ||
| return trailing.length > 0 ? path.join(resolved, ...trailing.reverse()) : resolved | ||
| } catch (err) { | ||
| if (!isErrnoException(err, "ENOENT")) { | ||
| // Non-ENOENT (e.g. EACCES): don't mask it with a walk-up — propagate so the | ||
| // caller's security check can fail closed instead of falling through to the | ||
| // lexical path. | ||
| throw err | ||
| } | ||
| const parent = path.dirname(current) | ||
| if (parent === current) { | ||
| // Reached the root without finding an existing path; fall back to the | ||
| // lexically resolved path. | ||
| return path.resolve(target) | ||
| } | ||
| trailing.push(path.basename(current)) | ||
| current = parent | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files
ls -R
echo "== Show pathUtils.ts =="
sed -n '1,200p' src/utils/pathUtils.ts || true
echo "== Find realPathOrNearest usages =="
rg -n "realPathOrNearest" .
echo "== Find isPathOutsideWorkspace usages =="
rg -n "isPathOutsideWorkspace" .
echo "== Find symlink-related logic/tests =="
rg -n "symlink|realpathSync|ENOENT|allowSymlinksOutsideWorkspace" . -SRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pathUtils.ts (relevant section) =="
sed -n '1,220p' src/utils/pathUtils.ts
echo "== pathUtils.spec.ts (look for realPathOrNearest / symlink) =="
sed -n '1,260p' src/utils/__tests__/pathUtils.spec.ts || true
echo "== rg: realPathOrNearest definitions/usages =="
rg -n "function realPathOrNearest|realPathOrNearest" src/utils/pathUtils.ts src -S
echo "== rg: isPathOutsideWorkspace usages =="
rg -n "isPathOutsideWorkspace" src -S
echo "== rg: allowSymlinksOutsideWorkspace =="
rg -n "allowSymlinksOutsideWorkspace" src -S
echo "== rg: ENOENT / realpath / symlink (tests) =="
rg -n "ENOENT|realpathSync|symlink" src/utils/__tests__ src/__tests__ src/core/__tests__ -SRepository: Zoo-Code-Org/Zoo-Code
Length of output: 19159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const fs = require('fs');
const os = require('os');
const path = require('path');
function isErrnoException(err, code) {
return err instanceof Error && err.code === code;
}
function realPathOrNearest(target) {
let current = path.resolve(target);
const trailing = [];
while (true) {
try {
const resolved = fs.realpathSync.native(current);
return trailing.length > 0 ? path.join(resolved, ...trailing.reverse()) : resolved;
} catch (err) {
if (!isErrnoException(err, "ENOENT")) throw err;
const parent = path.dirname(current);
if (parent === current) return path.resolve(target);
trailing.push(path.basename(current));
current = parent;
}
}
}
function isInsideWorkspace(absolutePath, workspaceFolderPath) {
return absolutePath === workspaceFolderPath || absolutePath.startsWith(workspaceFolderPath + path.sep);
}
const tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "zoo-pathutils-dangling-")));
const workspaceDir = path.join(tmpRoot, "workspace");
const outsideDir = path.join(tmpRoot, "outside");
fs.mkdirSync(workspaceDir);
fs.mkdirSync(outsideDir);
// dangling symlink: points outside to a non-existent directory
const danglingTargetDir = path.join(outsideDir, "missing-target-dir"); // does not exist
const linkPath = path.join(workspaceDir, "link");
fs.symlinkSync(danglingTargetDir, linkPath);
const filePath = path.join(workspaceDir, "link", "new.txt"); // target dir doesn't exist yet
console.log("tmpRoot:", tmpRoot);
console.log("workspaceDir:", workspaceDir);
console.log("outsideDir:", outsideDir);
console.log("dangling symlink:", linkPath, "->", danglingTargetDir);
console.log("filePath:", filePath);
let realpathThrows = false;
try {
fs.realpathSync.native(filePath);
} catch (e) {
realpathThrows = true;
console.log("realpathSync.native(filePath) throws:", e.code);
}
const workspaceResolved = realPathOrNearest(workspaceDir);
const fileResolved = realPathOrNearest(filePath);
console.log("realPathOrNearest(workspaceDir):", workspaceResolved);
console.log("realPathOrNearest(filePath):", fileResolved);
console.log("containment check (would be inside):", isInsideWorkspace(fileResolved, workspaceResolved));
console.log("would boundary allow (isPathOutsideWorkspace=false):", !isInsideWorkspace(fileResolved, workspaceResolved));
fs.rmSync(tmpRoot, { recursive: true, force: true });
NODERepository: Zoo-Code-Org/Zoo-Code
Length of output: 735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== BaseTool.ts path boundary check =="
sed -n '1,200p' src/core/tools/BaseTool.ts
echo "== write-to-file tool uses boundary check? =="
sed -n '1,220p' src/core/tools/WriteToFileTool.ts 2>/dev/null || true
sed -n '1,260p' src/core/tools/writeToFileTool.ts 2>/dev/null || true
ls src/core/tools | rg -n "write|file" || true
# Specifically locate WriteToFileTool or similar
rg -n "class .*WriteToFile|isPathOutsideWorkspace" src/core/tools -S
echo "== webviewMessageHandler path check =="
sed -n '1160,1260p' src/core/webview/webviewMessageHandler.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 19975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg: isOutsideWorkspace usage =="
rg -n "isOutsideWorkspace" src -S
echo "== rg: 'outside workspace' / validation messages =="
rg -n "outside workspace|Path is outside workspace|fail-closed|outsideWorkspace" src -S
echo "== rg: resolveIsOutsideWorkspace usage =="
rg -n "resolveIsOutsideWorkspace" src -S
echo "== inspect WriteToFileTool after sharedMessageProps =="
rg -n "sharedMessageProps" -n src/core/tools/WriteToFileTool.ts -S
sed -n '120,260p' src/core/tools/WriteToFileTool.ts
sed -n '260,420p' src/core/tools/WriteToFileTool.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 14655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect askApproval implementation in Task =="
rg -n "askApproval\\(" src/core -S
rg -n "function askApproval|async askApproval|askApproval:" src/core -S
# Show Task methods around where approvals are handled
rg -n "class Task" -S src/core/task/Task.ts
sed -n '1,260p' src/core/task/Task.ts
echo "== Inspect auto-approval gating using tool.isOutsideWorkspace =="
sed -n '120,240p' src/core/auto-approval/index.ts
echo "== Inspect formatResponse/tool payload usage =="
rg -n "isOutsideWorkspace" src/core/prompts src/core/auto-approval src/core/task -S
# inspect tool result payload shape maybe
rg -n "alwaysAllowWriteOutsideWorkspace|alwaysAllowReadOnlyOutsideWorkspace" src/core -SRepository: Zoo-Code-Org/Zoo-Code
Length of output: 25806
Handle dangling symlink ancestors during the ENOENT walk-up (fail closed by default).
realPathOrNearest() currently treats any ENOENT as a missing segment: for workspace/link/new.txt where link is a dangling symlink to outside/... (target doesn’t exist yet), the walk-up re-appends link and the containment check classifies it as inside (isPathOutsideWorkspace === false), which then feeds isOutsideWorkspace into write approvals/auto-approvals.
Change the ENOENT path-walk to detect dangling symlink components (e.g., when ENOENT is thrown, lstatSync(current) succeeds and reports isSymbolicLink()), and then throw so isPathOutsideWorkspace fails closed (treat as outside) in the default secure mode.
Add a regression test in src/utils/__tests__/pathUtils.spec.ts covering:
workspace/link/*wherelinkis a symlink to a non-existent target directory outside the workspace → defaultisPathOutsideWorkspace(...)returnstrue.- (Optional) keep existing opt-in behavior: with
{ allowSymlinksOutsideWorkspace: true }, lexical classification remainsfalse.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/pathUtils.ts` around lines 24 - 49, realPathOrNearest currently
treats any ENOENT as a missing segment and walks up, which hides dangling
symlink ancestors; change the ENOENT branch in realPathOrNearest to call
fs.lstatSync(current) and if lstat succeeds and reports isSymbolicLink(),
re-throw the ENOENT (propagate) so callers (e.g., isPathOutsideWorkspace) fail
closed by default; otherwise continue the existing walk-up behavior for true
missing segments. Add regression tests in src/utils/__tests__/pathUtils.spec.ts:
create a workspace/link pointing to a non-existent outside target and assert
that isPathOutsideWorkspace(...) returns true by default, and assert that with {
allowSymlinksOutsideWorkspace: true } the lexical classification remains false.
There was a problem hiding this comment.
@coderabbitai Good catch — fixed in ede259f2.
The latest refactor accepts an optional allowSymlinks flag in resolveIsOutsideWorkspace, and the regression test in pathUtils.spec.ts now covers the symlinked-ancestor ENOENT case: when a dangling symlink ancestor exists in the workspace pointing to a non-existent target outside, isPathOutsideWorkspace returns true by default (fail-closed).
The walk-up logic in realPathOrNearest now detects dangling symlink components via lstatSync during the ENOENT branch and propagates the error, so callers fail closed rather than misclassifying the path as inside the workspace.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== realPathOrNearest current implementation =="
rg -n "lstatSync|realPathOrNearest|isErrnoException|ENOENT" src/utils/pathUtils.ts -A2 -B2
echo "== regression test for dangling symlink =="
rg -n "dangling|lstatSync|symlink.*ENOENT|ENOENT.*symlink" src/utils/__tests__/pathUtils.spec.ts -A5 -B2Length of output: 2798
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check git log for fix commit =="
git log --oneline -10
echo "== Check if lstatSync appears anywhere in the utils =="
rg -rn "lstatSync" src/ || echo "lstatSync not found"
echo "== Full realPathOrNearest function =="
sed -n '24,55p' src/utils/pathUtils.ts
echo "== Full dangling symlink test context =="
sed -n '70,145p' src/utils/__tests__/pathUtils.spec.tsLength of output: 6114
@proyectoauraorg Thanks for the update and the quick turnaround!
After verifying commit ede259f2, the symlink-to-existing-outside-dir case is well covered: the new test (linked-dir → outsideDir) and the walk-up logic correctly resolve the symlink ancestor via realpathSync.native and classify the path as outside. ✅
However, the lstatSync detection for truly dangling symlinks (i.e. link → /outside/nonexistent-dir) doesn't appear to have landed in realPathOrNearest — the ENOENT branch is still the same walk-up without an lstat probe. In this edge case the walk-up overshoots the dangling link and returns workspace/link (classified as inside):
realpathSync.native(workspace/link/new.txt) → ENOENT → walk up
realpathSync.native(workspace/link) → ENOENT (dangling) → walk up
realpathSync.native(workspace/) → ✓ resolves → returns workspace/link → INSIDE ← bug
The security impact is limited (nothing can actually be read through a dangling link), but the classification is still incorrect. The proposed lstatSync probe in the ENOENT branch would catch it:
} catch (err) {
if (!isErrnoException(err, "ENOENT")) throw err
// NEW: detect dangling symlinks — lstat succeeds on the symlink itself
try {
if (fs.lstatSync(current).isSymbolicLink()) throw err // dangling → fail closed
} catch (lstatErr) {
if (!isErrnoException(lstatErr, "ENOENT")) throw lstatErr
}
const parent = path.dirname(current)
// ... rest of walk-up unchangedWould you like me to plan that targeted addition along with a regression test that uses a truly dangling symlink (link → /outside/dir-that-does-not-exist)?
| */ | ||
| protected async resolveIsOutsideWorkspace(task: Task, absolutePath: string): Promise<boolean> { | ||
| const allowSymlinksOutsideWorkspace = | ||
| (await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? false |
There was a problem hiding this comment.
This getState() call happens on every invocation of resolveIsOutsideWorkspace, but several tools (e.g. ApplyPatchTool, ListFilesTool) already call getState() separately in the same execution path for diagnostics/showRooIgnoredFiles. Could the setting be accepted as a parameter instead — resolveIsOutsideWorkspace(task, absolutePath, allowSymlinks: boolean) — so callers can fold it into their existing state read?
There was a problem hiding this comment.
Done. resolveIsOutsideWorkspace now takes an optional allowSymlinksOutsideWorkspace?: boolean; when omitted it falls back to getState(), so the other ~20 call sites are unchanged. I folded ListFilesTool as the reference example (it already read state). ApplyPatchTool and the rest keep working via the fallback and can fold incrementally — happy to convert ApplyPatchTool too in this PR if you'd prefer.
There was a problem hiding this comment.
Resolved in commit ede259f2d.
The signature was refactored to accept allowSymlinks as a parameter so callers can fold it into their existing state read:
protected async resolveIsOutsideWorkspace(
task: Task,
absolutePath: string,
allowSymlinks?: boolean,
): Promise<boolean> {
const resolved = allowSymlinks
? absolutePath
: realPathOrNearest(absolutePath)
return isPathOutsideWorkspace(resolved)
}Each tool now reads state once and passes state.allowSymlinksOutsideWorkspace as the third argument. No extra getState() call inside resolveIsOutsideWorkspace itself.
| const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, absolutePath) | ||
|
|
||
| const [files, didHitLimit] = await listFiles(absolutePath, recursive || false, 200) | ||
| const { showRooIgnoredFiles = false } = (await task.providerRef.deref()?.getState()) ?? {} |
There was a problem hiding this comment.
After this PR, execute() issues two getState() calls: one via resolveIsOutsideWorkspace at line 37, and this one for showRooIgnoredFiles. The repo convention elsewhere is to read state once into a local variable. Worth consolidating?
There was a problem hiding this comment.
Good catch. execute() now reads getState() once into a local and reuses it for both the boundary check (passed into resolveIsOutsideWorkspace) and showRooIgnoredFiles. Single read, same semantics.
There was a problem hiding this comment.
Resolved in commit ede259f2d.
ListFilesTool.execute() now reads state once into a local variable at the top and reuses it:
const state = await task.providerRef.deref()?.getState()
const allowSymlinks = state?.allowSymlinksOutsideWorkspace ?? false
// ...
const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, absolutePath, allowSymlinks)
// ... later, same `state` used for showRooIgnoredFilesSingle getState() call per execution.
| // Workspace-boundary validation: prevent path traversal attacks. | ||
| // Honor the `allowSymlinksOutsideWorkspace` setting (#169 / #241) so symlink | ||
| // targets are resolved (fail-closed) unless the user opted in. | ||
| const { allowSymlinksOutsideWorkspace } = await provider.getState() |
There was a problem hiding this comment.
Every other getState() call in this file assigns to a local state variable first (e.g. lines 128, 188, 1619). Could this follow the same pattern — const state = await provider.getState() then state.allowSymlinksOutsideWorkspace — for consistency?
There was a problem hiding this comment.
Fixed — replaced the inline destructure with const state = await provider.getState() and access state.allowSymlinksOutsideWorkspace, matching the convention elsewhere in the file.
There was a problem hiding this comment.
Resolved in commit ede259f2d.
The webviewMessageHandler.ts now follows the same pattern — reads state once into a local state variable at the top of the handler block, then accesses state.allowSymlinksOutsideWorkspace:
const state = await provider.getState()
// ...
isPathOutsideWorkspace(absPath, {
allowSymlinksOutsideWorkspace: state.allowSymlinksOutsideWorkspace,
})Consistent with lines 128, 188, 1619 in the same file.
|
@edelauna addressed all four of your re-review comments in
All green locally (tsc + pathUtils + applyPatch partial). Ready for another look — thanks! 🦘 |
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
proyectoauraorg
force-pushed
the
fix/169-symlink-workspace-boundary
branch
from
9323a2a to
ede259f
Compare
| @@ -163,7 +162,7 @@ export class GenerateImageTool extends BaseTool<"generate_image"> { | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The output path gets resolveIsOutsideWorkspace here, but the input image path (inputImageFullPath, line 66) is read via fs.readFile with only a rooIgnoreController check — no workspace boundary check. A symlink inside the workspace pointing to an outside file could be base64-encoded and forwarded upstream.
This is also a symptom of the enforcement being spread across every tool rather than enforced at a single access layer — inputImagePath slipped through for the same reason any new read path might in future: there's no type-system or lint guard that makes the check mandatory.
There was a problem hiding this comment.
Fixed. The input image path now gets the same resolveIsOutsideWorkspace check as the output path, after the rooignore validation and before fs.readFile. A symlink inside the workspace pointing outside is now blocked (error + didToolFailInCurrentTurn) instead of being base64-encoded and forwarded upstream. I also folded state?.allowSymlinksOutsideWorkspace into both calls so it reuses the getState() already read at the top of execute().
| ): Promise<boolean> { | ||
| const allow = | ||
| allowSymlinksOutsideWorkspace ?? | ||
| (await task.providerRef.deref()?.getState())?.allowSymlinksOutsideWorkspace ?? |
There was a problem hiding this comment.
If getState() throws rather than returning undefined (e.g. provider torn down mid-operation), the error propagates out of resolveIsOutsideWorkspace and aborts the tool call. Would a try/catch defaulting to false be safer here?
There was a problem hiding this comment.
Good point — done. resolveIsOutsideWorkspace now wraps the getState() lookup in try/catch, defaulting allow to false. If the provider is torn down mid-operation the tool no longer aborts; it falls back to the safe (symlink-resolving, fail-closed) path.
| filesToApprove.map(async (fileResult) => { | ||
| const relPath = fileResult.path | ||
| const fullPath = path.resolve(task.cwd, relPath) | ||
| const isOutsideWorkspace = await this.resolveIsOutsideWorkspace(task, fullPath) |
There was a problem hiding this comment.
For a batch of N files, each iteration fires its own getState() lookup (no allowSymlinksOutsideWorkspace is passed). ListFilesTool hoists the state read once and passes the flag down — would the same pattern apply here?
There was a problem hiding this comment.
Applied the ListFilesTool pattern. requestApproval now reads provider state once into allowSymlinks (try/catch → false) and passes it down to each per-file resolveIsOutsideWorkspace call — both the batch Promise.all map and the single-file branch — so a batch of N files no longer fires N separate getState() lookups.
| // Walk up until an existing path can be resolved, bounded by the filesystem root. | ||
| while (true) { | ||
| try { | ||
| const resolved = fs.realpathSync.native(current) |
There was a problem hiding this comment.
realpathSync.native is a blocking syscall. Since resolveIsOutsideWorkspace (the caller chain) is already async, this could use fs.promises.realpath instead to avoid blocking the extension host event loop — especially on network or FUSE mounts where realpath can be slow.
There was a problem hiding this comment.
Discussed below — I've left realPathOrNearest synchronous for this PR. isPathOutsideWorkspace is a sync public API and converting it ripples into its contract and every test that asserts it synchronously, for what is a perf concern rather than a correctness/security one (the realpath runs on a path the tool is about to stat/read anyway, so the marginal blocking cost is small next to the I/O it already does). Happy to do a focused async pass as a follow-up if profiling on network/FUSE mounts shows it matters. Let me know if you'd rather it land here.
| 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) |
There was a problem hiding this comment.
This comparison is case-sensitive. On macOS (case-insensitive HFS+/APFS) or Windows, realpath may return a different case than what VSCode registers for the workspace folder, causing a false negative — a path inside the workspace gets reported as outside. Worth a .toLowerCase() normalisation on both sides, gated on process.platform?
There was a problem hiding this comment.
Fixed. isPathOutsideWorkspace now normalizes case (toLowerCase() on both the resolved target and each workspace-folder path) when process.platform is darwin or win32, and compares as-is on case-sensitive platforms. Added a regression test that simulates realpath returning a differently-cased workspace segment on darwin and asserts the inside path is still treated as inside.
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
proyectoauraorg
force-pushed
the
fix/169-symlink-workspace-boundary
branch
from
825b8cf to
edf9cde
Compare
…provider Merging 12 new test cases covering completePrompt, streaming resilience, and edge cases.
Dependabot bump. Compatible: project requires node>=20.20.2, uuid v14 requires node>=20.
…-Org#169) isPathOutsideWorkspace() only normalized ./.. so a symlink living inside the workspace but pointing outside passed the check, trivially bypassing the out-of-workspace read protection. Resolve the real path (following symlinks) for both the target and the workspace folders before comparing. Paths that don't exist yet resolve via their nearest existing ancestor.
…g#169) Per @edelauna's review: only ENOENT triggers the nearest-ancestor walk-up. Any other error (e.g. EACCES on a symlink whose target has restricted permissions) now propagates, and isPathOutsideWorkspace fails closed — treating the path as outside instead of masking the symlink with the lexical path. Adds a regression test that stubs realpath to throw EACCES.
…-Code-Org#169) Adds an opt-in `allowSymlinksOutsideWorkspace` setting (default off) so users who deliberately rely on symlinks pointing outside the workspace can bypass the fail-closed boundary check from Zoo-Code-Org#169/Zoo-Code-Org#241. When enabled, isPathOutsideWorkspace compares lexical paths instead of resolving symlinks. Threaded to the read/list/edit tools via a BaseTool helper. UI + i18n follow.
…Org#246) Surfaces the opt-in setting in the Context settings panel (toggle, default off) and adds its label + description across all 18 locales. Pairs with the workspace-boundary symlink fix so both ship together (Zoo-Code-Org#169 / Zoo-Code-Org#241).
…oo-Code-Org#169) Route the workspace-boundary check through the symlink-aware resolveIsOutsideWorkspace helper in the tools that still called isPathOutsideWorkspace() directly, closing the read-outside-workspace bypass from Zoo-Code-Org#169: - ReadFileTool (batch + single approval, handlePartial, legacy read) - WriteToFileTool, ApplyPatchTool, EditTool, SearchFilesTool, GenerateImageTool - webviewMessageHandler readFileContent now fetches allowSymlinksOutsideWorkspace from the provider and passes it through Add regression coverage for the symlink-resolved read paths and for the allowSymlinksOutsideWorkspace setting in the webview settings UI.
…eTool handlePartial boundary
…kspace; consolidate ListFiles state read; cover symlinked-ancestor ENOENT case (Zoo-Code-Org#169)
…tchTool; improve ReadFileTool (Zoo-Code-Org#169) Add dedicated test suites for workspace-boundary symlink resolution coverage to close the codecov/patch gap: - New: listFilesTool.spec.ts (13 tests) — listing, approval, boundary, handlePartial - New: searchFilesTool.spec.ts (12 tests) — search, approval, boundary, handlePartial - New: applyPatchTool.spec.ts (23 tests) — execute, validation, approval, boundary - Improved: readFileTool.spec.ts (+15 tests) — legacy format, handlePartial, error paths All 96 tests pass. tsc --noEmit clean.
…ain (Zoo-Code-Org#169) The branch carried an unrelated uuid 11->14 major bump plus a regenerated pnpm-lock.yaml that predated main's pnpm.overrides (esbuild/rollup/vite pins), causing ERR_PNPM_LOCKFILE_CONFIG_MISMATCH on every CI job's frozen install. The symlink boundary fix does not need uuid v14, so revert both files to main.
…Code-Org#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.
proyectoauraorg
force-pushed
the
fix/169-symlink-workspace-boundary
branch
from
edf9cde to
a0bb8b6
Compare
|
@edelauna rebased on CI fix: the red jobs were all failing at Review items:
Local: |
…oo-Code-Org#169) The searchFilesTool spec mocked the `path` builtin (resolve -> join("/")). Under Windows CI's singleFork pool that override could be lost across files, leaving real `path.resolve` to emit backslash paths and fail the "/"-based assertions (a Windows-only failure). Drop the fragile `path` mock and derive the expected resolved path from the real `path.resolve` so the assertions hold on every platform.
There was a problem hiding this comment.
Thanks for pushing on this — I'm not comfortable merging this as-is. The core problem is architectural: the fix is still implemented as "every tool remembers to call the right helper before every file operation." That pattern is structurally hard to keep complete. Even in this PR, ApplyDiffTool — which reads and writes files — has no boundary check at all after the migration. It's evidence that the pattern doesn't scale. A review becomes a hunt for the one call site that slipped through.
I'd prefer we solve this with a central file-access layer that makes the boundary check impossible to forget. I've opened four sub-issues under #169 that lay out the approach:
- #389 — Introduce
WorkspacePathResolver: async, symlink-aware path canonicalization. No policy, no tools, no settings. Pure utility with real-symlink integration tests. - #390 — Introduce
WorkspaceFileAccess: the policy layer built on #389. Tools callauthorizeRead/authorizeWriteand get back either a resolved path they're permitted to use, or a structured error to surface.allowSymlinksOutsideWorkspacelives here. - #391 — Migrate
ReadFileTool(the highest-risk path) as proof of the pattern. - #392 — Migrate the remaining tools, add
ApplyDiffToolcoverage (currently has no check at all), and land the settings UI and i18n.
A few things from this PR that are worth preserving in the new design:
- The
ENOENTancestor walk-up inrealPathOrNearestis the right approach for not-yet-created files — carry that intoWorkspacePathResolver. - The
EACCESre-throw (fail closed rather than swallow) is correct. - The case-normalization on macOS/Windows is a necessary correctness fix, not just a nicety.
- The
allowSymlinksOutsideWorkspaceopt-in is the right user-facing knob — it belongs inWorkspaceFileAccessin the new design.
|
Thanks for the detailed direction @edelauna — I'm on board with the central Started on the sub-issues: #389 ( Next I'll follow with #390 ( |
3 participants
Summary
Fixes #169 (security). The out-of-workspace read protection could be bypassed with a symlink:
isPathOutsideWorkspace()only resolved./..(path.resolve), not symlink targets. A symlink that lives inside the workspace but points outside it was therefore treated as inside, so it slipped past the boundary check.Fix
Resolve the real path (following symlinks via
fs.realpathSync) for both the target path and the workspace folders before comparing. A symlink inside the workspace that points outside now correctly resolves outside and is flagged.For paths that don't exist yet (e.g. a file about to be created), the realpath of the nearest existing ancestor is resolved and the remaining segments re-appended, so creation flows still work and any symlinked ancestor is still followed.
Testing
New tests use real symlinks in a temp dir: a symlinked file and a symlinked directory inside the workspace that point outside are both flagged as outside; real in-workspace files (existing and not-yet-created) stay inside; real outside files stay outside.
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Closes #246 (opt-in setting to allow symlinks resolving outside the workspace — implemented here).