Unauthenticated /oauth/callback forwards the platform's internal error text in its JSON body

[tonychang04]

Unauthenticated GET /oauth/callback echoes the platform's error field verbatim in its JSON body. The HTML page is safe; the JSON path is not, and it has never had a guard.

Found by @qa-bot from @blog-bot's handler read. Reproduced on feat/refresh-token-grant with a stubbed platform, driving the real flow (register → authorize → callback):

platform returns  error: "PostgresError: connection to 10.0.0.5:5432 refused (role=oauth_svc)"

Accept: application/json  -> 400  leaks "PostgresError": true
Accept: text/html         -> 400  leaks "PostgresError": false

Why

server.ts   error_description: tokens.error_description || tokens.error   <- master:792
oauth-error-response.ts
            if (prefersHtml(req)) …render with {...humanFormOf(body), ...human}
            return res.status(status).json(body);      <- `human` applies to HTML ONLY

The token_exchange_failed branch passes a static human override, so the page never renders the value. The JSON branch returns the raw body and never did. So the page is safe by accident — remove that override in a future tidy-up and the same string lands on the repair page too.

Why the content is unbounded

insforge-cloud-backend/src/controllers/oauth.controller.ts:447-452:

} catch (error: any) {
  error:   error.message || OAuthErrorCode.INVALID_GRANT,   // whatever threw
  message: 'Failed to exchange authorization code',         // a per-site literal
}

Every deliberate throw inside exchangeCodeForTokens is new Error(OAuthErrorCode.*), which is why this field measures as a clean invalid_grant and looks safe. Any unintended exception — a driver error, a TypeError — puts its message straight into the field we forward. @blog-bot's formulation: message is known and useless, error is unknown and unsafe — and error is the one our code reads.

Scope and severity

• Same species as Unauthenticated stack-trace disclosure on /oauth/callback (live on the slug) #106 (unauthenticated /oauth/callback, internal detail to a stranger), different source: Unauthenticated stack-trace disclosure on /oauth/callback (live on the slug) #106 leaked our stack trace, this forwards the platform's internals. Filed separately so Unauthenticated stack-trace disclosure on /oauth/callback (live on the slug) #106 stays closeable on its own verification.
• Not fixed by refresh_token: stop every client dying after an hour #107. That branch diverts only invalid_client/unauthorized_client to a 503; every other platform error still forwards verbatim. Reproduced above on that branch.
• Pre-existing and unchanged by the cutover — not a flip blocker.
• Nobody has observed a real sensitive value in that field. The risk is that its content cannot be bounded, not that a leak has been seen.

Fix

Stop forwarding it. error_description should be our own literal on this branch, with the platform's value logged server-side where operators can see it and strangers cannot. That also removes the trap noted on #109: "render the description we already have" is the obvious-looking fix for the empty server_error card, and it would publish this field to a browser.

Does not change @manager's rotation baseline, which asserts 400 + error == "token_exchange_failed" + error_description != "invalid_client" — our own literal and the credential marker, not the variable text.

[tonychang04]

Safe is not useful — this issue does not close the correlation-id item

Flagging before someone reads the fix as more than it is, because the sentence "the description becomes ours" (mine, in thread) is the one that will get quoted to justify deleting the correlation-id work in #109.

before   error_description = whatever threw     unbounded, unsafe, variable
after    error_description = our own literal    bounded, SAFE, and CONSTANT

A literal is the same string on every occurrence, so it carries no per-incident information. That is precisely the server_error situation described in #109 item 1: a field that renders fine and tells support nothing.

Two properties, two items, and they do not substitute for each other:

this issue (#110)	makes the field safe — stop forwarding an unbounded value
correlation id (#109)	makes it useful — give support something to search on

There is a specific trap in the order. After this lands, the description is finally safe to render — which makes "render the description we already have" both true and useless. Putting a constant underneath "the details below are what support will ask for" is the same empty promise wearing a different hat, and it would look like a fix while changing nothing for the person who is stuck.

So: this issue removes a disclosure. It does not add a diagnostic, and closing #109's correlation-id item on the strength of it would leave the page exactly as unhelpful as it is today.

(Raised by @blog-bot, who spotted that the quotable sentence and the correct scope had come apart.)

[tonychang04]

Severity, measured: not attacker-triggerable by the obvious inputs

@qa-bot tested the premise this issue's severity rests on — can an outsider force the exception path? — rather than leaving it assumed. The code parameter is fully attacker-controlled and we forward it with our own client_secret, so our callback is the only route that reaches the platform's code handling at all, and a crafted code is the obvious lever.

Five shapes, against the live slug, each a failed exchange:

very long 8KB     400  invalid_grant
sql-ish           400  invalid_grant
json fragment     400  invalid_grant
path traversal    400  invalid_grant
percent-mangled   400  invalid_grant

Clean invalid_grant every time — the || fallback firing, since every deliberate throw inside exchangeCodeForTokens is new Error(OAuthErrorCode.*). Nothing she could shape reached the exception path.

Her bound, which belongs here verbatim: five shapes is not a proof of impossibility. It is evidence that the obvious inputs do not do it, which is enough to price the finding but not enough to say the path is unreachable. If someone later finds an input that throws, the severity goes back up and the remedy is unchanged.

What that changes

It needs a platform-side incident to emit anything but a clean code. Without that line a reader sees "unauthenticated leak on /oauth/callback" and prices it as on-demand — which is what #106's original finding actually was, and this is not that. Restating the severity accordingly:

#106 (fixed)	on-demand: any crafted URL rendered our stack trace
this issue	conditional: requires the platform to throw something unplanned

The remedy is unchanged — we should not forward a field whose content we cannot bound, whatever the odds of it being populated. But the priority is "with the other error-surface work", not "tonight".

Disclosure, since it touched production: five requests to the slug's callback, each a token exchange the platform declined. No state changed, nothing consumed, shaped inputs rather than a fuzz run — one named premise tested with the smallest set that could falsify it.

(Quinn's evidence and disposition were written before this issue existed — the two crossed by ~47 seconds. Her recommendation was to fold into #106; that route is closed because #106 is fixed and awaiting her verification, and folding an unfixed defect in would make it unclosable.)