ci(bump-callers): move every caller roster from an Actions variable to a secret (BE-6472)

.github/bump-callers/README.md

@@ -21,7 +21,7 @@ forward automatically instead of silently drifting commits behind.
 
 ## The fleets
 
-| Entrypoint | Triggers on a change to | Caller variable | Seeded |
+| Entrypoint | Triggers on a change to | Caller secret | Seeded |
 |---|---|---|---|
 | [`bump-cursor-review-callers.yml`](../workflows/bump-cursor-review-callers.yml) | `cursor-review.yml` or `cursor-review/**` | `CURSOR_REVIEW_CALLERS` | non-empty (hard-fails if empty) |
 | [`bump-agents-md-callers.yml`](../workflows/bump-agents-md-callers.yml) | `agents-md-integrity.yml` or `agents-md-integrity/**` | `AGENTS_MD_CALLERS` | empty `[]` (grows as callers land) |
@@ -36,21 +36,23 @@ forward automatically instead of silently drifting commits behind.
 
 Its 12 live callers are known and correctly wired (each pins a full 40-hex SHA
 against this repo's path, i.e. exactly what the rewrite moves). It is unseeded
-anyway, on purpose.
-
-Every roster reaches `bump-callers.sh` through the step's `env:` block, and
-Actions prints that block — values and all — before the script's `::add-mask::`
-can run. This repo is public, so each seeded fleet already publishes its roster
-in a world-readable log. That is the known gap documented in every `bump-*`
-header. The difference here is that two of this fleet's callers are non-public
-repos that appear in **no** already-seeded roster, so seeding would publish two
-names that are not out yet — and a public log entry cannot be unpublished.
-
-So the trade is: seed now and take an irreversible disclosure, or leave it
-unseeded and take a red run. The red run is reversible and is already the
-designed behaviour for an empty roster, so it wins. Seed the variable as the
-immediate follow-on to the masking fix — never as a way to turn that red run
-green.
+anyway — originally on purpose, and now only because seeding it is a separate,
+deliberate step that has not happened yet.
+
+The original reason was the run-log gap. Every roster reaches `bump-callers.sh`
+through the step's `env:` block, and Actions prints that block — values and all —
+before the script's `::add-mask::` can run. This repo is public, so while the
+rosters were repo **variables**, each seeded fleet published its roster in a
+world-readable log. Two of this fleet's callers are non-public repos that appear
+in **no** other roster, so seeding would have published two names that were not
+out yet — and a public log entry cannot be unpublished, while a red run can. So
+the red run won.
+
+**That blocker is gone** (BE-6472): every roster is a repo **secret** now, and
+the runner masks a secret everywhere, that env dump included. Seeding this fleet
+is unblocked and is tracked as the follow-on — do it deliberately, with
+`gh secret set` from the canonical `callers.json`, never reflexively to turn the
+red run green.
 
 ### Reusables with no fleet — deliberate, not an oversight
 
@@ -66,16 +68,16 @@ not hypothetical — the groom fleet omitted *its own* caller (`ci-groom.yml`) f
 `GROOM_CALLERS`, the pin sat unchanged from the day it was written, and the caller
 ended up failing at startup against a reusable it had drifted away from. Before
 adding a caller anywhere, check that its fleet exists **and** that the repo is in
-the variable; the second half is the one people skip.
+its roster secret; the second half is the one people skip.
 
 They stay as thin entrypoints rather than one matrix because their triggers
 differ: a `cursor-review.yml` change must not spuriously bump agents-md or
 pr-size callers, and vice versa. Everything else (masking, the PR-per-caller
 flow, the trailing-newline fix, the single-line PR body) lives once in
 `bump-callers.sh`. Registering a new fleet is: add a thin entrypoint (copy an
 existing one, swap the path filter + `VAR_NAME`/`TAG`/`WORKFLOW_FILE`/
-`ALLOW_EMPTY`), seed its variable, and add a row to this table + the paths in
-`test-bump-callers.yml`. **Then `workflow_dispatch` the new entrypoint once.**
+`ALLOW_EMPTY`), seed its roster secret, and add a row to this table + the paths
+in `test-bump-callers.yml`. **Then `workflow_dispatch` the new entrypoint once.**
 Landing a fleet does not touch the reusable it watches, so its own merge matches
 no path filter and fires no run — callers that were already stale when the fleet
 was created stay stale until the reusable next changes. Every entrypoint carries
@@ -145,29 +147,89 @@ value survives to be compared), and the `workflows_ref` key needs a real left
 boundary, so a longer key that merely ends in it (`upstream_workflows_ref: v1`)
 is not read as this repo's pin.
 
