Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),

## [Unreleased]

### Security

- Redact secret-shaped tokens (at minimum HuggingFace `hf_...` tokens) from `service.log`, the raw child `service-stderr.log` tee, and `app.log` before they are written, so the shareable **Export Diagnostics…** bundle cannot leak a credential embedded in third-party output (e.g. a token surfaced in a dependency's traceback).

## [1.2.0] - 2026-05-08

### Added
Expand Down
57 changes: 57 additions & 0 deletions backend/services/logging_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from __future__ import annotations

import logging
import re
from logging.handlers import RotatingFileHandler
from pathlib import Path

Expand All @@ -28,6 +29,61 @@
# transcription pipeline spawns workers that re-exec this binary).
_HANDLER_MARKER = "_mt_service_log_handler"

# Secret-shaped tokens to mask before anything reaches disk (story #139). Our own
# code never logs the HuggingFace token, but a third-party dependency could embed
# it in a traceback (e.g. a 401 body), which would then land in service.log and
# the user-shareable diagnostics zip. Keep every pattern in this one list so more
# credential shapes can be added later. The Swift side mirrors this list in
# macos/Sources/MeetingTranscriberKit/Logging/SecretRedaction.swift — keep them
# in sync (both are exercised with the shared `hf_TESTTOKEN...` test vector).
_SECRET_PATTERNS: list[tuple[re.Pattern[str], str]] = [
(re.compile(r"hf_[A-Za-z0-9]+"), "hf_***"),
]


def redact_secrets(text: str) -> str:
"""Mask every known secret shape in ``text``. Safe on arbitrary strings."""
for pattern, replacement in _SECRET_PATTERNS:
text = pattern.sub(replacement, text)
return text


class SecretRedactingFilter(logging.Filter):
"""Mask secrets in a record's message, traceback, and stack info.

Installed on the file handler (not the root logger): a filter added to a
logger only runs for records logged *directly* on it, so records propagated
up from child loggers — notably ``uvicorn`` — would bypass it. A handler
filter runs for every record the handler emits.

The traceback is pre-rendered here and stashed in ``record.exc_text`` so the
handler's formatter reuses the redacted text instead of re-rendering the raw
``exc_info``. Fails open: any formatting error leaves the record as-is rather
than raising back at the log call site.
"""

def filter(self, record: logging.LogRecord) -> bool:
try:
if record.args:
try:
record.msg = record.getMessage()
except Exception: # noqa: BLE001
# Malformed format record (e.g. mismatched %-args): keep the
# raw template but drop args so the formatter cannot raise.
record.msg = str(record.msg)
record.args = None
if isinstance(record.msg, str):
record.msg = redact_secrets(record.msg)
if record.exc_info and not record.exc_text:
record.exc_text = logging.Formatter().formatException(record.exc_info)
if record.exc_text:
record.exc_text = redact_secrets(record.exc_text)
if record.stack_info:
record.stack_info = redact_secrets(record.stack_info)
except Exception: # noqa: BLE001 — redaction must never break logging.
pass
return True


def _already_installed(root: logging.Logger, log_file: Path) -> bool:
for handler in root.handlers:
Expand Down Expand Up @@ -62,6 +118,7 @@ def configure_service_logging(log_dir: Path | None = None) -> Path:
)
handler.setLevel(logging.INFO)
handler.setFormatter(logging.Formatter(_LOG_FORMAT))
handler.addFilter(SecretRedactingFilter())
setattr(handler, _HANDLER_MARKER, True)
root.addHandler(handler)
return log_file
70 changes: 70 additions & 0 deletions docs/plans/139-redact-secrets-from-logs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Plan: Redact secrets from logs and the diagnostics export

