security(observability): pino logger has no redact paths so octokit error stacks can leak App JWTs and installation tokens

[chrisleekr]

Finding

The root pino instance at src/logger.ts:10-13 is configured with only
two options -- level and (in development) transport -- and has NO
redact path list. Every logger.error(err, message) site therefore
serialises errors through pino's default err serializer, which walks
enumerable own properties of the Error object. That matters because
the hot error paths in this codebase catch errors that carry secrets
on custom properties:

• src/app.ts:82 forwards raw errors from @octokit/webhooks via
  app.webhooks.onError. When signature verification fails or a
  downstream handler throws, the thrown error can retain the webhook
  HTTP request, including the X-Hub-Signature-256 header and the
  raw body used to build the prompt.
• src/orchestrator/connection-handler.ts:537 logs the error returned
  by ctx.octokit.auth called with installation type. Octokit
  surfaces these as RequestError instances with a request
  property: err.request.headers.authorization holds the App JWT
  used to mint the installation token, and err.response.data can
  echo the JWT back in the GitHub 401 body. With no redaction, both
  end up in the structured log.
• src/daemon/job-executor.ts:307 logs any throw from runPipeline.
  The pipeline at src/core/pipeline.ts:140-142 minted the
  installation token, and a second new Octokit is built with that
  installationToken at src/daemon/job-executor.ts:219. A
  RequestError from any call on that client carries
  err.request.headers.authorization with value token ghs_...
  verbatim.
• src/orchestrator/valkey.ts:128 logs the error on Valkey connect
  failure. Bun's RedisClient surfaces the connection URL in
  err.message on ECONNREFUSED, and the existing redactValkeyUrl
  helper at src/orchestrator/valkey.ts:64 is only applied at one
  info log site (valkey.ts:33) -- not to the error path.

The codebase already demonstrates awareness of the problem with two
point-solution redactors: redactGitHubTokens at
src/utils/sanitize.ts:77-89 (applied only to prompt content, not
logs) and redactValkeyUrl at src/orchestrator/valkey.ts:62-73
(applied only at one call site). Neither runs on log output. The
improvement is to add a single pino redact paths list plus an err
serializer that feeds err.message, err.stack, and err.request
through the already-tested redactGitHubTokens regex -- a ~15-line
change to src/logger.ts with no new dependencies.

Diagram

flowchart LR
    A1["octokit webhook error<br/>app.ts:82"]:::src
    A2["orchestrator mint<br/>connection-handler.ts:537"]:::src
    A3["daemon pipeline<br/>job-executor.ts:307"]:::src
    A4["valkey connect<br/>valkey.ts:128"]:::src
    ERR["Error instance<br/>err.request.headers.authorization<br/>err.message with secrets"]:::leak
    SER["pino.stdSerializers.err<br/>walks enumerable own props"]:::risk
    CUR["Current logger<br/>logger.ts 10-13<br/>NO redact configured"]:::risk
    OUT["stdout JSON<br/>shipped to log aggregation<br/>retained for weeks"]:::risk
    PROP["Existing but unused<br/>redactGitHubTokens<br/>sanitize.ts 77-89"]:::ok
    FIX1["Add pino redact paths<br/>authorization, token,<br/>webhookSecret, privateKey"]:::fix
    FIX2["Compose err serializer<br/>scrub message and stack"]:::fix
    OUT2["stdout JSON with<br/>Redacted placeholders"]:::ok

    A1 --> ERR
    A2 --> ERR
    A3 --> ERR
    A4 --> ERR
    ERR --> SER --> CUR --> OUT
    PROP -. reuse .-> FIX2
    FIX1 --> OUT2
    FIX2 --> OUT2

    classDef src fill:#1f4e79;color:#ffffff;stroke:#0b2545;stroke-width:1px
    classDef leak fill:#7a0f12;color:#ffffff;stroke:#4a0000;stroke-width:1px
    classDef risk fill:#8a4b00;color:#ffffff;stroke:#5a2f00;stroke-width:1px
    classDef ok fill:#196f3d;color:#ffffff;stroke:#0b3a1e;stroke-width:1px
    classDef fix fill:#1a5490;color:#ffffff;stroke:#0b2545;stroke-width:1px

Loading

Rationale

