Skip to content

fix(trueforge): keep OIDC exchange failures out of the error redirect - #687

Open
CaptainAni187 wants to merge 1 commit into
truefoundry:mainfrom
CaptainAni187:fix-oidc-exchange-error-leak
Open

fix(trueforge): keep OIDC exchange failures out of the error redirect#687
CaptainAni187 wants to merge 1 commit into
truefoundry:mainfrom
CaptainAni187:fix-oidc-exchange-error-leak

Conversation

@CaptainAni187

@CaptainAni187 CaptainAni187 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

The callback handler is deliberate about the IdP error branch, then reflects the raw token-exchange exception into /?error=. That message comes from openid-client and embeds the token-endpoint response and issuer host, so internal detail ends up in the address bar, browser history and support screenshots. Since the state cookie is set by any unauthenticated /auth/login visit, an attacker who induces a failing exchange also controls that text on a trusted origin.

Closes #422

Changes

  • Use the same generic login_failed reason as the neighbouring paths. Full detail already goes to the logger above via extractErrorLogFields, so nothing is lost for debugging.
  • Test that drives the catch branch and asserts neither the issuer host nor the upstream error code reaches the redirect

How was this tested?

jest --config jest.unit.config.cjs tests/unit/apis/auth.test.ts — 24 pass. Reverting the one line fails the new case, and the received value shows what was leaking:

Received: "/?error=server%20responded%20with%20an%20error%20in%20the%20response%20body"

Checklist

  • I have read the contributing guidelines
  • format:check passes repo-wide and eslint is clean on the touched files; ran the package's unit suite rather than the full pnpm test. Repo-wide lint:ci has 55 pre-existing errors, all in packages/frontend/* and DropdownMenu.tsx, none in files this PR touches
  • Tests added/updated where it makes sense
  • No hand-edits to generated code
  • Docs / .env.example — not applicable

Changeset included. On process: CONTRIBUTING asks for approval first, and all six help wanted issues are assigned or already have PRs, so there was nothing approved to pick up. Close this if you'd rather it went through the queue.


Note

Medium Risk
Changes OIDC callback failure redirects in the auth path; scope is one redirect reason plus tests, with no change to successful login or IdP error handling.

Overview
When authorization code exchange fails on the OIDC callback, the handler no longer puts the raw openid-client exception message into /?error=. Those messages can include token-endpoint bodies and issuer hosts, which would show up in the browser URL, history, and screenshots.

Failures now redirect with the same generic login_failed reason as state mismatch, missing code, and other callback paths. Full error detail remains on the server via the existing extractErrorLogFields log line.

A unit test stubs a failing token response and asserts the redirect is /?error=login_failed without the issuer host or upstream OAuth error code.

Reviewed by Cursor Bugbot for commit c8cffae. Bugbot is set up for automated code reviews on this repo. Configure here.

@changeset-bot

changeset-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c8cffae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@truefoundry/trueforge Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The callback handler curates the IdP error branch carefully, then reflects the
raw exception from the token exchange into /?error=. openid-client embeds the
token-endpoint response and issuer host in that message, so internal detail
reached the browser's address bar, where it persists in history and support
screenshots. An attacker who induces a failing exchange also controlled that
text on a trusted origin.

Use the same generic login_failed reason as the neighbouring paths; the full
error already goes to the logger above via extractErrorLogFields.

Fixes truefoundry#422
@CaptainAni187
CaptainAni187 force-pushed the fix-oidc-exchange-error-leak branch from e501fb4 to c8cffae Compare September 11, 2026 08:04
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.

OIDC login failure reflects raw openid-client error messages into the /?error= redirect (info disclosure + attacker-influenced UI text)

1 participant