diff --git a/install.sh b/install.sh index cfc84568..c63893b9 100755 --- a/install.sh +++ b/install.sh @@ -152,6 +152,14 @@ checked_repo() { # between the length and the control character, a character a terminal # draws as something other than itself, as the launcher's `ref_fault` has # it (#837). Not shared, because this file arrives alone down a pipe. +# +# THE SHAPE IS GIT'S RULE PART BY PART (#874), as `ref_fault`'s and +# `setup.sh`'s are: no `..`, no empty part between slashes, no part that +# starts with `.` or ends in `.lock`, and no name that ends in `.`, which is +# what `git check-ref-format --branch` asks of each part and not of the whole +# name alone. It gates the fetch: the version is the path of the URL below, +# and a version git would refuse is refused here, before the launcher is +# fetched. checked_version() { local ref="$1" what="$2" misdrawn [ -n "$ref" ] || die "$what is empty. Name a release, such as v1.0.1, or none for the newest." @@ -165,7 +173,7 @@ checked_version() { *[![:print:]]*) die "$what carries a control character, which is not a release tag, branch or commit." ;; esac case "$ref" in - [!A-Za-z0-9]* | *..* | *.lock | *[!A-Za-z0-9._/-]*) die "$what is '$ref', which is not a release tag, branch or commit: a ref starts with a letter or digit, uses only letters, digits, '.', '_', '/', '-', has no '..' and does not end in '.lock'." ;; + [!A-Za-z0-9]* | *..* | */.* | *.lock | *.lock/* | *//* | */ | *. | *[!A-Za-z0-9._/-]*) die "$what is '$ref', which is not a release tag, branch or commit: a ref starts with an ASCII letter or digit, uses only ASCII letters, digits, '.', '_', '/', '-', has no '..' (git reads it as a revision range), no empty part between slashes ('//', or a '/' at the end), no part that starts with '.' or ends in '.lock' (the name git gives the lock file it writes beside a ref), and does not end in '.'." ;; esac } diff --git a/openRepoShape b/openRepoShape index d46f045a..774ff488 100755 --- a/openRepoShape +++ b/openRepoShape @@ -869,6 +869,16 @@ shape_checkout() { # control check, whose `[![:print:]]` is the locale's, so the words do not # change with the locale, and before the shape, which echoes the value. # Those two are `ref_character_fault`'s, below. +# +# THE SHAPE IS GIT'S RULE PART BY PART (#874), as `setup.sh`'s and +# `install.sh`'s are: no `..`, no empty part between slashes, no part that +# starts with `.` or ends in `.lock`, and no name that ends in `.`, which is +# what `git check-ref-format --branch` asks of each part and not of the whole +# name alone. It is asked where a ref is USED, and each use is a gate: a +# record it refuses is refused before any request (`read_recorded_ref`), a +# ref an install would record is refused before anything is placed, and the +# doctor's checkout is refused before `git checkout`. A ref that passes here +# is one `setup.sh` is handed, by name, on its way to `git clone`. ref_fault() { local ref="$1" what="$2" fault if [ -z "$ref" ]; then @@ -882,7 +892,7 @@ ref_fault() { return 0 fi case "$ref" in - [!A-Za-z0-9]* | *..* | *.lock | *[!A-Za-z0-9._/-]*) printf '%s' "$what is '$ref', which is not a branch, tag or commit this tool will pass to \`git checkout\`: a ref starts with a letter or digit, uses only letters, digits, '.', '_', '/', '-', has no '..' and does not end in '.lock'." ;; + [!A-Za-z0-9]* | *..* | */.* | *.lock | *.lock/* | *//* | */ | *. | *[!A-Za-z0-9._/-]*) printf '%s' "$what is '$ref', which is not a branch, tag or commit this tool will pass to \`git checkout\`: a ref starts with an ASCII letter or digit, uses only ASCII letters, digits, '.', '_', '/', '-', has no '..' (git reads it as a revision range), no empty part between slashes ('//', or a '/' at the end), no part that starts with '.' or ends in '.lock' (the name git gives the lock file it writes beside a ref), and does not end in '.'." ;; esac fi } diff --git a/setup.sh b/setup.sh index 034fdbcb..b125b4bc 100755 --- a/setup.sh +++ b/setup.sh @@ -209,12 +209,23 @@ case "$SHAPE_REPO" in *) ;; # a well-formed value; passed to `git clone` unchanged below. esac -# Mirrors `setup-project.py`'s REF_RE: a ref starts with a letter or digit, -# holds only letters, digits, '.', '_', '/', '-', is never a `..` range, -# never ends in `.lock` (git's own lock-file name for one), and is at most -# 255 characters long. Matches nothing when $SHAPE_REF is empty - every -# alternative requires at least one character - so an absent --shape-ref -# passes through untouched. +# Mirrors `setup-project.py`'s `checked_ref`, which asks `REF_RE` and then +# `why_not_a_ref`: a ref starts with an ASCII letter or digit, holds only ASCII +# letters, digits, '.', '_', '/', '-', and is at most 255 characters long +# (`REF_RE`); and it is held to git's own rule PART BY PART (#857's +# `why_not_a_ref`, held here since #874), which `git check-ref-format --branch` +# applies to each part between slashes and not to the whole name alone: never a +# `..` range, no empty part (`//`, or a `/` at the end), no part that starts +# with `.` or ends in `.lock` (git's own lock-file name for one), and never a +# name that ends in `.`. The `case` below has one or two patterns for each, +# between the first-character one and the alphabet one. THIS CHECK GATES THE +# CLONE: it runs only here, on the self-bootstrap path, once +# `is_shape_checkout` has found no checkout to hand over to, and before `git +# clone`, so a ref git refuses is refused here and not by git's own `pathspec +# ... did not match` after the standard has been cloned; `setup-project.py` has +# not run yet to say it first. Matches nothing when $SHAPE_REF is empty - every +# alternative requires at least one character - so an absent --shape-ref passes +# through untouched. # # THREE CHECKS, IN THIS ORDER, AND THE ORDER IS THE POINT. The first two # refuse WITHOUT echoing $SHAPE_REF back, exactly like $OPENREPOSHAPE_REPO's @@ -233,7 +244,7 @@ case "$SHAPE_REF" in *) ;; # printable; the ref-syntax check below still has to pass. esac case "$SHAPE_REF" in -[!A-Za-z0-9]*|*..*|*.lock|*[!A-Za-z0-9._/-]*) die "--shape-ref is '$SHAPE_REF', which is not a branch, tag or commit this tool will pass to \`git checkout\`: a ref starts with a letter or digit, uses only letters, digits, '.', '_', '/', '-', has no '..' and does not end in '.lock'." ;; +[!A-Za-z0-9]*|*..*|*/.*|*.lock|*.lock/*|*//*|*/|*.|*[!A-Za-z0-9._/-]*) die "--shape-ref is '$SHAPE_REF', which is not a branch, tag or commit this tool will pass to \`git checkout\`: a ref starts with an ASCII letter or digit, uses only ASCII letters, digits, '.', '_', '/', '-', has no '..' (git reads it as a revision range), no empty part between slashes ('//', or a '/' at the end), no part that starts with '.' or ends in '.lock' (the name git gives the lock file it writes beside a ref), and does not end in '.'." ;; *) ;; # a valid ref shape; $SHAPE_REF is safe to echo back and check out. esac diff --git a/tests/conftest.py b/tests/conftest.py index 222e4256..9f8dedf6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2398,3 +2398,92 @@ def refusal_detail(stderr: str, code: str) -> list[str]: if lines[at].startswith("Remediation:")] assert ends, stderr return lines[starts[0]:ends[0]] + + +# --- git's rule for a ref, part by part, in the shell copies (#874) ---------- + +#: THE TABLE the three shell copies of the ref rule are held to, one answer +#: each: `setup.sh`'s `--shape-ref`, `install.sh`'s version and `openRepoShape`'s +#: `ref_fault` (`tests/test_setup_sh.py`, `tests/test_install_sh.py` and +#: `tests/test_openreposhape_command.py` read it). A name, and whether git +#: takes it as a branch name. Git asks its rule of each part between slashes, +#: not of the whole name alone: no empty part (`a//b`, `a/b/`), no part that +#: starts with `.` (`a/.b`) or ends in `.lock` (`a.lock/b`), no `..`, and no +#: name that ends in `.`. Each name is spelled with a letter or digit first +#: and letters, digits, `.`, `_`, `/` and `-` after, the characters a copy +#: takes, so what is asked is the shape and nothing else. The old line, +#: `*..* | *.lock`, tested only the whole name: it took the first thirteen +#: names below, which git refuses. +REF_PARTS = [pytest.param(name, takes, id=name) for name, takes in ( + # What the issue names: refused by git, and by no copy before #874. + ("a.lock/b", False), ("x/y.lock/z", False), ("release.lock/v1", False), + ("a/.b", False), ("a/.b/c", False), ("a//b", False), ("a/b/", False), + ("a.lock/", False), ("a.", False), + # rv873's two: a part that starts with `.`, deeper in the name. + ("a/b/.c/d", False), ("x/.y/z/w", False), + # The same kind, and taken by the old line too: a name that ends in `.` + # after a `/`, and a part that is a `.`. + ("a/b.", False), ("a/.", False), + # Refused before #874 as well, and refused still. + ("topic.lock", False), ("a..b", False), ("a/..", False), + ("a/b.lock", False), (".lock", False), ("/a", False), + # What must still pass: the refs this repository's tests and docs hand + # a copy, a commit's 40 hex digits, and workBenches' sentinel. + ("main", True), ("v1.0.0", True), ("v1.1.0", True), + ("topic.locked", True), ("feature/x-y_z", True), + ("0123456789abcdef0123456789abcdef01234567", True), + ("pinned-by-workBenches-no-fetch", True), + # What must still pass, next to the rule's edges: a part that only + # RESEMBLES a refused one, and a `.` that ends a part but not the name. + ("a.lockb", True), ("a.lock-x/b", True), ("a/b.lockb/c", True), + ("a./b", True), ("a/lock/b", True), ("refs/tags/v1.0", True), + ("v1.0.0-rc.1", True), + # What must still pass, with a part that starts with an upper-case + # letter, `_` or `-`: git takes each, and a copy that refused one would + # be refusing what git takes (`Feature/ABC-123`, `fix/_tmp`, `fix/-wip`). + ("Feature/ABC-123", True), ("fix/_tmp", True), ("fix/-wip", True), +)] + +#: THE ONE SENTENCE each copy's refusal ends with, word for word the same in +#: all three (#874): the rule it applied, in `setup-project.py`'s words where +#: they fit (`why_not_a_ref`, #857). The words say ASCII because the patterns +#: are, as `setup-project.py`'s `REF_RE` words do: "letters" alone describe +#: `Caf\xe9` as a value that passes (rv811, rv874). +REF_RULE_WORDS = ( + "a ref starts with an ASCII letter or digit, uses only ASCII letters, " + "digits, '.', '_', '/', '-', has no '..' (git reads it as a revision " + "range), no empty part between slashes ('//', or a '/' at the end), no " + "part that starts with '.' or ends in '.lock' (the name git gives the " + "lock file it writes beside a ref), and does not end in '.'.") + + +def git_takes_ref(name: str) -> bool: + """Whether the git that runs this suite takes `name` as a branch name. + ASKED of `git check-ref-format --branch`, never copied, so a git that + changes its mind changes the answer here and a table row is the one thing + to fix (#874).""" + proc = subprocess.run(["git", "check-ref-format", "--branch", name], + capture_output=True, text=True, check=False) + return proc.returncode == 0 + + +def assert_a_copy_answers_as_git_does(copy: str, name: str, takes: bool, + copy_takes: bool) -> None: + """THE ONE ANSWER (#874): git, the table's row and the shell copy `copy` + all take `name` or all refuse it. Git is asked first, so a row that + no longer says what this git says is named as the row and not as the + copy.""" + asked = git_takes_ref(name) + assert asked == takes, ( + f"the table says git {'takes' if takes else 'refuses'} {name!r}, but " + f"this git {'takes' if asked else 'refuses'} it") + assert copy_takes == asked, ( + f"{copy} {'takes' if copy_takes else 'refuses'} {name!r}, and git " + f"{'takes' if asked else 'refuses'} it") + + +def assert_the_refusal_states_the_ref_rule(said: str, opening: str) -> None: + """A copy's refusal opens with its own words and the name (`opening`) and + goes on to `REF_RULE_WORDS`, the whole rule it applied.""" + assert opening in said, said + assert REF_RULE_WORDS in said, said diff --git a/tests/test_install_sh.py b/tests/test_install_sh.py index de493f46..d3e6e4a5 100644 --- a/tests/test_install_sh.py +++ b/tests/test_install_sh.py @@ -45,7 +45,9 @@ import pytest -from conftest import BASH, BASH_SKIP, REPO, bash_path +from conftest import (BASH, BASH_SKIP, REF_PARTS, REPO, + assert_a_copy_answers_as_git_does, + assert_the_refusal_states_the_ref_rule, bash_path) from test_openreposhape_command import (BIN_DIR_SPELLINGS, COMMAND, COMMAND_REFUSAL_ENDS, CYGPATH_STATES, HOLDS_A_COLON, @@ -427,6 +429,32 @@ def test_a_version_that_is_not_a_ref_is_refused_before_any_request( assert "a" * 256 not in result.stderr +@pytest.mark.parametrize("name, takes", REF_PARTS) +def test_the_installer_takes_a_version_as_git_does_before_any_request( + net, name, takes): + """GIT'S RULE FOR A REF IS PER PART, AND THE INSTALLER HOLDS IT BEFORE + THE FETCH (#874). The version is the path of the URL the launcher is + fetched from, and its shape `case` tested only the whole name, so + `a.lock/b`, `a/.b` and `a//b` were asked of the network. Each name of the + table goes to the installer and, through `git check-ref-format + --branch`, to git, and the two must answer alike. + + The fakes serve a release's files at `v9.9.9` only, so a version the + installer takes asks curl for the launcher and is told 404, and one it + refuses asks nothing: it has taken the version when a call was logged. + """ + result = net.run(name) + asked = net.calls() != [] + assert_a_copy_answers_as_git_does("install.sh", name, takes, asked) + if asked: + return + net.refused(result) + assert_the_refusal_states_the_ref_rule( + result.stderr, + f"REFUSED: the version is '{name}', which is not a release tag, " + "branch or commit: ") + + def test_a_failed_fetch_names_its_url_and_installs_nothing(net): """An error from the server asks whether the release exists.""" del net.config["files"][raw("openRepoShape")] diff --git a/tests/test_openreposhape_command.py b/tests/test_openreposhape_command.py index df5a448c..99bac4e9 100644 --- a/tests/test_openreposhape_command.py +++ b/tests/test_openreposhape_command.py @@ -35,7 +35,10 @@ import pytest -from conftest import BASH, BASH_SKIP, REPO, bash_path, git, native_path +from conftest import (BASH, BASH_SKIP, REF_PARTS, REPO, + assert_a_copy_answers_as_git_does, + assert_the_refusal_states_the_ref_rule, bash_path, git, + native_path) COMMAND = REPO / "openRepoShape" SETUP = REPO / "setup.sh" @@ -2418,13 +2421,15 @@ def test_version_prints_a_path_a_utf_8_locale_prints(name): OTHER = "other" -def install_at(tmp_path, ref=None, command=COMMAND, opt_in=True): +def install_at(tmp_path, ref=None, command=COMMAND, opt_in=True, + env_also=None): """`--install` from `command` into `/bin`, with `$OPENREPOSHAPE_REF` set to `ref` when one is given, and asked to record - it (`$OPENREPOSHAPE_RECORD_REF=1`) unless `opt_in` is false; the result, - and the path of the command it places.""" + it (`$OPENREPOSHAPE_RECORD_REF=1`) unless `opt_in` is false, and with + `env_also` added to the environment; the result, and the path of the + command it places.""" bin_dir = tmp_path / "bin" - env = {"OPENREPOSHAPE_BIN_DIR": bash_path(bin_dir)} + env = {"OPENREPOSHAPE_BIN_DIR": bash_path(bin_dir), **(env_also or {})} if ref is not None: env["OPENREPOSHAPE_REF"] = ref if opt_in: @@ -2528,6 +2533,45 @@ def test_a_ref_that_is_not_one_is_refused_before_anything_is_placed( assert not installed.parent.exists(), "a refused install placed something" +@pytest.mark.parametrize("name, takes", REF_PARTS) +def test_an_install_records_a_ref_as_git_takes_it_and_refuses_the_rest( + tmp_path, name, takes): + """GIT'S RULE FOR A REF IS PER PART, AND `ref_fault`'S SHAPE LINE HOLDS + IT (#874). It tested only the whole name (`*..*`, `*.lock`), so an + install asked to record `a.lock/b`, `a/.b` or `a//b` wrote it beside the + command, where every later run hands it to the fetch of `setup.sh` and on + to `--shape-ref`. Each name of the table goes to the launcher and, + through `git check-ref-format --branch`, to git, and the two must answer + alike. + + Reached through `--install` with the opt-in, since `--version` asks a ref + only its characters (#866): a ref `ref_fault` takes is recorded, exit 0, + and one it refuses exits 2 with the rule it applied and places nothing. + A `gh` and a `curl` that log and fail stand first on `$PATH`: no `curl` is + run and no `gh` but the `gh auth status` that the machine report of an + install that placed the command asks. + """ + network = tmp_path / "network.log" + result, installed = install_at( + tmp_path, name, env_also=fake_github(tmp_path, (), log=network)) + asked = network.read_text(encoding="utf-8").splitlines() \ + if network.exists() else [] + assert [call for call in asked if call != "gh auth status"] == [] + recorded = record_of(installed) + assert_a_copy_answers_as_git_does("openRepoShape", name, takes, + result.returncode == 0) + if takes: + assert recorded.read_text(encoding="utf-8") == f"{name}\n" + return + assert result.returncode == 2, result.stdout + result.stderr + assert not installed.parent.exists(), "a refused install placed something" + assert asked == [], "a refused install asked for something" + assert_the_refusal_states_the_ref_rule( + result.stderr, + f"REFUSED: $OPENREPOSHAPE_REF is '{name}', which is not a branch, " + "tag or commit this tool will pass to `git checkout`: ") + + @pytest.mark.parametrize("text, shown", [ pytest.param(b"", "is empty", id="empty"), pytest.param(b"-x\n", "is '-x'", id="an-option"), diff --git a/tests/test_setup_sh.py b/tests/test_setup_sh.py index e9c6cf63..40ba53cc 100644 --- a/tests/test_setup_sh.py +++ b/tests/test_setup_sh.py @@ -55,6 +55,11 @@ from conftest import (STEP_FOUR_WORDS, assert_step_four_speaks_in_the_plans_words) +from conftest import (REF_PARTS, assert_a_copy_answers_as_git_does, + assert_the_refusal_states_the_ref_rule) + +from test_openreposhape_command import fake_github + SETUP = REPO / "setup.sh" DEGRADE_LINE = "authority is not wallet-carried in this org" @@ -917,6 +922,51 @@ def test_a_shape_ref_that_is_not_a_ref_is_refused_by_the_shim(tmp_path, ref): assert "--shape-ref" in result.stderr +@pytest.mark.parametrize("name, takes", REF_PARTS) +def test_the_shim_takes_a_shape_ref_as_git_does_before_any_clone( + tmp_path, name, takes): + """GIT'S RULE FOR A REF IS PER PART, AND THE SHIM HOLDS IT BEFORE THE + CLONE (#874). Its shape `case` tested only the whole name (`*..*`, + `*.lock`), so `a.lock/b`, `a/.b` and `a//b` printed `(0) self-bootstrap`, + cloned the standard, and ended in git's own `pathspec ... did not match`. + Each name of the table goes to the shim and, through `git + check-ref-format --branch`, to git, and the two must answer alike. + + `OPENREPOSHAPE_REPO` is a path nothing is at, so a ref the shim takes + goes on to `git clone` and fails there, offline and at once: it has + taken the ref when it printed `(0) self-bootstrap`. A ref it refuses + prints neither that nor a `git clone` line, exits 2 with the rule it + applied, and leaves nothing in `$TMPDIR`. A `gh` and a `curl` that log + and fail stand first on `$PATH`, and neither is called. + """ + outside = make_outside_checkout(tmp_path) + temporary = tmp_path / "tmp" + temporary.mkdir() + network = tmp_path / "network.log" + result = run_bare_setup(outside / "setup.sh", + "--org", "demoorg", "--project", "Sample", "--yes", + "--shape-ref", name, + "--local-remote-dir", str(tmp_path / "remotes"), + cwd=outside, + extra_env={"OPENREPOSHAPE_REPO": + str(tmp_path / "no-standard-here"), + "TMPDIR": str(temporary), + **fake_github(tmp_path, (), + log=network)}) + assert not network.exists(), network.read_text(encoding="utf-8") + took = "(0) self-bootstrap" in result.stdout + assert_a_copy_answers_as_git_does("setup.sh", name, takes, took) + assert list(temporary.iterdir()) == [], "a shape checkout was left behind" + if took: + return + assert result.returncode == 2, result.stderr + result.stdout + assert "git clone" not in result.stdout + assert_the_refusal_states_the_ref_rule( + result.stderr, + f"REFUSED: --shape-ref is '{name}', which is not a branch, tag or " + "commit this tool will pass to `git checkout`: ") + + @pytest.mark.parametrize("overlong", [ pytest.param("a" * 256, id="256-legal-shape"), pytest.param("a" * 299 + " ", id="300-illegal-shape"),