**Story**: #139
**Spec**: N/A (follow-up to #137 / PR #138)
**Branch**: feature/139-redact-secrets-from-logs
**Date**: 2026-09-03
**Mode**: TDD — both redactors are pure functions with clear input/output.

## Technical Decisions

### TD-1: Attach the redacting filter to the handler, not the root logger
- **Context**: AC1 asks for a filter "installed on the root logger". A `logging.Filter` added via `Logger.addFilter` only runs for records logged *directly* on that logger — records propagated up from child loggers (notably `uvicorn`/`uvicorn.access`) skip it.
- **Decision**: Add the filter to the `RotatingFileHandler` (which lives on the root logger). Handler-level filters run for every record the handler emits, including propagated ones.
- **Alternatives considered**: `root.addFilter(...)` — rejected, misses propagated records; a `RedactingFormatter` subclass — works but AC explicitly asks for a filter, and a filter keeps message/traceback handling in one place.

### TD-2: Pre-render redacted traceback into `record.exc_text`
- **Context**: A filter runs before the Formatter renders `record.exc_info`, so redacting `record.msg` alone leaves the traceback unmasked.
- **Decision**: In the filter, format the exception via `logging.Formatter().formatException(record.exc_info)`, redact it, and assign to `record.exc_text`. `Formatter.format` reuses a non-empty `exc_text` instead of re-rendering, so the on-disk traceback is redacted.
- **Alternatives considered**: clearing `exc_info` — rejected, loses structured info other handlers may want.

### TD-3: Fail-open filter
- **Context**: `record.getMessage()` / `formatException()` can raise (e.g. mismatched `%` args). Filters run before the handler's emit try/except, so a raise escapes to the log call site.
- **Decision**: Wrap the redaction work in try/except and return `True` on failure — never drop or raise from the redaction filter.

### TD-4: Per-chunk Swift tee redaction
- **Context**: The raw child stderr tee (`service-stderr.log`) bypasses Python logging entirely. For stderr that never passes through Python (uncaught interpreter tracebacks, C-level/third-party direct prints), the Swift tee is the *sole* redaction sink, not a backstop.
- **Decision**: Redact each `availableData` chunk at the single tee write site. Accepted limitation: a token split across two reads could evade masking (low risk — tracebacks arrive as a burst).
- **Alternatives considered**: newline-buffered tee with EOF flush — more robust but adds a second buffer to a concurrency-sensitive drain loop; deferred for simplicity.

## Files to Create or Modify

- `backend/services/logging_setup.py` — add `_SECRET_PATTERNS`, `redact_secrets()`, `SecretRedactingFilter`; attach filter to the handler.
- `tests/unit/test_logging_setup.py` — message/traceback on-disk redaction, fail-open, `redact_secrets` unit.
- `macos/Sources/MeetingTranscriberKit/Logging/SecretRedaction.swift` (new) — `SecretRedaction.redact(_:)`.
- `macos/Sources/MeetingTranscriberKit/Service/ServiceSupervisor.swift` — redact chunk before teeing.
- `macos/Sources/MeetingTranscriberKit/Logging/AppLog.swift` — redact lines before writing (app.log is bundled too).
- `macos/Sources/MeetingTranscriberKitTests/SecretRedactionTests.swift` (new) + `main.swift` registration.
- `CHANGELOG.md` — Unreleased entry.

## Approach per AC

### AC1/AC2: Redacting filter on the root logger covering message + traceback
`SecretRedactingFilter` attached to the `RotatingFileHandler`; redacts `record.msg` (args collapsed via `getMessage()`), `record.exc_text` (pre-rendered), and `record.stack_info`.

### AC3: Raw stderr tee redacted Swift-side
`SecretRedaction.redact` applied at the tee write site in `drainAndTee`; `AppLog` writes also routed through it.

### AC4: Unit test on-disk
Log a record/exception containing `hf_TESTTOKEN`; assert the on-disk `service.log` contains `hf_***`, not the token.

### AC5: Diagnostics zip free of the token
Python on-disk test as automated coverage; documented manual check in the PR (seed token, export, `unzip -p | grep`). Shared `hf_TESTTOKEN` vector in Python and Swift tests to surface pattern drift.

## Commit Sequence

1. Python redacting filter + tests
2. Swift redactor + tee/AppLog redaction + tests
3. CHANGELOG + plan doc

## Risks and Trade-offs

- Per-chunk Swift redaction: token split across reads could evade masking (documented, low risk).
- Two pattern lists (Python + Swift) can drift; mitigated by cross-reference comments and a shared test token.
- Redaction is forward-only — existing/rotated on-disk log content is not rewritten (our code never logged tokens historically).
- app.log carries only Swift-authored lifecycle strings, but is routed through the redactor anyway as near-free defense-in-depth.

## Deviations from Plan

- The fail-open test (`test_filter_neutralizes_malformed_format_record`) exercises `SecretRedactingFilter.filter()` directly rather than through the logging pipeline: pytest's own capture handler re-raises a malformed-`%`-args record independent of our filter, so a full-pipeline test could not isolate our filter's behavior. The filter now also drops `record.args` when `getMessage()` raises, so no downstream formatter can raise either.
- The Swift redactor also covers `app.log` (via `AppLog.line`), not only the `service-stderr.log` tee — near-free defense-in-depth for the third bundled sink.
46 changes: 46 additions & 0 deletions macos/Sources/MeetingTranscriberIntegrationTests/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,57 @@ func scenarioPostHandshakeFloodDoesNotBlock() {
check(!supervisor.isRunning, "flood: child terminated")
}

// MARK: - Scenario 6: secrets in child stderr are redacted in the tee (story #139)

func scenarioStderrTokenIsRedacted() {
// The child prints an hf_ token to stderr (as a third-party dep might in a
// traceback). The supervisor's tee must write the placeholder, not the token,
// to service-stderr.log — the file bundled into the diagnostics export. This
// drives the real drainAndTee → SecretRedaction seam end-to-end.
let tmp = FileManager.default.temporaryDirectory
.appendingPathComponent("supervisor-redact-\(UUID().uuidString)", isDirectory: true)
try? FileManager.default.createDirectory(at: tmp, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: tmp) }

let token = "hf_TESTTOKEN0123456789abcdef"
let nonce = UUID().uuidString
let code = """
import json, os, sys, time
print(json.dumps({"event": "ready", "port": 45998, "nonce": os.environ.get("MT_SERVICE_NONCE", "")}), flush=True)
sys.stderr.write("401 Unauthorized for \(token)\\n"); sys.stderr.flush()
time.sleep(30)
"""

let stderrLog = FileLog(fileName: "service-stderr.log", directory: tmp)
let supervisor = ServiceSupervisor(stderrLog: stderrLog)
do {
let port = try supervisor.start(python(code, nonce: nonce), timeout: 10)
check(port == 45998, "redact: handshake parsed (got \(port))")

// Give the tee a moment to drain the stderr line, then flush.
let deadline = Date().addingTimeInterval(5)
var body = ""
repeat {
stderrLog.flush()
body = (try? String(contentsOf: stderrLog.url, encoding: .utf8)) ?? ""
if body.contains("hf_***") { break }
Thread.sleep(forTimeInterval: 0.05)
} while Date() < deadline
check(!body.contains(token), "redact: token absent from service-stderr.log")
check(body.contains("hf_***"), "redact: placeholder written to service-stderr.log")
} catch {
check(false, "redact: start threw \(error)")
}
supervisor.terminate(gracePeriod: 2)
check(!supervisor.isRunning, "redact: child terminated")
}

