fix(evidence): close canonical contract gaps
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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)})
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user