From 37989a6cdc021ef22538b5433d97fa675b83d5a1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 16:37:02 +0000 Subject: [PATCH 1/3] WIP: update-shape.py: a path in the root or the upstream this user cannot read is `update-unreadable`, named from the root, on every Python, never `copy-missing`, `update-pin-missing` or a SKIPPED validator (#430) Slice B of #430's plan (issuecomment-6251069236), at a checkpoint. The code and its new tests are complete; the targeted test set is not yet all run against it. On Python 3.14 `Path.is_file()`, `is_dir()`, `exists()` and `is_symlink()` answer False to any OSError, and `os.path.lexists` always has; up to 3.13 they raise a refused stat. So on 3.14 a `scripts/` this user can list but not search was five `copy-missing` (and `apply` refused `update-conflict` over them), and a `contracts/` at 000 was `update-pin-missing`; on 3.12 each was a traceback. Every guard in update-shape.py that must tell "absent" from "unreadable" now stats the path itself, through a local `_is_a`, `_lexists` and `_is_link` with validate-pins.py's errno set. `read_pin` and `prepare` RAISE a refused read, for the doctor (#847); `main` refuses it as `update-unreadable`, `: cannot be read ()`, exit 2, with validate-pins.py's `_unreadable_file` advice. The guards changed: `read_pin` (and a pin at mode 000), `_classify_row` (and a copy at mode 000), `root_kind`, `prepare` (and a new search check of `/.`), `open_upstream`, `merged_into`, `find_additions`, the README reader (`unreadable`, never `absent`, never a raise), `run_validator` (refused, never SKIPPED) and `Rollback._remove`. `cmd_apply` turns a refused read after the first write into `update-unreadable` and rolls back. Tested so far against this change: - tests/test_update_shape_unreadable.py (new): 43 passed (35 of them fail on 50178305; the 8 that pass there are the controls); - 21 one-line mutants, each killed by a new test; - tests/test_update_shape*.py, batch 1 of 2: 335 passed, 3 skipped, as on 50178305; - render-cli-reference.py --check: ok. Not yet run against it, so not known green: test_update_shape*.py batch 2 (running at this commit), the doctor files, the trio and the other files that name what changed. Nothing is known red. Part of #430 Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_update_shape_unreadable.py | 595 ++++++++++++++++++++++++++ update-shape.py | 323 +++++++++++++- 2 files changed, 900 insertions(+), 18 deletions(-) create mode 100644 tests/test_update_shape_unreadable.py diff --git a/tests/test_update_shape_unreadable.py b/tests/test_update_shape_unreadable.py new file mode 100644 index 00000000..8dd1a2a4 --- /dev/null +++ b/tests/test_update_shape_unreadable.py @@ -0,0 +1,595 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`update-shape.py`: a path this user cannot read is refused by name, and +is never read as a path that is not there (#430, slice B). + +THE DEFECT. Up to Python 3.13 `Path.is_file()`, `is_dir()`, `exists()` and +`is_symlink()` raise a refused `stat`; from 3.14 they answer False to any +`OSError`, and `os.path.lexists` always has. Measured on main `50178305`, in +a scaffolded root with local remotes: + + * `scripts/` at 444: `check` and `apply --yes` ended in a traceback on + 3.12, and on 3.14 `check` called five copies `copy-missing` and `apply` + refused `update-conflict` over copies "pinned and gone from the root"; + * `contracts/` at 000: `check` a traceback at `read_pin` on 3.12, and on + 3.14 `REFUSED update-pin-missing: ...shape-pin.yaml does not exist`, + whose remedy is to adopt the project; + * `.github/workflows/` at 444: a traceback, or a false `copy-missing`. + +Every one of those files is there. Each cell now refuses `update-unreadable`, +`: cannot be read (Permission denied)`, with the remedy +`validate-pins.py`'s `_unreadable_file` gives, exit 2, on every Python. + +TWO KINDS OF CASE. The MODE cases change a real mode and run the tool as a +person does, so they skip on Windows and as root (`needs_a_mode`). The +STAND-IN cases run it in process with a `stat` refused by name +(`refused_stat`, which also answers `is_file()` and the rest as 3.14 does), +or an `lstat` (`refused_lstat`), so they hold 3.14's answer on any Python, +root included. The CONTROLS hold the other half: a copy, a pin, a validator +that is not there is absent as it always was. + +NOTHING HERE REACHES A NETWORK: the session's scaffold into local bare +remotes, with this checkout as `--upstream`, and `test_update_shape.py`'s +own upstream clone for the one `apply` that writes. +""" + +from __future__ import annotations + +import errno +import os +import stat +import sys +from pathlib import Path + +import pytest + +from conftest import REPO, run_script +#: The skip for a mode root reads through, and the stand-in for a refused +#: `stat`, from the file that owns them (#383), as the doctor's and +#: `validate-pins.py`'s #430 tests take them. +from test_bump_leg_unopenable import needs_a_mode, refused_stat +#: A mode for the block and the old one back after it, from the file that +#: owns it. +from test_validate_pins_unreadable_workflows_dir import mode_of +#: `update-shape.py` loaded as a module, and the A -> B upstream whose B +#: changes one copy, from the file that owns them. +from test_update_shape import ( # noqa: F401 + CHANGED, root, update_shape, upstream_and_project, +) + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import Refusal, load_yaml # noqa: E402 +from shape_materialize import SHAPE_REPOSITORY # noqa: E402 + +UPDATE = REPO / "update-shape.py" +UNREADABLE = "update-unreadable" +DENIED = "Permission denied" +PIN = "contracts/shape-pin.yaml" + +#: The pinned copies under `scripts/`, in the pin's order: a `scripts/` this +#: user cannot search refuses the `stat` of every one, and the first is +#: named. +SCRIPTS = ("validate-pins.py", "validate-manifest.py", "bootstrap.py", + "repo_shape.py", "validate-repository-naming.py") + +#: The words no refusal of a file that IS there may say. +ABSENT_WORDS = ("copy-missing", "does not exist", "pinned and gone", + "pinned but absent", "SKIPPED", "Traceback") + +#: #430's measured cells: where the mode goes, the mode, the names whose +#: `stat` the stand-in refuses, and the path the refusal names. +CELLS = [ + pytest.param("scripts", 0o444, SCRIPTS, "scripts/validate-pins.py", + id="scripts-444"), + pytest.param("contracts", 0o000, ("shape-pin.yaml",), PIN, + id="contracts-000"), + pytest.param(".github/workflows", 0o444, ("validate.yml",), + ".github/workflows/validate.yml", id="workflows-444"), +] + + +@pytest.fixture(autouse=True) +def refusals_where_capsys_reads_them(update_shape, monkeypatch): + """`main`'s refusals, run in this process, printed to `sys.stderr` as + it is when they are printed: `repo_shape.die` takes the stream as a + default bound when that module was imported, which is not the one a + test's `capsys` reads.""" + real = update_shape.die + monkeypatch.setattr(update_shape, "die", + lambda exc: real(exc, stream=sys.stderr)) + + +def cannot(named: str) -> str: + """`validate-pins.py`'s line for a path it cannot reach or open.""" + return f"{named}: cannot be read ({DENIED})" + + +def args_for(command: str, project: Path, *extra: str) -> list[str]: + """`check` or `apply --yes`, on `project`, with this checkout upstream.""" + return [command, "--root", str(project), "--upstream", str(REPO), + *(["--yes"] if command == "apply" else []), *extra] + + +def run(command: str, project: Path, *extra: str) -> tuple[int, str]: + """The tool in a subprocess: `(exit, stdout + stderr)`.""" + result = run_script(UPDATE, *args_for(command, project, *extra)) + return result.returncode, result.stdout + result.stderr + + +def run_here(update_shape, capsys, command: str, project: Path, + *extra: str) -> tuple[int, str]: + """The tool in this process, where a stand-in reaches it.""" + code = update_shape.main(args_for(command, project, *extra)) + out, err = capsys.readouterr() + return code, out + err + + +def assert_named(code: int, said: str, named: str, *, + inside: bool = True) -> None: + """The refusal every unreadable path gets: exit 2, `update-unreadable` + naming it first, the permission remedy, and none of the words for a + path that is not there.""" + assert code == 2, said + assert f"REFUSED {UNREADABLE}: {cannot(named)}\n" in said, said + assert "`chmod u+r` on a file, `chmod u+rx` on a directory" in said + above = ("(and on every directory above it inside the root)" if inside + else "(and on every directory above it)") + assert above in said, said + for words in ABSENT_WORDS: + assert words not in said, said + + +def pin_bytes(project: Path) -> bytes: + return (project / PIN).read_bytes() + + +def refused_lstat(monkeypatch, *suffixes: str) -> None: + """`os.lstat` refused (`Permission denied`) for a path ending in any of + `suffixes` (spelled with `/`), as for a name inside a directory this + user may not search; and `Path.is_symlink` answering as 3.14's does, + False to any `OSError`. `os.path.lexists` reads `os.lstat`, so it says + False here, as it says on every Python for a refused one.""" + real = os.lstat + + def refusing(path, *args, **kwargs): + if any(Path(os.fspath(path)).as_posix().endswith("/" + suffix) + for suffix in suffixes): + raise PermissionError(errno.EACCES, DENIED, os.fspath(path)) + return real(path, *args, **kwargs) + + def is_symlink(self): + try: + return stat.S_ISLNK(os.lstat(self).st_mode) + except (OSError, ValueError): + return False + + monkeypatch.setattr(os, "lstat", refusing) + monkeypatch.setattr(Path, "is_symlink", is_symlink) + + +# --- the measured cells, under a real mode ---------------------------------- + +@needs_a_mode +@pytest.mark.parametrize("relative, mode, _names, named", CELLS) +def test_check_names_what_it_cannot_read(project, relative, mode, _names, + named): + """THE ISSUE'S CELLS, `check`: a traceback on 3.12 and `copy-missing` or + `update-pin-missing` on 3.14, over files that are there.""" + with mode_of(project, relative, mode): + code, said = run("check", project) + assert_named(code, said, named) + + +@needs_a_mode +def test_apply_names_what_it_cannot_read_and_writes_nothing(project): + """`scripts/` at 444, `apply --yes`: it was a traceback, or on 3.14 + `update-conflict` asking for copies to be put back from history. The + refusal comes from `prepare`, before the first write.""" + before = pin_bytes(project) + with mode_of(project, "scripts", 0o444): + code, said = run("apply", project) + assert_named(code, said, "scripts/validate-pins.py") + assert "update-conflict" not in said + assert pin_bytes(project) == before + + +# --- the measured cells, as Python 3.14 answers them (in process) ----------- + +@pytest.mark.parametrize("command", ["check", "apply"]) +@pytest.mark.parametrize("_relative, _mode, names, named", CELLS) +def test_a_refused_stat_is_named_on_every_python( + update_shape, project, capsys, monkeypatch, command, _relative, + _mode, names, named): + """The same cells with each file's `stat` refused, and `is_file()` + answering False to it as 3.14's does: a tool that leans on `is_file()` + calls the files missing here, on any Python.""" + before = pin_bytes(project) + refused_stat(monkeypatch, *names) + code, said = run_here(update_shape, capsys, command, project) + assert_named(code, said, named) + assert pin_bytes(project) == before + + +# --- the controls: what is not there is absent, as it always was ------------ + +@pytest.mark.parametrize("as_314", [False, True], ids=["this-python", + "as-3.14"]) +def test_a_copy_that_is_gone_is_still_copy_missing( + update_shape, project, capsys, monkeypatch, as_314): + """A pinned copy that is NOT there is `copy-missing`, and `apply`'s + `update-conflict`, as before, with 3.14's `is_file()` or this Python's.""" + (project / "scripts" / "bootstrap.py").unlink() + if as_314: + refused_stat(monkeypatch, "nothing-is-called-this") + code, said = run_here(update_shape, capsys, "check", project) + assert code == 1, said + assert " copy-missing scripts/bootstrap.py\n" in said, said + assert UNREADABLE not in said + code, said = run_here(update_shape, capsys, "apply", project) + assert code == 2, said + assert "REFUSED update-conflict: " in said, said + assert "scripts/bootstrap.py is pinned and gone from the root" in said + assert UNREADABLE not in said + + +def test_a_pin_that_is_gone_is_still_missing(update_shape, project, capsys, + monkeypatch): + (project / PIN).unlink() + refused_stat(monkeypatch, "nothing-is-called-this") + code, said = run_here(update_shape, capsys, "check", project) + assert code == 2, said + assert "REFUSED update-pin-missing: " in said + assert "shape-pin.yaml does not exist" in said + assert UNREADABLE not in said + + +def test_a_readable_root_has_nothing_to_do(project): + code, said = run("check", project) + assert code == 0, said + assert "nothing to do" in said + + +# --- the other reads `check` and `apply` make -------------------------------- + +@needs_a_mode +def test_a_pin_this_user_cannot_open_is_named_from_the_root(project): + """A pin at mode 000 in a readable `contracts/`: its `stat` answers, and + its read is refused. `load_yaml`'s `yaml-unreadable` named the absolute + path and gave no remedy; the read's `OSError` is raised out of + `read_pin`, as a refused `stat` is, and refused the same way.""" + with mode_of(project, PIN, 0): + code, said = run("check", project) + assert_named(code, said, PIN) + assert "yaml-unreadable" not in said + + +def test_a_pin_whose_read_is_refused_is_named_on_every_python( + update_shape, project, capsys, monkeypatch): + real = Path.read_text + + def read_text(self, *args, **kwargs): + if self.name == "shape-pin.yaml": + raise PermissionError(errno.EACCES, DENIED, str(self)) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", read_text) + code, said = run_here(update_shape, capsys, "check", project) + assert_named(code, said, PIN) + + +@needs_a_mode +def test_a_copy_this_user_cannot_open_is_named(project): + """A copy at mode 000 in a readable directory: `is_file()` says yes on + every Python, and the read of its bytes raised out of `main` as a + traceback, exit 1, on every Python.""" + with mode_of(project, "scripts/bootstrap.py", 0): + code, said = run("check", project) + assert_named(code, said, "scripts/bootstrap.py") + + +def test_a_copy_whose_read_is_refused_is_named_on_every_python( + update_shape, project, capsys, monkeypatch): + real = Path.read_bytes + + def read_bytes(self): + if self.name == "bootstrap.py": + raise PermissionError(errno.EACCES, DENIED, str(self)) + return real(self) + + monkeypatch.setattr(Path, "read_bytes", read_bytes) + code, said = run_here(update_shape, capsys, "check", project) + assert_named(code, said, "scripts/bootstrap.py") + + +# --- the root itself --------------------------------------------------------- + +@needs_a_mode +def test_a_root_this_user_cannot_search_is_named_in_full(project): + """A root at 600: it is a directory, and nothing in it can be reached. + It was asked first as `root_kind`'s `family.yaml`, a file a project has + not got; it is named whole, as the directory it is, and the remedy does + not say "inside the root" of the root.""" + with mode_of(project, ".", 0o600): + code, said = run("check", project) + assert_named(code, said, str(project.resolve()), inside=False) + assert "family.yaml" not in said + + +@needs_a_mode +def test_a_root_under_a_folder_this_user_cannot_search_is_not_missing( + project): + """The root's own `stat` refused, its parent at 000: it was + `update-root-missing`, "is not a directory", on 3.14.""" + with mode_of(project.parent, ".", 0): + code, said = run("check", project) + assert_named(code, said, str(project), inside=False) + assert "update-root-missing" not in said + + +def test_a_root_whose_stat_is_refused_is_not_missing(update_shape, project, + capsys, monkeypatch): + refused_stat(monkeypatch, project.name) + code, said = run_here(update_shape, capsys, "check", project) + assert_named(code, said, str(project.resolve()), inside=False) + assert "update-root-missing" not in said + + +@pytest.mark.parametrize("present, refused", [ + pytest.param((), "family.yaml", id="the-family-manifest"), + pytest.param(("family.yaml",), "project.yaml", id="the-project-manifest"), +]) +def test_root_kind_raises_a_refused_stat(update_shape, tmp_path, monkeypatch, + present, refused): + """`root_kind` asks both manifests as `validate-pins.py` asks a file: + a refused `stat` is raised, never read as "no such manifest", which + would pick the other kind of root.""" + for name in present: + (tmp_path / name).write_text("kind: family\n", encoding="utf-8") + refused_stat(monkeypatch, refused) + with pytest.raises(PermissionError): + update_shape.root_kind(tmp_path) + + +def test_root_kind_still_reads_what_is_there(update_shape, tmp_path): + assert update_shape.root_kind(tmp_path) is update_shape.PROJECT_KIND + (tmp_path / "family.yaml").write_text("kind: family\n", encoding="utf-8") + assert update_shape.root_kind(tmp_path) is update_shape.FAMILY_KIND + + +# --- `--upstream` ------------------------------------------------------------ + +@needs_a_mode +@pytest.mark.parametrize("locked, named", [ + pytest.param("folder", "folder/shape", id="its-parent-at-000"), + pytest.param("folder/shape", "folder/shape/.git", id="itself-at-000"), +]) +def test_an_upstream_this_user_cannot_read_is_named(project, tmp_path, + locked, named): + """`--upstream` under a folder this user cannot search was, on 3.14, + "neither a path on this machine nor an `owner/repo` name" (and a + relative `a/b` spelling would have been cloned from github.com); one it + cannot search itself was `update-upstream-not-a-repo`. Named whole: it + is not inside the root.""" + (tmp_path / "folder" / "shape").mkdir(parents=True) + with mode_of(tmp_path, locked, 0): + result = run_script(UPDATE, "check", "--root", str(project), + "--upstream", str(tmp_path / "folder" / "shape")) + assert_named(result.returncode, result.stdout + result.stderr, + str(tmp_path.resolve() / named), inside=False) + + +@pytest.mark.parametrize("refused", [ + pytest.param("shape", id="the-path-itself"), + pytest.param(".git", id="its-git"), + pytest.param("HEAD", id="a-bare-repositorys-head"), +]) +def test_open_upstream_raises_a_refused_stat(update_shape, tmp_path, + monkeypatch, refused): + """Each of `open_upstream`'s three questions raises a refused `stat`, + where 3.14's answer refused `update-upstream-unresolvable` or + `update-upstream-not-a-repo` of a path that is there. `.git` is asked + first and a bare repository's `HEAD` only where it is not there.""" + shape = tmp_path / "shape" + shape.mkdir() + refused_stat(monkeypatch, refused) + with pytest.raises(PermissionError): + update_shape.open_upstream(str(shape)) + + +# --- a merged-away copy, and an addition ------------------------------------- + +def merged_root(tmp_path: Path, *, collision: bool) -> Path: + """A root holding `Makefile`, and `shape/Makefile` where `collision`.""" + (tmp_path / "shape").mkdir() + (tmp_path / "Makefile").write_text("all:\n", encoding="utf-8") + if collision: + (tmp_path / "shape" / "Makefile").write_text("all:\n", + encoding="utf-8") + return tmp_path + + +@needs_a_mode +def test_a_shape_folder_this_user_cannot_search_is_no_merge(update_shape, + tmp_path): + """`os.path.lexists` answers False to a refused `lstat` on EVERY Python, + so a `shape/` this user cannot search held "nothing" at `shape/Makefile`, + the row read `merged-away`, and `apply` would have dropped its pin row.""" + root = merged_root(tmp_path, collision=True) + with mode_of(root, "shape", 0), pytest.raises(PermissionError): + update_shape.merged_into(root, "shape/Makefile") + + +@pytest.mark.parametrize("collision, refuse", [ + pytest.param(True, "lstat", id="the-copy"), + pytest.param(False, "stat", id="the-file-it-was-merged-into"), +]) +def test_merged_into_raises_a_refused_read(update_shape, tmp_path, + monkeypatch, collision, refuse): + root = merged_root(tmp_path, collision=collision) + if refuse == "lstat": + refused_lstat(monkeypatch, "shape/Makefile") + else: + refused_stat(monkeypatch, "Makefile") + with pytest.raises(PermissionError): + update_shape.merged_into(root, "shape/Makefile") + + +def test_merged_into_still_reads_what_is_there(update_shape, tmp_path): + root = merged_root(tmp_path, collision=False) + assert update_shape.merged_into(root, "shape/Makefile") == "Makefile" + (root / "shape" / "Makefile").write_text("all:\n", encoding="utf-8") + assert update_shape.merged_into(root, "shape/Makefile") is None + + +class AddingUpstream: + """An upstream whose copy lists, at any commit, add `docs/new.md`.""" + + def blob(self, _rev: str, path: str) -> bytes | None: + if path == "scripts/shape_materialize.py": + return (b'COPIED_VERBATIM = ("docs/new.md",)\n' + b'COPIED_FROM_SHAPE = ()\nEXECUTABLE = ()\n') + if path == "templates/assembly-root/docs/new.md": + return b"the standard's\n" + return None + + +def additions(update_shape, root: Path) -> list[tuple[str, bool]]: + return [(add.path, add.present) for add in update_shape.find_additions( + root, [], AddingUpstream(), "0" * 40, update_shape.PROJECT_KIND)] + + +@needs_a_mode +def test_an_addition_under_a_folder_this_user_cannot_search_is_named( + update_shape, tmp_path): + """The root HAS `docs/new.md`, in a `docs/` at 000: it was `present` + False (`os.path.lexists`), so `check` offered `--add` over the + project's own file.""" + (tmp_path / "docs").mkdir() + (tmp_path / "docs" / "new.md").write_text("ours\n", encoding="utf-8") + with mode_of(tmp_path, "docs", 0), pytest.raises(PermissionError): + additions(update_shape, tmp_path) + + +def test_an_addition_whose_lstat_is_refused_is_raised(update_shape, tmp_path, + monkeypatch): + (tmp_path / "docs").mkdir() + refused_lstat(monkeypatch, "docs/new.md") + with pytest.raises(PermissionError): + additions(update_shape, tmp_path) + + +def test_an_addition_still_reads_what_is_there(update_shape, tmp_path): + assert additions(update_shape, tmp_path) == [("docs/new.md", False)] + (tmp_path / "docs").mkdir() + (tmp_path / "docs" / "new.md").write_text("ours\n", encoding="utf-8") + assert additions(update_shape, tmp_path) == [("docs/new.md", True)] + + +# --- the README's line, a reading that never raises -------------------------- + +@pytest.mark.parametrize("refuse", ["stat", "lstat"]) +def test_a_readme_this_user_cannot_stat_is_unreadable_not_absent( + update_shape, project, monkeypatch, refuse): + """`README.md`'s `stat` or `lstat` refused: 3.12 raised out of a + classification that promises never to, and 3.14 answered `absent`. It + is `unreadable`, with the system's reason, which `check` prints and + `apply` leaves alone: prose, so no exit code moves.""" + if refuse == "stat": + refused_stat(monkeypatch, "README.md") + else: + refused_lstat(monkeypatch, "README.md") + line = update_shape.ReadmeShapeLine(project, SHAPE_REPOSITORY, "0" * 40) + assert line.state == "unreadable" + assert line.verdict == f"unreadable (README.md is not readable — {DENIED})" + + +# --- `apply`'s validators, and a read refused after the first write ---------- + +def test_a_validator_whose_stat_is_refused_is_not_skipped(update_shape, + project, capsys, + monkeypatch): + refused_stat(monkeypatch, "validate-pins.py") + with pytest.raises(Refusal) as raised: + update_shape.run_validator(project, "scripts/validate-pins.py") + assert raised.value.code == UNREADABLE + assert raised.value.detail == cannot("scripts/validate-pins.py") + assert "SKIPPED" not in capsys.readouterr().out + + +def test_a_validator_that_is_not_there_is_still_skipped(update_shape, + project, capsys): + assert update_shape.run_validator(project, "scripts/not-there.py") == 0 + assert " scripts/not-there.py is absent; SKIPPED\n" in \ + capsys.readouterr().out + + +def apply_b(update_shape, root: Path, project: dict) -> int: + """`apply --at B --yes` in this process: B changes `CHANGED`, so the + run writes a copy, the pin and the manifest before its validators.""" + return update_shape.main(["apply", "--root", str(root), "--yes", + "--upstream", str(project["upstream"]), + "--at", project["b"]]) + + +def assert_rolled_back(said: str, root: Path, project: dict, + changed: bytes) -> None: + assert f"REFUSED {UNREADABLE}: {cannot('scripts/validate-pins.py')}\n" \ + in said, said + assert ("ROLLED BACK: every byte this command wrote has been put back, " + "and the tree is exactly as it was:") in said, said + assert "SKIPPED" not in said + assert load_yaml(root / PIN)["commit"].lower() == project["a"] + assert (root / CHANGED).read_bytes() == changed + + +def test_a_validator_unreadable_after_the_writes_rolls_them_back( + update_shape, root, upstream_and_project, capsys, monkeypatch): + """MID-RUN: the copies, the pin and the manifest are written, and then + the validator's `stat` is refused. 3.14's `is_file()` said "absent; + SKIPPED" and the run went on without its gate; 3.12's raise left the + writes behind a traceback. It is a refusal, and every byte goes back.""" + changed = (root / CHANGED).read_bytes() + real = update_shape._refuse_red_validators + + def turning_unreadable(where, kind): + refused_stat(monkeypatch, "validate-pins.py") + return real(where, kind) + + monkeypatch.setattr(update_shape, "_refuse_red_validators", + turning_unreadable) + assert apply_b(update_shape, root, upstream_and_project) == 2 + out, err = capsys.readouterr() + assert " copied scripts/validate-manifest.py" in out + assert_rolled_back(out + err, root, upstream_and_project, changed) + + +def test_any_read_refused_after_the_writes_rolls_them_back( + update_shape, root, upstream_and_project, capsys, monkeypatch): + """MID-RUN, a refused read raised as an `OSError` by whatever reads + after the first write: `apply` rolls back on a `Refusal` alone, so it + is made one, `update-unreadable`, and the rollback says what it put + back.""" + changed = (root / CHANGED).read_bytes() + + def refusing(where, _kind): + raise PermissionError(errno.EACCES, DENIED, + str(where / "scripts" / "validate-pins.py")) + + monkeypatch.setattr(update_shape, "_refuse_red_validators", refusing) + assert apply_b(update_shape, root, upstream_and_project) == 2 + out, err = capsys.readouterr() + assert_rolled_back(out + err, root, upstream_and_project, changed) + + +def test_a_rollback_that_cannot_stat_what_it_made_says_so(update_shape, + tmp_path, + monkeypatch): + """An `--add` file whose `stat` is refused when the rollback comes for + it: 3.14's `exists()` said False, nothing was removed or listed, and the + rollback said the tree was "exactly as it was".""" + ledger = update_shape.Rollback(tmp_path) + ledger.write_added(tmp_path / "docs" / "new.md", b"x\n", False) + refused_stat(monkeypatch, "new.md") + said = ledger.undo() + assert said.startswith("ROLLED BACK IN PART: the tree is NOT as it was.") + assert (f"file docs/new.md, written by --add, was not removed " + f"({DENIED}): rm -- {tmp_path / 'docs' / 'new.md'}") in said diff --git a/update-shape.py b/update-shape.py index b3816c22..aa6589ec 100755 --- a/update-shape.py +++ b/update-shape.py @@ -67,6 +67,11 @@ * A write that fails is a refusal, not a traceback (#285): a copy, an addition or the pin this process cannot write is `update-write-failed`, and every byte written before it is put back, as below. + * Nor is a read that fails an absence (#430). A pinned copy, the pin, the + root or the upstream this user cannot read -- a `scripts/` it can list + but not search, a `contracts/` at 000 -- is `update-unreadable`, by + name, where Python 3.14's `is_file()` called it missing and 3.12's + raised a traceback; a copy that is NOT there is `copy-missing` still. * Nothing is left half-done either (#202). With `--branch`, a branch git cannot create, an addition it cannot stage or a commit a hook refuses puts back the copies, the pin, the README line, the index and the @@ -129,7 +134,8 @@ EXIT CODES check 0 the pin names the target and no copied file differs 1 updates are available, or drift a human must resolve - 2 a REFUSAL: no pin, an unresolvable upstream, an unreadable root + 2 a REFUSAL: no pin, an unresolvable upstream, a path in the + root or the upstream this user cannot read apply 0 applied (or there was nothing to do) 2 a REFUSAL, including a validator that went red or a branch, stage or commit that failed (rolled back); or a push or pull @@ -144,6 +150,7 @@ import argparse import ast import contextlib +import errno import hashlib import json import os @@ -154,7 +161,7 @@ import subprocess import sys import tempfile -from collections.abc import Iterator +from collections.abc import Callable, Iterator from pathlib import Path from typing import NamedTuple @@ -280,6 +287,156 @@ def __init__(self, name: str, manifest: str, template_dir: str, "scripts/validate-family.py") +# --------------------------------------------------------------------------- +# A path this user cannot read is not a path that is not there (#430) +# --------------------------------------------------------------------------- +# +# Up to Python 3.13 `Path.is_file()`, `is_dir()`, `exists()` and +# `is_symlink()` RAISE a `stat` the operating system refused, and answer +# False only where nothing is there; from 3.14 they answer False to ANY +# `OSError`, and `os.path.lexists` always has. So on 3.14 a `scripts/` this +# user can list but not search was five copies `copy-missing`, which `apply` +# refused as `update-conflict` with a remedy that puts them back from +# history, and a `contracts/` at 000 was `update-pin-missing`, whose remedy +# is to adopt the project; on 3.12 each was a traceback. Every guard that +# must tell "absent" from "unreadable" asks `_is_a` or `_lexists`, which +# answer as 3.9 to 3.13 did, on every Python: a refused one is RAISED, and +# `main` refuses it by name (`unreadable_refusal`). +# +# RAISED, NOT REFUSED, OUT OF `read_pin` AND `prepare`, because +# `shape-doctor.py` tells the two apart: it names a raised `OSError` of a +# file in the root as `validate-pins.py` names it (#847), and reads a +# `Refusal` out of `prepare` as this checkout of the standard unable to +# answer. A LOCAL COPY of `validate-pins.py`'s question, as `bump-leg.py` +# and the doctor keep theirs: `scripts/repo_shape.py` is a shape copy, and a +# helper there would be an `upstream-changed` row in every project (#370, +# #610). + +#: The `OSError`s that `Path.is_file()` and `Path.is_dir()` answered False to +#: on Python 3.9 to 3.13 (pathlib's `_ignore_error`), because each means +#: there is nothing at the name to open: no such file, a part of the path +#: that is not a directory, a bad descriptor, a symlink loop, and Windows' +#: "not ready", "invalid name" and "cannot resolve". `validate-pins.py`'s +#: set, and `bump-leg.py`'s. Every other one is a read the system refused. +_ABSENT_ERRNOS = (errno.ENOENT, errno.ENOTDIR, errno.EBADF, errno.ELOOP) +_ABSENT_WINERRORS = (21, 123, 1921) + + +def _says_absent(exc: OSError) -> bool: + """Does `exc` say nothing is at the name (`_ABSENT_ERRNOS`), rather than + that the operating system refused to say what is?""" + return (exc.errno in _ABSENT_ERRNOS + or getattr(exc, "winerror", None) in _ABSENT_WINERRORS) + + +def _mode_of(path: Path, *, follow: bool = True) -> int | None: + """`path`'s mode, by `stat` (`Path.stat`, so a link is followed) or, not + following, by `os.lstat`; None where nothing is there (`_says_absent`), + and the `OSError` of one the operating system refused RAISED (#430). A + name the system cannot take at all (a NUL in it) is nothing there, as + `is_file()` answered it.""" + try: + return (path.stat() if follow else os.lstat(path)).st_mode + except OSError as exc: + if _says_absent(exc): + return None + raise + except ValueError: + return None + + +def _is_a(path: Path, kind: Callable[[int], bool]) -> bool: + """Whether `path` is there and of `kind` (`stat.S_ISREG`, `stat.S_ISDIR`, + `_anything`), following a link as `is_file()` does, and raising the + `OSError` of a `stat` the operating system refused: `validate-pins.py`'s + `_is_a` (#434), asked here (#430).""" + mode = _mode_of(path) + return mode is not None and kind(mode) + + +def _anything(_mode: int) -> bool: + """`_is_a`'s kind for "anything at all", which `exists()` asks.""" + return True + + +def _lexists(path: Path) -> bool: + """Whether anything at all is at `path`, a dangling link included: + `os.path.lexists`, which answers False to ANY `OSError` on every Python, + asked so that a refused `lstat` is raised (#430).""" + return _mode_of(path, follow=False) is not None + + +def _is_link(path: Path) -> bool: + """`Path.is_symlink()`, asked so that a refused `lstat` is raised + (#430), as `_is_a` raises a refused `stat`.""" + return stat.S_ISLNK(_mode_of(path, follow=False) or 0) + + +#: `update-unreadable`: a path this command must read, and the operating +#: system will not let it (#430). +UNREADABLE = "update-unreadable" + +#: What a path the operating system refused for ANOTHER reason than +#: permission is told (an I/O error, a name too long): `validate-pins.py`'s +#: `OTHER_REASON`, word for word (#712), which is not told to `chmod`. +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. ") + +#: What this command reads, as `update-unreadable`'s remedy says it. +FILES_READ = ("the shape pin, the manifest, every copy the shape pin lists " + "and the --upstream checkout") + + +def root_of(args) -> Path: + """`--root` as `prepare` reads it, and as `main` names a path from it.""" + return Path(args.root).expanduser().resolve() + + +def _named_from(root: Path, path: Path) -> tuple[str, bool]: + """`path` as `update-unreadable` names it, and whether it is inside + `root`: from the root and POSIX, as `validate-pins.py` names a file + (#712); the root itself, or a path outside it, such as the upstream, + in full.""" + try: + relative = path.relative_to(root) + except ValueError: + return str(path), False + if not relative.parts: + return str(path), False + return relative.as_posix(), True + + +def unreadable_refusal(root: Path, exc: OSError) -> Refusal | None: + """`update-unreadable` for `exc`, a read the operating system REFUSED of + the path it names; None where `exc` says nothing is there, or names no + path, neither of which this refusal can say (#430). + + `validate-pins.py`'s words (`_unreadable_file`, #712): the line `: + cannot be read ()`, and a remedy whose advice + follows the cause. A permission (`PermissionError`: `EACCES`, `EPERM`) + is `chmod u+r` on a file or `chmod u+rx` on a directory, and on every + directory above it -- inside the root, for a path in it -- and any other + cause is `OTHER_REASON`.""" + name = exc.filename + if _says_absent(exc) or not isinstance(name, (str, bytes, os.PathLike)): + return None + named, inside = _named_from(root, Path(os.fsdecode(name))) + above = " inside the root" if inside else "" + 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 " + f"it{above}), or fix who owns it. " + if isinstance(exc, PermissionError) else OTHER_REASON) + return Refusal( + UNREADABLE, + f"{named}: cannot be read ({exc.strerror or type(exc).__name__})", + f"Remediation: this command reads {FILES_READ}, and one it cannot " + "read is a question it cannot answer, never an absence. " + advice + + "Then run the same command again.") + + def root_kind(root: Path) -> Kind: """A root carrying `family.yaml` and no `project.yaml` is a family. @@ -287,8 +444,14 @@ def root_kind(root: Path) -> Kind: checkout should not have to tell the tool what they are standing in, and the two manifests are mutually exclusive by construction — a family has no legs, so it has no `project.yaml` to mirror a pin into. + + A REFUSED `stat` IS RAISED (#430), never read as "no such manifest", + which would pick the other kind of root. Both are in the root itself, so + only a root this user cannot search refuses one, and `prepare` asks + that first, by name. """ - if (root / "family.yaml").is_file() and not (root / "project.yaml").is_file(): + if (_is_a(root / "family.yaml", stat.S_ISREG) + and not _is_a(root / "project.yaml", stat.S_ISREG)): return FAMILY_KIND return PROJECT_KIND @@ -491,11 +654,19 @@ def open_upstream(spec: str) -> Upstream: asks `gh api` for the pinned tree with the operator's `gh` credentials (`skip` line and exit 0 without `gh`, a login or a network), and `--push` and `--pr` push the branch and open its pull request. + + A PATH THIS USER CANNOT READ IS NOT "NO SUCH PATH" (#430). Each of the + three questions raises a refused `stat`, which `main` names. On 3.14 a + path under a folder this user cannot search was "neither a path on this + machine nor an `owner/repo` name", and a relative `a/b` spelling was + then cloned from github.com; a checkout it cannot search was + `update-upstream-not-a-repo`. """ candidate = Path(spec).expanduser() - if candidate.exists(): + if _is_a(candidate, _anything): path = candidate.resolve() - if not (path / ".git").exists() and not (path / "HEAD").is_file(): + if (not _is_a(path / ".git", _anything) + and not _is_a(path / "HEAD", stat.S_ISREG)): raise Refusal( "update-upstream-not-a-repo", f"--upstream {spec} exists but is not a git repository", @@ -591,12 +762,20 @@ def read_pin(root: Path) -> tuple[dict, list[Row]]: `--branch` makes holds the unchanged link. Asked before `is_file()`, which follows a link, so a dangling one is named as the link it is rather than as a pin that "does not exist". + + A PIN THIS USER CANNOT READ IS NOT A PIN THAT DOES NOT EXIST (#430). Its + `stat` refused (a `contracts/` this user cannot search) was a traceback + on 3.12 and `update-pin-missing` on 3.14, whose remedy adopts the + project; its read refused (a pin at mode 000) was `load_yaml`'s + `yaml-unreadable`, naming the absolute path with no remedy. Both RAISE + the `OSError`, which `main` refuses by name and the doctor names in its + own row (`_read_yaml`). """ pin_path = root / "contracts" / "shape-pin.yaml" linked = linked_record(root) if linked is not None: raise rewrite_link_refusal([linked]) - if not pin_path.is_file(): + if not _is_a(pin_path, stat.S_ISREG): raise Refusal( "update-pin-missing", f"{pin_path} does not exist, so there is no record of which files " @@ -605,7 +784,21 @@ def read_pin(root: Path) -> tuple[dict, list[Row]]: "Remediation: this command updates a project that was scaffolded " "or adopted with this shape. A project without a shape pin is " "adopted with `adopt-project.py`, not updated.") - return check_pin(load_yaml(pin_path), pin_path, root) + return check_pin(_read_yaml(pin_path), pin_path, root) + + +def _read_yaml(path: Path): + """`load_yaml(path)`, with a read the operating system REFUSED raised as + its own `OSError` (#430), which `load_yaml` turns into `yaml-unreadable` + and raises its refusal from, as `validate-pins.py`'s `_load` reads it + (#712). Any other refusal is `load_yaml`'s, as it was.""" + try: + return load_yaml(path) + except Refusal as exc: + cause = exc.__cause__ + if isinstance(cause, OSError) and not _says_absent(cause): + raise cause from None + raise def check_pin(pin, pin_path, root: Path) -> tuple[dict, list[Row]]: @@ -706,6 +899,11 @@ def merged_into(root: Path, rel: str) -> str | None: `copy-missing` with its own refusal, as does a pinned copy deleted anywhere else. + A `shape/` THIS USER CANNOT SEARCH HOLDS NO ANSWER (#430). `lexists` + answers False to a refused `lstat` on every Python, so `shape/X` read + as "nothing there", the row as merged-away, and `apply` dropped its pin + row. `_lexists` and `_is_a` raise a refused read, which `main` names. + WHAT MERGING MEANS IS NOT JUDGED. The row is dropped and nothing is copied, so no byte of `X` is compared with the shape's: the follow-up is the person's, and `X` was never a shape copy of this pin. Read by @@ -716,9 +914,9 @@ def merged_into(root: Path, rel: str) -> str | None: if not rel.startswith(prefix): return None original = rel[len(prefix):] - if not original or os.path.lexists(root / rel): + if not original or _lexists(root / rel): return None - return original if (root / original).is_file() else None + return original if _is_a(root / original, stat.S_ISREG) else None def classify(root: Path, rows: list[Row], upstream: Upstream, pinned: str, @@ -773,9 +971,14 @@ def _classify_row(root: Path, row: Row, upstream: Upstream, pinned: str, in the order they were written in — the root's bytes, then the pinned blob, then the target blob — because `Upstream.blob` shells out to git and a reordering would change what a failing run prints. + + `copy-missing` IS NOTHING THERE, OR NOTHING THAT IS A FILE (#430): a + copy whose `stat` is refused is raised by `_is_a`, as one whose read is + refused is raised by the read, and `main` names either. On 3.14 a + `scripts/` this user can list but not search was five `copy-missing`. """ target_path = root / row.path - if not target_path.is_file(): + if not _is_a(target_path, stat.S_ISREG): row.state = COPY_MISSING row.detail = ("pinned but absent from the root; a human decides " "whether it was deleted on purpose") @@ -1156,6 +1359,15 @@ def symlink_on_the_way(root: Path, rel: str) -> tuple[str, str] | None: creates what the link points at, which a root is free to point outside itself. The first component that cannot be `lstat`ed ends the walk: the ones after a missing component are missing too. + + SO DOES ONE WHOSE `lstat` IS REFUSED, and that is deliberate (#430). + This answers "is a link on the way", and every caller asks the path + itself next, which names a refused read: `read_pin` and `_classify_row` + by `_is_a`, `find_additions` by `_lexists`, `Rollback.write` by the + `open`, which `ledger.writing` refuses. `_classify_row` asks only after + `_is_a` has searched every component. And the doctor asks + `linked_record` before it reads the pin, where a raise would end its + row before the pin could be named. """ here = root walked: list[str] = [] @@ -1213,6 +1425,9 @@ def find_additions(root: Path, rows: list[Row], upstream: Upstream, `exists`, so a symlink whose target is missing, and a symlink above the path, are the operator's and not an absence this tool may write into. A path under a FILE is absent, and is remembered as `blocked` (#285). + And one under a folder this user cannot search is neither (#430): + `_lexists` raises the refused `lstat`, where `lexists` said "nothing + there" and `check` offered `--add` over a file the root may hold. """ pinned = {row.path for row in rows} additions: list[Addition] = [] @@ -1227,7 +1442,7 @@ def find_additions(root: Path, rows: list[Row], upstream: Upstream, continue link = symlink_on_the_way(root, copy.path) additions.append(Addition( - copy, link is not None or os.path.lexists(root / copy.path), + copy, link is not None or _lexists(root / copy.path), data, link, not_a_directory_on_the_way(root, copy.path), left_dead_by(copy.path, rows))) return additions @@ -1882,7 +2097,17 @@ def _classify(self) -> str: the common answers cost no subprocess at all and a README nobody is editing is classified exactly as before. """ - if self.path.is_symlink(): + try: + linked = _is_link(self.path) + there = linked or _is_a(self.path, stat.S_ISREG) + except OSError as exc: + # A REFUSED `stat` OR `lstat` IS A READING TOO (#430), this + # one, as a refused read below is: 3.12's `is_file()` raised it + # out of a classification that must not raise, and 3.14's + # called a README that is there `absent`. + self.unreadable = f"not readable — {exc.strerror or exc}" + return README_UNREADABLE + if linked: # NOT WRITTEN THROUGH, EVER. `is_file()` follows the link, and a # rewrite would then put these bytes into the link's TARGET — # which a root is free to point outside itself — while the @@ -1890,7 +2115,7 @@ def _classify(self) -> str: # would not contain the edit it just made somebody else's file # (Copilot, PR #152). return README_SYMLINK - if not self.path.is_file(): + if not there: return README_ABSENT try: if self.path.stat().st_nlink > 1: @@ -2110,9 +2335,21 @@ def counted(rows: list[Row]) -> dict[str, int]: def prepare(args) -> tuple[Path, dict, list[Row], list[Addition], Upstream, str, str, Kind]: - root = Path(args.root).expanduser().resolve() - if not root.is_dir(): + """The root, its pin, its rows judged and its additions found, the + upstream opened, the two commits and the kind of root: everything + `check` reports and `apply` acts on, read before anything is written. + + A READ THE OPERATING SYSTEM REFUSED IS RAISED HERE, never refused + (#430): `main` names it as `update-unreadable`, and the doctor, which + calls this, names a refused read of a file in the root in its own row + (#847) and reads a `Refusal` from here as the standard unable to + answer. The root is asked first: its own `stat` (a folder above it + this user cannot search), then whether it can be searched at all + (`_search`).""" + root = root_of(args) + if not _is_a(root, stat.S_ISDIR): raise Refusal("update-root-missing", f"{root} is not a directory") + _search(root) kind = root_kind(root) pin, rows = read_pin(root) # THE MANIFEST IS THE OTHER RECORD `apply` REWRITES, so a link at it, or @@ -2138,6 +2375,17 @@ def prepare(args) -> tuple[Path, dict, list[Row], list[Addition], Upstream, return root, pin, rows, additions, upstream, pinned, target, kind +def _search(root: Path) -> None: + """Raise the `OSError` of a root this user cannot SEARCH (#430): one it + can `stat` and may not look inside, mode 600 say, where every path below + it is refused. Asked of `/.`, which the system resolves only by + searching the root, so the refusal names the root, in full, rather than + the first file under it that a reader asks about: `root_kind`'s + `family.yaml`, which a project has not got. `os.path.join`, because + `pathlib` drops a `.`.""" + os.stat(os.path.join(root, os.curdir)) + + def apply_command(root: Path, label: str, target: str) -> str: """The `apply` that `check`'s NEXT line hands a person to paste: this tool run again from the checkout of the standard, pointed at `root`. @@ -2307,7 +2555,22 @@ def confirm(args, rows: list[Row], taking: list[Addition], def run_validator(root: Path, script: str) -> int: - if not (root / script).is_file(): + """Run the root's own `script`, `0` where it passes; a script that is + not there is SKIPPED, and said. + + ONE THIS USER CANNOT READ IS REFUSED, never skipped (#430). On 3.14 a + refused `stat` read as "absent; SKIPPED", so the gate this run exists to + leave green was not run; on 3.12 it was raised out of `apply` after the + writes, which only a `Refusal` rolls back. It is `update-unreadable`, + a `Refusal`, so `cmd_apply` puts back every byte written before it.""" + try: + there = _is_a(root / script, stat.S_ISREG) + except OSError as exc: + refusal = unreadable_refusal(root, exc) + if refusal is None: + raise + raise refusal from exc + if not there: print(f" {script} is absent; SKIPPED") return 0 sys.stdout.flush() @@ -3195,6 +3458,12 @@ def _remove(self, path: Path, stuck: list[Path], done: list[str], write (review of PR #560), for `_restore`'s reason: unlinking one at the path deletes the operator's link (#239 measured that), and removing under one deletes at its end. It is listed instead. + + A `stat` REFUSED IS LISTED TOO (#430). What stands at `path` is + asked once, `_mode_of`, which raises a refused `stat` into the + handler below, where 3.14's `is_dir()` and `exists()` answered False + to it, nothing was removed or listed, and the rollback said the + tree was exactly as it was. """ name = self._name(path) directory = any(path in made.parents for made in self.created) @@ -3209,7 +3478,8 @@ def _remove(self, path: Path, stuck: list[Path], done: list[str], "what it leads to is yours to judge") return try: - if path.is_dir(): + mode = _mode_of(path) + if mode is not None and stat.S_ISDIR(mode): if not any(path.iterdir()): path.rmdir() done.append(f"removed {name}/ (made to hold an --add " @@ -3224,7 +3494,7 @@ def _remove(self, path: Path, stuck: list[Path], done: list[str], done.append(f"kept {name}/ (made to hold an --add " "file, and something this run did not " "write is in it now)") - elif path.exists(): + elif mode is not None: path.unlink() done.append(f"removed {name} (written by --add)") except OSError as exc: @@ -4847,6 +5117,15 @@ def cmd_apply(args) -> int: return 0 except (Refusal, CommandFailed) as exc: return _rolled_back(exc, ledger) + except OSError as exc: + # A READ REFUSED AFTER `prepare` (#430), the first write included: + # this arm is the rollback, and it answers a `Refusal`, so a refused + # read is made one, `update-unreadable`, and every byte goes back + # under it. Any other `OSError` is raised as it was. + refusal = unreadable_refusal(root, exc) + if refusal is None: + raise + return _rolled_back(refusal, ledger) finally: upstream.close() @@ -5771,6 +6050,14 @@ def main(argv: list[str] | None = None) -> int: # Reported where it happened, NEXT line included (#204); the exit # code is all that is left to give. return 2 + except OSError as exc: + # A PATH THIS USER CANNOT READ (#430): `read_pin` and `prepare` + # raise the refused read, for the doctor's sake, and it is refused + # here by name, exit 2. Any other `OSError` is raised as it was. + refusal = unreadable_refusal(root_of(args), exc) + if refusal is None: + raise + return die(refusal) if __name__ == "__main__": From 7086e8ca54f7e868e773fa7e35d2f491739ea22a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sun, 18 Oct 2026 18:47:20 +0000 Subject: [PATCH 2/3] update-shape.py: a path in the root or the upstream this user cannot read is `update-unreadable`, named from the root, on every Python, so a `scripts/` it can list but not search is no longer five `copy-missing`, a `contracts/` at 000 no longer `update-pin-missing`, a `shape/` it cannot search no longer drops a pin row as merged-away, and a validator it cannot stat is refused and rolled back, never SKIPPED (#430) This commit ends the WIP at 37989a6c: the targeted set is now all run against the change, and green. Its own change is in the new test file only: the two in-process read-refused tests are one parametrized test, so no twelve-line body is in the file twice. On Python 3.14 `Path.is_file()`, `is_dir()`, `exists()` and `is_symlink()` answer False to any OSError, and `os.path.lexists` always has; up to 3.13 they raise a refused stat. So, measured on 50178305: - in a root whose `scripts/` this user can list but not search, `check` called five copies `copy-missing`, and `apply` refused `update-conflict`, asking for them to be put back from history; - a `contracts/` at 000 was `update-pin-missing`, whose remedy adopts the project; - on 3.12 each of those was a traceback. Every guard in update-shape.py that must tell "absent" from "unreadable" now stats the path itself, through a local `_is_a`, `_lexists` and `_is_link` with validate-pins.py's errno set. ENOENT, ENOTDIR, EBADF and ELOOP are absent; any other OSError is raised. `read_pin` and `prepare` raise it rather than refuse it, because the doctor tells the two apart: it names a raised OSError of a file in the root (#847), and reads a Refusal out of `prepare` as CANNOT ANSWER. `main` refuses it as `update-unreadable`: `: cannot be read ()`, exit 2, with validate-pins.py's `_unreadable_file` advice. The guards: - `read_pin`, and a pin at mode 000, which was `yaml-unreadable` with an absolute path and no remedy; - `_classify_row`, and a copy at mode 000, a traceback on every Python; - `root_kind`; - `prepare`: the root's own stat, then a new search check of `/.`, so a root at 600 is named whole and not as a `family.yaml` a project has not got; - `open_upstream`'s three questions; - `merged_into`, where `lexists` read a `shape/` this user cannot search as a merged-away row, whose pin row `apply` would drop; - `find_additions`, which offered `--add` over a file the root may hold; - the README reader, which now reads a refused stat as `unreadable`, never `absent`, and still never raises. `apply` is never left half-written: - `run_validator` refuses an unreadable validator, where 3.14 SKIPPED it; - `cmd_apply` turns any refused read after the first write into `update-unreadable` and rolls back; - the rollback lists an `--add` file whose stat is refused, where 3.14 claimed "exactly as it was". The symlink walk keeps "no link" on an OSError: every caller asks the path itself next. Under 3.14 the doctor's false `DRIFTED (5 copy-missing)` and `DRIFTED (1 copy-missing)` are now `INVALID (pins)`, with no change to the doctor. Tests: - tests/test_update_shape_unreadable.py, new, 43 tests. On 50178305, 35 fail and the 8 controls pass; after, all 43 pass. 21 one-line mutants of the guards, each killed by one of them. - tests/test_update_shape*.py: 335 passed and 3 skipped, and 418 passed, as on 50178305. - test_shape_doctor.py and six test_doctor_* files: 831 passed, 9 skipped. - The trio and the other files that name what changed: 1142 passed, 3 skipped. - test_family.py -k update_shape: 4 passed. - The source-scanning tests: 460 passed. Part of #430 Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_update_shape_unreadable.py | 39 +++++++++++---------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/tests/test_update_shape_unreadable.py b/tests/test_update_shape_unreadable.py index 8dd1a2a4..977362aa 100644 --- a/tests/test_update_shape_unreadable.py +++ b/tests/test_update_shape_unreadable.py @@ -262,20 +262,6 @@ def test_a_pin_this_user_cannot_open_is_named_from_the_root(project): assert "yaml-unreadable" not in said -def test_a_pin_whose_read_is_refused_is_named_on_every_python( - update_shape, project, capsys, monkeypatch): - real = Path.read_text - - def read_text(self, *args, **kwargs): - if self.name == "shape-pin.yaml": - raise PermissionError(errno.EACCES, DENIED, str(self)) - return real(self, *args, **kwargs) - - monkeypatch.setattr(Path, "read_text", read_text) - code, said = run_here(update_shape, capsys, "check", project) - assert_named(code, said, PIN) - - @needs_a_mode def test_a_copy_this_user_cannot_open_is_named(project): """A copy at mode 000 in a readable directory: `is_file()` says yes on @@ -286,18 +272,25 @@ def test_a_copy_this_user_cannot_open_is_named(project): assert_named(code, said, "scripts/bootstrap.py") -def test_a_copy_whose_read_is_refused_is_named_on_every_python( - update_shape, project, capsys, monkeypatch): - real = Path.read_bytes - - def read_bytes(self): - if self.name == "bootstrap.py": +@pytest.mark.parametrize("method, name, named", [ + pytest.param("read_text", "shape-pin.yaml", PIN, id="the-pin"), + pytest.param("read_bytes", "bootstrap.py", "scripts/bootstrap.py", + id="a-copy"), +]) +def test_a_read_refused_is_named_on_every_python( + update_shape, project, capsys, monkeypatch, method, name, named): + """The two cases above in this process, the read refused by hand: the + pin's (`load_yaml`'s `read_text`) and a copy's (`read_bytes`).""" + real = getattr(Path, method) + + def refusing(self, *args, **kwargs): + if self.name == name: raise PermissionError(errno.EACCES, DENIED, str(self)) - return real(self) + return real(self, *args, **kwargs) - monkeypatch.setattr(Path, "read_bytes", read_bytes) + monkeypatch.setattr(Path, method, refusing) code, said = run_here(update_shape, capsys, "check", project) - assert_named(code, said, "scripts/bootstrap.py") + assert_named(code, said, named) # --- the root itself --------------------------------------------------------- From 59b12500db8c57b6eebe6806fc2ac3f6ff79af48 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Mon, 19 Oct 2026 15:59:07 +0000 Subject: [PATCH 3/3] update-shape.py: `prepare` closes the upstream on a refused read too, so an `update-unreadable` from `classify`, `find_additions` or `merged_into` no longer leaves the bare clone of the pin's `owner/repo` behind (#430) Fix round 1 of PR #976, from rv976's review: - MUST 1: `prepare` closed the upstream it had opened on `except Refusal:` alone. The refused reads this PR raises out of `classify`, `find_additions` and `merged_into` went past it to `main`. Without `--upstream`, every `update-unreadable` from there left the bare clone in the temporary directory, for `check` and `apply`. The handler is now `except (Refusal, OSError):`. No other `except Refusal:` holds an upstream or a temporary without a `finally`. - SHOULD 2: the advice split in `unreadable_refusal` is tested. A read refused with EIO or ENAMETOOLONG gets validate-pins.py's OTHER_REASON, word for word, and no `chmod`. - SHOULD 3: `Rollback._remove`'s `elif mode is not None:` is tested. An `--add` file that something else took away before the rollback is not listed as left, as on 50178305. Tests: tests/test_update_shape_unreadable.py, 48 passed (5 new). - The old `except Refusal:` fails both new upstream cases. - Giving every cause the `chmod` advice fails both new advice cases. - `else:` in place of `elif mode is not None:` fails the new rollback case. Lane: openreposhape-2 (openRepoShape-2) Co-Authored-By: Claude Opus 5.5 --- tests/test_update_shape_unreadable.py | 88 ++++++++++++++++++++++++++- update-shape.py | 11 +++- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/tests/test_update_shape_unreadable.py b/tests/test_update_shape_unreadable.py index 977362aa..7bbf9319 100644 --- a/tests/test_update_shape_unreadable.py +++ b/tests/test_update_shape_unreadable.py @@ -42,7 +42,7 @@ import pytest -from conftest import REPO, run_script +from conftest import REPO, load_without_bytecode, run_script #: The skip for a mode root reads through, and the stand-in for a refused #: `stat`, from the file that owns them (#383), as the doctor's and #: `validate-pins.py`'s #430 tests take them. @@ -61,6 +61,8 @@ from shape_materialize import SHAPE_REPOSITORY # noqa: E402 UPDATE = REPO / "update-shape.py" +#: The gate whose words `update-unreadable` gives (`_unreadable_file`, #712). +GATE = REPO / "templates" / "assembly-root" / "scripts" / "validate-pins.py" UNREADABLE = "update-unreadable" DENIED = "Permission denied" PIN = "contracts/shape-pin.yaml" @@ -209,6 +211,42 @@ def test_a_refused_stat_is_named_on_every_python( assert pin_bytes(project) == before +@pytest.fixture +def temporaries(update_shape, monkeypatch, tmp_path): + """Every upstream a run opens, handed a temporary directory of its own + to close, as `open_upstream` hands the bare clone it makes of the pin's + `owner/repo`: `--upstream` here is this checkout, which has none. The + list holds the directories, for a test to ask whether each is gone.""" + real = update_shape.open_upstream + made: list[Path] = [] + + def opening(spec): + upstream = real(spec) + temporary = tmp_path / f"openreposhape-upstream-{len(made)}" + temporary.mkdir() + (temporary / "HEAD").write_bytes(b"ref: refs/heads/main\n") + upstream._temporary = temporary + made.append(temporary) + return upstream + + monkeypatch.setattr(update_shape, "open_upstream", opening) + return made + + +@pytest.mark.parametrize("command", ["check", "apply"]) +def test_a_read_refused_in_prepare_closes_the_upstream( + update_shape, project, capsys, monkeypatch, temporaries, command): + """`prepare` closed the upstream it had opened on a `Refusal` alone, so + a refused `stat` out of `classify` went on to `main`'s refusal past it, + and the bare clone of the pin's `owner/repo` stayed in the temporary + directory, one per run (review of PR #976). It is closed on either.""" + refused_stat(monkeypatch, *SCRIPTS) + code, said = run_here(update_shape, capsys, command, project) + assert_named(code, said, "scripts/validate-pins.py") + assert len(temporaries) == 1, temporaries + assert not os.path.lexists(temporaries[0]), said + + # --- the controls: what is not there is absent, as it always was ------------ @pytest.mark.parametrize("as_314", [False, True], ids=["this-python", @@ -293,6 +331,36 @@ def refusing(self, *args, **kwargs): assert_named(code, said, named) +@pytest.fixture(scope="module") +def gate(): + """`validate-pins.py` of this working tree, loaded by path with no + bytecode written beside it, and `sys.path` put back after it puts its + own `scripts/` on it.""" + with pytest.MonkeyPatch.context() as patch: + patch.setattr(sys, "path", list(sys.path)) + return load_without_bytecode(GATE, "gate_update_shape_unreadable") + + +@pytest.mark.parametrize("cause, reason", [ + pytest.param(errno.EIO, "Input/output error", id="EIO"), + pytest.param(errno.ENAMETOOLONG, "File name too long", + id="ENAMETOOLONG"), +]) +def test_a_read_refused_for_another_reason_gets_the_gates_other_advice( + update_shape, project, gate, cause, reason): + """A read refused, and not for a permission: `chmod` cannot help it, so + the remedy is the gate's `OTHER_REASON`, word for word, and never the + permission advice every other test here reads (review of PR #976).""" + refusal = update_shape.unreadable_refusal( + project, OSError(cause, reason, str(project / "scripts" / "x.py"))) + assert refusal.code == UNREADABLE + assert refusal.detail == f"scripts/x.py: cannot be read ({reason})" + assert refusal.remediation.endswith( + f"never an absence. {gate.OTHER_REASON}{gate.RUN_AGAIN}"), \ + refusal.remediation + assert "chmod" not in refusal.remediation + + # --- the root itself --------------------------------------------------------- @needs_a_mode @@ -586,3 +654,21 @@ def test_a_rollback_that_cannot_stat_what_it_made_says_so(update_shape, assert said.startswith("ROLLED BACK IN PART: the tree is NOT as it was.") assert (f"file docs/new.md, written by --add, was not removed " f"({DENIED}): rm -- {tmp_path / 'docs' / 'new.md'}") in said + + +def test_an_add_file_gone_before_the_rollback_is_not_listed(update_shape, + tmp_path): + """An `--add` file something else took away between the write and the + rollback, a validator or the operator: there is nothing left to remove, + so nothing is listed as left, and the directory made for it goes as it + would have. An `unlink` asked regardless listed it `was not removed (No + such file or directory)` under "the tree is NOT as it was" (review of + PR #976).""" + ledger = update_shape.Rollback(tmp_path) + ledger.write_added(tmp_path / "docs" / "new.md", b"x\n", False) + (tmp_path / "docs" / "new.md").unlink() + said = ledger.undo() + assert said.startswith("ROLLED BACK: every byte this command wrote has " + "been put back"), said + assert " removed docs/ (made to hold an --add file)" in said, said + assert "new.md" not in said, said diff --git a/update-shape.py b/update-shape.py index 313028bf..3144eebd 100755 --- a/update-shape.py +++ b/update-shape.py @@ -2345,7 +2345,8 @@ def prepare(args) -> tuple[Path, dict, list[Row], list[Addition], Upstream, (#847) and reads a `Refusal` from here as the standard unable to answer. The root is asked first: its own `stat` (a folder above it this user cannot search), then whether it can be searched at all - (`_search`).""" + (`_search`). A read refused once the upstream is open closes it + first, as a `Refusal` does.""" root = root_of(args) if not _is_a(root, stat.S_ISDIR): raise Refusal("update-root-missing", f"{root} is not a directory") @@ -2369,7 +2370,13 @@ def prepare(args) -> tuple[Path, dict, list[Row], list[Addition], Upstream, upstream.resolve(pinned) classify(root, rows, upstream, pinned, target, kind) additions = find_additions(root, rows, upstream, target, kind) - except Refusal: + except (Refusal, OSError): + # CLOSED ON A REFUSED READ TOO (#430, review of PR #976): `classify`, + # `find_additions` and `merged_into` raise one out of here for + # `main`, or the doctor, to name, and closed on a `Refusal` alone, + # the bare clone `open_upstream` makes of the pin's `owner/repo` was + # left in the temporary directory by every `update-unreadable` from + # this block. upstream.close() raise return root, pin, rows, additions, upstream, pinned, target, kind