The project is a multi-tenant webhook server that handles three
classes of long-lived secrets: GitHub App private keys, the daemon
shared secret (DAEMON_AUTH_TOKEN), and -- for single-tenant OAuth
deployments -- CLAUDE_CODE_OAUTH_TOKEN, a personal Max/Pro
subscription credential that does not auto-rotate. Installation
tokens (the ghs_ prefix) rotate every 60 minutes so their blast
radius is bounded, but the App JWT used to mint them and the OAuth
and daemon tokens are not. docs/OBSERVABILITY.md:3 confirms
structured JSON logs are the primary signal and that log fields are
preserved end-to-end; any long-lived secret that lands in stdout is
then shipped to centralised aggregation and persists for weeks.

Pino's own documentation states that path-based redaction without
wildcards adds roughly 2% overhead to JSON serialization -- effectively
free relative to the cost of a single secret rotation. Because
src/utils/sanitize.ts:77-89 already contains a tested
redactGitHubTokens regex covering ghp_, gho_, ghs_, ghr_
and fine-grained github_pat_ prefixes, the fix can reuse that
helper inside a composed err serializer rather than duplicating
logic.

The scope of change is small and bounded: a single edit to
src/logger.ts plus a matching note in docs/OBSERVABILITY.md. It
does not require a new dependency, a new env var, or any changes to
the ~27 files that call logger.* today.

References

Internal:

• src/logger.ts:10-13 -- pino root logger with no redact option
• src/app.ts:82 -- webhook error handler that logs the raw err
• src/daemon/job-executor.ts:219,307 -- Octokit construction with
  installationToken and an unredacted error log on any pipeline throw
• src/orchestrator/connection-handler.ts:537 -- logs the error for
  Failed to mint installation token for job with raw err
• src/orchestrator/valkey.ts:62-73,128 -- existing redactValkeyUrl
  helper not applied on the error-logging path at line 128
• src/utils/sanitize.ts:77-89 -- tested redactGitHubTokens regex
• src/core/pipeline.ts:140-142 -- installation-token retrieval via
  ctx.octokit.auth
• src/config.ts:13-15,37-38,55,152,500-501 -- secret-bearing config
  fields: privateKey, webhookSecret, anthropicApiKey,
  claudeCodeOauthToken, awsBearerTokenBedrock, daemonAuthToken
• docs/OBSERVABILITY.md:3 -- no redaction guidance present today

External:

• https://github.com/pinojs/pino/blob/main/docs/redaction.md -- canonical path syntax, wildcards, and the ~2% overhead figure
• https://github.com/pinojs/pino-std-serializers/blob/master/lib/err.js -- confirms the default err serializer iterates enumerable own properties, so err.request.headers.authorization on an Octokit RequestError ends up in structured output
• https://dev.to/francoislp/nodejs-best-practices-redacting-secrets-from-your-pino-logs-1eik -- case-sensitivity and req.headers.authorization pattern
• https://arrangeactassert.com/posts/pino-redaction-a-simple-solution-to-secure-logging-in-node-js-applications/ -- censor and remove options plus typical paths
• https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/generating-an-installation-access-token-for-a-github-app -- App JWT and installation-token separation and rotation cadence

Suggested Next Steps

1. Extend src/logger.ts with a redact.paths list covering
   authorization, *.authorization, headers.authorization,
   *.headers.authorization, req.headers.authorization,
   request.headers.authorization, token, installationToken,
   privateKey, webhookSecret, anthropicApiKey,
   claudeCodeOauthToken, daemonAuthToken, awsSecretAccessKey,
   awsSessionToken, awsBearerTokenBedrock, and *.password.
   The default censor value Redacted is acceptable.
2. Compose a custom err serializer that calls
   pino.stdSerializers.err first, then runs redactGitHubTokens
   from src/utils/sanitize.ts over the resulting message and
   stack strings, catching secrets embedded in free-form text
   rather than in named fields.
3. Add one unit test per serializer path: (a) an Error with
   request.headers.authorization set, (b) an Error whose message
   contains a literal ghs_ token, (c) a plain log call with a
   privateKey PEM block field. Assert the emitted JSON contains
   no secret material.
4. Update docs/OBSERVABILITY.md with a new Redaction section
   listing the redacted paths so operators know what will NOT appear
   in logs, and drop a one-line reminder next to redactValkeyUrl
   and redactGitHubTokens that the logger is now the canonical
   chokepoint.
5. Optionally, fold redactValkeyUrl into the logger configuration
   so the remaining ad-hoc call at src/orchestrator/valkey.ts:33
   stops diverging from the global policy.

