fix(release): the reference verifier refuses a commit or a digest that ends in a newline

tests/e3_mutants.py

@@ -28,9 +28,9 @@ def m(id, old, new, kills=(), equivalent=None):
       ["envelope-legacy-v1-no-provenance-no-envelope", "real_legacy"]),
     # source commit
     m("source-commit-empty-accepted-as-form", 'if value is None or value == "":', "if value is None:", ["source-commit-empty"]),
-    m("source-commit-length-open", "[0-9a-f]{40}$", "[0-9a-f]{40,}$", ["source-commit-41-digits"]),
-    m("source-commit-uppercase-allowed", 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}$")', 'COMMIT_RE = re.compile(r"^[0-9a-fA-F]{40}$")', ["source-commit-uppercase"]),
-    m("source-commit-end-not-anchored", 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}$")', 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}")', ["source-commit-41-digits"]),
+    m("source-commit-length-open", "[0-9a-f]{40}\\Z", "[0-9a-f]{40,}\\Z", ["source-commit-41-digits"]),
+    m("source-commit-uppercase-allowed", 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}\\Z")', 'COMMIT_RE = re.compile(r"^[0-9a-fA-F]{40}\\Z")', ["source-commit-uppercase"]),
+    m("source-commit-end-not-anchored", 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}\\Z")', 'COMMIT_RE = re.compile(r"^[0-9a-f]{40}")', ["source-commit-41-digits"]),
     # artifacts block
     m("artifacts-empty-list-accepted", "if not files:\n        return \"artifacts_missing\"", "if files is None:\n        return \"artifacts_missing\"", ["artifacts-files-empty"]),
     m("artifacts-non-list-accepted", "return files if isinstance(files, list) else None", "return files", ["artifacts-files-not-a-list"]),
@@ -57,12 +57,12 @@ def m(id, old, new, kills=(), equivalent=None):
     m("order-equal-allowed", "if not path_key(before[\"path\"]) < path_key(after[\"path\"]):", "if path_key(before[\"path\"]) > path_key(after[\"path\"]):",
       equivalent="equal neighbours are the duplicate check's, which runs first; with it in place the order check is never offered an equal pair"),
     # digests and manifest
