Skip to content

Prevent FastAPI login handler from redirecting off site#12742

Merged
mekarpeles merged 3 commits into
internetarchive:masterfrom
jimchamp:hotfix-fast-api-login
May 14, 2026
Merged

Prevent FastAPI login handler from redirecting off site#12742
mekarpeles merged 3 commits into
internetarchive:masterfrom
jimchamp:hotfix-fast-api-login

Conversation

@jimchamp

Copy link
Copy Markdown
Collaborator

The redirect form field was passed directly into the Location header with no validation, allowing open redirect to arbitrary external URLs.

_safe_redirect() now rejects any URL with a scheme, netloc, or leading // — falling back to /. Same-origin paths pass through unchanged.

Also fixes secure=False on the session cookie (authn-011) and adds SameSite=lax to match security best practices.

Addresses GHSA-22xr-xvw5-m2r9

Technical

Testing

Screenshot

Stakeholders

Copilot AI review requested due to automatic review settings May 14, 2026 17:28
@jimchamp jimchamp added the Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle. label May 14, 2026

Copilot AI 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.

Pull request overview

Patches an open-redirect vulnerability (GHSA-22xr-xvw5-m2r9) in the FastAPI login handler by validating the post-login redirect value, and hardens the session cookie attributes.

Changes:

  • Adds _safe_redirect() helper that rejects URLs with a scheme, netloc, leading //, or that don't begin with /, falling back to /.
  • Uses the helper for the Location header on successful login.
  • Sets the session cookie with secure=True and samesite="lax" (was secure=False, no SameSite).

Comment thread openlibrary/fastapi/account.py Outdated
@mekarpeles mekarpeles added the Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed] label May 14, 2026
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
@mekarpeles mekarpeles merged commit d5dc81f into internetarchive:master May 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle. Priority: 0 Fix now: Issue prevents users from using the site or active data corruption. [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants