diff --git a/adopt-project.py b/adopt-project.py index c5051e02..e7702d92 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -170,6 +170,7 @@ import contextlib import datetime as _dt import itertools +import json import os import re import shlex @@ -8957,11 +8958,78 @@ def _create_leg_remotes(plan: Plan, names: dict, repositories: dict, "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN " "PAT (contents:read on the legs, fallback) — or the `validate` " "check cannot check them out.") + # The secrets that credential is kept in are the ROOT's, where the + # workflow runs, so the Free-plan hint goes by the root (#602). + _free_plan_hint_by_the_root(plan, repositories["assembly"]) + + +#: What `execute` prints where GitHub would not say what visibility the +#: adopted root has (#602): the question it could not answer, what the +#: Free-plan hint would then say, and how to answer it. `{root}` is the root. +ROOT_VISIBILITY_UNREAD = ( + "NOTE this run could not read {root}'s visibility from GitHub, so it " + "cannot say whether the Free-plan secrets hint applies. It applies when " + "{root} is private or internal and its organisation is on the GitHub Free " + "plan: organisation secrets are then not delivered to it, so org-level " + "SHAPE_LEGS_APP_ID / SHAPE_LEGS_APP_PRIVATE_KEY resolve to EMPTY in its " + "`validate` workflow and must be set as REPOSITORY secrets on {root} " + "instead. `gh repo view {root} --json visibility` says which it is.") + + +def _free_plan_hint_by_the_root(plan: Plan, root: str) -> None: + """The Free-plan secrets hint, BY THE ADOPTED ROOT (#602). + + The `validate` workflow that reads SHAPE_LEGS_APP_ID and + SHAPE_LEGS_APP_PRIVATE_KEY runs in the assembly root, and on the Free + plan an organisation secret is delivered only to a public repository. An + adopted root keeps the visibility it had, which is not the plan's + `visibility:` (the two new legs'), so the hint went by the wrong + repository: a public source adopted with private legs was told its + organisation secrets "resolve to EMPTY", and a private one was told the + legs were why. It is printed now only when GitHub says the root is + private or internal, naming the root, as `scaffold-project.py`'s + `_private_leg_note` has gated it since #542. Asked only once a leg is + private or internal: a public leg needs no credential, so no secret + decides whether the `validate` check reads it. + + A ROOT WHOSE VISIBILITY CANNOT BE READ IS NOT GUESSED. Read as private, + a public root would be given a false hint; read as public, a private one + would lose a true one with nothing said. `ROOT_VISIBILITY_UNREAD` says + so instead, with the condition and the command that settles it. It asks + nothing of the organisation's plan: `free_plan_secret_hint` answers None + alike for a paid plan and for one it could not read, so its answer could + not tell "the hint does not apply" from a second unknown. It is a NOTE + and not a refusal, because both legs exist by now and the run goes on to + push them. + """ + seen = _root_visibility(root) + if seen in ("private", "internal"): hint = free_plan_secret_hint( - plan.get("org", ""), repositories["assembly"], - f"{repositories['spec']} and {repositories['code']} are") + plan.get("org", ""), root, f"{root} is") if hint: print(hint) + elif seen is None: + print(ROOT_VISIBILITY_UNREAD.format(root=root)) + + +def _root_visibility(repository: str) -> str | None: + """`repository`'s visibility on GitHub, one of `VISIBILITY_CHOICES`, or + None when it cannot be read: `gh repo view` failed (a repository the + token cannot see, a rate limit, the network) or could not be started, + or answered anything but one of GitHub's three visibilities. Never asked + under `--local-remote-dir`, whose root is the source on disk.""" + try: + answer = run(["gh", "repo", "view", repository, "--json", + "visibility"]) + except (CommandFailed, OSError): + return None + try: + value = json.loads(answer) + except ValueError: + return None + value = value.get("visibility") if isinstance(value, dict) else None + seen = value.lower() if isinstance(value, str) else None + return seen if seen in VISIBILITY_CHOICES else None def _create_leg_remote(plan: Plan, names: dict, repositories: dict, diff --git a/scaffold-project.py b/scaffold-project.py index 271dd2f0..5a73dd13 100755 --- a/scaffold-project.py +++ b/scaffold-project.py @@ -1373,7 +1373,11 @@ def _private_leg_note(args, repositories: dict, local: bool, public root, new or reused, receives the organisation's secrets, and a hint that they "resolve to EMPTY" is false of it however private a leg is. The per-leg NOTE above stays, since a public root still needs a - credential to read a private leg. + credential to read a private leg. AND IT NAMES THE ROOT (#602): it said + the closed legs were private, so a public run over a reused private root + was told the secrets resolve to EMPTY because a leg is private, when the + reason is that the root is. `adopt-project.py` goes by its root the same + way. """ closed = [role for role in ("spec", "code") if standing[role] in ("private", "internal")] @@ -1393,7 +1397,8 @@ def _private_leg_note(args, repositories: dict, local: bool, f"check cannot check {them} out.") root_is_closed = standing["assembly"] in ("private", "internal") hint = root_is_closed and free_plan_secret_hint( - args.org, repositories["assembly"], f"{who} {are}") + args.org, repositories["assembly"], + f"{repositories['assembly']} is") if hint: print(hint) diff --git a/tests/test_adopt_free_plan_hint_root.py b/tests/test_adopt_free_plan_hint_root.py new file mode 100644 index 00000000..24b5009b --- /dev/null +++ b/tests/test_adopt_free_plan_hint_root.py @@ -0,0 +1,256 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`execute`'s Free-plan secrets hint goes by the ADOPTED ROOT, and names it +as its reason (#602). + +WHAT WAS WRONG. `_create_leg_remotes` printed `free_plan_secret_hint` on the +plan's `visibility`, which is the two new LEGS', and named the legs as its +subject. The `validate` workflow that reads `SHAPE_LEGS_APP_ID` and +`SHAPE_LEGS_APP_PRIVATE_KEY` runs in the assembly root, and an adopted root +keeps the visibility it had (`templates/assembly-root/project.yaml`). On the +Free plan an organisation secret is delivered only to a public repository, so +a public source adopted with private legs was told its organisation secrets +"resolve to EMPTY inside the workflow", which is false of it, and a private +root was told the legs were the reason, which they are not. The scaffold has +gated the hint on its root since #542; both tools now name the root too. + +WHAT IS PINNED. The per-leg NOTE is as it was: a public root still needs a +credential to read a private leg. The hint is printed only when the root, as +GitHub reports it, is private or internal, with the root as its subject. A +root whose visibility cannot be read is neither guessed closed (a false hint) +nor guessed public (a true one hidden): the run says it could not read it, +what the hint would then say, and how to look. `--local-remote-dir` has no +GitHub root, and asks and prints nothing about one. + +`gh` is `test_scaffold_reuse_seeded_legs.py`'s stub, first on PATH: it knows +the repositories it is given, each at its visibility, answers `repo create`, +and names the organisation's plan. Nothing reaches GitHub. It is a `#!` +script, so the runs that start it are `WINDOWS_SKIP`ped. +""" + +from __future__ import annotations + +import importlib.util + +import pytest + +from conftest import ADOPT, ORG, WINDOWS_SKIP +from test_scaffold_reuse_seeded_legs import fake_gh + +NORTHWIND = "Northwind" +ROOT = f"{ORG}/{NORTHWIND}" +NAMES = {"assembly": NORTHWIND, "spec": f"{NORTHWIND}-spec", + "code": f"{NORTHWIND}-code"} +SPEC_REPO, CODE_REPO = f"{ROOT}-spec", f"{ROOT}-code" + +#: The per-leg NOTE, which #602 leaves as it was. +LEG_NOTE = (f"NOTE {SPEC_REPO} and {CODE_REPO} are private: give {ROOT} a " + "way to read them — a GitHub App (SHAPE_LEGS_APP_ID + " + "SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or a SHAPE_LEGS_TOKEN PAT " + "(contents:read on the legs, fallback) — or the `validate` check " + "cannot check them out.") +#: `FREE_PLAN_HINT`'s first words, and its subject as it is put before +#: " private, so": the root now, the two legs on main. +HINT = f"NOTE {ORG} is on the GitHub FREE plan" +ROOT_SUBJECT = f"{ROOT} is private, so org-level" +LEG_SUBJECT = f"{SPEC_REPO} and {CODE_REPO} are private, so" +#: What a run that could not read the root's visibility says instead. +UNREAD = (f"NOTE this run could not read {ROOT}'s visibility from GitHub, " + "so it cannot say whether the Free-plan secrets hint applies. It " + f"applies when {ROOT} is private or internal and its organisation " + "is on the GitHub Free plan:") +LOOK = f"`gh repo view {ROOT} --json visibility` says which it is." +VIEW_ROOT = f"repo view {ROOT} --json visibility" +#: What `gh` says, on stderr and with exit 1, when its token is refused: an +#: answer of another kind than the not-found one the stub gives a repository +#: it does not know (`GraphQL: Could not resolve to a Repository`). +BAD_CREDENTIALS = ( + "HTTP 401: Bad credentials (https://api.github.com/graphql)\n" + "Try authenticating with: gh auth login") + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location("adopt_free_plan_hint_root", + ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def create_the_legs(adopter, monkeypatch, capsys, tmp_path, repos: dict, *, + plan: str | None = "free", visibility: str = "private", + local: bool = False, + view_errors: dict | None = None) -> tuple[str, list]: + """`_create_leg_remotes` for `NAMES` under `ORG`, as `cmd_execute` calls + it, with the stub `gh` knowing `repos` (`{name: VISIBILITY}`), naming + `plan` as the organisation's plan and failing `repo view` of each name in + `view_errors` (`{name: stderr text}`). Returns what it printed and the + `gh` calls it made, each as its arguments joined by spaces.""" + env, log = fake_gh(tmp_path, repos, plan=plan, view_errors=view_errors) + for key, value in env.items(): + monkeypatch.setenv(key, value) + if local: + urls = {role: str(tmp_path / "remotes" / f"{name}.git") + for role, name in NAMES.items()} + else: + urls = {role: f"https://github.com/{ORG}/{name}.git" + for role, name in NAMES.items()} + adopter._create_leg_remotes( + {"org": ORG, "visibility": visibility}, NAMES, + {role: f"{ORG}/{name}" for role, name in NAMES.items()}, urls, + "main", local, tmp_path / "work") + out, _err = capsys.readouterr() + return out, log.read_text(encoding="utf-8").splitlines() + + +@WINDOWS_SKIP +def test_a_public_root_over_private_legs_gets_the_note_and_no_hint( + adopter, monkeypatch, capsys, tmp_path): + """The issue's own case: a public source adopted with private legs. The + root's workflow is given the organisation's secrets on the Free plan, so + the hint would be false of it; the legs still need a credential, so the + NOTE stays. On main the hint was printed, naming the legs.""" + out, calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + {NORTHWIND: "PUBLIC"}) + assert LEG_NOTE in out, out + assert "resolve to EMPTY" not in out, out + assert UNREAD not in out, out + assert calls.count(VIEW_ROOT) == 1, calls + + +@WINDOWS_SKIP +@pytest.mark.parametrize("seen", ["PRIVATE", "INTERNAL"]) +def test_a_closed_root_gets_the_hint_naming_the_root( + adopter, monkeypatch, capsys, tmp_path, seen): + """A private or internal root on the Free plan gets the hint, once, with + the root as the repository that is private and the one its `gh secret + set` lines name. On main the hint named the legs as the reason. + + `FREE_PLAN_HINT`'s word is "private" for an internal root too, as it is + in the scaffold's: an internal repository belongs to an enterprise's + organisation, whose plan is not `free`, so outside a stub the two never + meet. The row is here for the gate, which counts internal as closed.""" + out, calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + {NORTHWIND: seen}) + assert LEG_NOTE in out, out + assert HINT in out, out + assert ROOT_SUBJECT in out, out + assert LEG_SUBJECT not in out, out + assert f"--repo {ROOT} --body" in out, out + assert out.count("resolve to EMPTY") == 1, out + assert UNREAD not in out, out + assert out.index(LEG_NOTE) < out.index(HINT), "the NOTE comes first" + assert calls.count(VIEW_ROOT) == 1, calls + + +@WINDOWS_SKIP +@pytest.mark.parametrize("repos, errors", [ + ({}, None), + ({NORTHWIND: "PRIVATE"}, {NORTHWIND: BAD_CREDENTIALS}), +], ids=["not-found", "http-401"]) +def test_a_root_whose_visibility_cannot_be_read_is_named_not_guessed( + adopter, monkeypatch, capsys, tmp_path, repos, errors): + """`gh repo view` of the root fails, by either answer `gh` gives. The + stub does not know the root (`not-found`, as `gh` says of a repository it + cannot see), or knows it, private, and answers `HTTP 401: Bad credentials` + (`http-401`: the token was refused, so a private root's true hint is the + one hidden). Only the first is gh's word for absent, and neither is + read as a visibility. Taking the root as private would print a hint that + is false of a public root; taking it as public would hide one that is + true of a private root. The run says which question it could not answer, + what the hint would say, and how to answer it, and prints the hint + itself for no root, and it does not stop: both legs exist by now. On + main the hint was printed, naming the legs, on the plan's visibility + alone.""" + out, calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + repos, view_errors=errors) + assert LEG_NOTE in out, out + assert UNREAD in out, out + assert LOOK in out, out + assert (f"must be set as REPOSITORY secrets on {ROOT} instead" + in out), out + assert HINT not in out, out + assert LEG_SUBJECT not in out, out + assert out.index(LEG_NOTE) < out.index(UNREAD), "the NOTE comes first" + assert calls.count(VIEW_ROOT) == 1, calls + + +@WINDOWS_SKIP +def test_a_closed_root_on_a_paid_plan_gets_no_hint(adopter, monkeypatch, + capsys, tmp_path): + """The control: on a paid plan the organisation's secrets reach a private + repository, so a private root gets the NOTE and no hint, as on main.""" + out, _calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + {NORTHWIND: "PRIVATE"}, plan="team") + assert LEG_NOTE in out, out + assert "resolve to EMPTY" not in out, out + assert UNREAD not in out, out + + +@WINDOWS_SKIP +def test_public_legs_ask_nothing_of_the_root(adopter, monkeypatch, capsys, + tmp_path): + """Public legs need no credential, so the workflow's secrets decide + nothing: no NOTE, no hint, and the root's visibility is not asked.""" + out, calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + {NORTHWIND: "PRIVATE"}, visibility="public") + assert "NOTE" not in out, out + assert not [call for call in calls if call.startswith("repo view")], calls + + +@WINDOWS_SKIP +def test_a_local_remote_dir_has_no_github_root_to_ask(adopter, monkeypatch, + capsys, tmp_path): + """Under `--local-remote-dir` the root is the source on disk and the legs + are bare repositories beside it: no repository has a visibility, and no + workflow is given any secret. Nothing is asked of `gh` and no NOTE is + printed, as on main.""" + out, calls = create_the_legs(adopter, monkeypatch, capsys, tmp_path, + {NORTHWIND: "PRIVATE"}, local=True) + assert "NOTE" not in out, out + assert calls == [], calls + for name in (NAMES["spec"], NAMES["code"]): + assert (tmp_path / "remotes" / f"{name}.git").is_dir() + + +@pytest.mark.parametrize("answer, seen", [ + ('{"visibility":"PRIVATE"}', "private"), + ('{"visibility":"public"}', "public"), + ('{"visibility":"INTERNAL"}', "internal"), + ('{"visibility":"SECRET"}', None), + ('{"visibility":null}', None), + ("{}", None), + ('["PRIVATE"]', None), + ("PRIVATE", None), + ("", None), +], ids=["private", "public", "internal", "unknown-word", "null", "no-field", + "not-an-object", "not-json", "empty"]) +def test_the_root_is_one_of_the_three_visibilities_or_unread( + adopter, monkeypatch, answer, seen): + """What `gh repo view --json visibility` answers is read as GitHub's + three visibilities, in any case, and anything else as unread: never as a + visibility it does not name.""" + asked: list = [] + + def gh(args, *_rest, **_keywords): + asked.append(args) + return answer + + monkeypatch.setattr(adopter, "run", gh) + assert adopter._root_visibility(ROOT) == seen + assert asked == [["gh", "repo", "view", ROOT, "--json", "visibility"]] + + +@pytest.mark.parametrize("failure", ["command", "no-gh"]) +def test_a_gh_that_fails_or_cannot_run_is_unread(adopter, monkeypatch, + failure): + """A failed `gh repo view` (a repository it cannot see, a rate limit, + the network) and a `gh` that cannot be started are unread, not a + refusal: both legs exist by now, and the run goes on to push them.""" + def gh(args, *_rest, **_keywords): + if failure == "command": + raise adopter.CommandFailed(args, None, 1, "HTTP 403") + raise FileNotFoundError(2, "No such file or directory", "gh") + + monkeypatch.setattr(adopter, "run", gh) + assert adopter._root_visibility(ROOT) is None diff --git a/tests/test_scaffold_reuse_seeded_legs.py b/tests/test_scaffold_reuse_seeded_legs.py index 4f64df28..7d401efe 100644 --- a/tests/test_scaffold_reuse_seeded_legs.py +++ b/tests/test_scaffold_reuse_seeded_legs.py @@ -30,6 +30,7 @@ import http.server import importlib.util +import json import os import re import stat @@ -308,7 +309,10 @@ def test_a_leg_somebody_else_made_is_refused(tmp_path): #: `FAKE_GH_FULL` names it too (`org/name,...`), when its commit listing has #: one commit; it answers `repo create` and `repo edit` with success, answers #: `api orgs/` with `FAKE_GH_PLAN` when that is set (the Free-plan hint -#: asks it for `.plan.name`), and logs every call. +#: asks it for `.plan.name`), answers `repo view` of each repository that +#: `FAKE_GH_VIEW_ERRORS` (JSON, `{"org/name": "stderr text"}`) names with that +#: text and exit 1, known or not (a 401, where an unknown one is not-found), +#: and logs every call. FAKE_GH = textwrap.dedent("""\ #!/usr/bin/env python3 import json @@ -321,9 +325,13 @@ def test_a_leg_somebody_else_made_is_refused(tmp_path): KNOWN = dict(pair.split("=") for pair in os.environ["FAKE_GH_REPOS"].split(",") if pair) FULL = os.environ.get("FAKE_GH_FULL", "").split(",") + ERRORS = json.loads(os.environ.get("FAKE_GH_VIEW_ERRORS", "{}")) if argv[:2] == ["repo", "view"]: repo = argv[2] + if repo in ERRORS: + print(ERRORS[repo], file=sys.stderr) + sys.exit(1) if repo not in KNOWN: print("GraphQL: Could not resolve to a Repository", file=sys.stderr) sys.exit(1) @@ -348,12 +356,14 @@ def test_a_leg_somebody_else_made_is_refused(tmp_path): """) -def fake_gh(tmp_path, repos: dict, full=(), - plan: str | None = None) -> tuple[dict, Path]: +def fake_gh(tmp_path, repos: dict, full=(), plan: str | None = None, + view_errors: dict | None = None) -> tuple[dict, Path]: """`FAKE_GH` first on PATH, knowing `repos` (`{name: VISIBILITY}`, each - under `ORG`) and holding commits in each of `full`, and naming `plan` as - the organisation's GitHub plan when one is given: the environment for the - run, and the log of every call. Nothing it answers reaches GitHub.""" + under `ORG`) and holding commits in each of `full`, naming `plan` as the + organisation's GitHub plan when one is given, and failing `repo view` of + each name in `view_errors` (`{name: stderr text}`, each under `ORG`) with + that text: the environment for the run, and the log of every call. + Nothing it answers reaches GitHub.""" fakebin = tmp_path / "fakebin" fakebin.mkdir() gh = fakebin / "gh" @@ -368,6 +378,9 @@ def fake_gh(tmp_path, repos: dict, full=(), "FAKE_GH_FULL": ",".join(f"{ORG}/{name}" for name in full)} if plan is not None: env["FAKE_GH_PLAN"] = plan + if view_errors: + env["FAKE_GH_VIEW_ERRORS"] = json.dumps( + {f"{ORG}/{name}": text for name, text in view_errors.items()}) return env, log @@ -471,8 +484,12 @@ def test_the_visibility_text_follows_each_leg_as_it_stands( #: runs. On the Free plan an organisation secret reaches only a public #: repository, so the hint is true when the root is private or internal and #: false when it is public, whatever a leg is. `None` is no hint; otherwise -#: the legs it names, as `FREE_PLAN_HINT` puts them before " private, so". -BOTH_LEGS = f"{SPEC_REPO} and {CODE_REPO} are" +#: the repository it names as the reason, as `FREE_PLAN_HINT` puts it before +#: " private, so": the root (#602), where it named the closed legs. +ROOT_IS = f"{ORG}/{PROJECT} is" +#: What the hint named as its reason before #602, which no row may print. +LEG_SUBJECTS = (f"{SPEC_REPO} is", f"{CODE_REPO} is", + f"{SPEC_REPO} and {CODE_REPO} are") @WINDOWS_SKIP @@ -494,16 +511,17 @@ def test_the_visibility_text_follows_each_leg_as_it_stands( None), # And the other way: a public run over a root that is private on GitHub. ({PROJECT: "PRIVATE", f"{PROJECT}-code": "PRIVATE"}, - ("--visibility", "public"), "free", f"{CODE_REPO} is"), + ("--visibility", "public"), "free", ROOT_IS), # An internal root is closed too, and its run creates the spec leg # internal. ({PROJECT: "INTERNAL", f"{PROJECT}-code": "PRIVATE"}, (), "free", - BOTH_LEGS), + ROOT_IS), # Controls: a private root with both legs private, reused and created - # new, says it as it always did; and a paid plan says nothing. + # new, gets the hint as before, with the root as its subject (#602 moved + # only that); and a paid plan says nothing. ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PRIVATE", - f"{PROJECT}-code": "PRIVATE"}, (), "free", BOTH_LEGS), - ({}, (), "free", BOTH_LEGS), + f"{PROJECT}-code": "PRIVATE"}, (), "free", ROOT_IS), + ({}, (), "free", ROOT_IS), ({PROJECT: "PRIVATE", f"{PROJECT}-spec": "PRIVATE", f"{PROJECT}-code": "PRIVATE"}, (), "team", None), ], ids=["public-run-new-root", "public-root-reused", @@ -520,7 +538,9 @@ def test_the_free_plan_hint_goes_by_the_root_as_it_stands( in every case a leg is closed, since a public root still needs a credential to read a private leg; the hint only when the root, as it stands, is private or internal: GitHub's for a reused root, else the - run's.""" + run's. #602: and it names the root as the repository that is private, + where it named the closed legs, which are not why the secrets resolve to + EMPTY.""" env, _log = fake_gh(tmp_path, repos, plan=plan) result = on_github(tmp_path, "--reuse-empty-repo", *flags, env=env) assert result.returncode == 0, result.stderr + result.stdout @@ -532,6 +552,8 @@ def test_the_free_plan_hint_goes_by_the_root_as_it_stands( assert f"NOTE {ORG} is on the GitHub FREE plan" in printed, printed assert f"{named} private, so org-level" in printed, printed assert printed.count("resolve to EMPTY") == 1, printed + for subject in LEG_SUBJECTS: + assert f"{subject} private, so" not in printed, printed # --- a leg that gh sees and git cannot read (#542) --------------------------