From 293d96e1a685aa135dcce3e042dcbaafeee31a45 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 12 Jul 2026 03:15:17 +0200 Subject: [PATCH] fix(evidence): close canonical contract gaps --- .superpowers/sdd/evidence-task-1-report.md | 25 +++++++++ harness/tests/test_corpus_models.py | 53 ++++++++++++++++++-- harness/tests/test_evidence_port_contract.py | 37 +++++++++++++- harness/tht/corpus/models.py | 34 +++++++++++-- harness/tht/ports/evidence.py | 42 +++++++++++++--- 5 files changed, 176 insertions(+), 15 deletions(-) diff --git a/.superpowers/sdd/evidence-task-1-report.md b/.superpowers/sdd/evidence-task-1-report.md index a9694f52..c8c6dd18 100644 --- a/.superpowers/sdd/evidence-task-1-report.md +++ b/.superpowers/sdd/evidence-task-1-report.md @@ -71,3 +71,28 @@ Follow-up verification: - Fresh unrestricted harness attempt: 479 passed, 5 deselected; the same environmental boundary remains (47 Docker socket setup errors, four Docker parity failures, one isolated `uv build` network failure). + +## Final blocker follow-up + +The remaining four contract blockers were closed in a third TDD cycle: + +- `EvidenceSourceError` now always exposes the fixed public message/`args` value `evidence source + operation failed`; caller diagnostics are not retained. Category, details and args cannot be + reassigned, details remain recursively frozen and credential-screened, and an original exception + is available only when callers use standard exception chaining. +- Canonical document/chunk provenance stores only URI scheme, authority and path. Userinfo is + rejected; query strings and fragments are removed unconditionally, including AWS `X-Amz-*`, SAS + `sig`, and fragment token material. +- Binding model bases override Pydantic's unchecked `model_copy(update=...)`: merged values always + pass full field/model validation, so invalid copied records and top-level manifests fail. +- A canonical document/chunk `content_hash` must equal SHA-256 of the exact stored text encoded as + UTF-8. This establishes the normalization boundary explicitly: line-ending/frontmatter/text + normalization happens before model construction; the canonical models never rewrite content. + +Final follow-up verification: + +- Focused contract suite: 45 passed. +- Focused Ruff: passed. +- Harness excluding Docker-backed L0 and network-dependent packaging: 476 passed, 5 deselected. +- Fresh unrestricted harness attempt: 486 passed, 5 deselected, with the unchanged environmental + failures (47 Docker setup errors, four Docker parity failures, one isolated `uv build` failure). diff --git a/harness/tests/test_corpus_models.py b/harness/tests/test_corpus_models.py index 520632dc..0e9ee673 100644 --- a/harness/tests/test_corpus_models.py +++ b/harness/tests/test_corpus_models.py @@ -1,3 +1,4 @@ +import hashlib from datetime import UTC, datetime, timedelta, timezone import pytest @@ -7,26 +8,28 @@ from tht.corpus.models import CanonicalChunk, CanonicalDocument, CorpusManifest def document(source_uri: str = "https://host/a.md") -> CanonicalDocument: + content = "# A" return CanonicalDocument( document_id="doc:abc", source_id="source:a", source_uri=source_uri, source_fingerprint="etag:abc", - content_hash=f"sha256:{'d' * 64}", + content_hash=f"sha256:{hashlib.sha256(content.encode()).hexdigest()}", title="A", - content="# A", + content=content, media_type="text/markdown", pipeline_version="evidence-v1", ) def chunk() -> CanonicalChunk: + content = "# A" return CanonicalChunk( chunk_id="chunk:abc:0", document_id="doc:abc", ordinal=0, - content="# A", - content_hash=f"sha256:{'e' * 64}", + content=content, + content_hash=f"sha256:{hashlib.sha256(content.encode()).hexdigest()}", source_uri="https://host/a.md", pipeline_version="evidence-v1", ) @@ -156,7 +159,6 @@ def test_vector_generation_requires_embedding_compatibility(): ("document_id", "not-namespaced"), ("content_hash", "sha256:not-hex"), ("source_uri", "https://user:pass@host/a"), - ("source_uri", "https://host/a?refresh_token=secret"), ], ) def test_canonical_document_rejects_malformed_or_sensitive_provenance(field, value): @@ -164,6 +166,47 @@ def test_canonical_document_rejects_malformed_or_sensitive_provenance(field, val CanonicalDocument.model_validate({**document().model_dump(), field: value}) +@pytest.mark.parametrize( + "source_uri", + [ + "https://host/a?X-Amz-Credential=abc&X-Amz-Signature=secret#access_token=bad", + "https://host/a?sig=sas-secret&sp=r#section", + ], +) +def test_canonical_provenance_strips_query_and_fragment(source_uri): + doc = document(source_uri=source_uri) + canonical_chunk = chunk().model_copy(update={"source_uri": source_uri}) + manifest = CorpusManifest( + pipeline_version="evidence-v1", documents=[doc], chunks=[canonical_chunk] + ) + + assert doc.source_uri == "https://host/a" + assert canonical_chunk.source_uri == "https://host/a" + payload = manifest.model_dump_json() + assert "X-Amz" not in payload + assert "sas-secret" not in payload + assert "access_token" not in payload + + +@pytest.mark.parametrize("factory", [document, chunk]) +def test_content_hash_must_match_exact_canonical_utf8(factory): + record = factory() + with pytest.raises(ValidationError, match="exact canonical UTF-8 content"): + type(record).model_validate({**record.model_dump(), "content": record.content + "\n"}) + + +def test_model_copy_revalidates_records_and_manifests(): + with pytest.raises(ValidationError, match="namespaced"): + document().model_copy(update={"document_id": "invalid"}) + manifest = CorpusManifest( + pipeline_version="evidence-v1", + embedding_model="embed-v1", + embedding_dimensions=768, + ) + with pytest.raises(ValidationError, match="set together"): + manifest.model_copy(update={"embedding_dimensions": None}) + + def test_manifest_datetimes_are_aware_and_normalized_to_utc(): with pytest.raises(ValidationError, match="timezone-aware"): CorpusManifest(created_at=datetime(2026, 7, 12), pipeline_version="evidence-v1") diff --git a/harness/tests/test_evidence_port_contract.py b/harness/tests/test_evidence_port_contract.py index 2dc48d0e..eb28fc13 100644 --- a/harness/tests/test_evidence_port_contract.py +++ b/harness/tests/test_evidence_port_contract.py @@ -176,7 +176,7 @@ def test_datetimes_must_be_aware_and_are_normalized_to_utc(): def test_source_errors_are_typed_retryable_and_safe(): transient = EvidenceSourceError( - "remote source unavailable", + "password=hunter2 at https://user:secret@host", category=EvidenceSourceErrorCategory.TRANSIENT, details={"status": 503}, ) @@ -188,6 +188,16 @@ def test_source_errors_are_typed_retryable_and_safe(): assert transient.retryable is True assert permanent.retryable is False assert transient.details["status"] == 503 + assert str(transient) == "evidence source operation failed" + assert transient.args == ("evidence source operation failed",) + assert "hunter2" not in repr(transient) + with pytest.raises(AttributeError): + transient.category = EvidenceSourceErrorCategory.PERMANENT + with pytest.raises(AttributeError): + transient.args = ("leak",) + with pytest.raises(AttributeError): + transient.details = {"unsafe": True} + assert "hunter2" not in repr(transient.__dict__) with pytest.raises(TypeError): transient.details["status"] = 200 with pytest.raises(ValueError, match="credential-like"): @@ -202,3 +212,28 @@ def test_source_errors_are_typed_retryable_and_safe(): category=EvidenceSourceErrorCategory.PERMANENT, details={"not_json": object()}, ) + + +def test_source_error_preserves_original_only_through_exception_chaining(): + cause = RuntimeError("transport diagnostic with password=hunter2") + error = EvidenceSourceError( + "ignored unsafe diagnostic", + category=EvidenceSourceErrorCategory.TRANSIENT, + ) + + try: + raise error from cause + except EvidenceSourceError as caught: + assert caught.__cause__ is cause + assert "hunter2" not in str(caught) + assert "hunter2" not in caught.args + + +def test_model_copy_revalidates_source_and_acquired_records(): + source = SourceObject(source_id="source:a", uri="file:///a", fingerprint="sha256:a") + acquired = AcquiredDocument(source=source, content=b"a") + + with pytest.raises(ValidationError, match="namespaced"): + source.model_copy(update={"source_id": "invalid"}) + with pytest.raises(ValidationError, match="timezone-aware"): + acquired.model_copy(update={"acquired_at": datetime(2026, 7, 12)}) diff --git a/harness/tht/corpus/models.py b/harness/tht/corpus/models.py index 0439df9b..4c4f93f5 100644 --- a/harness/tht/corpus/models.py +++ b/harness/tht/corpus/models.py @@ -1,13 +1,16 @@ """Immutable records emitted by the Evidence preprocessing pipeline.""" +import hashlib import re +from collections.abc import Mapping from datetime import UTC, datetime +from typing import Self from pydantic import BaseModel, ConfigDict, Field, JsonValue, field_validator, model_validator from tht.ports.evidence import ( + canonical_provenance_uri, normalize_aware_datetime, - validate_canonical_uri, validate_namespaced_value, validate_safe_metadata, ) @@ -29,11 +32,24 @@ def _validate_hash(value: str) -> str: return value +def _require_content_hash(content: str, content_hash: str) -> None: + expected = f"sha256:{hashlib.sha256(content.encode('utf-8')).hexdigest()}" + if content_hash != expected: + raise ValueError("content_hash must match the exact canonical UTF-8 content") + + class _CanonicalValue(BaseModel): model_config = ConfigDict( frozen=True, extra="forbid", validate_default=True, revalidate_instances="always" ) + def model_copy(self, *, update: Mapping[str, object] | None = None, deep: bool = False) -> Self: + """Copy through full field and model validation, including manifest invariants.""" + data = self.model_dump(round_trip=True) + if update: + data.update(update) + return type(self).model_validate(data) + class _WithMetadata(_CanonicalValue): metadata: dict[str, JsonValue] = Field(default_factory=dict) @@ -41,6 +57,7 @@ class _WithMetadata(_CanonicalValue): class CanonicalDocument(_WithMetadata): + """Normalized text whose hash covers the exact stored UTF-8 content bytes.""" document_id: str source_id: str source_uri: str @@ -54,13 +71,19 @@ class CanonicalDocument(_WithMetadata): _document_id = field_validator("document_id")(_validate_namespaced_id) _source_id = field_validator("source_id")(_validate_namespaced_id) - _source_uri = field_validator("source_uri")(validate_canonical_uri) + _source_uri = field_validator("source_uri")(canonical_provenance_uri) _source_fingerprint = field_validator("source_fingerprint")(validate_namespaced_value) _content_hash = field_validator("content_hash")(_validate_hash) _modified_at = field_validator("modified_at")(normalize_aware_datetime) + @model_validator(mode="after") + def content_hash_matches(self) -> "CanonicalDocument": + _require_content_hash(self.content, self.content_hash) + return self + class CanonicalChunk(_WithMetadata): + """Chunk text whose hash covers the exact stored UTF-8 content bytes.""" chunk_id: str document_id: str ordinal: int = Field(ge=0) @@ -72,7 +95,12 @@ class CanonicalChunk(_WithMetadata): _chunk_id = field_validator("chunk_id")(_validate_namespaced_id) _document_id = field_validator("document_id")(_validate_namespaced_id) _content_hash = field_validator("content_hash")(_validate_hash) - _source_uri = field_validator("source_uri")(validate_canonical_uri) + _source_uri = field_validator("source_uri")(canonical_provenance_uri) + + @model_validator(mode="after") + def content_hash_matches(self) -> "CanonicalChunk": + _require_content_hash(self.content, self.content_hash) + return self class CorpusManifest(_WithMetadata): diff --git a/harness/tht/ports/evidence.py b/harness/tht/ports/evidence.py index 14fafde4..8cd64f89 100644 --- a/harness/tht/ports/evidence.py +++ b/harness/tht/ports/evidence.py @@ -4,8 +4,8 @@ import re from collections.abc import Iterable, Mapping, Sequence from datetime import UTC, datetime from enum import Enum -from typing import Protocol, runtime_checkable -from urllib.parse import parse_qsl, urlsplit +from typing import Protocol, Self, runtime_checkable +from urllib.parse import parse_qsl, urlsplit, urlunsplit from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, field_validator @@ -97,6 +97,20 @@ def validate_canonical_uri(value: str) -> str: return value +def canonical_provenance_uri(value: str) -> str: + """Return only stable URI identity; transport query/fragment data is never provenance.""" + try: + parsed = urlsplit(value) + _ = parsed.port + except ValueError as error: + raise ValueError("invalid canonical URI") from error + if not parsed.scheme: + raise ValueError("canonical URI must include a scheme") + if parsed.username is not None or parsed.password is not None: + raise ValueError("canonical URI must not contain credentials in userinfo") + return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", "")) + + def normalize_aware_datetime(value: datetime | None) -> datetime | None: if value is None: return None @@ -121,6 +135,13 @@ class _EvidenceValue(BaseModel): val_json_bytes="base64", ) + def model_copy(self, *, update: Mapping[str, object] | None = None, deep: bool = False) -> Self: + """Copy through validation; Pydantic's unchecked update-copy is unsafe for contracts.""" + data = self.model_dump(round_trip=True) + if update: + data.update(update) + return type(self).model_validate(data) + class SourceObject(_EvidenceValue): source_id: str = Field(min_length=1) @@ -159,14 +180,23 @@ class EvidenceSourceError(Exception): def __init__( self, - message: str, + _message: str, *, category: EvidenceSourceErrorCategory, details: dict[str, JsonValue] | None = None, ) -> None: - super().__init__(message) - self.category = EvidenceSourceErrorCategory(category) - self.details = validate_safe_metadata(_JSON_METADATA.validate_python(details or {})) + super().__init__("evidence source operation failed") + object.__setattr__(self, "category", EvidenceSourceErrorCategory(category)) + object.__setattr__( + self, + "details", + validate_safe_metadata(_JSON_METADATA.validate_python(details or {})), + ) + + def __setattr__(self, name: str, value) -> None: + if name in {"args", "category", "details"} and hasattr(self, name): + raise AttributeError(f"{name} is immutable") + super().__setattr__(name, value) @property def retryable(self) -> bool: