Skip to content

feat(security): migrate remaining tools to WorkspaceFileAccess — write, edit, list, search, image (#169) #392

@edelauna

Description

@edelauna

Part of #169. Sub-issue 4 of 4. Depends on #391.

Context

After ReadFileTool is migrated (#391), the remaining tools still use the old isPathOutsideWorkspace / resolveIsOutsideWorkspace pattern. These are lower urgency than read (write tools already gated by explicit user approval flows) but must be migrated to close #169 fully and to make WorkspaceFileAccess the only call site for boundary enforcement.

Tools to migrate in this PR:

Also: add the allowSymlinksOutsideWorkspace opt-in setting to the settings UI and i18n (deferred from earlier PRs). Wire the global setting through to WorkspaceFileAccess as the default when provider state is unavailable. This closes #246 as well.

Developer Note

For each tool, the migration is the same pattern established in #391:

  1. Replace isPathOutsideWorkspace / resolveIsOutsideWorkspace with workspaceFileAccess.authorizeRead or authorizeWrite as appropriate.
  2. On !access.ok, surface access.message as a tool error and return before any fs operation.
  3. Use access.resolvedPath for the actual filesystem call.
  4. Hoist getState() once per execute() invocation and pass the allowSymlinksOutsideWorkspace flag down — do not fire a separate getState() per file in loops.

ApplyDiffTool special note: this tool currently has zero boundary check. Add authorizeRead before the file read in the UpdateFile/DeleteFile hunk handler, and authorizeWrite before the file write. The read-before-check race (content read before boundary check fires) identified in review is resolved naturally by using access.resolvedPath — the authorization must succeed before any fs call.

ListFilesTool / SearchFilesTool: authorizeRead on the root directory is sufficient for the tool-level check. The ripgrep --follow flag causing enumeration of out-of-workspace symlink targets is a separate issue and out of scope here.

Settings UI (closes #246):

  • Add allowSymlinksOutsideWorkspace checkbox to ContextManagementSettings.tsx under the workspace boundary section.
  • Add i18n keys to all locale files (English first, machine-translate others or leave as English fallback).
  • Wire through global-settings.ts and vscode-extension-host.ts.

Cleanup:

  • Remove resolveIsOutsideWorkspace from BaseTool.ts — no longer needed.
  • Remove any remaining direct imports of isPathOutsideWorkspace from tool files.
  • isPathOutsideWorkspace in pathUtils.ts can be deprecated or removed once no callers remain; keep it if WorkspaceFileAccess delegates to it internally.

Tests: Each migrated tool should have at least one test verifying that a symlink-pointing-outside path is blocked, using the real WorkspaceFileAccess (not a mock of the boundary check).

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