-## The caller variables
+## The caller secrets
 
 This repo is **public** — the workflow files and Actions run logs are both
 publicly viewable — and most callers are private, so caller names must never
 appear in a committed file or in the logs. Each fleet's caller list lives in a
-repo-level Actions **variable** (config, not a credential) as a JSON array of
-`{"repo","file","label"}` objects (`label` optional). `bump-callers.sh`
-`::add-mask::`es every repo name out of the run logs before echoing it.
+repo-level Actions **secret** as a JSON array of `{"repo","file","label"}`
+objects (`label` optional). `bump-callers.sh` `::add-mask::`es every repo name
+out of the run logs before echoing it.
+
+### Why a secret and not a variable (BE-6472)
+
+These rosters are configuration, not credentials, so a variable is the intuitive
+home — and that is what they were until BE-6472. The problem is *how* the roster
+reaches the script: each entrypoint hands it over through the step's `env:`
+block, and Actions prints a step's env block **before** the step runs, so the raw
+value landed in this public repo's log ahead of any masking the script could
+possibly do. A live run of one of these fleets rendered `GH_TOKEN: ***` directly
+above an unmasked `CALLERS_JSON` in the same block — which is also the proof that
+a secret fixes it: the runner masked the secret in the very dump that printed the
+variable in full. (The run is cited in the ticket rather than here; this file is
+public, and a pointer to a log still holding the old roster is not something to
+publish.)
+
+The obvious alternative — fetch the roster at run time with `gh variable get`
+under a permission-narrowed app token — is **not implementable**:
+`actions/create-github-app-token` has no `permission-variables` input at any
+version, and the underlying `POST /app/installations/{id}/access_tokens` API has
+no variables permission key at all. A narrowed token can never read a variable.
+
+The cost of the move is **read-back**: there is no `gh secret get`, so you cannot
+diff what a fleet holds against what you meant to set. Two things cover that:
+
+- the canonical `callers.json` in a private infra/ops repo is the **sole** source
+  of truth (it always should have been), and
+- every run that gets past roster validation logs a line of the form
+  `roster: <N> caller(s), sha256 <digest>` (a run that hard-fails on a missing or
+  malformed roster says so in its error instead), which you reproduce from the
+  canonical file with
+
+  ```bash
+  jq -cS . callers.json | sha256sum
+  ```
+
+  Equal digests mean the fleet ran exactly that roster. The digest is taken over
+  the **canonical** (`jq -cS`) form, not the raw secret bytes, so pretty-printing,
+  key order and a trailing newline cannot make the same roster fingerprint two
+  ways; array order is preserved, since that is the order repos are bumped in.
+  A fleet that no-ops on an empty roster (`ALLOW_EMPTY: true`) logs the line too,
+  as `roster: 0 caller(s), sha256 n/a (roster empty or unset)` — there is no
+  canonical form to hash, and the point of printing it anyway is that the run
+  shape you most need to recognize is not the one with no audit line at all.
+
+  What the digest gives an outside reader, stated precisely: a sha256 is not
+  reversible, but it **is** a check function, so a guess can be tested against it
+  offline. The preimage is the entire canonical array — every repo, its file path
+  and label, and their order — so a confirmable guess means reconstructing the
+  whole roster verbatim, not testing whether one repo is a member; the count is
+  the only bound on that space. Closing even this residual means a keyed HMAC and
+  an operator-held fingerprint key (which the reproduction command would then
+  need too) — deliberately not done, and worth revisiting only if a roster's
+  contents ever become guessable in bulk.
+
+Adding/removing a caller still needs **no public commit** — set the secret:
 
-> **Known gap.** Each entrypoint hands the roster to the script through the
-> step's `env:`, and Actions prints a step's env block *before* the step runs —
-> so the raw roster appears in the (public) log ahead of any masking. Closing it
-> means fetching the variable at run time (`gh variable get`) and masking it
-> before first use, which needs a token permission the fleets do not mint today.
-> It is fleet-wide; no single entrypoint can fix it. Until then, assume the
-> roster is public.
+```bash
+jq -c . callers.json | gh secret set AGENTS_MD_CALLERS --repo Comfy-Org/github-workflows
+```
+
+Keep the canonical `callers.json` in a private infra/ops repo so roster edits
+have a reviewed source of truth (the org audit log records each edit).
 
-Adding/removing a caller needs **no public commit** — edit the variable:
+**Finish the cutover by deleting the old variables.** A `*_CALLERS` variable left
+behind after its secret is seeded still holds the private roster in this repo's
+Actions config, readable by anything with variables-read access — the pre-BE-6472
+copy of the exact data the move exists to hide. Once a fleet's first post-merge
+run has logged the expected count and digest:
 
 ```bash
-gh variable set AGENTS_MD_CALLERS --repo Comfy-Org/github-workflows \
-  --body "$(jq -c . callers.json)"
+gh variable delete AGENTS_MD_CALLERS --repo Comfy-Org/github-workflows
 ```
 
