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

オープン
  • 何か問題がありますか？
#800 コメント 0 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@richshaw がすでに取り組んでいます。

2026年11月5日 から。

  • #802 @richshaw による — オープン

評価

この issue はまだ評価されていません。

説明

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

主要言語
TypeScript
スター
19
フォーク
4
平均マージ
19時間 27分
マージ済み PR（30日）
199

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

richshaw/OpenSignup のほかの issue

richshaw/OpenSignup の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。