diff --git a/docs/error-codes.md b/docs/error-codes.md index 032bace..119ebca 100644 --- a/docs/error-codes.md +++ b/docs/error-codes.md @@ -9,31 +9,31 @@ All TRACE test failures emit a structured error code of the form `TR--/` or a `did:` URI | -| TR-ENV-004 | One or more required fields are absent | Add the missing field(s); check the [Schema Reference](https://trace.agentrust-io.com/docs/schema/) for the full required set | +| TR-ENV-004 | `cnf` is absent or not an object, `cnf.jwk` is absent or not an object, or `cnf.jwk.kty` is absent | Populate `cnf.jwk` with at least `kty`. This checks that one field, not the schema's full required set, which structural validation covers | ## TR-SIG — Signature | Code | Description | How to fix | |------|-------------|------------| -| TR-SIG-001 | Signature algorithm is not Ed25519 | Generate an Ed25519 key (`generate_key()`) and re-sign; ES256 and RS256 are not accepted | -| TR-SIG-002 | `cnf.jwk` missing or malformed | Populate `cnf.jwk` with the OKP public key `{"kty":"OKP","crv":"Ed25519","x":"..."}` — `sign_record()` does this automatically | -| TR-SIG-003 | Signature verification failed | Re-sign the record with `sign_record(record, key)`; the record fields must not have changed after signing | -| TR-SIG-004 | Private key material (`d` member) found in `cnf.jwk` | Remove the `d` field before embedding the JWK; `key_to_jwk()` returns the public-only form | +| TR-SIG-001 | In a `cmcp-runtime` envelope: `signature` is missing or empty, or the Ed25519 verification outcome for the claim | Sign the claim with an Ed25519 key and leave the signed fields unchanged. A plain TRACE record reports its signature outcome under TR-SIG-005, not here | +| TR-SIG-002 | In a `cmcp-runtime` envelope: `cnf.jwk` is not an OKP/Ed25519 key, or `cnf.jwk.x` is missing | Populate `cnf.jwk` with the OKP public key `{"kty":"OKP","crv":"Ed25519","x":"..."}`; `sign_record()` does this automatically. A plain TRACE record reports key type under TR-SIG-004 | +| TR-SIG-004 | `cnf.jwk` carries private key material (a `d` member), or `cnf.jwk.kty` is missing or names an unsupported key type (`OKP` and `EC` are accepted) | Remove `d` and embed only the public form of the JWK; `key_to_jwk()` returns it. For key type, use `OKP` or `EC`; Ed25519 signature verification additionally requires `kty: "OKP"` with `crv: "Ed25519"`, and a supported key that is not that pair passes this check and fails TR-SIG-005 | +| TR-SIG-005 | The signature check outcome: the Ed25519 verification result, a signature that cannot be verified, a signature left unchecked because `cnf.jwk` carried private key material, or no signature at all. With no signature it is FAIL at Level 1 and above and `UNVERIFIED` at Level 0, which is not a pass | Sign the record with `sign_record(record, key)` and do not change the signed fields afterwards. An unsigned record is reported as unverified rather than skipped, so it cannot be read as a benign omission | ## TR-RTE — Runtime | Code | Description | How to fix | |------|-------------|------------| -| TR-RTE-001 | `runtime.platform` is not a recognised TEE enum value | Use one of: `software-only`, `tpm2`, `sev-snp`, `tdx`, `opaque` | +| TR-RTE-001 | `runtime` is missing or not an object, or `runtime.platform` is not in the registered set, or is `software-only` at Level 1 and above | Use a value from the `runtime.platform` enum in `schemas/trace-claim.json`. `software-only` carries no hardware attestation evidence and is accepted only at Level 0 | | TR-RTE-002 | `runtime.measurement` is not a valid `sha256:` digest | Provide a 64-character hex digest prefixed with `sha256:`; for Level 0 all-zeros is conventional | -| TR-RTE-003 | RIM URI present but does not resolve to a valid reference image | Remove `runtime.rim_uri` if not using a RIM, or ensure the URI returns a valid reference manifest over HTTPS | +| TR-RTE-003 | `runtime.rim_uri` is present and is not an `https://` URI | Remove `runtime.rim_uri` if not using a RIM, or set it to an `https://` URI. The URI is not resolved and the manifest behind it is not checked; this is a format check | ## TR-POL — Policy | Code | Description | How to fix | |------|-------------|------------| | TR-POL-001 | `policy.bundle_hash` is not a valid `sha256:` digest | Compute `sha256:` + hex digest of your Cedar policy bundle bytes | -| TR-POL-002 | `policy.enforcement_mode` is not `enforce`, `advisory`, or `silent` | Replace `"strict"` or `"monitor"` with `"enforce"`, `"advisory"`, or `"silent"` | +| TR-POL-002 | `policy.enforcement_mode` is not `enforce`, `advisory`, `silent`, or `declared` | Replace `"strict"` or `"monitor"` with one of the four accepted values; `"declared"` is the honest value for a producer that binds a policy without evaluating it | ## TR-TXN — Transcript @@ -46,8 +46,7 @@ All TRACE test failures emit a structured error code of the form `TR---`. +Error codes follow the form `TR--`. A failing or unverified finding +carries its code at the front of the message; a passing one usually does not, so the +module column is what identifies a `PASS`. The JSON and HTML reports carry the code as +its own field for every finding. ## Next steps diff --git a/docs/tutorials/writing-conformance-tests.md b/docs/tutorials/writing-conformance-tests.md index 21e14a1..1a13b72 100644 --- a/docs/tutorials/writing-conformance-tests.md +++ b/docs/tutorials/writing-conformance-tests.md @@ -139,7 +139,7 @@ def test_md5_bundle_hash_fails_tr_pol_001(): def test_unknown_enforcement_mode_fails_tr_pol_002(): trace = _policy_trace( bundle_hash="sha256:" + "b" * 64, - enforcement_mode="strict", # not in {enforce, advisory, silent} + enforcement_mode="strict", # not in {enforce, advisory, silent, declared} ) codes = {f.code for f in tr_pol.check(trace) if f.failed()} assert "TR-POL-002" in codes @@ -230,10 +230,10 @@ Common codes you will encounter: |------|-------|-----| | TR-ENV-001 | `eat_profile` | Must be `tag:agentrust-io.com,2026:trace-v0.2` | | TR-ENV-002 | `iat` | Must be a Unix timestamp in the last 24 hours | -| TR-SIG-001 | `signature` | Signature missing or does not verify | -| TR-SIG-002 | `cnf.jwk` | Key must be OKP/Ed25519 | +| TR-SIG-005 | `signature` | Signature missing, unverifiable, or does not verify | +| TR-SIG-004 | `cnf.jwk` | Supported key type, and no private key material | | TR-POL-001 | `policy.bundle_hash` | Must match `sha256:<64 hex chars>` | -| TR-POL-002 | `policy.enforcement_mode` | Must be `enforce`, `advisory`, or `silent` | +| TR-POL-002 | `policy.enforcement_mode` | Must be `enforce`, `advisory`, `silent`, or `declared` | | TR-RTE-001 | `runtime.platform` | Must be a registered TEE platform enum | When a finding carries `status == Status.UNVERIFIED`, the record has no signature. This is not a benign skip at Level 1 or above. diff --git a/src/trace_tests/modules/tr_sig.py b/src/trace_tests/modules/tr_sig.py index ac40add..071d776 100644 --- a/src/trace_tests/modules/tr_sig.py +++ b/src/trace_tests/modules/tr_sig.py @@ -51,6 +51,14 @@ def _canonical_json(d: dict[str, Any]) -> bytes: def _verify_ed25519(pub_x: str, sig_b64: str, body: bytes) -> tuple[bool, str]: + """Verify *sig_b64* over *body*, returning ``(ok, message)``. + + The messages name no error code. Two callers attach them to findings, ``check`` + under TR-SIG-005 and ``check_cmcp_runtime`` under TR-SIG-001, so a code written + here would contradict one of them and, before this, contradicted both: a + malformed signature was published as a TR-SIG-005 finding whose text read + "TR-SIG-003". The finding's ``code`` is the only place a code belongs. + """ try: from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey from cryptography.exceptions import InvalidSignature @@ -61,18 +69,39 @@ def _verify_ed25519(pub_x: str, sig_b64: str, body: bytes) -> tuple[bool, str]: pub_bytes = _b64url_decode(pub_x) pub_key = Ed25519PublicKey.from_public_bytes(pub_bytes) except Exception as exc: - return False, f"TR-SIG-002: invalid public key in cnf.jwk.x: {exc}" + return False, f"invalid public key in cnf.jwk.x: {exc}" try: sig_bytes = _b64url_decode(sig_b64) except Exception as exc: - return False, f"TR-SIG-003: invalid base64url signature: {exc}" + return False, f"invalid base64url signature: {exc}" try: pub_key.verify(sig_bytes, body) return True, "Ed25519 signature verified" except InvalidSignature: - return False, "TR-SIG-001: signature verification failed" + return False, "signature verification failed" + + +def _jwk_of(container: Any) -> dict[str, Any]: + """The JWK under ``container["cnf"]["jwk"]``, or ``{}`` when it is not an object. + + A malformed record must produce a finding, not an exception. ``runner.run`` calls + every module without a ``try``, so anything raised here ends the run rather than + failing the record, and the caller sees a traceback where a verdict belongs. + + The outer ``isinstance`` is for ``check_cmcp_runtime``, which passes + ``record["trace"]`` and so can hand this anything at all. ``check`` passes the + ``trace`` it was given, which for the plain format is the record itself and is + already a dict; that function reads ``trace`` directly elsewhere and is not + hardened against a non-dict ``trace``. Whether one can reach it is a question + about ``loader.extract_trace``, not about this helper. + """ + if not isinstance(container, dict): + return {} + cnf = container.get("cnf") + jwk = cnf.get("jwk") if isinstance(cnf, dict) else None + return jwk if isinstance(jwk, dict) else {} def check_cmcp_runtime(record: dict[str, Any]) -> list[Finding]: @@ -84,7 +113,7 @@ def check_cmcp_runtime(record: dict[str, Any]) -> list[Finding]: findings.append(Finding("TR-SIG-001", Status.FAIL, "TR-SIG-001: signature field is missing or empty")) return findings - jwk = record.get("trace", {}).get("cnf", {}).get("jwk", {}) + jwk = _jwk_of(record.get("trace")) kty = jwk.get("kty") crv = jwk.get("crv") x = jwk.get("x") @@ -117,16 +146,24 @@ def check(trace: dict[str, Any], record: dict[str, Any], fmt: str, level: int = return check_cmcp_runtime(record) findings: list[Finding] = [] - jwk = trace.get("cnf", {}).get("jwk", {}) + jwk = _jwk_of(trace) kty = jwk.get("kty") crv = jwk.get("crv") x = jwk.get("x") if "d" in jwk: findings.append(Finding( - rule="TR-SIG-002", - status=Status.FAIL, - message="cnf.jwk must not contain private key material ('d' field present in JWK)", + "TR-SIG-004", Status.FAIL, + "TR-SIG-004: cnf.jwk must not contain private key material " + "('d' member present in the JWK)", + )) + # The signature is not checked against a key the record should never have + # carried. Say so rather than returning nothing: a consumer reading TR-SIG-005 + # to learn whether the signature was verified would otherwise find no finding + # at all, which is the benign-omission reading UNVERIFIED exists to prevent. + findings.append(Finding( + "TR-SIG-005", Status.UNVERIFIED, + "TR-SIG-005: signature not checked; cnf.jwk carries private key material", )) return findings diff --git a/tests/test_docs_match_the_modules.py b/tests/test_docs_match_the_modules.py new file mode 100644 index 0000000..3f3547d --- /dev/null +++ b/tests/test_docs_match_the_modules.py @@ -0,0 +1,188 @@ +"""The published error codes and record samples must agree with the modules. + +Every check here comes from drift that was live in `main`, not from first principles. +`TR-SIG-005` was carried by every signature finding and documented nowhere. `TR-ANC-002` +was documented in three files and named by no module, with two descriptions that +disagreed. `docs/levels.md` showed an `anchor` object the closed schema does not define, +and a `runtime.platform` of `sev-snp`, which is not in the enum. + +Documentation that disagrees with the code is worse than absent documentation: it reads +as verified. Nothing checked it, which is why all of it survived. +""" +from __future__ import annotations + +import ast +import json +import pathlib +import re +from typing import Any + +import jsonschema + +REPO = pathlib.Path(__file__).resolve().parents[1] +MODULES = REPO / "src" / "trace_tests" / "modules" +ERROR_CODES = REPO / "docs" / "error-codes.md" +SCHEMA = REPO / "schemas" / "trace-claim.json" +DOCS = REPO / "docs" + +_CODE = re.compile(r"TR-[A-Z]{3}-\d{3}") +_DOCUMENTED_ROW = re.compile(r"^\| (TR-[A-Z]{3}-\d{3}) ", re.M) +_JSON_BLOCK = re.compile(r"```json\n(.*?)```", re.S) + + +def _codes(text: str) -> set[str]: + return set(_CODE.findall(text)) + + +def _codes_in_code(source: str) -> set[str]: + """Codes that appear in a module's string literals, ignoring prose about them. + + Comments and docstrings are not the module naming a code, they are the module + talking about one. Counting them let a code stay "named" after its last real use + was removed: a docstring in ``tr_sig`` explaining why ``TR-SIG-003`` must not + appear in a message was, on its own, enough to keep the deleted code looking + alive. ``ast`` drops comments, and module, class and function docstrings are + skipped explicitly. + """ + tree = ast.parse(source) + docstrings = set() + for node in ast.walk(tree): + if isinstance(node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)): + body = getattr(node, "body", None) + if body and isinstance(body[0], ast.Expr) and isinstance(body[0].value, ast.Constant) \ + and isinstance(body[0].value.value, str): + docstrings.add(id(body[0].value)) + + found: set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Constant) or not isinstance(node.value, str): + continue + if id(node) in docstrings: + continue + found |= _codes(node.value) + return found + + +def test_the_code_set_named_by_the_modules_matches_the_code_set_documented() -> None: + """Matched on codes *named in* module source, not on what a ``Finding`` carries. + + Those two sets are the same today, measured, and were not: ``TR-SIG-003`` used to + appear only inside a message string that a ``TR-SIG-005`` finding carried, so it + was never a ``Finding.code``, and matching on ``Finding.code`` would have demanded + deleting a row that documented a real condition. The branch that removed that + prefix closed the gap. The match stays on literals anyway: this reads source text + and cannot tell which literal reaches a ``Finding``, and a message naming its own + code is the convention here rather than a defect. + + Codes are read from string literals via ``ast``, not from the file text, so a + comment or docstring about a code does not count as naming it. + + This is set membership in both directions and nothing more. It cannot tell whether + a row describes what its code reports, which was a second kind of drift and was + live here too: the ``TR-SIG-004`` row described private key material in ``cnf.jwk``, + a condition the module never reports under that code. Catching that would mean + comparing prose to behaviour, so the rows are checked by reading them. + """ + named = set() + for path in sorted(MODULES.glob("*.py")): + named |= _codes_in_code(path.read_text(encoding="utf-8")) + # A row, not a mention. A code named in passing somewhere on the page is not + # documented, and treating it as documented would let the check be satisfied + # by prose that tells a reader nothing. + documented = set(_DOCUMENTED_ROW.findall(ERROR_CODES.read_text(encoding="utf-8"))) + + assert named == documented, ( + f"named by a module, undocumented: {sorted(named - documented)}\n" + f"documented, named by no module: {sorted(documented - named)}\n" + f"Add the row to {ERROR_CODES.relative_to(REPO)}, or delete it. A code in one " + "place and not the other is a claim nobody checked." + ) + + +def _without_required(node: Any) -> Any: + """The schema with every ``required`` list dropped, except inside ``if`` and ``not``. + + The documented samples are fragments: "changes from Level 1", not whole records. + Validating them as published fails on absent fields and says nothing about the + fields that are present. Dropping ``required`` leaves every statement about a value + that *is* there: ``additionalProperties``, ``enum``, ``pattern``, ``type``. + + ``if`` and ``not`` are left intact deliberately. Stripping ``required`` from an + ``if`` makes it vacuously true, which fires the matching ``then`` against records + the condition was never meant to reach. The schema's ``origin`` rule does exactly + that: strip its ``if`` and every sample is required to be ``software-only``. + """ + if isinstance(node, dict): + return { + key: value if key in ("if", "not") else _without_required(value) + for key, value in node.items() + if key != "required" + } + if isinstance(node, list): + return [_without_required(item) for item in node] + return node + + +def _drop_elisions(node: Any) -> Any: + """Remove string values that are visibly abbreviated for the page. + + A documented sample writes a signature as ``eyJhbGciOiJFZERTQSJ9...``. That is a + reader's placeholder, not a claim about the format, and holding it to the schema's + base64url pattern would report the page style as a defect. + """ + if isinstance(node, dict): + return {k: _drop_elisions(v) for k, v in node.items() + if not (isinstance(v, str) and "..." in v)} + if isinstance(node, list): + return [_drop_elisions(v) for v in node] + return node + + +def test_every_json_sample_in_the_docs_agrees_with_the_packaged_schema() -> None: + """Two drifts lived here: an ``anchor`` object the closed schema does not define, + and a ``runtime.platform`` of ``sev-snp``, which is not in the enum. A reader + copying either sample produced a record this suite rejects. + + Every ``.md`` under ``docs/`` is scanned rather than a list of pages kept by hand, + because a hand-maintained list of what gets checked is the same defect this exists + to catch, in the one place it would not show. + + Prose lists of valid values are not checked, because checking them means reading + them. ``docs/error-codes.md``, ``docs/levels.md`` and ``docs/modules/tr-rte.md`` all + listed platform values that do not exist; only the sample was mechanically catchable. + """ + schema = json.loads(SCHEMA.read_text(encoding="utf-8")) + assert schema.get("additionalProperties") is False, ( + "This test assumes the packaged schema is closed. If that changed, an unknown " + "field in a sample is no longer necessarily an error and this needs rewriting." + ) + validator = jsonschema.Draft202012Validator(_without_required(schema)) + + failures: dict[str, list[str]] = {} + validated = 0 + unparsed = 0 + for page in sorted(DOCS.rglob("*.md")): + for block in _JSON_BLOCK.findall(page.read_text(encoding="utf-8")): + try: + sample = json.loads(block) + except json.JSONDecodeError: + unparsed += 1 + continue # prose-annotated fragment, not a record + if not isinstance(sample, dict): + continue + validated += 1 + errors = [e.message for e in validator.iter_errors(_drop_elisions(sample))] + if errors: + failures.setdefault(str(page.relative_to(REPO)), []).extend(errors) + + assert not failures, ( + f"JSON samples in the documentation disagree with {SCHEMA.name}, so a reader " + f"copying one gets a record this suite rejects: {failures}" + ) + # Without this the check degrades to nothing the moment the samples stop parsing + # or the fences change, and it degrades silently, reporting a pass over no work. + assert validated, ( + f"no JSON object sample under {DOCS.relative_to(REPO)} was validated " + f"({unparsed} block(s) did not parse). Either the samples are gone or the fence " + "this reads has changed; a check over nothing must not report a pass." + ) diff --git a/tests/test_findings_are_self_consistent.py b/tests/test_findings_are_self_consistent.py new file mode 100644 index 0000000..5a437b9 --- /dev/null +++ b/tests/test_findings_are_self_consistent.py @@ -0,0 +1,150 @@ +"""A finding's message must not name an error code other than its own. + +`report.py` publishes both: the JSON artifact carries `code` per finding and the HTML +table prints it beside the text. When they disagree, the reader has two codes for one +result and no way to tell which the tooling meant. + +They did disagree. `_verify_ed25519` returned messages prefixed with `TR-SIG-001`, +`TR-SIG-002` and `TR-SIG-003`, and two callers attached those messages to findings of +their own: `check` under `TR-SIG-005` and `check_cmcp_runtime` under `TR-SIG-001`. A +record with a malformed signature was published as a TR-SIG-005 finding reading +"TR-SIG-003: invalid base64url signature", so the forwarded report named a code the +suite had not used and the docs describe as something else. + +Naming a module's own code in its message is the convention everywhere else here and +is left alone; this only rejects naming a different one. +""" +from __future__ import annotations + +import copy +import inspect +import json +import pathlib +import re +from typing import Any + +import pytest + +from trace_tests.modules import tr_anc, tr_env, tr_pol, tr_rte, tr_sca, tr_sig, tr_txn +from trace_tests.result import Finding + +VECTORS = pathlib.Path(__file__).resolve().parent / "vectors" + +MODULES = { + "tr_env": tr_env, "tr_sig": tr_sig, "tr_pol": tr_pol, "tr_rte": tr_rte, + "tr_txn": tr_txn, "tr_anc": tr_anc, "tr_sca": tr_sca, +} + +_CODE = re.compile(r"TR-[A-Z]{3}-\d{3}") + +#: Values chosen to reach the error branches rather than to be exhaustive: a bad +#: base64 key, a key of the wrong length, a signature that decodes but does not +#: verify, and a signature that does not decode at all. +SIGNATURE_CASES: tuple[Any, ...] = ("!!!not-base64!!!", "AAAA", 123, None, "", [1]) + + +def _plain() -> dict[str, Any]: + raw = json.loads((VECTORS / "signed_root.json").read_text(encoding="utf-8")) + return dict(raw.get("record", raw)) + + +def _cmcp() -> dict[str, Any]: + return json.loads((VECTORS / "valid_cmcp_runtime.json").read_text(encoding="utf-8")) + + +def _call(module: Any, record: dict[str, Any], fmt: str = "trace") -> list[Finding]: + params = list(inspect.signature(module.check).parameters) + if params[:3] == ["trace", "record", "fmt"]: + trace = record.get("trace", record) if fmt == "cmcp-runtime" else record + return list(module.check(trace, record, fmt, 0)) + if "level" in params: + return list(module.check(record, 0)) + return list(module.check(record)) + + +def _foreign_codes(findings: list[Finding]) -> list[str]: + out = [] + for finding in findings: + for named in _CODE.findall(finding.message): + if named != finding.code: + out.append(f"{finding.code} message names {named}: {finding.message}") + return out + + +@pytest.mark.parametrize("name", sorted(MODULES)) +def test_no_finding_names_a_code_other_than_its_own(name: str) -> None: + module = MODULES[name] + offenders: list[str] = [] + + records = [_plain()] + for field in ("signature", "cnf", "runtime", "policy", "appraisal", "transparency"): + for junk in SIGNATURE_CASES: + record = _plain() + record[field] = junk + records.append(record) + for junk in SIGNATURE_CASES: + record = _plain() + record["cnf"]["jwk"]["x"] = junk + records.append(record) + record = _plain() + record["cnf"]["jwk"] = junk + records.append(record) + record = _plain() + record["cnf"]["jwk"]["d"] = "AAAA" + records.append(record) + + examined = 0 + for record in records: + try: + findings = _call(module, copy.deepcopy(record)) + except Exception: # noqa: BLE001 - covered by test_modules_never_raise + continue + examined += len(findings) + offenders += _foreign_codes(findings) + + assert not offenders, ( + f"{name} publishes findings whose message names a different code:\n " + + "\n ".join(sorted(set(offenders))) + ) + # Every record here is skipped when the module raises, so without this the check + # reports a pass over nothing the moment a module stops returning findings at all. + assert examined, ( + f"no finding from {name} was examined: every record raised, so this check " + "looked at nothing and would have reported a pass either way" + ) + + +def test_the_cmcp_path_also_keeps_its_findings_self_consistent() -> None: + """The other caller of the same helper, which the parametrised test never reaches. + + ``check`` attaches those messages under TR-SIG-005 and ``check_cmcp_runtime`` + attaches them under TR-SIG-001. A code written in the helper could only ever match + one of the two, which is the reason it names none. + """ + offenders: list[str] = [] + examined = 0 + for junk in SIGNATURE_CASES: + for path in (("signature",), ("trace", "cnf", "jwk", "x")): + record = _cmcp() + node: Any = record + ok = True + for key in path[:-1]: + if not isinstance(node.get(key), dict): + ok = False + break + node = node[key] + if not ok: + continue + node[path[-1]] = junk + try: + findings = _call(tr_sig, record, "cmcp-runtime") + except Exception: # noqa: BLE001 - covered by test_modules_never_raise + continue + examined += len(findings) + offenders += _foreign_codes(findings) + + assert not offenders, ( + "the cmcp path publishes findings whose message names a different code:\n " + + "\n ".join(sorted(set(offenders))) + ) + assert examined, "no cmcp finding was examined: every envelope raised" diff --git a/tests/test_modules_never_raise.py b/tests/test_modules_never_raise.py new file mode 100644 index 0000000..f03b830 --- /dev/null +++ b/tests/test_modules_never_raise.py @@ -0,0 +1,246 @@ +"""A malformed record must produce a finding, never an exception. + +``runner.run`` calls every module directly, with no ``try``. A module that raises on +a record it does not understand ends the whole run: the caller gets a traceback where +a verdict belongs, and the record is neither passed nor failed. + +``tr_sig`` did exactly that in five ways. ``Finding(rule=...)`` raised ``TypeError`` on +the one check meant to catch a record that embeds its own private key, and reading +``cnf`` or ``cnf.jwk`` raised ``AttributeError`` whenever either was not an object. The +packaged schema does not forbid a ``d`` member in ``cnf.jwk``, so nothing rejected such +a record before the module saw it. The other six modules already guarded their inputs +with ``isinstance``; this pins that for all seven. + +Scope: the record is a dict throughout and its *fields* are malformed. The cmcp case +below hands ``check`` an envelope whose ``trace`` is junk, but it does so directly. +``loader.extract_trace`` does return ``record["trace"]`` unchecked, so reading it in +isolation suggests a hole; there is not one on the path the tool takes, because +``load_record`` refuses a cmcp envelope whose ``trace`` is not a dict before that runs, +and ``extract_trace`` is unexported with ``runner.run`` as its only caller. A library +caller that assembles a record by hand and calls ``runner.run`` without the loader can +still reach it. +""" +from __future__ import annotations + +import copy +import inspect +import json +import pathlib +from typing import Any + +import pytest + +from trace_tests.modules import tr_anc, tr_env, tr_pol, tr_rte, tr_sca, tr_sig, tr_txn +from trace_tests.result import Finding, Status +from trace_tests.runner import run + +VECTORS = pathlib.Path(__file__).resolve().parent / "vectors" + +MODULES = { + "tr_env": tr_env, "tr_sig": tr_sig, "tr_pol": tr_pol, "tr_rte": tr_rte, + "tr_txn": tr_txn, "tr_anc": tr_anc, "tr_sca": tr_sca, +} + +#: Values a record can carry where an object or a string is expected. `True` is here +#: because `isinstance(True, int)`; `False` and `0` because a bare truthiness test +#: reads them as absent, which is a different branch from a wrong type. +JUNK: tuple[Any, ...] = ("a-string", 123, None, [1, 2], True, False, 0, {}, "") + +TOP_LEVEL = ( + "cnf", "runtime", "policy", "tool_transcript", "build_provenance", + "transparency", "appraisal", "signature", "model", "subject", "iat", +) + + +def _record() -> dict[str, Any]: + raw = json.loads((VECTORS / "signed_root.json").read_text(encoding="utf-8")) + return dict(raw.get("record", raw)) + + +def _call(module: Any, record: dict[str, Any]) -> list[Finding]: + """Invoke a module's ``check`` whatever its parameter list happens to be. + + Read from the signature rather than written down here, so a module that gains a + parameter is still exercised instead of quietly dropping out of this test. + """ + params = list(inspect.signature(module.check).parameters) + if params[:3] == ["trace", "record", "fmt"]: + return list(module.check(record, record, "trace", 0)) + if "level" in params: + return list(module.check(record, 0)) + return list(module.check(record)) + + +def _mutations() -> list[tuple[str, dict[str, Any]]]: + cases: list[tuple[str, dict[str, Any]]] = [] + for field in TOP_LEVEL: + for junk in JUNK: + record = _record() + record[field] = junk + cases.append((f"{field}={junk!r}", record)) + for junk in JUNK: + record = _record() + if isinstance(record.get("cnf"), dict): + record["cnf"]["jwk"] = junk + cases.append((f"cnf.jwk={junk!r}", record)) + record = _record() + record["cnf"]["jwk"]["d"] = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + cases.append(("cnf.jwk carries d", record)) + # Absence is its own class. Replacing a field with junk never exercises the branch + # a module takes when the key is not there at all. + for field in TOP_LEVEL: + record = _record() + record.pop(field, None) + cases.append((f"{field} removed", record)) + record = _record() + if isinstance(record.get("cnf"), dict): + record["cnf"].pop("jwk", None) + cases.append(("cnf.jwk removed", record)) + return cases + + +@pytest.mark.parametrize("name", sorted(MODULES)) +def test_no_module_raises_on_a_record_whose_fields_are_malformed(name: str) -> None: + module = MODULES[name] + raised: list[str] = [] + for label, record in _mutations(): + try: + findings = _call(module, copy.deepcopy(record)) + except Exception as exc: # noqa: BLE001 - the point is that nothing escapes + raised.append(f"{label} -> {type(exc).__name__}: {exc}") + continue + if not findings: + raised.append(f"{label} -> returned no findings at all") + + assert not raised, ( + f"{name}.check raised or returned nothing on {len(raised)} malformed record(s). " + f"runner.run has no try, so each of these ends the run instead of failing the " + f"record:\n " + "\n ".join(raised) + ) + + +def test_a_record_embedding_its_own_private_key_fails_rather_than_raising() -> None: + """The condition the check exists for, named as its own case. + + The generic test above would pass if this raised in a module that had no such + check at all. This one asserts the verdict, so removing the check fails here + rather than going unnoticed. + """ + record = _record() + record["cnf"]["jwk"]["d"] = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + findings = tr_sig.check(record, record, "trace", 0) + + leak = [f for f in findings if f.status is Status.FAIL and "private key" in f.message] + assert leak, f"no finding reports the embedded private key: {findings}" + assert leak[0].code == "TR-SIG-004", ( + f"the leak is reported under {leak[0].code!r}; docs/error-codes.md documents " + "this condition under TR-SIG-004" + ) + + +def test_a_leaked_key_still_reports_whether_the_signature_was_checked() -> None: + """The leak check returns early, so nothing else in the module runs. + + Before this branch that path raised, so the state was unreachable and no consumer + had met it. Making it reachable without a TR-SIG-005 would publish a record with no + signature verdict of any kind: not pass, not fail, not unverified. One consumer in + this suite already reads that finding with a bare ``next``. + """ + record = _record() + record["cnf"]["jwk"]["d"] = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + findings = tr_sig.check(record, record, "trace", 0) + by_code = {f.code: f for f in findings} + + assert "TR-SIG-005" in by_code, ( + f"a leaked-key record reports no signature verdict at all: {findings}" + ) + assert by_code["TR-SIG-005"].status is Status.UNVERIFIED, ( + "the signature was not checked, so it is unverified rather than passed or failed" + ) + assert by_code["TR-SIG-004"].status is Status.FAIL + + +@pytest.mark.parametrize("level", [0, 1, 2]) +def test_the_runner_completes_on_a_record_that_embeds_its_own_private_key(level: int) -> None: + """The regression as it was actually met, one layer above the module. + + The traceback came out of ``runner.run``, which calls each module with no ``try``. + Testing ``tr_sig.check`` alone would still pass if some later change moved the same + failure into the runner, so the path that broke is exercised here as well. + """ + record = _record() + record["cnf"]["jwk"]["d"] = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + results = run(record, "trace", level) + + sig = results["TR-SIG"] + assert sig, "the runner produced no TR-SIG findings for a record it should reject" + assert any(f.code == "TR-SIG-004" and f.status is Status.FAIL for f in sig), sig + assert any(f.code == "TR-SIG-005" and f.status is Status.UNVERIFIED for f in sig), sig + + +def test_no_finding_from_any_module_repeats_the_key_it_found() -> None: + """A finding about a leaked private key must not carry the key. + + Findings travel: ``report.py`` publishes every message into a JSON and an HTML + artifact meant to be forwarded. A message that quoted the offending value to be + helpful would copy the private key into the thing the reader sends on. The report + itself carries only the record's digest, so a message is the only place this can + go wrong, and it can go wrong in any module rather than only the one that reports + the leak. + """ + secret = "Zm9yYmlkZGVuLXByaXZhdGUta2V5LW1hdGVyaWFs" + record = _record() + record["cnf"]["jwk"]["d"] = secret + + offenders = [] + for name, module in sorted(MODULES.items()): + for finding in _call(module, copy.deepcopy(record)): + if secret in finding.message: + offenders.append(f"{name} {finding.code}: {finding.message}") + + assert not offenders, ( + "a finding repeats the private key it is reporting:\n " + "\n ".join(offenders) + ) + + +def _cmcp_record() -> dict[str, Any]: + return json.loads((VECTORS / "valid_cmcp_runtime.json").read_text(encoding="utf-8")) + + +def test_the_cmcp_path_does_not_raise_on_a_malformed_envelope() -> None: + """The other entry point, which the parametrised test above never reaches. + + ``check`` dispatches to ``check_cmcp_runtime`` on ``fmt == "cmcp-runtime"`` and + every case above passes ``"trace"``, so the branch that reads ``record["trace"]`` + three levels deep was hardened without being exercised. It reads a value the caller + supplies rather than the extracted trace, so it can be handed anything. + """ + raised: list[str] = [] + for junk in JUNK: + for path in (("trace",), ("trace", "cnf"), ("trace", "cnf", "jwk"), ("signature",)): + record = _cmcp_record() + node: Any = record + for key in path[:-1]: + if not isinstance(node, dict) or not isinstance(node.get(key), dict): + node = None + break + node = node[key] + if node is None: + continue + node[path[-1]] = junk + label = ".".join(path) + f"={junk!r}" + try: + findings = tr_sig.check(record.get("trace", {}), record, "cmcp-runtime", 0) + except Exception as exc: # noqa: BLE001 - the point is that nothing escapes + raised.append(f"{label} -> {type(exc).__name__}: {exc}") + continue + if not findings: + raised.append(f"{label} -> returned no findings at all") + + assert not raised, ( + "tr_sig.check on a cmcp envelope raised or returned nothing:\n " + "\n ".join(raised) + ) +