From ba1d064053326d24359e4a910959af9c0cc85e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez?= Date: Thu, 16 Jul 2026 12:03:05 +0200 Subject: [PATCH 1/2] fix(logging): never redact trace/span correlation keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Presidio-style redaction ran over log records after the trace ids were injected, and a 32-char trace_id hex can contain a 7-digit island that matches the PHONE pattern, so the redactor could mutilate trace_id/span_id and silently break log↔trace correlation. Add a hardcoded NEVER_REDACT allowlist {trace_id, span_id, trace_flags, correlationId} in the structlog redactor that wins even over an explicit deny, and reorder the structlog pre-chain so redaction runs BEFORE trace-id injection (defense in depth). Adds a regression test that reproduces the PHONE corruption and asserts the keys survive while a genuine phone in the message is still redacted. --- src/pyfly/logging/redaction/processor.py | 7 +++++++ src/pyfly/logging/structlog_adapter.py | 6 ++++-- tests/logging/test_redaction_processor.py | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/pyfly/logging/redaction/processor.py b/src/pyfly/logging/redaction/processor.py index 24ea8cf8..bc7c652c 100644 --- a/src/pyfly/logging/redaction/processor.py +++ b/src/pyfly/logging/redaction/processor.py @@ -23,6 +23,11 @@ _REDACTED = "" +# Correlation keys are NEVER redacted, regardless of allow/deny config. A trace id is a +# 32-char hex string whose digit substrings can match generic PII patterns (e.g. PHONE), +# so an unguarded redactor can mutilate it and silently break log↔trace correlation. +_NEVER_REDACT = frozenset({"trace_id", "span_id", "trace_flags", "correlationId"}) + def make_structlog_redactor( redactor: Redactor, @@ -35,6 +40,8 @@ def make_structlog_redactor( def processor(_logger: Any, _method: str, event_dict: dict[str, Any]) -> dict[str, Any]: for key, value in list(event_dict.items()): + if key in _NEVER_REDACT: + continue if key in deny: event_dict[key] = _REDACTED continue diff --git a/src/pyfly/logging/structlog_adapter.py b/src/pyfly/logging/structlog_adapter.py index 09df5d94..267737c4 100644 --- a/src/pyfly/logging/structlog_adapter.py +++ b/src/pyfly/logging/structlog_adapter.py @@ -54,13 +54,13 @@ def configure(self, config: Config) -> None: fmt = props.format.lower() # Shared pre-chain applied to BOTH structlog and foreign (stdlib) records. + # Redaction runs BEFORE trace-id injection so the correlation keys are never seen + # by the redactor (defense in depth on top of the NEVER_REDACT allowlist). timestamper = structlog.processors.TimeStamper(fmt="iso") shared_pre: list[structlog.types.Processor] = [ structlog.contextvars.merge_contextvars, structlog.stdlib.add_log_level, structlog.stdlib.add_logger_name, - _add_trace_ids, - timestamper, ] if redactor is not None: shared_pre.append( @@ -69,6 +69,8 @@ def configure(self, config: Config) -> None: make_structlog_redactor(redactor, props.redaction.allow_fields, props.redaction.deny_fields), ) ) + shared_pre.append(_add_trace_ids) + shared_pre.append(timestamper) renderer: structlog.types.Processor = ( structlog.processors.JSONRenderer() diff --git a/tests/logging/test_redaction_processor.py b/tests/logging/test_redaction_processor.py index b3ec3747..393c658f 100644 --- a/tests/logging/test_redaction_processor.py +++ b/tests/logging/test_redaction_processor.py @@ -36,6 +36,24 @@ def test_structlog_allow_fields_limits_scanning(): assert out["note"] == "keep bob@x.io" # not in allow list -> untouched +def test_trace_ids_are_never_redacted_even_with_phone_pattern(): + # A trace id with a 7-digit island bounded by hex letters matches the PHONE pattern; + # left unguarded the redactor would mutilate it and silently break log<->trace correlation. + tid = "a1234567ffffffffffffffffffffffff" + r = RegexRedactor(["PHONE"]) + assert r.redact(tid) != tid # sanity: PHONE *would* corrupt it if unguarded + # deny_fields lists trace_id on purpose: NEVER_REDACT must win even over an explicit deny. + proc = make_structlog_redactor(r, allow_fields=[], deny_fields=["trace_id"]) + out = proc( + None, + "info", + {"event": "call 555 1234 now", "trace_id": tid, "span_id": "1234567abcdef000"}, + ) + assert out["trace_id"] == tid # correlation key preserved verbatim + assert out["span_id"] == "1234567abcdef000" # ditto, despite its own digit island + assert out["event"] == "call now" # a genuine phone in the message is still redacted + + def test_stdlib_filter_redacts_message(): r = RegexRedactor(["EMAIL"]) flt = RedactionFilter(r, allow_fields=[], deny_fields=[]) From 904b6bff816ea6ef8727a560a7f2f9b27c971f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez?= Date: Thu, 16 Jul 2026 12:17:04 +0200 Subject: [PATCH 2/2] style(logging): apply ruff format to redaction regression test --- tests/logging/test_redaction_processor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/logging/test_redaction_processor.py b/tests/logging/test_redaction_processor.py index 393c658f..a2e320cb 100644 --- a/tests/logging/test_redaction_processor.py +++ b/tests/logging/test_redaction_processor.py @@ -49,9 +49,9 @@ def test_trace_ids_are_never_redacted_even_with_phone_pattern(): "info", {"event": "call 555 1234 now", "trace_id": tid, "span_id": "1234567abcdef000"}, ) - assert out["trace_id"] == tid # correlation key preserved verbatim - assert out["span_id"] == "1234567abcdef000" # ditto, despite its own digit island - assert out["event"] == "call now" # a genuine phone in the message is still redacted + assert out["trace_id"] == tid # correlation key preserved verbatim + assert out["span_id"] == "1234567abcdef000" # ditto, despite its own digit island + assert out["event"] == "call now" # a genuine phone in the message is still redacted def test_stdlib_filter_redacts_message():