Skip to content

feat(security): migrate ReadFileTool and webview readFileContent to WorkspaceFileAccess (#169) #391

@edelauna

Description

@edelauna

Part of #169. Sub-issue 3 of 4. Depends on #390.

Context

ReadFileTool is the highest-risk path for the symlink escape identified in #169: it reads arbitrary file content and returns it directly to the LLM. It is also the most complex tool to migrate (batch reads, line ranges, streaming partial display) so proving the pattern here gives high confidence before migrating write/edit tools.

The webview readFileContent handler in webviewMessageHandler.ts is included in this PR because it shares the same risk profile and is small enough to bundle without inflating scope.

PR #241 can be used as a reference for the call sites to migrate — it correctly identified all the isPathOutsideWorkspace usages in this tool. The difference here is that instead of calling this.resolveIsOutsideWorkspace(...) and then proceeding with raw fs, the tool calls workspaceFileAccess.authorizeRead(...) and uses access.resolvedPath for the actual read.

Developer Note

Migrate src/core/tools/ReadFileTool.ts:

  • Replace every call to isPathOutsideWorkspace / resolveIsOutsideWorkspace with workspaceFileAccess.authorizeRead({ task, requestedPath, source: "read_file" }).
  • On !access.ok, surface access.message as a tool error and return early — before any fs read.
  • Use access.resolvedPath (the canonicalized path) for the actual fs.promises.readFile call, not the raw input path.
  • The batch approval path (filesToApprove.length > 1) should call authorizeRead per file inside the Promise.all, passing the pre-fetched provider state via an optional parameter to avoid N getState() calls.

Migrate src/core/webview/webviewMessageHandler.ts (the readFileContent message handler):

  • Same pattern: authorizeRead before the fs read, early return with { content: null, error: access.message } on failure.

Remove the allowSymlinksOutsideWorkspace field from ExtensionState / globalSettingsSchema only if no other tool still references it — it will likely need to stay until sub-issue 4 completes.

Tests:

  • Normal in-workspace read: content returned correctly.
  • Path outside workspace: tool error surfaced, no fs call made.
  • Symlink inside workspace pointing outside: blocked (uses WorkspaceFileAccess real implementation, not a mock of the boundary check itself).
  • allowSymlinksOutsideWorkspace: true: symlink read permitted.
  • Batch read with mixed inside/outside paths: outside paths blocked individually, inside paths proceed.
  • Webview handler: outside path returns { content: null, error: "…" }.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions