Repository navigation
Conversation
A relative link joined onto the workspace root kept its .. segments, so the prefix check treated it as a workspace file and the server rejected it. Collapse . and .. lexically on the path, after ~/ expansion and before that check, while keeping any line or column suffix.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }; | ||
| } | ||
|
|
||
| if (path.startsWith("/")) { |
There was a problem hiding this comment.
🟠 High src/terminal-links.ts:304
For a forward-slash UNC link such as //server/share/dir/../file.md, this branch treats // as a POSIX root and returns /server/share/file.md; on Windows, the viewer therefore targets the local drive path instead of the network share. Recognize the //server/share prefix as a UNC root before the POSIX-root branch so collapsing preserves both leading separators.
+ const forwardSlashUnc = /^\/\/[^/]+\/[^/]+/.exec(path);
+ if (forwardSlashUnc?.[0]) {
+ return {
+ prefix: forwardSlashUnc[0],
+ separator: "/",
+ rooted: true,
+ segments: path.slice(forwardSlashUnc[0].length).split(/[\\/]/),
+ trailingSeparator,
+ };
+ }
+
if (path.startsWith("/")) {🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/terminal-links.ts around line 304:
For a forward-slash UNC link such as `//server/share/dir/../file.md`, this branch treats `//` as a POSIX root and returns `/server/share/file.md`; on Windows, the viewer therefore targets the local drive path instead of the network share. Recognize the `//server/share` prefix as a UNC root before the POSIX-root branch so collapsing preserves both leading separators.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes shared file-link resolution across terminal, markdown, preview, Git, and file-manager flows by adding substantial cross-platform path normalization. An unresolved High-severity finding shows that forward-slash UNC paths can still be rewritten to local POSIX paths, requiring human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughPath resolution now collapses ChangesFile Link Path Resolution
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to File links appear to resolve as intended, with no confirmed issue requiring a fix before merge. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to Sibling-file links now reach the same authorized, read-only file-opening behavior already available through absolute links. The reviewed change does not grant additional filesystem permissions or weaken workspace-relative path checks. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Superseded; redoing from scratch. |
1 participant
What Changed
resolvePathLinkTargetnow collapses.and..lexically on the path only, after~/expansion and before the workspace-membership check. A:lineor:line:columnsuffix is preserved. Absolute markdown links go through the same collapse, so/repo/../other/notes.mdis not classified as a file inside/repo.An in-workspace link such as
docs/guide/../readme.mdstays a workspace file...does not climb above a POSIX root, a Windows drive root, or a UNC share. The server still rejects a relative path that escapes the workspace.apps/mobileis unchanged.Why
A markdown or inline link like
[notes.md](../other/notes.md)was joined onto the workspace root and left..in the string.workspaceRelativeFilePathonly checks that the string starts with the root, so/home/me/project/../other/notes.mdbecame the workspace-relative path../other/notes.md. The server rejects that path, and the file viewer fails instead of opening the sibling file read-only the way an absolute or~/link already does.Fixes #16354
Verification
Base:
upstream/main0f4f02abb6ab5428c06a7e4b6c76b3ec84c62498(merges cleanly with currentmain)Head:
b40dfe4099d4a4da61d0ca9d354673df6f2b7641Platform: Linux 6.12.94+ x86_64, Node v24.13.1 (Vite+ runtime and capture server on Node v24.21.0).
vp test run apps/web/src/terminal-links.test.ts apps/web/src/markdown-links.test.ts— 2 files passed, 94 tests passed.vp run --filter @t3tools/web typecheck— exit 0 (tsc --noEmit).vp run --filter @t3tools/web test— 457 files passed, 6146 tests passed.vp check— 0 errors and 893 warnings in 4848 files.vp run knip:check— exit 0.Manual check: a throwaway repo at
/tmp/fx/projectcontainslinks.mdwith[notes.md](../other/notes.md)and[readme.md](docs/guide/../readme.md). The target file is/tmp/fx/other/notes.md, outside the project.links.mdwas opened in the Files panel's rendered markdown preview, thenotes.mdchip was hovered, then clicked.On
main, the tooltip is/tmp/fx/project/../other/notes.mdand the breadcrumbs areproject > .. > other > notes.md. The click does not open the file; the panel shows "The requested path is outside the workspace" for workspace/tmp/fx/project.On this branch, the tooltip is
/tmp/fx/other/notes.mdand the breadcrumbs aretmp > fx > other > notes.md. The click opens the sibling file read-only and showsSIBLING_NOTES_OUTSIDE_WORKSPACE.Limitations: the collapse is lexical and does not follow symlinks. The server still rejects a relative path that escapes the workspace. Mobile's resolver was not changed.
UI Changes
Checklist