fix: validate /api/set-workspace path — reject traversal, symlink escape, non-existent (closes #15)

[timon0305]

Problem

POST /api/set-workspace accepted any string in body.path, did ~/ expansion, and stored the result in a module-global consumed by every subsequent file lookup. A client (anyone who reaches the dashboard, including a hostile JS payload — see related XSS issue #11) could post { "path": "/etc" } or { "path": "/home/user/.ssh" } and the app would then serve those files as Cursor data. Symlink escape (e.g. /tmp/cursor-link → /) bypassed any naive startswith check.

Change

• utils/path_validation.py (new) — validate_workspace_path(raw) with WorkspacePathError. os.path.realpath() collapses .. AND resolves symlinks in one step; rejects non-string/empty, non-existent, non-directory, no-Cursor-markers; returns the canonical real path so the override is stored canonically, not as the caller sent it. Lives outside api/ so tests don't pull Flask into scope (existing test-suite convention).
• api/config_api.py — handler calls the validator. Returns 400 { error: "<reason>" } on rejection (was silently 200); on accept returns 200 { success: true, path: "<canonical>" }.
• tests/test_workspace_path_validation.py (new) — 11 regression cases.

Test plan

Unit (python3 -m unittest discover tests): 148/148 OK (was 137; +11 new).

Live HTTP smoke against the running app, all 9 cases:

Case	Payload	Expected	Got
A	empty body	400 path is required	✅
B	empty path string	400 path is required	✅
C	non-existent path	400 path does not exist	✅
D	file /etc/passwd	400 path is not a directory	✅
E	dir /etc (no markers)	400 no Cursor workspaceStorage	✅
F	traversal …/storage/../.. lands at /tmp	400 (.. collapsed, then markers reject)	✅
G	symlink → /	400 (realpath unmasked it)	✅
H	real fake workspace (state.vscdb in subdir)	200 { path: "<canonical>", success: true }	✅
I	symlink → real workspace	200 { path: "<realpath>" } (canonicalised — input was the symlink path)	✅

Case I is the meaningful one: even though the operator sent …/good-link, the override stored is the realpath. Every subsequent read goes through canonical, not the link — a future swap of the link target can't redirect reads.

Closes #15.

Summary by CodeRabbit

• New Features

  • Workspace path setting now strictly validates input, rejects non-object JSON with HTTP 400, enforces workspace heuristics, and stores the canonical (realpath-resolved) directory on success with a success response.
  • Validation failures return clear HTTP 400 error messages; other unexpected failures map to HTTP 500.

• Tests

  • Added thorough tests covering path validation, canonicalization, symlink behavior, traversal protection, and API regression cases for input types and success responses.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ee93769-57f9-432e-a484-3df450f245b9

📥 Commits

Reviewing files that changed from the base of the PR and between 75468a2 and a63297f.

📒 Files selected for processing (2)

• api/config_api.py
• tests/test_workspace_path_validation.py

---

📝 Walkthrough

Walkthrough

Adds a canonicalizing workspace path validator and integrates it into the /api/set-workspace endpoint: the validator resolves ~ and symlinks via os.path.realpath(), enforces existence/directory-ness and a Cursor workspace marker (state.vscdb), returns the canonical path on success, and maps validation failures to HTTP 400.

Changes

Workspace Path Validation & API Integration

Layer / File(s)	Summary
Exception & Helper utils/path_validation.py	Adds WorkspacePathError and _has_cursor_workspace_markers() to detect Cursor workspaces by checking for state.vscdb in immediate subdirectories.
Core Validator utils/path_validation.py	Adds validate_workspace_path(raw_path: str) -> str: rejects non-strings/empty, expands ~, canonicalizes with os.path.realpath(), verifies existence and directory-ness, enforces Cursor marker heuristic, returns canonical path or raises WorkspacePathError.
API Wiring / Error Mapping api/config_api.py	/api/set-workspace now requires JSON object bodies, calls validate_workspace_path(), stores the returned canonical path via set_workspace_path_override(...), returns {"success": true, "path": canonical}, maps WorkspacePathError → HTTP 400 with {"error": ...}, other exceptions → HTTP 500.
Tests tests/test_workspace_path_validation.py	Adds _make_cursor_workspace_dir() and tests: TestValidateWorkspacePath (multiple acceptance/rejection cases, canonicalization, POSIX symlink cases) and TestSetWorkspaceApi (API returns 400 for non-dict JSON, 200 with canonical path for valid dict).

sequenceDiagram
    participant Client
    participant Server as API (/api/set-workspace)
    participant Validator as validate_workspace_path
    participant FS as Filesystem

    Client->>Server: POST /api/set-workspace { "path": "<input>" }
    Server->>Validator: validate_workspace_path("<input>")
    Validator->>FS: expanduser + realpath + stat checks
    FS-->>Validator: existence/type/marker results
    alt validation OK
        Validator-->>Server: return canonical_path
        Server->>Server: set_workspace_path_override(canonical_path)
        Server-->>Client: 200 {"success": true, "path": canonical_path}
    else validation fails (WorkspacePathError)
        Validator-->>Server: raise WorkspacePathError(reason)
        Server-->>Client: 400 {"error": reason}
    else unexpected error
        Server-->>Client: 500 {"error": "internal server error"}
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• Security: validate /api/set-workspace path (reject traversal, symlink escape, non-existent paths) #15 — Implements the described workspace path validation, canonicalization, and API error mapping.

Poem

🐇 Through symlinks, tildes, and dot‑dot fray,
I chase the real path, keep stray bytes at bay.
State markers guide my nimble feet,
Canonical trails make workspaces neat.
Hop on, safe paths — the rabbit says hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the main change: adding path validation to /api/set-workspace to reject traversal, symlink escapes, and non-existent paths, addressing the security vulnerability in issue #15.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

utils/path_validation.py (1)

30-37: ⚡ Quick win

Unify the workspace-marker contract with discovery code.

This validator accepts based on state.vscdb, while api/workspaces.py discovery currently keys off workspace.json. That split can validate a path successfully but still yield no workspaces later. Please centralize one shared marker predicate (or shared marker set) and reuse it in both places.

Also applies to: 77-81

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utils/path_validation.py` around lines 30 - 37, The validator
_has_cursor_workspace_markers currently checks for state.vscdb while
api/workspaces.py discovery looks for workspace.json, causing inconsistent
acceptance; refactor by extracting a shared predicate or marker set (e.g.,
WORKSPACE_MARKERS or a function like is_workspace_marker_present(path: str)) and
replace the ad-hoc checks in _has_cursor_workspace_markers and the discovery
code in api/workspaces.py to both call that single helper so both validation and
discovery use the same marker(s) (ensure the helper checks immediate
subdirectories the same way _has_cursor_workspace_markers intended and include
both state.vscdb and workspace.json).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/config_api.py`:
- Around line 79-80: The current code assigns body =
request.get_json(silent=True) or {} then does raw = body.get("path", "") which
breaks if body is a non-dict JSON (array/string/number); update the handler to
check isinstance(body, dict) before accessing .get(): if not a dict, raise
WorkspacePathError so the existing exception handling returns a 400. Modify the
block that uses request.get_json, the body variable and raw assignment (and
mirror the pattern used in validate_workspace_path()) to ensure only dicts are
.get()-accessed.

In `@tests/test_workspace_path_validation.py`:
- Around line 101-106: The test test_traversal_into_non_workspace_is_rejected
relies on resolving to the system /tmp which is nondeterministic; change the
constructed escape target so it stays inside the test-owned temp tree: when
creating escape from storage (created by _make_cursor_workspace_dir(self.tmp)),
join ".." segments that resolve back into a new directory under self.tmp (e.g.,
create a sibling/parent folder inside self.tmp and use os.path.join(storage,
"..", "..", "<inside-temp-name>") or explicitly create an outside_dir under
self.tmp and point escape at that), then assert validate_workspace_path(escape)
raises WorkspacePathError; update any setup so the escaped target is created
under self.tmp to make the test deterministic.

---

Nitpick comments:
In `@utils/path_validation.py`:
- Around line 30-37: The validator _has_cursor_workspace_markers currently
checks for state.vscdb while api/workspaces.py discovery looks for
workspace.json, causing inconsistent acceptance; refactor by extracting a shared
predicate or marker set (e.g., WORKSPACE_MARKERS or a function like
is_workspace_marker_present(path: str)) and replace the ad-hoc checks in
_has_cursor_workspace_markers and the discovery code in api/workspaces.py to
both call that single helper so both validation and discovery use the same
marker(s) (ensure the helper checks immediate subdirectories the same way
_has_cursor_workspace_markers intended and include both state.vscdb and
workspace.json).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 31572f67-cd0b-4b53-9fbe-9953b971112f

📥 Commits

Reviewing files that changed from the base of the PR and between f8b3cb3 and 75468a2.

📒 Files selected for processing (3)

• api/config_api.py
• tests/test_workspace_path_validation.py
• utils/path_validation.py

[timon0305]

Closed in favour of the in-repo equivalent so CI can run.