A failed database query logs its parameters, so OAuth tokens and participant details can end up in the logs

Aperta
  • Qualcosa non va?
#800 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@richshaw ci sta già lavorando.

Dal 5/11/2026.

  • #802 di @richshaw — aperta

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

bug pipeline:building

What happens

When a query fails, Drizzle wraps the error in a DrizzleQueryError whose message is Failed query: <sql>\nparams: <every bind value>, with the same values in stack and an enumerable params field. The app logs these errors whole:

  • provider.on('server_error') in src/oauth/provider.ts logs { err }. oidc-provider stores opaque refresh tokens and authorization codes under their own value as the adapter id, and DrizzleOidcAdapter (src/oauth/adapter.ts) queries oauth_records by that id. So a token refresh that hits a database hiccup writes a live refresh token (30-day lifetime) into the log line.
  • handle() in src/lib/api-response.ts logs { err: e } for any unhandled route error. A failed participant or commitment insert in commitParticipantToSlot logs the participant's email, name and notes.

src/lib/log.ts redacts by key name only (*.token, *.password, …), so nothing touches err.message, err.stack or err.params. The code already knows about this: keyErrorSummary in src/oauth/keys.ts works around it for signing-key queries, and only there.

This is hardening. It needs a database fault plus access to the logs, but anyone who can read the logs could then reuse a refresh token, and participant details land in whatever collects the logs.

Fix, in outline

  • One err serializer in src/lib/log.ts. When the error is a DrizzleQueryError, log it as the query text plus the driver's own error (cause message and code), never the params. Everything else goes through pino.stdSerializers.err as now.
  • That covers every log site that uses the err key, including the two above. Check no log call passes a query error under a different key.
  • keyErrorSummary can stay, or reuse the same helper.

Watch for

  • The Postgres error's own detail. For a unique violation it can read Key (...)=(...) already exists, carrying column values. Leave out cause.detail too, or decide deliberately to keep it, and say which in the PR.
  • Pretty logs in dev. pino-pretty uses the same serializers, so dev output changes the same way. That's fine.
  • CLAUDE.md "Logging" says to extend redact.paths for new secret-shaped fields. Add a line saying query errors are logged without their params.

Done when

  • A unit test logs a fake DrizzleQueryError (params like FAKE_TOKEN and [email protected]) through the real logger and asserts neither value appears in the output, while the query text and the driver's message do.
  • pnpm lint && pnpm format:check && pnpm typecheck && pnpm knip && pnpm test pass.

Found by the pipeline's security scout in OAuth, logging and the participant commit path, at f7ce6df. Checked by an independent run that sent a refresh-token lookup and a participant insert to a dead database through the real adapter, handle() and log.ts in production mode, and saw the fake token, email and name in the JSON log lines. https://claude.ai/code/session_016YZ9SUoktNabfXNaSwoFiG

Lingua principale
TypeScript
Stelle
19
Fork
4
Merge medio
19h 38m
PR unite (30g)
197

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di richshaw/OpenSignup

Tutte le issue di richshaw/OpenSignup

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.