Skip to content

docs: validate state.returnTo before navigating#99

Open
nicknisi wants to merge 3 commits into
mainfrom
docs/validate-state-returnto
Open

docs: validate state.returnTo before navigating#99
nicknisi wants to merge 3 commits into
mainfrom
docs/validate-state-returnto

Conversation

@nicknisi

Copy link
Copy Markdown
Member

What

Updates the "Passing Data Through Auth Flows" README example. The previous example navigated directly to a value read from state:

onRedirectCallback={({ state }) => {
  if (state?.returnTo) {
    window.location.href = state.returnTo; // unvalidated
  }
}}

Replaced with an origin-resolve + same-origin check, plus a [!WARNING] callout that state is untrusted input.

Why

state round-trips through the OAuth redirect as plaintext in the URL and is not validated by WorkOS. Copying the documented example verbatim and populating returnTo from request-derived data yields an open redirect — or XSS, since window.location.href = 'javascript:...' executes. The docs are de-facto the happy path here, so the example itself was teaching the vulnerable pattern.

The replacement resolves returnTo against window.location.origin and only navigates if it stays same-origin, which rejects absolute URLs, protocol-relative //evil.com, and javascript: URIs in one step — no fragile string matching.

🤖 Generated with Claude Code

The "Passing Data Through Auth Flows" example showed
`window.location.href = state.returnTo` with no validation. Since `state`
round-trips through the redirect as plaintext in the URL and is not validated
by WorkOS, an unvalidated `returnTo` is an open-redirect / `javascript:`-URI
sink. Replace the example with an origin-resolve + same-origin check and add a
warning that `state` contents are untrusted.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes an open redirect (and potential XSS) vulnerability in the README documentation example by replacing unvalidated window.location.href = state.returnTo with an origin-resolved, same-origin-checked navigation pattern.

  • The new example wraps new URL(state.returnTo, window.location.origin) in a try-catch (handling malformed input), compares url.origin to window.location.origin, and only then navigates using url.href — correctly blocking absolute URLs, protocol-relative URLs, and javascript: URIs.
  • A [!WARNING] callout was added to the docs reminding readers that state is untrusted input that should never be passed directly to window.location.href.

Confidence Score: 5/5

Safe to merge. The change is documentation-only and replaces a vulnerable pattern with a well-structured same-origin validation that correctly handles all known bypass vectors.

The PR removes a documented open-redirect/XSS pattern and replaces it with a robust origin-checked navigation example. The try-catch handles malformed URLs, the origin equality check blocks absolute and protocol-relative URLs, and navigating via url.href (not a reconstructed path) sidesteps the pathname reassembly bypass. No runtime code is changed — only README documentation.

No files require special attention. The only changed file is README.md.

Important Files Changed

Filename Overview
README.md Documentation fix: replaces vulnerable unvalidated redirect with origin-checked same-origin navigation, adds security warning callout. Logic is correct — try-catch handles malformed URLs, url.href avoids the pathname reassembly bypass noted in prior review.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["onRedirectCallback({ state })"] --> B{typeof state?.returnTo === 'string'?}
    B -- No --> Z[return / no navigation]
    B -- Yes --> C["new URL(state.returnTo, window.location.origin)"]
    C -- throws TypeError --> Z
    C -- success --> D{url.origin === window.location.origin?}
    D -- No --> Z
    D -- Yes --> E["window.location.href = url.href"]

    style Z fill:#f44,color:#fff
    style E fill:#4a4,color:#fff
Loading

Reviews (3): Last reviewed commit: "docs: navigate to validated url.href, no..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

Comment thread README.md
Building on the try/catch guard: reconstructing from url.pathname can yield a
protocol-relative path (e.g. returnTo "https://your-app//evil.com" → pathname
"//evil.com") that redirects off-site even though the origin check passed.
Navigating to the origin-validated url.href avoids this.
@nicknisi nicknisi requested a review from gjtorikian June 11, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant