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

Abierto
  • ¿Algo no está bien?
#800 0 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@richshaw ya está trabajando en esto.

Desde el 5/11/2026.

  • #802 de @richshaw — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

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

Lenguaje dominante
TypeScript
Estrellas
19
Forks
4
Merge medio
19 h 27 min
PR fusionados (30 d)
199

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de richshaw/OpenSignup

Todos los issues de richshaw/OpenSignup

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.