Unauthenticated stack-trace disclosure on /oauth/callback (live on the slug)

[tonychang04]

Live on the deployed slug, unauthenticated

GET /oauth/callback?error=x&error_description=a&error_description=b
Accept: text/html

400, and the body is Express's error page:
  TypeError: text.replace is not a function
    at escapeHtml2 (file:///app/dist/http-server.js:1701:15)
    at renderOAuthErrorPage (file:///app/dist/http-server.js:1694:10)
    at sendOAuthError (file:///app/dist/http-server.js:1746:49)

Absolute container paths and the dependency tree, to anyone who crafts the URL.
No login required — /oauth/callback is unauthenticated by design, and
error_description comes straight off req.query, where a repeated parameter
parses as an array.

master @ 28b9b26 reproduces it locally too, so this is not a deployment
artefact.

The JSON path is fine — it is the HTML path only

same URL, no Accept: text/html   ->  {"error":"x","error_description":["a","b"]}   clean
?error=a&error=b                 ->  clean page (error is switched on, never escaped raw)
?error=x                         ->  clean page (control)

So it is one field, on the browser navigation that the callback always is.

This is a regression the rewrite introduced, and the reason is worth stating

Production (mcp.insforge.dev, 1.0.0) does not leak — but not because it
handles the array. It has no HTML error page at all, and answers JSON
regardless of Accept:

mcp.insforge.dev  same request  ->  {"error":"weird","error_description":["a","b"]}

The repair page — added to help stuck users — is what introduced the
disclosure. Cutting mcp.insforge.dev over to this build ships the leak to
production.

Previously reported and shipped anyway

I raised this exact TypeError: text.replace is not a function against PR #84
on 2026-07-31, as a review finding. #84 merged without it being fixed, and it
has been live since. A finding recorded only as a comment on an unmerged PR has
no owner once that PR merges — hence this issue, so it is tracked independently
of whichever PR happens to carry the fix.

Fix

Normalising in humanFormOf (PR #103, b03a467) covers it, because
sendOAuthError spreads humanFormOf(body) straight into the renderer, so
nothing reaches escapeHtml unnormalised. Worth also making escapeHtml
defensive about non-strings, since it owns every browser-visible field and
currently trusts its input.

Filed separately from #103 so the leak does not depend on that PR landing.

[tonychang04]

Closed on the deployed artifact: slug now returns 400 / 2100 bytes with zero leak markers, container reports 1.2.12, verified independently after deploy bc08124a.

But the fix reached one host of nine. Sweeping mcp-bf35bbc3-*.fly.dev:

-41 -44 -45   v1.2.10  500  LEAKING  ioredis MaxRetriesPerRequestError + /app/node_modules/… path
-50           v1.2.10  400  clean (JSON only)
-60 -72       v1.2.10  400  LEAKING  TypeError at /app/dist/http-server.js
-76 -78       v1.2.12  400  LEAKING
-79           v1.2.12  400  clean  <- today's deploy

Seven of nine live hosts still disclose filesystem paths, unauthenticated, across three retired builds. They accumulate per deploy and no future deploy reaches them. Tracking here rather than reopening — the code fix is correct and shipped; the residue is an infrastructure action in the Manufact org.

[tonychang04]

Correcting the count in my earlier comment: it is 17 of 23, not 7 of 9.

My first sweep used a 6-second timeout. Fly autostarts stopped machines on first request, so a host that was merely asleep timed out and scored as absent:

6s timeout    9 live / 7 leaking     <- what I reported
6s timeout   19 live                 (re-run)
6s timeout   23 live                 (re-run)
15s timeout  23 live / 17 leaking    stable across three runs

Leaking: -41 -43 -44 -45 -52 -53 -55 -56 -58 -60 -68 -70 -72 -73 -75 -76 -78. Still a lower bound — suffixes 1-90 only, and a slower cold start would drop out the same way.

Method note for whoever cleans these up: a timeout turns "asleep" into "gone", and the failure is silent — you get a smaller number rather than an error. Re-sweep with a long timeout after destroying them rather than trusting either figure.

[tonychang04]

Related surface, same endpoint, different source — worth folding in or splitting out.

The platform's token-endpoint handler returns error: error.message || INVALID_GRANT (Blair's read of the platform source), so that field carries whatever threw internally. Our callback does error_description: tokens.error_description || tokens.error, and the platform has no error_description — so we forward the unbounded field.

Measured on master with a simulated internal throw:

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

HTML page  internal string NOT rendered  (token_exchange_failed passes an explicit
                                          copy override to sendOAuthError)
JSON body  {"error":"token_exchange_failed",
            "error_description":"PostgresError: connection to 10.0.0.5:5432
             refused (role=oauth_svc)"}          <- forwarded verbatim

Two notes: the HTML path is safe by accident — remove that override and the same string renders on the repair page. And the internal message was simulated (I can't make the real platform throw); what's measured is our forwarding behaviour, not the platform's content.

[tonychang04]

Severity note on the forwarded-error surface above: it needs a platform incident to emit anything.

The callback gates on our own sealed state before any exchange is attempted, so the caller must have started a real flow with us — but DCR is open, so "unauthenticated" is still fair. The question that decides severity is whether an outsider can push the platform off its clean invalid_grant and onto its exception path, since the code is fully attacker-controlled and we forward it with our own client_secret.

Tested against the deployed slug, five shaped codes, each one declined 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 fallback every time — nothing reached the exception path.

Bound on that claim: five shapes is not a proof of impossibility, only evidence that the obvious inputs don't do it. Enough to price the finding, not enough to call the path unreachable.

So the exposure is: during a platform incident, internal diagnostics reach whoever happens to be mid-login and land in their client's logs — materially different from this issue's original finding, which was a crafted URL producing a stack trace on demand. Worth stating here so the next reader doesn't price them the same.