From 620741235daccf617411d4a0ef3bd9c73db46293 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 22:20:25 +0000 Subject: [PATCH 1/2] bootstrap.py and repo_shape.py: a `.shape-upstream-tip` or a YAML file a Windows editor saved with a byte-order mark reads as the text it holds, so the upstream notice names the tip, a marked `project.yaml` is no longer `schema_version is None`, and every reader of a marked pin, the doctor's included, reads it alike (#855) Issue #855 has two lines, filed by lane 5 from rv815's NIT 8 on #815. Both are measured on main 9f235da2 by loading the shipped scripts and calling them, and by running them in a scaffold. Line 1. `shape_upstream_notice` read `.shape-upstream-tip` with `read_text(encoding="utf-8").strip()`. `strip()` keeps U+FEFF, so a cache holding EF BB BF and a commit, with LF or CRLF, did not match `COMMIT_40` and the notice said nothing. The cache is read as `utf-8-sig` now, and each of the three says `shape: the upstream is at bbbbbbbbbbbb and this project is pinned at aaaaaaaaaaaa.` Line 2. `load_yaml` read the mark as the first character of the first key (U+FEFF and `commit`), and refused a file whose first line is a comment: `line 1: expected `key: value`, got '\ufeff# a comment'`. In a scaffold a marked `project.yaml` was `FINDING manifest-schema-version: schema_version is None, expected 1`, `make bootstrap` ended `bootstrap FAILED: manifest (exit 1)`, the doctor said `INVALID (manifest)`, and `update-shape.py apply` re-pinned and rolled back (`REFUSED update-validators-red`). `parse_yaml` takes off every byte-order mark the text starts with now, as `shape_advisory.py` already did, and `load_yaml` still decodes as `utf-8`, so its not-UTF-8 refusal still counts the byte from the file's first byte. The same scaffold is `manifest ok`, `pins ok`, `bootstrap ok` with the notice, `COMPLIANT`, and `apply` re-pins and keeps the mark in both files. WHY THE PARSER, NOT `load_yaml` (the coordinator's ruling, as first given, was `utf-8-sig` at both reads, on the condition that every reader agrees). Five readers hand `parse_yaml` a pin's text without `load_yaml`: the doctor's `pin_at`, `update-shape.py`'s `_committed_pin` and `_accepted_in_the_pin`, `adopt-project.py`'s `_pinned_shape_paths`, and `shape_materialize.pin_values_in`. With the mark taken off in `load_yaml` alone, a committed pin a person saved with the mark above a comment line, re-pinned by `apply` and read by every validator, was one `pin_at` read as broken; with the working pin then broken, the doctor's NEXT was `git checkout '~1' -- contracts/shape-pin.yaml`: the pin from before the hand-save, naming the commit `apply` had moved it off. Taken off in the parser, the NEXT is `checkout HEAD -- contracts/shape-pin.yaml`. tests/test_bom_reads.py: both lines in each shape the issue gives, with no mark, one and two; the four readers above against `load_yaml`; the advisory against `load_yaml`; a marked scaffold bootstrapping ok; the not-UTF-8 byte counted from the file's start; and the doctor's NEXT after a re-pin of a hand-saved pin. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/repo_shape.py | 29 ++- templates/assembly-root/scripts/bootstrap.py | 5 +- tests/test_bom_reads.py | 233 +++++++++++++++++++ 3 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 tests/test_bom_reads.py diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index cba29b7f..842c9420 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -1036,9 +1036,34 @@ def _block_value(text: str, line: int) -> Any: raise +#: U+FEFF, the BYTE-ORDER MARK, which a Windows editor may start a file a +#: person saved with (#855): `parse_yaml` takes it off. +BYTE_ORDER_MARK = "\ufeff" + + def parse_yaml(text: str) -> Any: - """Parse the supported YAML subset. Raises `YamlError` on anything else.""" - recs = _tokenise(text) + """Parse the supported YAML subset. Raises `YamlError` on anything else. + + A BYTE-ORDER MARK AT THE START IS NO PART OF THE TEXT (#855), and every + one there is taken off. YAML 1.2.2 allows a mark at the start of a + document and none inside one (5.2; 9.1.1's `l-document-prefix` is a + mark, then comment lines). Kept, it was the first character of the + first key, so a `project.yaml` that says `schema_version: 1` was + `schema_version is None` to `validate-manifest.py`, and a file whose + first line is a comment was refused at line 1. + + HERE, NOT IN `load_yaml`'S DECODE, because one file read two ways is + worse than either answer. The readers of a pin at a commit + (`shape-doctor.py`'s `pin_at`, `update-shape.py`'s `_committed_pin`, + `adopt-project.py`'s `_pinned_shape_paths`) and `pin_values_in` hand + this the text of no file `load_yaml` read: with the mark taken off + there alone, `pin_at` read a pin every validator reads as broken, and + the doctor's NEXT put back the pin from before it. Every mark goes, as + `shape_advisory.py` takes them off, so the two agree. `load_yaml` still + decodes as `utf-8`, so the byte its refusal names is counted from the + file's first byte, the mark's three included. The tools write no mark. + """ + recs = _tokenise(text.lstrip(BYTE_ORDER_MARK)) if not recs: return None value, end = _parse_nodes(recs, 0, recs[0].indent) diff --git a/templates/assembly-root/scripts/bootstrap.py b/templates/assembly-root/scripts/bootstrap.py index f92f8047..bffddde6 100755 --- a/templates/assembly-root/scripts/bootstrap.py +++ b/templates/assembly-root/scripts/bootstrap.py @@ -882,7 +882,10 @@ def shape_upstream_notice(root: Path) -> None: if tip is None: cache = root / SHAPE_UPSTREAM_CACHE if cache.is_file(): - tip = cache.read_text(encoding="utf-8").strip() + # `utf-8-sig` takes off the byte-order mark a Windows editor may + # start the file with (#855). `strip()` keeps U+FEFF, so a marked + # tip read as no commit and the notice said nothing. + tip = cache.read_text(encoding="utf-8-sig").strip() if not tip or not COMMIT_40.match(tip): return if tip.lower() == pinned: diff --git a/tests/test_bom_reads.py b/tests/test_bom_reads.py new file mode 100644 index 00000000..dfed4820 --- /dev/null +++ b/tests/test_bom_reads.py @@ -0,0 +1,233 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A file a Windows editor saved with a byte-order mark is read as the text +it holds, by every reader of it (#855). + +THE DEFECT, measured on main 9f235da2 by loading the shipped scripts and +calling them, and by running them in a scaffold: + +- `.shape-upstream-tip` holding the bytes EF BB BF and a commit, with LF or + CRLF, made the assembly root's upstream notice say nothing: + `read_text(encoding="utf-8").strip()` kept U+FEFF, which `COMMIT_40` + does not match. +- `repo_shape.load_yaml` read the mark as the first character of the first + key (`commit` was U+FEFF and `commit`), and refused a file whose first + line is a comment, at line 1. So a marked `project.yaml` was `FINDING + manifest-schema-version: schema_version is None, expected 1`, `make + bootstrap` ended `bootstrap FAILED: manifest (exit 1)`, and a marked + `contracts/shape-pin.yaml` left the notice silent. + +THE RULE NOW. The cache is read as `utf-8-sig`. `parse_yaml` takes off +every byte-order mark the text starts with, so `load_yaml` and each reader +that hands the parser a pin's text without reading a file (the doctor's +`pin_at`, `update-shape.py`'s `_committed_pin`, `adopt-project.py`'s +`_pinned_shape_paths`, `pin_values_in`) read the same mapping, as +`shape_advisory.py` already did. A fix in `load_yaml` alone left `pin_at` +reading a committed pin every validator reads as broken, and the doctor's +NEXT put back the pin from before it; the regression row below is that run. + +Every mark is written by this file as bytes (`codecs.BOM_UTF8`), never +typed. +""" + +from __future__ import annotations + +import codecs +from pathlib import Path + +import pytest + +from conftest import REPO, git, load_without_bytecode, run_script +from test_doctor_pin_restore_merges import apply_on, moved_on +from test_shape_doctor import ( # noqa: F401 + SHAPE_PIN, SPEC_PIN, UPSTREAM_TARGET, commit_in, doctor, project, rows_of, + scaffolded_here, standard, +) + +import shape_advisory # noqa: E402 +import shape_materialize # noqa: E402 +from repo_shape import Refusal, load_yaml # noqa: E402 + +MARK = codecs.BOM_UTF8 +#: The line a person may put above the file's first key before saving it. +COMMENT = b"# saved by hand\n" +PINNED = "a" * 40 +TIP = "b" * 40 +#: The first words of the notice's one line, naming the cache's commit. +NOTICE = (f" shape: the upstream is at {TIP[:12]} and this project is " + f"pinned at {PINNED[:12]}. ") +BOOTSTRAP = REPO / "templates" / "assembly-root" / "scripts" / "bootstrap.py" +#: The issue's two shapes of a hand-saved file, each holding the same keys. +SHAPES = { + "first-key": f"commit: {PINNED}\nother: 1\n".encode(), + "comment-first": COMMENT + f"commit: {PINNED}\nother: 1\n".encode(), +} + + +@pytest.fixture(scope="module") +def notice(): + """The assembly root's `shape_upstream_notice`, loaded from the template + beside this checkout's `scripts/repo_shape.py`.""" + return load_without_bytecode(BOOTSTRAP, + "bootstrap_bom_reads").shape_upstream_notice + + +@pytest.fixture(scope="module") +def tools(): + """The three tools whose readers hand `parse_yaml` a pin's text.""" + return {name: load_without_bytecode(REPO / f"{name}.py", + f"{name.replace('-', '_')}_bom") + for name in ("update-shape", "shape-doctor", "adopt-project")} + + +def a_root(base: Path, pin: bytes, cache: bytes) -> Path: + """A root holding only `contracts/shape-pin.yaml` and the cache.""" + (base / "contracts").mkdir(parents=True) + (base / SHAPE_PIN).write_bytes(pin) + (base / ".shape-upstream-tip").write_bytes(cache) + return base + + +@pytest.mark.parametrize("cache", [ + pytest.param(f"{TIP}\n".encode(), id="no-mark"), + pytest.param(MARK + f"{TIP}\n".encode(), id="mark"), + pytest.param(MARK + f"{TIP}\r\n".encode(), id="mark-and-crlf"), +]) +def test_the_notice_names_the_tip_a_marked_cache_holds(notice, tmp_path, + capsys, cache): + """THE ISSUE'S FIRST LINE: each says the upstream's tip.""" + notice(a_root(tmp_path, f"commit: {PINNED}\n".encode(), cache)) + assert capsys.readouterr().out.startswith(NOTICE) + + +@pytest.mark.parametrize("marks", [0, 1, 2], ids=["no-mark", "mark", + "two-marks"]) +@pytest.mark.parametrize("shape", list(SHAPES)) +def test_load_yaml_reads_a_marked_file_as_the_keys_it_holds(tmp_path, shape, + marks): + """THE ISSUE'S SECOND LINE, its first two shapes: no first key spelled + with U+FEFF, and no refusal at line 1. Two marks read as one does, as + `shape_advisory.py` reads them.""" + path = tmp_path / "hand-saved.yaml" + path.write_bytes(MARK * marks + SHAPES[shape]) + assert sorted(load_yaml(path)) == ["commit", "other"] + + +@pytest.mark.parametrize("marks", [0, 1, 2], ids=["no-mark", "mark", + "two-marks"]) +def test_the_notice_reads_a_marked_shape_pin(notice, tmp_path, capsys, + marks): + """The second line's third shape: the pin as the notice reads it.""" + notice(a_root(tmp_path, MARK * marks + f"commit: {PINNED}\n".encode(), + f"{TIP}\n".encode())) + assert capsys.readouterr().out.startswith(NOTICE) + + +def test_a_byte_that_is_not_utf_8_is_counted_from_the_files_first_byte( + tmp_path): + """The mark's three bytes are counted: `load_yaml` decodes as `utf-8`, + and the parser, not the decode, takes the mark off. `utf-8-sig` would + count from after the mark and name byte 4 of a file whose 0xff is its + byte 7, where the remedy says to count from the file's start.""" + path = tmp_path / "hand-saved.yaml" + path.write_bytes(MARK + b"k: v\xff\n") + with pytest.raises(Refusal) as refused: + load_yaml(path) + assert refused.value.detail.endswith( + ": not UTF-8 at byte 7 (invalid start byte)"), refused.value.detail + + +@pytest.mark.parametrize("marks", [1, 2], ids=["mark", "two-marks"]) +def test_the_advisory_reads_a_marked_manifest_as_load_yaml_does(project, + marks): + """`shape_advisory.py` took every mark off before #855; the two read the + same mapping now.""" + manifest = project / "project.yaml" + manifest.write_bytes(MARK * marks + manifest.read_bytes()) + tree = shape_advisory.Tree(project, None) + assert shape_advisory._read_mapping(tree, "project.yaml") == \ + load_yaml(manifest) + + +def test_a_root_whose_hand_saved_files_carry_the_mark_bootstraps_ok(project): + """Every reader a root runs agrees: the three validators and the notice. + On main this run ended `bootstrap FAILED: manifest (exit 1)`.""" + for name in ("project.yaml", SHAPE_PIN, SPEC_PIN, + "contracts/code-pin.yaml"): + (project / name).write_bytes(MARK + (project / name).read_bytes()) + (project / ".shape-upstream-tip").write_bytes(MARK + f"{TIP}\r\n".encode()) + result = run_script(project / "scripts" / "bootstrap.py", cwd=project, + env={"SHAPE_UPSTREAM_PATH": ""}) + assert result.returncode == 0, result.stdout + result.stderr + assert f" shape: the upstream is at {TIP[:12]} " in result.stdout + assert result.stdout.endswith("\nbootstrap ok\n"), result.stdout + + +# --------------------------------------------------------------------------- +# The readers that hand `parse_yaml` a pin's text, with a committed shape pin +# saved by hand: the mark, a comment line, then the pin the scaffold wrote. +# Each reads what `load_yaml` reads of the same bytes. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def hand_saved(project) -> Path: + """`project` with that shape pin committed on `main`.""" + pin = project / SHAPE_PIN + pin.write_bytes(MARK + COMMENT + pin.read_bytes()) + git("add", "--", SHAPE_PIN, cwd=project) + commit_in(project, "the shape pin, saved by hand") + return project + + +def pinned_paths(root: Path) -> list[str]: + """The `files:` paths of `root`'s shape pin, as `load_yaml` reads it.""" + return [str(row["path"]) for row in load_yaml(root / SHAPE_PIN)["files"]] + + +def test_committed_pin_reads_the_commit_load_yaml_reads(hand_saved, tools): + committed = tools["update-shape"]._committed_pin(hand_saved, SHAPE_PIN) + assert committed == load_yaml(hand_saved / SHAPE_PIN)["commit"] + + +def test_pin_at_reads_the_rows_load_yaml_reads(hand_saved, tools): + doctor_module = tools["shape-doctor"] + rows = doctor_module.pin_at(doctor_module.Context(hand_saved), "HEAD") + assert rows == pinned_paths(hand_saved) + + +def test_pinned_shape_paths_reads_the_rows_load_yaml_reads(hand_saved, + tools): + blob = git("rev-parse", f"HEAD:{SHAPE_PIN}", cwd=hand_saved).stdout + rows = tools["adopt-project"]._pinned_shape_paths(hand_saved, + blob.strip()) + assert rows == pinned_paths(hand_saved) + + +def test_pin_values_in_reads_the_leg_pin_load_yaml_reads(project): + """As `bump-leg.py` hands it the text of a leg pin it is to rewrite.""" + pin = project / SPEC_PIN + pin.write_bytes(MARK + COMMENT + pin.read_bytes()) + read = load_yaml(pin) + assert shape_materialize.pin_values_in(pin.read_text(encoding="utf-8")) \ + == (read["commit"], read["digests"]["tree_sha256"]) + + +def test_the_doctor_puts_back_a_hand_saved_pin_from_head_after_a_re_pin( + standard, hand_saved, tmp_path): + """THE REGRESSION ROW. `update-shape.py apply` re-pins the hand-saved + pin onto a newer standard, keeping the mark and the comment, and the + validators read it. The working pin is then broken. Its NEXT puts back + HEAD's pin, the one every validator reads; with the mark taken off in + `load_yaml` alone it was `checkout '~1'`: the + pin from before the hand-save, naming the commit the re-pin moved it + off.""" + moved = tmp_path / "moved" / "openRepoShape" + moved.parent.mkdir() + git("clone", "-q", str(standard), str(moved), cwd=tmp_path) + target = moved_on(moved, "c2", UPSTREAM_TARGET) + apply_on(moved, hand_saved, target, "shape/re-pin") + pin = hand_saved / SHAPE_PIN + pin.write_bytes(pin.read_bytes() + b"files: [\n") + row = rows_of(doctor(moved, hand_saved, "--json"))["shape-currency"] + assert row["detail"]["restore"] == "uncommitted", row["detail"] + assert f"checkout HEAD -- {SHAPE_PIN}" in row["next"], row["next"] From a949534176f4421096a3b7730ccd7b6677ebc579 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 03:21:16 +0000 Subject: [PATCH 2/2] tests/test_bom_reads.py: only the marks that start the text are taken off, so a mark inside a quoted value, at the start of a value or at the start of a later key stays text, and the notice says nothing of a pin whose commit holds one (#855) Round 1 of #868, RV868's SHOULD-1 (#868 issuecomment-6245096782): a mutant that took off every U+FEFF anywhere, `text.replace(BYTE_ORDER_MARK, "")` in `parse_yaml`, survived 749 tests. Four rows now pin the strip to the start of the text, each mark written as bytes from `codecs.BOM_UTF8`: - `load_yaml` reads `k: "x"` as `{"k": "x"}`, as YAML 1.2.2 section 5.2 allows a mark inside a quoted scalar; - `commit: ` and 40 `a` as a commit that starts with U+FEFF; - `k: v`, then `j: w`, as the keys `k` and U+FEFF and `j`; - and the assembly root's upstream notice says nothing for that pin. PyYAML 6.0.3 reads the three files the same (measured). The mutant is killed now (4 failed), and so are RV868's mutant B and lane 4's four. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- tests/test_bom_reads.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_bom_reads.py b/tests/test_bom_reads.py index dfed4820..b60752b6 100644 --- a/tests/test_bom_reads.py +++ b/tests/test_bom_reads.py @@ -48,6 +48,8 @@ from repo_shape import Refusal, load_yaml # noqa: E402 MARK = codecs.BOM_UTF8 +#: The mark as the one character the reader reads it as, U+FEFF. +MARK_TEXT = MARK.decode("utf-8") #: The line a person may put above the file's first key before saving it. COMMENT = b"# saved by hand\n" PINNED = "a" * 40 @@ -136,6 +138,35 @@ def test_a_byte_that_is_not_utf_8_is_counted_from_the_files_first_byte( ": not UTF-8 at byte 7 (invalid start byte)"), refused.value.detail +@pytest.mark.parametrize("body, read", [ + pytest.param(b'k: "' + MARK + b'x"\n', {"k": MARK_TEXT + "x"}, + id="in-a-quoted-value"), + pytest.param(b"commit: " + MARK + PINNED.encode() + b"\n", + {"commit": MARK_TEXT + PINNED}, id="at-the-start-of-a-value"), + pytest.param(b"k: v\n" + MARK + b"j: w\n", + {"k": "v", MARK_TEXT + "j": "w"}, + id="at-the-start-of-the-second-key"), +]) +def test_a_mark_after_the_start_of_the_text_is_text(tmp_path, body, read): + """ONLY THE START IS TAKEN OFF (RV868's SHOULD-1). YAML 1.2.2 section + 5.2 allows a mark inside a quoted scalar, as text, and none inside a + document, which the reader reads as text. PyYAML 6.0.3 reads all three + as these do. A strip of every mark anywhere would read each of them as + the text without it.""" + path = tmp_path / "hand-saved.yaml" + path.write_bytes(body) + assert load_yaml(path) == read + + +def test_the_notice_says_nothing_of_a_pin_whose_commit_holds_the_mark( + notice, tmp_path, capsys): + """A pin whose `commit:` value starts with the mark names no commit, as + `git rev-parse --verify` refuses one (RV868), so the notice is silent.""" + notice(a_root(tmp_path, b"commit: " + MARK + PINNED.encode() + b"\n", + f"{TIP}\n".encode())) + assert capsys.readouterr().out == "" + + @pytest.mark.parametrize("marks", [1, 2], ids=["mark", "two-marks"]) def test_the_advisory_reads_a_marked_manifest_as_load_yaml_does(project, marks):