From ae30dbd0e0eecfb7092c5ef257da2367d114fc62 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 13:08:28 +0000 Subject: [PATCH 1/3] repo_shape.py and bootstrap.py: `COMMIT_RE`, `SHA256_RE` and `COMMIT_40` end in `\Z`, not `$`, so a pin's `commit:` or `tree_sha256:` that ends in a line break is refused as not 40 or 64 hex by every validator and `adopt-project.py` (#1017) `$` also matches just before a trailing line feed, and every caller reads these patterns with `.match`, so a `commit:` or `tree_sha256:` read as the hex and a line feed (the double-quoted escape for a line feed, or a block scalar) passed as well formed. On main, validate-manifest.py and validate-family.py passed one (`manifest ok`, `family ok`), validate-pins.py refused one for another reason (`pin-commit-unresolvable`, `pin-digest-mismatch`), and the assembly root's bootstrap.py, which holds a copy of the commit pattern as `COMMIT_40`, printed its upstream notice over one. `\Z` matches only at the very end of the value, so each caller's `.match` refuses a trailing line feed as it already refused a carriage return and line feed, and a space. Nothing else about the patterns changes: 40 and 64 digits, either case. Rows: tests/test_pin_pattern_line_break.py (the three patterns, the holder's gate, the upstream notice, the doctor's `pin_commit`), and a row each in test_pin_validator.py, test_manifest_validator.py, test_adopt_pins.py and test_shape_doctor.py. Each fails on main and passes with the change. Closes #1017 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- scripts/repo_shape.py | 7 +- templates/assembly-root/scripts/bootstrap.py | 4 +- tests/test_adopt_pins.py | 17 +- tests/test_manifest_validator.py | 23 ++ tests/test_pin_pattern_line_break.py | 266 +++++++++++++++++++ tests/test_pin_validator.py | 39 +++ tests/test_shape_doctor.py | 6 + 7 files changed, 356 insertions(+), 6 deletions(-) create mode 100644 tests/test_pin_pattern_line_break.py diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 49eb0412..00759f1e 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -65,8 +65,11 @@ #: Exactly 40 hex, case-insensitive, normalised to lowercase before comparison. #: 40 means 40 — an abbreviated oid, a branch name or a tag cannot pass, which #: is `neutral-product-pin`'s "a tag can be moved" written as a regex. -COMMIT_RE = re.compile(r"^[0-9a-fA-F]{40}$") -SHA256_RE = re.compile(r"^[0-9a-fA-F]{64}$") +#: BOTH END IN `\Z`, NEVER `$` (#1017): `$` also matches just before a trailing +#: line feed, so a `commit:` or `tree_sha256:` read as `"\n"` (a +#: double-quoted escape, or a block scalar) passed `.match` as well formed. +COMMIT_RE = re.compile(r"^[0-9a-fA-F]{40}\Z") +SHA256_RE = re.compile(r"^[0-9a-fA-F]{64}\Z") #: A project id is the lowercase machine name; the GitHub topic is derived. PROJECT_ID_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index 6309c115..99c8bf06 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -160,7 +160,9 @@ #: a dependency again, which is the whole thing the copies exist to avoid. SHAPE_UPSTREAM_ENV = "SHAPE_UPSTREAM_PATH" SHAPE_UPSTREAM_CACHE = ".shape-upstream-tip" -COMMIT_40 = re.compile(r"^[0-9a-fA-F]{40}$") +#: `\Z`, NEVER `$` (#1017): `$` also matches before a trailing line feed, as it +#: did in `repo_shape.COMMIT_RE`, which this is a copy of. +COMMIT_40 = re.compile(r"^[0-9a-fA-F]{40}\Z") #: Where a remote-tracking ref lives. Only an upstream there holds a branch's #: commits on the remote's behalf; an upstream of `.` is another local branch. REMOTE_TRACKING = "refs/remotes/" diff --git a/tests/test_adopt_pins.py b/tests/test_adopt_pins.py index 86c12462..d6824ce2 100644 --- a/tests/test_adopt_pins.py +++ b/tests/test_adopt_pins.py @@ -1246,6 +1246,17 @@ def malformed(glass) -> dict: digest=digest[:12]), f"{entry} `tree_sha256: '{digest[:12]}'`, which is " "not 64 hex"), + # `$` matched just before a trailing line feed (#1017), so the + # double-quoted `\n` after the hex, which the plan's reader reads + # as a line feed, was a well formed commit and a well formed digest. + "commit-line-break": ("pins:\n" + pin_entry(commit=commit + "\\n", + digest=digest), + f"{entry} `commit: '{commit}\\n'`, which is " + "not 40 hex"), + "digest-line-break": ("pins:\n" + pin_entry(commit=commit, + digest=digest + "\\n"), + f"{entry} `tree_sha256: '{digest}\\n'`, which " + "is not 64 hex"), "another-product": ("pins:\n" + pin_entry( repository="opensoft/openPane", commit=commit, digest=digest), @@ -1354,9 +1365,9 @@ def cases(glass): "old-plan-forged")] CASE_IDS += [("plan-pin-malformed", name) for name in ( "not-a-list", "a-number", "short-commit", "short-digest", - "another-product", "no-owner", "no-source", "source-line-break", - "source-bare-path", "source-no-space", "source-by-hand", - "source-clone-nul", "a-number-commit")] + "commit-line-break", "digest-line-break", "another-product", "no-owner", + "no-source", "source-line-break", "source-bare-path", "source-no-space", + "source-by-hand", "source-clone-nul", "a-number-commit")] CASE_IDS += [("plan-pin-collides", name) for name in ("case-pair", "shape", "variable-pair")] diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index effabe6d..7d9114a6 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -38,6 +38,29 @@ def test_a_scaffolded_manifest_is_valid(project): assert "manifest ok" in result.stdout +def shape_of(project) -> dict: + return parse_yaml((project / "project.yaml").read_text())["shape"] + + +def test_a_shape_commit_that_ends_in_a_line_break_is_a_finding(project): + """`$` matched just before a trailing line feed (#1017), so a + `shape.commit` read as the 40 hex and a line feed (the double-quoted + `\\n`, or a block scalar) was `manifest ok`.""" + commit = shape_of(project)["commit"] + edit(project, f'commit: "{commit}"', f'commit: "{commit}\\n"') + result = validate(project) + assert result.returncode == 1, result.stderr + assert "manifest-shape-commit" in result.stderr + + +def test_a_shape_digest_that_ends_in_a_line_break_is_a_finding(project): + digest = shape_of(project)["digests"]["tree_sha256"] + edit(project, f'tree_sha256: "{digest}"', f'tree_sha256: "{digest}\\n"') + result = validate(project) + assert result.returncode == 1, result.stderr + assert "manifest-shape-digest" in result.stderr + + def test_wrong_kind_is_a_finding(project): edit(project, "kind: project-manifest", "kind: project-register") result = validate(project) diff --git a/tests/test_pin_pattern_line_break.py b/tests/test_pin_pattern_line_break.py new file mode 100644 index 00000000..a9ac3a4b --- /dev/null +++ b/tests/test_pin_pattern_line_break.py @@ -0,0 +1,266 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A pin's `commit:` or `tree_sha256:` that ends in a line break is not well +formed (#1017). + +THE DEFECT, measured on main 2a119f28 (Python 3.12.3). `repo_shape.COMMIT_RE` +and `SHA256_RE` were `^[0-9a-fA-F]{40}$` and `^[0-9a-fA-F]{64}$`, and every +caller reads them with `.match`. In Python `$` also matches just before a +trailing line feed, so: + +| pattern | value | `.match` on main | +|-------------|------------------------|------------------| +| `COMMIT_RE` | 40 hex | a match | +| `COMMIT_RE` | 40 hex and `"\\n"` | a match | +| `COMMIT_RE` | 40 hex and `"\\r\\n"` | None | +| `COMMIT_RE` | 40 hex and `" "` | None | +| `SHA256_RE` | 64 hex | a match | +| `SHA256_RE` | 64 hex and `"\\n"` | a match | +| `SHA256_RE` | 64 hex and `"\\r\\n"` | None | +| `SHA256_RE` | 64 hex and `" "` | None | + +A line feed at the end of a value reaches them through YAML: the reader takes +the double-quoted escape `"\\n"` and a block scalar (`commit: |`) as the +hex and a line feed. Through that, on main: + +- `validate-manifest.py` passed a `shape.commit` and a + `shape.digests.tree_sha256` that ended in one (`manifest ok`, exit 0), and + `validate-family.py` the same two in `family.yaml`; +- `validate-pins.py` took a leg pin's `commit:` for 40 hex and refused it as + `pin-commit-unresolvable`, naming the commit as one the leg's object store + lacks, and a digest as `pin-digest-mismatch`, a recomputation that differs; +- `adopt-project.py check` read a plan's `commit:` as well formed. + +THE RULE NOW. Both patterns end in `\\Z`, which matches at the very end of the +value and nowhere else, so each caller's `.match` refuses a value that ends in +a line break, as it refuses one that ends in `"\\r\\n"` or `" "`. The assembly +root's `bootstrap.py` holds a COPY of `COMMIT_RE`, `COMMIT_40`, which ends the +same way. Everything else about the two patterns is unchanged: 40 means 40, a +digit more or fewer is refused, and the case of the hex does not matter. + +The caller rows are beside the files they exercise: `test_pin_validator.py` +(`validate-pins.py`), `test_manifest_validator.py` (`validate-manifest.py`), +`test_adopt_pins.py` (`adopt-project.py check`) and `test_shape_doctor.py` +(`update-shape.py`'s `read_pin`, through the doctor). The rows here are the +patterns, their copy, the holder's gate and the two readers that take a pin's +`commit:` without a gate of their own in front of them. + +No network and no GitHub: the family is the offline one of `test_family.py`. +""" + +from __future__ import annotations + +import functools +import re +import sys + +import pytest + +from conftest import REPO, load_without_bytecode +from test_family import family, validate # noqa: F401 +from test_family_add_refusals import holder # noqa: F401 + +sys.path.insert(0, str(REPO / "scripts")) +import repo_shape # noqa: E402 + +BOOTSTRAP = REPO / "templates" / "assembly-root" / "scripts" / "bootstrap.py" +DOCTOR = REPO / "shape-doctor.py" + +#: Sixteen digits in each case, so a value cut from it holds both. +HEX = "0123456789abcdefABCDEF" +COMMIT = (HEX * 2)[:40] +DIGEST = (HEX * 3)[:64] + + +@functools.cache +def bootstrap_copy(): + """The assembly root's `bootstrap.py`, loaded from the template.""" + return load_without_bytecode(BOOTSTRAP, "bootstrap_line_break") + + +@functools.cache +def doctor_module(): + return load_without_bytecode(DOCTOR, "shape_doctor_line_break") + + +#: `(the pattern, how many digits it takes)`: the two of `repo_shape`, and +#: the copy of the first that `bootstrap.py` holds. +PATTERNS = [ + pytest.param(lambda: repo_shape.COMMIT_RE, 40, id="COMMIT_RE"), + pytest.param(lambda: repo_shape.SHA256_RE, 64, id="SHA256_RE"), + pytest.param(lambda: bootstrap_copy().COMMIT_40, 40, + id="bootstrap-COMMIT_40"), +] + +#: What may not follow the digits: a line feed (the one `$` took), a +#: carriage return and line feed, a space, a second line feed, a lone +#: carriage return and a tab. +AFTER = [ + pytest.param("\n", id="line-feed"), + pytest.param("\r\n", id="crlf"), + pytest.param(" ", id="space"), + pytest.param("\n\n", id="two-line-feeds"), + pytest.param("\r", id="carriage-return"), + pytest.param("\t", id="tab"), +] + + +def digits_of(taken: int) -> str: + return COMMIT if taken == 40 else DIGEST + + +@pytest.mark.parametrize("pattern, taken", PATTERNS) +@pytest.mark.parametrize("form", [str.lower, str.upper, str], + ids=["lower", "upper", "mixed"]) +def test_a_pattern_takes_its_digits_in_any_case(pattern, taken, form): + """What it took on main, and takes now.""" + assert pattern().match(form(digits_of(taken))) is not None + + +@pytest.mark.parametrize("pattern, taken", PATTERNS) +def test_a_pattern_takes_exactly_its_digits(pattern, taken): + """40 means 40, and 64 means 64: a digit fewer or more is refused.""" + digits = digits_of(taken) + assert pattern().match(digits[:-1]) is None + assert pattern().match(digits + "0") is None + + +@pytest.mark.parametrize("pattern, taken", PATTERNS) +@pytest.mark.parametrize("after", AFTER) +def test_a_pattern_refuses_a_value_that_ends_in_a_line_break_or_a_space( + pattern, taken, after): + """THE ISSUE'S ROWS: `"\\n"`, `"\\r\\n"` and `" "` after the digits. Only + the first was taken on main, by `$`.""" + assert pattern().match(digits_of(taken) + after) is None + + +@pytest.mark.parametrize("pattern, taken", PATTERNS) +def test_a_pattern_refuses_a_line_break_before_the_digits(pattern, taken): + """The start of the value is `^`'s, which is not multiline.""" + assert pattern().match("\n" + digits_of(taken)) is None + + +# --- the holder's gate ------------------------------------------------------- + + +def with_a_line_feed(text: str, key: str, value: str) -> str: + """`text` with the first `key: ""` read as `` and a line + feed: the double-quoted `\\n` escape, which a file spells with a + backslash and an `n`.""" + old = f'{key}: "{value}"' + assert old in text, f"fixture drift: {old!r} not in the file" + return text.replace(old, f'{key}: "{value}\\n"', 1) + + +def holder_text(root) -> str: + return (root / "family.yaml").read_text(encoding="utf-8") + + +def member_pin(root) -> tuple[str, str]: + """`(commit, tree_sha256)` of the first member's pin in `family.yaml`, + which the `pin:` block of its row holds.""" + text = holder_text(root) + commit = re.search(r'^ commit: "([0-9a-f]{40})"', text, re.M) + digest = re.search(r'^ tree_sha256: "([0-9a-f]{64})"', text, re.M) + assert commit and digest, text + return commit.group(1), digest.group(1) + + +def holder_shape(root) -> tuple[str, str]: + """`(commit, tree_sha256)` of the holder's own `shape:` block.""" + text = holder_text(root) + commit = re.search(r'^ commit: "([0-9a-f]{40})"', text, re.M) + digest = re.search(r'^ tree_sha256: "([0-9a-f]{64})"', text, re.M) + assert commit and digest, text + return commit.group(1), digest.group(1) + + +def test_the_holders_gate_passes_the_family_as_scaffolded(holder): + """The baseline of the rows below: no finding, exit 0.""" + gate = validate(holder) + assert gate.returncode == 0, gate.stderr + gate.stdout + + +def test_a_member_pin_commit_that_ends_in_a_line_break_is_refused(holder): + """`member-pin-commit`, the refusal a pin of 12 digits gets; a commit + that `$` let through was named as one the member's object store lacks.""" + commit, _ = member_pin(holder) + (holder / "family.yaml").write_bytes(with_a_line_feed( + holder_text(holder), " commit", commit).encode("utf-8")) + gate = validate(holder) + assert gate.returncode == 2, gate.stderr + gate.stdout + assert gate.stderr.startswith("REFUSED member-pin-commit: "), gate.stderr + + +def test_a_member_pin_digest_that_ends_in_a_line_break_is_malformed(holder): + _, digest = member_pin(holder) + (holder / "family.yaml").write_bytes(with_a_line_feed( + holder_text(holder), " tree_sha256", digest).encode("utf-8")) + gate = validate(holder) + assert gate.returncode == 1, gate.stderr + gate.stdout + assert "FINDING member-digest-malformed: " in gate.stderr, gate.stderr + + +def test_a_family_shape_commit_that_ends_in_a_line_break_is_a_finding(holder): + commit, _ = holder_shape(holder) + (holder / "family.yaml").write_bytes(with_a_line_feed( + holder_text(holder), " commit", commit).encode("utf-8")) + gate = validate(holder) + assert gate.returncode == 1, gate.stderr + gate.stdout + assert "FINDING family-shape-commit: " in gate.stderr, gate.stderr + + +def test_a_family_shape_digest_that_ends_in_a_line_break_is_a_finding(holder): + _, digest = holder_shape(holder) + (holder / "family.yaml").write_bytes(with_a_line_feed( + holder_text(holder), " tree_sha256", digest).encode("utf-8")) + gate = validate(holder) + assert gate.returncode == 1, gate.stderr + gate.stdout + assert "FINDING family-shape-digest: " in gate.stderr, gate.stderr + + +# --- the two readers with no gate in front of them ---------------------------- + +#: A commit the cache can name that the pin below is not at. +TIP = "b" * 40 +#: The first words of the notice's one line, over `TIP`. +NOTICE = f" shape: the upstream is at {TIP[:12]} and this project is pinned at " + + +def a_root(base, pin: bytes): + """A root holding only `contracts/shape-pin.yaml` and a cached tip.""" + (base / "contracts").mkdir(parents=True) + (base / "contracts" / "shape-pin.yaml").write_bytes(pin) + (base / ".shape-upstream-tip").write_bytes(f"{TIP}\n".encode()) + return base + + +def test_the_upstream_notice_names_a_pin_that_is_40_hex(tmp_path, capsys): + """The baseline of the row below: the notice is not silent for a reason + of its own.""" + bootstrap_copy().shape_upstream_notice( + a_root(tmp_path, f'commit: "{COMMIT.lower()}"\n'.encode())) + assert capsys.readouterr().out.startswith(NOTICE) + + +def test_the_upstream_notice_is_silent_over_a_pin_that_ends_in_a_line_break( + tmp_path, capsys): + """An absent answer is no finding: a pin that is not a commit leaves the + notice silent, and `COMMIT_40` took one that ends in a line feed for one.""" + bootstrap_copy().shape_upstream_notice( + a_root(tmp_path, f'commit: "{COMMIT.lower()}\\n"\n'.encode())) + assert capsys.readouterr().out == "" + + +@pytest.mark.parametrize("value, said", [ + pytest.param(COMMIT.lower(), COMMIT.lower(), id="40-hex"), + pytest.param(COMMIT.lower() + "\\n", None, id="40-hex-and-a-line-feed"), +]) +def test_the_doctor_reads_a_pins_commit_as_the_validators_do(tmp_path, value, + said): + """`shape-doctor.py`'s `pin_commit` is the commit a leg's pin names, or + None for one the validators refuse.""" + (tmp_path / "contracts").mkdir() + (tmp_path / "contracts" / "spec-pin.yaml").write_bytes( + f'commit: "{value}"\n'.encode()) + assert doctor_module().pin_commit( + tmp_path, "contracts/spec-pin.yaml") == said diff --git a/tests/test_pin_validator.py b/tests/test_pin_validator.py index 1fad48fa..81bd1ae6 100644 --- a/tests/test_pin_validator.py +++ b/tests/test_pin_validator.py @@ -123,6 +123,19 @@ def test_a_commit_that_is_not_40_hex_refuses(project): assert "pin-tag-only" in result.stderr +def test_a_commit_that_ends_in_a_line_break_refuses(project): + """`$` matched just before a trailing line feed (#1017), so a `commit:` + read as the 40 hex and a line feed (the double-quoted `\\n`, or a block + scalar) passed as 40 hex, and the refusal named the commit as one the + leg's object store lacks (`pin-commit-unresolvable`).""" + commit = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + edit_pin(project, "spec-pin.yaml", f'commit: "{commit}"', + f'commit: "{commit}\\n"') + result = validate(project) + assert result.returncode == 2 # not 40 hex: a refusal, not a finding + assert "REFUSED pin-tag-only: " in result.stderr + + # --- drift case 2: the digest no longer describes the pinned bytes ---------- def test_a_digest_that_does_not_recompute_is_a_finding(project): @@ -134,6 +147,19 @@ def test_a_digest_that_does_not_recompute_is_a_finding(project): assert "pin-digest-mismatch" in result.stderr +def test_a_digest_that_ends_in_a_line_break_is_malformed(project): + """The same `$` (#1017): a `tree_sha256:` of the 64 hex and a line feed + was a digest to recompute and compare, `pin-digest-mismatch`, where it is + not 64 hex.""" + pin = (project / "contracts" / "spec-pin.yaml").read_text() + recorded = pin.split("tree_sha256: \"")[1].split("\"")[0] + edit_pin(project, "spec-pin.yaml", f'tree_sha256: "{recorded}"', + f'tree_sha256: "{recorded}\\n"') + result = validate(project) + assert result.returncode == 1 + assert "pin-digest-malformed" in result.stderr + + def test_an_unstated_digest_definition_is_a_finding(project): edit_pin(project, "spec-pin.yaml", "digest_definition: sorted-ls-tree-r-v1", "digest_definition: whatever-git-does") @@ -270,6 +296,19 @@ def test_a_missing_shape_pin_refuses(project): assert "shape-pin-missing" in result.stderr +def test_a_shape_pin_commit_that_ends_in_a_line_break_is_a_finding(project): + """The same `$` (#1017), in the pin of the shape itself: its `commit:` + was 40 hex, and the only finding was that `project.yaml` names another + string (`shape-pin-manifest-disagree`).""" + pin = (project / "contracts" / "shape-pin.yaml").read_text() + commit = pin.split("commit: \"")[1].split("\"")[0] + edit_pin(project, "shape-pin.yaml", f'commit: "{commit}"', + f'commit: "{commit}\\n"') + result = validate(project) + assert result.returncode == 1 + assert "FINDING shape-pin-commit: " in result.stderr + + def test_a_pin_path_that_disagrees_with_the_manifest_is_a_finding(project): edit_pin(project, "spec-pin.yaml", 'submodule_path: "spec"', 'submodule_path: "specification"') diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index b520d39b..26d962fb 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -5260,6 +5260,12 @@ def pin_with(old: str, new: str): id="a-list-not-a-mapping"), pytest.param(pin_with(r'^commit: .*$', 'commit: "zzzz"'), "update-pin-commit", id="a-commit-that-is-not-40-hex"), + # `$` matched just before a trailing line feed (#1017): the double-quoted + # `\n` after the hex is a commit that is not 40 hex. In the replacement + # of `re.sub`, `\\` is the one backslash the file spells it with. + pytest.param(pin_with(r'^commit: .*$', + 'commit: "' + "1" * 40 + r'\\n"'), + "update-pin-commit", id="a-commit-ending-in-a-line-break"), pytest.param(other_digest_definition, "update-pin-digest-definition", id="another-digest-definition"), pytest.param(pin_with(r'(?s)^files:.*\Z', 'files: []\n'), From 03b2b97b9d3cccc42b4703cb2f7c978693488266 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 16:34:14 +0000 Subject: [PATCH 2/3] tests/test_pin_pattern_line_break.py: a line feed ending a neutral pin's commit or digest, or a record's `source_commit:`, has a row, so `.rstrip` before the match in validate-pins.py or validate-manifest.py fails; each read asserts one value (#1017) Fix round 1 for #1126, from its independent review. `member_pin` and `holder_shape` ended in `assert commit and digest, text` (SonarCloud S9073): when one of the two reads found nothing, the message could not say which. Each value now has its own assert and a message that names it. Two callers of the patterns had no row of their own, so a `.rstrip("\n")` before the match survived every row: the neutral product's pin in validate-pins.py (`commit:` is refused as `neutral-pin-tag-only`, `tree_sha256:` is `neutral-pin-digest-malformed`) and the `source_commit:` of a `submodules:` or `answered_paths:` row in validate-manifest.py (`manifest-submodule-commit`, `manifest-answered-commit`). On main the first was the product's checkout refusing the name (`neutral-pin-source-unreadable`), the second a digest recompute (`neutral-pin-digest-mismatch`), the third `manifest ok`. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pin_pattern_line_break.py | 67 ++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/tests/test_pin_pattern_line_break.py b/tests/test_pin_pattern_line_break.py index a9ac3a4b..c1c6e027 100644 --- a/tests/test_pin_pattern_line_break.py +++ b/tests/test_pin_pattern_line_break.py @@ -41,8 +41,10 @@ (`validate-pins.py`), `test_manifest_validator.py` (`validate-manifest.py`), `test_adopt_pins.py` (`adopt-project.py check`) and `test_shape_doctor.py` (`update-shape.py`'s `read_pin`, through the doctor). The rows here are the -patterns, their copy, the holder's gate and the two readers that take a pin's -`commit:` without a gate of their own in front of them. +patterns, their copy, the holder's gate, the two callers that had no row +beside them (the neutral product's pin in `validate-pins.py` and a record +row's `source_commit:` in `validate-manifest.py`) and the two readers that +take a pin's `commit:` without a gate of their own in front of them. No network and no GitHub: the family is the offline one of `test_family.py`. """ @@ -58,6 +60,11 @@ from conftest import REPO, load_without_bytecode from test_family import family, validate # noqa: F401 from test_family_add_refusals import holder # noqa: F401 +from test_manifest_validator import ( # noqa: F401 + ANSWER_RECORD, SUBMODULE_RECORD, assert_refused) +from test_neutral_pin_validator import ( + edit_pin as edit_neutral_pin, scaffold_with_pin, + validate as validate_pins) sys.path.insert(0, str(REPO / "scripts")) import repo_shape # noqa: E402 @@ -161,7 +168,8 @@ def member_pin(root) -> tuple[str, str]: text = holder_text(root) commit = re.search(r'^ commit: "([0-9a-f]{40})"', text, re.M) digest = re.search(r'^ tree_sha256: "([0-9a-f]{64})"', text, re.M) - assert commit and digest, text + assert commit, f"fixture drift: no member pin commit in {text}" + assert digest, f"fixture drift: no member pin tree_sha256 in {text}" return commit.group(1), digest.group(1) @@ -170,7 +178,8 @@ def holder_shape(root) -> tuple[str, str]: text = holder_text(root) commit = re.search(r'^ commit: "([0-9a-f]{40})"', text, re.M) digest = re.search(r'^ tree_sha256: "([0-9a-f]{64})"', text, re.M) - assert commit and digest, text + assert commit, f"fixture drift: no shape commit in {text}" + assert digest, f"fixture drift: no shape tree_sha256 in {text}" return commit.group(1), digest.group(1) @@ -218,6 +227,56 @@ def test_a_family_shape_digest_that_ends_in_a_line_break_is_a_finding(holder): assert "FINDING family-shape-digest: " in gate.stderr, gate.stderr +# --- the two callers with no row beside them -------------------------------- +# +# Each reads a value with `COMMIT_RE` or `SHA256_RE` and no other gate, so +# nothing but the pattern refuses a line feed there: a `.rstrip("\n")` before +# the match would leave every row above green. + + +def test_a_neutral_pin_commit_that_ends_in_a_line_break_is_refused(tmp_path): + """`validate-pins.py` reads the neutral product's pin with `COMMIT_RE` + and no other gate. On main a commit of 40 hex and a line feed passed + it, and the finding was the product's checkout refusing the name + (`neutral-pin-source-unreadable`), not that it is not 40 hex.""" + root, product, commit = scaffold_with_pin(tmp_path) + edit_neutral_pin(root, f'commit: "{commit}"', f'commit: "{commit}\\n"') + gate = validate_pins(root, "--pin-source", str(product)) + assert gate.returncode == 2, gate.stderr + gate.stdout + assert "REFUSED neutral-pin-tag-only: " in gate.stderr, gate.stderr + + +def test_a_neutral_pin_digest_that_ends_in_a_line_break_is_malformed(tmp_path): + """On main the digest passed `SHA256_RE` and was recomputed and compared + (`neutral-pin-digest-mismatch`); it is not 64 hex.""" + root, product, _ = scaffold_with_pin(tmp_path) + pin = (root / "contracts" / "openglass-pin.yaml").read_text() + recorded = pin.split('tree_sha256: "')[1].split('"')[0] + edit_neutral_pin(root, f'tree_sha256: "{recorded}"', + f'tree_sha256: "{recorded}\\n"') + gate = validate_pins(root, "--pin-source", str(product)) + assert gate.returncode == 1, gate.stderr + gate.stdout + assert "FINDING neutral-pin-digest-malformed: " in gate.stderr, gate.stderr + + +@pytest.mark.parametrize("record, code", [ + pytest.param(SUBMODULE_RECORD, "manifest-submodule-commit", + id="submodules"), + pytest.param(ANSWER_RECORD, "manifest-answered-commit", + id="answered-paths"), +]) +def test_a_recorded_source_commit_that_ends_in_a_line_break_is_a_finding( + project, record, code): + """`validate-manifest.py` reads a record row's `source_commit:` with + `COMMIT_RE`, and the two records share the check. On main a commit of 40 + hex and a line feed was `manifest ok`.""" + found = re.search(r'source_commit: "([0-9a-f]{40})"', record) + assert found, "fixture drift: the record names no source_commit" + commit = found.group(1) + assert_refused(project, record, f'source_commit: "{commit}"', + f'source_commit: "{commit}\\n"', code) + + # --- the two readers with no gate in front of them ---------------------------- #: A commit the cache can name that the pin below is not at. From 4c1f80cf4db6215d8861bd43cd57f72df957d82a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 13:20:43 +0000 Subject: [PATCH 3/3] tests/test_pin_pattern_line_break.py: the module docstring says `validate-family.py` passed only the digest of `family.yaml`'s `shape:` on main, and gave `shape-pin-manifest-disagree` over the commit, not that it passed both (#1017) NIT 1 of rc1126's re-check of #1126 (issuecomment-6340973564). The docstring said validate-family.py passed `shape.commit` and `shape.digests.tree_sha256` in family.yaml on main. Only the digest passed (`family ok`, exit 0). The commit passed `COMMIT_RE` too, so `family-shape-commit` was not raised, but `contracts/shape-pin.yaml` holds the commit without the line feed, and the gate gave `shape-pin-manifest-disagree` with the same hex printed on both sides. The clause now says that. A docstring only: no code and no row changes. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_pin_pattern_line_break.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_pin_pattern_line_break.py b/tests/test_pin_pattern_line_break.py index c1c6e027..7ceacae6 100644 --- a/tests/test_pin_pattern_line_break.py +++ b/tests/test_pin_pattern_line_break.py @@ -23,8 +23,12 @@ hex and a line feed. Through that, on main: - `validate-manifest.py` passed a `shape.commit` and a - `shape.digests.tree_sha256` that ended in one (`manifest ok`, exit 0), and - `validate-family.py` the same two in `family.yaml`; + `shape.digests.tree_sha256` that ended in one (`manifest ok`, exit 0); +- `validate-family.py` passed the digest in `family.yaml` (`family ok`, exit + 0) and took the commit for 40 hex too, so no `family-shape-commit` was + raised, but it gave `shape-pin-manifest-disagree` over it, since + `contracts/shape-pin.yaml` holds the commit without the line feed (the same + hex is printed on both sides); - `validate-pins.py` took a leg pin's `commit:` for 40 hex and refused it as `pin-commit-unresolvable`, naming the commit as one the leg's object store lacks, and a digest as `pin-digest-mismatch`, a recomputation that differs;