Skip to content

fix(auth): constrain GitHub OAuth return paths - #664

Merged
IvGolovach merged 1 commit into
junhoyeo:mainfrom
IvGolovach:codex/oauth-return-to-safety-20260601
Jun 4, 2026
Merged

fix(auth): constrain GitHub OAuth return paths#664
IvGolovach merged 1 commit into
junhoyeo:mainfrom
IvGolovach:codex/oauth-return-to-safety-20260601

Conversation

@IvGolovach

@IvGolovach IvGolovach commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Sanitize GitHub OAuth returnTo values before they are stored in the OAuth state cookie.
  • Re-sanitize the cookie value in the callback before redirecting.
  • Build the final redirect with new URL(...) so only same-origin relative app paths are used.

Why

The OAuth start route accepted a raw returnTo query parameter and the callback later concatenated it with the site base URL. Values such as @evil.test/path, absolute URLs, protocol-relative URLs, backslashes, or encoded variants could produce unsafe redirect targets or confusing userinfo-style URLs. OAuth redirects should only return users to safe relative paths inside the app.

Diff scope

  • packages/frontend/src/lib/auth/returnTo.ts: adds a shared sanitizer with a safe /leaderboard fallback.
  • packages/frontend/src/app/api/auth/github/route.ts: stores only sanitized returnTo values in the state cookie.
  • packages/frontend/src/app/api/auth/github/callback/route.ts: sanitizes again at callback time and redirects via new URL(returnTo, baseUrl).
  • packages/frontend/__tests__/api/githubAuthReturnTo.test.ts: adds route-level regression coverage for safe relative paths and unsafe external/userinfo/backslash/encoded inputs.

Branch integrity

  • Base branch: main
  • Validated base SHA: a86e688d620939d2c973c6d5625baa815ea223d7
  • Ahead/behind against fetched origin/main: 0 behind / 1 ahead
  • Merge base: a86e688d620939d2c973c6d5625baa815ea223d7
  • Diff proof was computed against the fetched base.

Commit integrity

  • Introduced commit: ac9bbd6 fix(auth): constrain GitHub OAuth return paths
  • Final PR diff contains only the intended OAuth route, sanitizer, and regression test changes.
  • Ledger: not applicable — not required for selected validation mode/change family.
  • Version: not applicable — not required for selected validation mode/change family.

Diff hygiene

  • git diff --name-status origin/main...HEAD: A packages/frontend/__tests__/api/githubAuthReturnTo.test.ts, M packages/frontend/src/app/api/auth/github/callback/route.ts, M packages/frontend/src/app/api/auth/github/route.ts, A packages/frontend/src/lib/auth/returnTo.ts
  • git diff --check origin/main...HEAD: PASS, no output

Validation mode and proof

  • Validation mode: Mode 3 — security-sensitive OAuth redirect behavior.
  • bun x vitest run __tests__/api/githubAuthReturnTo.test.ts: PASS, 15 tests
  • bun x eslint src/app/api/auth/github/route.ts src/app/api/auth/github/callback/route.ts src/lib/auth/returnTo.ts __tests__/api/githubAuthReturnTo.test.ts: PASS
  • Full package TypeScript check not run as merge proof locally because the current project-wide check is blocked by unrelated existing frontend errors outside this diff; remote PR CI should provide final gate status after the PR is opened.

CI context confirmation

  • Pending — required GitHub Actions checks will run after the PR is created.
  • CI workflow context names unchanged.

Runtime safety

  • Reviewed changed runtime paths: GitHub OAuth start route, callback route, and shared return-path sanitizer.
  • No new blocking locks, unbounded queues, network calls, or database writes were introduced.
  • Invalid, external, protocol-relative, backslash, and encoded unsafe values fall back to /leaderboard.
  • No invariant regression introduced.

Migration notes

Not applicable — no DB migration changed.

Documentation integrity

Not applicable — no docs, commands, or user-facing option names changed.

Rollback plan

Rollback: revert this PR.
DB downgrade: not applicable.
Data repair: not applicable.
Operational caveats: none known.

Known residual risks

  • Remote CI has not run yet because the PR has not been opened.
  • Existing unrelated frontend TypeScript errors remain outside this diff.

Summary by cubic

Sanitized GitHub OAuth returnTo values to prevent open redirects. Only same‑origin relative paths are allowed; unsafe inputs fall back to /leaderboard.

  • Bug Fixes
    • Added sanitizeAuthReturnTo with a safe /leaderboard default in packages/frontend/src/lib/auth/returnTo.ts.
    • Start route stores only sanitized returnTo; callback re-sanitizes and redirects via new URL(returnTo, baseUrl).
    • Blocked absolute, protocol-relative, backslash, userinfo-style, and encoded variants; added regression tests in packages/frontend/__tests__/api/githubAuthReturnTo.test.ts.

Written for commit ac9bbd6. Summary will update on new commits.

Review in cubic

Validation
* Validation tier: Tier 3 - auth redirect safety affects security-sensitive runtime behavior.
* git diff --check: PASS
* git diff --cached --check: PASS
* bun x vitest run __tests__/api/githubAuthReturnTo.test.ts: PASS
* bun x eslint src/app/api/auth/github/route.ts src/app/api/auth/github/callback/route.ts src/lib/auth/returnTo.ts __tests__/api/githubAuthReturnTo.test.ts: PASS
* Ledger: not applicable - not required for selected validation tier/change family.
* Version: not applicable - not required for selected validation tier/change family.
* Not run: bun x tsc --noEmit --pretty false - project-wide check is blocked by unrelated existing frontend errors outside this diff.

Rollback
* git revert HEAD
@vercel

vercel Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tokscale Ready Ready Preview, Comment Jun 1, 2026 10:15pm

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 4 files

Re-trigger cubic

@IvGolovach
IvGolovach merged commit 9300d24 into junhoyeo:main Jun 4, 2026
7 checks passed
@IvGolovach
IvGolovach deleted the codex/oauth-return-to-safety-20260601 branch June 4, 2026 22:41
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.

2 participants