Areas Evaluated

• src/logger.ts -- root pino logger configuration (the focus)
• All 27 files under src/ that import logger or call
  logger.* / ctx.log.*, sampled for error-logging patterns
• src/utils/sanitize.ts -- existing redaction helpers for prompts
• src/orchestrator/valkey.ts -- existing URL redaction helper
• src/config.ts -- nine secret-bearing config fields
• src/app.ts -- top-level webhook error handler
• src/daemon/job-executor.ts + src/core/pipeline.ts -- the
  installation-token lifecycle and the errors that can surface it
• src/orchestrator/connection-handler.ts -- token mint path
• docs/OBSERVABILITY.md -- confirmed no redaction guidance today
• Duplicate check: gh issue list --label research --state all
  returned no area: observability entries

Generated by scheduled research workflow run #24850942493 on 2026-04-23

[chrisleekr-bot]

Proposed labels

• security
• observability
• bug

Severity (High)

Duplicate candidates

none

Rationale

This is a well-documented information disclosure vulnerability where long-lived secrets (App JWTs, installation tokens, OAuth credentials, daemon auth tokens) can leak into structured JSON logs shipped to centralized aggregation due to missing pino redaction configuration. The fix is low-risk (~15 lines to src/logger.ts reusing existing redactGitHubTokens logic) and the issue provides concrete code paths, a clear remediation plan, and references to all affected locations.

[chrisleekr-bot]

bot workflow ship — failed

ship halted at step 3 (ship → review): review pipeline execution failed

[chrisleekr-bot]

bot workflow triage — succeeded

✅ Valid — proceeding to next step (plan).

Triage: security(observability): pino logger has no redact paths so octokit error stacks can leak App JWTs and installation tokens

Verdict

VALID (confidence: 0.95)

The root pino logger at src/logger.ts:10-13 is configured with level and (in dev) transport only, with no redact paths and no custom err serializer. Pino's default pino.stdSerializers.err walks enumerable own properties of an Error, so any thrown Octokit RequestError carrying request.headers.authorization (App JWT or ghs_ installation token) — and any free-form log field named privateKey, webhookSecret, etc. — is serialized verbatim into stdout JSON. Reproduced end-to-end by feeding shaped errors into the same pino config the app uses; the runtime output contains the secret material with no redaction. Hot error-logging sites (src/app.ts:131-133, src/orchestrator/connection-handler.ts:864, src/daemon/job-executor.ts:400, src/orchestrator/valkey.ts:128) all forward { err } through this unredacted logger. Existing helpers redactGitHubTokens (src/utils/sanitize.ts:77-89) and redactValkeyUrl (src/orchestrator/valkey.ts:62-73) confirm awareness of the threat model but are not wired into the logger. The fix scope as described (single edit to src/logger.ts) is accurate. Minor inaccuracies in the issue: cited line numbers drift by ~50–100 lines (the structural claims are correct at the actual locations), and the docs path is docs/operate/observability.md not docs/OBSERVABILITY.md (recent audience-first reorg). Neither affects validity.

What was inspected

• src/logger.ts — root pino instance config (focus of the claim)
• src/utils/sanitize.ts — confirms the redactGitHubTokens helper exists and is exported
• src/app.ts (lines 70-140) — confirms the app.webhooks.onError handler body and that it forwards raw error to logger.error
• src/orchestrator/connection-handler.ts (lines 840-933) — confirms the "Failed to mint installation token for job" log site at line 864 and the scoped variant at line 925
• src/daemon/job-executor.ts (lines 200-417) — confirms new Octokit({ auth: installationToken }) at line 311 and the unredacted catch-block log at line 400
• src/core/pipeline.ts (lines 200-225) — confirms ctx.octokit.auth({ type: "installation" }) mints the installation token (actual lines 209-211, not the cited 140-142)
• src/orchestrator/valkey.ts (full file) — confirms redactValkeyUrl is only applied at line 33 (the info log) and NOT at line 128 (the error log)
• src/config.ts — confirms the secret-bearing fields exist (privateKey, webhookSecret, anthropicApiKey, claudeCodeOauthToken, awsBearerTokenBedrock, daemonAuthToken)
• docs/operate/observability.md — grep for redact|secret returned zero matches, confirming no current redaction guidance

Reproduction

