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.
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 modifiedapi/routes.py. - These endpoints use a different validator (
api.helpers.safe_resolve), not theapi.workspace.safe_resolve_wsthat #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)
/api/file/raw(routes.py~8277) + inline HTML preview (_serve_inline_html_preview, ~8494) — validates via_file_raw_target()thentarget.open("rb")/target.read_bytes()by pathname. Fix: read throughopen_anchored_fd(Path(s.workspace), target, want_dir=False)and stream/read from the fd.- Folder download zip (
routes.py~9033) — preflights paths thenZipFile.write(path)reopens each by pathname. Fix: store workspace-relative paths during collection, reopen each viaopen_anchored_fd()immediately before writing, write viaZipInfo/zf.open()from the verified fd. - 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 aftersafe_resolve(). Fix: create →open_anchored_create_fd; mkdir →make_anchored_dir; save → a new anchoredO_WRONLY|O_TRUNC|O_NOFOLLOWhelper; delete/rename → anchoredunlinkat/renameat-style helpers (or refuse when they can't be done without following pathnames). - Archive upload/extraction cleanup (
upload.py~313shutil.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-safeO_CREAT|O_EXCL|O_NOFOLLOWcreate.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_fdfallback preserved (no bricking — same pattern as #3398'slist_dirsplit).
Filed from the #3398 review (Codex round-4 findings). Severity: medium (race-only; the no-race exfil path is already closed).
Security hardening to close TOCTOU races across multiple file-API route handlers using anchored file descriptors.
- api/routes.py
- api/helpers.py
- api/workspace.py
- api/upload.py
- tests/test_security.py