Skip to content

[#139] Redact secrets from logs and the diagnostics export - #140

Merged
julien731 merged 6 commits into
mainfrom
feature/139-redact-secrets-from-logs
Sep 3, 2026
Merged

julien731 merged 6 commits into
mainfrom
feature/139-redact-secrets-from-logs

Conversation

@julien731

Copy link
Copy Markdown
Member

Closes #139

Summary

Defense-in-depth follow-up to #137 / PR #138. Masks secret-shaped tokens (at minimum HuggingFace hf_[A-Za-z0-9]+ → hf_***) before they are written to any log bundled into the shareable Export Diagnostics… zip, so a token embedded in third-party output (e.g. surfaced in a dependency's traceback) cannot leak.

Three sinks are covered:

  • service.log (Python) — a SecretRedactingFilter on the rotating file handler redacts the formatted message, exception/traceback text, and stack info.
  • service-stderr.log (Swift) — the raw child stderr/stdout tee redacts each chunk before writing.
  • app.log (Swift) — lifecycle lines are redacted too, as it is bundled into the export.

Approach

  • Python: the filter is attached to the RotatingFileHandler, not the root logger — a logger-level filter only runs for records logged directly on it and would miss records propagated from child loggers (uvicorn). The traceback is pre-rendered and stashed in record.exc_text so the formatter reuses the redacted text instead of re-rendering raw exc_info. The filter fails open and drops args on a malformed-format record, so redaction can never raise back at the log call site.
  • Swift: a shared SecretRedaction.redact(_:) mirrors the Python pattern list. Redaction happens at the single tee write site (covering both stdout-post-handshake and stderr). Per-chunk redaction carries one documented limitation: a token split across two availableData reads could evade masking — low risk, since tracebacks arrive as a burst.
  • Pattern lists live in one place per runtime (_SECRET_PATTERNS / SecretRedaction), cross-referenced, and both exercised with the shared hf_TESTTOKEN0123456789abcdef vector so drift surfaces.
  • Redaction is forward-only; existing/rotated on-disk logs are not rewritten (our own code never logged tokens historically).

Plan: docs/plans/139-redact-secrets-from-logs.md. Architect plan review and code review both passed.

Verification

  • ruff check . and ruff format --check .: pass.
  • pytest: 393 passed — new on-disk tests confirm a message token and an exception-traceback token land as hf_***, plus a fail-open unit for malformed records.
  • swift build + swift run MeetingTranscriberKitTests: 270 passed — SecretRedaction units, a tee-writes-redacted-to-disk check, and an end-to-end AC5 test that exports the diagnostics zip, extracts it, and asserts the seeded token is absent while hf_*** is present.

@julien731 julien731 added the feature New feature or enhancement label Sep 3, 2026
@julien731 julien731 self-assigned this Sep 3, 2026
@julien731

Copy link
Copy Markdown
Member Author

QA Confidence Verdict — Story #139 (redact secrets from logs & diagnostics export)

Verified by running both suites and inspecting the production wiring. No web UI is in scope for this story (native macOS app + Python service), so Playwright was not applicable; verification is test-execution + code inspection.

  • Python: .venv/bin/python -m pytest tests/unit/test_logging_setup.py -q → 12 passed
  • Swift: swift run MeetingTranscriberKitTests → 270 passed, 0 failed

Per-AC verdict

AC1 — redacting filter in configure_service_logging() masking hf_[A-Za-z0-9]+ → hf_***: PASS
SecretRedactingFilter is added to the rotating file handler installed on the root logger (logging_setup.py:121). Pattern + placeholder correct (_SECRET_PATTERNS). Minor nuance: the filter lives on the handler, not literally on the root logger object — intentional and documented, because a logger-level filter skips records propagated up from child loggers (uvicorn). A handler filter runs for every emitted record, so it correctly covers the root logger's output. Meets intent.

AC2 — covers formatted message + exception/traceback text: PASS
Filter redacts record.msg (after resolving args), record.exc_text (pre-rendering the traceback so the formatter reuses the redacted text), and record.stack_info. test_exception_traceback_token_is_redacted_on_disk asserts the token is gone, hf_*** present, and Traceback still present (redaction, not omission).

AC3 — raw Swift stderr tee (service-stderr.log) redacted: PASS (impl) / test gap
ServiceSupervisor.drainAndTee runs SecretRedaction.redact on each chunk before FileLog.write (ServiceSupervisor.swift:176-181); app.log is redacted in AppLog.line. Production wiring confirmed (AppState.swift:28,37,52). Documented boundary limitation: a token split across two availableData reads could evade masking — acceptable for defense-in-depth, worth noting.
Gap: no automated test drives the tee seam (drainAndTee → redact) with a real token. SecretRedactionTests and DiagnosticsExporterTests both call SecretRedaction.redact(...) in the test before writing, so they exercise redact() + FileLog.write independently, not the supervisor's tee. Integration scenario 5 floods 200KB through the tee but with no token. AC3 currently rests on code inspection for the integration point.

AC4 — unit test: on-disk log contains placeholder not token: PASS (genuine, not tautological)
test_message_token_is_redacted_on_disk / ..._exception_... call the real configure_service_logging, log through the installed handler, then read the on-disk service.log. End-to-end through the production filter. Solid.

AC5 — grep exported zip for seeded token, confirm absent: PASS (partially tautological)
DiagnosticsExporterTests seeds service-stderr.log, exports, extracts with ditto, greps: asserts raw token absent and hf_*** present. Valid round-trip proof that the exporter bundles + zips without leaking already-redacted content. Caveat: the seed is redacted by the test before being written, so it proves the exporter/zip path preserves redaction — not that the production tee produced it. Combined with the AC3 finding, the "token never reaches the zip in production" claim is not fully closed by an automated test.

Extensibility (story note): PASS

Pattern list centralized per runtime (_SECRET_PATTERNS in Python, SecretRedaction.patterns in Swift). The two lists are manually kept in sync via a shared hf_TESTTOKEN0123456789abcdef test vector — a divergence in one runtime's pattern surfaces in that runtime's test, but a missing new pattern on one side would not auto-fail. Documented in both files.

What needs human eyes

  • Whether the AC3/AC5 gap (no test exercising the real drainAndTee → redact → service-stderr.log path with a token) is acceptable, or warrants one integration test: child prints hf_... to stderr → assert on-disk service-stderr.log contains hf_***.
  • Redaction scope: only HF tokens today (AC minimum). Confirm no other secret shapes (bearer tokens, API keys) are expected in service output for this story.

Risk areas

  • Chunk-boundary token split in the tee (documented, low probability).
  • Cross-runtime pattern drift relies on manual sync + per-runtime vectors.

Suggested QA focus

Quick glance. Core behavior is genuinely tested on the Python side (AC1/AC2/AC4). The residual is the Swift tee integration seam (AC3) and the pre-redacted seed in the AC5 exporter test — both low-risk given code inspection, but the tee test gap is the one thing to decide on before closing.

@julien731
julien731 merged commit 69a915d into main Sep 3, 2026
4 checks passed
@julien731
julien731 deleted the feature/139-redact-secrets-from-logs branch September 3, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redact secrets from logs and the diagnostics export

1 participant