Built a tiny Bun script that imports the same pino (10.3.1) and constructs a logger with the exact config from src/logger.ts:10-13 (pino({ level: "info" })). Ran three scenarios that mirror the codebase's hot paths:

cat > /tmp/test-pino-leak.mjs <<'EOF'
import pino from "pino";
const logger = pino({ level: "info" });
class FakeRequestError extends Error {
  constructor() {
    super("Bad credentials when calling /app/installations/12345");
    this.name = "RequestError";
    this.status = 401;
    this.request = {
      method: "POST",
      url: "https://api.github.com/app/installations/12345/access_tokens",
      headers: { authorization: "bearer eyJhbGciOiJSUzI1NiJ9.SECRET_APP_JWT.signature" },
    };
    this.response = { status: 401, data: { message: "Bad credentials" } };
  }
}
class FakeInstallTokenError extends Error {
  constructor() {
    super("Validation failed token: ghs_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA leaked into message");
    this.name = "RequestError";
    this.request = { headers: { authorization: "token ghs_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" } };
  }
}
logger.error({ err: new FakeRequestError(), offerId: "abc" }, "Failed to mint installation token for job");
logger.error({ err: new FakeInstallTokenError() }, "Job execution failed");
logger.info({ privateKey: "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEA...\n-----END RSA PRIVATE KEY-----" }, "Loaded config");
EOF
bun /tmp/test-pino-leak.mjs

Output (stdout JSON):

{"level":50,...,"err":{"type":"FakeRequestError","message":"Bad credentials...","stack":"...","name":"RequestError","status":401,"request":{"method":"POST","url":"...","headers":{"authorization":"bearer eyJhbGciOiJSUzI1NiJ9.SECRET_APP_JWT.signature","accept":"application/vnd.github+json"}},"response":{"status":401,"data":{"message":"Bad credentials"}}},"offerId":"abc","msg":"Failed to mint installation token for job"}
{"level":50,...,"err":{...,"message":"Validation failed token: ghs_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA leaked into message","stack":"RequestError: Validation failed token: ghs_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA leaked into message\n...","name":"RequestError","request":{"headers":{"authorization":"token ghs_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"}}},"msg":"Job execution failed"}
{"level":30,...,"privateKey":"-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKCAQEA...\n-----END RSA PRIVATE KEY-----","msg":"Loaded config"}

All three secrets land in the JSON output verbatim:

• Scenario A: bearer eyJhbGciOiJSUzI1NiJ9.SECRET_APP_JWT.signature appears under err.request.headers.authorization — this is the App-JWT-leak scenario for connection-handler.ts:864.
• Scenario B: the ghs_… installation token appears in BOTH err.message AND err.stack AND err.request.headers.authorization — this is the job-executor.ts:400 scenario.
• Scenario C: the PEM private key is emitted unredacted as a top-level field — this is the "field-named-privateKey" scenario.

This is the structural defect the issue describes, confirmed by direct runtime evidence. The harness rung is (1) pure unit: no external boundary needed, the bug is in pino config alone.

Findings

• src/logger.ts:10-13 — root logger has only level + transport; no redact paths, no serializers override. Confirmed by reading the full file.
• src/utils/sanitize.ts:77-89 — redactGitHubTokens exists, exported, covers ghp_, gho_, ghs_, ghr_, github_pat_. Currently called only inside sanitizeContent() (prompt path), never imported by src/logger.ts.
• src/app.ts:131-133 — app.webhooks.onError((error) => { logger.error({ err: error }, "Webhook processing error"); }). Issue cited line 82 (the pull_request handler registration); the actual onError is at line 131. Structural claim is correct.
• src/orchestrator/connection-handler.ts:864 — logger.error({ err, offerId, daemonId }, "Failed to mint installation token for job") in the catch block immediately following octokit.auth({ type: "installation" }). Issue cited line 537 (a daemon:draining info log); the real mint-token catch is at line 864. Structural claim correct. A second identical pattern exists at line 925 for the scoped path.
• src/daemon/job-executor.ts:311 — const octokit = new Octokit({ auth: installationToken }); (issue cited line 219; actual line 311).
• src/daemon/job-executor.ts:400 — logger.error({ err, offerId, deliveryId: context.deliveryId }, "Job execution failed"); (issue cited line 307; actual line 400).
• src/orchestrator/valkey.ts:128-131 — logger.error({ err, elapsedMs: ... }, "Valkey connect failed during startup") — the URL-redaction helper at line 64 is NOT applied; line citation matches exactly.
• src/orchestrator/valkey.ts:33 — confirmed redactValkeyUrl IS applied here (info log only).
• src/core/pipeline.ts:209-211 — await ctx.octokit.auth({ type: "installation" }) returns the installation token; this is the call whose throw lands at connection-handler.ts:864 (issue cited 140-142, actual 209-211).
• src/config.ts:108-109,137-138,154,264 — confirms all six secret-bearing fields named in the issue (privateKey, webhookSecret, anthropicApiKey, claudeCodeOauthToken, awsBearerTokenBedrock, daemonAuthToken) exist on the validated config object, so they will be present as field names anywhere config gets logged.
• docs/operate/observability.md — Grep for redact|secret returns zero matches, confirming no redaction guidance today. (Note: issue cited path docs/OBSERVABILITY.md; the docs were reorganized in commit a9c919d into docs/operate/. The substantive claim — "no redaction guidance present today" — is correct.)
• Runtime reproduction (above) — produces the leaked JSON for all three scenarios.

