[JahyunBaek]

ELI5

When you open a file from outside your projects — say, a path printed in the terminal — Orca first gives itself permission for that one file, then checks that permission before it reads the file. On Windows, a mapped network drive (X: from net use) or a subst drive has two names for the same file: X:\docs\page-1.png, and the real location behind it. The permission was written down under the first name and checked under the second, so the check never found it and the file never opened. Now the permission is written down under both names.

What Changed

Before: on a mapped or subst drive, clicking a terminal file link shows the action popover, but Open file does nothing — no editor tab, no message. The read behind it fails with Access denied: path resolves outside allowed directories.

After: the file opens.

Mechanism. authorizeExternalPath (src/main/ipc/filesystem-auth.ts) remembers the path as given and its fs.realpathSync result. Every later read goes through resolveAuthorizedPath, which canonicalizes with the native fs.promises.realpath. For most paths the two agree. For a mapped or subst drive they don't: Node's default realpathSync is a JavaScript implementation that walks the path and keeps the drive letter, while the native call asks Windows for the final path and gets the UNC share or the subst target. The grant now also remembers realpathSync.native — the form the check computes — inside the existing try, right after the JavaScript realpath.

Why

• Nothing new becomes reachable. The extra entry is the canonical name of the file or folder that was just granted. Every path allowed before is still allowed, and a sibling of a granted file stays denied (covered by a test, and checked on a real drive).
• One place, every caller. Terminal links, editor opens, drops and markdown links all grant through authorizeExternalPath, so the fix applies to all of them without touching each read.
• Same try on purpose. The grant runs synchronously on the main thread. If the JavaScript realpath has already failed — a missing file or an unreachable share — the native one would fail the same way, so it is not attempted, and a disconnected drive is not waited on twice.

Alternatives considered:

• Make the check use the JavaScript realpath. The rest of this module canonicalizes with the native realpath too (normalizeExistingPath, the parent and missing-path checks), so this would move the mismatch rather than remove it.
• Replace the JavaScript realpath with the native one. A smaller diff, but it drops an entry existing callers get today. Adding is strictly safer than replacing.

Linked Issue

Fixes #23018

Visual Proof

Windows 11, the e2e harness (electron-headless, background launch), a subst Q: drive holding docs\page-1.png. The terminal prints Q:\docs\page-1.png; the link is clicked and Open file chosen.

The popover, the same before and after:

Before (main)	After (this PR)

Testing

• New src/main/ipc/filesystem-auth-external-grant-native-realpath.test.ts. It mocks only the two realpath calls to model a drive the native realpath resolves elsewhere, so it runs on every platform. Against main, the two grant cases fail with the issue's exact error; on this branch all four pass. The no-second-wait case fails if the native call is given its own try, and the sibling case passes either way — it guards against widening.

• Real drive, no mocks (Windows 11). authorizeExternalPath then resolveAuthorizedPath on a subst drive: a file grant and a folder grant are denied on main and allowed here; an ungranted sibling is denied on both.

• Realpath forms (Windows 11). The new entry equals what the check computes for a subst drive, 8.3 short names, lower-cased paths, WSL UNC paths (\\wsl.localhost\…, \\wsl$\…, where nothing changes) and missing files (both throw).

• End to end (Windows 11). The Visual Proof run above, from a temporary spec that is not part of this PR: on main nothing opens and no toast appears; here page-1.png opens in the editor.

• Regression. 89 test files — everything importing filesystem-auth, everything under src/main/ipc/filesystem-*, and the tests of modules that call authorizeExternalPath — run with and without the change. None fails because of it: 26 fail identically on unmodified main on this Windows machine (13 file-watcher timing cases; the rest are symlink, Unicode-normalization, POSIX-path, WSL rg and OS-opened-markdown delivery cases). Two more failed only in the with-change batch; run alone, one fails the same way on main, and the other — a real file-watcher test that never reaches filesystem-auth — passed 5 of 5.

