fix(api): relax drive path validation, harden archive, rename export route

[mmabrouk]

Context

Part 2 of 2, stacked on #5411. These two PRs implement the pre-merge fixes agreed for #5400 ("[AGE-3965] refactor(drive): virtualize surfaces + unify Files drawer" by @ardaerzin) so it can merge on a sound base. #5411 carries the correctness fixes; this PR carries the contract, validation, and naming fixes. Its base is #5411's branch, so the diff here is only this PR's changes.

The headline is a validation bug that breaks #5400's own lazy-browse flow on real repositories. validate_file_path used a character allowlist, [\w. -]+ per path segment, which rejects folders agents genuinely create: Next.js route groups like app/(auth)/[slug], npm scopes like @scope/pkg, c++.md, and names containing ,, #, ~, or non-ASCII characters. Listings surface those names (agents write them through prefix-scoped signed credentials, which bypass validation), but every round-trip endpoint (expand, read, download) validates path and returned 422. So the file browser showed folders it could never open. The old UI fetched the whole tree in one call with no path, so this rarely fired. The new lazy flow sends path on every expansion, which promotes the latent restriction into a first-class product bug.

Changes

Relax path validation from an allowlist to a denylist. validate_file_path now rejects only genuinely unsafe input: absolute paths, ./../empty segments, NUL, and control characters. Everything else, including the real-world folder names above, passes. Realistic-name tests pin the new behavior.

Harden the archive inputs in the same pass. These changes pull the opposite direction from the relaxation above, so they belong in one commit. ArchiveMount.path and prefix are now validated. Zip members whose keys smuggle .. or backslash traversal are skipped with a warning log instead of minting zip-slip entries that would escape the extraction directory on the client. The Content-Disposition filename fallback is ASCII-only, so CJK and emoji filenames no longer 500 on the latin-1 header encode.

Rename POST /files/archive to POST /files/export. "Archive" is this router's own soft-delete verb, registered three lines below, and the collision was what forced a route-ordering hack. The rename removes the ambiguity and the fragile ordering together. All three frontend call sites are updated. Renaming after the frontend ships against /archive would be a breaking change, so it lands now.

Constrain the listing parameters. depth gets le=1 (the service only implements depth == 1; depth=2 silently fell through to the most expensive full-tree branch). order becomes a Literal (any other string previously returned the flat view unsorted with no error). Invalid values now return 422 instead of silently choosing the wrong or most expensive path.

Name the anonymous tuple. The archive path's (UUID, str, str) triple becomes a MountArchiveSource DTO, threaded through the service, archive utils, router, and tests. api/AGENTS.md names bare-tuple returns as an anti-pattern, and every sibling method here returns a DTO. (The companion MountFilePage DTO from the original change is dropped: the flat paged listing it named was removed with the Files drawer's flat view in 387c62ebef.)

Document total and soften the archive docstring. total counts the entries a view returns before any limit, but its unit follows the view: leaf files only in the recency listing (order and limit set), files-plus-folders in the shallow (depth=1) and browse modes. That per-view meaning is now documented. The archive docstring no longer claims to handle files over 4 GB, which the product owner has ruled out of scope.

Verification

Live QA (recorded). A wire matrix and 60-second UI recording were run on an isolated docker stack against the pre-387c62ebef tip: special-character folder paths returning 200, constraint violations returning 422, export returning 404 on a bad mount, valid zips downloading, the CJK header staying safe, and the old /archive route being gone. All of these target surviving code and remain valid. The recording is attached as a comment on the base PR #5411 (the canonical copy for both PRs).

Tests. The mounts file-ops suite covers the relaxed validation, the export rename, the parameter constraints, and the zip-slip skip (uv run pytest oss/tests/pytest/unit/test_mounts_file_ops.py -q). The full API unit suite is green on the rebased tip.

Independent review. Codex reviewed this diff at high reasoning effort. Its one confirmed catch on this branch, a backslash-based zip-slip path, is fixed and regression-tested.

Merge order

Merge #5411 into fe-refactor/drive-surfaces first, then this PR into #5411's branch, then #5400 into main. Merging is Mahmoud's decision. Ready for review, do not merge without Mahmoud's go.

https://claude.ai/code/session_01PTkjdoAPbhSEFR3ubpgzBU

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agenta-documentation	Ready	Preview, Comment	Jul 20, 2026 3:30pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Mount archive downloads now use /files/export and MountArchiveSource objects. Path and archive-entry validation reject unsafe segments, attachment headers are hardened, and paged file listing uses a MountFilePage response with stricter query validation.

Changes

Mounts API and archive handling