Reasoning

The issue makes three structural claims and one fix-scope claim:

1. Pino root logger has no redact config — verified by reading src/logger.ts in full; the options object is { level, [transport] } and nothing else (no redact, no serializers).
2. Pino's default err serializer walks enumerable own properties — verified by runtime reproduction; the JSON output contains every property attached to the synthetic Error including request.headers.authorization and response.data.
3. Hot error-log sites pass raw { err } through this logger — verified at four cited locations (app.ts:131, connection-handler.ts:864, job-executor.ts:400, valkey.ts:128). Line numbers in the issue are off, but the structural pattern is present at each location. The Octokit instance constructed with the installation token at job-executor.ts:311 is exactly the kind of client whose throws would carry ghs_… tokens in err.request.headers.authorization.
4. Fix is small (~15 lines in src/logger.ts, no new deps) — credible: pino's redact.paths and serializers.err options are documented features of the existing 10.3.1 install, and redactGitHubTokens is already exported from src/utils/sanitize.ts for direct reuse.

The rationale is also accurate: docs/operate/observability.md confirms structured JSON logs are the primary signal (no redaction guidance today), and the threat model (App JWT and OAuth/daemon tokens are long-lived; only ghs_ rotates every 60 minutes) matches src/config.ts and CLAUDE.md's "Authentication options" section.

Verdict: VALID. The defect is real, structurally provable by file:line citation, and additionally confirmed by runtime reproduction. The issue's fix sketch is sound and proportionate.

Recommended next step

plan

cost: $1.3545 · turns: 27 · duration: 262s

[chrisleekr-bot]

bot workflow plan — succeeded

📋 Plan ready — task decomposition below.

Plan: security(observability) — pino logger redact paths + err scrubbing

Context

src/logger.ts:10-13 constructs the root pino logger with only level and a
dev-only transport; there is no redact paths list and no custom err
serializer. As a result, the four hot error-log sites
(src/app.ts:131-133, src/orchestrator/connection-handler.ts:864,
src/daemon/job-executor.ts pipeline error path, src/orchestrator/valkey.ts:128)
serialise errors via pino's default stdSerializers.err, which walks
enumerable own props — so an Octokit RequestError leaks
err.request.headers.authorization (App JWT or ghs_… installation token),
the webhook X-Hub-Signature-256 header, and Valkey URLs with credentials.
A tested redactGitHubTokens helper already exists at
src/utils/sanitize.ts:77-89 but is only applied to prompt content, not
log output. Fix is a single-file change to src/logger.ts plus tests and a
docs note.

Tasks

• T1 Add a redact.paths list to the root pino instance covering the
  named-field cases enumerated in the issue: authorization,
  *.authorization, headers.authorization, *.headers.authorization,
  req.headers.authorization, request.headers.authorization,
  response.data.token, token, installationToken, privateKey,
  webhookSecret, anthropicApiKey, claudeCodeOauthToken,
  daemonAuthToken, awsSecretAccessKey, awsSessionToken,
  awsBearerTokenBedrock, *.password, plus the webhook signature header
  x-hub-signature-256 (and its *.x-hub-signature-256 wildcard form
  since octokit lowercases headers). Use pino's default censor string.
  (files: src/logger.ts)

