Skip to content

fix(proxy): reject paths containing encoded '?' or '#' - #217

Draft
alukach wants to merge 1 commit into
mainfrom
fix/reject-encoded-path-delimiters
Draft

alukach wants to merge 1 commit into
mainfrom
fix/reject-encoded-path-delimiters

Conversation

@alukach

@alukach alukach commented Sep 30, 2026

Copy link
Copy Markdown
Member

The server decodes the request path, so an item id containing an encoded ? or # (e.g. a%3Fb) makes request.url.path stop at .../items/a. Every middleware (routing, auth, filter checks) and the reverse proxy use that truncated path, so they agree with each other, but the request acts on a different record from the one the client addressed. For example, DELETE /collections/c/items/a%3Fb deletes item a.

The reverse proxy now returns 400 for such paths instead of forwarding them.

Why not forward the encoded path instead? About 10 places use request.url.path, including EnforceAuthMiddleware. Moving only some of them to the encoded path would make auth and filter decisions disagree with what gets forwarded, and that would be a bypass. Rejecting is the small, safe fix. The trade-off is that ids containing ? or # can't be reached through the proxy; before this change such requests quietly acted on the wrong item.

The test fails on main, where the DELETE is forwarded.

🤖 Generated with Claude Code

The server decodes the path, so request.url.path (used for routing,
auth, filter checks and forwarding) is truncated at a decoded '?'/'#'.
e.g. DELETE /collections/c/items/a%3Fb was checked against and applied
to item 'a'. Reject such requests instead of acting on another record.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label Sep 30, 2026
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Claude finished @alukach's task in 11s —— View job


I'll analyze this and get back to you.


💰 Estimated review cost: $0.09 · 0m11s · 4 turns

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (be008ba) to head (42da32c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
+ Coverage   89.76%   89.77%   +0.01%     
==========================================
  Files          30       30              
  Lines        1348     1350       +2     
  Branches      180      181       +1     
==========================================
+ Hits         1210     1212       +2     
  Misses         97       97              
  Partials       41       41              
Flag Coverage Δ
unittests 89.77% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant