From 9ecf7bf389bb36d5e1808eae2e46c2b3c052755c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 14:14:29 +0000 Subject: [PATCH 1/3] validate-pins.py and repo_shape.py: a pin, the manifest or a shape copy the gate cannot reach or read is refused by name, relative to the root, once and with its cause's advice, and every name and path it prints is spelled (#712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #712 gathers lane 4's checklist lines L4-103, L4-104, L4-106 and L4-144, with the riders L4-105, L4-143 and L4-102, on the assembly root's lockstep gate. This takes all seven. L4-103. The leg pin, the shape pin, each shape copy, each neutral-product pin and the manifest were asked with `Path.is_file()`. Up to Python 3.13 it raised a refused `stat`, so a `contracts/` at mode 000 or r-- ended the run in a PermissionError traceback, exit 1; on 3.14 it answered False, and the run refused `pin-missing`, a present file said not to exist, under THE LOCKSTEP RULE's remedy. The five sites are `_is_file` now: #633's `_is_a`, and a refused `stat` is a refusal naming the file. A name no `stat` can be handed (a NUL in a role or a product) is no file, as `is_file()` answered. A shape copy at mode 000 passed its `stat` and ended in `file_sha256`'s PermissionError on every Python, in the same function; its open is the same refusal. A copy has no id for one, so it is `shape-copy-unreadable`, beside `shape-copy-missing` and `shape-copy-drift`; the YAML files keep `yaml-unreadable`, the id `load_yaml` gives them. L4-104. A pin this user cannot open was `load_yaml`'s line, `: [Errno 13] Permission denied: ''`, with the lockstep remedy, which moves a pin and says nothing of access. `_load` reads the `OSError` the refusal is raised from, which `load_yaml` has chained since it was written, and names the file relative to the root, POSIX, once: `contracts/spec-pin.yaml: cannot be read (Permission denied)`, with a remedy of its own, so `main` adds no other. Read off the cause and not off new library words, so a root whose `scripts/repo_shape.py` is older than its `validate-pins.py` names the file the same way (#370, #610). `load_yaml` itself names the path once for every caller: `: cannot be read ()`. One rule for every file the gate names, on the coordinator's ruling: the other refusals `load_yaml` raises here (not UTF-8, not parsable) keep their id, words and remedy and name the file relative to the root, and so do `not a mapping` for the four YAML files, `shape-pin-missing`, `manifest-missing` and `manifest-no-legs`, which named the absolute path raw. L4-105. Every file this gate cannot open is told what its cause needs. A permission keeps the words it had (a workflow) or gets bump-leg.py's (#383) for a pin, the manifest or a copy: `chmod u+r` on a file, `u+rx` on each directory above it. Any other cause, an I/O error or a name too long, gets bump-leg.py's other sentence and is never told to `chmod`. `_Unreadable` carries the cause, so one workflow refusal naming both kinds says both. L4-106, L4-144 and every other print, on the coordinator's ruling (option A). A workflow's name, a pin's path built from a role or a `neutral_product_pins:` entry, a leg's path, role and repository, a pin's `source_repository`, a declared product, a shape row's path and digest, the shape pin's and the manifest's commits, a `--pin-source` value with no path and the root in a printed `bump-leg.py` line each reached stderr or stdout raw: a newline began a line, a bidi control reordered one, a C1 control broke one, an ESC was the control it is. Each is printed as `_shown` spells it now: `repr` where a character is not printable, the same bytes where every one is. A word of a printed command is `shlex.quote`d as before where it is printable, and `_shown`'s where it is not (`_quoted`): no quoting makes a control character pasteable, and quoted it still reached the terminal. The workflow-reference note's repository is not spelled: it prints only a repository equal to a reference `WORKFLOW_REF_RE` matched, which is printable ASCII. L4-143. A `--pin-source` under a `neutral_product_pins:` that is not a list was noted "project.yaml declares no neutral-product pin"; it says the field is not a list and points at the `manifest-neutral-product-pins` finding. L4-102. `PinSources` keys `--pin-source` products by `pin_product_key`, the pin file's own fold, as #638 keys the naming gate's `LinkSources`: `casefold()` took `openss=` for `openß`, two pin files. Re-pin: `templates/assembly-root/scripts/validate-pins.py` and `scripts/repo_shape.py` are pinned shape copies. As #634 says, no digest is recorded in this repository; each root's `contracts/shape-pin.yaml` reads both as `upstream-changed`, which `update-shape.py apply` re-copies and re-pins. Tests: tests/test_validate_pins_named_and_spelled.py (new): the issue's modes through the real gate, the five sites under a refused `stat` on every Python, every file named relative to the root, each cause's advice, one row for each print over a newline, a bidi control, a C1 control, an ESC and a byte that is not UTF-8 (and a printable control), the issue's own pin entry and workflow names, the L4-143 note and the L4-102 keys. Three existing tests held the old absolute path: tests/test_refusal_default_remediation.py (the `yaml-unparsable` line under the lockstep remedy), tests/test_bump_leg_unreadable.py (the validator names the other leg's pin POSIX now, on Windows too) and a docstring in tests/test_validate_pins_unreadable_workflow.py. Closes #712 Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/repo_shape.py | 11 +- .../assembly-root/scripts/validate-pins.py | 434 +++++-- tests/test_bump_leg_unreadable.py | 11 +- tests/test_refusal_default_remediation.py | 6 +- tests/test_validate_pins_named_and_spelled.py | 1052 +++++++++++++++++ .../test_validate_pins_unreadable_workflow.py | 6 +- 6 files changed, 1394 insertions(+), 126 deletions(-) create mode 100644 tests/test_validate_pins_named_and_spelled.py diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 570b7b3a..370a7140 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -999,11 +999,20 @@ def load_yaml(path: Path) -> Any: and only then fails to decode it, so the `OSError` clause never sees it and, uncaught, it left every reader as a Python traceback (#192) rather than the `REFUSED yaml-unreadable` line a missing file already gets. + + AN `OSError` NAMES THE FILE ONCE (#712). `str()` of one is `[Errno 13] + Permission denied: ''`, so `: ` named the path twice, + the second time in Python's quoting. The line is `: cannot be read + ()`, the words `validate-pins.py` gives a workflow + file it cannot open (#359), and the refusal is still raised `from` the + `OSError`, which that validator reads to name the file relative to its + root. """ try: text = Path(path).read_text(encoding="utf-8") except OSError as exc: - raise Refusal("yaml-unreadable", f"{path}: {exc}") from exc + raise Refusal("yaml-unreadable", + f"{path}: cannot be read ({_os_reason(exc)})") from exc except UnicodeDecodeError as exc: raise Refusal( "yaml-unreadable", diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index d628d6c2..1baefc1b 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -45,10 +45,11 @@ recheckable neutral-product pin disagrees 1 a FINDING: drift a reviewer can act on 2 a REFUSAL: the question could not be asked — an uninitialized submodule, - an unresolvable commit, an unreadable pin or workflow file (one that is - not UTF-8, or that this user cannot open), or a `.github/workflows/` - directory this user cannot list or search (#434). An unanswerable pin - question is never an implicit pass. + an unresolvable commit, an unreadable pin, manifest, workflow file or + pinned shape copy (one that is not UTF-8, or that this user cannot + open or reach, #712), or a `.github/workflows/` directory this user + cannot list or search (#434). An unanswerable pin question is never an + implicit pass. No git to run is one too (`git-not-runnable`, #412), a git that is missing, will not run or cannot be executed: the gitlinks and the digests are read through git. @@ -166,10 +167,13 @@ def unused(self, text: str) -> None: class _Unreadable(NamedTuple): """One workflow file that could not be read (#359): the line that names it, - and whether it failed on its ENCODING (`True`) or on being opened.""" + whether it failed on its ENCODING (`True`) or on being opened, and, for + one that could not be opened, whether the operating system refused a + PERMISSION (#712): any other cause gets its own advice, not `chmod`.""" line: str encoding: bool + permission: bool = False def _read_workflow(root: Path, path: Path) -> tuple[str, _Unreadable | None]: @@ -183,17 +187,18 @@ def _read_workflow(root: Path, path: Path) -> tuple[str, _Unreadable | None]: it into git and into an editor that opens it from the root. A file this user cannot open (`OSError`) is the same line with the operating system's own reason, because it is the same unguarded read. + The name is spelled as `_named` spells it (#712): a newline in it began a + line of its own, and a bidi control reordered the line it was printed on. """ - rel = path.relative_to(root).as_posix() + rel = _named(root, path) try: return path.read_text(encoding="utf-8"), None except UnicodeDecodeError as exc: return "", _Unreadable( f"{rel}: not UTF-8 at byte {exc.start} ({exc.reason})", True) except OSError as exc: - return "", _Unreadable( - f"{rel}: cannot be read ({exc.strerror or type(exc).__name__})", - False) + return "", _Unreadable(_cannot_read(rel, exc), False, + isinstance(exc, PermissionError)) def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: @@ -204,6 +209,12 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: them all before it raises. The first line follows the id and the rest follow it, one file to a line, so `shape-doctor.py`, which quotes the first `REFUSED` line of this script's output, quotes a file's name. + + THE ADVICE FOLLOWS THE CAUSE (#712), as the directory's does (#434): + "restore read access" was said of every `cannot be read` file, and a + file whose `stat` or read failed with an I/O error or a name too long + is not put right by a mode. A permission keeps its sentence; any other + cause gets `OTHER_REASON`, the words `bump-leg.py` gives it (#383). """ advice = "" if any(one.encoding for one in files): @@ -213,12 +224,14 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: "as UTF-8 (a UTF-16 file with a byte-order mark needs converting, " "not editing), or replace that byte with the character it was " "meant to be, and commit it. ") - if not all(one.encoding for one in files): + if any(one.permission for one in files): advice += ( "A `cannot be read` file is not an encoding failure: this user " "cannot open it, so restore read access to it, and search access " "to every directory above it inside the root (a file in a " "directory this user can list but not search cannot be opened). ") + if any(not one.encoding and not one.permission for one in files): + advice += OTHER_REASON return Refusal( "workflow-unreadable", "\n".join(one.line for one in files), @@ -266,6 +279,115 @@ def _is_a(path: Path, kind: Callable[[int], bool]) -> bool: return kind(mode) +def _named(root: Path, path: Path) -> str: + """`path` as this validator names a file (#712): relative to `root`, POSIX + on every platform, as #633's `workflow-unreadable` lines are, and spelled + by `_shown`, so a name holding a newline, a bidi or C1 control, an ESC or + a byte that is not UTF-8 prints as `repr` spells it and stays on its + line. A path that is not under the root (a role or a product spelled as + an absolute path) is named whole, since `relative_to` would end the run + in a `ValueError`.""" + try: + return _shown(path.relative_to(root).as_posix()) + except ValueError: + return _shown(str(path)) + + +def _cannot_read(named: str, exc: OSError) -> str: + """`: cannot be read ()`: the line + for a file whose `stat` or open the operating system refused, in every + refusal that names one (#359, #434, #712).""" + return f"{named}: cannot be read ({exc.strerror or type(exc).__name__})" + + +#: What a file the operating system refused for ANOTHER reason than +#: permission is told (#712): an I/O error, a name longer than the +#: filesystem takes, a file replaced while this ran. No `chmod` puts one +#: right. `bump-leg.py`'s words for the same refusal (#383). +OTHER_REASON = ( + "Where the operating system gave another reason, put a regular file this " + "user can open back in its place, or take away one nobody wants, and make " + "sure nothing is changing the directory while this runs. ") + + +def _unreadable_file(code: str, named: str, exc: OSError) -> Refusal: + """The refusal for a pin, the manifest or a pinned shape copy that this + validator must read and the operating system will not let it (#712): + its `stat` refused (a `contracts/` this user cannot search) or its open + refused (a pin at mode 000). + + NAMED ONCE, RELATIVE TO THE ROOT. A pin this user could not open was + `load_yaml`'s line, `: [Errno 13] Permission denied: + ''`, under THE LOCKSTEP RULE's remedy, which moves a pin + and says nothing of access. This is `_cannot_read`'s line, with a remedy + of its own, so `main` adds no other. + + THE ADVICE FOLLOWS THE CAUSE, in `bump-leg.py`'s words for the same + file (#383): a permission (`PermissionError`, which is `EACCES` and + `EPERM`) is put right by the mode or the owner, and any other cause is + `OTHER_REASON`, which is not told to `chmod`. + """ + advice = ( + "Make what this user may not open readable by this user: `chmod u+r` " + "on a file, `chmod u+rx` on a directory (and on every directory above " + "it inside the root), or fix who owns it. " + if isinstance(exc, PermissionError) else OTHER_REASON) + return Refusal( + code, _cannot_read(named, exc), + "Remediation: this validator reads each leg's pin, the shape pin, " + "each neutral-product pin, project.yaml and every copy the shape pin " + "lists, and one it cannot read is a question it cannot answer, never " + "a pass. " + + advice + "Then run the same command again.") + + +def _is_file(root: Path, path: Path, code: str = "yaml-unreadable", + named: str | None = None) -> bool: + """Whether `path` is a file, asked as `_is_a` asks it, or `code`'s + refusal naming it (`named`, else `_named`) where its `stat` was refused + (#712). The four `is_file()` calls on a pin and the manifest, and the + one on each shape copy, are this. + + `Path.is_file()` raised a refused `stat` out of the run on Python 3.9 to + 3.13, so a `contracts/` this user could not search ended in a + `PermissionError` traceback, exit 1; on 3.14 it answered False, and the + run refused `pin-missing`, a present file said not to exist, under the + lockstep remedy. A name the operating system cannot take at all (a NUL + in a role or a product) is no file, as `is_file()` answered it.""" + try: + return _is_a(path, stat.S_ISREG) + except OSError as exc: + raise _unreadable_file(code, named or _named(root, path), + exc) from exc + except ValueError: + return False + + +def _load(root: Path, path: Path): + """`load_yaml(path)`, with every refusal it raises naming the file as + `_named` does (#712). + + `load_yaml` names the path it was given, which is absolute here and in + the platform's own spelling, raw. A file this user cannot open is + `_unreadable_file`'s refusal, `yaml-unreadable`, the id it always had; + any other keeps its id, its words and its remedy, with the path at its + start named relative to the root. Asked of the refusal and not of new + words in `repo_shape`, so a root whose `scripts/repo_shape.py` is older + than this file names the file the same way (#370, #610): `load_yaml` + has raised its refusal `from` the `OSError` since it was written.""" + try: + return load_yaml(path) + except Refusal as exc: + if isinstance(exc.__cause__, OSError): + raise _unreadable_file("yaml-unreadable", _named(root, path), + exc.__cause__) from exc.__cause__ + absolute = f"{path}: " + if not exc.detail.startswith(absolute): + raise + raise Refusal(exc.code, _named(root, path) + ": " + + exc.detail[len(absolute):], exc.remediation) from exc + + def _workflows_unlistable(root: Path, workflows: Path, exc: OSError) -> Refusal: """`workflow-unreadable` for the DIRECTORY, named as one (#434). @@ -345,10 +467,11 @@ def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: if not _is_a(path, stat.S_ISREG): continue except OSError as exc: - # The words `_read_workflow` gives a file it cannot open. + # The words `_read_workflow` gives a file it cannot open, the + # name spelled and the cause kept for the advice (#712). unreadable.append(_Unreadable( - f"{path.relative_to(root).as_posix()}: cannot be read " - f"({exc.strerror or type(exc).__name__})", False)) + _cannot_read(_named(root, path), exc), False, + isinstance(exc, PermissionError))) continue text, problem = _read_workflow(root, path) if problem is not None: @@ -368,23 +491,27 @@ def _load_leg_pin(root: Path, role, path: str, repository: str, Split from `_check_leg` for #132: everything that answers "does the pin file itself look right" belongs together. Returns the pin, its path - relative to `root`, and its `commit:` lowercased. + relative to `root` as `_named` spells it, and its `commit:` lowercased. + + A pin that cannot be reached or read is refused by name (#712): see + `_is_file` and `_load`. Every name is spelled, the leg's role and + repository in the lines below as the pin's path is. """ pin_path = root / "contracts" / f"{role}-pin.yaml" - if not pin_path.is_file(): + # POSIX, on every platform: a finding names a path in THIS repository, + # and `str()` of a relative Path on Windows spells it with backslashes — + # a path the reader cannot paste back into git. + rel = _named(root, pin_path) + if not _is_file(root, pin_path): raise Refusal( "pin-missing", - f"leg {role!r} declares {repository} at {path!r} but " - f"{pin_path.relative_to(root).as_posix()} does not exist", + f"leg {role!r} declares {_shown(repository)} at {path!r} but " + f"{rel} does not exist", ) - pin = load_yaml(pin_path) + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("pin-unreadable", f"{rel}: not a mapping") - # POSIX, on every platform: a finding names a path in THIS repository, - # and `str()` of a relative Path on Windows spells it with backslashes — - # a path the reader cannot paste back into git. - rel = pin_path.relative_to(root).as_posix() if pin.get("kind") != "pinned_contract_manifest": report.finding("pin-wrong-kind", f"{rel}: kind is {pin.get('kind')!r}, expected " @@ -406,7 +533,8 @@ def _load_leg_pin(root: Path, role, path: str, repository: str, if pin.get("submodule_path") != path: report.finding("pin-path-mismatch", f"{rel}: submodule_path is {pin.get('submodule_path')!r} " - f"but project.yaml puts the {role} leg at {path!r}") + f"but project.yaml puts the {_shown(str(role))} leg at " + f"{path!r}") if pin.get("source_repository") != repository: report.finding("pin-repository-mismatch", f"{rel}: source_repository is " @@ -493,10 +621,22 @@ def _gitlink_repairs(root: Path, role, gitlink: str, commit: str) -> str: "commit and the second the pin's", gitlink, commit) +def _quoted(value) -> str: + """`value` as one word of a command this validator prints: `shlex.quote`d, + as every such word always was, unless a character in it is not printable, + and then as `_shown` spells it (#712). Quoted, an ESC or a line break in + a root's path or a leg's role reached the terminal as the control it is; + spelled, the word cannot be pasted as the value it names, and no quoting + could make it so.""" + text = str(value) + return shlex.quote(text) if text.isprintable() else _shown(text) + + def _bump_command(root: Path, role) -> str: - """`bump-leg.py` for this root's leg `role`, up to its `--to` (#478).""" - return (f"{PYTHON} scripts/bump-leg.py --root {shlex.quote(str(root))} " - f"--leg {shlex.quote(str(role))} --to") + """`bump-leg.py` for this root's leg `role`, up to its `--to` (#478), + each word as `_quoted` spells it (#712).""" + return (f"{PYTHON} scripts/bump-leg.py --root {_quoted(root)} " + f"--leg {_quoted(role)} --to") #: What a `bump-leg.py` line needs to commit, said once for every finding @@ -576,10 +716,11 @@ def _put_back(paths: list[str]) -> str: other leg's, which holds the next line back and asks for this step again. AN UNDO IS FOLLOWED BY `git submodule update` over the legs: it puts the gitlink back and leaves the leg checked out at the move, a - root `bump-leg.py` refuses as dirty.""" + root `bump-leg.py` refuses as dirty. Each path is spelled (#712).""" one = len(paths) == 1 - which, moved = (f"{paths[0]}'s", "it") if one else ("one leg's", "them") - legs = " ".join(shlex.quote(path) for path in paths) + which, moved = ((f"{_shown(paths[0])}'s", "it") if one + else ("one leg's", "them")) + legs = " ".join(_quoted(path) for path in paths) return (f"put {which} gitlink back at its pin (check that leg out at the " "pin's commit and commit its gitlink in this root, with every " "workflow reference to its repository at that commit too), or " @@ -593,15 +734,17 @@ def _held_then(lines: str, whom: str, back: Back | None, for `whom`. Never a line while `back`'s own digest stands, since two legs are then out of step with no gitlink off; and in the gitlink's own finding (`other_legs`), how its move is kept if it was meant: the - whole commit, which is the only `--to` `bump-leg.py` takes.""" + whole commit, which is the only `--to` `bump-leg.py` takes. `back`'s + path is spelled (#712); `whom` comes spelled.""" if back is not None and back.digest: - return (f"names no line while {back.path}'s own pin-digest-mismatch " - "stands.") + return (f"names no line while {_shown(back.path)}'s own " + "pin-digest-mismatch stands.") if back is not None and other_legs: after = ("after that line" if lines.endswith("line") else "after one of them") - return (f"names {lines} for {whom}, and if {back.path}'s move was " - f"meant, bumping {back.path} to {back.gitlink} {after} keeps " + named = _shown(back.path) + return (f"names {lines} for {whom}, and if {named}'s move was " + f"meant, bumping {named} to {back.gitlink} {after} keeps " "it.") return f"names {lines} for {whom}." @@ -627,20 +770,21 @@ def _gitlink_repairs_held_back(paths: list[str], others: list[str], by hand, for `back`. The gitlink's own finding is given `other_legs`, `{path: codes}`, and `lines` are then that leg's; the other leg's finding names its own `lines`, for itself by its path, since "that leg" - would read as `back`. `_held_then` says what follows. + would read as `back`. `_held_then` says what follows. Every path is + spelled (#712). """ what, first, whom = [], [], "that leg" if len(paths) > 1: - what.append(" and ".join(paths) + (" are both" if len(paths) == 2 - else " are all") + what.append(" and ".join(_shown(path) for path in paths) + + (" are both" if len(paths) == 2 else " are all") + " off their pins") whom = "the leg that is left" elif other_legs: - whom = " and ".join(other_legs) + whom = " and ".join(_shown(path) for path in other_legs) elif back is not None: - what.append(f"{back.path} is off its pin") - whom = paths[0] - what += [f"{path} is out of step too ({', '.join(codes)})" + what.append(f"{_shown(back.path)} is off its pin") + whom = _shown(paths[0]) + what += [f"{_shown(path)} is out of step too ({', '.join(codes)})" for path, codes in (other_legs or {}).items()] if others: what.append("this validator also reports " + ", ".join(others)) @@ -777,9 +921,10 @@ def _out_of_step(report: Report) -> tuple[list, list[str]]: def _curable_ending(root: Path, one: Curable, legs: list, untagged: list[str], off: dict, named) -> str: """What `_end_curable_findings` adds to the finding `one`, or `""`. - `off` is `{role: Back}` of each leg whose gitlink is off its pin.""" + `off` is `{role: Back}` of each leg whose gitlink is off its pin. The + leg's path is spelled (#712).""" if one.role in off: - return (f"Either bump-leg.py line under {one.path}'s " + return (f"Either bump-leg.py line under {_shown(one.path)}'s " "pin-gitlink-mismatch above clears this finding too." if one.role == named else "") # Every other leg out of step, as the `Back` of its gitlink off its pin, @@ -801,28 +946,31 @@ def _check_gitlink_matches_pin(root: Path, role, path: str, commit: str, rel: str, report: Report) -> str: """Fact 1 vs fact 2: the recorded gitlink against the pin file's `commit:`. Split from `_check_leg` for #132. Returns the gitlink so the - caller can check it again against fact 3, the workflow references. + caller can check it again against fact 3, the workflow references. The + role and the path are spelled (#712). """ - with _git_runs(f"the {role} leg's gitlink recorded at {path!r} could " - "not be read"): + role_named = _shown(str(role)) + with _git_runs(f"the {role_named} leg's gitlink recorded at {path!r} " + "could not be read"): gitlink = recorded_gitlink(root, path) if gitlink is None: raise Refusal( "pin-gitlink-absent", - f"no gitlink (mode 160000) recorded at {path!r}. The {role} leg is " - "declared in project.yaml but this repository does not record it " - "as a submodule.", + f"no gitlink (mode 160000) recorded at {path!r}. The {role_named} " + "leg is declared in project.yaml but this repository does not " + "record it as a submodule.", ) if gitlink != commit: report.finding( "pin-gitlink-mismatch", - f"{path}: gitlink {gitlink} != {rel} commit {commit}\n " + LOCKSTEP, + f"{_shown(path)}: gitlink {gitlink} != {rel} commit {commit}\n " + + LOCKSTEP, leg=role, ) report.mismatches.append((len(report.findings) - 1, role, path, gitlink, commit)) else: - report.note(f"{path}: gitlink == {rel} commit {commit[:12]}") + report.note(f"{_shown(path)}: gitlink == {rel} commit {commit[:12]}") return gitlink @@ -831,20 +979,22 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, """The digest: split from `_check_leg` for #132. Recomputes the leg's tree sha256 at the pinned commit, because a gitlink and a pin file that agree on a commit may still disagree on that commit's bytes. A mismatch - is one `bump-leg.py` clears, since it recomputes the digest (#478). + is one `bump-leg.py` clears, since it recomputes the digest (#478). The + leg's path is spelled in every line (#712). """ submodule = root / path + named = _shown(path) if not (submodule / ".git").exists(): raise Refusal( "pin-submodule-uninitialized", - f"{path} is not an initialized submodule checkout, so the pinned " + f"{named} is not an initialized submodule checkout, so the pinned " "digest cannot be recomputed. Presence is not identity and an " "unreadable surface must fail rather than degrade.", ) # `_git_runs` OUTSIDE the `try`: the refusal it raises is a `Refusal`, and # inside, the `except Refusal` below would turn "no git to run" into # `pin-commit-unresolvable`, which says the commit is missing. - with _git_runs(f"{path}: the pinned commit {commit[:12]} could not be " + with _git_runs(f"{named}: the pinned commit {commit[:12]} could not be " "looked up in that submodule"): try: git_out(["rev-parse", "--verify", "--quiet", @@ -852,7 +1002,7 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, except Refusal as exc: raise Refusal( "pin-commit-unresolvable", - f"{path}: the pinned commit {commit} is not in that " + f"{named}: the pinned commit {commit} is not in that " f"submodule's object store ({exc.detail})", ) from exc recorded = (pin.get("digests") or {}).get("tree_sha256") @@ -867,7 +1017,7 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, "unstated is a number, not an identity.", ) else: - with _git_runs(f"{path}: the tree digest at {commit[:12]} could not " + with _git_runs(f"{named}: the tree digest at {commit[:12]} could not " "be recomputed"): actual = tree_digest(submodule, commit) if actual.lower() != recorded.lower(): @@ -882,7 +1032,8 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, report.curable.append(Curable(len(report.findings) - 1, role, path, commit, None)) else: - report.note(f"{path}: tree digest recomputes ({recorded[:12]}…)") + report.note(f"{named}: tree digest recomputes " + f"({recorded[:12]}…)") def _check_workflow_refs_for_leg(refs, repository: str, path: str, @@ -900,7 +1051,7 @@ def _check_workflow_refs_for_leg(refs, repository: str, path: str, if wf_sha != gitlink: report.finding( "pin-workflow-ref-mismatch", - f"{wf_path.name}: `{wf_repo}@{wf_sha}` != the gitlink " + f"{_shown(wf_path.name)}: `{wf_repo}@{wf_sha}` != the gitlink " f"{gitlink} recorded at {path!r}\n " + LOCKSTEP, leg=role, ) @@ -929,14 +1080,19 @@ def _check_shape_pin_header(root: Path, manifest: dict, report: Report Split from `_check_shape_pin` for #132. Returns the pin and its path relative to `root`. + + Named relative to the root in every line, the two refusals included, + which named it absolute, and refused by name where it cannot be reached + or read (#712). The two commits it compares are spelled: neither is + asked to be 40 hex before the finding that names both. """ pin_path = root / "contracts" / "shape-pin.yaml" - if not pin_path.is_file(): - raise Refusal("shape-pin-missing", f"{pin_path} does not exist") - pin = load_yaml(pin_path) - rel = pin_path.relative_to(root).as_posix() + rel = _named(root, pin_path) + if not _is_file(root, pin_path): + raise Refusal("shape-pin-missing", f"{rel} does not exist") + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("shape-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("shape-pin-unreadable", f"{rel}: not a mapping") if pin.get("revision_kind") != "commit": report.finding("pin-tag-only", f"{rel}: revision_kind is {pin.get('revision_kind')!r}") @@ -947,8 +1103,8 @@ def _check_shape_pin_header(root: Path, manifest: dict, report: Report declared = (manifest.get("shape") or {}).get("commit") if declared and str(declared).lower() != commit.lower(): report.finding("shape-pin-manifest-disagree", - f"{rel} commit {commit} != project.yaml shape.commit " - f"{declared}") + f"{rel} commit {_shown(commit)} != project.yaml " + f"shape.commit {_shown(str(declared))}") return pin, rel @@ -963,16 +1119,26 @@ def _check_shape_copy(root: Path, row, rel: str, report: Report) -> None: report.finding("shape-pin-row", f"{rel}: a files row is not a mapping") return target = root / str(row.get("path")) - if not target.is_file(): + # A copy whose `stat` or read is refused is `shape-copy-unreadable`, a + # refusal naming it as the row does (#712). `is_file()` raised a refused + # `stat` out of the run up to Python 3.13, and called the copy "pinned + # but absent" on 3.14; a copy at mode 000 ended in `file_sha256`'s + # `PermissionError` on every Python. The row's path and digest are + # spelled, as the pin spells them. + named = _shown(str(row.get("path"))) + if not _is_file(root, target, "shape-copy-unreadable", named): report.finding("shape-copy-missing", - f"{rel}: {row.get('path')} is pinned but absent") + f"{rel}: {named} is pinned but absent") return - actual = file_sha256(target) + try: + actual = file_sha256(target) + except OSError as exc: + raise _unreadable_file("shape-copy-unreadable", named, exc) from exc if actual != str(row.get("sha256", "")).lower(): report.finding( "shape-copy-drift", - f"{row.get('path')}: sha256 {actual}\n" - f" {rel} records {row.get('sha256')}\n" + f"{named}: sha256 {actual}\n" + f" {rel} records {_shown(str(row.get('sha256')))}\n" " A shape file was edited in place. Either revert it, or " "carry the change upstream to openRepoShape and re-pin.", ) @@ -1032,9 +1198,9 @@ class PinSources: that a source keyed to a product `a` that nothing declares, which no pin ever looked up — the digest went to `gh api` and a `skip`, exit 0, with the skip line itself suggesting the `--pin-source` the person had just - passed. So the text before the first `=` is a PRODUCT only when it - casefolds to a declared pin's name, the same casefold the lookup below - uses, and any other value is a bare PATH, whole. A declared name wins + passed. So the text before the first `=` is a PRODUCT only when its + `pin_product_key` is a declared pin's, the key the lookup below uses, + and any other value is a bare PATH, whole. A declared name wins even when a directory is spelled like the whole value (`openGlass=x/openGlass` beside a directory `openGlass=x`): the reading is decided by the manifest and never by what happens to be on the disk, and @@ -1049,14 +1215,24 @@ class PinSources: A VALUE THAT NAMES NO PATH IS REFUSED (#442), `pin-source-no-path`: `openGlass=` with nothing after its `=`, or an empty value. Either was `Path("")`, which is `.`, so the directory this ran in was asked for the - pinned commit and the finding blamed `.`. `.` typed still means it. + pinned commit and the finding blamed `.`. `.` typed still means it. The + value and the product it names are spelled, as `_shown` spells them. + + KEYED BY `pin_product_key`, the pin file's own fold (#712, after #638). + `casefold()` took `openss=` for `openß`, two pin files, where the chain + readers' `LinkSources` read them as two: the two copied tools read + `[PRODUCT=]PATH` one way over ASCII alone. A pin's digest stopped a wrong + source from passing; the source was simply looked up under the other + spelling. """ def __init__(self, values: list[str], declared) -> None: #: The declared pins, spelled as `project.yaml` spells them. self.declared = [p for p in declared if isinstance(p, str) and p] - names = {product.casefold(): product for product in self.declared} - #: `{product casefolded: (typed, path)}`, the last value given wins. + names = {pin_product_key(product): product + for product in self.declared} + #: `{pin_product_key(product): (typed, path)}`, the last value given + #: wins. self.keyed: dict[str, tuple[str, Path]] = {} #: `(typed, path)` of the last bare value, if any. self.bare: tuple[str, Path] | None = None @@ -1069,18 +1245,19 @@ def __init__(self, values: list[str], declared) -> None: self.used: set[str] = set() for typed in values: key, sep, path = typed.partition("=") - product = names.get(key.casefold()) if sep else None + product = names.get(pin_product_key(key)) if sep else None self.reading[typed] = product if product is not None: - earlier = self.keyed.get(product.casefold()) - self.keyed[product.casefold()] = (typed, Path(path)) + earlier = self.keyed.get(pin_product_key(product)) + self.keyed[pin_product_key(product)] = (typed, Path(path)) else: earlier = self.bare self.bare = (typed, Path(typed)) self.replaced.pop(typed, None) if earlier is not None and earlier[0] != typed: self.replaced[earlier[0]] = typed - no_path = [f"`--pin-source {typed}` names no path for {product}" + no_path = [f"`--pin-source {_shown(typed)}` names no path for " + f"{_shown(product)}" if typed else "`--pin-source ''` names no path" for typed, product in self.reading.items() if not (typed.partition("=")[2] if product else typed)] @@ -1097,14 +1274,16 @@ def read_as_path(self, typed: str | None) -> str: or `""` — what a person who meant `PRODUCT=PATH` and mistyped the product needs told, since nothing else in the run names it. Nothing when no pin is declared: the note has already said so. The key is - spelled as `_shown` spells the value (#610).""" + spelled as `_shown` spells the value (#610), and so is each declared + pin, which is the manifest's text (#712).""" if typed is None or self.reading.get(typed) is not None \ or "=" not in typed or not self.declared: return "" key = typed.partition("=")[0] what = f"`{_shown(key)}` is not" if key else "nothing before its `=` is" + declared = ", ".join(_shown(product) for product in self.declared) return (f"It was read as a PATH: {what} a declared neutral-product " - f"pin (declared: {', '.join(self.declared)}).") + f"pin (declared: {declared}).") def resolve_neutral_pin_source(root: Path, product: str, repository: str, @@ -1115,8 +1294,9 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, Tried in order, the most explicit ask first: - 1. `--pin-source =` (case-insensitive on the name, which - must be a declared pin's for the value to be read this way, #295) + 1. `--pin-source =` (case-insensitive on the name as + `pin_product_key` folds it, #712, and it must be a declared pin's + for the value to be read this way, #295) 2. `SHAPE_PIN_SOURCE_` in the environment 3. a checkout sitting BESIDE this assembly root, named for the pinned repository (`opensoft/openGlass` -> `../openGlass`) — the ordinary @@ -1135,19 +1315,20 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, Returns `(path, how, typed)`, `typed` being the `--pin-source` value as given when one of those answered, so the caller can mark it used (#295). `(None, None, None)` when nothing above answers; the caller's next step - is `gh api`, and after that, a SKIP. + is `gh api`, and after that, a SKIP. `how` is printed, so the product + and the sibling's path in it are spelled (#712). """ - keyed = sources.keyed.get(product.casefold()) + keyed = sources.keyed.get(pin_product_key(product)) if keyed is not None: typed, path = keyed - return path, f"--pin-source {product}=...", typed + return path, f"--pin-source {_shown(product)}=...", typed env_name = pin_source_env_name(product) env_value = None if env_shared else os.environ.get(env_name) if env_value: return Path(env_value), f"${env_name}", None sibling = root.parent / repo_basename(repository) if (sibling / ".git").exists(): - return sibling, f"sibling checkout ({sibling})", None + return sibling, f"sibling checkout ({_shown(str(sibling))})", None if sources.bare is not None: typed, path = sources.bare return path, "--pin-source", typed @@ -1198,6 +1379,12 @@ def _check_neutral_pin_entry_and_load(root: Path, product, report: Report the entry itself is malformed — already reported, and there is no pin file name to go read — so the caller moves on to the next entry exactly as the original loop's `continue` did. + + THE PIN'S PATH IS SPELLED (#712). The entry was `repr`'d and the path + built from it was not, so `["\\x1b[31mRED"]` put an ESC on stderr inside + `contracts/[31mred-pin.yaml`, and in the remedy's `--pin`. The pin + is named relative to the root in every line, and refused by name where + it cannot be reached or read. """ if not isinstance(product, str) or not product: report.finding( @@ -1207,21 +1394,21 @@ def _check_neutral_pin_entry_and_load(root: Path, product, report: Report ) return None pin_path = root / "contracts" / f"{product.lower()}-pin.yaml" - if not pin_path.is_file(): + rel = _named(root, pin_path) + if not _is_file(root, pin_path): raise Refusal( "neutral-pin-missing", - f"project.yaml declares a pin on {product!r} but " - f"{pin_path.relative_to(root).as_posix()} does not exist", + f"project.yaml declares a pin on {product!r} but {rel} does not " + "exist", "Remediation: `scaffold-project.py --pin " - f"{product}@` writes that file. A declaration in " + f"{_shown(product)}@` writes that file. A declaration in " "project.yaml with no pin file beside it is a claim with no " "referent — either restore the file or remove the " "declaration.", ) - pin = load_yaml(pin_path) - rel = pin_path.relative_to(root).as_posix() + pin = _load(root, pin_path) if not isinstance(pin, dict): - raise Refusal("neutral-pin-unreadable", f"{pin_path}: not a mapping") + raise Refusal("neutral-pin-unreadable", f"{rel}: not a mapping") return pin, rel @@ -1299,11 +1486,13 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, Split from `_check_neutral_product_pins` for #132. Returns `(actual, how)`, or `None` when nothing could answer — a finding or a skip is already reported in that case, exactly as the original loop's - `continue` left it. + `continue` left it. The product and the pin's `source_repository` are + spelled wherever they are printed (#712). """ env_shared = pin_product_key(product) in shared source, how, typed = resolve_neutral_pin_source(root, product, repository, sources, env_shared) + named = _shown(repository) if source is not None: if typed is not None: sources.used.add(typed) @@ -1313,11 +1502,11 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, report.finding( "neutral-pin-source-unreadable", f"{rel}: {_shown(str(source))} ({how}) could not answer for " - f"{repository} @ {commit}: {exc.detail}" + f"{named} @ {commit}: {exc.detail}" + "".join(f"\n {line}" for line in ( sources.read_as_path(typed), _also_a_directory(typed, sources), - f"Remediation: point it at a clone of {repository} that " + f"Remediation: point it at a clone of {named} that " "has that commit (`git fetch` in it first), or drop it " "so the next source in the order answers.") if line), ) @@ -1329,13 +1518,13 @@ def _recompute_neutral_pin_digest(root: Path, product: str, repository: str, # bare `--pin-source`, which answers for every pin that has no more # specific answer: the keyed form is the one that says which pin it # is for (#275). - offered = (f"A --pin-source {product}= or" if env_shared else - f"A --pin-source, {pin_source_env_name(product)}, or") + offered = (f"A --pin-source {_shown(product)}= or" if env_shared + else f"A --pin-source, {pin_source_env_name(product)}, or") report.skip( - f"{product}: pin commit {commit[:12]} digest NOT " + f"{_shown(product)}: pin commit {commit[:12]} digest NOT " f"rechecked — no local checkout ({exc.code}: " f"{exc.detail}). {offered} a checkout at " - f"../{repo_basename(repository)} beside " + f"{_shown('../' + repo_basename(repository))} beside " "this assembly root would verify it offline.", ) return None @@ -1376,7 +1565,7 @@ def _check_one_neutral_product_pin(root: Path, product, report: Report, "descent from has moved, or the pin was hand-edited.", ) else: - report.note(f"{product}: pin commit {commit[:12]} digest " + report.note(f"{_shown(product)}: pin commit {commit[:12]} digest " f"recomputes ({how})") @@ -1449,7 +1638,8 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, which `or []` read as an empty list, are the finding too. """ products = manifest.get("neutral_product_pins") - if products is not None and not isinstance(products, list): + not_a_list = products is not None and not isinstance(products, list) + if not_a_list: report.finding( "manifest-neutral-product-pins", f"neutral_product_pins is {products!r}, expected a list of " @@ -1461,10 +1651,11 @@ def _check_neutral_product_pins(root: Path, manifest: dict, report: Report, shared = frozenset(pin_source_env_shared(products)) for product in products: _check_one_neutral_product_pin(root, product, report, sources, shared) - _note_unused_pin_sources(sources, report) + _note_unused_pin_sources(sources, report, not_a_list) -def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: +def _note_unused_pin_sources(sources: PinSources, report: Report, + not_a_list: bool = False) -> None: """A note for each `--pin-source` value no declared pin read (#295), naming it as typed and why: a later value replaced it, its pin was never recomputed, nothing is declared, or every pin had a more specific @@ -1472,7 +1663,13 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: that is the mistyped key's only trace in the run. "As typed" is `_shown`'s (#610): a value that is not printable, and the - later value that replaced it, are named as `repr` spells them. + later value that replaced it, are named as `repr` spells them, and so is + the declared product a keyed value names (#712). + + A FIELD THAT IS NOT A LIST IS NAMED (#712): `not_a_list` says that + `neutral_product_pins` was `manifest-neutral-product-pins`'s finding, + so it declares nothing, and the note points at that finding, where it + said only that nothing is declared and left the reader to find why. """ for typed, product in sources.reading.items(): if typed in sources.used: @@ -1481,7 +1678,12 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: if later is not None: why = f"the later `--pin-source {_shown(later)}` replaced it" elif product is not None: - why = f"{product}'s digest was not recomputed from any source" + why = (f"{_shown(product)}'s digest was not recomputed from any " + "source") + elif not sources.declared and not_a_list: + why = ("project.yaml's neutral_product_pins is not a list, so it " + "declares no neutral-product pin (see the " + "manifest-neutral-product-pins finding)") elif not sources.declared: why = "project.yaml declares no neutral-product pin" else: @@ -1496,23 +1698,27 @@ def _note_unused_pin_sources(sources: PinSources, report: Report) -> None: def _load_legs_to_check(root: Path) -> tuple[dict, list[dict]]: """The manifest and its non-assembly legs, or a Refusal naming why there is nothing here to check. Split from `main` for #132. + + `project.yaml` is named relative to the root, as every file this + validator names is, where three refusals named its absolute path raw, + and it is refused by name where it cannot be reached or read (#712). """ manifest_path = root / "project.yaml" - if not manifest_path.is_file(): + if not _is_file(root, manifest_path): raise Refusal( "manifest-missing", - f"{manifest_path} does not exist, so there is no declaration of " + "project.yaml does not exist, so there is no declaration of " "which legs to check. A one-repository project has no legs and " "does not run this validator.", ) - manifest = load_yaml(manifest_path) + manifest = _load(root, manifest_path) if not isinstance(manifest, dict): - raise Refusal("manifest-unreadable", f"{manifest_path}: not a mapping") + raise Refusal("manifest-unreadable", "project.yaml: not a mapping") legs = [leg for leg in (manifest.get("legs") or []) if isinstance(leg, dict) and leg.get("role") != "assembly"] if not legs: raise Refusal("manifest-no-legs", - f"{manifest_path}: no non-assembly legs declared") + "project.yaml: no non-assembly legs declared") return manifest, legs diff --git a/tests/test_bump_leg_unreadable.py b/tests/test_bump_leg_unreadable.py index 0a07f891..4423c362 100644 --- a/tests/test_bump_leg_unreadable.py +++ b/tests/test_bump_leg_unreadable.py @@ -350,12 +350,11 @@ def test_the_other_legs_pin_stays_the_validators_refusal(project, scaffolded, commit_bytes(project, Path("contracts") / "code-pin.yaml", LATIN1_COMMENT, append=True) - # The file is named by the VALIDATOR, whose `load_yaml` prints the Path it - # was given, absolute and in the platform's own spelling: - # `...\Atlas\contracts\code-pin.yaml` on Windows. This tool's own refusals - # name a file by its repository path, `as_posix()`, and are asserted as - # such everywhere else in this module; this one is not its text. - pin = str(Path("contracts") / "code-pin.yaml") + # The file is named by the VALIDATOR, relative to the root and POSIX on + # every platform, as this tool's own refusals name one (#712). It named + # the Path its `load_yaml` was given, absolute and in the platform's own + # spelling: `...\Atlas\contracts\code-pin.yaml` on Windows. + pin = "contracts/code-pin.yaml" stderr = refused_and_untouched( project, scaffolded, advanced, "bump-leg-validators-red", "yaml-unreadable", pin) diff --git a/tests/test_refusal_default_remediation.py b/tests/test_refusal_default_remediation.py index 181bf3f5..fcfd910c 100644 --- a/tests/test_refusal_default_remediation.py +++ b/tests/test_refusal_default_remediation.py @@ -255,7 +255,8 @@ def _break_spec_pin(project: Path, how: str) -> str: f"'{short}', which is not 40 hex. An abbreviated oid, a " "branch or a tag is a moving reference.") pin.write_bytes(b"commit: [unclosed\n") - return (f"REFUSED yaml-unparsable: {pin.resolve()}: not a flow " + # Named relative to the root, as the gate names every file (#712). + return ("REFUSED yaml-unparsable: contracts/spec-pin.yaml: not a flow " "collection: '[unclosed'") @@ -265,7 +266,8 @@ def test_validate_pins_prints_the_lockstep_remedy_as_it_always_did( """The assembly root's own gate is the one place the lockstep remedy is the right one, and it prints it under each refusal that names no other: its own (`pin-tag-only`) and one `repo_shape` raises for it - (`yaml-unparsable`), byte for byte what it printed before #370.""" + (`yaml-unparsable`), byte for byte what it printed before #370, under + a line that names the pin relative to the root since #712.""" line = _break_spec_pin(project, how) checked = run_script(project / "scripts" / "validate-pins.py", "--quiet", cwd=project) diff --git a/tests/test_validate_pins_named_and_spelled.py b/tests/test_validate_pins_named_and_spelled.py new file mode 100644 index 00000000..17540f6b --- /dev/null +++ b/tests/test_validate_pins_named_and_spelled.py @@ -0,0 +1,1052 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`validate-pins.py` refuses a pin, the manifest or a shape copy it cannot +reach or read BY NAME, names every file relative to the root and once, gives +each cause its own advice, and spells every name and path it prints (#712). + +The issue's lines, from lane 4's checklist triage on #459: + + * L4-103. A `contracts/` this user cannot search ended the run in a + `PermissionError` traceback, exit 1, at `_load_leg_pin`'s + `pin_path.is_file()` on Python 3.12 and 3.13, and on 3.14 refused + `pin-missing`, a present file said not to exist, under the lockstep + remedy. The five `is_file()` calls (the leg pin, the shape pin, each + shape copy, each neutral-product pin and the manifest) are `_is_file` + now: `_is_a`'s answer (#633), where a refused `stat` is a refusal + naming the file. A shape copy at mode 000 ended in `file_sha256`'s + `PermissionError` on every Python; it is `shape-copy-unreadable` too. + * L4-104. A pin this user cannot open was `REFUSED yaml-unreadable: + : [Errno 13] Permission denied: ''`, + under THE LOCKSTEP RULE's remedy. It is named relative to the root, + once, with a remedy of its own, and so is the file in every refusal + `load_yaml` raises here and every file this validator names. + * L4-105 (rider). A workflow file whose `stat` or read failed for another + cause than permission was told to restore read access to it. + * L4-106 and L4-144. A workflow's name, and a pin's path built from a + `neutral_product_pins:` entry, reached stderr raw: a newline began a + line of its own, a bidi control reordered one, a C1 control broke one, + an ESC was the control it is and a byte that is not UTF-8 was `\\udcXX`. + Every name and path the file prints is spelled by `_shown` now, with a + row below for each print. + * L4-143 (rider). The "not used" note for a `--pin-source` under a + `neutral_product_pins:` that is not a list points at that finding. + * L4-102 (rider). `PinSources` keys by `pin_product_key`, the pin file's + own fold, where `casefold()` took `openss=` for `openss`'s sharp twin. + +THE HARNESS. The gate runs as the copy a real scaffold wrote +(`tests/test_validate_pins_unreadable_workflow.py`'s `validate`), and +in-process as that copy loaded as a module (`validator`, from +`tests/test_validate_pins_without_git.py`). `refused_stat` +(`tests/test_bump_leg_unopenable.py`) stands in for a `stat` the operating +system refuses and for Python 3.14's `is_file()`, so those rows run on every +Python, as root and on Windows. The mode rows skip on Windows and as root, +and a row that needs a file whose name holds a control character skips on +Windows, whose file names cannot hold one. + +NOTHING HERE REACHES A NETWORK: the scaffold uses `--local-remote-dir`, and +each row that would ask `gh api` replaces `tree_digest_from_gh`. +""" + +from __future__ import annotations + +import errno +import os +import re +import stat +import sys +from pathlib import Path + +import pytest + +from conftest import REPO, clear_ambient_pin_sources, run_script +from test_bump_leg_unopenable import refused_stat +from test_load_yaml_refusals import refusal_for +from test_manifest_validator import edit +from test_validate_pins_unreadable_workflow import ( + CANNOT_RUN_AS_ROOT, WORKFLOWS, leg_workflow, not_utf8_line, validate, +) +from test_validate_pins_unreadable_workflows_dir import ( + FILE_ADVICE, mode_of, tree_bytes, +) +# A fixture is used by being named in this module's namespace, which is what +# this import is for: the project's own `validate-pins.py`, loaded as a module. +from test_validate_pins_without_git import validator # noqa: F401 + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import double_quoted # noqa: E402 + +#: What the refusal of a file the gate cannot reach or read says first. +READS = ( + "Remediation: this validator reads each leg's pin, the shape pin, each " + "neutral-product pin, project.yaml and every copy the shape pin lists, " + "and one it cannot read is a question it cannot answer, never a pass. ") +#: Its advice for a permission, in `bump-leg.py`'s words (#383). +PERMISSION = ( + "Make what this user may not open readable by this user: `chmod u+r` on " + "a file, `chmod u+rx` on a directory (and on every directory above it " + "inside the root), or fix who owns it. ") +#: Its advice, and a workflow's, for any other cause (#383's words too). +OTHER = ( + "Where the operating system gave another reason, put a regular file this " + "user can open back in its place, or take away one nobody wants, and make " + "sure nothing is changing the directory while this runs. ") +AGAIN = "Then run the same command again." + +#: Hex letters: an all-digit `commit:` reads back as a number. +COMMIT = "c" * 40 +OTHER_COMMIT = "d" * 40 +SHAPE_PIN = "contracts/shape-pin.yaml" + +#: A file whose name holds a control character, which Windows cannot hold. +ON_DISK = pytest.mark.skipif( + os.name == "nt", + reason="needs a file whose name holds a control character, which a " + "Windows file name cannot hold") + +#: The issue's names that print as something other than themselves: a +#: newline, a bidi control (U+202E), a C1 control (U+0085), an ESC and a +#: byte that is not UTF-8, as `surrogateescape` keeps one. +UNPRINTABLE = [ + pytest.param("a\nb", id="newline"), + pytest.param("rev‮elif", id="bidi-control"), + pytest.param("c1\u0085x", id="c1-control"), + pytest.param("x\x1b[31mRED", id="esc"), + pytest.param("und\udcffx", id="undecodable-byte"), +] +#: The control: a name that prints as itself, and still does. +PRINTABLE = "plain-name" + + +def refused(code: str, named: str, reason: str = "Permission denied", + advice: str = PERMISSION) -> str: + """The whole of stderr for a file the gate cannot reach or open: the + file named once, relative to the root, with the system's reason, then + its own remedy and no other.""" + return (f"REFUSED {code}: {named}: cannot be read ({reason})\n" + f"{READS}{advice}{AGAIN}\n") + + +def declare_openxdox(project: Path) -> None: + """`project.yaml` declares one neutral-product pin, whose file is not + there: the fourth `is_file()` site is reached.""" + edit(project, "neutral_product_pins: []", + "neutral_product_pins: [openXdox]") + + +def refusal_of_the_run(validator, project: Path, values=()): + """The `Refusal` the gate's checks raise over `project`, failing the test + if they finish or raise anything else. The class is the project's own + copy's, which is the one its `main()` catches (SonarCloud S5958).""" + with pytest.raises(validator.Refusal) as caught: + validator._run_all_checks(project, validator.Report(), list(values)) + return caught.value + + +def failing(monkeypatch, method: str, failures: dict) -> None: + """`Path.` raising `failures[name]` for each file so named, and + answering as it does for every other.""" + real = getattr(Path, method) + + def fails(self, *args, **kwargs): + if self.name in failures: + raise failures[self.name] + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, method, fails) + + +def os_error(code: int) -> OSError: + """The `OSError` the system raises for `code`, with its own words.""" + return OSError(code, os.strerror(code)) + + +# --- L4-103 and L4-104: what the gate prints, run as a project runs it ------- + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("relative, mode, code, named", [ + pytest.param("contracts", 0, "yaml-unreadable", "contracts/spec-pin.yaml", + id="contracts-000"), + pytest.param("contracts", stat.S_IRUSR, "yaml-unreadable", + "contracts/spec-pin.yaml", id="contracts-r--"), + pytest.param("contracts/spec-pin.yaml", 0, "yaml-unreadable", + "contracts/spec-pin.yaml", id="leg-pin-000"), + pytest.param(SHAPE_PIN, 0, "yaml-unreadable", SHAPE_PIN, + id="shape-pin-000"), + pytest.param("project.yaml", 0, "yaml-unreadable", "project.yaml", + id="manifest-000"), + pytest.param("AGENTS-shape.md", 0, "shape-copy-unreadable", + "AGENTS-shape.md", id="shape-copy-000"), +]) +def test_a_file_the_gate_cannot_reach_or_open_is_refused_by_name( + project, relative, mode, code, named): + """THE ISSUE'S ROWS. A `contracts/` at 000 or r-- was a `PermissionError` + traceback, exit 1, on 3.12 and 3.13, and `pin-missing` on 3.14; a pin + at 000 named its absolute path twice under THE LOCKSTEP RULE's remedy; + a shape copy at 000 was a traceback on every Python. Each is now one + refusal, exit 2, and the whole of stderr: the file relative to the root, + named once, the system's reason, the permission advice. Nothing is + written, and with the mode put back the same command passes.""" + before = tree_bytes(project) + with mode_of(project, relative, mode): + result = validate(project) + assert result.stderr == refused(code, named), result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert result.stdout == "", result.stdout + assert tree_bytes(project) == before, "a validator writes nothing" + again = validate(project) + assert again.returncode == 0, again.stdout + again.stderr + assert "pins ok" in again.stdout, again.stdout + + +# --- L4-103 on every Python: the five `is_file()` sites ---------------------- + +#: Each site, as `(the name whose stat is refused, code, the file named)`. +SITES = [ + pytest.param("spec-pin.yaml", "yaml-unreadable", "contracts/spec-pin.yaml", + id="leg-pin"), + pytest.param("shape-pin.yaml", "yaml-unreadable", SHAPE_PIN, + id="shape-pin"), + pytest.param("AGENTS-shape.md", "shape-copy-unreadable", + "AGENTS-shape.md", id="shape-copy"), + pytest.param("openxdox-pin.yaml", "yaml-unreadable", + "contracts/openxdox-pin.yaml", id="neutral-pin"), + pytest.param("project.yaml", "yaml-unreadable", "project.yaml", + id="manifest"), +] + + +@pytest.mark.parametrize("name, code, named", SITES) +def test_a_refused_stat_is_refused_by_name_on_every_python( + validator, project, monkeypatch, name, code, named): + """ON PYTHON 3.14 `is_file()` ANSWERS FALSE to a refused `stat`, so the + pin in a `contracts/` this user cannot search was "missing" and a shape + copy "pinned but absent". Asked of the system, the answer is the same on + every Python, root included: the file, refused by name.""" + declare_openxdox(project) + refused_stat(monkeypatch, name) + refusal = refusal_of_the_run(validator, project) + assert refusal.code == code + assert refusal.detail == f"{named}: cannot be read (Permission denied)" + assert refusal.remediation == READS + PERMISSION + AGAIN + + +@pytest.mark.parametrize("call, code", [ + pytest.param(lambda v, root: v._load_leg_pin( + root, "a\x00b", "spec", "o/r", v.Report()), "pin-missing", + id="leg-pin"), + pytest.param(lambda v, root: v._check_neutral_pin_entry_and_load( + root, "a\x00b", v.Report()), "neutral-pin-missing", id="neutral-pin"), +]) +def test_a_name_the_system_cannot_take_is_still_no_file(validator, project, + call, code): + """A NUL in a role or a product is a name no `stat` can be handed (a + `ValueError`), which `is_file()` answered False: still the missing + file's refusal, its name spelled, and never a traceback.""" + with pytest.raises(validator.Refusal) as caught: + call(validator, project) + assert caught.value.code == code + assert "\\x00" in caught.value.detail, caught.value.detail + + +# --- L4-104: every file named relative to the root --------------------------- + +def unlinked(relative: str): + def break_it(project: Path) -> None: + (project / relative).unlink() + return break_it + + +def written(relative: str, data: bytes): + def break_it(project: Path) -> None: + declare_openxdox(project) + (project / relative).write_bytes(data) + return break_it + + +@pytest.mark.parametrize("break_it, code, detail", [ + pytest.param(unlinked("project.yaml"), "manifest-missing", + "project.yaml does not exist, so there is no declaration of " + "which legs to check. A one-repository project has no legs " + "and does not run this validator.", id="manifest-missing"), + pytest.param(lambda project: (project / "project.yaml").write_bytes( + b"- a\n"), "manifest-unreadable", "project.yaml: not a mapping", + id="manifest-not-a-mapping"), + pytest.param(lambda project: (project / "project.yaml").write_bytes( + b"legs: []\n"), "manifest-no-legs", + "project.yaml: no non-assembly legs declared", id="manifest-no-legs"), + pytest.param(written("contracts/spec-pin.yaml", b"- a\n"), + "pin-unreadable", "contracts/spec-pin.yaml: not a mapping", + id="leg-pin-not-a-mapping"), + pytest.param(unlinked(SHAPE_PIN), "shape-pin-missing", + f"{SHAPE_PIN} does not exist", id="shape-pin-missing"), + pytest.param(written(SHAPE_PIN, b"- a\n"), "shape-pin-unreadable", + f"{SHAPE_PIN}: not a mapping", id="shape-pin-not-a-mapping"), + pytest.param(written("contracts/openxdox-pin.yaml", b"- a\n"), + "neutral-pin-unreadable", + "contracts/openxdox-pin.yaml: not a mapping", + id="neutral-pin-not-a-mapping"), +]) +def test_every_file_is_named_relative_to_the_root(validator, project, + break_it, code, detail): + """The coordinator's ruling on #712: one rule for every file this gate + names. `not a mapping`, a missing shape pin and the three manifest + refusals named the ABSOLUTE path, raw; each is the path relative to the + root, POSIX, as #633's `workflow-unreadable` lines are.""" + break_it(project) + refusal = refusal_of_the_run(validator, project) + assert (refusal.code, refusal.detail) == (code, detail) + assert str(project) not in str(refusal) + + +def test_a_pin_that_is_not_utf8_keeps_its_words_and_names_it_relative( + validator, project): + """`load_yaml`'s other refusals keep their id, their words and their + remedy, and name the file as this gate names every file (#712).""" + pin = project / "contracts" / "spec-pin.yaml" + data = pin.read_bytes() + b"# caf\xe9\n" + pin.write_bytes(data) + refusal = refusal_of_the_run(validator, project) + assert refusal.code == "yaml-unreadable" + assert refusal.detail == not_utf8_line("contracts/spec-pin.yaml", data) + assert refusal.remediation.startswith( + "Remediation: every YAML file this tool reads must be UTF-8. ") + + +def test_a_pin_that_does_not_parse_names_it_relative(validator, project): + (project / "contracts" / "spec-pin.yaml").write_bytes( + b"commit: [unclosed\n") + refusal = refusal_of_the_run(validator, project) + assert refusal.code == "yaml-unparsable" + assert refusal.detail.startswith("contracts/spec-pin.yaml: "), refusal + assert str(project) not in str(refusal) + + +def test_the_gate_names_the_pin_so_beside_an_older_library( + validator, project, monkeypatch): + """THE SKEW RULE (#370, #610): a root may keep a `repo_shape.py` older + than its `validate-pins.py`, whose `load_yaml` gave an `OSError` as + `: [Errno 13] Permission denied: ''`. The gate's line is read + off the `OSError` the refusal was raised from, which `load_yaml` has + always chained, and not off those words.""" + real = validator.load_yaml + + def older(path): + if Path(path).name != "spec-pin.yaml": + return real(path) + exc = PermissionError(errno.EACCES, "Permission denied", str(path)) + raise validator.Refusal("yaml-unreadable", f"{path}: {exc}") from exc + + monkeypatch.setattr(validator, "load_yaml", older) + refusal = refusal_of_the_run(validator, project) + assert refusal.detail == ( + "contracts/spec-pin.yaml: cannot be read (Permission denied)") + assert refusal.remediation == READS + PERMISSION + AGAIN + + +def test_load_yaml_names_a_file_it_cannot_open_once(tmp_path): + """The library's own line, for every tool that reads YAML: the path once + and the system's reason, where `str()` of the `OSError` named the path a + second time in Python's quoting (#712).""" + path = tmp_path / "absent.yaml" + refusal = refusal_for(path) + assert refusal.code == "yaml-unreadable" + assert refusal.detail == ( + f"{path}: cannot be read ({os.strerror(errno.ENOENT)})") + assert isinstance(refusal.__cause__, FileNotFoundError) + + +# --- L4-105: each cause its own advice --------------------------------------- + +CAUSES = [ + pytest.param("stat", errno.EIO, id="stat-io-error"), + pytest.param("stat", errno.ENAMETOOLONG, id="stat-name-too-long"), + pytest.param("read_text", errno.EIO, id="read-io-error"), +] + + +@pytest.mark.parametrize("method, code", CAUSES) +def test_a_workflow_that_fails_for_another_cause_is_not_told_to_chmod( + validator, project, monkeypatch, method, code): + """A workflow file whose `stat` or read fails with an I/O error or a name + too long is not put right by a mode: it gets `OTHER`, and not the + permission sentence it was given whatever the cause.""" + (project / WORKFLOWS / "legs.yml").write_bytes(leg_workflow(project)) + failing(monkeypatch, method, {"legs.yml": os_error(code)}) + with pytest.raises(validator.Refusal) as caught: + validator._workflow_refs(project) + refusal = caught.value + assert refusal.code == "workflow-unreadable" + assert refusal.detail == ( + f"{WORKFLOWS}/legs.yml: cannot be read ({os.strerror(code)})") + assert OTHER in refusal.remediation, refusal.remediation + assert FILE_ADVICE not in refusal.remediation, refusal.remediation + assert "chmod" not in refusal.remediation, refusal.remediation + + +def test_each_cause_in_one_refusal_gets_its_own_advice(validator, project, + monkeypatch): + """One refusal names every file (#359), so a permission and an I/O error + in one run are told both, the permission's first.""" + for name in ("legs.yml", "other.yml"): + (project / WORKFLOWS / name).write_bytes(b"on: [push]\n") + failing(monkeypatch, "stat", {"legs.yml": PermissionError( + errno.EACCES, "Permission denied")}) + failing(monkeypatch, "read_text", {"other.yml": os_error(errno.EIO)}) + with pytest.raises(validator.Refusal) as caught: + validator._workflow_refs(project) + refusal = caught.value + assert refusal.detail.splitlines() == [ + f"{WORKFLOWS}/legs.yml: cannot be read (Permission denied)", + f"{WORKFLOWS}/other.yml: cannot be read " + f"({os.strerror(errno.EIO)})"] + assert FILE_ADVICE in refusal.remediation, refusal.remediation + assert OTHER in refusal.remediation, refusal.remediation + assert (refusal.remediation.index(FILE_ADVICE) + < refusal.remediation.index(OTHER)), refusal.remediation + + +@pytest.mark.parametrize("name, code, named", SITES) +@pytest.mark.parametrize("failure", [errno.EIO, errno.ENAMETOOLONG], + ids=lambda code: errno.errorcode[code]) +def test_a_file_whose_stat_fails_for_another_cause_is_told_so( + validator, project, monkeypatch, name, code, named, failure): + """The same rule at the five sites: the system's own reason, and + `OTHER`, which never says `chmod`.""" + declare_openxdox(project) + failing(monkeypatch, "stat", {name: os_error(failure)}) + refusal = refusal_of_the_run(validator, project) + assert refusal.code == code + assert refusal.detail == ( + f"{named}: cannot be read ({os.strerror(failure)})") + assert refusal.remediation == READS + OTHER + AGAIN + + +def read_refused(validator, monkeypatch, name: str, error: OSError) -> None: + """Reading `name` fails with `error`: `load_yaml`'s `read_text` for a + YAML file, `file_sha256` for a shape copy.""" + if name.endswith(".yaml"): + failing(monkeypatch, "read_text", {name: error}) + return + real = validator.file_sha256 + + def sha256(path): + if Path(path).name == name: + raise error + return real(path) + + monkeypatch.setattr(validator, "file_sha256", sha256) + + +@pytest.mark.parametrize("name, code, named", [ + row for row in SITES if row.id != "neutral-pin"]) +@pytest.mark.parametrize("failure, reason, advice", [ + pytest.param(PermissionError(errno.EACCES, "Permission denied"), + "Permission denied", PERMISSION, id="permission"), + pytest.param(os_error(errno.EIO), os.strerror(errno.EIO), OTHER, + id="io-error"), +]) +def test_a_file_that_cannot_be_opened_is_refused_by_name( + validator, project, monkeypatch, name, code, named, failure, reason, + advice): + """THE OPEN, AFTER THE `stat`: a pin or the manifest at mode 000 was + `load_yaml`'s line under the lockstep remedy, and a shape copy at 000 + `file_sha256`'s traceback. The file is named once, relative to the + root, with its cause's advice. (A neutral-product pin is opened by the + same `_load` as the others; this root declares none to open.)""" + read_refused(validator, monkeypatch, name, failure) + refusal = refusal_of_the_run(validator, project) + assert refusal.code == code + assert refusal.detail == f"{named}: cannot be read ({reason})" + assert refusal.remediation == READS + advice + AGAIN + + +# --- L4-106, L4-144 and every other print: each name and path spelled -------- + +def refusal_text(validator, call, *args) -> str: + """`str()` of the `Refusal` `call(*args)` raises: what `main` prints.""" + with pytest.raises(validator.Refusal) as caught: + call(*args) + return str(caught.value) + + +def report_text(report) -> str: + """Every finding and note a check added, as the gate prints them.""" + return "\n".join(report.findings + report.notes) + + +def no_git(*_args, **_kwargs): + """A git that is not there, as `subprocess` names it.""" + raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), "git") + + +def gitlink(validator, monkeypatch, answer) -> None: + """`recorded_gitlink` answering `answer`, or raising it.""" + def recorded(root, path): + if isinstance(answer, BaseException): + raise answer + return answer + monkeypatch.setattr(validator, "recorded_gitlink", recorded) + + +def initialized(monkeypatch, value: str) -> None: + """The leg at `value` is a checkout: its `.git` is there.""" + real = Path.exists + + def exists(self, *args, **kwargs): + if self.name == ".git" and self.parent.name == value: + return True + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "exists", exists) + + +def valid_pin(validator) -> dict: + """A pin whose digest fields are well formed.""" + return {"revision_kind": "commit", "commit": COMMIT, + "digests": {"tree_sha256": "a" * 64}, + "digest_definition": validator.TREE_DIGEST_DEFINITION, + "source_repository": "o/r"} + + +def written_pin(root: Path, name: str, body: bytes) -> None: + (root / "contracts" / f"{name}-pin.yaml").write_bytes(body) + + +def leg_pin_text(v, root, value, body: bytes | None = None) -> str: + """What `_load_leg_pin` raises for the leg `value`, its pin `body`.""" + if body is not None: + written_pin(root, value, body) + return refusal_text(v, v._load_leg_pin, root, value, "spec", "o/r", + v.Report()) + + +def leg_pin_path(value: str) -> str: + return f"contracts/{value}-pin.yaml" + + +# Each print site: `(validator, root, monkeypatch, value) -> (printed, the +# values in it that must be spelled, one entry per time each is printed)`. + +def pin_missing_path(v, root, mp, value): + return leg_pin_text(v, root, value), [leg_pin_path(value)] + + +def pin_missing_repository(v, root, mp, value): + return refusal_text(v, v._load_leg_pin, root, "nosuch", "spec", value, + v.Report()), [value] + + +def pin_cannot_be_read(v, root, mp, value): + refused_stat(mp, f"{value}-pin.yaml") + return leg_pin_text(v, root, value), [leg_pin_path(value)] + + +def pin_not_a_mapping(v, root, mp, value): + return leg_pin_text(v, root, value, b"- a\n"), [leg_pin_path(value)] + + +def pin_not_utf8(v, root, mp, value): + return leg_pin_text(v, root, value, b"a: caf\xe9\n"), [leg_pin_path(value)] + + +def pin_unparsable(v, root, mp, value): + return (leg_pin_text(v, root, value, b"commit: [unclosed\n"), + [leg_pin_path(value)]) + + +def pin_commit_refused(v, root, mp, value): + return (leg_pin_text(v, root, value, b"kind: pinned_contract_manifest\n" + b"revision_kind: commit\ncommit: abc\n"), + [leg_pin_path(value)]) + + +def pin_findings(v, root, mp, value): + """pin-wrong-kind, pin-tag-only, pin-path-mismatch and + pin-repository-mismatch each name the pin; the path line names the + leg's role too.""" + written_pin(root, value, ( + f"kind: other\nrevision_kind: tag\ncommit: {COMMIT}\n" + "submodule_path: elsewhere\nsource_repository: o/other\n").encode()) + report = v.Report() + v._load_leg_pin(root, value, "spec", "o/r", report) + assert len(report.findings) == 4, report.findings + return report_text(report), [leg_pin_path(value)] * 4 + [value] + + +def gitlink_absent(v, root, mp, value): + gitlink(v, mp, None) + return refusal_text(v, v._check_gitlink_matches_pin, root, value, "spec", + COMMIT, "rel", v.Report()), [value] + + +def gitlink_unread(v, root, mp, value): + gitlink(v, mp, FileNotFoundError(errno.ENOENT, "No such file", "git")) + return refusal_text(v, v._check_gitlink_matches_pin, root, value, "spec", + COMMIT, "rel", v.Report()), [value] + + +def gitlink_mismatch(v, root, mp, value): + gitlink(v, mp, OTHER_COMMIT) + report = v.Report() + v._check_gitlink_matches_pin(root, "spec", value, COMMIT, "rel", report) + return report_text(report), [value] + + +def gitlink_note(v, root, mp, value): + gitlink(v, mp, COMMIT) + report = v.Report() + v._check_gitlink_matches_pin(root, "spec", value, COMMIT, "rel", report) + return report_text(report), [value] + + +def digest_text(v, root, value, pin=None) -> str: + return refusal_text(v, v._check_pin_digest, root, value, COMMIT, + pin or {}, "rel", v.Report()) + + +def digest_uninitialized(v, root, mp, value): + return digest_text(v, root, value), [value] + + +def digest_lookup_unrun(v, root, mp, value): + initialized(mp, value) + mp.setattr(v, "git_out", no_git) + return digest_text(v, root, value), [value] + + +def digest_commit_unresolvable(v, root, mp, value): + initialized(mp, value) + + def refused_by_git(*_args, **_kwargs): + raise v.Refusal("git-failed", "it said no") + + mp.setattr(v, "git_out", refused_by_git) + return digest_text(v, root, value), [value] + + +def digest_recompute_unrun(v, root, mp, value): + initialized(mp, value) + mp.setattr(v, "git_out", lambda *_args, **_kwargs: "") + mp.setattr(v, "tree_digest", no_git) + return digest_text(v, root, value, valid_pin(v)), [value] + + +def digest_note(v, root, mp, value): + initialized(mp, value) + mp.setattr(v, "git_out", lambda *_args, **_kwargs: "") + mp.setattr(v, "tree_digest", lambda *_args, **_kwargs: "a" * 64) + report = v.Report() + v._check_pin_digest(root, value, COMMIT, valid_pin(v), "rel", report) + return report_text(report), [value] + + +def workflow_reference(v, root, mp, value): + report = v.Report() + v._check_workflow_refs_for_leg( + [(Path(WORKFLOWS) / f"{value}.yml", "o/r", OTHER_COMMIT)], "o/r", + "spec", COMMIT, report, "spec") + return report_text(report), [f"{value}.yml"] + + +def workflow_named(root: Path, value: str, data: bytes) -> str: + rel = f"{WORKFLOWS}/{value}.yml" + (root / rel).write_bytes(data) + return rel + + +def workflow_not_utf8(v, root, mp, value): + rel = workflow_named(root, value, b"# caf\xe9\n") + return refusal_text(v, v._workflow_refs, root), [rel] + + +def workflow_unopened(v, root, mp, value): + rel = workflow_named(root, value, b"on: [push]\n") + failing(mp, "read_text", {f"{value}.yml": PermissionError( + errno.EACCES, "Permission denied")}) + return refusal_text(v, v._workflow_refs, root), [rel] + + +def workflow_stat_refused(v, root, mp, value): + rel = workflow_named(root, value, b"on: [push]\n") + refused_stat(mp, f"{value}.yml") + return refusal_text(v, v._workflow_refs, root), [rel] + + +def bump_root(v, root, mp, value): + where = root / value + return v._bump_command(where, "spec"), [str(where)] + + +def bump_role(v, root, mp, value): + return v._bump_command(root, value), [value] + + +def put_back(v, root, mp, value): + """It names the leg, and then in `git submodule update -- `.""" + return v._put_back([value]), [value, value] + + +def held_while_digest_stands(v, root, mp, value): + return v._held_then("the bump-leg.py line", "that leg", + v.Back(value, OTHER_COMMIT, True), None), [value] + + +def held_move_kept(v, root, mp, value): + return v._held_then("the bump-leg.py line", "x", + v.Back(value, OTHER_COMMIT, False), + {"code": ["pin-digest-mismatch"]}), [value, value] + + +def held_two_off(v, root, mp, value): + return v._gitlink_repairs_held_back([value, "code"], []), [value, value] + + +def held_other_leg(v, root, mp, value): + return v._gitlink_repairs_held_back( + ["spec"], [], "the bump-leg.py line", + v.Back("spec", OTHER_COMMIT, False), + {value: ["pin-digest-mismatch"]}), [value, value] + + +def held_back_off(v, root, mp, value): + return v._gitlink_repairs_held_back( + ["code"], ["pin-wrong-kind"], "the bump-leg.py line", + v.Back(value, OTHER_COMMIT, False)), [value] * 3 + + +def held_for_whom(v, root, mp, value): + return v._gitlink_repairs_held_back( + [value], [], "the bump-leg.py line", + v.Back("spec", OTHER_COMMIT, False)), [value] + + +def curable_under_gitlink(v, root, mp, value): + one = v.Curable(0, "spec", value, COMMIT, None) + return v._curable_ending(root, one, ["spec"], [], { + "spec": v.Back(value, OTHER_COMMIT, False)}, "spec"), [value] + + +def shape_commits(v, root, mp, value): + """shape-pin-manifest-disagree names the pin's commit and the + manifest's, neither of which is asked to be 40 hex first. The pin's is + named twice: `shape-pin-commit` has always `repr`'d it.""" + pin = root / SHAPE_PIN + text = pin.read_bytes().decode("utf-8") + line = "commit: " + double_quoted(value) + pin.write_bytes(re.sub(r"^commit: .*$", lambda _match: line, text, + count=1, flags=re.M).encode("utf-8")) + report = v.Report() + v._check_shape_pin_header(root, {"shape": {"commit": value + "x"}}, report) + return report_text(report), [value, value, value + "x"] + + +def copy_missing(v, root, mp, value): + report = v.Report() + v._check_shape_copy(root, {"path": value, "sha256": "0" * 64}, SHAPE_PIN, + report) + return report_text(report), [value] + + +def copy_drift(v, root, mp, value): + """The copy's path and the digest its row records.""" + (root / value).write_bytes(b"x\n") + report = v.Report() + v._check_shape_copy(root, {"path": value, "sha256": value}, SHAPE_PIN, + report) + return report_text(report), [value, value] + + +def copy_unreadable(v, root, mp, value): + refused_stat(mp, value) + return refusal_text(v, v._check_shape_copy, root, + {"path": value, "sha256": "0" * 64}, SHAPE_PIN, + v.Report()), [value] + + +def source_no_path(v, root, mp, value): + return refusal_text(v, v.PinSources, [f"{value}="], [value]), [ + f"{value}=", value] + + +def source_read_as_path(v, root, mp, value): + return v.PinSources(["k=x"], [value]).read_as_path("k=x"), [value] + + +def recompute(v, root, mp, product, repository, sources, shared=frozenset()): + """`_recompute_neutral_pin_digest`'s report, with no ambient variable + and no `gh` to answer.""" + clear_ambient_pin_sources(mp) + + def no_gh(*_args, **_kwargs): + raise v.Refusal("gh-not-found", "no gh here") + + mp.setattr(v, "tree_digest_from_gh", no_gh) + report = v.Report() + v._recompute_neutral_pin_digest(root, product, repository, COMMIT, "rel", + sources, report, shared) + return report_text(report) + + +def keyed_source(v, root, mp, value): + """The keyed source's `how` names the product.""" + sources = v.PinSources([f"{value}={root / 'nowhere'}"], [value]) + return recompute(v, root, mp, value, "o/r", sources), [value] + + +def source_repository(v, root, mp, value): + """The finding, and its remedy, name the pin's `source_repository`.""" + sources = v.PinSources([f"openX={root / 'nowhere'}"], ["openX"]) + return recompute(v, root, mp, "openX", value, sources), [value, value] + + +def sibling_source(v, root, mp, value): + """A checkout beside the root, named for the repository: the source and + its `how` name it.""" + sibling = root.parent / value + (sibling / ".git").mkdir(parents=True) + text = recompute(v, root, mp, "openX", f"o/{value}", + v.PinSources([], ["openX"])) + return text, [str(sibling), str(sibling), f"o/{value}", f"o/{value}"] + + +def skip_line(v, root, mp, value): + return recompute(v, root, mp, value, "o/r", + v.PinSources([], [value])), [value] + + +def skip_line_shared(v, root, mp, value): + shared = frozenset({v.pin_product_key(value)}) + return recompute(v, root, mp, value, "o/r", v.PinSources([], [value]), + shared), [value, value] + + +def skip_line_beside(v, root, mp, value): + return recompute(v, root, mp, "openX", f"o/{value}", + v.PinSources([], ["openX"])), [f"../{value}"] + + +def neutral_pin_missing(v, root, mp, value): + """L4-144: the pin's path, built from the entry, and the remedy's + `--pin`; the entry itself was already `repr`'d.""" + text = refusal_text(v, v._check_neutral_pin_entry_and_load, root, value, + v.Report()) + return text, [f"contracts/{value.lower()}-pin.yaml", value, value] + + +def neutral_pin_not_a_mapping(v, root, mp, value): + (root / "contracts" / f"{value.lower()}-pin.yaml").write_bytes(b"- a\n") + text = refusal_text(v, v._check_neutral_pin_entry_and_load, root, value, + v.Report()) + return text, [f"contracts/{value.lower()}-pin.yaml"] + + +def neutral_note(v, root, mp, value): + mp.setattr(v, "_check_neutral_pin_entry_and_load", + lambda *_args: (valid_pin(v), "rel")) + mp.setattr(v, "_recompute_neutral_pin_digest", + lambda *_args, **_kwargs: ("a" * 64, "how")) + report = v.Report() + v._check_one_neutral_product_pin(root, value, report, + v.PinSources([], [value])) + return report_text(report), [value] + + +def unused_keyed(v, root, mp, value): + report = v.Report() + v._note_unused_pin_sources(v.PinSources([f"{value}=x"], [value]), report) + return report_text(report), [f"{value}=x", value] + + +PRINTS = [ + pytest.param(pin_missing_path, id="pin-missing-path"), + pytest.param(pin_missing_repository, id="pin-missing-repository"), + pytest.param(pin_cannot_be_read, id="pin-cannot-be-read"), + pytest.param(pin_not_a_mapping, id="pin-not-a-mapping", marks=ON_DISK), + pytest.param(pin_not_utf8, id="pin-not-utf-8", marks=ON_DISK), + pytest.param(pin_unparsable, id="pin-unparsable", marks=ON_DISK), + pytest.param(pin_commit_refused, id="pin-commit-not-40-hex", + marks=ON_DISK), + pytest.param(pin_findings, id="pin-findings", marks=ON_DISK), + pytest.param(gitlink_absent, id="gitlink-absent"), + pytest.param(gitlink_unread, id="gitlink-git-not-runnable"), + pytest.param(gitlink_mismatch, id="gitlink-mismatch"), + pytest.param(gitlink_note, id="gitlink-note"), + pytest.param(digest_uninitialized, id="digest-uninitialized"), + pytest.param(digest_lookup_unrun, id="digest-lookup-git-not-runnable"), + pytest.param(digest_commit_unresolvable, id="digest-commit-unresolvable"), + pytest.param(digest_recompute_unrun, id="digest-git-not-runnable"), + pytest.param(digest_note, id="digest-note"), + pytest.param(workflow_reference, id="workflow-reference-mismatch"), + pytest.param(workflow_not_utf8, id="workflow-not-utf-8", marks=ON_DISK), + pytest.param(workflow_unopened, id="workflow-unopened", marks=ON_DISK), + pytest.param(workflow_stat_refused, id="workflow-stat-refused", + marks=ON_DISK), + pytest.param(bump_root, id="bump-line-root"), + pytest.param(bump_role, id="bump-line-leg"), + pytest.param(put_back, id="put-back"), + pytest.param(held_while_digest_stands, id="held-while-digest-stands"), + pytest.param(held_move_kept, id="held-move-kept"), + pytest.param(held_two_off, id="held-two-legs-off"), + pytest.param(held_other_leg, id="held-other-leg-out-of-step"), + pytest.param(held_back_off, id="held-back-off-its-pin"), + pytest.param(held_for_whom, id="held-names-its-leg"), + pytest.param(curable_under_gitlink, id="curable-under-gitlink"), + pytest.param(shape_commits, id="shape-pin-commits"), + pytest.param(copy_missing, id="shape-copy-missing"), + pytest.param(copy_drift, id="shape-copy-drift", marks=ON_DISK), + pytest.param(copy_unreadable, id="shape-copy-unreadable"), + pytest.param(source_no_path, id="pin-source-no-path"), + pytest.param(source_read_as_path, id="pin-source-read-as-path"), + pytest.param(keyed_source, id="pin-source-keyed"), + pytest.param(source_repository, id="pin-source-repository"), + pytest.param(sibling_source, id="pin-source-sibling", marks=ON_DISK), + pytest.param(skip_line, id="skip-product"), + pytest.param(skip_line_shared, id="skip-shared-variable"), + pytest.param(skip_line_beside, id="skip-checkout-beside"), + pytest.param(neutral_pin_missing, id="neutral-pin-missing"), + pytest.param(neutral_pin_not_a_mapping, id="neutral-pin-not-a-mapping", + marks=ON_DISK), + pytest.param(neutral_note, id="neutral-pin-note"), + pytest.param(unused_keyed, id="pin-source-not-used"), +] + + +@pytest.mark.parametrize("value", UNPRINTABLE) +@pytest.mark.parametrize("site", PRINTS) +def test_every_name_and_path_it_prints_is_spelled(validator, project, + monkeypatch, site, value): + """ONE ROW FOR EACH PRINT (#712). Every name and path that comes from the + disk, the manifest, a pin or the command line is printed as `_shown` + spells it: `repr` where a character in it is not printable, so a newline + does not begin a line, a bidi control does not reorder the line it is + on, a C1 control does not break it and an ESC is not the control it is. + Each value is printed spelled, as many times as it is printed, and + never raw.""" + printed, values = site(validator, project, monkeypatch, value) + for one in values: + assert one not in printed, (one, printed) + assert printed.count(repr(one)) >= values.count(one), (one, printed) + + +@pytest.mark.parametrize("site", PRINTS) +def test_a_printable_name_is_printed_as_it_always_was(validator, project, + monkeypatch, site): + """The control: `_shown` leaves a printable name the bytes it always + was. Each value is printed as many times, and at least one of those + prints is bare: only a print that `repr`'d the value before #712 (an + entry, a commit) still quotes it.""" + printed, values = site(validator, project, monkeypatch, PRINTABLE) + for one in values: + assert printed.count(one) >= values.count(one), (one, printed) + assert printed.count(repr(one)) < values.count(one), (one, printed) + + +# --- the issue's own inputs, as a project runs the gate ---------------------- + +def test_the_issue_s_pin_entry_reaches_stderr_spelled(project): + """L4-144'S MEASUREMENT: `neutral_product_pins: ["[31mRED"]` is a + list, so the entry is one pin. Its path and the remedy's `--pin` carried + the ESC (octal 033) to stderr; the entry itself was already `repr`'d.""" + edit(project, "neutral_product_pins: []", + "neutral_product_pins: [" + double_quoted("\x1b[31mRED") + "]") + result = validate(project) + assert result.returncode == 2, result.stdout + result.stderr + assert "\x1b" not in result.stderr, result.stderr + assert result.stderr == ( + "REFUSED neutral-pin-missing: project.yaml declares a pin on " + "'\\x1b[31mRED' but 'contracts/\\x1b[31mred-pin.yaml' does not exist\n" + "Remediation: `scaffold-project.py --pin '\\x1b[31mRED'@` " + "writes that file. A declaration in project.yaml with no pin file " + "beside it is a claim with no referent — either restore the file " + "or remove the declaration.\n"), result.stderr + + +@ON_DISK +def test_the_issue_s_workflow_names_are_one_line_each(project): + """L4-106'S MEASUREMENT: a `.yml` named with a newline printed as two + lines, so `N workflow file(s)` was followed by N+1. Each name holding a + newline, a bidi control, a C1 control, an ESC or a byte that is not + UTF-8 is now one line, spelled, so the refusal is N lines and the + remedy. The files are not UTF-8, so no mode is needed and root sees the + same refusal.""" + data = b"# caf\xe9\n" + names = sorted(f"{param.values[0]}.yml" for param in UNPRINTABLE) + for name in names: + (project / WORKFLOWS / name).write_bytes(data) + result = validate(project) + assert result.returncode == 2, result.stdout + result.stderr + lines = result.stderr.splitlines() + named = [not_utf8_line(repr(f"{WORKFLOWS}/{name}"), data) + for name in names] + assert lines[:len(names)] == [ + "REFUSED workflow-unreadable: " + named[0], *named[1:]], lines + assert len(lines) == len(names) + 1, lines + assert lines[-1].startswith( + f"Remediation: {len(names)} workflow file(s) under "), lines[-1] + + +# --- L4-143: the note points at the finding ---------------------------------- + +@pytest.mark.parametrize("spelled, why, code", [ + pytest.param("openXdox", "project.yaml's neutral_product_pins is not a " + "list, so it declares no neutral-product pin (see the " + "manifest-neutral-product-pins finding)", 1, + id="not-a-list"), + pytest.param("[]", "project.yaml declares no neutral-product pin", 0, + id="an-empty-list"), +]) +def test_an_unused_pin_source_says_why_nothing_is_declared(project, spelled, + why, code): + """L4-143'S MEASUREMENT: under a scalar `neutral_product_pins:`, which + declares no pin and is `manifest-neutral-product-pins`'s finding, the + note said only that nothing is declared. It points at the finding; an + empty list, which is no finding, says what it always said.""" + edit(project, "neutral_product_pins: []", + f"neutral_product_pins: {spelled}") + result = run_script(project / "scripts" / "validate-pins.py", + "--pin-source", "openXdox=../nowhere", cwd=project) + assert result.returncode == code, result.stdout + result.stderr + assert (f" note --pin-source openXdox=../nowhere: not used — {why}." + in result.stdout.splitlines()), result.stdout + assert ("manifest-neutral-product-pins" in result.stderr) == bool(code) + + +# --- L4-102: a `--pin-source` key is the pin file's fold --------------------- + +SHARP, SS = "openß", "openss" + + +def test_a_key_another_pin_file_s_name_folds_to_is_no_key(validator, project, + monkeypatch): + """`openss=` is not `openß`'s key: the two are two pin files, and + `casefold()` took one for the other. It is read as a bare PATH, and the + note's sentence says why.""" + clear_ambient_pin_sources(monkeypatch) + sources = validator.PinSources([f"{SS}=/x"], [SHARP]) + assert sources.reading == {f"{SS}=/x": None} + assert sources.keyed == {} + assert sources.read_as_path(f"{SS}=/x") == ( + f"It was read as a PATH: `{SS}` is not a declared neutral-product pin " + f"(declared: {SHARP}).") + _, how, _ = validator.resolve_neutral_pin_source( + project, SHARP, f"o/{SHARP}", sources) + assert how == "--pin-source", "the bare reading, not the keyed one" + + +def test_case_still_folds_as_the_pin_file_folds_it(validator): + """`lower()` is the pin file's fold: `OPENß=` is `openß`'s key.""" + sources = validator.PinSources(["OPENß=/x"], [SHARP]) + assert sources.reading == {"OPENß=/x": SHARP} + + +def test_two_pin_files_one_casefold_each_keep_their_own_source( + validator, project, monkeypatch): + """Both declared: each keyed value answers for its own pin, and neither + replaces the other.""" + clear_ambient_pin_sources(monkeypatch) + sources = validator.PinSources([f"{SS}=/a", f"{SHARP}=/b"], [SHARP, SS]) + assert sources.replaced == {} + for product, path in ((SS, "/a"), (SHARP, "/b")): + found, how, typed = validator.resolve_neutral_pin_source( + project, product, f"o/{product}", sources) + assert (found, how, typed) == ( + Path(path), f"--pin-source {product}=...", f"{product}={path}") diff --git a/tests/test_validate_pins_unreadable_workflow.py b/tests/test_validate_pins_unreadable_workflow.py index ffd66c78..b74e5053 100644 --- a/tests/test_validate_pins_unreadable_workflow.py +++ b/tests/test_validate_pins_unreadable_workflow.py @@ -202,9 +202,9 @@ def test_the_doctors_pins_row_names_the_file(project): # --- controls: what must not change ----------------------------------------- def test_a_pin_that_is_not_utf8_is_still_yaml_unreadable(project): - """Row (d): the refusal the workflow's one is modelled on, unchanged. Its - path is the absolute one `load_yaml` has always printed; the workflow - refusal is relative to the root, as the issue asks.""" + """Row (d): the refusal the workflow's one is modelled on, its id and + its words unchanged. Its path is relative to the root since #712, as + the workflow refusal's is.""" pin = project / "contracts" / "spec-pin.yaml" commit_bytes(project, "contracts/spec-pin.yaml", pin.read_bytes() + b"# caf\xe9\n", "latin-1 in the pin") From ebb1ee0918a5eb70fa990d3652df4778022128da Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 16:26:42 +0000 Subject: [PATCH 2/3] validate-pins.py: a leg checkout this user cannot search is `pin-submodule-unreadable` and a checkout beside the root that it cannot search is a source that cannot answer, so neither ends in a PermissionError, and the test's bidi control is an escape (#712) Fix round 1 for #827, on lane 3's review RV827 (#827 issuecomment-6236474296), with the coordinator's rulings. M1. tests/test_validate_pins_named_and_spelled.py held U+202E written raw in its bidi-control row, which SonarCloud refuses (text:S6389) and the quality gate failed on. It is the escape `\u202e`, as the row's neighbours are; the value tested is the same, and no file in the tree holds a literal bidi control. S1. Two more `exists()` calls asked a `stat` the operating system can refuse, and neighbour the issue's `contracts/` case. `_check_pin_digest`'s `(submodule / ".git").exists()` raised `PermissionError` out of the run, exit 1, with a leg's checkout at 000 or r-- on 3.11 to 3.13, and from 3.14 would have called the leg an uninitialized submodule. It is `pin-submodule-unreadable`, beside `pin-submodule-uninitialized`, naming the `.git` it could not reach, with the cause's advice: `spec/.git: cannot be read (Permission denied)`. `resolve_neutral_pin_source`'s `(sibling / ".git").exists()` raised it too for a `../` at 000; something is there, so it is the source, and `_digest_from_source` names why it cannot answer: the pin's `neutral-pin-source-unreadable` finding, `... could not answer for testorg/openXdox @ : Permission denied`. Both ask `_is_there`, `_is_a` with any kind, where a NUL in a name is nothing there, as `exists()` answered. S2. `_named`'s fallback for a path outside the root (a role or a product spelled as an absolute path, which `relative_to` refused with a `ValueError`) had no test. Two rows refuse such a role and such a product by name, `pin-missing` and `neutral-pin-missing`, with no `ValueError`. N1. "Then run the same command again." is `RUN_AGAIN`, said once (SonarCloud S1192), and `_unreadable_file` takes what the validator reads as `reads`, `FILES_READ` by default, so the checkout's refusal says what it reads in the same sentence. N2. `refusal_of_the_run` builds its `Report` and its values before the `pytest.raises` block (SonarCloud S5778). The digest rows' stand-in for an initialized checkout answers the `stat` that `_is_there` asks, and each digest row asserts the refusal it is about, so none passes on another refusal that happens to spell the same path. S3 and N3 are recorded in the pull request's "What it cannot tell", for #460's and #459's checklists; N4 matches `bump-leg.py` and stands. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- .../assembly-root/scripts/validate-pins.py | 69 ++++++-- tests/test_validate_pins_named_and_spelled.py | 151 ++++++++++++++++-- 2 files changed, 195 insertions(+), 25 deletions(-) diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index 1baefc1b..0da907a5 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -242,7 +242,7 @@ def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: "workflow it skipped could hide a `@` reference that is out of " "lockstep, which is what it exists to find, and a lossy read would " "call green a tree that the project's other readers refuse. " - + advice + "Then run the same command again.") + + advice + RUN_AGAIN) #: The `OSError`s that `Path.is_file()` and `Path.is_dir()` answered False to @@ -309,8 +309,17 @@ def _cannot_read(named: str, exc: OSError) -> str: "user can open back in its place, or take away one nobody wants, and make " "sure nothing is changing the directory while this runs. ") +#: The sentence every refusal of a file or directory this validator cannot +#: read ends on, said once (#712). +RUN_AGAIN = "Then run the same command again." -def _unreadable_file(code: str, named: str, exc: OSError) -> Refusal: +#: What this validator reads, as `_unreadable_file`'s remedy says it. +FILES_READ = ("each leg's pin, the shape pin, each neutral-product pin, " + "project.yaml and every copy the shape pin lists") + + +def _unreadable_file(code: str, named: str, exc: OSError, + reads: str = FILES_READ) -> Refusal: """The refusal for a pin, the manifest or a pinned shape copy that this validator must read and the operating system will not let it (#712): its `stat` refused (a `contracts/` this user cannot search) or its open @@ -325,7 +334,8 @@ def _unreadable_file(code: str, named: str, exc: OSError) -> Refusal: THE ADVICE FOLLOWS THE CAUSE, in `bump-leg.py`'s words for the same file (#383): a permission (`PermissionError`, which is `EACCES` and `EPERM`) is put right by the mode or the owner, and any other cause is - `OTHER_REASON`, which is not told to `chmod`. + `OTHER_REASON`, which is not told to `chmod`. `reads` names what the + validator reads, a leg's checkout where that is what it cannot reach. """ advice = ( "Make what this user may not open readable by this user: `chmod u+r` " @@ -334,11 +344,8 @@ def _unreadable_file(code: str, named: str, exc: OSError) -> Refusal: if isinstance(exc, PermissionError) else OTHER_REASON) return Refusal( code, _cannot_read(named, exc), - "Remediation: this validator reads each leg's pin, the shape pin, " - "each neutral-product pin, project.yaml and every copy the shape pin " - "lists, and one it cannot read is a question it cannot answer, never " - "a pass. " - + advice + "Then run the same command again.") + f"Remediation: this validator reads {reads}, and one it cannot read " + "is a question it cannot answer, never a pass. " + advice + RUN_AGAIN) def _is_file(root: Path, path: Path, code: str = "yaml-unreadable", @@ -363,6 +370,23 @@ def _is_file(root: Path, path: Path, code: str = "yaml-unreadable", return False +def _is_there(path: Path) -> bool: + """Whether anything is at `path` (a checkout's `.git`, a file or a + directory), asked as `_is_a` asks it, raising the `OSError` of a `stat` + the operating system refused (#712, after RV827's S1). + + NOT `Path.exists()`, which raised a refused `stat` out of the run up to + Python 3.13, so a leg checkout or a checkout beside the root that this + user cannot search ended in a `PermissionError` traceback, and which + answers False to it from 3.14, so the leg read as an uninitialized + submodule. A name the system cannot take at all is nothing there, as + `exists()` answered it.""" + try: + return _is_a(path, lambda _mode: True) + except ValueError: + return False + + def _load(root: Path, path: Path): """`load_yaml(path)`, with every refusal it raises naming the file as `_named` does (#712). @@ -422,8 +446,7 @@ def _workflows_unlistable(root: Path, workflows: Path, "neither skips one nor reads one lossily: a workflow it skipped " "could hide a `@` reference that is out of lockstep, which is " "what it exists to find, and a directory it cannot list is one whose " - "workflows it cannot read. " + advice - + "Then run the same command again.") + "workflows it cannot read. " + advice + RUN_AGAIN) def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: @@ -981,10 +1004,22 @@ def _check_pin_digest(root: Path, path: str, commit: str, pin: dict, rel: str, agree on a commit may still disagree on that commit's bytes. A mismatch is one `bump-leg.py` clears, since it recomputes the digest (#478). The leg's path is spelled in every line (#712). + + A CHECKOUT THIS USER CANNOT SEARCH IS REFUSED BY NAME (#712, after + RV827's S1), `pin-submodule-unreadable`, naming its `.git`: the digest + cannot be recomputed, and the leg is not uninitialized, which is what + Python 3.14's `exists()` would have said of it. """ submodule = root / path named = _shown(path) - if not (submodule / ".git").exists(): + try: + initialized = _is_there(submodule / ".git") + except OSError as exc: + raise _unreadable_file( + "pin-submodule-unreadable", _named(root, submodule / ".git"), exc, + "each leg's checkout, to recompute its pinned tree digest from " + "the leg's own objects") from exc + if not initialized: raise Refusal( "pin-submodule-uninitialized", f"{named} is not an initialized submodule checkout, so the pinned " @@ -1317,6 +1352,12 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, `(None, None, None)` when nothing above answers; the caller's next step is `gh api`, and after that, a SKIP. `how` is printed, so the product and the sibling's path in it are spelled (#712). + + A CHECKOUT BESIDE THE ROOT THAT THIS USER CANNOT SEARCH IS THE SOURCE + (#712, after RV827's S1): something is there, so it answers, and + `_digest_from_source` names why it cannot, a finding. `exists()` raised + its `PermissionError` out of the run up to Python 3.13, and from 3.14 + would have passed it over for `gh api`. """ keyed = sources.keyed.get(pin_product_key(product)) if keyed is not None: @@ -1327,7 +1368,11 @@ def resolve_neutral_pin_source(root: Path, product: str, repository: str, if env_value: return Path(env_value), f"${env_name}", None sibling = root.parent / repo_basename(repository) - if (sibling / ".git").exists(): + try: + beside = _is_there(sibling / ".git") + except OSError: + beside = True + if beside: return sibling, f"sibling checkout ({_shown(str(sibling))})", None if sources.bare is not None: typed, path = sources.bare diff --git a/tests/test_validate_pins_named_and_spelled.py b/tests/test_validate_pins_named_and_spelled.py index 17540f6b..942fb205 100644 --- a/tests/test_validate_pins_named_and_spelled.py +++ b/tests/test_validate_pins_named_and_spelled.py @@ -107,7 +107,7 @@ #: byte that is not UTF-8, as `surrogateescape` keeps one. UNPRINTABLE = [ pytest.param("a\nb", id="newline"), - pytest.param("rev‮elif", id="bidi-control"), + pytest.param("rev\u202eelif", id="bidi-control"), pytest.param("c1\u0085x", id="c1-control"), pytest.param("x\x1b[31mRED", id="esc"), pytest.param("und\udcffx", id="undecodable-byte"), @@ -136,8 +136,9 @@ def refusal_of_the_run(validator, project: Path, values=()): """The `Refusal` the gate's checks raise over `project`, failing the test if they finish or raise anything else. The class is the project's own copy's, which is the one its `main()` catches (SonarCloud S5958).""" + report, given = validator.Report(), list(values) with pytest.raises(validator.Refusal) as caught: - validator._run_all_checks(project, validator.Report(), list(values)) + validator._run_all_checks(project, report, given) return caught.value @@ -235,6 +236,9 @@ def test_a_refused_stat_is_refused_by_name_on_every_python( id="leg-pin"), pytest.param(lambda v, root: v._check_neutral_pin_entry_and_load( root, "a\x00b", v.Report()), "neutral-pin-missing", id="neutral-pin"), + pytest.param(lambda v, root: v._check_pin_digest( + root, "a\x00b", COMMIT, {}, "rel", v.Report()), + "pin-submodule-uninitialized", id="leg-checkout"), ]) def test_a_name_the_system_cannot_take_is_still_no_file(validator, project, call, code): @@ -247,6 +251,119 @@ def test_a_name_the_system_cannot_take_is_still_no_file(validator, project, assert "\\x00" in caught.value.detail, caught.value.detail +#: What the refusal of a leg checkout this user cannot search says first. +CHECKOUT_READS = ( + "Remediation: this validator reads each leg's checkout, to recompute its " + "pinned tree digest from the leg's own objects, and one it cannot read " + "is a question it cannot answer, never a pass. ") + + +@CANNOT_RUN_AS_ROOT +@pytest.mark.parametrize("mode", [0, stat.S_IRUSR], ids=["000", "r--"]) +def test_a_leg_checkout_the_gate_cannot_search_is_refused_by_name(project, + mode): + """RV827'S S1, THE LEG: with `spec/` at 000 or r--, `exists()` on its + `.git` raised `PermissionError` out of the run, exit 1, on 3.11 to 3.13, + and on 3.14 would have called the leg an uninitialized submodule. It is + `pin-submodule-unreadable`, naming the `.git` it could not reach, with + the permission advice, and nothing is written.""" + before = tree_bytes(project) + with mode_of(project, "spec", mode): + result = validate(project) + assert result.stderr == ( + "REFUSED pin-submodule-unreadable: spec/.git: cannot be read " + f"(Permission denied)\n{CHECKOUT_READS}{PERMISSION}{AGAIN}\n"), \ + result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert tree_bytes(project) == before, "a validator writes nothing" + + +@pytest.mark.parametrize("failure, advice", [ + pytest.param(PermissionError(errno.EACCES, "Permission denied"), + PERMISSION, id="permission"), + pytest.param(os_error(errno.EIO), OTHER, id="io-error"), +]) +def test_a_leg_checkout_whose_stat_is_refused_is_refused_on_every_python( + validator, project, monkeypatch, failure, advice): + """The same leg, in-process, so on every Python, as root and on Windows: + `refused_stat` answers as Python 3.14's `exists()` does, which is what + read the leg as uninitialized. Each cause has its advice.""" + failing(monkeypatch, "stat", {".git": failure}) + monkeypatch.setattr(Path, "exists", lambda self: False) + refusal = refusal_of_the_run(validator, project) + assert refusal.code == "pin-submodule-unreadable" + assert refusal.detail == ( + f"spec/.git: cannot be read ({failure.strerror})") + assert refusal.remediation == CHECKOUT_READS + advice + AGAIN + + +def beside_the_root(project: Path) -> Path: + """`../openXdox`, the checkout a declared `openXdox` pin is looked for + beside the root, holding a `.git`.""" + sibling = project.parent / "openXdox" + (sibling / ".git").mkdir(parents=True) + return sibling + + +def test_a_checkout_beside_the_root_whose_stat_is_refused_is_the_source( + validator, project, monkeypatch): + """RV827'S S1, THE SIBLING, on every Python: something is at + `../openXdox`, so it is the source, where `exists()` raised its + `PermissionError` out of the run up to 3.13 and from 3.14 would have + passed it over for `gh api`.""" + clear_ambient_pin_sources(monkeypatch) + sibling = beside_the_root(project) + refused_stat(monkeypatch, ".git") + found = validator.resolve_neutral_pin_source( + project, "openXdox", "testorg/openXdox", + validator.PinSources([], ["openXdox"])) + assert found == (sibling, f"sibling checkout ({sibling})", None) + + +@CANNOT_RUN_AS_ROOT +def test_a_checkout_beside_the_root_this_user_cannot_search_cannot_answer( + validator, project, monkeypatch): + """With `../openXdox` at 000, the source cannot answer: the pin's + `neutral-pin-source-unreadable` finding names it and the system's + reason, and the run goes on.""" + clear_ambient_pin_sources(monkeypatch) + sibling = beside_the_root(project) + report = validator.Report() + sources = validator.PinSources([], ["openXdox"]) + with mode_of(project.parent, "openXdox", 0): + found = validator._recompute_neutral_pin_digest( + project, "openXdox", "testorg/openXdox", COMMIT, "rel", sources, + report) + assert found is None + assert report.findings[0].startswith( + f"FINDING neutral-pin-source-unreadable: rel: {sibling} (sibling " + f"checkout ({sibling})) could not answer for testorg/openXdox @ " + f"{COMMIT}: Permission denied\n"), report.findings + + +@pytest.mark.parametrize("call, code, pin", [ + pytest.param(lambda v, root, name: v._load_leg_pin( + root, name, "spec", "o/r", v.Report()), "pin-missing", + lambda name: f"{name}-pin.yaml", id="leg-role"), + pytest.param(lambda v, root, name: v._check_neutral_pin_entry_and_load( + root, name, v.Report()), "neutral-pin-missing", + lambda name: f"{name.lower()}-pin.yaml", id="neutral-product"), +]) +def test_a_name_spelled_as_an_absolute_path_is_refused_by_name( + validator, project, call, code, pin): + """RV827'S S2: a role or a product spelled as an absolute path puts its + pin outside the root, where `relative_to` raised a `ValueError` out of + the run, exit 1. It is the missing pin's refusal, naming the path whole: + `_named`'s fallback.""" + name = str(project.parent / "nonexistent-w712" / "x") + with pytest.raises(validator.Refusal) as caught: + call(validator, project, name) + assert caught.value.code == code + assert caught.value.detail.endswith( + f"{project / 'contracts' / pin(name)} does not exist"), \ + caught.value.detail + + # --- L4-104: every file named relative to the root --------------------------- def unlinked(relative: str): @@ -488,15 +605,17 @@ def recorded(root, path): def initialized(monkeypatch, value: str) -> None: - """The leg at `value` is a checkout: its `.git` is there.""" - real = Path.exists + """The leg at `value` is a checkout: a `stat` of its `.git`, which is + what `_is_there` asks, finds a directory, on any platform.""" + real = Path.stat + found = os.stat_result((stat.S_IFDIR | 0o755,) + (0,) * 9) - def exists(self, *args, **kwargs): + def answer(self, *args, **kwargs): if self.name == ".git" and self.parent.name == value: - return True + return found return real(self, *args, **kwargs) - monkeypatch.setattr(Path, "exists", exists) + monkeypatch.setattr(Path, "stat", answer) def valid_pin(validator) -> dict: @@ -598,19 +717,24 @@ def gitlink_note(v, root, mp, value): return report_text(report), [value] -def digest_text(v, root, value, pin=None) -> str: - return refusal_text(v, v._check_pin_digest, root, value, COMMIT, +def digest_text(v, root, value, code: str, pin=None) -> str: + """`_check_pin_digest`'s refusal for the leg at `value`, which must be + `code`'s: a row spelled in another refusal would test nothing.""" + text = refusal_text(v, v._check_pin_digest, root, value, COMMIT, pin or {}, "rel", v.Report()) + assert text.startswith(f"REFUSED {code}: "), text + return text def digest_uninitialized(v, root, mp, value): - return digest_text(v, root, value), [value] + return digest_text(v, root, value, "pin-submodule-uninitialized"), [ + value] def digest_lookup_unrun(v, root, mp, value): initialized(mp, value) mp.setattr(v, "git_out", no_git) - return digest_text(v, root, value), [value] + return digest_text(v, root, value, "git-not-runnable"), [value] def digest_commit_unresolvable(v, root, mp, value): @@ -620,14 +744,15 @@ def refused_by_git(*_args, **_kwargs): raise v.Refusal("git-failed", "it said no") mp.setattr(v, "git_out", refused_by_git) - return digest_text(v, root, value), [value] + return digest_text(v, root, value, "pin-commit-unresolvable"), [value] def digest_recompute_unrun(v, root, mp, value): initialized(mp, value) mp.setattr(v, "git_out", lambda *_args, **_kwargs: "") mp.setattr(v, "tree_digest", no_git) - return digest_text(v, root, value, valid_pin(v)), [value] + return digest_text(v, root, value, "git-not-runnable", + valid_pin(v)), [value] def digest_note(v, root, mp, value): From eea10815838c8a5f1b23d275b56cdc446ed74d14 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 18:48:29 +0000 Subject: [PATCH 3/3] tests: the sibling row at mode 000 stubs `tree_digest_from_gh`, so no mutant that passes the checkout over can reach `gh api` (#712) RV827's N7 on #827 (issuecomment-6237751677): `test_a_checkout_beside_the_root_this_user_cannot_search_cannot_answer` did not stub `tree_digest_from_gh`, and under a mutant that passed the sibling over it fell through to `gh api`, which conftest's guard refused. The stub that `recompute` used is `no_gh_api` now, shared by both. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- tests/test_validate_pins_named_and_spelled.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/test_validate_pins_named_and_spelled.py b/tests/test_validate_pins_named_and_spelled.py index 942fb205..4997c930 100644 --- a/tests/test_validate_pins_named_and_spelled.py +++ b/tests/test_validate_pins_named_and_spelled.py @@ -327,6 +327,7 @@ def test_a_checkout_beside_the_root_this_user_cannot_search_cannot_answer( `neutral-pin-source-unreadable` finding names it and the system's reason, and the run goes on.""" clear_ambient_pin_sources(monkeypatch) + no_gh_api(validator, monkeypatch) sibling = beside_the_root(project) report = validator.Report() sources = validator.PinSources([], ["openXdox"]) @@ -595,6 +596,15 @@ def no_git(*_args, **_kwargs): raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), "git") +def no_gh_api(validator, monkeypatch) -> None: + """`tree_digest_from_gh` refusing, as a machine with no `gh` does: no + row here asks GitHub, even under a mutant that passes a source over.""" + def no_gh(*_args, **_kwargs): + raise validator.Refusal("gh-not-found", "no gh here") + + monkeypatch.setattr(validator, "tree_digest_from_gh", no_gh) + + def gitlink(validator, monkeypatch, answer) -> None: """`recorded_gitlink` answering `answer`, or raising it.""" def recorded(root, path): @@ -900,11 +910,7 @@ def recompute(v, root, mp, product, repository, sources, shared=frozenset()): """`_recompute_neutral_pin_digest`'s report, with no ambient variable and no `gh` to answer.""" clear_ambient_pin_sources(mp) - - def no_gh(*_args, **_kwargs): - raise v.Refusal("gh-not-found", "no gh here") - - mp.setattr(v, "tree_digest_from_gh", no_gh) + no_gh_api(v, mp) report = v.Report() v._recompute_neutral_pin_digest(root, product, repository, COMMIT, "rel", sources, report, shared)