Security [HIGH]: Pino logger has no redact paths — passwords/tokens logged cleartext #46

Closed
opened 2026-04-16 22:05:10 +02:00 by Hartmut · 1 comment
Owner

Problem

packages/api/src/lib/logger.ts configures pino without any redact: option. assistant-tools.ts:744 logs params verbatim — for tool set_user_password, params.password is written to logs. Any Authorization/cookie headers similarly at risk.

Evidence

  • packages/api/src/lib/logger.ts:1-26 — no redact option
  • packages/api/src/router/assistant-tools.ts:742-746 — logger.info({ params }) with password in params
  • packages/api/src/router/assistant-chat-loop.ts:265 — audit entry stores parsedArgs verbatim → password visible to DB admins

Impact

Passwords, API keys, session tokens written cleartext to structured logs and audit DB. If logs ship to SIEM/ELK/Sentry, secrets propagate externally.

Proposed Fix

(1) Add redact: { paths: ['*.password', '*.apiKey', '*.token', '*.secret', '*.authorization', 'params.password', 'req.headers.authorization', 'req.headers.cookie'], censor: '[REDACTED]' } to pino config. (2) Replace verbose params log with whitelisted-keys-only. (3) Per-tool sensitive-field scrub list applied before audit insert.

Acceptance Criteria

  • pino redact config in place, verified by unit test
  • set_user_password tool: log output contains [REDACTED]
  • Audit log row for set_user_password: no plaintext password

Parent Epic: #1
Source: Full-Codebase Security Audit 2026-04-16 (C-11, C-14, A-16)

## Problem `packages/api/src/lib/logger.ts` configures pino without any `redact:` option. `assistant-tools.ts:744` logs `params` verbatim — for tool `set_user_password`, `params.password` is written to logs. Any Authorization/cookie headers similarly at risk. ## Evidence - `packages/api/src/lib/logger.ts:1-26 — no redact option` - `packages/api/src/router/assistant-tools.ts:742-746 — logger.info({ params }) with password in params` - `packages/api/src/router/assistant-chat-loop.ts:265 — audit entry stores parsedArgs verbatim → password visible to DB admins` ## Impact Passwords, API keys, session tokens written cleartext to structured logs and audit DB. If logs ship to SIEM/ELK/Sentry, secrets propagate externally. ## Proposed Fix (1) Add `redact: { paths: ['*.password', '*.apiKey', '*.token', '*.secret', '*.authorization', 'params.password', 'req.headers.authorization', 'req.headers.cookie'], censor: '[REDACTED]' }` to pino config. (2) Replace verbose `params` log with whitelisted-keys-only. (3) Per-tool sensitive-field scrub list applied before audit insert. ## Acceptance Criteria - [ ] pino redact config in place, verified by unit test - [ ] set_user_password tool: log output contains [REDACTED] - [ ] Audit log row for set_user_password: no plaintext password --- Parent Epic: #1 Source: Full-Codebase Security Audit 2026-04-16 (C-11, C-14, A-16)
Hartmut added the security label 2026-04-16 22:05:10 +02:00
Author
Owner

Resolved across two commits, covering both the stdout logger and the DB audit path.

Layer A — pino stdout redact (main commit 534945f, verified at packages/api/src/lib/logger.ts:8-40):
Redact paths configured at logger init:
password, *.password, *.*.password, newPassword, *.newPassword, currentPassword, *.currentPassword, passwordHash, *.passwordHash, token, *.token, *.*.token, accessToken, *.accessToken, refreshToken, *.refreshToken, apiKey, *.apiKey, authorization, *.authorization, cookie, *.cookie, totp, *.totp, totpSecret, *.totpSecret, secret, *.secret, req.headers.authorization, req.headers.cookie, res.headers["set-cookie"]. Censor: [REDACTED].

Layer B — audit DB redact (commit 3d89d7d, packages/api/src/lib/audit.ts):
The AI assistant chat loop persists tool-call parsedArgs into the AuditLog JSONB column at packages/api/src/router/assistant-chat-loop.ts:265 — pino redacts only protect stdout, not DB writes. createAuditEntry now deep-walks before, after, and metadata recursively (max depth 8) and replaces any key whose name (case-insensitive) matches password, newPassword, currentPassword, oldPassword, passwordHash, token, accessToken, refreshToken, sessionToken, apiKey, authorization, cookie, secret, totpSecret, backupCode(s) with [REDACTED] before the DB write. Diff is computed post-redaction so the existing empty-diff skip still fires correctly.

Tests: 12 new cases in packages/api/src/__tests__/audit-redaction.test.ts.

Closing.

Resolved across two commits, covering both the stdout logger and the DB audit path. **Layer A — pino stdout redact** (main commit 534945f, verified at `packages/api/src/lib/logger.ts:8-40`): Redact paths configured at logger init: `password`, `*.password`, `*.*.password`, `newPassword`, `*.newPassword`, `currentPassword`, `*.currentPassword`, `passwordHash`, `*.passwordHash`, `token`, `*.token`, `*.*.token`, `accessToken`, `*.accessToken`, `refreshToken`, `*.refreshToken`, `apiKey`, `*.apiKey`, `authorization`, `*.authorization`, `cookie`, `*.cookie`, `totp`, `*.totp`, `totpSecret`, `*.totpSecret`, `secret`, `*.secret`, `req.headers.authorization`, `req.headers.cookie`, `res.headers["set-cookie"]`. Censor: `[REDACTED]`. **Layer B — audit DB redact** (commit 3d89d7d, `packages/api/src/lib/audit.ts`): The AI assistant chat loop persists tool-call `parsedArgs` into the AuditLog JSONB column at `packages/api/src/router/assistant-chat-loop.ts:265` — pino redacts only protect stdout, not DB writes. `createAuditEntry` now deep-walks `before`, `after`, and `metadata` recursively (max depth 8) and replaces any key whose name (case-insensitive) matches `password`, `newPassword`, `currentPassword`, `oldPassword`, `passwordHash`, `token`, `accessToken`, `refreshToken`, `sessionToken`, `apiKey`, `authorization`, `cookie`, `secret`, `totpSecret`, `backupCode(s)` with `[REDACTED]` before the DB write. Diff is computed post-redaction so the existing empty-diff skip still fires correctly. Tests: 12 new cases in `packages/api/src/__tests__/audit-redaction.test.ts`. Closing.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Hartmut/CapaKraken#46