Layer / File(s)	Summary
Archive export contracts and routing api/oss/src/apis/fastapi/mounts/router.py, api/oss/src/apis/fastapi/mounts/utils.py, web/oss/src/components/Drives/driveMedia.ts, api/oss/tests/pytest/unit/test_mounts_file_ops.py	The archive handler is renamed to export_mount_files, the /files/export route is wired through the backend and frontend, and archive inputs use MountArchiveSource objects.
Archive path and header safety api/oss/src/core/mounts/service.py, api/oss/src/apis/fastapi/mounts/utils.py, api/oss/tests/pytest/unit/test_mounts_file_ops.py, api/oss/tests/pytest/unit/test_mounts_service.py	Path validation rejects empty, control, absolute, and traversal segments; unsafe archive members are skipped; and attachment filenames use stricter sanitization and encoding.
Paged file listing contract and validation api/oss/src/core/mounts/dtos.py, api/oss/src/core/mounts/service.py, api/oss/src/apis/fastapi/mounts/router.py, api/oss/tests/pytest/unit/test_mounts_file_ops.py	list_files_page returns MountFilePage, listing parameters are constrained, total-count documentation is clarified, and paging tests use the page object.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DriveMedia
  participant MountsRouter
  participant MountsService
  participant stream_mounts_archive
  DriveMedia->>MountsRouter: POST /mounts/files/export
  MountsRouter->>MountsService: build archive work list
  MountsService-->>MountsRouter: safe archive members
  MountsRouter->>stream_mounts_archive: stream MountArchiveSource list
  stream_mounts_archive-->>DriveMedia: archive download

Loading

Possibly related PRs

• Agenta-AI/agenta#5400: Introduced the related mount file-page and archive/download API surface.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title is concise and accurately summarizes the main validation, archive, and export-route changes.
Description check	✅ Passed	The description is clearly related and matches the validation, archive, routing, DTO, and test updates in the diff.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pr-5400-fixes-contract

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[mmabrouk]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

api/oss/tests/pytest/unit/test_mounts_file_ops.py (1)

106-120: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Strengthen the injection-focused assertions.

The parametrized cases ('a"; DROP TABLE.zip', "a\nb.zip") target header-injection risk, but the test only checks latin-1 round-trip and presence of filename*=UTF-8''. It doesn't assert that ", \, or control characters (\n) are actually absent from the quoted filename= value — the specific guarantee _content_disposition_attachment claims to provide.

✅ Proposed stronger assertion

     def test_header_is_latin_1_safe_with_utf_8_filename(self, filename):
         header = _content_disposition_attachment(filename)

         header.encode("latin-1")
         assert "filename*=UTF-8''" in header
+        # The quoted ASCII fallback must never carry the raw quote/backslash/control chars.
+        quoted = header.split('filename="', 1)[1].split('"; filename*=', 1)[0]
+        assert '"' not in quoted and "\\" not in quoted and "\n" not in quoted

api/oss/src/core/mounts/service.py (2)

1119-1142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Docstring still describes the old tuple return.

Line 1133 says Returns \(files, next_cursor)`but the function now returns aMountFilePageobject (line 1129); callers must usepage.files/page.next_cursor`.

📝 Proposed docstring fix

-        paint is fast no matter how big the mount is. Returns `(files, next_cursor)`; `next_cursor`
-        is None once the listing is exhausted.
+        paint is fast no matter how big the mount is. Returns a `MountFilePage` with `.files` and
+        `.next_cursor`; `next_cursor` is None once the listing is exhausted.

---

1274-1301: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Docstring still describes the old tuple-mount shape.

The docstring says each mount is a (mount_id, zip_prefix, source_path) tuple, but the parameter is now List[MountArchiveSource] (line 1278) with field archive_prefix, not zip_prefix.

📝 Proposed docstring fix

-        Each mount is a ``(mount_id, zip_prefix, source_path)``:
-        ``source_path`` scopes it to a FOLDER within the mount ("" = the whole mount, for "download
-        all"); ``zip_prefix`` places its files under ``prefix/`` in the zip (e.g. "agent-files" for
+        Each mount is a `MountArchiveSource(mount_id, archive_prefix, source_path)`:
+        ``source_path`` scopes it to a FOLDER within the mount ("" = the whole mount, for "download
+        all"); ``archive_prefix`` places its files under ``prefix/`` in the zip (e.g. "agent-files" for
         the folded agent mount). Folder markers are skipped. Each work item is a

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3bd18b06-6f4b-4ccb-91a1-708b4d0f1739

📥 Commits

Reviewing files that changed from the base of the PR and between c93b418 and dc98760.

⛔ Files ignored due to path filters (1)

• web/packages/agenta-api-client/src/generated/api/resources/mounts/client/Client.ts is excluded by !**/generated/**

📒 Files selected for processing (8)

• api/oss/src/apis/fastapi/mounts/models.py
• api/oss/src/apis/fastapi/mounts/router.py
• api/oss/src/apis/fastapi/mounts/utils.py
• api/oss/src/core/mounts/dtos.py
• api/oss/src/core/mounts/service.py
• api/oss/tests/pytest/unit/test_mounts_file_ops.py
• api/oss/tests/pytest/unit/test_mounts_service.py
• web/oss/src/components/Drives/driveMedia.ts

[github-actions]

Railway Preview Environment

Image tag	pr-5412-da03a44
Status	Failed
Railway logs	Open logs
Logs	View workflow run
Updated at 2026-07-20T15:51:33.943Z