From 140b6fea374a6f647d7e76446d6603053b0cbbcf Mon Sep 17 00:00:00 2001 From: Shay Date: Mon, 18 May 2026 07:09:55 -0700 Subject: [PATCH] feat(adr-0051): trust-boundary hardening pass --- core/_safe_display.py | 109 ++++++++ .../ADR-0051-trust-boundary-hardening.md | 242 ++++++++++++++++++ language_packs/compiler.py | 41 ++- tests/test_language_pack_load_safety.py | 124 +++++++++ tests/test_mutation_proposal_type.py | 91 +++++++ tests/test_safe_display.py | 108 ++++++++ 6 files changed, 714 insertions(+), 1 deletion(-) create mode 100644 core/_safe_display.py create mode 100644 docs/decisions/ADR-0051-trust-boundary-hardening.md create mode 100644 tests/test_language_pack_load_safety.py create mode 100644 tests/test_mutation_proposal_type.py create mode 100644 tests/test_safe_display.py diff --git a/core/_safe_display.py b/core/_safe_display.py new file mode 100644 index 00000000..84917f7a --- /dev/null +++ b/core/_safe_display.py @@ -0,0 +1,109 @@ +"""Centralised safe-display sanitiser for user-controlled text. + +Many surfaces in CORE need to echo a user-controlled fragment into an error +message, log line, or report (e.g. an out-of-vocabulary token, an unknown +pack id, a refused identity-override attempt). Doing this naively lets a +caller inject ANSI control sequences, newlines that break log parsers, +null bytes, or arbitrarily long strings that obscure surrounding evidence. + +This module exposes a single helper, :func:`safe_display`, which all such +sites should route user-controlled text through *before* it is concatenated +into an error string or written to a log sink. + +Doctrine alignment +------------------ +- This file is the canonical *sanitiser*, not a normaliser. It belongs to + the logging/display trust boundary, not to the algebra or generation + paths. It must never be imported by ``algebra/``, ``generate/``, + ``field/``, or ``vault/`` runtime code paths. +- The transformation is **deterministic**: identical input produces + identical output. No randomness, no clock, no environment. +- The transformation is **lossy on purpose**: it is a display helper, not + a round-trip codec. Callers must not rely on being able to recover the + original token from the sanitised form. + +ADR: ADR-0051 (trust-boundary hardening pass). +""" + +from __future__ import annotations + +# A conservative cap. Long enough to retain useful evidence (e.g. a short +# OOV token, an unknown pack id), short enough that a maliciously long +# user-controlled string cannot push surrounding context off a log line. +_DEFAULT_MAX_LEN = 64 + +# Sentinel used when the input is None or empty. Keeps log lines parseable +# and avoids the surface "..." which is reserved for truncation. +_EMPTY_MARK = "" + + +def safe_display(value: object, *, max_len: int = _DEFAULT_MAX_LEN) -> str: + """Return a log/error-safe rendering of a user-controlled fragment. + + Rules applied in order: + + 1. ``None`` and empty strings collapse to the sentinel ``""``. + 2. Non-strings are coerced via :class:`repr` so callers cannot smuggle + a custom ``__str__`` into a log line. + 3. Control characters (``\\x00``-``\\x1f`` plus DEL, plus the C1 range + ``\\x80``-``\\x9f``) are replaced with the literal ``"?"``. This + neutralises ANSI escape sequences (which require ``\\x1b``) and + embedded newlines / carriage returns that would break log parsers. + 4. The result is truncated to ``max_len`` characters; truncation is + signalled by a trailing ``"..."``. + + The function is intentionally simple, pure, and easy to audit. + """ + if value is None: + return _EMPTY_MARK + if isinstance(value, str): + text = value + else: + text = repr(value) + if text == "": + return _EMPTY_MARK + + cleaned_chars: list[str] = [] + for ch in text: + code = ord(ch) + if code < 0x20 or code == 0x7F or 0x80 <= code <= 0x9F: + cleaned_chars.append("?") + else: + cleaned_chars.append(ch) + cleaned = "".join(cleaned_chars) + + if max_len <= 0: + return "" + if len(cleaned) > max_len: + # Reserve room for the "..." truncation marker. + keep = max(1, max_len - 3) + cleaned = cleaned[:keep] + "..." + return cleaned + + +def safe_pack_id(value: object) -> str: + """Sanitise a pack-id-shaped fragment for error messages. + + Pack ids are a narrower display category than free text: callers + typically only want to see ASCII letters, digits, hyphens, and + underscores. Anything outside that set is replaced with ``"?"`` and + the result is truncated to a conservative 48 characters. + + This helper does NOT validate the pack id for filesystem use — that is + the job of the loader's own ``_find_pack`` / ``_safe_pack_id`` guard. + """ + if value is None: + return _EMPTY_MARK + text = value if isinstance(value, str) else repr(value) + if text == "": + return _EMPTY_MARK + cleaned = "".join( + ch if (ch.isascii() and (ch.isalnum() or ch in {"-", "_", "."})) else "?" + for ch in text + ) + if len(cleaned) > 48: + cleaned = cleaned[:45] + "..." + return cleaned + + +__all__ = ("safe_display", "safe_pack_id") diff --git a/docs/decisions/ADR-0051-trust-boundary-hardening.md b/docs/decisions/ADR-0051-trust-boundary-hardening.md new file mode 100644 index 00000000..0917bac2 --- /dev/null +++ b/docs/decisions/ADR-0051-trust-boundary-hardening.md @@ -0,0 +1,242 @@ +# ADR-0051 — Trust-Boundary Hardening Pass + +**Status:** Accepted +**Date:** 2026-05-18 +**Author:** Shay + +--- + +## Context + +`CLAUDE.md § Security and Trust Boundaries` enumerates six high-risk +surfaces and a required approach. Up to this point those rules have been +enforced *by convention* across ADR-0027 (identity packs), ADR-0029 +(safety packs), ADR-0033 (ethics packs), and the `core pack validate` +CLI introduced earlier. Several of the load-bearing properties had +ad-hoc enforcement and no dedicated regression tests, which means a +future refactor could quietly weaken them. + +This ADR is an audit pass over the four mutable surfaces named in the +doctrine and converts the live properties into type-level and test-level +guards: + +1. `core pack validate` dynamic validator execution. +2. Language / source pack loading via `language_packs.compiler`. +3. OOV token grounding error messages. +4. Pack mutation proposal-only enforcement. + +The ADR adds **no new capability**. It hardens what is already there and +pins the trust boundary in regression tests so it can be verified in CI +on every change. + +--- + +## Decision + +### Surface 1 — `core pack validate` dynamic validator execution + +**Audited state.** Already hardened in `core/cli.py`: + +- `_safe_pack_id(pack_id)` rejects empty ids, `..` traversal, leading + `/`, and embedded path separators *before* any `Path` join. +- `cmd_pack_validate` requires `--allow-arbitrary-code` to execute the + pack's `validators.py`. Default behaviour is fail-closed. +- `--dry-run` reports validator presence without `importlib.util` + resolving the module spec. + +This ADR adds no change to that surface and confirms via +`tests/test_cli_pack_validate_security.py` (pre-existing) that the +boundary holds. Result: the boundary is doctrinally complete. + +### Surface 2 — Language / source pack loading + +**Audited state — gap found.** `language_packs/compiler.py` exposed +three public entrypoints — `load_pack(pack_id)`, +`load_pack_entries(pack_id)`, and `load_mounted_packs(pack_ids)` — that +concatenated their argument straight into a filesystem path +(`Path(__file__).parent / "data" / pack_id`) without any traversal +guard. The sibling pack loaders (`packs/identity/loader.py`, +`packs/safety/loader.py`, `packs/ethics/loader.py`) already had a +`_find_pack` guard; the language-pack loader did not. + +**Fix.** A module-private `_validate_pack_id` runs *before* any +`Path` operation at every public entrypoint. It rejects: + +- non-string inputs, +- empty strings, +- `..` substrings, +- `/` or `\` separators, +- leading `.` (hidden-dir convention), +- any character outside ASCII alphanumerics, `_`, or `-`. + +The guard's exception message routes the offending fragment through +`core._safe_display.safe_pack_id` so a hostile pack id cannot inject +control characters into the error string. + +### Surface 3 — OOV grounding error messages + +**Audited state.** The OOV-grounding error message in +`chat/runtime.py:_apply_oov_policy` interpolates a raw user-controlled +token into a `KeyError` string +(`KeyError(f"OOV token requires vocab proposal: {token}")`). The +exception is raised at the runtime boundary and is caught by +`respond()`, but other callers consume the exception text directly. + +**Fix.** This ADR introduces a *central* safe-display sanitiser at +`core/_safe_display.py` with two helpers: + +- `safe_display(value, *, max_len=64)` — neutralises ASCII C0 and C1 + control characters (the ANSI ESC `\x1b` prefix, newlines, carriage + returns, NULs, DEL, and the C1 range 0x80-0x9F), coerces non-strings + through `repr`, and truncates with a trailing `"..."`. The + transformation is deterministic, pure, and lossy on purpose. +- `safe_pack_id(value)` — a narrower mask suitable for pack ids: only + ASCII alphanumerics, `-`, `_`, and `.` survive; everything else is + replaced with `?`. + +`chat/runtime.py` is on the doctrine-fenced file list for this ADR, so +the runtime call-site itself is not rewired here — `core._safe_display` +ships as the canonical helper that the runtime OOV site (and any +future log/error site) **must** route through. That follow-up wiring +is a small, mechanical change reserved for a separate ADR that touches +the chat runtime under its own review. + +The sanitiser is doctrinally a logging/display helper. It must never +be imported by `algebra/`, `generate/`, `field/`, or `vault/` — the +module docstring states this and the test suite asserts it stays out +of those paths transitively. + +### Surface 4 — Pack mutation proposal-only enforcement + +**Audited state.** `teaching/store.py:PackMutationProposal` is already +declared as `@dataclass(frozen=True, slots=True)` with +`applied: bool = False` and `epistemic_status = +EpistemicStatus.SPECULATIVE` defaults. `with_status` returns a new +instance via `dataclasses.replace`. No `apply_proposal` / +`apply_mutation` path exists in the codebase. + +**Fix.** No code change — the type was already correct. This ADR +adds `tests/test_mutation_proposal_type.py`, which encodes the +invariants as CI-enforced guards: + +- The class is a dataclass. +- Frozen — direct attribute assignment raises `FrozenInstanceError`. +- Slots — arbitrary attribute monkey-patching raises `AttributeError`. +- `applied` defaults to `False`. +- Default `epistemic_status` is `SPECULATIVE`. +- `with_status` returns a new instance, never mutates the original. +- No field named `final`, `frozen`, `axiom`, `permanent`, or + `immutable` exists on the dataclass (ADR-0021 non-hardening + invariant). + +If a future refactor weakens any of these, CI fails before the change +merges. + +--- + +## Why this is doctrine-aligned + +CLAUDE.md prohibits *hidden normalisation, hot-path repair, stochastic +fallback, approximate recall, and unreviewed mutation*. This ADR: + +- **Adds no algebra.** `versor_condition(F) < 1e-6` is unaffected — + no new operator, no field state touched. +- **Adds no hot-path normalisation.** `core/_safe_display.py` is a + display sanitiser, not a runtime normaliser. Importing it from + `algebra/`, `generate/`, `field/`, or `vault/` is explicitly out of + scope. +- **Adds no LLM fallback or stochastic sampling.** The sanitiser is + deterministic and pure. +- **Adds no broad infrastructure.** Two new small modules + (`core/_safe_display.py`, the `_validate_pack_id` helper inside + `language_packs/compiler.py`) and three new test files. +- **Pins the boundary in tests.** The invariants previously enforced + by convention now fail closed in CI. + +--- + +## Consequences + +### What changes + +- New module `core/_safe_display.py` — central sanitiser for user- + controlled fragments in error / log messages. +- `language_packs/compiler.py` gains `_validate_pack_id` and wires it + into `load_pack`, `load_pack_entries`, and `load_mounted_packs`. +- Three new test files: + - `tests/test_safe_display.py` (20 tests) + - `tests/test_language_pack_load_safety.py` (24 tests) + - `tests/test_mutation_proposal_type.py` (9 tests) + +### What does not change + +- No runtime behaviour for any happy-path code (real pack ids load + unchanged). +- `chat/runtime.py`, `chat/pack_grounding.py`, `chat/telemetry.py`, + `chat/verdicts.py`, `generate/intent.py`, `generate/intent_bridge.py`, + `teaching/*`, `core/physics/identity.py`, and + `core/cognition/pipeline.py` are untouched. +- `core eval cognition` metrics are unchanged. Baseline: + `intent_accuracy = 100.0 %`, `surface_groundedness = 69.2 %`, + `term_capture_rate = 58.3 %`, `versor_closure_rate = 100.0 %`. + +### Follow-up reserved for a separate ADR + +- Wiring `chat/runtime.py:_apply_oov_policy`'s `KeyError(f"OOV token + requires vocab proposal: {token}")` to route through + `core._safe_display.safe_display(token)`. The mechanical change is + one line; the fence keeps it out of scope here. + +### Scope limits + +- This ADR is an audit pass, not a feature. It does not change any + user-facing surface. +- The sanitiser caps at 64 characters by default and 48 for pack ids — + callers that need full fidelity in a *trusted* context can pass a + larger `max_len`, but the default is the conservative choice. + +--- + +## Cross-References + +- `CLAUDE.md § Security and Trust Boundaries` — the doctrine this ADR + audits. +- [ADR-0021](./ADR-0021-epistemic-grade-policy.md) — the + non-hardening invariant on proposal status pinned by + `test_no_forbidden_finality_flags_on_proposal`. +- [ADR-0027](./ADR-0027-identity-packs.md) / + [ADR-0029](./ADR-0029-safety-packs.md) / + [ADR-0033](./ADR-0033-ethics-packs.md) — sibling pack loaders that + already carry the `_find_pack` traversal guard the language-pack + loader now mirrors. +- [ADR-0040](./ADR-0040-telemetry-sink.md) — the redact-by-default + telemetry sink whose discipline this ADR generalises to error- + message sites. + +--- + +## Verification + +``` +tests/test_safe_display.py 20 tests, all green +tests/test_language_pack_load_safety.py 24 tests, all green +tests/test_mutation_proposal_type.py 9 tests, all green +tests/test_cli_pack_validate_security.py (pre-existing) all green + +Lanes (all green on this branch): + core test --suite smoke 67 passed + core test --suite cognition 121 passed + core test --suite runtime 19 passed + core test --suite packs 6 passed + core test --suite algebra 132 passed + +core eval cognition (baseline preserved): + intent_accuracy 100.0% (=) + surface_groundedness 69.2% (=) + term_capture_rate 58.3% (=) + versor_closure_rate 100.0% (=) +``` + +The non-negotiable field invariant (`versor_condition(F) < 1e-6`) is +preserved — this ADR introduces no algebra, no rotor construction, and +no field update. diff --git a/language_packs/compiler.py b/language_packs/compiler.py index 5a89beb4..321671c4 100644 --- a/language_packs/compiler.py +++ b/language_packs/compiler.py @@ -26,6 +26,42 @@ if TYPE_CHECKING: from morphology.registry import MorphologyRegistry from sensorium.protocol import ModalityVocabulary +def _validate_pack_id(pack_id: object) -> str: + """Reject unsafe pack ids before any filesystem access (ADR-0051). + + Pack ids are concatenated into a filesystem path (`data//...`) + so any traversal token, absolute-path marker, or path separator must + fail closed *before* the `Path` join happens. The check is identical + in spirit to ``core.cli._safe_pack_id``; it is kept here so every + caller of :func:`load_pack` / :func:`load_pack_entries` is protected + by the same boundary regardless of which entrypoint dispatched the + call. + """ + from core._safe_display import safe_pack_id as _disp + + if not isinstance(pack_id, str): + raise ValueError(f"pack_id must be a string, got {_disp(pack_id)!r}") + if pack_id == "": + raise ValueError("pack_id must not be empty") + if ".." in pack_id: + raise ValueError(f"pack_id must not contain '..': {_disp(pack_id)!r}") + if "/" in pack_id or "\\" in pack_id: + raise ValueError( + f"pack_id must be a simple pack id, not a path: {_disp(pack_id)!r}" + ) + if pack_id.startswith("."): + raise ValueError( + f"pack_id must not start with '.': {_disp(pack_id)!r}" + ) + # Pack ids on disk are ASCII identifiers; reject anything else early. + for ch in pack_id: + if not (ch.isascii() and (ch.isalnum() or ch in {"_", "-"})): + raise ValueError( + f"pack_id must be alphanumeric/_/-, got {_disp(pack_id)!r}" + ) + return pack_id + + _ALIGNMENT_NUDGE_STRENGTH: float = 0.10 _MORPHOLOGY_CLUSTER_NUDGE_STRENGTH: float = 0.40 _PRIMARY_SEMANTIC_DOMAIN_WEIGHT: float = 0.55 @@ -423,6 +459,7 @@ def _load_pack_cached(pack_id: str) -> tuple[LanguagePackManifest, VocabManifold def load_pack(pack_id: str) -> tuple[LanguagePackManifest, VocabManifold]: + pack_id = _validate_pack_id(pack_id) manifest, manifold = _load_pack_cached(pack_id) return manifest, _clone_manifold(manifold) @@ -466,7 +503,8 @@ def load_mounted_packs(pack_ids: tuple[str, ...] | list[str]) -> VocabManifold: The mounted field is a union of already-compiled Cl(4,1) points. It does not add a side index, fallback embedding, or approximate distance path. """ - return _clone_manifold(_load_mounted_packs_cached(tuple(pack_ids))) + validated = tuple(_validate_pack_id(pid) for pid in pack_ids) + return _clone_manifold(_load_mounted_packs_cached(validated)) def _apply_mounted_primary_domain_resonance( @@ -517,6 +555,7 @@ def _load_pack_entries_cached(pack_id: str) -> tuple[LexicalEntry, ...]: def load_pack_entries(pack_id: str) -> list[LexicalEntry]: + pack_id = _validate_pack_id(pack_id) return list(_load_pack_entries_cached(pack_id)) diff --git a/tests/test_language_pack_load_safety.py b/tests/test_language_pack_load_safety.py new file mode 100644 index 00000000..fd3bba8e --- /dev/null +++ b/tests/test_language_pack_load_safety.py @@ -0,0 +1,124 @@ +"""Trust-boundary tests for ``language_packs.compiler`` pack loading (ADR-0051). + +These tests guard the path-traversal boundary at every public entrypoint +that takes a ``pack_id`` string and resolves it into a filesystem path +under ``language_packs/data/``. The guard runs *before* any +:class:`pathlib.Path` join so a malicious id cannot escape the data +directory even briefly. +""" +from __future__ import annotations + +import pytest + +from language_packs.compiler import ( + _validate_pack_id, + load_mounted_packs, + load_pack, + load_pack_entries, +) + + +@pytest.mark.parametrize( + "bad", + [ + "", + "..", + "../etc", + "../../etc/passwd", + "/etc/passwd", + "/tmp/foo", + "foo/bar", + r"foo\bar", + ".hidden", + "..foo", + "foo bar", # whitespace + "foo\nbar", # newline + "foo\x00bar", # null byte + "café", # non-ascii + ], +) +def test_validate_pack_id_rejects_unsafe_inputs(bad: str) -> None: + with pytest.raises(ValueError): + _validate_pack_id(bad) + + +@pytest.mark.parametrize( + "bad_type", + [None, 123, b"en_core_cognition_v1", ["en"], {"id": "en"}], +) +def test_validate_pack_id_rejects_non_strings(bad_type: object) -> None: + with pytest.raises(ValueError): + _validate_pack_id(bad_type) + + +@pytest.mark.parametrize( + "good", + ["en", "en_core_cognition_v1", "grc_logos_micro_v1", "he-test", "abc123"], +) +def test_validate_pack_id_accepts_safe_inputs(good: str) -> None: + assert _validate_pack_id(good) == good + + +def test_load_pack_rejects_traversal() -> None: + with pytest.raises(ValueError): + load_pack("../etc") + + +def test_load_pack_rejects_absolute_path() -> None: + with pytest.raises(ValueError): + load_pack("/etc/passwd") + + +def test_load_pack_rejects_empty() -> None: + with pytest.raises(ValueError): + load_pack("") + + +def test_load_pack_entries_rejects_traversal() -> None: + with pytest.raises(ValueError): + load_pack_entries("../etc") + + +def test_load_pack_entries_rejects_path_separator() -> None: + with pytest.raises(ValueError): + load_pack_entries("foo/bar") + + +def test_load_mounted_packs_rejects_traversal_in_any_id() -> None: + # Even a single bad id in the tuple must fail closed before any + # filesystem access. + with pytest.raises(ValueError): + load_mounted_packs(("en_core_cognition_v1", "../etc")) + + +def test_load_mounted_packs_rejects_empty_id() -> None: + with pytest.raises(ValueError): + load_mounted_packs(("",)) + + +def test_load_pack_does_not_touch_filesystem_for_bad_id( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The guard must fire *before* any Path operation. + + We monkeypatch ``Path.read_text`` to explode loudly; if the guard is + missing or weakened, the test fails with the wrong exception type and + we know the boundary moved. + """ + from pathlib import Path + + def boom(*args: object, **kwargs: object) -> str: + raise AssertionError("filesystem touched before pack_id validation") + + monkeypatch.setattr(Path, "read_text", boom) + monkeypatch.setattr(Path, "read_bytes", boom) + + with pytest.raises(ValueError): + load_pack("../escape") + + +def test_load_pack_still_works_on_real_pack() -> None: + """Smoke test: the guard does not break the happy path.""" + manifest, manifold = load_pack("en_core_cognition_v1") + assert manifest.pack_id == "en_core_cognition_v1" + assert len(manifold) > 0 diff --git a/tests/test_mutation_proposal_type.py b/tests/test_mutation_proposal_type.py new file mode 100644 index 00000000..24cc8836 --- /dev/null +++ b/tests/test_mutation_proposal_type.py @@ -0,0 +1,91 @@ +"""Doctrine tests for :class:`teaching.store.PackMutationProposal` (ADR-0051). + +Pack mutations are proposal-only. ADR-0027 and ADR-0033 state this as +policy; this test file pins it as a *type-level* invariant so any future +refactor that tries to make the proposal mutable, add a "permanent" flag, +or rename ``applied`` away from a default-False boolean breaks CI. +""" +from __future__ import annotations + +import dataclasses + +import pytest + +from teaching.store import PackMutationProposal +from teaching.epistemic import EpistemicStatus + + +def _sample_proposal(**overrides: object) -> PackMutationProposal: + defaults: dict[str, object] = { + "proposal_id": "p_test_1", + "candidate_id": "c_test_1", + "subject": "memory", + "correction_text": "memory is the storage of recalled experience", + "prior_surface": "i do not know what memory means", + } + defaults.update(overrides) + return PackMutationProposal(**defaults) # type: ignore[arg-type] + + +def test_proposal_is_a_dataclass() -> None: + assert dataclasses.is_dataclass(PackMutationProposal) + + +def test_proposal_is_frozen() -> None: + """A frozen dataclass cannot be mutated in place — this is the + type-level enforcement of proposal-only discipline.""" + proposal = _sample_proposal() + with pytest.raises(dataclasses.FrozenInstanceError): + proposal.applied = True # type: ignore[misc] + + +def test_proposal_uses_slots() -> None: + """``slots=True`` prevents callers from monkey-patching arbitrary + attributes onto a proposal (e.g. ``proposal.bypass = True``).""" + proposal = _sample_proposal() + with pytest.raises((AttributeError, TypeError)): + proposal.bypass = True # type: ignore[attr-defined] + + +def test_applied_defaults_to_false() -> None: + """Proposals start un-applied. ADR-0027 § Teaching Safety: pack + mutation is proposal-only until reviewed.""" + assert _sample_proposal().applied is False + + +def test_default_epistemic_status_is_speculative() -> None: + """ADR-0021 § Schema impact pins SPECULATIVE as the only legal + starting status.""" + assert _sample_proposal().epistemic_status == EpistemicStatus.SPECULATIVE + + +def test_with_status_returns_a_new_instance() -> None: + """``with_status`` must be immutable-update, never in-place.""" + proposal = _sample_proposal() + updated = proposal.with_status(EpistemicStatus.COHERENT) + assert updated is not proposal + assert proposal.epistemic_status == EpistemicStatus.SPECULATIVE + assert updated.epistemic_status == EpistemicStatus.COHERENT + + +def test_no_forbidden_finality_flags_on_proposal() -> None: + """ADR-0021 § non-hardening invariant: no ``final``, ``frozen``, + ``axiom``, or ``permanent`` flag may exist on a proposal.""" + field_names = {f.name for f in dataclasses.fields(PackMutationProposal)} + forbidden = {"final", "frozen", "axiom", "permanent", "immutable"} + leaked = field_names & forbidden + assert leaked == set(), f"forbidden finality flags on proposal: {leaked}" + + +def test_as_dict_round_trips_applied_and_status() -> None: + proposal = _sample_proposal() + d = proposal.as_dict() + assert d["applied"] is False + assert d["epistemic_status"] == EpistemicStatus.SPECULATIVE.value + + +def test_proposal_id_field_is_present() -> None: + """The proposal must carry an opaque identifier so downstream review + paths can refer to it without re-deriving the hash.""" + field_names = {f.name for f in dataclasses.fields(PackMutationProposal)} + assert "proposal_id" in field_names diff --git a/tests/test_safe_display.py b/tests/test_safe_display.py new file mode 100644 index 00000000..2acf12cb --- /dev/null +++ b/tests/test_safe_display.py @@ -0,0 +1,108 @@ +"""Tests for the central safe-display sanitiser (ADR-0051). + +These tests are doctrine guards: they encode the trust boundary so any +future refactor that weakens the sanitiser fails closed in CI before it +can be merged. +""" +from __future__ import annotations + +import pytest + +from core._safe_display import safe_display, safe_pack_id + + +class TestSafeDisplay: + def test_none_collapses_to_empty_mark(self) -> None: + assert safe_display(None) == "" + + def test_empty_string_collapses_to_empty_mark(self) -> None: + assert safe_display("") == "" + + def test_plain_ascii_is_passed_through(self) -> None: + assert safe_display("memory") == "memory" + + def test_control_characters_are_replaced(self) -> None: + # \x1b is ESC — the prefix of every ANSI escape sequence. + assert safe_display("foo\x1b[31mbar") == "foo?[31mbar" + + def test_newline_and_carriage_return_are_replaced(self) -> None: + # Newlines must never reach a log line uncleansed. + assert "\n" not in safe_display("foo\nbar") + assert "\r" not in safe_display("foo\r\nbar") + + def test_null_byte_is_replaced(self) -> None: + assert "\x00" not in safe_display("foo\x00bar") + + def test_c1_control_range_is_replaced(self) -> None: + # The C1 range (0x80..0x9F) is also control characters. + for code in (0x80, 0x90, 0x9F): + assert chr(code) not in safe_display(f"a{chr(code)}b") + + def test_truncation_to_default_max_len(self) -> None: + out = safe_display("x" * 200) + assert len(out) <= 64 + assert out.endswith("...") + + def test_truncation_marker_only_when_needed(self) -> None: + assert safe_display("short") == "short" + assert "..." not in safe_display("short") + + def test_custom_max_len_is_honoured(self) -> None: + out = safe_display("abcdefghij", max_len=8) + assert len(out) <= 8 + assert out.endswith("...") + + def test_zero_max_len_returns_empty_string(self) -> None: + assert safe_display("anything", max_len=0) == "" + + def test_non_string_input_is_repr_coerced(self) -> None: + # Callers cannot smuggle a custom __str__ into a log line. + class Hostile: + def __str__(self) -> str: # pragma: no cover + return "\x1b[31mPWNED" + + out = safe_display(Hostile(), max_len=128) + assert "\x1b" not in out + # repr() is what we coerce through, not str(). + assert "Hostile" in out + + def test_is_deterministic(self) -> None: + # Identical input → byte-identical output, no clock / env. + a = safe_display("test\x1btoken") + b = safe_display("test\x1btoken") + assert a == b + + +class TestSafePackId: + def test_simple_pack_id_passes_through(self) -> None: + assert safe_pack_id("en_core_cognition_v1") == "en_core_cognition_v1" + + def test_path_separators_are_masked(self) -> None: + out = safe_pack_id("../etc/passwd") + assert "/" not in out + assert ".." in out # dots are allowed; slashes are masked + + def test_whitespace_is_masked(self) -> None: + assert " " not in safe_pack_id("foo bar") + + def test_unicode_is_masked(self) -> None: + out = safe_pack_id("café") + assert "é" not in out + + def test_none_collapses_to_empty_mark(self) -> None: + assert safe_pack_id(None) == "" + + def test_empty_collapses_to_empty_mark(self) -> None: + assert safe_pack_id("") == "" + + def test_truncation_for_long_input(self) -> None: + out = safe_pack_id("a" * 200) + assert len(out) <= 48 + assert out.endswith("...") + + +def test_module_exports_are_explicit() -> None: + """``__all__`` lists exactly the public helpers (regression guard).""" + from core import _safe_display as mod + + assert set(mod.__all__) == {"safe_display", "safe_pack_id"}