From 74c81eafb75975694425a27bbd7e0b774eba3974 Mon Sep 17 00:00:00 2001 From: how2how2how2-arch Date: Sat, 24 Oct 2026 17:14:54 +0800 Subject: [PATCH] test: expand trace redaction regression coverage Cover the gaps named in #29 that the existing tests did not reach: - credentials in exception messages, including a multi-line traceback and a dotenv-style KEY=value line; - string items inside lists, with per-item finding paths; - the Authorization: Bearer header form, asserted to redact exactly once; - ordinary values (ints, floats, bools, None, empty strings, Devanagari text, and a literal [REDACTED]) preserved untouched. Add direct coverage for prepare_trace_for_persistence, which had none: event redaction entries and their JSON-pointer paths, a credential inside a manifest failure message recorded against the manifest sentinel id, appending to an existing event redactions list, rejection of a credential in a protected structural field, non-mutation of the input trace, and stability across a second pass. Record the formats the current pattern set does not redact as a parametrized regression pin, so adding a pattern for one of them is a deliberate change rather than an unnoticed one. README already documents redaction as best effort, so this claims nothing stronger. No production code changed. --- tests/unit/test_redaction.py | 264 ++++++++++++++++++++++++++++++++++- 1 file changed, 262 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_redaction.py b/tests/unit/test_redaction.py index 361278c..fdb63a8 100644 --- a/tests/unit/test_redaction.py +++ b/tests/unit/test_redaction.py @@ -1,13 +1,24 @@ from __future__ import annotations +import json from copy import deepcopy +from datetime import UTC, datetime from typing import cast +import pytest from hypothesis import given from hypothesis import strategies as st -from stepfork.trace import JsonValue -from stepfork.trace.redaction import REDACTION_REPLACEMENT, redact_json +from stepfork.trace import Event, JsonValue, Trace +from stepfork.trace.manifest import FailureInfo, RunStatus, TraceManifest +from stepfork.trace.models import RunStart +from stepfork.trace.redaction import ( + MANIFEST_REDACTION_ID, + REDACTION_REPLACEMENT, + RedactionError, + prepare_trace_for_persistence, + redact_json, +) SYNTHETIC_BEARER = "Bearer synthetic_token_123456" SYNTHETIC_OPENAI = "sk-synthetic1234567890" @@ -125,3 +136,252 @@ def test_redaction_is_idempotent_for_string_maps(payload: dict[str, str]) -> Non twice = redact_json(once).value assert once == twice + + +# --- Issue #29: exception messages, nested payload shapes, preserved values --- + + +def test_credential_in_exception_message_is_redacted() -> None: + payload = {"error": "401 Unauthorized: Bearer synthetic_abcdefgh12345678"} + + result = redact_json(payload) + + text = cast(dict[str, str], result.value)["error"] + assert "synthetic_abcdefgh12345678" not in text + assert text == f"401 Unauthorized: Bearer {REDACTION_REPLACEMENT}" + assert [(f.path, f.reason) for f in result.findings] == [ + ("/error", "credential_pattern") + ] + + +def test_credential_inside_multiline_traceback_is_redacted() -> None: + payload = { + "error": ( + "Traceback (most recent call last):\n" + ' File "agent.py", line 12, in call\n' + f" raise RuntimeError('{SYNTHETIC_GHP}')\n" + ) + } + + result = redact_json(payload) + text = cast(dict[str, str], result.value)["error"] + assert SYNTHETIC_GHP not in text + assert text.count(REDACTION_REPLACEMENT) == 1 + assert 'File "agent.py", line 12, in call' in text + + +def test_authorization_bearer_header_is_redacted_exactly_once() -> None: + result = redact_json({"note": "Authorization: Bearer synthetic_abcdefgh12345678"}) + + text = cast(dict[str, str], result.value)["note"] + assert text == f"Authorization: Bearer {REDACTION_REPLACEMENT}" + assert text.count(REDACTION_REPLACEMENT) == 1 + + +def test_credentials_in_string_lists_are_redacted_per_item() -> None: + result = redact_json({"log": ["ok", SYNTHETIC_OPENAI, "done"]}) + + assert cast(dict[str, list[str]], result.value)["log"] == [ + "ok", + REDACTION_REPLACEMENT, + "done", + ] + assert [(f.path, f.reason) for f in result.findings] == [ + ("/log/1", "credential_pattern") + ] + + +def test_secret_assigned_in_a_dotenv_style_line_is_redacted() -> None: + result = redact_json({"note": f"OPENAI_API_KEY={SYNTHETIC_OPENAI}"}) + + text = cast(dict[str, str], result.value)["note"] + assert SYNTHETIC_OPENAI not in text + assert text.startswith("OPENAI_API_KEY=") + + +def test_ordinary_values_are_preserved_exactly() -> None: + payload: dict[str, object] = { + "count": 3, + "ratio": 0.5, + "flag": True, + "nil": None, + "name": "alice", + "empty": "", + "city": "काठमाडौँ", + "already": REDACTION_REPLACEMENT, + } + + result = redact_json(cast(JsonValue, payload)) + + assert result.value == payload + assert result.findings == [] + + +def test_empty_payload_produces_no_findings() -> None: + result = redact_json({}) + + assert result.value == {} + assert result.findings == [] + + +@pytest.mark.parametrize( + ("label", "payload"), + [ + ("bare token key", {"token": "synthetic12345678"}), + ("basic auth", {"note": "Authorization: Basic c3ludGhldGljOnBhc3N3b3Jk"}), + ("aws access key", {"note": "AKIAIOSFODNN7EXAMPLE"}), + ("slack token", {"note": "xoxb-123456789012-syntheticabcd"}), + ("google api key", {"note": "AIzaSyA-synthetic1234567890abcdefg"}), + ("gitlab pat", {"note": "glpat-synthetic12345678"}), + ( + "private key block", + { + "note": ( + "-----BEGIN RSA PRIVATE KEY-----MIIE-----END RSA PRIVATE KEY-----" + ) + }, + ), + ("word-adjacent sk-", {"note": f"prefix{SYNTHETIC_OPENAI}suffix"}), + ("sk- below the 8-char floor", {"note": "sk-abcdefg"}), + ], +) +def test_formats_outside_the_pattern_set_pass_through( + label: str, payload: dict[str, str] +) -> None: + """Pin today's known gap: README documents redaction as best-effort. + + These inputs are not redacted by the current pattern set. This is a + regression pin, not an endorsement -- if a pattern is added for one of + these formats, move that case to the redacted cases above. + """ + result = redact_json(cast(JsonValue, payload)) + + assert result.value == payload, label + assert result.findings == [], label + + +# --- Issue #29: the persistence path, which had no direct coverage --- + + +def _manifest( + status: RunStatus = RunStatus.COMPLETED, + failure: FailureInfo | None = None, +) -> TraceManifest: + return TraceManifest( + run_id="run-1", + agent_name="synthetic-agent", + created_at=datetime(2026, 1, 1, tzinfo=UTC), + status=status, + failure=failure, + ) + + +def _trace(events: list[Event]) -> Trace: + return Trace( + run_id="run-1", + agent_name="synthetic-agent", + created_at=datetime(2026, 1, 1, tzinfo=UTC), + status=RunStatus.COMPLETED, + events=events, + ) + + +def test_prepare_trace_for_persistence_records_event_redactions() -> None: + event = RunStart( + id="evt-1", + run_id="run-1", + input={"api_key": "synthetic-secret", "city": "kathmandu"}, + ) + + prepared = prepare_trace_for_persistence(_trace([event]), _manifest()) + + persisted = prepared.trace.events[0].model_dump(mode="json") + assert persisted["input"]["api_key"] == REDACTION_REPLACEMENT + assert persisted["input"]["city"] == "kathmandu" + assert persisted["redactions"] == ["/input/api_key"] + assert [ + (entry.event_id, entry.path, entry.reason) + for entry in prepared.redactions.entries + ] == [("evt-1", "/input/api_key", "sensitive_key")] + assert "synthetic-secret" not in json.dumps(persisted) + + +def test_prepare_trace_for_persistence_redacts_manifest_failure_message() -> None: + manifest = _manifest( + status=RunStatus.FAILED, + failure=FailureInfo( + type="AuthError", + message="401 from provider: Bearer synthetic_abcdefgh12345678", + step=0, + ), + ) + + prepared = prepare_trace_for_persistence(_trace([]), manifest) + + assert prepared.manifest.failure is not None + assert "synthetic_abcdefgh12345678" not in prepared.manifest.failure.message + assert prepared.manifest.failure.message.endswith(REDACTION_REPLACEMENT) + assert [(entry.event_id, entry.path) for entry in prepared.redactions.entries] == [ + (MANIFEST_REDACTION_ID, "/failure/message") + ] + + +def test_prepare_trace_for_persistence_appends_to_existing_redactions() -> None: + event = RunStart( + id="evt-1", + run_id="run-1", + redactions=["/earlier/redaction"], + input={"password": "synthetic-secret"}, + ) + + prepared = prepare_trace_for_persistence(_trace([event]), _manifest()) + + persisted = prepared.trace.events[0].model_dump(mode="json") + assert persisted["redactions"] == ["/earlier/redaction", "/input/password"] + + +def test_prepare_trace_for_persistence_rejects_credential_in_protected_field() -> None: + event = RunStart(id=SYNTHETIC_GHP, run_id="run-1", input={}) + + with pytest.raises(RedactionError): + prepare_trace_for_persistence(_trace([event]), _manifest()) + + +def test_prepare_trace_for_persistence_does_not_mutate_the_input_trace() -> None: + event = RunStart(id="evt-1", run_id="run-1", input={"api_key": "synthetic-secret"}) + trace = _trace([event]) + original = deepcopy(trace.model_dump(mode="json")) + + prepare_trace_for_persistence(trace, _manifest()) + + assert trace.model_dump(mode="json") == original + + +def test_prepare_trace_for_persistence_is_idempotent_for_persisted_data() -> None: + """Re-preparing an already prepared trace is stable, except for the manifest. + + The persisted trace and manifest are stable across a second pass, but the + redaction manifest comes back empty: its entries are derived from this + pass's findings, and an already-redacted payload produces none. The event's + own ``redactions`` list still holds the path, so nothing is lost from the + bundle, but a second pass over a loaded trace would report no redactions. + Pinned here so that changing it is a deliberate decision. + """ + event = RunStart( + id="evt-1", + run_id="run-1", + input={"api_key": "synthetic-secret", "error": f"Bearer {SYNTHETIC_GHP}"}, + ) + + once = prepare_trace_for_persistence(_trace([event]), _manifest()) + twice = prepare_trace_for_persistence(once.trace, once.manifest) + + assert once.trace.model_dump(mode="json") == twice.trace.model_dump(mode="json") + assert once.manifest.model_dump(mode="json") == twice.manifest.model_dump( + mode="json" + ) + assert [entry.path for entry in once.redactions.entries] == [ + "/input/api_key", + "/input/error", + ] + assert twice.redactions.entries == []