fix: canonicalize symlinks (including dangling links) in workspace boundary check#2787
Open
laileni-aws wants to merge 4 commits into
Open
fix: canonicalize symlinks (including dangling links) in workspace boundary check#2787laileni-aws wants to merge 4 commits into
laileni-aws wants to merge 4 commits into
Conversation
…undary check The workspace-boundary acceptance check (requiresPathAcceptance) canonicalized paths with fs.realpath and, for paths that do not exist yet, fell back to realpath(parent) + basename. That fallback returned the in-workspace link name for a symlink whose target does not exist yet (a "dangling" symlink), so such a path was treated as in-workspace and a create through it (e.g. fsWrite create) could land outside the workspace without prompting for approval. Add resolveSymlinkAwarePath, which follows a symlink at the leaf even when its target does not exist, resolves symlinked ancestor directories and symlink chains (with a cycle guard), and re-appends not-yet-created segments so the result reflects where a read/write would actually land. Also canonicalize workspace folders before the containment comparison so a workspace located under a symlinked directory (e.g. macOS /tmp -> /private/tmp) does not produce false prompts. Add OS-agnostic real-filesystem tests covering dangling leaf symlinks, symlinked ancestors, symlink chains, the existing-target regression, and the symlinked-workspace-root case. Also add hardening TODOs to grepSearch and lspApplyWorkspaceEdit (both currently not enabled) to route their boundary checks through the same symlink-aware helper before they are enabled.
The Windows CI runner's os.tmpdir() returns an 8.3 short path (e.g. C:\Users\RUNNER~1\...), while fs.realpath inside resolveSymlinkAwarePath expands it to the long form (C:\Users\runneradmin\...). The expected values were built with path.join (string-only), so they kept the short name and mismatched the realpath-resolved actual values. Re-read the workspace and outside directories through fs.realpathSync after creating them so both sides of the assertions are canonical on all platforms.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2787 +/- ##
==========================================
- Coverage 60.20% 59.73% -0.47%
==========================================
Files 281 281
Lines 71191 71269 +78
Branches 4575 4517 -58
==========================================
- Hits 42861 42573 -288
- Misses 28241 28608 +367
+ Partials 89 88 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
….realpath The previous attempt canonicalized the test's expected paths with fs.realpathSync, but on Windows fs.realpathSync does not expand 8.3 short names (e.g. RUNNER~1) while fs.promises.realpath (used by the production resolveSymlinkAwarePath) does (runneradmin), so the assertions still mismatched by short-vs-long form. Add a `canon` helper that normalizes a path through fs.promises.realpath (the same call the production guard uses), resolving the deepest existing ancestor for not-yet-created paths, and apply it to BOTH the actual and expected values in every path-equality assertion. This makes the assertions independent of which 8.3/long form the platform surfaces. The production code is unchanged.
Two existing requiresPathAcceptance tests asserted that isInWorkspace was called with the raw workspace-folder strings and path.resolve(filePath). The symlink-aware canonicalization now passes filesystem-canonicalized folders and a symlink-resolved path, which on Windows differ from the raw POSIX-style test fixtures (path.resolve adds a drive letter), so the exact-arg match failed on the Windows runner while passing on Linux. Replace the exact calledWith(...) checks with platform-agnostic assertions: isInWorkspace was consulted, and with an absolute resolved path. The exact canonical form is covered by the real-filesystem tests in symlinkBoundary.test.ts. The behavioral assertions (requiresAcceptance true/false) are unchanged.
ashishrp-aws
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens the agentic-chat workspace-boundary check so a symlink whose target does not exist yet (a "dangling" symlink) cannot be used to write outside the opened workspace without prompting for approval.
Background
requiresPathAcceptance(shared byfsWrite,fsRead,fsReplace,listDirectory,fileSearch) canonicalized paths withfs.realpath, and for paths that do not exist yet it fell back torealpath(parent) + basename. For a dangling symlink that fallback returned the in-workspace link name, so the path was treated as in-workspace and a create through it (e.g.fsWrite create) could land outside the workspace with no approval prompt. A symlink to an existing outside target was already handled; only the not-yet-existing (dangling) target slipped through.Change
resolveSymlinkAwarePath: follows a symlink at the leaf even when its target does not exist, resolves symlinked ancestor directories and symlink chains (with a cycle guard), and re-appends not-yet-created segments, so the result reflects where a read/write would actually land./tmp -> /private/tmp) does not produce false prompts.requiresPathAcceptancenow uses both, fixing the write path and hardening every read tool that shares the check.grepSearchandlspApplyWorkspaceEdit(both currently not enabled) to route their boundary checks through the same helper before they are enabled.Tests
New OS-agnostic real-filesystem tests (
symlinkBoundary.test.ts) covering: dangling leaf symlink, nonexistent leaf under a symlinked ancestor, symlink chains, the existing-target regression (still prompts), a normal new in-workspace file (no prompt), and a workspace located under a symlinked directory (no false prompt). Tests skip gracefully where the platform/user cannot create symlinks.Verification
tsc --build: cleanBefore Fix:
After Fix: