feat(security): introduce WorkspaceFileAccess — central workspace boundary authorization (#390)#436
Conversation
… path canonicalization (Zoo-Code-Org#389) First of four sub-issues under Zoo-Code-Org#169. Adds `src/utils/WorkspacePathResolver.ts` with a single async `resolveRealPath(target)` that canonicalizes a path by following symlinks via `fs.promises.realpath`: - Walks up to the nearest existing ancestor on ENOENT and re-appends the trailing segments, so not-yet-created files under a symlinked ancestor still resolve correctly. - Re-throws non-ENOENT errors (EACCES, ELOOP) so callers can fail closed rather than fall back to a lexical path. - Case-normalizes the result on macOS/Windows for reliable comparison against uri.fsPath. Pure utility — no workspace policy, settings, or tool changes (those land in WorkspaceFileAccess (Zoo-Code-Org#390) and the migrations in Zoo-Code-Org#391/Zoo-Code-Org#392). Covered by integration tests using real symlinks in a temp directory.
…ndary authorization (Zoo-Code-Org#390) Sub-issue 2 of Zoo-Code-Org#169, building on WorkspacePathResolver (Zoo-Code-Org#389/Zoo-Code-Org#428). Adds src/core/workspace/WorkspaceFileAccess.ts. `authorizeRead`/`authorizeWrite` return either `{ ok: true, resolvedPath }` or a structured `{ ok: false, reason }` (outside_workspace | symlink_escapes_workspace | realpath_failed | permission_denied), so tools can no longer perform a boolean check and then act on a stale path — the missed `ApplyDiffTool` check in Zoo-Code-Org#241 is exactly the failure mode this prevents. Default behavior canonicalizes the requested path and every workspace folder via `resolveRealPath` and fails closed; the `allowSymlinksOutsideWorkspace` opt-in restores the pre-Zoo-Code-Org#169 lexical behavior. The setting is introduced here (its home) in GlobalSettings and ExtensionState. No tool migrations, UI, or i18n in this PR.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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! |
Part of #169. Sub-issue 2 of 4. Depends on #428 (#389
WorkspacePathResolver).Note
Stacked on #428. This branch is built on top of
feat/389-workspace-path-resolver, so the diff currently includes theWorkspacePathResolvercommit. Once #428 merges tomain, I'll rebase and that commit drops out, leaving only theWorkspaceFileAccesschange. Draft until #428 lands.What
Adds
src/core/workspace/WorkspaceFileAccess.ts— the policy layer #390 calls for. Tools request an authorized operation instead of doing a rawisPathOutsideWorkspace()boolean check followed by their ownfsaccess. The decoupled check-then-act pattern is structurally easy to get wrong; the missedApplyDiffToolcheck in #241 is exactly the failure mode this prevents.resolveRealPath(feat(security): introduce WorkspacePathResolver — safe async symlink-aware path canonicalization #389); grants access only when the real path lands inside a real folder. A folder that can't be resolved is skipped (can't prove containment), never treated as a match.allowSymlinksOutsideWorkspacerestores the pre-[BUG] vscode out-of-workspace read protection trivially circumvented by symlinks #169 lexical behavior (symlinks not followed for the boundary decision).allowSymlinksOutsideWorkspaceis read fromtask.providerRef.deref()?.getState()with a try/catch — provider may be torn down — defaulting tofalse.symlink_escapes_workspaceis distinguished fromoutside_workspaceso callers/telemetry can tell a deliberate out-of-tree path apart from one that looks inside but escapes via a symlink.The
allowSymlinksOutsideWorkspacesetting is introduced here (its home per the issue) inGlobalSettings+ExtensionState.Scope
Per the issue: does not change any tool behavior. No UI, no i18n, no tool migrations — those are #391/#392.
Tests
src/core/workspace/__tests__/WorkspaceFileAccess.spec.ts— real temp dirs + real symlinks (nofsmocking), covering all six scenarios from the issue plus plainly-outside, provider-throws, and no-workspace-open. Skips symlink/EACCES cases cleanly on Windows/root.tsc --noEmitcleaneslint --max-warnings=0clean (src + types)