Skip to content

fix(uploads): prevent path traversal in /api/uploads/[fileId] route - #776

Open
sebastionoss wants to merge 1 commit into
rowboatlabs:mainfrom
sebastionoss:security/cwe22-uploads-route-3349
Open

fix(uploads): prevent path traversal in /api/uploads/[fileId] route#776
sebastionoss wants to merge 1 commit into
rowboatlabs:mainfrom
sebastionoss:security/cwe22-uploads-route-3349

Conversation

@sebastionoss

Copy link
Copy Markdown

Summary

The PUT and GET handlers in apps/rowboat/app/api/uploads/[fileId]/route.ts build a filesystem path by passing the dynamic [fileId] route segment straight into path.join(UPLOADS_DIR, fileId). Because path.join collapses .. segments, a client can escape UPLOADS_DIR and read/write arbitrary paths reachable by the Node process — e.g. PUT /api/uploads/..%2F..%2Fetc%2Fmyfile writes outside the uploads root.

Why this is reachable without auth

apps/rowboat/middleware.ts matches /api/* explicitly and, for those paths, only sets CORS headers and returns — it does not call authCheck. authCheck is scoped to /projects, /billing, /onboarding and is additionally guarded by USE_AUTH === 'true'. So the uploads route is reachable by any network client that can reach the app, regardless of Auth0 configuration.

Fix

Add a small resolveUploadPath(fileId) helper that:

  1. Rejects empty input, NUL bytes, and any input containing / or \fileId is meant to be a single opaque segment.
  2. Resolves the candidate path against path.resolve(UPLOADS_DIR).
  3. Verifies the resolved absolute path is strictly contained within the resolved uploads root (resolved === root || resolved.startsWith(root + path.sep)).

Both the PUT (user-supplied fileId) and GET (stored doc.data.path segment, which could be poisoned) paths route through the helper. On failure, PUT returns 400 Invalid file ID; GET returns 404 File not found.

The change is 30 additions / 3 deletions and touches only the one route file. No behavioural change for valid, opaque fileId values.

Proof of Concept

With the app running locally (docker compose up per the README), against the pre-fix code:

# Arbitrary write outside UPLOADS_DIR (/uploads)
curl -X PUT --data-binary 'pwned' \
  'http://localhost:3000/api/uploads/..%2F..%2Ftmp%2Fpwned.txt'

# On the server:
$ cat /tmp/pwned.txt
pwned

After the fix, the same request returns:

HTTP/1.1 400 Bad Request
{"error":"Invalid file ID"}

A benign upload with an opaque id (e.g. curl -X PUT --data-binary @foo.pdf http://localhost:3000/api/uploads/abc123) still succeeds and writes to /uploads/abc123.

Testing

I exercised resolveUploadPath with the payload set below (Node, UPLOADS_DIR=/uploads):

Input Expected Result
abc123 accept → /uploads/abc123 accept
../etc/passwd reject reject
..%2Fetc%2Fpasswd (decoded by Next router to ../etc/passwd) reject reject
....//....//etc/passwd reject (contains /) reject
/etc/passwd reject reject
foo/bar reject reject
foo\\bar reject reject
abc\u0000.jpg reject reject
`` (empty) reject reject

8/9 malicious payloads rejected at the input-shape check; the remaining case (../../uploads/x — a traversal that happens to land back in root) is additionally caught by the startsWith(root + sep) containment check. The legitimate opaque-id case still writes and reads correctly.

Adversarial review

Before submitting I tried to disprove this:

  • Could the Next.js router itself strip .. from the dynamic segment? No — Next passes the URL-decoded segment through to the handler; ..%2F arrives as ../ inside fileId.
  • Is there a router-level dependencies=/middleware gate I missed? I re-read apps/rowboat/middleware.ts: the /api/* branch returns early after setting CORS and never calls authCheck. No other middleware file covers this route.
  • Is UPLOADS_DIR typically on a read-only volume in production? The default is /uploads (writable by design, since the same route writes uploads). Even in read-only-mount deployments, the GET path still allows read of any file the process can fs.access, which is a confidentiality bug on its own.
  • Would doc.data.path in GET always be trusted? It comes from the docs repository, which is populated from user-controlled upload flows, so treating it as untrusted here is correct defence-in-depth.

Notes for maintainers

  • Diff is minimal and behaviour-preserving for well-formed ids.
  • Happy to add a Jest test alongside if you'd like — I kept the PR to the single file to make review trivial, but can extend on request.

Discovered by the Sebastion AI GitHub App.

Validate the user-supplied fileId (and stored doc.data.path segment) is a single opaque path component contained within UPLOADS_DIR before writing or reading. Previously, path.join(UPLOADS_DIR, fileId) resolved encoded traversal sequences, allowing PUT/GET to escape the uploads directory.
brianlane added a commit to brianlane/newCoworker that referenced this pull request Jul 22, 2026
…F, rag-worker) (#838)

Bumps the Rowboat pin bb32686b -> f422019e (brianlane/rowboat
newcoworker/upgrades-jul-2026), which cherry-picks three unmerged
upstream community fixes onto our hardened fork branch:

- rowboatlabs/rowboat#776: path traversal in /api/uploads/[fileId]
  (Rowboat :3000 is published through the tenant tunnel; the route has
  no auth, so traversal was reachable)
- rowboatlabs/rowboat#547: SSRF guard on webhook + custom MCP server
  URLs (blocks private/internal IP targets like loopback services)
- rowboatlabs/rowboat#701: rag-worker clears stale doc embeddings
  before upsert (KVM8 jobs-worker + qdrant)

Upstream main itself has zero apps/rowboat commits since our pin base
(all new work is the apps/x desktop app), so this is a cherry-pick
bump, not an upstream merge.

Also fixes the integration Mongo seed: the agent 'model' field was
dropped in April (1de8587), and Rowboat's createAgent has no model
fallback - every turn crashed with "Cannot read properties of
undefined (reading 'startsWith')", so the kvm suites failed on main
with the OLD pin too. Restoring the field (production deploy-client.sh
seeds always set it) makes the suites pass again.

Validated: agent-tool-seed-parity, test:integration:kvm2 and
test: integration:kvm8 (real stacks built from the new SHA) all green.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant