From e0cd0cb7a39b1cad17f0e86795291d0a7a01818d Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Wed, 5 Aug 2026 11:16:23 +0000 Subject: [PATCH] fix(docs-sync): the mirror is fetched from the forge in play, never a built-in one (#201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit heavy-duty/ceremony exists on two forges and the same ref names a different tree on each — this forge's 0.4.1 carries lib/forge*.sh, GitHub's carries none of it. The fetch URL was hard-coded to github.com, so a consumer's `.ceremony/` mirror was verified against a tree it never pinned, and the fetch returned HTTP 200 while doing it: --check reported drift the consumer could not fix, and --fix would have rewritten a correct mirror into the wrong one. The host now comes from GITHUB_SERVER_URL, which Actions injects on both forges and which lib/forge.sh already selects the whole backend on. Unset, with no --source, is a refusal naming the variable rather than a guess — the same rule the pin itself has always followed. The fetch path had no test coverage at all: every existing row passes --source, which overrides the fetch entirely. It is now driven against a PATH-stubbed curl that records the URL and serves a tarball, so the real tar pipeline still runs and which forge a pin resolves against is asserted. Refs #201 --- actions/docs-sync/docs-sync.sh | 25 ++++++++++- changelog.d/201.md | 24 ++++++++++ test/docs-sync.test.sh | 81 +++++++++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 changelog.d/201.md diff --git a/actions/docs-sync/docs-sync.sh b/actions/docs-sync/docs-sync.sh index ba42647..3a9be9c 100644 --- a/actions/docs-sync/docs-sync.sh +++ b/actions/docs-sync/docs-sync.sh @@ -148,11 +148,32 @@ if [ -n "$source_dir" ]; then else # The repo is public: a plain tarball fetch, no auth, no git. Works for a # tag, a branch, or a commit SHA alike. + # + # THE FORGE COMES FROM THE ENVIRONMENT, NEVER FROM THIS FILE (#201). + # heavy-duty/ceremony exists on two forges and the same ref names a + # DIFFERENT TREE on each: `0.4.1` on this forge carries lib/forge.sh, + # lib/forge-github.sh and lib/forge-forgejo.sh; GitHub's `0.4.1` carries + # none of them. A hard-coded host therefore verified a consumer's mirror + # against a tree it never pinned — and did it with HTTP 200, so --check + # reported drift the consumer could not fix and --fix would have rewritten + # a correct mirror into the wrong one. The version numbers agreeing is the + # hazard, not the protection (#197 decision 2). + # + # GITHUB_SERVER_URL is what Actions injects on both forges, and + # lib/forge.sh already selects the whole backend on it — so a consumer run + # that reached this line has it. Unset means we do not know which forge the + # pin refers to, and guessing is what this issue is about: refuse instead, + # the same way the pin itself is never guessed. + [ -n "${GITHUB_SERVER_URL:-}" ] || die \ + "docs-sync: GITHUB_SERVER_URL is unset, so the forge holding" \ + " heavy-duty/ceremony@$ref is unknown — and the same ref names a" \ + " different tree on each forge. Set it to the forge this consumer is" \ + " pinned against, or pass --source . This tool never guesses a forge." fetch_tmp="$(mktemp -d)" - url="https://github.com/heavy-duty/ceremony/archive/${ref}.tar.gz" + url="${GITHUB_SERVER_URL%/}/heavy-duty/ceremony/archive/${ref}.tar.gz" curl -fsSL "$url" | tar -xz --strip-components=1 -C "$fetch_tmp" || die \ "docs-sync: cannot fetch heavy-duty/ceremony@$ref ($url) —" \ - " does the pinned ref exist?" + " does the pinned ref exist on that forge?" src="$fetch_tmp" origin="heavy-duty/ceremony@$ref" fi diff --git a/changelog.d/201.md b/changelog.d/201.md new file mode 100644 index 0000000..c859f87 --- /dev/null +++ b/changelog.d/201.md @@ -0,0 +1,24 @@ +### Fixed + +- `actions/docs-sync` fetches the doctrine mirror from the forge named by + `GITHUB_SERVER_URL` instead of a hard-coded `github.com` (#201). + +- The same pin ref names a different tree on each forge, so a consumer's mirror + was verified against a tree it never pinned — and with HTTP 200, so `--check` + reported drift that could not be fixed (#201). + +- A fetch that cannot name its forge now refuses instead of guessing: no + `GITHUB_SERVER_URL` and no `--source` exits naming the variable, having + reached for no network (#201). + +- A failed fetch names the URL it actually tried, and asks whether the ref + exists on that forge rather than in the abstract (#201). + +### Added + +- `test/docs-sync.test.sh` drives the fetch path, which had no coverage at all: + every existing row passes `--source`, which overrides the fetch entirely + (#201). + +- A stubbed `curl` records the requested URL and serves a tarball, so which + forge a pin resolves against is a tested decision rather than plumbing (#201). diff --git a/test/docs-sync.test.sh b/test/docs-sync.test.sh index 6987304..129f663 100644 --- a/test/docs-sync.test.sh +++ b/test/docs-sync.test.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Contract tests for actions/docs-sync (issue #19). Constructed SOURCE trees # (a fake ceremony: manifest + docs) and CONSUMER trees (a release.yml -# caller with the pin line), driven offline via --source — the fetch path -# needs the network and is exercised by consumers, not here. The fake +# caller with the pin line), driven offline via --source. The fetch path is +# driven too, against a PATH-stubbed curl rather than the network (#201) — +# which forge a pin resolves against is a decision, not plumbing. The fake # source's doc set is deliberately NOT the real five: a script that # hardcodes the vendored list instead of reading the manifest fails these # rows. set -u, not -e: failing commands are behavior for the harness to @@ -291,4 +292,80 @@ check "unknown flag refused" 1 "unknown argument" \ check "--source without a directory refused" 1 "no such directory" \ in_consumer env-wired --check --source "$TMP/does-not-exist" +# --- the fetch path: which forge, and never a guessed one (#201) --------------- + +# The fetch path had no coverage at all before this: every row above passes +# --source, which overrides the fetch entirely, so the URL the tool actually +# builds was asserted nowhere. It is asserted here with a PATH-stubbed curl +# that records the URL and serves a tarball of the fake source tree — no +# network, and the real tar pipeline still runs, so --strip-components stays +# honest. CURL_FAIL makes the stub fail the way a missing ref does. +FETCHBIN="$TMP/fetchbin" +mkdir -p "$FETCHBIN" +cat >"$FETCHBIN/curl" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "${!#}" >>"$CURL_URL_LOG" +[ -z "${CURL_FAIL:-}" ] || exit 22 +exec tar -cz -C "$(dirname "$CURL_SRC")" "$(basename "$CURL_SRC")" +STUB +chmod +x "$FETCHBIN/curl" + +export CURL_URL_LOG="$TMP/curl-urls" CURL_SRC="$SRC" + +consumer fetched 0.4.1 + +# fetch_sync — the fetch path, no --source. Truncates +# the URL log first so requested_url always answers about this run. +fetch_sync() { + local server="$1" + shift + : >"$CURL_URL_LOG" + (cd "$TMP/fetched" && PATH="$FETCHBIN:$PATH" GITHUB_SERVER_URL="$server" \ + bash "$SCRIPT" "$@") +} +requested_url() { cat "$CURL_URL_LOG"; } + +check "the fetch mirrors the pin fetched from the forge in the environment" 0 \ + "added .ceremony/RULES.md" fetch_sync https://forgejo.example.test --fix +check "...and the URL asked for names that forge, not a built-in one" 0 \ + "https://forgejo.example.test/heavy-duty/ceremony/archive/0.4.1.tar.gz" \ + requested_url + +# One pin ref, two forges, two trees — the whole reason #201 exists. The same +# consumer must fetch from whichever forge it is running on. +fetch_sync https://github.com --fix >/dev/null 2>&1 +check "the same pin on another forge fetches from that forge instead" 0 \ + "https://github.com/heavy-duty/ceremony/archive/0.4.1.tar.gz" requested_url + +fetch_sync https://forgejo.example.test/ --fix >/dev/null 2>&1 +check "a trailing slash on the server URL does not double the separator" 0 \ + "https://forgejo.example.test/heavy-duty/ceremony/archive/0.4.1.tar.gz" \ + requested_url + +# Unset is not github.com. A tool that never guesses a ref must not guess a +# forge either — and it must refuse BEFORE reaching for the network. +no_server_sync() { + : >"$CURL_URL_LOG" + (cd "$TMP/fetched" && PATH="$FETCHBIN:$PATH" \ + env -u GITHUB_SERVER_URL bash "$SCRIPT" --check) +} +check "no GITHUB_SERVER_URL and no --source → refuse, naming the variable" 1 \ + "GITHUB_SERVER_URL is unset" no_server_sync +check "...and the refusal says it never guesses a forge" 1 \ + "never guesses a forge" no_server_sync +nothing_fetched() { [ ! -s "$CURL_URL_LOG" ]; } +check "...and nothing was fetched before refusing" 0 "" nothing_fetched + +# A ref that does not resolve on the forge in play: the message must name the +# URL actually attempted, so "does the pinned ref exist" is answerable. +fetch_fail() { + : >"$CURL_URL_LOG" + (cd "$TMP/fetched" && PATH="$FETCHBIN:$PATH" CURL_FAIL=1 \ + GITHUB_SERVER_URL=https://forgejo.example.test bash "$SCRIPT" --check) +} +check "a failed fetch names the URL it tried" 1 \ + "https://forgejo.example.test/heavy-duty/ceremony/archive/0.4.1.tar.gz" fetch_fail +check "...and asks about the ref on that forge, not in the abstract" 1 \ + "exist on that forge" fetch_fail + summary