-Keep the canonical `callers.json` in a private infra/ops repo so variable edits
-have a reviewed source of truth (the org audit log records each edit).
+Do that for every migrated fleet. Delete only **after** the run confirms the
+secret is good — the variable is the rollback.
+
+The `VAR_NAME` env key the entrypoints pass keeps its historical name even though
+it now names a secret; it exists only to name the roster in an error message.

.github/bump-callers/bump-callers.sh

@@ -15,17 +15,24 @@
 # This repo is PUBLIC (both the workflow files and the Actions run logs are
 # publicly viewable) and most callers are private, so caller names must never
 # appear in a committed file or in the logs. Each caller list lives in a
-# repo-level Actions variable (config, not a credential — a variable, not a
-# secret) as a JSON array of {"repo","file","label","wire_bot"} objects, and
-# every repo name is `::add-mask::`ed out of the run logs before it is ever
-# echoed.
+# repo-level Actions SECRET as a JSON array of {"repo","file","label","wire_bot"}
+# objects. A secret rather than a variable (BE-6472): the entrypoint hands the
+# roster to this script through the step's `env:`, and Actions prints that env
+# block BEFORE the step runs — so a variable's raw value was published in the
+# public run log ahead of the `::add-mask::` calls below, which cannot run any
+# earlier than the script itself. A secret is runner-masked everywhere, that
+# dump included. The per-repo masking below is still load-bearing: Actions masks
+# the roster as ONE exact string, not each repo name inside it, so a name echoed
+# on its own would otherwise print in the clear.
 #
 # Required environment:
 #   GH_TOKEN       Token with contents:write + pull-requests:write on the callers
 #                  (the Cloud Code Bot app token generated by the entrypoint).
 #   NEW_SHA        The github-workflows commit to pin callers to (github.sha).
-#   CALLERS_JSON   The caller list (value of the fleet's Actions variable).
-#   VAR_NAME       Name of that variable — used only in error messages.
+#   CALLERS_JSON   The caller list (value of the fleet's Actions secret).
+#   VAR_NAME       Name of that secret — used only in error messages. (Named
+#                  VAR_NAME for its whole history; kept so every entrypoint does
+#                  not have to churn, and it still names the roster to fix.)
 #   TAG            Human tag for branch/commit/PR text (e.g. "cursor-review").
 #   WORKFLOW_FILE  Reusable workflow filename referenced in the PR body.
 # Optional:
@@ -40,7 +47,7 @@
 #                    consulted for an entry whose `wire_bot` field is true; unset
 #                    (the agents-md-integrity entrypoint never sets it) means no
 #                    fleet here uses wiring, so a stray `wire_bot: true` in that
-#                    fleet's variable is a harmless no-op with a warning, not a
+#                    fleet's roster is a harmless no-op with a warning, not a
 #                    failure — this keeps the script itself cursor-review-agnostic.
 
 # NOTE: deliberately no `set -e`. Each caller is bumped independently in
@@ -125,17 +132,25 @@ STRIPPED="${CALLERS_JSON//[[:space:]]/}"
 # hard-fail so a silent no-op dispatcher (which would leave every caller
 # un-bumped) is impossible.
 if [[ "$ALLOW_EMPTY" == "true" ]] && { [[ -z "$STRIPPED" ]] || [[ "$STRIPPED" == "[]" ]]; }; then
+  # Log the fingerprint line HERE too, so "every run prints one" (README,
+  # AGENTS.md) holds on the no-op path as well. Without it the one run shape an
+  # operator most needs to recognize — a fleet that bumped nothing — is the one
+  # with no audit line at all, indistinguishable at a glance from a run that
+  # died before reaching the fingerprint. There is no digest to print: an empty
+  # roster has no canonical form, and hashing "" would publish one fixed
+  # well-known constant that says nothing.
+  echo "roster: 0 caller(s), sha256 n/a (roster empty or unset)"
   echo "${VAR_NAME} has no callers yet — nothing to bump for ${TAG}."
   exit 0
 fi
 
 # Load the caller list. Hard-fail loudly on a missing/empty/invalid value.
 if [[ -z "$STRIPPED" ]]; then
-  echo "::error::${VAR_NAME} variable is missing or empty. Seed it with the caller list — see this workflow's header comment for the update flow."
+  echo "::error::${VAR_NAME} secret is missing or empty. Seed it with the caller list — see this workflow's header comment for the update flow."
   exit 1
 fi
 if ! jq -e 'type == "array" and length > 0 and all(.[]; (.repo | type == "string" and . != "") and (.file | type == "string" and . != "") and (.wire_bot == null or (.wire_bot | type == "boolean")))' <<<"$CALLERS_JSON" >/dev/null 2>&1; then
