From e9ca16f3a15d6b8deab1ac82601fbcb215658620 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 21:54:37 +0000 Subject: [PATCH 1/5] Stop the DNS resolver e2e tests certifying a binary that was never built The e2e scenarios build fips and fips-gateway in a builder image and extract them with two docker cp calls whose stderr went to /dev/null and whose exit status was never tested. Because build_fips_for_e2e ended in a log call it returned 0 on every path, so the caller's guard could not fire. A failed extraction left the previous run's binaries in the cache, they satisfied the caller's executable check, and the five e2e legs then exercised the previous commit's code and reported green. The extraction now deletes the cached binaries before it starts, which is what makes the stale case impossible rather than merely detected, and it checks each docker cp independently, reports the failure with docker's own message, verifies each extracted file is non-empty and executable, and returns an explicit status. This mirrors what the deb-install suite already does. Confirmed by inducing the failure rather than by observing a green run. With both extractions broken and a stale binary in place, the old code logged "Cached fips (26936 bytes)" and went on to build the runtime image around it; the new code reports both docker errors, deletes the stale binaries and fails the suite. Breaking either extraction on its own is caught separately, and an unmodified run passes 7 of 7. --- testing/dns-resolver/test.sh | 40 +++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 54ed1fa..905954e 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -270,13 +270,43 @@ DOCKERFILE fi log "Extracting fips + fips-gateway binaries from builder image" - local cid - cid=$(docker create "$builder_tag") - docker cp "$cid:/src/target/release/fips" "$FIPS_BIN_CACHE" >/dev/null 2>&1 - docker cp "$cid:/src/target/release/fips-gateway" "$FIPS_GATEWAY_BIN_CACHE" >/dev/null 2>&1 + # Drop the previous run's binaries before extracting. Without this, a failed + # extraction below leaves them in place, they satisfy the caller's -x check, + # and the e2e scenarios silently exercise the previous commit's code. + rm -f "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE" + + local cid err + if ! cid=$(docker create "$builder_tag" 2>&1); then + echo " ERROR: docker create failed: $cid" + return 1 + fi + + local rc=0 spec bin dest + for spec in "fips:$FIPS_BIN_CACHE" "fips-gateway:$FIPS_GATEWAY_BIN_CACHE"; do + bin="${spec%%:*}" + dest="${spec#*:}" + if ! err=$(docker cp "$cid:/src/target/release/$bin" "$dest" 2>&1); then + echo " ERROR: extracting $bin from the builder image failed: $err" + rc=1 + fi + done docker rm "$cid" >/dev/null - chmod +x "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE" + [ "$rc" -eq 0 ] || return 1 + + if ! chmod +x "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"; then + echo " ERROR: chmod +x failed on the extracted binaries" + return 1 + fi + + for dest in "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE"; do + if [ ! -s "$dest" ] || [ ! -x "$dest" ]; then + echo " ERROR: extracted binary missing, empty or not executable: $dest" + return 1 + fi + done + log "Cached fips ($(stat -c %s "$FIPS_BIN_CACHE") bytes) + fips-gateway ($(stat -c %s "$FIPS_GATEWAY_BIN_CACHE") bytes)" + return 0 } # ───────────────────────────────────────────────────────────────────── From 99cb0a51fd782ea8f08bf511255b11731c512524 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 21:56:41 +0000 Subject: [PATCH 2/5] Reject unknown keys when loading a chaos scenario The scenario loader read every section and member through raw.get with a default, so a mistyped key was silently ignored and the block it belonged to stayed default-constructed while the YAML still looked correct on the page. Writing "assertion:" for "assertions:" disarmed a scenario's only assertions and left it unable to return a non-zero exit code; "link_flap:" for "link_flaps:" turned off the churn the scenario existed to inject, and a calm mesh then ran under the name of a chaos test. Keys are now checked at load time, at the top level and inside every section with a fixed schema, including the netem policy bodies, the {min, max} ranges and the assertion bodies themselves. The error names the offending key, the section it appeared in, and the keys that section does understand. Three mappings are deliberately exempt because their keys are names the author chooses rather than a schema, and two sub-trees are passed through whole; all five are listed at the key sets with the reason. Adding an assertion type now means registering it, so the next person to add one gets a rejection rather than a silent no-op. That coupling is the point and it is written down where they will see it. Verified against the seventeen scenarios in the tree, all of which still load, and against fixtures for each failure mode: a top-level typo, a nested typo, a typo inside an assertion body, and a section left empty, which previously raised an unrelated AttributeError from the next line of the loader. --- testing/chaos/sim/scenario.py | 119 ++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 5 deletions(-) diff --git a/testing/chaos/sim/scenario.py b/testing/chaos/sim/scenario.py index 79a7228..139c2e0 100644 --- a/testing/chaos/sim/scenario.py +++ b/testing/chaos/sim/scenario.py @@ -239,15 +239,90 @@ class Scenario: fips_overrides: dict = field(default_factory=dict) +# Keys each fixed-schema section understands. A key absent from these sets is a +# typo, and silently ignoring it leaves the block default-constructed while the +# YAML still looks correct — a mistyped "assertion:" disarms a scenario's only +# assertions, a mistyped "link_flap:" turns off the chaos it was meant to inject. +# +# Three mappings are deliberately NOT listed, because their keys are names the +# scenario author chooses rather than a schema: netem.mutation.policies, +# link_swap.policies and topology.transport_mix. Two sub-trees are passed through +# whole and are likewise not checked: fips_overrides and topology.params. +# +# NOTE: adding a new assertion type means adding it to _ASSERTION_KEYS below and +# giving it an entry here, or scenarios using it will be rejected at load. +_TOP_KEYS = { + "scenario", "topology", "netem", "link_flaps", "traffic", "node_churn", + "peer_churn", "bandwidth", "ingress", "link_swap", "assertions", "logging", + "fips_overrides", +} +_SECTION_KEYS = { + "scenario": {"name", "seed", "duration_secs"}, + "topology": { + "num_nodes", "algorithm", "params", "ensure_connected", "subnet", + "ip_start", "default_transport", "transport_mix", + }, + "netem": {"enabled", "default_policy", "link_policies", "mutation"}, + "netem.link_policies[]": {"edges", "policy", "policy_name"}, + "netem.mutation": {"interval_secs", "fraction", "policies"}, + "link_flaps": { + "enabled", "interval_secs", "max_down_links", "down_duration_secs", + "protect_connectivity", + }, + "traffic": { + "enabled", "max_concurrent", "interval_secs", "duration_secs", + "parallel_streams", + }, + "node_churn": { + "enabled", "interval_secs", "max_down_nodes", "down_duration_secs", + "protect_connectivity", + }, + "peer_churn": {"enabled", "interval_secs", "ephemeral_fraction"}, + "bandwidth": {"enabled", "tiers_mbps"}, + "ingress": {"enabled", "tiers_kbps", "burst_bytes"}, + "link_swap": {"enabled", "interval_secs", "policies", "edges"}, + "link_swap.edges[]": {"edge", "policy"}, + "assertions": {"bloom_send_rate", "min_parent_switches"}, + "logging": {"rust_log", "output_dir"}, +} +_ASSERTION_KEYS = { + "bloom_send_rate": {"window_secs", "max_per_node"}, + "min_parent_switches": {"min_total"}, +} +_NETEM_POLICY_KEYS = { + "delay_ms", "jitter_ms", "loss_pct", "duplicate_pct", "reorder_pct", + "corrupt_pct", +} +_RANGE_KEYS = {"min", "max"} + + +def _reject_unknown(data, known, context: str): + """Raise unless every key in `data` is one the loader understands.""" + if data is None: + raise ValueError( + f"{context}: section is present but empty; give it a body or remove it" + ) + if not isinstance(data, dict): + raise ValueError(f"{context}: expected a mapping, got {type(data).__name__}") + unknown = sorted(str(k) for k in set(data) - set(known)) + if unknown: + raise ValueError( + f"{context}: unknown key(s): {', '.join(unknown)} " + f"(known: {', '.join(sorted(known))})" + ) + + def _parse_range(data, name: str) -> Range: """Parse a {min, max} dict into a Range.""" if isinstance(data, dict): + _reject_unknown(data, _RANGE_KEYS, name) return Range(min=float(data["min"]), max=float(data["max"])) raise ValueError(f"{name}: expected {{min, max}} dict, got {type(data).__name__}") -def _parse_netem_policy(data: dict) -> NetemPolicy: +def _parse_netem_policy(data: dict, name: str = "netem policy") -> NetemPolicy: """Parse a netem policy from a dict with [min, max] lists or {min, max} dicts.""" + _reject_unknown(data, _NETEM_POLICY_KEYS, name) policy = NetemPolicy() for attr in ( "delay_ms", @@ -273,16 +348,22 @@ def load_scenario(path: str) -> Scenario: with open(path) as f: raw = yaml.safe_load(f) + if raw is None: + raise ValueError(f"{path}: file is empty") + _reject_unknown(raw, _TOP_KEYS, "(top level)") + s = Scenario() # Scenario section sc = raw.get("scenario", {}) + _reject_unknown(sc, _SECTION_KEYS["scenario"], "scenario") s.name = sc.get("name", os.path.splitext(os.path.basename(path))[0]) s.seed = int(sc.get("seed", 42)) s.duration_secs = int(sc.get("duration_secs", 120)) # Topology section tc = raw.get("topology", {}) + _reject_unknown(tc, _SECTION_KEYS["topology"], "topology") s.topology.num_nodes = int(tc.get("num_nodes", 10)) s.topology.algorithm = tc.get("algorithm", "random_geometric") s.topology.params = tc.get("params", {}) @@ -298,33 +379,43 @@ def load_scenario(path: str) -> Scenario: # Netem section nc = raw.get("netem", {}) + _reject_unknown(nc, _SECTION_KEYS["netem"], "netem") s.netem.enabled = nc.get("enabled", False) if "default_policy" in nc: - s.netem.default_policy = _parse_netem_policy(nc["default_policy"]) + s.netem.default_policy = _parse_netem_policy( + nc["default_policy"], "netem.default_policy" + ) if "link_policies" in nc: for lp_data in nc["link_policies"]: + _reject_unknown( + lp_data, _SECTION_KEYS["netem.link_policies[]"], "netem.link_policies[]" + ) override = LinkPolicyOverride( edges=lp_data.get("edges", []), ) if "policy" in lp_data: - override.policy = _parse_netem_policy(lp_data["policy"]) + override.policy = _parse_netem_policy( + lp_data["policy"], "netem.link_policies[].policy" + ) if "policy_name" in lp_data: override.policy_name = lp_data["policy_name"] s.netem.link_policies.append(override) if "mutation" in nc: mc = nc["mutation"] + _reject_unknown(mc, _SECTION_KEYS["netem.mutation"], "netem.mutation") s.netem.mutation.interval_secs = _parse_range( mc.get("interval_secs", {"min": 15, "max": 30}), "netem.mutation.interval_secs" ) s.netem.mutation.fraction = float(mc.get("fraction", 0.3)) if "policies" in mc: s.netem.mutation.policies = { - name: _parse_netem_policy(pdata) + name: _parse_netem_policy(pdata, f"netem.mutation.policies.{name}") for name, pdata in mc["policies"].items() } # Link flaps section lf = raw.get("link_flaps", {}) + _reject_unknown(lf, _SECTION_KEYS["link_flaps"], "link_flaps") s.link_flaps.enabled = lf.get("enabled", False) if "interval_secs" in lf: s.link_flaps.interval_secs = _parse_range(lf["interval_secs"], "link_flaps.interval_secs") @@ -337,6 +428,7 @@ def load_scenario(path: str) -> Scenario: # Traffic section tf = raw.get("traffic", {}) + _reject_unknown(tf, _SECTION_KEYS["traffic"], "traffic") s.traffic.enabled = tf.get("enabled", False) s.traffic.max_concurrent = int(tf.get("max_concurrent", 3)) if "interval_secs" in tf: @@ -347,6 +439,7 @@ def load_scenario(path: str) -> Scenario: # Node churn section nc2 = raw.get("node_churn", {}) + _reject_unknown(nc2, _SECTION_KEYS["node_churn"], "node_churn") s.node_churn.enabled = nc2.get("enabled", False) if "interval_secs" in nc2: s.node_churn.interval_secs = _parse_range(nc2["interval_secs"], "node_churn.interval_secs") @@ -359,6 +452,7 @@ def load_scenario(path: str) -> Scenario: # Peer churn section pc = raw.get("peer_churn", {}) + _reject_unknown(pc, _SECTION_KEYS["peer_churn"], "peer_churn") s.peer_churn.enabled = pc.get("enabled", False) if "interval_secs" in pc: s.peer_churn.interval_secs = _parse_range(pc["interval_secs"], "peer_churn.interval_secs") @@ -366,6 +460,7 @@ def load_scenario(path: str) -> Scenario: # Bandwidth section bw = raw.get("bandwidth", {}) + _reject_unknown(bw, _SECTION_KEYS["bandwidth"], "bandwidth") s.bandwidth.enabled = bw.get("enabled", False) if "tiers_mbps" in bw: tiers = bw["tiers_mbps"] @@ -375,6 +470,7 @@ def load_scenario(path: str) -> Scenario: # Ingress section ig = raw.get("ingress", {}) + _reject_unknown(ig, _SECTION_KEYS["ingress"], "ingress") s.ingress.enabled = ig.get("enabled", False) if "tiers_kbps" in ig: tiers = ig["tiers_kbps"] @@ -385,18 +481,22 @@ def load_scenario(path: str) -> Scenario: # Link swap section (deterministic asymmetric link-cost flapping). ls = raw.get("link_swap", {}) + _reject_unknown(ls, _SECTION_KEYS["link_swap"], "link_swap") s.link_swap.enabled = ls.get("enabled", False) if "interval_secs" in ls: s.link_swap.interval_secs = float(ls["interval_secs"]) if "policies" in ls: s.link_swap.policies = { - name: _parse_netem_policy(pdata) + name: _parse_netem_policy(pdata, f"link_swap.policies.{name}") for name, pdata in ls["policies"].items() } if "edges" in ls: for edata in ls["edges"]: if not isinstance(edata, dict): raise ValueError("link_swap.edges entries must be dicts") + _reject_unknown( + edata, _SECTION_KEYS["link_swap.edges[]"], "link_swap.edges[]" + ) edge = str(edata.get("edge", "")) policy = str(edata.get("policy", "")) if not edge or not policy: @@ -405,20 +505,29 @@ def load_scenario(path: str) -> Scenario: # Assertions section (post-run control-socket-based checks). asrt = raw.get("assertions", {}) + _reject_unknown(asrt, _SECTION_KEYS["assertions"], "assertions") if "bloom_send_rate" in asrt: bsr = asrt["bloom_send_rate"] + _reject_unknown( + bsr, _ASSERTION_KEYS["bloom_send_rate"], "assertions.bloom_send_rate" + ) s.assertions.bloom_send_rate = BloomSendRateAssertion( window_secs=int(bsr.get("window_secs", 30)), max_per_node=int(bsr.get("max_per_node", 30)), ) if "min_parent_switches" in asrt: mps = asrt["min_parent_switches"] + _reject_unknown( + mps, _ASSERTION_KEYS["min_parent_switches"], + "assertions.min_parent_switches", + ) s.assertions.min_parent_switches = MinParentSwitchesAssertion( min_total=int(mps.get("min_total", 1)), ) # Logging section lg = raw.get("logging", {}) + _reject_unknown(lg, _SECTION_KEYS["logging"], "logging") s.logging.rust_log = lg.get("rust_log", "info") s.logging.output_dir = lg.get("output_dir", "./sim-results") From e578a11b7ab7d9e6426419b1d327b1bfcf7ed9d9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 21:59:28 +0000 Subject: [PATCH 3/5] Compare every CI leg in the parity guard, not a folded token The guard collapsed all thirteen chaos legs to the token "chaos" and all five deb-install legs to "deb-install", so sixteen of the thirty-four integration legs were invisible to it. Deleting twelve chaos legs and four deb-install legs from a copy of the workflow left it still reporting "CI parity OK". The fold existed because the guard compared the cosmetic suite name, which really does differ between the runners; both sides have carried a directly comparable scenario field all along, so the fix is to compare that instead, along with the chaos flags. The local suite set is now discovered by sweeping ci-local.sh for suite arrays rather than reading a hardcoded list of variable names, and the deb-install distro list is read from the suite's own script. A suite dispatched with no backing array is invisible to either approach, so every run_suite arm is now checked to have one and the guard names any that does not. That is not hypothetical: next dispatches its mixed-profile suite without an array, and the guard could only report it as missing from the local runner without being able to say why. The guard also now refuses to run rather than failing obscurely when python3 or pyyaml is absent, since a parity check that cannot run must not look like one that passed. Verified by breaking each thing it guards: legs deleted from the workflow, drifted chaos flags, a fabricated suite array, and a dispatch arm whose array was removed. That last case initially did not fire because the arm pattern was pinned to the wrong indentation, which the test caught. --- testing/check-ci-parity.sh | 241 ++++++++++++++++++++++++++----------- 1 file changed, 174 insertions(+), 67 deletions(-) diff --git a/testing/check-ci-parity.sh b/testing/check-ci-parity.sh index deadc6f..f66e377 100755 --- a/testing/check-ci-parity.sh +++ b/testing/check-ci-parity.sh @@ -11,19 +11,30 @@ # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # -# Granularity-only differences folded before comparison (same coverage, -# different matrix shape — NOT a divergence): -# deb-install — GitHub splits into per-distro legs (deb-install-debian12/ -# debian13/ubuntu22/ubuntu24/ubuntu26); local runs the same -# distro set in one suite. Folded to "deb-install". -# chaos-* — GitHub fans each chaos scenario into its own matrix leg -# (type: chaos); local runs them all via the one CHAOS_SUITES -# path. The individual scenario names also differ cosmetically -# between runners (e.g. chaos-smoke-10 vs churn-mixed-10). -# Folded to a single "chaos" token on both sides. -# dns-resolver — single leg / single suite both sides; runs all scenarios. +# What is compared, and at what granularity: +# chaos — per scenario, plus its flags. GitHub fans each scenario +# into its own matrix leg carrying `scenario:` (and +# optionally `chaos_flags:`); local lists the same scenarios +# in CHAOS_SUITES as "display scenario flags". The `suite:` +# names differ cosmetically between runners and are ignored +# — `scenario:` is the identity. +# deb-install — per distro. GitHub splits into per-distro legs carrying +# `scenario:`; local runs the same distro set in one suite, +# enumerated by ALL_SCENARIOS in deb-install/test.sh. +# everything else — per suite name. # -# Exit 0 = parity clean. Exit 1 = unexpected divergence (suite names printed). +# dns-resolver is the one leg still compared at leg granularity rather than +# per scenario: it is a single leg and a single suite on both sides, and it +# runs all of its scenarios internally. Its scenario list is NOT cross-checked. +# +# The local suite set is discovered by sweeping ci-local.sh for *_SUITES arrays +# rather than from a hardcoded list of variable names, and every run_suite +# dispatch arm is then checked to have a backing array — a suite dispatched +# without one is invisible to a name-list sweep, which is how a real divergence +# went unnoticed. +# +# Exit 0 = parity clean. Exit 1 = unexpected divergence. Exit 2 = the guard +# could not run (missing file or missing dependency); never treated as a pass. # ───────────────────────────────────────────────────────────────────────────── set -uo pipefail @@ -32,113 +43,209 @@ PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" CI_LOCAL="$SCRIPT_DIR/ci-local.sh" CI_YML="$PROJECT_ROOT/.github/workflows/ci.yml" +DEB_TEST="$SCRIPT_DIR/deb-install/test.sh" # Deliberate local-only allowlist (suites intentionally absent from GitHub). ALLOWLIST="tor-socks5 tor-directory" -for f in "$CI_LOCAL" "$CI_YML"; do +for f in "$CI_LOCAL" "$CI_YML" "$DEB_TEST"; do if [[ ! -f "$f" ]]; then echo "check-ci-parity: missing file: $f" >&2 exit 2 fi done -# Extract and normalize both suite sets in Python (robust YAML parse of the -# matrix; regex extraction of the bash suite arrays). Folding rules above are -# applied identically to both sides so only genuine divergence surfaces. -python3 - "$CI_LOCAL" "$CI_YML" "$ALLOWLIST" <<'PY' +if ! command -v python3 >/dev/null 2>&1; then + echo "check-ci-parity: python3 not found; cannot verify CI parity" >&2 + exit 2 +fi +if ! python3 -c "import yaml" >/dev/null 2>&1; then + echo "check-ci-parity: python3 module 'yaml' not found; cannot verify CI parity" >&2 + echo "check-ci-parity: install it with 'pip3 install pyyaml'" >&2 + exit 2 +fi + +python3 - "$CI_LOCAL" "$CI_YML" "$DEB_TEST" "$ALLOWLIST" <<'PY' import re import sys -ci_local_path, ci_yml_path, allowlist_raw = sys.argv[1], sys.argv[2], sys.argv[3] +import yaml + +ci_local_path, ci_yml_path, deb_test_path, allowlist_raw = sys.argv[1:5] allowlist = set(allowlist_raw.split()) - -def fold(name): - """Collapse granularity-only matrix shape into canonical suite identity.""" - if name.startswith("chaos-") or name == "chaos": - return "chaos" - if name.startswith("deb-install"): - return "deb-install" - return name - - -# ── Local: parse the suite arrays from ci-local.sh ─────────────────────────── with open(ci_local_path, encoding="utf-8") as fh: local_src = fh.read() +with open(deb_test_path, encoding="utf-8") as fh: + deb_src = fh.read() -def bash_array(var): +def bash_array_entries(var): + """Full entries of a bash array, in order, quotes stripped.""" m = re.search(rf"^{var}=\((.*?)\)", local_src, re.MULTILINE | re.DOTALL) if not m: return [] body = m.group(1) - # Quoted entries (chaos uses "display scenario flags"): first token is name. quoted = re.findall(r'"([^"]*)"', body) if quoted: - return [entry.split()[0] for entry in quoted if entry.strip()] + return [e.strip() for e in quoted if e.strip()] return [tok for tok in body.split() if tok.strip()] +def discovered_arrays(): + """Every *_SUITES array in ci-local.sh, by name. + + Swept rather than hardcoded: a suite whose array is not on a fixed list + would otherwise be invisible to this guard. + """ + return { + name + "_SUITES": bash_array_entries(name + "_SUITES") + for name in re.findall(r"^([A-Z_]+)_SUITES=\(", local_src, re.MULTILINE) + } + + +arrays = discovered_arrays() + +# ── Local side ─────────────────────────────────────────────────────────────── +# Chaos: "display scenario flags" — compare the scenario and its flags. +local_chaos = {} +for entry in arrays.get("CHAOS_SUITES", []): + parts = entry.split() + if len(parts) < 2: + continue + local_chaos[parts[1]] = " ".join(parts[2:]) + +# deb-install: one local suite that runs the distro set enumerated in its script. +m = re.search(r'^ALL_SCENARIOS="([^"]*)"', deb_src, re.MULTILINE) +local_deb = set(m.group(1).split()) if m else set() + +# Everything else: suite names, with NAT stored bare and prefixed at use. local = set() -# Static, rekey, gateway, sidecar, acl, firewall, nostr, stun, dns, deb. -for var in ("STATIC_SUITES", "REKEY_SUITES", "ADMISSION_SUITES", - "GATEWAY_SUITES", "SIDECAR_SUITES", "ACL_SUITES", - "FIREWALL_SUITES", "NOSTR_RELAY_SUITES", "STUN_FAULTS_SUITES", - "DNS_RESOLVER_SUITES", "DEB_INSTALL_SUITES"): - local.update(bash_array(var)) -# Chaos display names → fold to "chaos". -for _ in bash_array("CHAOS_SUITES"): - local.add("chaos") -# NAT scenarios are stored bare (cone/symmetric/lan) and prefixed nat- at use. -for scen in bash_array("NAT_SUITES"): - local.add(f"nat-{scen}") -# TOR_SUITES is the deliberate local-only set — excluded from the default path. - -local = {fold(n) for n in local} - -# ── GitHub: parse the integration matrix suite: values from ci.yml ─────────── -import yaml # noqa: E402 +for name, entries in arrays.items(): + if name in ("CHAOS_SUITES", "DEB_INSTALL_SUITES"): + continue + names = [e.split()[0] for e in entries] + if name == "NAT_SUITES": + local |= {f"nat-{n}" for n in names} + else: + local |= set(names) +# ── GitHub side ────────────────────────────────────────────────────────────── with open(ci_yml_path, encoding="utf-8") as fh: doc = yaml.safe_load(fh) include = doc["jobs"]["integration"]["strategy"]["matrix"]["include"] -github = set() +github_chaos, github_deb, github = {}, set(), set() for leg in include: if "suite" not in leg: continue - # Chaos legs carry inconsistent suite: names (chaos-smoke-10 vs - # churn-mixed-10) but a uniform type: chaos — fold via type, not name. - if str(leg.get("type", "")) == "chaos": - github.add("chaos") + kind = str(leg.get("type", "")) + if kind == "chaos": + github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", "")) + elif kind == "deb-install": + github_deb.add(str(leg["scenario"])) else: - github.add(fold(str(leg["suite"]))) + github.add(str(leg["suite"])) -# ── Diff (subtract allowlist from local before comparison) ─────────────────── +# ── Dispatch cross-check: every run_suite arm needs a backing array ────────── +# A suite dispatched without an array is invisible to the sweep above, so the +# guard would report it as GitHub-only forever without ever naming the cause. +body = re.search(r"^run_suite\(\).*?^\}", local_src, re.MULTILINE | re.DOTALL) +dispatch_uncovered = [] +if body is None: + print("check-ci-parity: could not locate run_suite() in ci-local.sh", file=sys.stderr) + sys.exit(2) + +# Arms sit at one fixed indentation inside the case block. Pin to it, taken from +# the first arm rather than assumed, so a body line that happens to end in ')' +# cannot be read as an arm. +arm_re = re.compile(r"^([ \t]+)([a-z0-9|*_-]+)\)", re.MULTILINE) +first = arm_re.search(body.group(0)) +if first is None: + print("check-ci-parity: no dispatch arms found in run_suite()", file=sys.stderr) + sys.exit(2) + +indent = first.group(1) +known = set(local) | set(local_chaos) | local_deb | {"deb-install"} +for m in arm_re.finditer(body.group(0)): + if m.group(1) != indent: + continue + for arm in m.group(2).split("|"): + if arm == "*": + continue # the unknown-suite error arm, not a suite + if arm == "chaos-*": + # Dispatches any chaos- through a fallback, so its vocabulary + # is unbounded; the chaos scenario comparison covers it instead. + continue + if arm not in known: + dispatch_uncovered.append(arm) + +# ── Diff ───────────────────────────────────────────────────────────────────── local_cmp = {n for n in local if n not in allowlist} local_only = sorted(local_cmp - github) github_only = sorted(github - local_cmp) +chaos_local_only = sorted(set(local_chaos) - set(github_chaos)) +chaos_github_only = sorted(set(github_chaos) - set(local_chaos)) +chaos_flag_drift = sorted( + (s, local_chaos[s], github_chaos[s]) + for s in set(local_chaos) & set(github_chaos) + if local_chaos[s] != github_chaos[s] +) +deb_local_only = sorted(local_deb - github_deb) +deb_github_only = sorted(github_deb - local_deb) -if local_only or github_only: - print("CI parity FAILED: integration suite sets diverge.\n") +problems = (local_only or github_only or chaos_local_only or chaos_github_only + or chaos_flag_drift or deb_local_only or deb_github_only + or dispatch_uncovered) + +if problems: + print("CI parity FAILED: the two runners do not cover the same work.\n") if local_only: - print(" Local-only (in ci-local.sh, missing from ci.yml, " - "not in deliberate allowlist):") + print(" Suites local-only (in ci-local.sh, missing from ci.yml, " + "not in the deliberate allowlist):") for n in local_only: print(f" - {n}") if github_only: - print(" GitHub-only (in ci.yml, missing from local default path):") + print(" Suites GitHub-only (in ci.yml, missing from the local default path):") for n in github_only: print(f" - {n}") - print("\n Resolve by adding the suite to the other runner, or by adding " - "it\n to the deliberate local-only allowlist in " - "check-ci-parity.sh with a\n stated reason.") + if chaos_local_only: + print(" Chaos scenarios local-only:") + for n in chaos_local_only: + print(f" - {n}") + if chaos_github_only: + print(" Chaos scenarios GitHub-only:") + for n in chaos_github_only: + print(f" - {n}") + if chaos_flag_drift: + print(" Chaos scenarios whose flags differ between runners:") + for name, lflags, gflags in chaos_flag_drift: + print(f" - {name}: local '{lflags}' vs GitHub '{gflags}'") + if deb_local_only: + print(" deb-install distros local-only:") + for n in deb_local_only: + print(f" - {n}") + if deb_github_only: + print(" deb-install distros GitHub-only:") + for n in deb_github_only: + print(f" - {n}") + if dispatch_uncovered: + print(" run_suite dispatches these with no backing *_SUITES array, so " + "this guard\n cannot see them in the local set:") + for n in dispatch_uncovered: + print(f" - {n}") + print("\n Resolve by adding the suite to the other runner, by giving a " + "dispatchable\n suite a *_SUITES array, or by adding it to the " + "deliberate local-only\n allowlist in check-ci-parity.sh with a " + "stated reason.") sys.exit(1) -print("CI parity OK: integration suite sets match " +total = len(github) + len(github_chaos) + len(github_deb) +print("CI parity OK: both runners cover the same work " "(allowlist: " + ", ".join(sorted(allowlist)) + ").") -print(f" {len(github)} canonical suites compared on each side.") +print(f" {len(github)} suites, {len(github_chaos)} chaos scenarios " + f"(flags compared), {len(github_deb)} deb-install distros " + f"— {total} legs on each side.") sys.exit(0) PY From 56f00058d40729b396443446f14e2646d3902c44 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 22:03:08 +0000 Subject: [PATCH 4/5] Run the CI parity guard in both runners The guard that keeps "local green" and "GitHub green" meaning the same thing was invoked by nothing. Its only entry point was an exec behind an explicit --check-parity flag, which by exec-ing could not be combined with an actual run, and no workflow step called it at all. A guard nobody runs is a guard that does not exist, which is how the mixed-profile divergence on next survived unnoticed. It now runs as the first stage of every local run and as its own job on GitHub. Locally it reports through the same result-recording path as every other stage, so a divergence sets the run's exit status instead of scrolling past. It is deliberately placed above the mode branches, so a divergence also fails --only, --test-only and --build-only runs: whichever subset was asked for, the claim that a local result means what a GitHub result means is what has broken. The GitHub side installs pyyaml explicitly rather than assuming the runner image carries it. --check-parity keeps working exactly as before. Three comment blocks that described the old folded comparison are corrected here, in the workflow, the local runner and the testing README; they were spread across three files and only two mention the guard by name, so the third is best found by searching for what it claims rather than for the script. --- .github/workflows/ci.yml | 22 ++++++++++++++++------ testing/README.md | 7 +++++-- testing/ci-local.sh | 32 ++++++++++++++++++++++++-------- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b33deb..b84e3f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,12 +38,12 @@ env: # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # -# Granularity-only differences (same coverage, different matrix shape — -# NOT a divergence): -# deb-install — split here into per-distro legs (debian12/debian13/ -# ubuntu22/ubuntu24/ubuntu26) for parallelism; local runs the -# same distro set in one suite. -# dns-resolver — single leg here; runs all scenarios (same as local). +# The two runners express the same work in different matrix shapes, and the +# parity guard compares through that shape rather than around it: chaos legs +# are compared per scenario (and per flag) via their `scenario:` field, +# deb-install legs per distro. The one leg still compared at leg granularity +# is dns-resolver — a single leg here, running all of its scenarios +# internally, exactly as the local suite does. # ───────────────────────────────────────────────────────────────────────────── # ───────────────────────────────────────────────────────────────────────────── @@ -52,6 +52,16 @@ env: # Builds on Linux x86_64, Linux aarch64, and macOS. # ───────────────────────────────────────────────────────────────────────────── jobs: + ci-parity: + name: CI parity + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Install Python deps + run: pip3 install --quiet pyyaml + - name: Check local and GitHub runners cover the same work + run: bash testing/check-ci-parity.sh + fmt: name: Format check runs-on: ubuntu-latest diff --git a/testing/README.md b/testing/README.md index 2dd2760..6933a43 100644 --- a/testing/README.md +++ b/testing/README.md @@ -79,8 +79,11 @@ a per-commit gate; not part of `ci-local.sh`. clippy, unit tests, and the integration suites (including the chaos scenarios) — mirroring the GitHub `ci.yml` integration matrix. Run `./ci-local.sh --help` for the full option list and `--list` for the -available suites. `--check-parity` verifies the local suite set matches -the GitHub matrix (see [check-ci-parity.sh](check-ci-parity.sh)). +available suites. Every run starts with a parity check that verifies the +local suite set covers the same work as the GitHub matrix, per scenario for +chaos and per distro for deb-install; a divergence fails the run. GitHub +runs the same check as its own `ci-parity` job. `--check-parity` runs it +alone (see [check-ci-parity.sh](check-ci-parity.sh)). ### Per-run isolation and the `FIPS_CI_RUN_ID` override diff --git a/testing/ci-local.sh b/testing/ci-local.sh index ffdb60d..c8a4efc 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -1,5 +1,6 @@ #!/bin/bash -# Run the CI pipeline locally: build, unit tests, integration tests. +# Run the CI pipeline locally: CI parity check, build, unit tests, +# integration tests. # # Usage: ./ci-local.sh [options] # @@ -53,19 +54,19 @@ # (.github/workflows/ci.yml) MUST run the same integration suites, EXCEPT for # the deliberate local-only entries below. Adding a suite to one runner # without the other means "local green" and "GitHub green" stop being -# equivalent. testing/check-ci-parity.sh enforces this and fails on drift. +# equivalent. testing/check-ci-parity.sh enforces this and fails on drift; it +# runs as the first stage of every local run, before the build. # # Deliberate local-only (NOT on the GitHub gate), with reason: # tor-socks5 — requires live Tor network; opt-in via --with-tor, # unreliable on GitHub-hosted runners. # tor-directory — same; live Tor dependency. # -# Granularity-only differences (same coverage, different matrix shape — -# NOT a divergence): -# deb-install — local runs all distros sequentially in one suite; GitHub -# splits into per-distro legs (debian12/debian13/ubuntu22/ -# ubuntu24/ubuntu26 — the same distro set). -# dns-resolver — single suite both sides; runs all scenarios. +# The two runners express the same work in different matrix shapes, and the +# guard compares through that shape rather than around it: chaos legs are +# compared per scenario (and per flag), deb-install legs per distro. The one +# leg still compared at leg granularity is dns-resolver — a single suite on +# both sides that runs all of its scenarios internally. # ───────────────────────────────────────────────────────────────────────────── set -uo pipefail @@ -1034,6 +1035,16 @@ print_summary() { echo "" } +# Verify the local default suite set and the GitHub matrix still cover the +# same work. Runs first: it takes about a second, and a divergence should be +# reported before a half-hour suite rather than after it. +run_ci_parity() { + local rc=0 + info "[ci-parity] Comparing the local suite set against the GitHub matrix" + "$SCRIPT_DIR/check-ci-parity.sh" || rc=$? + record "ci-parity" $rc +} + # ── Main ─────────────────────────────────────────────────────────────────── main() { @@ -1042,6 +1053,11 @@ main() { stage "FIPS Local CI" info "Project root: $PROJECT_ROOT" + # Above the mode branches deliberately, so --only, --test-only and + # --build-only are gated too: a divergence invalidates any claim that a + # local run means what a GitHub run means, whichever subset was asked for. + run_ci_parity + if [[ "$TEST_ONLY" == true ]]; then run_tests elif [[ "$BUILD_ONLY" == true ]]; then From 428773490fcae74c5810cd92c5f42a44b3bf8fc9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 22 Jul 2026 22:26:45 +0000 Subject: [PATCH 5/5] Close the gaps an adversarial review found in the new CI guards Four of these are places the parity guard could stop covering something without saying so, which is the failure mode it exists to prevent. Removing the deb-install suite array left it green, because the dispatch cross-check compared against a hardcoded name rather than the array it was meant to read. An arm-shaped line at an unexpected indent, or one written in quotes, was dropped silently by the arm pattern; unexpected indents are now a hard error rather than a quiet skip, and quoted arms parse. A matrix leg with a type of chaos or deb-install but no scenario key raised a Python traceback instead of reporting a problem, and a leg carrying a scenario but no suite key was skipped entirely even though scenario is now the identity for those legs. The e2e builder no longer folds docker create's stderr into the container id. Docker prints warnings on success as well as failure, so a platform-mismatch warning would have become part of the id and made every later reference to that container fail, turning a working extraction into a spurious red. Also corrects the comment about registering a new chaos assertion type, which named only one of the two key sets that actually need it. --- testing/chaos/sim/scenario.py | 5 ++-- testing/check-ci-parity.sh | 44 ++++++++++++++++++++++++++++------- testing/dns-resolver/test.sh | 12 +++++++--- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/testing/chaos/sim/scenario.py b/testing/chaos/sim/scenario.py index 139c2e0..3d925d3 100644 --- a/testing/chaos/sim/scenario.py +++ b/testing/chaos/sim/scenario.py @@ -249,8 +249,9 @@ class Scenario: # link_swap.policies and topology.transport_mix. Two sub-trees are passed through # whole and are likewise not checked: fips_overrides and topology.params. # -# NOTE: adding a new assertion type means adding it to _ASSERTION_KEYS below and -# giving it an entry here, or scenarios using it will be rejected at load. +# NOTE: adding a new assertion type means registering it in TWO places below — +# _SECTION_KEYS["assertions"] (so the block accepts its name) and _ASSERTION_KEYS +# (so its own members are checked) — or scenarios using it are rejected at load. _TOP_KEYS = { "scenario", "topology", "netem", "link_flaps", "traffic", "node_churn", "peer_churn", "bandwidth", "ingress", "link_swap", "assertions", "logging", diff --git a/testing/check-ci-parity.sh b/testing/check-ci-parity.sh index f66e377..a369063 100755 --- a/testing/check-ci-parity.sh +++ b/testing/check-ci-parity.sh @@ -136,16 +136,26 @@ with open(ci_yml_path, encoding="utf-8") as fh: include = doc["jobs"]["integration"]["strategy"]["matrix"]["include"] github_chaos, github_deb, github = {}, set(), set() +malformed = [] for leg in include: - if "suite" not in leg: + if "suite" not in leg and "scenario" not in leg: continue kind = str(leg.get("type", "")) - if kind == "chaos": - github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", "")) - elif kind == "deb-install": - github_deb.add(str(leg["scenario"])) - else: + if kind in ("chaos", "deb-install"): + # scenario: is the identity for these; suite: is cosmetic. + if "scenario" not in leg: + malformed.append(f"{leg.get('suite', '(unnamed leg)')} has type " + f"{kind} but no scenario:") + continue + if kind == "chaos": + github_chaos[str(leg["scenario"])] = str(leg.get("chaos_flags", "")) + else: + github_deb.add(str(leg["scenario"])) + elif "suite" in leg: github.add(str(leg["suite"])) + else: + malformed.append(f"leg with scenario {leg['scenario']} has no suite: " + f"and no chaos/deb-install type") # ── Dispatch cross-check: every run_suite arm needs a backing array ────────── # A suite dispatched without an array is invisible to the sweep above, so the @@ -159,14 +169,26 @@ if body is None: # Arms sit at one fixed indentation inside the case block. Pin to it, taken from # the first arm rather than assumed, so a body line that happens to end in ')' # cannot be read as an arm. -arm_re = re.compile(r"^([ \t]+)([a-z0-9|*_-]+)\)", re.MULTILINE) +arm_re = re.compile(r"^([ \t]+)['\"]?([a-z0-9|*_.-]+)['\"]?\)", re.MULTILINE) first = arm_re.search(body.group(0)) if first is None: print("check-ci-parity: no dispatch arms found in run_suite()", file=sys.stderr) sys.exit(2) indent = first.group(1) -known = set(local) | set(local_chaos) | local_deb | {"deb-install"} +# An arm-shaped line at a different indent is not skipped silently: it would +# make this check quietly stop covering a suite, which is the failure mode the +# check exists to prevent. +odd_arms = [ + m.group(2) for m in arm_re.finditer(body.group(0)) if m.group(1) != indent +] +if odd_arms: + print("check-ci-parity: run_suite has arm-shaped lines at an unexpected " + f"indent, so the dispatch check cannot be trusted: {', '.join(odd_arms)}", + file=sys.stderr) + sys.exit(2) +known = (set(local) | set(local_chaos) | local_deb + | {e.split()[0] for e in arrays.get("DEB_INSTALL_SUITES", [])}) for m in arm_re.finditer(body.group(0)): if m.group(1) != indent: continue @@ -197,7 +219,7 @@ deb_github_only = sorted(github_deb - local_deb) problems = (local_only or github_only or chaos_local_only or chaos_github_only or chaos_flag_drift or deb_local_only or deb_github_only - or dispatch_uncovered) + or dispatch_uncovered or malformed) if problems: print("CI parity FAILED: the two runners do not cover the same work.\n") @@ -230,6 +252,10 @@ if problems: print(" deb-install distros GitHub-only:") for n in deb_github_only: print(f" - {n}") + if malformed: + print(" Matrix legs this guard cannot identify:") + for n in malformed: + print(f" - {n}") if dispatch_uncovered: print(" run_suite dispatches these with no backing *_SUITES array, so " "this guard\n cannot see them in the local set:") diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 905954e..faf071a 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -275,11 +275,17 @@ DOCKERFILE # and the e2e scenarios silently exercise the previous commit's code. rm -f "$FIPS_BIN_CACHE" "$FIPS_GATEWAY_BIN_CACHE" - local cid err - if ! cid=$(docker create "$builder_tag" 2>&1); then - echo " ERROR: docker create failed: $cid" + # stderr goes to its own file rather than into $cid: docker prints + # warnings (a platform mismatch, say) on success too, and folding them + # into the id would leave every later reference pointing at nothing. + local cid err errfile + errfile=$(mktemp) + if ! cid=$(docker create "$builder_tag" 2>"$errfile"); then + echo " ERROR: docker create failed: $(cat "$errfile")" + rm -f "$errfile" return 1 fi + rm -f "$errfile" local rc=0 spec bin dest for spec in "fips:$FIPS_BIN_CACHE" "fips-gateway:$FIPS_GATEWAY_BIN_CACHE"; do