Skip to content

fix(runtime): validate redirect_uri on OAuth /authorize requests - #5122

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/oauth-authorize-redirect-uri-validation-w4
Jul 23, 2026
Merged

fix(runtime): validate redirect_uri on OAuth /authorize requests#5122
pedrofrxncx merged 1 commit into
mainfrom
fix/oauth-authorize-redirect-uri-validation-w4

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Source: bug found while auditing packages/runtime/src/oauth.ts for missing validation on untrusted input (this tick's runtime-utilities focus area).

Why a maintainer wants this: createOAuthHandlers() implements a stateless OAuth 2.1 authorization-server proxy for MCP servers. It already defines isValidRedirectUri() (only allows https:, localhost/127.0.0.1, or a non-http custom scheme) and calls it during dynamic client registration (RFC 7591), but handleAuthorize() — the actual /authorize endpoint — never called it; it only checked that redirect_uri was non-empty. That means any client can point redirect_uri at an arbitrary http:// URL, which the server happily stores in its stateless encoded state param and later redirects the user's browser to (with the OAuth authorization code attached) once the upstream provider completes the exchange — a classic OAuth open-redirect / code-leak vector (RFC 6749 §10.6).

Failure scenario: GET /authorize?response_type=code&redirect_uri=http://attacker.example.com/callback was accepted and, after the user approved access upstream, the flow redirected to http://attacker.example.com/callback?code=..., handing the attacker a valid authorization code exchangeable for an access token. The fix applies the existing isValidRedirectUri() check to handleAuthorize(), returning 400 invalid_request for a rejected URI, matching the check already enforced for client registration.

Regression test: packages/runtime/src/oauth.test.ts — new describe("OAuth /authorize handler") block asserts a plain-http:// external redirect_uri is rejected with 400, and a valid https:// redirect_uri still proceeds to the upstream authorization redirect (302).

Reviewer command: bun test packages/runtime/src/oauth.test.ts

Checks run locally: bun run fmt (clean), cd packages/runtime && bunx tsc --noEmit (clean), bun test packages/runtime/src/oauth.test.ts (5→7 tests, all pass). Full CI validates the rest.


Summary by cubic

Fixes an OAuth open-redirect/code leak by validating redirect_uri on /authorize using isValidRedirectUri(). Insecure or external http:// URIs now return 400 to prevent redirects to attacker-controlled sites.

  • Bug Fixes
    • Apply isValidRedirectUri() in handleAuthorize() within packages/runtime/src/oauth.ts to allow only https, localhost/127.0.0.1, or custom schemes.
    • Add tests in packages/runtime/src/oauth.test.ts to reject http:// and allow https:// redirect URIs.

Written for commit 3fde859. Summary will update on new commits.

Review in cubic

The createOAuthHandlers() authorization-server proxy already defines
isValidRedirectUri() (https/localhost/custom-scheme only) and applies it
during dynamic client registration, but handleAuthorize() never called it
on the actual /authorize request — it only checked that redirect_uri was
present. A crafted redirect_uri (e.g. a plain http:// URL to an
attacker-controlled host) would be accepted, stored in the stateless
pending-auth state, and later used to redirect the user's browser with the
authorization code attached after the upstream OAuth exchange completes.
@pedrofrxncx
pedrofrxncx merged commit f5d36a5 into main Jul 23, 2026
15 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/oauth-authorize-redirect-uri-validation-w4 branch July 23, 2026 19:49
decocms Bot pushed a commit that referenced this pull request Jul 23, 2026
PR: #5122 fix(runtime): validate redirect_uri on OAuth /authorize requests
Bump type: patch

- @decocms/runtime (packages/runtime/package.json): 2.3.3 -> 2.3.4

Deploy-Scope: both
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