From c3cf51544be346833bad06e1ec3fc90dff74078b Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 20 Oct 2026 21:01:43 +0000 Subject: [PATCH 1/3] the family root's siblings.py: with a token in use, the clone and the fetch that carry it run in bootstrap.py's `untraced()` environment, so `make siblings` and `family.py siblings` write the token to no trace the caller asks git for (#1055) `make siblings` and `family.py siblings` gave the credential's `-c url.https://x-access-token:@github.com/.insteadOf=...` rewrite, in argv, to two gits that ran in the caller's own environment: the clone and the fetch in `place`. A trace writes a command's arguments, so every trace2 target the caller named (the `GIT_TRACE2*` variables, or `trace2.eventTarget`, `normalTarget` and `perfTarget` in the global or the system config) held the token in the `start` event of each, one line per member. `GIT_TRACE_PERFORMANCE`'s file held it too, on git 2.43.0 and 2.47.1 alike. `scrub` kept it out of the output, and only that part held. Found by lane 5's review of #1045 (rv996) and measured in #1055. - `rewrite_environment(token)` is new: `untraced()` from the `bootstrap.py` beside the file where a token is in use, and None, the caller's own environment, where none is. The clone and the fetch each run with `env=rewrite_environment(token)`. With no token both stay traced where the caller asked. - The member's own `scripts/bootstrap.py`, which `place` runs after a clone, is given no token and keeps the caller's environment and its traces. #1023's jobserver handling is untouched. - `untraced` is imported, as #1023 imports `member_make_environment`, not copied: one answer to which traces a command carrying the token is kept out of, for `make bootstrap` and `make siblings`. A holder that keeps a `bootstrap.py` from before #996 is refused by name, `update-accept-local-missing-name` (#536), before a byte is written, so no `ImportError` reaches `make siblings`. The docstrings say the promise in full: never written into `.git/config`, never persisted, never printed, never traced. - `tests/test_family_siblings_untraced.py` (27 rows) crosses `make siblings` cloning, `make siblings` fetching and `family.py siblings` with the trace variables, the global config and the system config, for `SHAPE_LEGS_TOKEN` and `SHAPE_LEGS_APP_TOKEN` (18 rows: exit 0, `siblings ok`, the token in no output and in no file under the test's directory, every trace file written, the member's own bootstrap still traced), and with no token (9 rows: still traced as asked). All 18 token rows fail on main's `siblings.py`; the 9 controls pass. Eleven mutants, each alone, are all killed. - Runs on git 2.43.0: the new file 27 passed; 1623 passed, 4 skipped, and 2415 passed, 8 skipped, over 41 files (the new file, every file that runs `siblings.py`, the pinned-copy and upgrade-row files, the trio). git 2.47.1: the new file 27 passed. `templates/family-root/scripts/siblings.py` is in `FAMILY_COPIED_VERBATIM`, so each holder gets one `upstream-changed` row for it, as #1045 gave one for `bootstrap.py`. Closes #1055 Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- templates/family-root/scripts/siblings.py | 63 ++++- tests/test_family_siblings_untraced.py | 315 ++++++++++++++++++++++ tests/test_update_shape_kept_helper.py | 2 +- 3 files changed, 366 insertions(+), 14 deletions(-) create mode 100644 tests/test_family_siblings_untraced.py diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 2c1e59b1..174028a6 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -78,6 +78,14 @@ through git, as a remote is, and not copied file by file, so a repack running in it cannot fail the clone half way (#440). + THE TOKEN IS IN NO TRACE (#1055). The clone, and the fetch below, + carry the credential's rewrite in their argv, which a trace + writes, so with a token in use each runs with every `GIT_TRACE*` + variable taken out and trace2 turned off (`rewrite_environment`), + as `make bootstrap` runs every git it gives the token (#996). The + member's own bootstrap is given no token, and keeps the traces + the caller asked for. + PRESENT `git fetch`, AND NOTHING ELSE. No checkout, no reset, no pull, no stash, no branch switch. Somebody is working in that clone — that is what it is for — and a utility that "helpfully" moved their HEAD @@ -207,20 +215,25 @@ # token they used or where a member is mounted. So is the environment a # member's make gets under the holder's `make -jN` # (`member_make_environment`, #888, #987): one reading of `MAKEFLAGS` and of -# the member's make for every verb. Both files are copies of openRepoShape -# that travel together in `contracts/shape-pin.yaml`. A holder that keeps its -# own `bootstrap.py` with `update-shape.py apply --accept-local`, while the -# apply writes this file, is refused before a byte is written for each name -# this list takes that its kept copy does not define +# the member's make for every verb. And so is the environment a git given the +# token runs in (`untraced`, #996, #1055): one answer to which traces a +# command carrying the token is kept out of, for `make bootstrap` and `make +# siblings` alike. Both files are copies of openRepoShape that travel +# together in `contracts/shape-pin.yaml`. A holder that keeps its own +# `bootstrap.py` with `update-shape.py apply --accept-local`, while the apply +# writes this file, is refused before a byte is written for each name this +# list takes that its kept copy does not define # (`update-accept-local-missing-name`, #536), and merges its `bootstrap.py` # by hand first. That refusal is the price of one copy of the jobserver code -# (#987); #950 had copied the code here to spare such a holder it. The -# function's name is new with #987 so that a kept `bootstrap.py` of #950's, -# whose `member_make_env` took no make, is refused too, rather than ending -# `make park` in a `TypeError`. +# (#987), and of the trace rule (#1055); #950 had copied the code here to +# spare such a holder it. The function's name is new with #987 so that a +# kept `bootstrap.py` of #950's, whose `member_make_env` took no make, is +# refused too, rather than ending `make park` in a `TypeError`; `untraced` +# is new with #996, so a kept `bootstrap.py` from before it is refused by +# that name. from bootstrap import ( # noqa: E402 REWRITE_TARGETS, credential, member_make_environment, member_path, - members_of, + members_of, untraced, ) MANIFEST = "family.yaml" @@ -599,7 +612,9 @@ def git_prefix() -> tuple[list[str], str, str]: Resolved by `bootstrap.py`'s own `credential()`, in the same order, and used ONLY through a `url.<...>.insteadOf` rewrite for the duration of one - command: never written into `.git/config`, never persisted, never printed. + command: never written into `.git/config`, never persisted, never + printed, and never traced: each command given it runs in + `rewrite_environment`'s environment (#1055). """ source, token = credential() args: list[str] = [] @@ -610,6 +625,27 @@ def git_prefix() -> tuple[list[str], str, str]: return args, source, token +def rewrite_environment(token: str) -> dict[str, str] | None: + """The environment of a git given `git_prefix`'s rewrite: `bootstrap.py`'s + own `untraced()` where a token is in use, and None, this process's own, + where none is (#1055). + + A TRACE WRITES A COMMAND'S ARGUMENTS, and the rewrite that carries the + token is one: with `SHAPE_LEGS_TOKEN` or `SHAPE_LEGS_APP_TOKEN` set, the + clone in `place` and the fetch wrote it, as the `start` event of each, to + every trace2 target the caller named, in the environment (`GIT_TRACE2`, + `GIT_TRACE2_EVENT`, `GIT_TRACE2_PERF`) or in the global or the system + config (`trace2.eventTarget`, `normalTarget`, `perfTarget`), and + `GIT_TRACE_PERFORMANCE`'s file held it too, on git 2.43.0 and 2.47.1 + alike (measured). So both run with every + `GIT_TRACE*` variable taken out and trace2's three targets set to `0`, + as `make bootstrap` runs every git it gives the token (#996). With no + token there is nothing to keep out, and the two keep the traces the + caller asked for. The member's own `scripts/bootstrap.py`, which `place` + runs after a clone, is given no token and keeps them too.""" + return untraced() if token else None + + def scrub(text: str, token: str) -> str: """git puts the url it tried into its error text, token and all.""" return text.replace(token, "") if token else text @@ -982,7 +1018,7 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], *([f"--branch={branch}"] if branch else []), "--", url, str(target)], capture_output=True, text=True, encoding="utf-8", - errors="replace", check=False) + errors="replace", check=False, env=rewrite_environment(token)) if proc.returncode != 0: sibling.state = "CLONE FAILED" sibling.finding = ( @@ -1011,7 +1047,8 @@ def place(root: Path, row: dict, urls: dict[str, str], prefix: list[str], print(f" [{who}] present at {at}; fetching only") fetched = subprocess.run(["git", *prefix, "fetch", "--quiet", "origin"], cwd=str(target), capture_output=True, text=True, - encoding="utf-8", errors="replace", check=False) + encoding="utf-8", errors="replace", check=False, + env=rewrite_environment(token)) # NO CHECKOUT, NO RESET, NO PULL: somebody is working in here. sibling.state = "present, fetched" if fetched.returncode == 0 \ else "present, fetch failed" diff --git a/tests/test_family_siblings_untraced.py b/tests/test_family_siblings_untraced.py new file mode 100644 index 00000000..ef5c8fe8 --- /dev/null +++ b/tests/test_family_siblings_untraced.py @@ -0,0 +1,315 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The family holder's pinned `scripts/siblings.py` writes the token in +`SHAPE_LEGS_TOKEN` or `SHAPE_LEGS_APP_TOKEN` to no trace the caller asked +git for (#1055), as the holder's `bootstrap.py` writes it to none (#996). + +WHAT LANE 5 MEASURED, at main `1a47ed07`, on git 2.43.0 and 2.47.1: `make +siblings` run twice, so it clones both members and then fetches both, and +`family.py siblings`, with a token set and traces asked for, ended +`siblings ok`, exit 0, with the token in no output. But every trace2 target +the caller named held it, in the environment, in the global config or in +the system config: the `start` event of the clone in `place` and of the +fetch, which carry the credential's `-c url...insteadOf` rewrite in their +argv, one line for each member. `GIT_TRACE_PERFORMANCE`, which the issue did +not ask for, writes the same argv (`TRACE_VARIABLES`). + +WHAT IT DOES NOW. Where a token is in use, the clone and the fetch run in +`untraced()`'s environment, which `siblings.py` imports from the +`bootstrap.py` beside it (`rewrite_environment`): every `GIT_TRACE*` +variable taken out, and trace2's three targets set to `0`, which turns off +a target the global or the system config names too. With no token, both +keep the caller's environment and are traced where the caller asked. The +member's own `scripts/bootstrap.py`, which `place` runs after a clone, is +given no token, and keeps the caller's traces either way. + +THE ROWS. Each entry point (`ENTRIES`: `make siblings` cloning, `make +siblings` again fetching, `family.py siblings` cloning) is run with each way +of asking for a trace (`TRACED_BY`), with each variable a credential arrives +in, and with none: +- with a token: exit 0, `siblings ok`, every trace the way asks for written, + the member's own bootstrap traced where a member was cloned, and the token + in no output and in no file under the test's directory: the run's TMPDIR, + every trace and config file, and the clones' `.git/config` among them; +- with none, the control: the same, and the trace holds the clone, or the + fetch, that `siblings.py` itself ran, at each member. + +THE SAME FAMILY as `tests/test_family.py`, laid out as its `estate` lays it +out, and offline: a `gh` that refuses (`offline`), with TMPDIR a directory +of the test's own, and every remote a local path that the token's rewrite of +GitHub's urls never reaches, so the clones and the fetches succeed with the +fake token too. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from collections.abc import Callable +from pathlib import Path +from typing import NamedTuple + +import pytest + +from conftest import NO_BYTECODE, run_script +from test_family import ( # noqa: F401 (the `family` and `estate` fixtures) + FAMILY, MEMBERS, allow_file_protocol, estate, family, +) +from test_family_add_refusals import offline # noqa: F401 +from test_family_bootstrap_stops_named import CREDENTIALS, files_holding +from test_siblings_family_name import MAKE, NO_CREDENTIAL, needs_make + +#: A token for the runs with a credential in use: the fixture's members are +#: local paths, which its rewrite of GitHub's urls never reaches. +TOKEN = "w1055-a-token-no-forge-takes" + +#: What a run that placed both members says last. +PLACED = "siblings ok: 2 member(s) beside the holder in " + +#: The trace variables lane 5's probe set (#1055), each to a file under the +#: run's TMPDIR, and `GIT_TRACE_PERFORMANCE`, whose file holds each git's +#: whole command line, `-c` and all (measured, git 2.43.0 and 2.47.1). +TRACE_VARIABLES = { + "GIT_TRACE": "trace.txt", + "GIT_TRACE2": "trace2-normal.txt", + "GIT_TRACE2_EVENT": "trace2-event.json", + "GIT_TRACE2_PERF": "trace2-perf.txt", + "GIT_TRACE_PACKET": "trace-packet.txt", + "GIT_TRACE_CURL": "trace-curl.txt", + "GIT_TRACE_PERFORMANCE": "trace-performance.txt", +} + +#: trace2's three targets as a config file names them, and the file each is +#: given, after a prefix that says which config named it. +TARGETS = {"eventTarget": "event.json", "normalTarget": "normal.txt", + "perfTarget": "perf.txt"} + + +def targets_section(temp: Path, prefix: str) -> bytes: + """A `[trace2]` section naming each of `TARGETS` as a file in `temp`.""" + return "".join([ + "\n[trace2]\n", + *(f'\t{key} = "{(temp / (prefix + name)).as_posix()}"\n' + for key, name in TARGETS.items())]).encode("utf-8") + + +def by_the_variables(env: dict, temp: Path, tmp_path: Path) -> dict: + """`env` with each of `TRACE_VARIABLES` set to its file in `temp`.""" + return {**env, **{name: str(temp / file) + for name, file in TRACE_VARIABLES.items()}} + + +def by_the_global_config(env: dict, temp: Path, tmp_path: Path) -> dict: + """`env` with a global config of the test's own: the session's, which + turns git's automatic maintenance off, with trace2's three targets + added.""" + config = tmp_path / "traced-global-gitconfig" + config.write_bytes(Path(os.environ["GIT_CONFIG_GLOBAL"]).read_bytes() + + targets_section(temp, "cfg-")) + return {**env, "GIT_CONFIG_GLOBAL": str(config)} + + +def by_the_system_config(env: dict, temp: Path, tmp_path: Path) -> dict: + """`env` with a system config of the test's own, naming trace2's three + targets, and read whatever the environment running the suite says.""" + config = tmp_path / "traced-system-gitconfig" + config.write_bytes(targets_section(temp, "sys-")) + return {**env, "GIT_CONFIG_SYSTEM": str(config), + "GIT_CONFIG_NOSYSTEM": "0"} + + +class Traced(NamedTuple): + """A way a caller asks git for traces: `asks` lays it over a run's + environment; `event` is the file trace2's events go to; `written` the + files a run writes, with a token in use or none, since the run's other + gits are traced either way. `GIT_TRACE_PACKET` is written by a clone or + a fetch alone, and `GIT_TRACE_CURL` for an http remote alone.""" + + asks: Callable[[dict, Path, Path], dict] + event: str + written: tuple[str, ...] + + +#: The three ways the issue measured: the variables, and trace2's targets +#: in the global config and in the system config. +TRACED_BY = { + "variables": Traced(by_the_variables, "trace2-event.json", ( + "trace.txt", "trace2-normal.txt", "trace2-event.json", + "trace2-perf.txt", "trace-performance.txt")), + "global-config": Traced(by_the_global_config, "cfg-event.json", tuple( + f"cfg-{name}" for name in TARGETS.values())), + "system-config": Traced(by_the_system_config, "sys-event.json", tuple( + f"sys-{name}" for name in TARGETS.values())), +} + + +def events(path: Path) -> list[dict]: + """The trace2 events in `path`, a line each; a line that is no event is + skipped.""" + found = [] + for line in path.read_bytes().decode("utf-8", "replace").splitlines(): + try: + event = json.loads(line) + except ValueError: + continue + if isinstance(event, dict): + found.append(event) + return found + + +def traced_runs(path: Path) -> list[tuple[list, str]]: + """`(argv, work tree)` of each git whose `start` event `path` holds: + the work tree is its own repository's (`def_repo`, repository 1), as a + real path, or `""` where its events name none.""" + runs: dict[str, dict] = {} + for event in events(path): + run = runs.setdefault(str(event.get("sid")), {}) + if event.get("event") == "start": + run["argv"] = event.get("argv") or [] + elif (event.get("event") == "def_repo" and event.get("repo") == 1 + and isinstance(event.get("worktree"), str)): + run["worktree"] = os.path.realpath(event["worktree"]) + return [(run["argv"], run.get("worktree", "")) for run in runs.values() + if "argv" in run] + + +def siblings_clones(path: Path) -> list[str]: + """Where each `git clone -q --no-local` traced in `path` cloned to, as + a real path: the clone `place` runs with no token, whose last argument + is the member's slot beside the holder.""" + return sorted(os.path.realpath(argv[-1]) + for argv, _ in traced_runs(path) + if argv[1:4] == ["clone", "-q", "--no-local"]) + + +def siblings_fetches(path: Path) -> list[str]: + """The work tree of each `git fetch --quiet origin` traced in `path`: + the fetch `place` runs in a member's clone with no token.""" + return sorted(worktree for argv, worktree in traced_runs(path) + if argv[1:] == ["fetch", "--quiet", "origin"]) + + +def in_a_leg(path: Path, folder: Path) -> list[str]: + """Each member some git traced in `path` ran in a leg of, a work tree + under its clone beside the holder that is not the clone itself: only + the member's own bootstrap runs a git there, where a clone with a token + in use is not traced.""" + trees = [Path(worktree) for _, worktree in traced_runs(path) if worktree] + return [project for project in MEMBERS + if any(Path(os.path.realpath(folder / project)) in tree.parents + for tree in trees)] + + +def make_siblings(holder: Path, env: dict) -> subprocess.CompletedProcess: + """`make siblings` in `holder`, under `env` laid over this process's + environment, with this interpreter as `PYTHON` and no flags handed + down from a make above (`test_family_makefile_siblings.make`).""" + laid = {name: value for name, value in {**os.environ, **env}.items() + if name not in ("MAKEFLAGS", "MAKELEVEL", "MFLAGS")} + return subprocess.run([MAKE, "siblings", f"PYTHON={sys.executable}"], + cwd=str(holder), env=laid, capture_output=True, + text=True, encoding="utf-8", errors="replace", + stdin=subprocess.DEVNULL, check=False) + + +def cloning_by_make(estate: dict, env: dict, setup: dict + ) -> subprocess.CompletedProcess: + """`make siblings`, with no member beside the holder: it clones both.""" + return make_siblings(estate["holder"], env) + + +def fetching_by_make(estate: dict, env: dict, setup: dict + ) -> subprocess.CompletedProcess: + """`make siblings` again: a first run under `setup`, with no token and + no trace, clones both members, and the second, under `env`, fetches + both.""" + first = make_siblings(estate["holder"], setup) + assert first.returncode == 0, first.stdout + first.stderr + return make_siblings(estate["holder"], env) + + +def cloning_by_family_py(estate: dict, env: dict, setup: dict + ) -> subprocess.CompletedProcess: + """`family.py siblings --family-root `, which runs the holder's + own `scripts/siblings.py` in the caller's environment (#543): it clones + both.""" + return run_script(FAMILY, "siblings", "--family-root", + str(estate["holder"]), env={**NO_BYTECODE, **env}) + + +class Entry(NamedTuple): + """An entry point of `make siblings` and what it does: `run` runs it; + `ran` reads, from a trace, where `siblings.py`'s own clone or fetch ran; + `bootstrapped` says whether each member's own bootstrap runs.""" + + run: Callable[[dict, dict, dict], subprocess.CompletedProcess] + ran: Callable[[Path], list[str]] + bootstrapped: bool + + +ENTRIES = { + "make-clone": pytest.param(Entry(cloning_by_make, siblings_clones, True), + marks=needs_make, id="make-clone"), + "make-fetch": pytest.param(Entry(fetching_by_make, siblings_fetches, + False), + marks=needs_make, id="make-fetch"), + "family-py": pytest.param(Entry(cloning_by_family_py, siblings_clones, + True), id="family-py"), +} + + +def offline_env(offline) -> dict: + """The environment every run here starts from: `offline`'s `gh` and + TMPDIR, the file transport allowed to the fixture's local remotes as + the suite allows it, and no credential, whatever the machine running + the suite exports.""" + return {**offline.env, **allow_file_protocol(), **NO_CREDENTIAL} + + +def written(temp: Path, traced: Traced) -> list[str]: + """Those of `traced.written` that are files in `temp`.""" + return [name for name in traced.written if (temp / name).is_file()] + + +@pytest.mark.parametrize("variable", CREDENTIALS) +@pytest.mark.parametrize("traced", TRACED_BY.values(), ids=TRACED_BY) +@pytest.mark.parametrize("entry", ENTRIES.values()) +def test_with_a_token_in_use_the_token_is_in_no_trace( + family, estate, offline, tmp_path, entry, traced, variable): + """#1055: a token in `variable`, and traces asked for `traced`'s way. + The run places both members and says so, every trace is written by the + run's other gits, and the member's own bootstrap, after a clone, is + traced; the token is in no output and in no file under the test's + directory. Before, the clone's and the fetch's `start` events wrote it + to every trace2 target, and `GIT_TRACE_PERFORMANCE` wrote it too.""" + setup = offline_env(offline) + env = traced.asks({**setup, variable: TOKEN}, offline.temp, tmp_path) + + result = entry.run(estate, env, setup) + + assert result.returncode == 0, result.stdout + result.stderr + assert PLACED in result.stdout, result.stdout + assert TOKEN not in result.stdout + result.stderr + assert files_holding(tmp_path, TOKEN) == [] + assert written(offline.temp, traced) == list(traced.written) + assert in_a_leg(offline.temp / traced.event, estate["folder"]) == ( + list(MEMBERS) if entry.bootstrapped else []) + + +@pytest.mark.parametrize("traced", TRACED_BY.values(), ids=TRACED_BY) +@pytest.mark.parametrize("entry", ENTRIES.values()) +def test_with_no_token_the_clone_and_the_fetch_are_traced_as_asked( + family, estate, offline, tmp_path, entry, traced): + """The control (#1055): with no credential, the clone and the fetch + `siblings.py` runs keep the caller's environment, so the trace asked + for `traced`'s way holds each, at each member.""" + env = traced.asks(offline_env(offline), offline.temp, tmp_path) + + result = entry.run(estate, env, offline_env(offline)) + + assert result.returncode == 0, result.stdout + result.stderr + assert PLACED in result.stdout, result.stdout + assert written(offline.temp, traced) == list(traced.written) + assert entry.ran(offline.temp / traced.event) == sorted( + os.path.realpath(estate["folder"] / project) for project in MEMBERS) diff --git a/tests/test_update_shape_kept_helper.py b/tests/test_update_shape_kept_helper.py index c104a1eb..f55ba029 100644 --- a/tests/test_update_shape_kept_helper.py +++ b/tests/test_update_shape_kept_helper.py @@ -557,7 +557,7 @@ def test_the_reader_finds_all_three_of_validate_pins_statements(update_shape): def test_what_siblings_takes_from_the_holders_bootstrap_is_found_defined( update_shape): """The other helper a shipped copy imports from: the holder's - `siblings.py` takes five names from its `bootstrap.py`.""" + `siblings.py` takes six names from its `bootstrap.py`.""" family = REPO / "templates" / "family-root" / "scripts" taken = update_shape._names_imported( (family / "siblings.py").read_bytes())["bootstrap"] From 041f7831bb24e90a817842d3cc50ee488694a65a Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Tue, 20 Oct 2026 22:44:55 +0000 Subject: [PATCH 2/3] the family root's siblings.py and its test: a row with a member whose remote is GitHub's, behind a dead proxy, finds a clone that keeps the caller's GIT_TRACE, and the member's own bootstrap is said to be given no token in its argv (#1055) Lane 5's independent review of #1075 (rv1075, issuecomment-6302090874) found the code ready, with two SHOULDs and a NIT. This commit answers the test SHOULD and the NIT; the PR body answers the other SHOULD, whose gap is #1082's. - SHOULD 2: every row used a local file remote, so a leak through `GIT_TRACE` on an https member, which is what a real github.com member is, had no test. rv1075's mutant R4, which keeps the caller's `GIT_TRACE` in `rewrite_environment`'s environment, passed all 27 rows. With an https member, git writes the clone's rewritten url, token and all, in `GIT_TRACE`'s `run_command: git remote-https` lines, offline, behind a dead proxy. `test_an_https_member_with_a_token_in_use_is_in_no_trace` gives one member of the estate a `https://github.com//.git` url and `http.proxy=http://127.0.0.1:9` in git's command scope, sets a fake `SHAPE_LEGS_TOKEN` and the trace variables, and asserts: the proxy is set, `make siblings` exits 2, the clone exited 128, the other member is placed, and the token is in no output and in no file under the test's directory, with every trace the variables ask for still written. - NIT: `siblings.py`'s module docstring and `rewrite_environment`'s, and the test file's, said the member's own bootstrap "is given no token". It inherits `SHAPE_LEGS_TOKEN` and `SHAPE_LEGS_APP_TOKEN` in its environment, because `run_member_bootstrap` passes no `env`, and the shipped one reads neither. They now say it is given no token in its argv. `rewrite_environment`'s docstring also names the https measurement. The code of `siblings.py` is unchanged: its syntax tree, docstrings aside, is the same as before. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- templates/family-root/scripts/siblings.py | 12 ++-- tests/test_family_siblings_untraced.py | 82 ++++++++++++++++++++++- 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/templates/family-root/scripts/siblings.py b/templates/family-root/scripts/siblings.py index 174028a6..8757ad18 100755 --- a/templates/family-root/scripts/siblings.py +++ b/templates/family-root/scripts/siblings.py @@ -83,8 +83,9 @@ writes, so with a token in use each runs with every `GIT_TRACE*` variable taken out and trace2 turned off (`rewrite_environment`), as `make bootstrap` runs every git it gives the token (#996). The - member's own bootstrap is given no token, and keeps the traces - the caller asked for. + member's own bootstrap is given no token in its argv (it inherits + the token variables; the shipped one reads neither), and keeps + the traces the caller asked for. PRESENT `git fetch`, AND NOTHING ELSE. No checkout, no reset, no pull, no stash, no branch switch. Somebody is working in that clone — that @@ -637,12 +638,15 @@ def rewrite_environment(token: str) -> dict[str, str] | None: `GIT_TRACE2_EVENT`, `GIT_TRACE2_PERF`) or in the global or the system config (`trace2.eventTarget`, `normalTarget`, `perfTarget`), and `GIT_TRACE_PERFORMANCE`'s file held it too, on git 2.43.0 and 2.47.1 - alike (measured). So both run with every + alike (measured). With an https member, as a `github.com` one is, + `GIT_TRACE`'s own file held it as well, in the `run_command: git + remote-https` lines (measured). So both run with every `GIT_TRACE*` variable taken out and trace2's three targets set to `0`, as `make bootstrap` runs every git it gives the token (#996). With no token there is nothing to keep out, and the two keep the traces the caller asked for. The member's own `scripts/bootstrap.py`, which `place` - runs after a clone, is given no token and keeps them too.""" + runs after a clone, is given no token in its argv (it inherits the token + variables; the shipped one reads neither) and keeps them too.""" return untraced() if token else None diff --git a/tests/test_family_siblings_untraced.py b/tests/test_family_siblings_untraced.py index ef5c8fe8..45e51f32 100644 --- a/tests/test_family_siblings_untraced.py +++ b/tests/test_family_siblings_untraced.py @@ -20,7 +20,8 @@ a target the global or the system config names too. With no token, both keep the caller's environment and are traced where the caller asked. The member's own `scripts/bootstrap.py`, which `place` runs after a clone, is -given no token, and keeps the caller's traces either way. +given no token in its argv (it inherits the token variables, and the shipped +one reads neither), and keeps the caller's traces either way. THE ROWS. Each entry point (`ENTRIES`: `make siblings` cloning, `make siblings` again fetching, `family.py siblings` cloning) is run with each way @@ -38,12 +39,21 @@ of the test's own, and every remote a local path that the token's rewrite of GitHub's urls never reaches, so the clones and the fetches succeed with the fake token too. + +AND ONE ROW WITH AN HTTPS MEMBER (`HTTPS_MEMBER`), which the rewrite does +reach, as it reaches a `github.com` member. A local path is no https url, so +`GIT_TRACE`'s own file holds no token for it; for an https one the clone's +`run_command: git remote-https` lines hold the rewritten url, measured on git +2.43.0 and 2.47.1 by lane 5's review of #1075. The row stays offline behind a +dead proxy: the clone fails at once, `siblings.py` says so, and nothing +leaves the host. """ from __future__ import annotations import json import os +import re import subprocess import sys from collections.abc import Callable @@ -52,9 +62,11 @@ import pytest -from conftest import NO_BYTECODE, run_script +from conftest import ( + NO_BYTECODE, appended_to_command_scope, command_scope_at, run_script, +) from test_family import ( # noqa: F401 (the `family` and `estate` fixtures) - FAMILY, MEMBERS, allow_file_protocol, estate, family, + FAMILY, MEMBERS, ORG, allow_file_protocol, estate, family, ) from test_family_add_refusals import offline # noqa: F401 from test_family_bootstrap_stops_named import CREDENTIALS, files_holding @@ -67,6 +79,14 @@ #: What a run that placed both members says last. PLACED = "siblings ok: 2 member(s) beside the holder in " +#: A proxy nothing listens at (port 9, `discard`): an https clone through it +#: fails at once, and nothing leaves the host. +DEAD_PROXY = "http://127.0.0.1:9" + +#: The member the https row gives a GitHub url, and the member it leaves a +#: local path. +HTTPS_MEMBER, LOCAL_MEMBER = MEMBERS + #: The trace variables lane 5's probe set (#1055), each to a file under the #: run's TMPDIR, and `GIT_TRACE_PERFORMANCE`, whose file holds each git's #: whole command line, `-c` and all (measured, git 2.43.0 and 2.47.1). @@ -272,6 +292,28 @@ def written(temp: Path, traced: Traced) -> list[str]: return [name for name in traced.written if (temp / name).is_file()] +def given_a_github_url(holder: Path, project: str) -> None: + """`project`'s url in the holder's `.gitmodules`, a local path, spelled + as GitHub's: `https://github.com//.git`, which the token's + rewrite reaches. The file is read by `siblings.py` as it stands.""" + path = holder / ".gitmodules" + spelled, found = re.subn( + rf"(?m)^(\turl = ).*/{project}\.git$", + rf"\g<1>https://github.com/{ORG}/{project}.git", + path.read_text(encoding="utf-8")) + assert found == 1, path.read_text(encoding="utf-8") + path.write_text(spelled, encoding="utf-8") + + +def behind_a_dead_proxy(env: dict) -> dict: + """`env` with `http.proxy` at `DEAD_PROXY` in git's command scope, after + the entries `env` and this process hold, and with no list of hosts that + skip a proxy: no https clone made under it leaves the host.""" + held = {**os.environ, **env} + return {**env, **appended_to_command_scope( + held, [("http.proxy", DEAD_PROXY)]), "NO_PROXY": "", "no_proxy": ""} + + @pytest.mark.parametrize("variable", CREDENTIALS) @pytest.mark.parametrize("traced", TRACED_BY.values(), ids=TRACED_BY) @pytest.mark.parametrize("entry", ENTRIES.values()) @@ -313,3 +355,37 @@ def test_with_no_token_the_clone_and_the_fetch_are_traced_as_asked( assert written(offline.temp, traced) == list(traced.written) assert entry.ran(offline.temp / traced.event) == sorted( os.path.realpath(estate["folder"] / project) for project in MEMBERS) + + +@needs_make +def test_an_https_member_with_a_token_in_use_is_in_no_trace( + family, estate, offline, tmp_path): + """#1055, lane 5's review of #1075 (rv1075): the token in + `SHAPE_LEGS_TOKEN`, the trace variables set, and one member whose remote + is GitHub's (`HTTPS_MEMBER`), behind a dead proxy. The rewrite reaches + its clone, which fails with git's 128, whichever way this git is built + to fail an https clone (at the proxy, or for want of its https helper), + and `make siblings` exits 2; the other member is placed. Before, + `GIT_TRACE`'s file held the token in the clone's `run_command: git + remote-https` lines, and so did the trace2 files; the token is now in no + output and in no file under the test's directory, every trace the + variables ask for is written by the run's other gits, and a mutant that + keeps the caller's `GIT_TRACE` for the clone is found.""" + traced = TRACED_BY["variables"] + setup = offline_env(offline) + env = behind_a_dead_proxy( + traced.asks({**setup, "SHAPE_LEGS_TOKEN": TOKEN}, offline.temp, + tmp_path)) + given_a_github_url(estate["holder"], HTTPS_MEMBER) + assert f"http.proxy={DEAD_PROXY}" in command_scope_at( + env, estate["holder"]) + + result = cloning_by_make(estate, env, setup) + + assert result.returncode == 2, result.stdout + result.stderr + assert f"{HTTPS_MEMBER}: git clone exited 128:" in result.stderr + assert (estate["folder"] / LOCAL_MEMBER / ".git").is_dir() + assert not (estate["folder"] / HTTPS_MEMBER).exists() + assert TOKEN not in result.stdout + result.stderr + assert files_holding(tmp_path, TOKEN) == [] + assert written(offline.temp, traced) == list(traced.written) From 962ae43c1be6bd179eeb68529edc97ac3338fb0f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 10:39:01 +0000 Subject: [PATCH 3/3] the https row of the family root's siblings.py test: its git gets a global file, no system file and no ssh of its own, so a caller's `url..insteadOf` for GitHub's url sends the clone nowhere, and the row runs per stand-in config (#1055) #1075's landing gate (log c1075.log, #1075 issuecomment-6304830183) ran the https row with neither GIT_CONFIG_GLOBAL=/dev/null nor GIT_SSH_COMMAND=false. conftest.py's session global file includes the caller's, and the operator's ~/.gitconfig has `url.git@github.com:.insteadOf=https://github.com/`, which beats the token's `-c` rewrite of the same prefix: git keeps the first of two equal prefixes, and config files are read before the command line. So the row's https://github.com/InkRouter/IRRS.git went over ssh to a real private repository, the dead proxy never applied, the clone succeeded, `make siblings` exited 0, and the row failed `returncode == 2`. The writers' runs, which export both variables, passed. - `hermetic_config` gives the row's git a global file of its own, `session_global_config("")` (conftest's own: automatic maintenance off, no include of the caller's file), `GIT_CONFIG_NOSYSTEM=1` and `GIT_SSH_COMMAND=false`. It is the least invasive way: only the one row with an https url sets it, conftest.py and every other row stay as they were, and the other rows' remotes are local paths that no rewrite of GitHub's url reaches. - `url_after_config` checks before the clone that `git ls-remote --get-url` leaves the url unrewritten under the row's config, so a config that reaches the row fails by name, not by cloning something. - The row is parametrized by `CALLERS_CONFIG`: none, and a scratch global and a scratch system config of the test's own that rewrite the url to the family's own IRRS remote (`as_the_callers_config`, built from tmp files, no network, never the real ~/.gitconfig). Without that, the two stand-ins clone locally and exit 0; with it the clone fails at the proxy with git's 128, IRSS is placed, and the token is in no output and in no file. The code of siblings.py is not touched. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_family_siblings_untraced.py | 109 ++++++++++++++++++++++--- 1 file changed, 99 insertions(+), 10 deletions(-) diff --git a/tests/test_family_siblings_untraced.py b/tests/test_family_siblings_untraced.py index 45e51f32..0b7f5dc9 100644 --- a/tests/test_family_siblings_untraced.py +++ b/tests/test_family_siblings_untraced.py @@ -47,6 +47,17 @@ 2.43.0 and 2.47.1 by lane 5's review of #1075. The row stays offline behind a dead proxy: the clone fails at once, `siblings.py` says so, and nothing leaves the host. + +AND THAT ROW DOES NOT DEPEND ON THE CALLER'S GIT CONFIG (#1075's landing +gate). `conftest.py`'s session global file includes the caller's, and a +caller's `url..insteadOf` for GitHub's url, which `~/.gitconfig` of the +person running the suite may hold, outranks the token's `-c` rewrite of the +same prefix: the gate's row went over ssh to a real repository, and the +clone succeeded. So the row gives its git a global file and no system file of +its own (`hermetic_config`), checks what git makes of the url before the +token's `-c` (`url_after_config`), and runs once for each stand-in for a +caller's config that would send the url to a reachable remote +(`as_the_callers_config`). """ from __future__ import annotations @@ -64,6 +75,7 @@ from conftest import ( NO_BYTECODE, appended_to_command_scope, command_scope_at, run_script, + session_global_config, ) from test_family import ( # noqa: F401 (the `family` and `estate` fixtures) FAMILY, MEMBERS, ORG, allow_file_protocol, estate, family, @@ -292,17 +304,19 @@ def written(temp: Path, traced: Traced) -> list[str]: return [name for name in traced.written if (temp / name).is_file()] -def given_a_github_url(holder: Path, project: str) -> None: +def given_a_github_url(holder: Path, project: str) -> str: """`project`'s url in the holder's `.gitmodules`, a local path, spelled - as GitHub's: `https://github.com//.git`, which the token's - rewrite reaches. The file is read by `siblings.py` as it stands.""" + as GitHub's, which the token's rewrite reaches, and that url: + `https://github.com//.git`. The file is read by + `siblings.py` as it stands.""" + url = f"https://github.com/{ORG}/{project}.git" path = holder / ".gitmodules" spelled, found = re.subn( - rf"(?m)^(\turl = ).*/{project}\.git$", - rf"\g<1>https://github.com/{ORG}/{project}.git", + rf"(?m)^(\turl = ).*/{project}\.git$", rf"\g<1>{url}", path.read_text(encoding="utf-8")) assert found == 1, path.read_text(encoding="utf-8") path.write_text(spelled, encoding="utf-8") + return url def behind_a_dead_proxy(env: dict) -> dict: @@ -314,6 +328,68 @@ def behind_a_dead_proxy(env: dict) -> dict: held, [("http.proxy", DEAD_PROXY)]), "NO_PROXY": "", "no_proxy": ""} +def hermetic_config(env: dict, tmp_path: Path) -> dict: + """`env` with git's configuration the row's own, so that the caller's + cannot send an https url anywhere: a global file of the session's own + settings alone (`session_global_config("")`: automatic maintenance off, + and no include of the caller's file), no system file, and no ssh. + + THE LEAST INVASIVE WAY: only the one row that has an https url sets it. + The session's file stays as `conftest.py` writes it, with the caller's + settings still applying to every row whose remotes are local paths, which + no rewrite of GitHub's url reaches; and `conftest.py` is not touched. + `GIT_CONFIG_GLOBAL` set takes git's two user-level files out of the + reading, and `GIT_CONFIG_NOSYSTEM` the system file, whatever + `GIT_CONFIG_SYSTEM` says. `GIT_SSH_COMMAND=false` is the second wall: a + url somehow sent over ssh runs `false`, not a real ssh.""" + config = tmp_path / "hermetic-global-gitconfig" + config.write_bytes(session_global_config("")) + return {**env, "GIT_CONFIG_GLOBAL": str(config), + "GIT_CONFIG_NOSYSTEM": "1", "GIT_SSH_COMMAND": "false"} + + +def url_after_config(env: dict, cwd: Path, url: str) -> str: + """What git makes of `url` under `env` laid over this process's: `git + ls-remote --get-url`, which applies the config's `insteadOf` rewrites + and asks nothing of the network, read and never written. The token's + `-c` is not in it, so it is the caller's config alone.""" + shown = subprocess.run( + ["git", "ls-remote", "--get-url", url], cwd=str(cwd), + env={**os.environ, **env}, capture_output=True, text=True, + encoding="utf-8", errors="replace", check=False) + assert shown.returncode == 0, shown.stderr + shown.stdout + return shown.stdout.strip() + + +#: Where a caller's git config can be, for the https row: none that rewrites +#: the url (the control), the global file, and the system file. +CALLERS_CONFIG = ("none", "global", "system") + + +def as_the_callers_config(monkeypatch, tmp_path: Path, where: str, + url: str, remote: Path) -> None: + """Stand in for a caller whose git config sends `url`, GitHub's url of + the https member, to `remote`, a bare repository of the test's own that + a clone reaches: a scratch file of the test's own, in the global + (`GIT_CONFIG_GLOBAL`, which the session file includes) or the system + scope (`GIT_CONFIG_SYSTEM`) of this process, and never a file of the + person running the suite. Nothing leaves the host. `where` of `none` + changes nothing.""" + if where == "none": + return + uri = remote.resolve().as_uri().replace("\\", "\\\\").replace( + '"', '\\"') + rewrite = f'[url "{uri}"]\n\tinsteadOf = {url}\n'.encode("utf-8") + config = tmp_path / f"callers-{where}-gitconfig" + if where == "global": + config.write_bytes(session_global_config("") + rewrite) + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(config)) + else: + config.write_bytes(rewrite) + monkeypatch.setenv("GIT_CONFIG_SYSTEM", str(config)) + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "0") + + @pytest.mark.parametrize("variable", CREDENTIALS) @pytest.mark.parametrize("traced", TRACED_BY.values(), ids=TRACED_BY) @pytest.mark.parametrize("entry", ENTRIES.values()) @@ -358,8 +434,9 @@ def test_with_no_token_the_clone_and_the_fetch_are_traced_as_asked( @needs_make +@pytest.mark.parametrize("callers", CALLERS_CONFIG) def test_an_https_member_with_a_token_in_use_is_in_no_trace( - family, estate, offline, tmp_path): + family, estate, offline, tmp_path, monkeypatch, callers): """#1055, lane 5's review of #1075 (rv1075): the token in `SHAPE_LEGS_TOKEN`, the trace variables set, and one member whose remote is GitHub's (`HTTPS_MEMBER`), behind a dead proxy. The rewrite reaches @@ -370,15 +447,27 @@ def test_an_https_member_with_a_token_in_use_is_in_no_trace( remote-https` lines, and so did the trace2 files; the token is now in no output and in no file under the test's directory, every trace the variables ask for is written by the run's other gits, and a mutant that - keeps the caller's `GIT_TRACE` for the clone is found.""" + keeps the caller's `GIT_TRACE` for the clone is found. + + WHATEVER THE CALLER'S GIT CONFIG SAYS: `callers` stands in for one that + sends the https url to the member's own bare remote, a path the clone + reaches, in the global or the system scope (`as_the_callers_config`). + The row's git reads neither (`hermetic_config`), the url is checked to + be unrewritten before the token's `-c`, and the clone still fails at the + proxy; a row whose git read the caller's file cloned from the remote, + placed both members and exited 0 (the landing gate's, on a person's + `~/.gitconfig`).""" traced = TRACED_BY["variables"] setup = offline_env(offline) - env = behind_a_dead_proxy( + env = behind_a_dead_proxy(hermetic_config( traced.asks({**setup, "SHAPE_LEGS_TOKEN": TOKEN}, offline.temp, - tmp_path)) - given_a_github_url(estate["holder"], HTTPS_MEMBER) + tmp_path), tmp_path)) + url = given_a_github_url(estate["holder"], HTTPS_MEMBER) + as_the_callers_config(monkeypatch, tmp_path, callers, url, + estate["remotes"] / f"{HTTPS_MEMBER}.git") assert f"http.proxy={DEAD_PROXY}" in command_scope_at( env, estate["holder"]) + assert url_after_config(env, estate["holder"], url) == url result = cloning_by_make(estate, env, setup)