-  echo "::error::${VAR_NAME} is not a non-empty JSON array of {repo,file,label,wire_bot} objects (each needing a non-empty repo and file, and a boolean wire_bot if present). Fix the variable — see the workflow header."
+  echo "::error::${VAR_NAME} is not a non-empty JSON array of {repo,file,label,wire_bot} objects (each needing a non-empty repo and file, and a boolean wire_bot if present). Fix the secret — see the workflow header."
   exit 1
 fi
 
@@ -155,9 +170,61 @@ if (( ${#CALLERS[@]} == 0 )); then
   exit 1
 fi
 
+# Roster fingerprint — the ONE auditable trace of WHICH roster this run used.
+# Moving the roster into a secret (BE-6472) costs read-back: there is no
+# `gh secret get`, so an operator can no longer diff what the fleet holds against
+# the canonical callers.json. The digest restores that without disclosure — it
+# CONFIRMS a roster you already hold. Print the count too: "9 caller(s)" is the
+# cheap sanity check that catches a truncated or half-pasted roster at a glance.
+#
+# What it does NOT give an outside reader, stated precisely rather than as
+# "reveals nothing": a sha256 is not reversible, but it IS a check function, so
+# anyone can test a guess against it offline. The preimage is the ENTIRE
+# canonical array — every repo name, its file path and label, and the order they
+# appear in — so a confirmable guess means reconstructing the whole roster
+# verbatim, not testing whether one repo is a member. That is a far weaker
+# oracle than a per-name one, and the count is the only bound on it. If that
+# residual ever needs closing, the fix is a keyed HMAC (an operator-held
+# fingerprint key, which the reproduction command below would then need too) —
+# deliberately not done here, since it buys a new shared secret and a silent
+# fallback path for a guess space this size.
+# Keep it a HASH — never a sample, a prefix, or a per-repo listing, all of which
+# would re-leak into the public log the very names the secret move just removed.
+#
+# Hash the CANONICAL form (`jq -cS`), not the raw secret bytes, so the digest is
+# actually reproducible by the operator holding callers.json:
+#
+#     jq -cS . callers.json | sha256sum        # must equal the digest logged here
+#
+# A raw-bytes hash would not be. `gh secret set` stores whatever bytes it is fed,
+# and the two ways people set one differ — `printf '%s' "$v" | gh secret set`
+# stores no trailing newline, a `jq -c … | gh secret set` pipe does — so the same
+# roster could fingerprint two ways and an operator comparing them would see a
+# spurious mismatch. Canonicalizing also absorbs pretty-printing and key order,
+# neither of which the script cares about. It deliberately does NOT absorb array
+# order, which is semantic (it is the order repos are bumped in). Safe to run
+# unguarded: the jq validation above already proved this parses.
+roster_sha256() {
+  # sha256sum is coreutils (the ubuntu-latest runner); shasum is the macOS/BSD
+  # spelling, for contributors running the suite locally. Both print
+  # "<hex>  <file>", hence the cut.
+  if command -v sha256sum >/dev/null 2>&1; then sha256sum; else shasum -a 256; fi
+}
+# Compute into a variable and SHAPE-CHECK it rather than inlining the pipeline in
+# the echo. Inlined, `echo` reports the exit status of `echo` — so if neither
+# hash command existed (or jq/cut failed), the substitution would come back empty
+# and the line would print `sha256 ` with nothing after it, on a green run, with
+# no `set -e` to catch it. An operator comparing digests would then read a
+# silently missing value as a mismatch. Say "unavailable" out loud instead.
+ROSTER_DIGEST=$(jq -cS . <<<"$CALLERS_JSON" | roster_sha256 | cut -d' ' -f1)
+if [[ ! "$ROSTER_DIGEST" =~ ^[0-9a-f]{64}$ ]]; then
+  ROSTER_DIGEST="unavailable (no sha256sum or shasum on PATH)"
+fi
+echo "roster: ${#CALLERS[@]} caller(s), sha256 ${ROSTER_DIGEST}"
+
 # Bump ONE caller repo, committing EVERY file that repo pins onto a single
 # stable bump branch. Called once per repo with that repo's entries, so a repo
-# listed more than once in the caller variable (a monorepo pinning the reusable
+# listed more than once in the caller roster (a monorepo pinning the reusable
 # workflow from several workflow files) lands ALL its files on ONE branch/PR.
 # Every external call is guarded, so a bad repo returns non-zero here (caught by
 # the loop) instead of killing the run. A repo whose files are all missing or