-    m("digest-prefix-optional", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^(sha256:)?[0-9a-f]{64}$")', ["digest-no-prefix"]),
-    m("digest-uppercase-allowed", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-fA-F]{64}$")', ["digest-uppercase"]),
-    m("digest-length-open-above", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64,}$")', ["digest-65-digits"]),
-    m("digest-length-open-below", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{63,64}$")', ["digest-63-digits"]),
-    m("digest-any-algorithm", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^sha[0-9]+:[0-9a-f]{64}$")', ["digest-other-algorithm"]),
-    m("digest-end-not-anchored", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}")', ["digest-65-digits"]),
+    m("digest-prefix-optional", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^(sha256:)?[0-9a-f]{64}\\Z")', ["digest-no-prefix"]),
+    m("digest-uppercase-allowed", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-fA-F]{64}\\Z")', ["digest-uppercase"]),
+    m("digest-length-open-above", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64,}\\Z")', ["digest-65-digits"]),
+    m("digest-length-open-below", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{63,64}\\Z")', ["digest-63-digits"]),
+    m("digest-any-algorithm", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^sha[0-9]+:[0-9a-f]{64}\\Z")', ["digest-other-algorithm"]),
+    m("digest-end-not-anchored", 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}\\Z")', 'DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}")', ["digest-65-digits"]),
     m("manifest-digest-form-not-checked", "if not isinstance(declared, str) or not DIGEST_RE.match(declared):", "if False:", ["manifest-digest-absent", "manifest-digest-no-prefix"]),
     m("manifest-digest-form-reported-as-the-file-reason", 'return "artifact_manifest_digest_form", "artifacts.manifest_digest is absent', 'return "artifact_digest_form", "artifacts.manifest_digest is absent', ["manifest-digest-absent", "manifest-digest-no-prefix"]),
     m("file-digest-form-reported-as-the-manifest-reason", 'return "artifact_digest_form", "files[%d].digest', 'return "artifact_manifest_digest_form", "files[%d].digest', ["digest-no-prefix", "file-digest-error-wins"]),

tests/e3_vectors_fullmatch.py

@@ -0,0 +1,30 @@
+"""Negative vectors for a value that ends in a newline (the follow-up to PR #25; docs/en/RELEASE-RECORD-CONTRACT.md sections 5 and 6).
+
+The contract says a commit is 40 lowercase hexadecimal digits and a digest is `sha256:` followed by 64 lowercase hexadecimal digits. In Python an end anchor `$` also
+matches in front of a final newline, so the first version of the verifier accepted `<40 hex>\\n` and `sha256:<64 hex>\\n`. These vectors are the proof of that defect (they
+fail against the verifier of d2d8ec7) and the guard of its fix. They are kept apart from tests/e3_vectors.py, which is frozen (tests/fixtures/envelope3/FROZEN.sha256).
+
+The same defects are applied to the golden record and re-authenticated, like every vector of e3_vectors.py, so the refusal has to come from the contract check.
+"""
+from e3_vectors import FIX, s, v
+
+COMMIT = "f0cf873c0e2f889d440cc7411535a63a89a41e9f"            # the commit of the worked example
+FILE_DIGEST = "sha256:b6a98d9ce9a2d9149288fa3df42d377c3e42737afdcdaf714e33c0a100b51060"       # artifacts.files.0.digest of the golden record
+MANIFEST_DIGEST = "sha256:39e022575a650d2c2cf04a4d65ae8744e04d0ccb3699fb417db9c4c160b2ec53"   # artifacts.manifest_digest of the golden record
+
+# the three forms the contract states, each followed by a final newline
+NEWLINE = [
+    v("source-commit-trailing-newline", "record", "source_commit_form", [s("provenance.source_commit", COMMIT + "\n")]),
+    v("digest-trailing-newline", "record", "artifact_digest_form", [s("artifacts.files.0.digest", FILE_DIGEST + "\n"), FIX]),
+    v("manifest-digest-trailing-newline", "record", "artifact_manifest_digest_form", [s("artifacts.manifest_digest", MANIFEST_DIGEST + "\n")]),
+]
+
+# controls: they are refused before and after the fix, so they show the boundary of the defect and not a new refusal
+CONTROLS = [
+    v("source-commit-two-trailing-newlines", "record", "source_commit_form", [s("provenance.source_commit", COMMIT + "\n\n")]),
+    v("digest-trailing-carriage-return", "record", "artifact_digest_form", [s("artifacts.files.0.digest", FILE_DIGEST + "\r"), FIX]),
+    v("digest-leading-newline", "record", "artifact_digest_form", [s("artifacts.files.0.digest", "\n" + FILE_DIGEST), FIX]),
+    v("manifest-digest-trailing-space", "record", "artifact_manifest_digest_form", [s("artifacts.manifest_digest", MANIFEST_DIGEST + " ")]),
+]
+
+ALL = NEWLINE + CONTROLS

tests/fixtures/envelope3/AMENDMENTS.yaml

@@ -0,0 +1,18 @@
+# Amendments to the frozen yardstick of PR #25 (merge commit d2d8ec7).
+#
+# tests/fixtures/envelope3/FROZEN.sha256 is NOT edited: it keeps the digests of the accepted state and is still checked against the blobs of d2d8ec7.
+# A file of the yardstick may differ from that state only if it is listed here, with the digest it had (from_sha256, which has to be the one in FROZEN.sha256) and the digest it
+# has now (to_sha256, which has to be the working tree), and a reason. tests/test_conformance_frozen.py checks all three, so an amendment cannot be added silently.
+amendments:
+  - path: tools/release_record_conformance.py
+    from_sha256: 6e9f14d99768aa28c734e258d58f6bc6d11438c3149e005bb5b20ad9d1742560
+    to_sha256: 5ada2263895d1257e31a0ecc52c3007ac03788ced2ef64170b734a3bc8a29183
+    change: >-
+      DIGEST_RE and COMMIT_RE end with \Z instead of $. In Python $ also matches in front of a final newline, so the verifier accepted "<40 hex>\n" as a source_commit and "sha256:<64 hex>\n" as an artifact digest, and gave the wrong reason for a manifest digest with a final newline.
+    vectors: "tests/e3_vectors_fullmatch.py; the proof is tests/test_release_record_conformance_fullmatch.py"
+  - path: tests/e3_mutants.py
+    from_sha256: 5de8fa3d9d1d0855d87454b9c99f3f493b03898871813d0387c06fc9d5cdbf2a
+    to_sha256: 6a473876174cc2c518898802a9e56e3e3768ea1c6d6a886448dbdcc896cb43fb
+    change: >-
+      Mechanical: the nine mutants that rewrite DIGEST_RE or COMMIT_RE carry the text of the changed lines (the end anchor $ is \Z in them). No mutant was added, removed or retargeted.
+    vectors: "tests/e3_vectors_fullmatch.py; the proof is tests/test_release_record_conformance_fullmatch.py"

tests/test_conformance_frozen.py

@@ -7,16 +7,22 @@
 
 Two readings: the working tree against the list (always), and, where the history is present (the gate fetches all of it), the list against the blobs of the merge
 commit itself, so the list cannot be edited together with a file without the mismatch showing.
+
+A change of the yardstick that is wanted goes through tests/fixtures/envelope3/AMENDMENTS.yaml, never through an edit of the list: an amendment names the file, the digest it
+had (which has to be the one of the list), the digest it has now (which has to be the working tree) and the reason, so that the difference to the accepted state is one
+reviewable file. The list itself keeps the digests of d2d8ec7 and is still compared with its blobs.
 """
 import hashlib
 import os
 import pathlib
 import subprocess
 
 import pytest
+import yaml
 
 ROOT = pathlib.Path(__file__).resolve().parents[1]
 LIST = ROOT / "tests" / "fixtures" / "envelope3" / "FROZEN.sha256"
+AMENDMENTS = ROOT / "tests" / "fixtures" / "envelope3" / "AMENDMENTS.yaml"
 MERGE_COMMIT = "d2d8ec7422490053644779d3cca9fce388bb2886"
 REQUIRED = os.environ.get("CIC_REQUIRE_RELEASE_HISTORY") == "1"
 
@@ -29,6 +35,18 @@ def frozen():
     return out
 
 
+def amendments():
+    return (yaml.safe_load(AMENDMENTS.read_text(encoding="utf-8")) or {}).get("amendments") or []
+
+
+def expected():
+    """The digest each file has to have now: the frozen one, or the `to_sha256` of its amendment."""
+    out = dict(frozen())
+    for a in amendments():
+        out[a["path"]] = a["to_sha256"]
+    return out
+
+
 def has_commit(commit):
     return subprocess.run(["git", "-C", str(ROOT), "cat-file", "-e", commit + "^{commit}"], capture_output=True).returncode == 0
 
@@ -42,9 +60,26 @@ def test_the_list_names_the_yardstick():
 
 
 @pytest.mark.parametrize("path", sorted(frozen()))
-def test_the_working_tree_matches_the_frozen_digest(path):
-    assert hashlib.sha256((ROOT / path).read_bytes()).hexdigest() == frozen()[path], (
-        "%s differs from the state accepted in %s: change the yardstick in a change of its own, not together with the compiler" % (path, MERGE_COMMIT[:7]))
+def test_the_working_tree_matches_the_frozen_digest_or_its_amendment(path):
+    assert hashlib.sha256((ROOT / path).read_bytes()).hexdigest() == expected()[path], (
+        "%s differs from the state accepted in %s and from its amendment, if it has one: change the yardstick in a change of its own that lists it in AMENDMENTS.yaml, "
+        "not together with the compiler" % (path, MERGE_COMMIT[:7]))
+
+
+def test_every_amendment_starts_from_the_frozen_digest_and_changes_something():
+    seen = set()
+    for a in amendments():
+        assert a["path"] in frozen(), "%s is not part of the frozen yardstick" % a["path"]
+        assert a["path"] not in seen, "%s is amended twice: one amendment per file, from the accepted state" % a["path"]
+        seen.add(a["path"])
+        assert a["from_sha256"] == frozen()[a["path"]], "the amendment of %s does not start from the frozen digest" % a["path"]
+        assert a["to_sha256"] != a["from_sha256"], "the amendment of %s changes nothing" % a["path"]
+        assert len(a["to_sha256"]) == 64 and str(a.get("change", "")).strip(), "the amendment of %s has no digest or no reason" % a["path"]
+
+
+def test_the_released_records_are_not_amendable():
+    """A release record is evidence, not part of the yardstick that may move: the list holds the v0.1.5 record and no amendment may touch it."""
+    assert all(not a["path"].startswith("release/") for a in amendments())
 
 
 @pytest.mark.parametrize("path", sorted(frozen()))

tests/test_release_record_conformance_fullmatch.py

@@ -0,0 +1,90 @@
+"""A value that ends in a newline is not a commit and not a digest (follow-up to PR #25; vectors in tests/e3_vectors_fullmatch.py).
+
+Three things are shown, not one: (1) the vectors are refused by the verifier now; (2) they were NOT refused by the verifier of d2d8ec7, read from git, so the defect is
+reproduced and not remembered; (3) the fix changes the verdict on no real record: the released v0.2.1 record still passes all three layers.
+"""
+import hashlib
+import importlib.util
+import os
+import pathlib
+import subprocess
+import sys
+
+import pytest
+
+sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
+import e3_fixtures as fx  # noqa: E402
+import e3_vectors_fullmatch as nv  # noqa: E402
+
+V = fx.load_verifier()
+ROOT = fx.ROOT
+MERGE_COMMIT = "d2d8ec7422490053644779d3cca9fce388bb2886"      # the verifier as it was accepted and frozen
+V021_SOURCE = "106965b8cea7cce2081beb77008753af21e68b46"
+V021_RELEASE_COMMIT = "5e8c3219b52399e3e764b0b0bb27de710f5445a4"
+V021_RECORD_SHA256 = "e5d8268aede0d24f02e60a151099a2fdc7615172ed3105b0a05b6b5d19addf63"
+REQUIRED = os.environ.get("CIC_REQUIRE_RELEASE_HISTORY") == "1"
+CONTEXT = {"$GOLDEN_COMMIT": nv.COMMIT}
+
+
+def has_commit(commit):
+    return subprocess.run(["git", "-C", str(ROOT), "cat-file", "-e", commit + "^{commit}"], capture_output=True, check=False).returncode == 0
+
+
+def result_of(verifier, vector):
+    bundle = fx.apply_ops(fx.load_golden(), vector["ops"], CONTEXT)
+    fx.reauthenticate(bundle, vector["reauth"])
+    return verifier.check_record(bundle), verifier.check_crypto(bundle)
+
+
+@pytest.mark.parametrize("vector", nv.ALL, ids=lambda x: x["id"])
+def test_the_value_is_refused_with_the_reason_of_its_form(vector):
+    record, crypto = result_of(V, vector)
+    assert record.status == "FAIL" and record.reason == vector["reason"], (record.status, record.reason, record.detail)
+    assert crypto["status"] == vector["crypto"], "re-authenticated: the refusal has to come from the contract check, not from a stale signature"
+
+
+@pytest.mark.parametrize("vector", nv.NEWLINE, ids=lambda x: x["id"])
+def test_the_verifier_of_d2d8ec7_did_not_refuse_it_for_that_reason(vector, tmp_path):
+    """The proof that the defect existed. The old verifier is read from git; where the history is required its absence is a failure, not a skip."""
+    if not has_commit(MERGE_COMMIT):
+        assert not REQUIRED, "CIC_REQUIRE_RELEASE_HISTORY=1 but %s is not in this clone (fetch-depth: 0)" % MERGE_COMMIT
+        pytest.skip("the merge commit is not in this clone")
+    source = subprocess.run(["git", "-C", str(ROOT), "show", "%s:tools/release_record_conformance.py" % MERGE_COMMIT], capture_output=True, check=True).stdout
+    old_path = tmp_path / "release_record_conformance_d2d8ec7.py"
+    old_path.write_bytes(source)
+    spec = importlib.util.spec_from_file_location("release_record_conformance_d2d8ec7", old_path)
+    old = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(old)
+    record, _crypto = result_of(old, vector)
+    assert not (record.status == "FAIL" and record.reason == vector["reason"]), "the old verifier already refused %s: there was no defect to fix" % vector["id"]
+
+
+@pytest.mark.parametrize("vector", nv.CONTROLS, ids=lambda x: x["id"])
+def test_the_controls_were_refused_before_the_fix_too(vector, tmp_path):
+    if not has_commit(MERGE_COMMIT):
+        assert not REQUIRED, "CIC_REQUIRE_RELEASE_HISTORY=1 but %s is not in this clone (fetch-depth: 0)" % MERGE_COMMIT
+        pytest.skip("the merge commit is not in this clone")
+    source = subprocess.run(["git", "-C", str(ROOT), "show", "%s:tools/release_record_conformance.py" % MERGE_COMMIT], capture_output=True, check=True).stdout
+    old_path = tmp_path / "release_record_conformance_d2d8ec7.py"
+    old_path.write_bytes(source)
+    spec = importlib.util.spec_from_file_location("release_record_conformance_d2d8ec7", old_path)
+    old = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(old)
+    record, _crypto = result_of(old, vector)
+    assert record.status == "FAIL" and record.reason == vector["reason"]
+
+
+def test_the_released_v021_record_still_passes_every_layer(tmp_path):
+    """The fix may not change what is said about a release that exists. The record is read from the release COMMIT and pinned by its SHA-256."""
+    if not (has_commit(V021_SOURCE) and has_commit(V021_RELEASE_COMMIT)):
+        assert not REQUIRED, "CIC_REQUIRE_RELEASE_HISTORY=1 but the commits of release v0.2.1 are not in this clone (fetch-depth: 0)"
+        pytest.skip("the history of release v0.2.1 is not in this clone")
+    shown = subprocess.run(["git", "-C", str(ROOT), "show", "%s:release/cic-primitives-v0.2.1.yaml" % V021_RELEASE_COMMIT], capture_output=True, check=True)
+    assert hashlib.sha256(shown.stdout).hexdigest() == V021_RECORD_SHA256, "the record of v0.2.1 is not the frozen one"
+    record_path = tmp_path / "cic-primitives-v0.2.1.yaml"
+    record_path.write_bytes(shown.stdout)
+    out = subprocess.run([sys.executable, str(ROOT / "tools" / "release_record_conformance.py"), str(record_path), "--repo", str(ROOT), "--require", "record,blobs,crypto"],
+                         capture_output=True, text=True, check=False)
+    assert out.returncode == 0, out.stdout + out.stderr
+    for layer in ("record   PASS", "blobs    PASS", "crypto   PASS"):
+        assert layer in out.stdout