Skip to content

Name the rejected path when a workspace-root confinement check fails #4293

Description

@atomantic

Problem

Four routes confine a caller-supplied filesystem path with isWithinAllowedRoots() and all four reject it with a message that names no path:

  • server/routes/git.js:34path is outside allowed directories (403)
  • server/routes/commands.js:43workspacePath is outside allowed directories (400)
  • server/routes/scaffold.js:261 — same class of rejection
  • server/routes/detect.js:70valid:false with Path is outside the configured workspace roots (PORTOS_WORKSPACE_ROOTS)

The server log line that reaches the user is therefore just:

❌ Route error [GET /api/git/submodules/status]: path is outside allowed directories

Which path? Which root did it miss? Diagnosing the recent Windows secondary-drive bug took a code read because the log named neither. The same guesswork applies to the other three sites.

Decision (already made — do not re-litigate)

Add one shared formatter to server/lib/workspaceRoots.js:

export function outsideAllowedRootsMessage(realPath, { field = 'path' } = {}) {  }

It renders the rejected realpath'd path and the roots that were checked, e.g.
path is outside allowed directories: <realPath> (allowed: ~, /tmp, /Users, …; on Windows also any non-system drive).
All four call sites use it, so the wording stays identical everywhere and the log names the path.

Chosen over the alternatives:

  • Log-only, keep the response terse — rejected: the four sites already differ in status code and wording, and a shared formatter fixes the drift at the same time.
  • Per-route ad-hoc strings — rejected: that is what exists today and it drifted.

Privacy note: a realpath'd path embeds the OS home directory, so do not echo it in a response body a user might paste into a public issue — the formatter is for the server-side console line; the HTTP body keeps the existing terse message. (Root CLAUDE.md, "Sensitive Data & Privacy": no home-directory paths in published artifacts.)

Files

  • server/lib/workspaceRoots.js (new export + README row in server/lib/README.md)
  • server/routes/git.js, server/routes/commands.js, server/routes/scaffold.js, server/routes/detect.js
  • Tests: server/lib/workspaceRoots.test.js, plus the route suites that assert the current status codes

Acceptance criteria

  • All four confinement sites log a single-line, emoji-prefixed message naming the rejected realpath and the roots checked
  • HTTP status codes and response bodies are unchanged (no home path in a response body)
  • One formatter in server/lib/workspaceRoots.js, exported from the barrel and documented in server/lib/README.md
  • Existing route tests still pass

Context

Deferred from the Windows secondary-drive fix (server/lib/workspaceRoots.js platform-aware roots) to keep that PR to one concern; touching all four routes' messages there would have churned four unrelated test suites.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

code-qualityProposed from a dead-code/duplication auditin-progressClaimed and being workedplanTracked by /do:replan

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions