Skip to content

Commit 661beb5

Browse files
committed
✅ test(ci): correct the env bounding claim and close its three side routes
The round-6 comments claimed the bound was on declared bindings in general -- "so there is no declared binding the harness fails to model". It is not. It is a bound on three `env:` maps, and three declared routes reach a `run:` body without touching any of them, each producing a live fail-open while the gate exits 0: * `jobs.<id>.container.env` -- a container on the `test` reporter plus one guard line makes the required context unconditionally green. The injection is at job level and appears in neither `.env` nor `.jobs[job].env`, and `steps | length` cannot see it either. * a prior step writing `$GITHUB_ENV` -- an extra step in BOTH classifiers, so the byte-identity check still passes, exporting a glob a one-line guard then reads. A `Taskfile.yml`-only PR classifies docs-only. The reporters were safe only because check_reporter pins them to one step; the classifier had no counterpart. * the step's `shell:` -- never asserted anywhere while the harness hardcodes bash. `shell: bash -n {0}` makes GitHub syntax-check the body and exit 0 forever. None is present in the shipped files, so this is latent. What is not latent is the claim: this lane's whole history is the distance between a claimed bound and an actual one, and putting a false bounding claim into the file that IS the record of what is bounded is that failure once more -- it is what the next reviewer would trust instead of re-deriving. So the three sentences now say the bound is on the three `env:` maps, and say plainly what it does not establish. The three routes are blocked too, since they are cheap: `container` must be null, the classifier must have exactly two steps (mirroring the reporters' one), and the executed step must declare `shell: bash` -- asserted at all three harness sites, one mutant each, killed by message. And the residual list gains (d), the class those three belong to and which remains open, and (e), that the classifier step's env VALUES are unasserted where the reporters' are pinned byte-exact -- no fail-open was constructible through them, but an asymmetry recorded is better than one assumed away. 90 assertions, 64 message-anchored mutants, 4 tautology guard rails.
1 parent 5b78189 commit 661beb5

1 file changed

Lines changed: 80 additions & 13 deletions

File tree

hack/test/ci_docs_gate_test.sh

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
# assertions below read the workflow YAML with yq, and -- for the two pieces of shell that
2828
# carry the actual decisions -- EXECUTE the exact `run:` bodies the workflows will run, with
2929
# the exact `env:` bindings the workflows declare, against a truth table and a scratch git
30-
# repository. "Exact" is enforced, not assumed: assert_step_env_exactly() pins each step's env
31-
# key set to the one its harness exports, and requires the workflow- and job-level `env:` maps
32-
# to be absent, so there is no declared binding the harness fails to model.
30+
# repository. "Exact" is enforced for the three `env:` MAPS -- assert_step_env_exactly() pins
31+
# each step's env key set to the one its harness exports and requires the workflow- and
32+
# job-level `env:` maps to be absent. That bounds those three maps. It does NOT bound the class
33+
# of declared bindings in general: a value can still reach the body by other declared routes,
34+
# and the ones known are listed under RESIDUAL ASSUMPTIONS below.
3335
#
3436
# Follows the local conventions of hack/test/dist_ci_wiring_test.sh:
3537
# * every anchor is scoped to one named job, and must resolve to exactly one match;
@@ -139,14 +141,35 @@ assert_job_can_fail_build() {
139141
# -- the gate passed while the required context `test` reported GREEN for CODE=true
140142
# RESULT=failure, past all 154 truth-table combinations.
141143
#
142-
# So the map is bounded, not sampled: the step's env keys must be EXACTLY the set the harness
143-
# exports, and neither the workflow nor the job may add more through their own `env:`. With that
144-
# held, exporting those keys and nothing else is the same thing as running the body "with the
145-
# exact env: bindings the workflow declares", which is what the header of this file claims.
144+
# So those maps are bounded, not sampled: the step's env keys must be EXACTLY the set the
145+
# harness exports, and neither the workflow nor the job may add more through their own `env:`.
146+
# Three narrow companions below block the routes that would otherwise sidestep that -- a job
147+
# `container:` (whose `env` is neither of those maps), an extra step (which could write
148+
# `$GITHUB_ENV` for a later step in the same job), and a non-`bash` `shell:`.
149+
#
150+
# What this does NOT establish, and an earlier version of this comment wrongly claimed it did:
151+
# that the harness models every declared binding. It models these maps, under these companion
152+
# assertions, for these two workflows. Other routes exist; the known ones are in RESIDUAL
153+
# ASSUMPTIONS below, and a new one belongs there rather than in a widened claim here.
146154
assert_step_env_exactly() {
147-
local workflow="$1" job="$2" idx="$3" expected="$4" label="$5"
155+
local workflow="$1" job="$2" idx="$3" expected="$4" label="$5" steps="$6"
148156
local actual
149157

158+
# A `container:` block carries its own `env`, which is neither the workflow map nor the job
159+
# map, so the key-set assertion below cannot see it. Verified live: a container on the `test`
160+
# reporter plus one guard line makes the required context unconditionally green.
161+
[[ "$(yq eval ".jobs[\"${job}\"].container" "${workflow}")" == "null" ]] ||
162+
fail "job '${job}' in ${workflow} declares a 'container:' -- its own env: reaches every step without appearing in any map this harness models, so a binding could be live in the job and absent here"
163+
# An extra step can write \$GITHUB_ENV for a later step in the same job -- a declared binding
164+
# that appears in no env: map at all. check_reporter already pins the reporters to one step;
165+
# this pins every job whose body is executed here, the classifier included.
166+
[[ "$(yq eval ".jobs[\"${job}\"].steps | length" "${workflow}")" == "${steps}" ]] ||
167+
fail "job '${job}' in ${workflow} has $(yq eval ".jobs[\"${job}\"].steps | length" "${workflow}") steps, expected exactly ${steps} -- an extra step can export a binding into the environment of the ${label} step by writing \$GITHUB_ENV, which no env: map declares and this harness would not export"
168+
# The harness runs the body under bash. `shell: bash -n {0}` would make GitHub syntax-check it
169+
# and exit 0 forever, so the required context would be permanently green.
170+
[[ "$(yq eval ".jobs[\"${job}\"].steps[${idx}].shell" "${workflow}")" == "bash" ]] ||
171+
fail "the ${label} step of job '${job}' in ${workflow} declares 'shell: $(yq eval ".jobs[\"${job}\"].steps[${idx}].shell" "${workflow}")', expected 'bash' -- this harness executes the body with bash, and any other shell (or a directive like 'bash -n {0}', which only syntax-checks) means the job runs something different from what is tested here"
172+
150173
actual="$(yq eval "((.jobs[\"${job}\"].steps[${idx}].env // {}) | keys | sort | join(\",\"))" "${workflow}")"
151174
[[ "${actual}" == "${expected}" ]] ||
152175
fail "the ${label} step of job '${job}' in ${workflow} declares env keys [${actual:-none}], expected exactly [${expected}] -- this harness exports precisely those and nothing else, so any other declared binding is present when the job runs and ABSENT when the body is tested here. That is how a fail-open hides: the body reads a variable no assertion knows about."
@@ -341,7 +364,7 @@ check_reporter() {
341364

342365
# The truth table below exports CODE and RESULT. Anything else the step declares would be
343366
# live in the job and missing here -- see assert_step_env_exactly.
344-
assert_step_env_exactly "${workflow}" "${reporter}" 0 "CODE,RESULT" "report"
367+
assert_step_env_exactly "${workflow}" "${reporter}" 0 "CODE,RESULT" "report" 1
345368

346369
body="$(yq eval ".jobs[\"${reporter}\"].steps[0].run" "${workflow}")"
347370
[[ -n "${body}" && "${body}" != "null" ]] ||
@@ -603,8 +626,27 @@ check_docs_set_mirrors_paths_ignore() {
603626
# the harness rather than reading a declared input -- keying on `$GITHUB_JOB`, say, which
604627
# differs between the `changes` job and the `lint` job this gate runs in -- diverges here
605628
# from the job with no `env:` change at all, so assert_step_env_exactly() cannot see it.
606-
# That is different in kind from a declared binding the harness fails to model (which IS
607-
# bounded, above): closing it would require running the real job, which no gate can do.
629+
# Closing it would require running the real job, which no gate can do.
630+
# (d) DECLARED BINDINGS ARE BOUNDED BY MAP, NOT BY CLASS. assert_step_env_exactly() bounds the
631+
# three `env:` maps (step, job, workflow) and blocks three specific side routes, each of
632+
# which was verified to produce a live fail-open at gate exit 0, and none of which is
633+
# present in the shipped files:
634+
# * `jobs.<id>.container.env` -- a container on the `test` reporter plus one guard line
635+
# makes the required context unconditionally green; the injection is at job level and
636+
# appears in neither `.env` nor `.jobs[job].env`. Blocked by requiring `container` null.
637+
# * a prior step writing `$GITHUB_ENV` -- an extra step in BOTH classifiers (so the
638+
# byte-identity check still passes) exporting a glob, plus a one-line guard, made a
639+
# `Taskfile.yml`-only PR classify docs-only. Blocked by pinning the step count.
640+
# * the step's `shell:` -- `shell: bash -n {0}` makes GitHub syntax-check the body and
641+
# exit 0 forever. Blocked by requiring `shell: bash`.
642+
# Those three are closed for these two workflows. The CLASS is not: GitHub has other ways
643+
# to put a value in front of a `run:` body, and a newly found one belongs in this list
644+
# rather than in a widened claim next to assert_step_env_exactly().
645+
# (e) The classifier step's env VALUES are unasserted -- only its key set is -- unlike the
646+
# reporters, whose CODE/RESULT expressions are pinned byte-exact. No fail-open was
647+
# constructible through them (every substitution collapses to a non-`pull_request` event,
648+
# an empty SHA, a failed diff or an empty diff, all of which emit `true`), but the
649+
# asymmetry is coverage that is not there, so it is recorded rather than assumed away.
608650
check_docs_set_behaves_like_paths_ignore() {
609651
local workflow="$1"
610652
local idx body tmpdir funcs driver corpus verdicts repo
@@ -626,7 +668,7 @@ check_docs_set_behaves_like_paths_ignore() {
626668
fail "${workflow}'s push paths-ignore list is empty -- the reference matcher would call every path code and this check would pass for the wrong reason"
627669

628670
idx="$(job_step_index "${workflow}" changes '.value.id == "filter"' 'id: filter classification')" || exit 1
629-
assert_step_env_exactly "${workflow}" changes "${idx}" "BASE_SHA,EVENT_NAME,HEAD_SHA" "classification"
671+
assert_step_env_exactly "${workflow}" changes "${idx}" "BASE_SHA,EVENT_NAME,HEAD_SHA" "classification" 2
630672
body="$(yq eval ".jobs.changes.steps[${idx}].run" "${workflow}")"
631673
tmpdir="$(mktemp -d)"
632674
printf '%s\n' "${body}" >"${tmpdir}/filter.sh"
@@ -769,7 +811,7 @@ check_changes_filter() {
769811
idx="$(job_step_index "${workflow}" changes '.value.id == "filter"' 'id: filter classification')" || exit 1
770812
assert_step_can_fail_build "${workflow}" changes "${idx}" "classification"
771813
# run_filter below exports EVENT_NAME/BASE_SHA/HEAD_SHA and nothing else.
772-
assert_step_env_exactly "${workflow}" changes "${idx}" "BASE_SHA,EVENT_NAME,HEAD_SHA" "classification"
814+
assert_step_env_exactly "${workflow}" changes "${idx}" "BASE_SHA,EVENT_NAME,HEAD_SHA" "classification" 2
773815

774816
body="$(yq eval ".jobs.changes.steps[${idx}].run" "${workflow}")"
775817
[[ -n "${body}" && "${body}" != "null" ]] ||
@@ -1196,6 +1238,31 @@ mutant_rejected "${CI_WORKFLOW}" "${MUTANTS}/filter-undeclared-env.yaml" \
11961238
'expected exactly [BASE_SHA,EVENT_NAME,HEAD_SHA]' \
11971239
check_changes_filter "${MUTANTS}/filter-undeclared-env.yaml"
11981240

1241+
# The three side routes that reach a `run:` body without appearing in any env: map. Each was
1242+
# built as a live fail-open and each passed the round-6 gate at exit 0; none is present in the
1243+
# shipped files. See RESIDUAL ASSUMPTIONS (d) for what these do and do not settle.
1244+
yq eval '.jobs.test.container = {"image": "alpine:3.22", "env": {"MAINT_MODE": "on"}}' "${CI_WORKFLOW}" \
1245+
>"${MUTANTS}/reporter-container-env.yaml"
1246+
mutant_rejected "${CI_WORKFLOW}" "${MUTANTS}/reporter-container-env.yaml" \
1247+
"the 'test' reporter job gains a container: whose own env reaches the report step" \
1248+
"declares a 'container:'" \
1249+
check_reporter "${MUTANTS}/reporter-container-env.yaml" test test-suite "test"
1250+
1251+
# shellcheck disable=SC2016 # a yq program and a literal $GITHUB_ENV, not shell expansions
1252+
yq eval '.jobs.changes.steps += [{"name": "inject", "shell": "bash", "run": "echo EXTRA_DOCS_GLOB=*.yml >>\"${GITHUB_ENV}\""}]' \
1253+
"${CI_WORKFLOW}" >"${MUTANTS}/classifier-extra-step.yaml"
1254+
mutant_rejected "${CI_WORKFLOW}" "${MUTANTS}/classifier-extra-step.yaml" \
1255+
'the classifier job gains a step that writes a binding into GITHUB_ENV' \
1256+
'expected exactly 2 -- an extra step can export a binding' \
1257+
check_changes_filter "${MUTANTS}/classifier-extra-step.yaml"
1258+
1259+
yq eval '.jobs.test.steps[0].shell = "bash -n {0}"' "${CI_WORKFLOW}" \
1260+
>"${MUTANTS}/reporter-syntax-check-shell.yaml"
1261+
mutant_rejected "${CI_WORKFLOW}" "${MUTANTS}/reporter-syntax-check-shell.yaml" \
1262+
"the 'test' reporter step is run under 'bash -n {0}', which only syntax-checks it" \
1263+
"expected 'bash'" \
1264+
check_reporter "${MUTANTS}/reporter-syntax-check-shell.yaml" test test-suite "test"
1265+
11991266
# The same injection through the two maps every step inherits, so neither half of the
12001267
# "no other declared binding" assertion is left untested.
12011268
yq eval '.env.MAINT_MODE = "on"' "${CI_WORKFLOW}" >"${MUTANTS}/workflow-level-env.yaml"

0 commit comments

Comments
 (0)