From 8110793f61b830632540f049aaacc23d998e9b7c Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 12 Jul 2026 06:50:23 +0200 Subject: [PATCH] fix(evidence): make result summaries safe --- .superpowers/sdd/evidence-task-5c-report.md | 16 +++++++++++++ harness/tests/test_corpus_pipeline.py | 26 ++++++++++++++++++++- harness/tests/test_preprocess_cli.py | 21 +++++++++++++++++ harness/tht/cli/preprocess_cmd.py | 6 +++-- harness/tht/corpus/pipeline.py | 16 +++++++++++-- 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/.superpowers/sdd/evidence-task-5c-report.md b/.superpowers/sdd/evidence-task-5c-report.md index 77d41c3b..556bd02b 100644 --- a/.superpowers/sdd/evidence-task-5c-report.md +++ b/.superpowers/sdd/evidence-task-5c-report.md @@ -170,3 +170,19 @@ and fail-closed tests, real Docker lifecycle, scoped Ruff/diff, and the full har Focused tests and scoped Ruff/diff pass. The contemporaneous full suite reaches an unrelated Task 6 DWH snapshot fixture missing its newly required workspace identity. + +### Safe result representation and exact text totals + +- `PipelineResult.manifest` is explicitly excluded from dataclass representation and the custom + representation is fixed-size operational data only. It omits manifest ids, documents, chunks, + content, metadata, and errors; `str(result)` inherits the same safe representation. +- Text-mode Evidence success output reads the uncapped aggregate totals from `payload["counts"]` + rather than the intentionally capped identifier arrays. +- Regression coverage builds a thousand-document/chunk manifest containing content and + credential-like metadata secrets, checks bounded `repr`/`str`, and verifies exact totals above + the 100-item public-array cap. + +Focused Evidence verification passes (`67 passed`), and scoped Ruff is clean. The full harness run +is not green in this sandbox: Docker-backed tests cannot access the daemon, wheel packaging cannot +use the restricted build environment, and concurrent Task 6 DWH binding changes currently fail two +DWH tests. None of those failures touch the Evidence files in this follow-up. diff --git a/harness/tests/test_corpus_pipeline.py b/harness/tests/test_corpus_pipeline.py index f394bebc..2e62499d 100644 --- a/harness/tests/test_corpus_pipeline.py +++ b/harness/tests/test_corpus_pipeline.py @@ -5,7 +5,7 @@ import pytest from tht.corpus.chunk import ChunkPolicy from tht.corpus.pipeline import CorpusPipeline, PipelineError, PipelineResult from tht.corpus.store import CorpusStore -from tht.corpus.models import CanonicalChunk, CorpusManifest +from tht.corpus.models import CanonicalChunk, CanonicalDocument, CorpusManifest from tht.ports.evidence import AcquiredDocument, SourceObject from tht.ports.vector import VectorCapabilities, VectorHealth @@ -362,6 +362,30 @@ def test_pipeline_result_public_dump_is_bounded_and_excludes_evidence_content(tm assert len(json.dumps(large)) < 25_000 +def test_pipeline_result_repr_is_bounded_and_excludes_manifest_secrets(): + secret = "TOP_SECRET_CONTENT" + manifest = CorpusManifest.model_construct( + manifest_id="gen:" + "a" * 64, + documents=tuple(CanonicalDocument.model_construct(content=secret) for _ in range(1000)), + chunks=tuple(CanonicalChunk.model_construct(content=secret) for _ in range(1000)), + metadata={"password": secret, "credential": "Bearer " + secret}, + ) + result = PipelineResult( + "succeeded", "gen:" + "a" * 64, True, (), (), (), manifest, + run_id="b" * 32, + ) + + rendered = repr(result) + assert str(result) == rendered + assert len(rendered) < 1000 + assert secret not in rendered + assert "password" not in rendered + assert "credential" not in rendered + assert "manifest" not in rendered.lower() + assert "documents" not in rendered + assert "chunks" not in rendered + + def test_reused_corpus_root_rejects_workspace_rename_before_any_mutation(tmp_path): vectors = Vectors() first = pipeline(tmp_path, Source([(item("one", "a"), "stable")]), vectors=vectors) diff --git a/harness/tests/test_preprocess_cli.py b/harness/tests/test_preprocess_cli.py index ee4a498a..8040865e 100644 --- a/harness/tests/test_preprocess_cli.py +++ b/harness/tests/test_preprocess_cli.py @@ -72,6 +72,27 @@ def test_preprocess_real_failed_stage_result_exits_nonzero(monkeypatch, tmp_path assert "secret" not in response.output +def test_preprocess_evidence_text_uses_uncapped_result_counts(monkeypatch, tmp_path): + import tht.cli.preprocess_cmd as command + + result = SimpleNamespace(model_dump=lambda mode=None: { + "status": "succeeded", "run_id": "a" * 32, + "generation": "gen:" + "b" * 64, "published": True, + "changed": ["fs:item"] * 100, + "unchanged": ["fs:item"] * 100, + "removed": ["fs:item"] * 100, + "counts": {"changed": 1001, "unchanged": 902, "removed": 803}, + }) + monkeypatch.setattr(command, "run_from_config", lambda *args, **kwargs: result) + + response = CliRunner().invoke( + app, ["preprocess", "evidence", "-c", str(tmp_path / "workspace.yaml")] + ) + + assert response.exit_code == 0, response.output + assert "changed=1001 unchanged=902 removed=803" in response.output + + def test_preprocess_resume_rejects_generation_id_before_configuration(monkeypatch, tmp_path): import tht.cli.preprocess_cmd as command diff --git a/harness/tht/cli/preprocess_cmd.py b/harness/tht/cli/preprocess_cmd.py index 55634db3..7e075bc6 100644 --- a/harness/tht/cli/preprocess_cmd.py +++ b/harness/tht/cli/preprocess_cmd.py @@ -172,9 +172,11 @@ def evidence_cmd( if json_output: typer.echo(json.dumps(payload, ensure_ascii=False, sort_keys=True)) else: + counts = payload["counts"] typer.echo( - f"OK: run={payload['run_id']} generation={payload['generation']} changed={len(payload['changed'])} " - f"unchanged={len(payload['unchanged'])} removed={len(payload['removed'])}" + f"OK: run={payload['run_id']} generation={payload['generation']} " + f"changed={counts['changed']} unchanged={counts['unchanged']} " + f"removed={counts['removed']}" ) diff --git a/harness/tht/corpus/pipeline.py b/harness/tht/corpus/pipeline.py index 8312d8b7..625a5ff1 100644 --- a/harness/tht/corpus/pipeline.py +++ b/harness/tht/corpus/pipeline.py @@ -7,7 +7,7 @@ import json import re import uuid from collections.abc import Mapping, Sequence -from dataclasses import asdict, dataclass +from dataclasses import asdict, dataclass, field from datetime import UTC from pathlib import Path @@ -45,10 +45,22 @@ class PipelineResult: changed: tuple[str, ...] unchanged: tuple[str, ...] removed: tuple[str, ...] - manifest: CorpusManifest + manifest: CorpusManifest = field(repr=False) run_id: str | None = None resumed_from: str | None = None + def __repr__(self) -> str: + counts = { + "changed": len(self.changed), + "unchanged": len(self.unchanged), + "removed": len(self.removed), + } + return ( + f"PipelineResult(status={self.status!r}, generation={self.generation!r}, " + f"published={self.published!r}, counts={counts!r}, " + f"run_id={self.run_id!r}, resumed_from={self.resumed_from!r})" + ) + def model_dump(self, mode=None): def bounded(values: tuple[str, ...]) -> list[str]: return [value[:200] for value in values[:100]]