github →

New request

#3450: [security] Extend symlink-swap TOCTOU hardening to api/routes.py file-API endpoints (raw/zip/editor) + upload cleanup

We'll provision a sandbox, run an agent against the issue, and open a draft PR. You can pull the branch and iterate from there.

Issue
bug

Summary

A security review of #3398 (workspace symlink-escape containment, shipped v0.51.221) surfaced a pre-existing TOCTOU (time-of-check-to-time-of-use) class affecting the api/routes.py file-API endpoints that #3398 did not touch. These endpoints validate a path with api.helpers.safe_resolve() and then re-open / read / write / delete by pathname, so a path component swapped to an external symlink between validation and use can escape the workspace.

This is the same race class #3398 closed for the workspace-tree read/list/upload/extract paths (via the new portable anchored openat-walk helpers open_anchored_fd / open_anchored_create_fd / make_anchored_dir in api/workspace.py). This issue tracks extending that hardening to the remaining routes.py file-API surface.

Why it was scoped separately from #3398

  • #3398's diff is confined to api/workspace.py + api/upload.py (+ tests). It never modified api/routes.py.
  • These endpoints use a different validator (api.helpers.safe_resolve), not the api.workspace.safe_resolve_ws that #3398 hardened.
  • The race is pre-existing in master — it predates #3398 and was not introduced or widened by it. #3398 is a net security improvement (it closes the no-race static symlink-escape exfil path and the workspace-tree TOCTOU); this is the remaining, lower-likelihood surface.

Threat model / likelihood

TOCTOU here requires an attacker to win a sub-millisecond swap race against the server's own file API. The higher-likelihood, no-race exfiltration path (an in-workspace symlink to ~/.ssh / ~/.hermes/auth.json read through /api/list / read_file_content) is already closed by #3398. This issue is the race-only residue on the broader endpoint set.

Endpoints to harden (from the review)

  1. /api/file/raw (routes.py ~8277) + inline HTML preview (_serve_inline_html_preview, ~8494) — validates via _file_raw_target() then target.open("rb") / target.read_bytes() by pathname. Fix: read through open_anchored_fd(Path(s.workspace), target, want_dir=False) and stream/read from the fd.
  2. Folder download zip (routes.py ~9033) — preflights paths then ZipFile.write(path) reopens each by pathname. Fix: store workspace-relative paths during collection, reopen each via open_anchored_fd() immediately before writing, write via ZipInfo/zf.open() from the verified fd.
  3. File-editor endpoints (routes.py ~11437): write/create (target.write_text()), mkdir (target.mkdir()), delete (target.unlink() / shutil.rmtree()), rename (source.rename(dest)) — all pathname ops after safe_resolve(). Fix: create → open_anchored_create_fd; mkdir → make_anchored_dir; save → a new anchored O_WRONLY|O_TRUNC|O_NOFOLLOW helper; delete/rename → anchored unlinkat/renameat-style helpers (or refuse when they can't be done without following pathnames).
  4. Archive upload/extraction cleanup (upload.py ~313 shutil.rmtree(dest_dir) on failure, ~499 archive unlink) — pathname cleanup after anchored create. Fix: anchored recursive cleanup under the workspace, or skip auto-cleanup when it can't be done without following pathnames.

Building blocks already in place (from #3398, v0.51.221)

api/workspace.py now has the portable anchored primitives to build on (Linux/macOS via openat+dir_fd, Windows path-based fallback gated on os.supports_dir_fd):

  • open_anchored_fd(root, target, *, want_dir) — race-safe read/list open.
  • open_anchored_create_fd(root, dest) — race-safe O_CREAT|O_EXCL|O_NOFOLLOW create.
  • make_anchored_dir(root, dest) — race-safe anchored mkdir walk.

This issue needs two more siblings: an anchored truncate-open (for editor save/overwrite) and anchored unlink/rename helpers.

Acceptance

  • All four endpoint groups read/write/delete/rename through anchored fd helpers (no pathname op after the containment check).
  • Regression tests with the validate-then-symlink-swap pattern (mirroring tests/test_workspace_symlink_containment.py's TOCTOU tests) for each endpoint.
  • Windows/no-dir_fd fallback preserved (no bricking — same pattern as #3398's list_dir split).

Filed from the #3398 review (Codex round-4 findings). Severity: medium (race-only; the no-race exfil path is already closed).

Assessmentadvisory
bug●●● hard90% confidence

Security hardening to close TOCTOU races across multiple file-API route handlers using anchored file descriptors.

Likely files
  • api/routes.py
  • api/helpers.py
  • api/workspace.py
  • api/upload.py
  • tests/test_security.py
Create the request

This opens a fresh agent run and a draft PR for issue #3450.

Cancel