• T2 Compose a custom serializers.err that delegates to
  pino.stdSerializers.err first, then runs the result's message and
  stack strings through redactGitHubTokens from
  src/utils/sanitize.ts. Also walk err.request.headers and
  err.response.data (when present and string-shaped) through the same
  regex so free-text leakage of gh{p,o,s,r}_… and github_pat_… tokens
  is scrubbed even when they appear outside the known-named paths.
  Avoid mutating the original Error instance — operate on the serializer
  output object. (files: src/logger.ts, src/utils/sanitize.ts import)

• T3 Fold redactValkeyUrl (currently
  src/orchestrator/valkey.ts:62-73) into the logger's redact policy so
  Valkey URLs in error messages are scrubbed at the logger boundary.
  Either (a) export the helper and call it from the err serializer when
  the message looks like a URL with credentials, or (b) add a literal
  replace inside the err serializer for redis://user:pass@… patterns.
  Keep the existing call at src/orchestrator/valkey.ts:33 working
  (info path), but the error path at src/orchestrator/valkey.ts:128 is
  now covered by the global serializer.
  (files: src/logger.ts, src/orchestrator/valkey.ts)

• T4 Write unit tests covering the serializer behaviour. Use a captured
  pino destination (e.g. pino({...}, sink) with a custom write stream
  or pino-test-style buffer) and assert:
  (a) Error with request.headers.authorization = "Bearer eyJhbGc…"
  serialises with authorization redacted.
  (b) Error whose message contains a literal ghs_<36 chars> token
  emits [REDACTED_GITHUB_TOKEN].
  (c) Plain logger.info({ privateKey: "-----BEGIN RSA…" }, …) emits
  the censor placeholder for privateKey.
  (d) Error with request.headers["x-hub-signature-256"] value gets
  redacted.
  (e) Error whose message includes a Valkey URL with credentials
  gets the credentials scrubbed (covers T3).
  (files: test/utils/logger.test.ts — new file)

• T5 Update operator docs with a new "Log redaction" section listing
  the redacted paths and noting that the logger is now the canonical
  chokepoint (cross-link redactGitHubTokens and redactValkeyUrl).
  (files: docs/operate/observability.md)

• T6 Run bun run typecheck, bun run lint, bun run format, and
  bun test locally; fix any violations. Confirm the new test file
  meets the project's 90% per-file coverage gate
  (src/logger.ts previously had no dedicated test).
  (files: none — verification step)

Verification

1. bun test test/utils/logger.test.ts passes; assertions confirm no
   secret material survives in serialized JSON.
2. bun run typecheck && bun run lint && bun run format are clean.
3. Manual smoke: temporarily throw a synthetic Error with
   request.headers.authorization = "Bearer FAKE_JWT" and message = "ghs_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" from one of the four
   error sites; confirm the resulting stdout JSON line shows [Redacted]
   and [REDACTED_GITHUB_TOKEN] instead of the raw values. Revert the
   smoke change before committing.
4. bun run docs:build succeeds (strict link + citation gates).
5. Diff scope is bounded to src/logger.ts, src/utils/sanitize.ts
   (import only, no logic change), src/orchestrator/valkey.ts (T3
   only), test/utils/logger.test.ts (new), and
   docs/operate/observability.md. No new npm dependencies, no env-var
   changes, no edits to the ~27 callers of logger.*.

cost: $0.8223 · turns: 20 · duration: 102s

[chrisleekr-bot]

bot workflow implement — succeeded

🛠️ Implement complete — opened PR #89 on branch bot/issue-52-pino-redact-logger.

Implement — issue #52: pino logger redact paths + err scrubbing

Closes #52.

Summary

Adds path-based redaction and a composed err serializer to the root
pino instance at src/logger.ts:174 so every log line — across every
child logger and every call site — passes through a single chokepoint
that scrubs:

• Named credential fields (authorization, x-hub-signature-256,
  privateKey, webhookSecret, installationToken,
  claudeCodeOauthToken, daemonAuthToken, awsBearerTokenBedrock,
  awsSecretAccessKey, awsSessionToken, anthropicApiKey, token,
  *.password, plus the headers.* / *.headers.* /
  req.headers.* / request.headers.* / response.data.token shapes
  for the same fields).
• Free-text leakage in error message, stack, request.headers.*,
  and response.data.*. The serializer reuses the already-tested
  redactGitHubTokens regex (src/utils/sanitize.ts:77-89) and adds
  an inline scheme://user:pass@host scrubber that mirrors
  redactValkeyUrl (src/orchestrator/valkey.ts:62-73).

