fix: scope Ory-Base-URL-Rewrite* headers to Ory-bound requests#446
fix: scope Ory-Base-URL-Rewrite* headers to Ory-bound requests#446aeneasr wants to merge 2 commits into
Conversation
The ory proxy/tunnel reverse proxy attached the Ory-No-Custom-Domain-Redirect, Ory-Base-URL-Rewrite, and Ory-Base-URL-Rewrite-Token headers to every outbound request, including those forwarded to the developer's own upstream app. The last header carries a temporary project API key, which is only consumed by Ory and has no reason to reach the upstream. Scope these headers to Ory-bound requests only as a defense-in-depth hardening, and add a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe reverse-proxy request middleware was extracted into ChangesProxy middleware
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloudx/proxy/helpers.go`:
- Around line 300-318: Update the proxy rewrite handler returned by the
surrounding function to remove all incoming Ory-No-Custom-Domain-Redirect,
Ory-Base-URL-Rewrite, and Ory-Base-URL-Rewrite-Token headers before the
host-specific logic. Then retain the existing selective header setting,
including only setting the token when apiKey is non-empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: db446403-fff8-47b7-b646-8044f97f0bae
📒 Files selected for processing (2)
cmd/cloudx/proxy/helpers.gocmd/cloudx/proxy/helpers_test.go
Prevents clients from spoofing Ory-Base-URL-Rewrite*, Ory-Base-URL-Rewrite-Token, and Ory-No-Custom-Domain-Redirect headers. Spoofed headers would otherwise be forwarded unchanged to the developer's upstream app, or — when no API key is configured — passed through to Ory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The
ory proxy/ory tunnelreverse proxy attached theOry-No-Custom-Domain-Redirect,Ory-Base-URL-Rewrite, andOry-Base-URL-Rewrite-Tokenheaders to every outbound request, including those forwarded to the developer's own upstream application. The last header carries the temporary project API key, which is only consumed by Ory and has no reason to reach the upstream. This change scopes all three headers to Ory-bound requests only (extracting the request middleware into a testablereqMiddlewarefunction) and adds a regression test that pins the behavior. Tunnel mode is unaffected, since its upstream is Ory, so every request still takes the Ory branch.Related Issue or Design Document
This was raised via a HackerOne report. It is not an exploitable vulnerability (the upstream is the developer's own app, the tool is development-only, and the temporary key is short-lived and auto-deleted on shutdown), so it is triaged as informational. This PR is a defense-in-depth / hygiene hardening: don't send a credential to a destination that doesn't need it.
Checklist
Further comments
The header-scoping logic was extracted from an inline closure in
runReverseProxyinto a package-levelreqMiddleware(conf, oryURL, apiKey)function purely to make it unit-testable, since the proxy package previously had zero test coverage andrunReverseProxyblocks onListenAndServe. The new test was confirmed to fail against the pre-fix code and pass after the fix.Summary by CodeRabbit
Bug Fixes
Tests