• CI static analysis, run locally. The PR Checks static-analysis steps that apply to this change, the two it routes in (verify:localization-extraction, smoke:orcad-terminal), and pnpm typecheck — 25 in all: 23 pass. The other two fail the same way on unmodified main on this machine. verify:skill-bundle-manifest compares the CRLF resources/skills/*.json of a core.autocrlf=true checkout byte for byte ([Bug]: pnpm lint cannot pass on a stock Windows checkout (CRLF line endings) #14479). smoke:orcad-terminal passes its terminal round trip, then hits EPERM deleting its temp folder right after stopping the server — a Windows cleanup race in the script.

• Not tested: a real net use network share (a subst drive reproduces the same JavaScript/native split); macOS and Linux by hand (CI runs the unit test).

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

AI Disclosure

Written with Claude Opus 5.5 (Claude Code). I reproduced the bug on my Windows machine before the fix, reviewed the change, and ran every check above locally.

Review

The judgement call is where to fix it: at the grant (this PR) or at the check. I chose the grant because the native canonical form the check uses is what the rest of the module already compares against.

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

AI code review summary (Claude Opus 5.5):

• Cross-platform — on macOS and Linux the native form normally equals the JavaScript one and nothing changes; where it differs, it is again the form the check already uses. On Windows, checked against subst drives, 8.3 short names, casing and WSL paths (see Testing).
• SSH / remote / local — terminal links on SSH and runtime paths skip the local grant (terminal-file-open-routing.ts: the relay or runtime is the security boundary). The change is confined to the local main-process grant. WSL \\wsl.localhost\… and \\wsl$\… paths give the same form both ways and are unaffected. Folder workspaces and git worktrees are not involved — the grant is per path.
• Agents and integrations — no agent, integration or git-provider code is touched. Paths printed by any agent CLI go through the same grant, so they all benefit the same way.
• Performance — one extra realpath per grant, and only after the JavaScript one succeeded, so a missing file or unreachable share costs nothing extra. The grant set stays bounded at 4,096 entries; on most paths the two forms are the same string and nothing is added.
• UI quality — no UI code changed. The existing popover and editor flow now completes instead of stopping silently (see Visual Proof).
• Security — no widening: the added entry names the object that was granted, and an ungranted sibling stays denied (unit test and real drive). Who may call authorizeExternalPath is unchanged.
• Mobile / backwards compatibility — no wire, schema or IPC change.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

🤖 Generated with Claude Code

https://claude.ai/code/session_01ADAPnmpPBMctKbwkj2i9NS

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds Windows mapped drive support to file access checks.

The PR appears safe to merge, with non-blocking concerns about grant eviction and main-thread latency on mapped drives.

Findings

1.  Aliases evict grants sooner ▶
2.  Native resolution blocks main thread ▶

Summary

The PR records the native canonical path alongside existing external-file grants so mapped and subst drive paths can pass the subsequent authorization check.

• Adds platform-independent tests for file and folder grants, a failed initial resolution, and sibling denial.
• The extra alias increases grant-set turnover, and resolving it synchronously can delay interactive main-process work.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["External path grant: X:\file"] --> B["Remember drive-letter path"]
  A --> C["JavaScript realpath"]
  C --> D["Remember JavaScript path"]
  C --> E["Synchronous native realpath"]
  E --> F["Remember canonical target"]
  G["Later read"] --> H["Canonicalize and check remembered paths"]
  F --> H
  B --> H
  D --> H

Loading

_{Reviews (1) · Last reviewed commit: "test(filesystem): model the mapped drive..."}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Native-realpath grant on Windows mapped/subst drives — authorizeExternalPath now also records resolve(realpathSync.native(resolvedTarget)) inside the existing try, so the grant matches the native canonical form that resolveAuthorizedPath compares against.
• New regression test (filesystem-auth-external-grant-native-realpath.test.ts) — models a JS/native realpath split with the shared isDescendantOrEqual helper, covering file grant, folder grant, the shared-try skip, and non-widening to siblings.

I verified the premise that the read check uses a native realpath (fs.promises.realpath calls the binding.realpath binding in lib/internal/fs/promises.js), confirmed the test passes on the branch and fails with the exact Access denied error when the two added lines are reverted (2 of 4 cases), and checked the LRU-bound test still holds since nonexistent flood paths throw before the native call. The added entry is the canonical name of the just-granted object, so reachability does not widen.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Repository guideline files applied to this review (1)

AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 72c903d9-6c22-4936-a19f-4b9a612e1ee5

📥 Commits

Reviewing files that changed from the base of the PR and between 3ab3c92 and 6b1a77e.

📒 Files selected for processing (2)

• src/main/ipc/filesystem-auth-external-grant-native-realpath.test.ts
• src/main/ipc/filesystem-auth.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

authorizeExternalPath now records the native realpath of a target in addition to its JavaScript realpath. Tests cover mapped-drive file and folder grants, JavaScript realpath failure, and access to a sibling file.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6b1a7

The change addresses mapped-drive file access while preserving the existing authorization boundaries. No actionable merge-blocking risk is identified; normal checks should complete before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6b1a7

The change aligns file permissions with Windows canonical paths while preserving path-scope checks. No new authorization bypass was demonstrated. Drive-remapping behavior and the trust placed in grant callers remain incompletely established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Under stable mappings, the additional authorization covers the backing canonical name of the granted file or folder descendants. Its effects extend beyond opening files because local write and delete handlers consume the same authorization helper; exposure remains constrained by path checks and the filesystem permissions of the application process.

Trust Boundaries and Controls

• observed — The existing bridge allows renderer-supplied paths to reach the main-process grant handler, which does not visibly validate the sender or require a separate approval token in this block. This authority predates the PR; no differential caller exposure was found. Validation elsewhere in the application was not established.

Resilience and Maintainability Implications

• inferred — The base already used process-local path-string grants without caller or filesystem-object identity metadata. Head inherits that lifecycle and adds a native alias. The final canonical check limits ordinary remapping to an unrelated unauthorized target, but it does not establish invalidation of previously recorded aliases or an object-identity guarantee. Those policy gaps do not establish a new bypass.

Hardening Proposals

• proposed — Specify whether external grants authorize path aliases or durable filesystem identities, then validate drive remapping and native-resolution failure against that policy before changing grant lifecycle semantics.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: enabling file opening on Windows mapped and subst drives.
Description check	✅ Passed	The description is complete and follows the repository template. It explains the user impact, mechanism, rationale, linked issue, visual proof, testing, AI disclosure, review considerations, cross-pla…
Linked Issues check	✅ Passed	Issue [#23018] requires mapped-drive paths to pass the external authorization check, or for the failure to be surfaced. The PR records both the JavaScript realpath and `resolve(realpathSync.native(res…
Out of Scope Changes check	✅ Passed	The implementation change and the added test file directly support issue [#23018]. The tests verify the mapped-drive authorization behavior and preserve access isolation. No unrelated production behav…

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[JahyunBaek]

Superseded by #24489, which removed the grant list this PR patched; the reporter confirmed #23018 fixed on v1.4.224. Closing.