await scenarioStubService()
scenarioNoisyStdout()
scenarioSigkillEscalation()
scenarioNonceMismatch()
scenarioPostHandshakeFloodDoesNotBlock()
scenarioStderrTokenIsRedacted()

let summary = "\n\(passed) passed, \(failed) failed\n"
FileHandle.standardOutput.write(Data(summary.utf8))
Expand Down
6 changes: 5 additions & 1 deletion macos/Sources/MeetingTranscriberKit/Logging/AppLog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public enum AppLog {
}

private static func line(_ level: String, _ message: String) -> String {
"\(ISO8601DateFormatter().string(from: Date())) \(level) app: \(message)"
// app.log is bundled into the diagnostics export; redact as
// defense-in-depth even though these are Swift-authored breadcrumbs that
// never interpolate tokens today (story #139).
let safe = SecretRedaction.redact(message)
return "\(ISO8601DateFormatter().string(from: Date())) \(level) app: \(safe)"
}
}
38 changes: 38 additions & 0 deletions macos/Sources/MeetingTranscriberKit/Logging/SecretRedaction.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Story #139 — redact secrets from logs and the diagnostics export.
// Plan: docs/plans/139-redact-secrets-from-logs.md

import Foundation

/// Masks secret-shaped tokens before text is written to a shareable log file.
///
/// The raw child stderr tee (`service-stderr.log`) and the app's own `app.log`
/// are both bundled into the user-shareable diagnostics zip, and the raw stderr
/// bypasses Python's logging formatter entirely — so for third-party output that
/// never passes through Python (e.g. an uncaught traceback printed by
/// `huggingface_hub`/`pyannote`), this is the *sole* redaction sink, not a
/// backstop. Keep the pattern list in sync with the Python side in
/// `backend/services/logging_setup.py` (`_SECRET_PATTERNS`); both are exercised
/// with the shared `hf_TESTTOKEN...` test vector so a divergence surfaces.
public enum SecretRedaction {
/// (pattern, replacement) pairs. Add more credential shapes here.
private static let patterns: [(NSRegularExpression, String)] = {
let specs = [
(#"hf_[A-Za-z0-9]+"#, "hf_***"),
]
return specs.compactMap { pattern, replacement in
(try? NSRegularExpression(pattern: pattern)).map { ($0, replacement) }
}
}()

/// Return `text` with every known secret shape masked. Safe on any string.
public static func redact(_ text: String) -> String {
var result = text
for (regex, replacement) in patterns {
let range = NSRange(result.startIndex..., in: result)
result = regex.stringByReplacingMatches(
in: result, range: range, withTemplate: replacement
)
}
return result
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,18 @@ public final class ServiceSupervisor {
while true {
let chunk = handle.availableData
if chunk.isEmpty { break } // EOF
if matched || teeBeforeMatch { self?.stderrLog?.write(chunk) }
// Redact secrets before teeing: this raw child output bypasses
// Python's logging redaction, and the file is bundled into the
// shareable diagnostics zip (story #139). This single write site
// serves both the stderr tee and the post-handshake stdout tee.
// Per-chunk redaction accepts one boundary limitation: a token
// split across two availableData reads could evade masking.
if matched || teeBeforeMatch {
let redacted = SecretRedaction.redact(
String(decoding: chunk, as: UTF8.self)
)
self?.stderrLog?.write(Data(redacted.utf8))
}
if matched { continue } // keep draining + teeing, stop parsing
buffer.append(chunk)
while let newline = buffer.firstIndex(of: 0x0A) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,36 @@ func runDiagnosticsExporterTests() {
// Summary carries the identifying header.
expect(DiagnosticsExporter.summary().contains("MeetingTranscriber diagnostics"),
"summary has header")

// AC5 (story #139): a token that passed through the redacting write path
// must be absent from the exported zip. Seed a log the way the stderr tee
// would write it (raw child output run through SecretRedaction.redact),
// export, extract, and grep the extracted tree for the raw token.
let seededToken = "hf_TESTTOKEN0123456789abcdef"
let teed = SecretRedaction.redact("Traceback: 401 Unauthorized for \(seededToken)\n")
try? Data(teed.utf8).write(to: logs.appendingPathComponent("service-stderr.log"))

let secretDest = tmp.appendingPathComponent("diag-secret.zip")
do {
try DiagnosticsExporter.exportDiagnostics(to: secretDest, logsDirectory: logs)
} catch {
expect(false, "secret export threw \(error)")
}

let extracted = tmp.appendingPathComponent("extracted-\(UUID().uuidString)", isDirectory: true)
let ditto = Process()
ditto.executableURL = URL(fileURLWithPath: "/usr/bin/ditto")
ditto.arguments = ["-x", "-k", secretDest.path, extracted.path]
try? ditto.run()
ditto.waitUntilExit()

var zipBody = ""
if let files = FileManager.default.enumerator(at: extracted, includingPropertiesForKeys: nil) {
for case let url as URL in files {
zipBody += (try? String(contentsOf: url, encoding: .utf8)) ?? ""
}
}
expect(!zipBody.contains(seededToken), "exported zip is free of the seeded token")
expect(zipBody.contains("hf_***"), "exported zip carries the redacted placeholder")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Story #139 — redact secrets from logs and the diagnostics export.
// Plan: docs/plans/139-redact-secrets-from-logs.md

import Foundation
import MeetingTranscriberKit

// Shared canonical vector — the Python test (`_SEEDED_TOKEN`) uses the same
// string so a pattern divergence between the two runtimes surfaces.
private let seededToken = "hf_TESTTOKEN0123456789abcdef"

func runSecretRedactionTests() {
suite("SecretRedaction") {
// Masks an hf_ token embedded in surrounding text.
let redacted = SecretRedaction.redact("401 Unauthorized for \(seededToken) here")
expect(!redacted.contains(seededToken), "token removed, got \(redacted.debugDescription)")
expect(redacted.contains("hf_***"), "placeholder present, got \(redacted.debugDescription)")

// Leaves non-secret text untouched.
expectEqual(
SecretRedaction.redact("plain line, no secrets"),
"plain line, no secrets",
"non-secret text unchanged"
)

// Masks every occurrence, not just the first.
let multi = SecretRedaction.redact("\(seededToken) and \(seededToken)")
expect(!multi.contains(seededToken), "all occurrences masked, got \(multi.debugDescription)")

// The stderr tee redacts before writing to the bundled log file.
let dir = FileManager.default.temporaryDirectory
.appendingPathComponent("redaction-tests-\(UUID().uuidString)", isDirectory: true)
try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: dir) }

let stderrLog = FileLog(fileName: "service-stderr.log", directory: dir)
let chunk = "Traceback: RuntimeError 401 for \(seededToken)\n"
stderrLog.write(Data(SecretRedaction.redact(chunk).utf8))
stderrLog.flush()
let onDisk = (try? String(contentsOf: stderrLog.url, encoding: .utf8)) ?? ""
expect(!onDisk.contains(seededToken), "tee redacts token on disk, got \(onDisk.debugDescription)")
expect(onDisk.contains("hf_***"), "tee wrote placeholder, got \(onDisk.debugDescription)")
}
}
1 change: 1 addition & 0 deletions macos/Sources/MeetingTranscriberKitTests/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ runPlainTextRendererTests()
runOverviewAggregationsTests()
runFileLogTests()
runDiagnosticsExporterTests()
runSecretRedactionTests()

// (unnamed-speaker suite runs inside runSpeakerColorTests)
TestRunner.shared.finish()
Loading
Loading