fix(security): remove double URL decode enabling path traversal

[hobostay]

Summary

• Remove redundant decodeURIComponent() calls in files.ts that enable path traversal via double-encoded sequences
• Hono's router already decodes path parameters once; the explicit decodeURIComponent decoded a second time, allowing %252e%252e%252f to become ../ and bypass isSafePath

Details

Affected file: packages/core/src/studio-api/routes/files.ts (lines 44, 196)

A request to /projects/1/files/%252e%252e%252f%252e%252e%252fetc/passwd would:

1. Be decoded by Hono to /projects/1/files/%2e%2e%2f%2e%2e%2fetc/passwd
2. After stripping the prefix, decodeURIComponent decodes %2e%2e%2f%2e%2e%2f to ../../etc/passwd
3. The isSafePath check passes because the resolved path appears to be ../../etc/passwd relative to project.dir

The same vulnerability exists in the remove-element route.

Test plan

• Verify normal file operations still work (read, write, delete)
• Verify double-encoded paths like %252e%252e%252f are blocked by isSafePath
• Verify single-encoded paths like %2e%2e%2f are also blocked

🤖 Generated with Claude Code

[jrusso1020]

Verified the bug. Hono's router decodes path params once; the explicit decodeURIComponent at files.ts:44 (and :196 for remove-element) decodes a second time, so a request like /projects/:id/files/%252e%252e%252f... would land at isSafePath already decoded to ../... relative to project.dir. Removing the redundant decode is the correct fix — the null-byte check still runs and isSafePath does the canonical containment check on resolve(project.dir, filePath). LGTM.

[vanceingalls]

Additive review at 230d55a4 — @jrusso1020 already verified and approved.

Audited

• packages/core/src/studio-api/routes/files.ts:44, :196 (the two decodeURIComponent removals)
• The isSafePath containment check downstream

Strength

James covered the bug — Hono's router decodes path params once; the explicit decodeURIComponent at the studio routes decoded a second time, so a request like /projects/:id/files/%252e%252e%252f... would land at isSafePath already containing .. segments. Removing the redundant decode is the correct fix.

Important — Format CI is failing

Format check is failed on 230d55a4. Diff is 3 lines of code change in one file; the format failure is almost certainly a pre-existing prettier disagreement that this branch missed. Run bun run format locally on packages/core/src/studio-api/routes/files.ts and amend.

Per Rule 5, can't APPROVE while Format is red — but it's cosmetic, not substantive. Should be green after one format pass.

Important — Rule 2 audit: any OTHER decodeURIComponent in this codebase?

grep -rn "decodeURIComponent" packages/core/src/studio-api/routes/ packages/cli/src/server/

If there's a third route handler that also calls decodeURIComponent on a Hono-decoded path param, it has the same bug. Vai's diff scans only show the two sites in files.ts that this PR already fixes — but worth a confirming grep before merge.

Nit

• The PR body is excellent — clear repro, attack vector explained, two-call-site enumeration. Reference-quality framing for a security PR from an external contributor.

Verdict

Verdict: COMMENT
Reasoning: Fix is correct (James verified). Format CI red — needs a prettier pass before merge. External-contributor security PR — Vance check before merging once CI clears. Recommend a Rule 2 grep for other decodeURIComponent sites in the studio-api routes.

— Vai

[vanceingalls]

hobostay's path-traversal security fix (remove double URL decode). @jrusso1020 already APPROVED on this SHA. Vai's prior comment-review held off stamping per the no-stamp-without-Vance-nod policy on external contributors. Vance authorized.

Verdict: APPROVE
Reasoning: James verified, security-positive change, no concerns from Vai's pass.

— Vai