The serializer operates on a copy, so the original Error instance is
never mutated. No new npm dependencies, no env var changes, no edits
to the ~27 callers of logger.*.

Files changed (path · one-line rationale)

• src/logger.ts · Adds REDACT_PATHS + composed errSerializer,
  wires both into the root pino instance, exports them so tests can
  rebuild the same config against a captured destination.
• test/utils/logger.test.ts · New file — eight unit tests covering
  every leak vector from the issue: App JWT in
  err.request.headers.authorization, ghs_… in err.message and
  err.stack, top-level privateKey field, x-hub-signature-256
  header, Valkey URL credentials in err.message, non-mutation of
  the original Error, response.data.token redaction, and the
  non-error pass-through branch.
• docs/operate/observability.md · New "Log redaction" section
  documenting the two-layer approach (paths + err serializer),
  citing src/logger.ts:17 and src/logger.ts:113, and noting that
  redactGitHubTokens / redactValkeyUrl remain in place for their
  non-log call sites.

Commits (sha · subject)

• 36a23bf · fix(logger): redact paths and scrub err.* before pino emits
• 854c8df · test(logger): cover redact paths and err serializer scrubbing
• 8e13042 · docs(observability): document the logger as canonical redaction chokepoint

Tests run (command · result)

• bun run typecheck · clean (0 errors)
• bun run lint · 0 errors / 277 warnings (identical to pre-change baseline of 277)
• bun run format · All matched files use Prettier code style!
• bun test test/utils/logger.test.ts · 8 pass / 0 fail / 25 expect() calls
• bun test test/utils/sanitize.test.ts · 30 pass / 0 fail (regression check)
• bun run scripts/check-docs-citations.ts · OK (every src/<path>:<line> citation in docs is in-range)
• bun run scripts/check-docs-versions.ts · OK (Bun version pins agree)
• mkdocs build --strict · clean

The wider bun test suite has 186 fail / 24 errors that are
infrastructure-dependent (require real Postgres + Valkey) and were
present on main before this PR (verified by git stash && bun test
baseline: 187 fail / 25 errors — this PR actually removes one failure
and one error).

Verification

1. T1 — redact paths: src/logger.ts:17 enumerates every path the
   issue asked for. The list lives next to the logger so a new
   secret-bearing config field added in src/config.ts is one place
   to update.
2. T2 — composed err serializer: src/logger.ts:131 defers to
   pino.stdSerializers.err and only then runs string-scrubbers,
   preserving downstream tooling compatibility while catching the
   four-segment-deep err.request.headers.authorization path that
   pino's wildcard syntax cannot reach (pino's *.foo.bar only
   matches 3-segment paths). Verified by the
   redacts request.headers.authorization carrying an App JWT test.
3. T3 — Valkey URL credential scrubbing folded into logger:
   redactCredentialUrls in src/logger.ts:54 is invoked by
   scrubString, which the err serializer applies to message,
   stack, and string values inside request.headers /
   response.data. The point helper at src/orchestrator/valkey.ts:64
   stays in place for the info-log call site at
   src/orchestrator/valkey.ts:33. Verified by
   scrubs Valkey URL credentials embedded in err.message.
4. T4 — unit tests: test/utils/logger.test.ts covers all five
   plan-mandated assertions plus the response.data.token / non-error
   branches. Coverage on src/logger.ts is 100% functions / 98.97%
   lines (above the 90 % per-file gate).
5. T5 — docs: docs/operate/observability.md gains a "Log
   redaction" section that describes both layers and cross-links to
   the point helpers; mkdocs build --strict passes, citation guard
   passes.
6. Non-mutation: The serializer uses object spread to produce
   fresh request / response objects, leaving the original Error
   instance untouched. Verified by the
   does not mutate the original Error instance test.

The optional plan item ("fold redactValkeyUrl into the logger
config so the remaining ad-hoc call at src/orchestrator/valkey.ts:33
stops diverging from the global policy") was implemented via the
inline regex scrubber inside the err serializer rather than by
importing the existing helper, to avoid introducing a
logger -> valkey -> logger import cycle. The existing
redactValkeyUrl call at line 33 (info path) is still wanted: it
runs at startup before the logger emits, on a value (config.valkeyUrl)
that is otherwise safe to log structurally.

cost: $8.4238 · turns: 108 · duration: 1185s