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

Đang mở
  • Có gì không ổn?
#800 0 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@richshaw đang làm issue này rồi.

Từ ngày 5/11/2026.

  • #802 của @richshaw — đang mở

Đánh giá

Issue này chưa được đánh giá.

Mô tả

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

Ngôn ngữ chính
TypeScript
Star
19
Fork
4
Merge trung bình
19 giờ 27 phút
Pull request đã merge (30 ngày)
199

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của richshaw/OpenSignup

Tất cả issue của richshaw/OpenSignup

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.