From 88f5830f49466296d1eaa5c851bd7a733d8b7f2c Mon Sep 17 00:00:00 2001 From: Pau Aliagas Date: Tue, 20 Oct 2026 00:51:56 +0200 Subject: [PATCH] ci: a runner that refuses unshare fails build-and-test and the JACK2 job, and a guard holds it (#107) The else branch taken when the runner refuses a private user and pid namespace printed a warning and, in the build-and-test job, ran make sabotage without the LV2 guard, so a run that skipped the JACK and LV2 scenarios went green. Both else branches now print an error and exit 1. tests/ci_guards.py reads every workflow and requires an else that exits 1 after each if unshare, proving first that it sees a planted warn-only else and a planted missing else; make test runs it as check-ci-guards, and make sabotage breaks one branch back to a warning and is caught. --- .github/workflows/ci.yml | 13 ++++--- Makefile | 6 ++- tests/ci_guards.py | 80 ++++++++++++++++++++++++++++++++++++++++ tests/sabotage.py | 14 +++++-- 4 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 tests/ci_guards.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a6599d..0d39c03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,8 +80,8 @@ jobs: # The daemon with the installed workers over jack, in a PipeWire of its own: omx-clap-host for CLAP and mod-host # for LV2, driving the fixture bundle of this repository (a stereo gain, tests/lv2_fixture.c) from a world of its # own, so that the worker's bundle set is one directory. It needs a private user namespace with its own /proc; a - # runner that refuses one cannot run it, and the step says so. The LV2 steps are required to have run: a run that - # skipped them fails here. + # runner that refuses one cannot run it, and the step fails (tests/ci_guards.py holds both branches to that). The + # LV2 steps are required to have run: a run that skipped them fails here. - name: Test the daemon with real workers over jack, CLAP and LV2, then break every guard on purpose run: | set -euo pipefail @@ -101,8 +101,8 @@ jobs: OMX_CLAP_HOST=/usr/bin/omx-clap-host make sabotage 2>&1 | tee sabotage.log if grep -q 'skip sabotage (no LV2 bundle' sabotage.log; then echo 'an LV2 sabotage was skipped'; exit 1; fi else - echo "::warning::test-jack SKIPPED: this runner refuses a private user and pid namespace with its own /proc" - make sabotage + echo "::error::test-jack did not run: this runner refuses a private user and pid namespace with its own /proc" + exit 1 fi jack2: @@ -151,7 +151,7 @@ jobs: # The daemon with the real worker over JACK2's own jackd, in the private namespace jack_e2e.sh builds. # It needs a private user namespace with its own /proc; a runner that refuses one cannot run it, and the - # step says so. + # step fails. - name: Test the daemon with a real worker over JACK2's jackd run: | export FAKE_COMPRESSOR_CLAP=/usr/libexec/omx-clap-host/tests/fake_compressor.clap @@ -160,7 +160,8 @@ jobs: if unshare --user --map-root-user --net --pid --fork --mount-proc true; then make test-jack2 OMX_CLAP_HOST=/usr/bin/omx-clap-host else - echo "::warning::test-jack2 SKIPPED: this runner refuses a private user and pid namespace with its own /proc" + echo "::error::test-jack2 did not run: this runner refuses a private user and pid namespace with its own /proc" + exit 1 fi - name: Name what this job does not run diff --git a/Makefile b/Makefile index 5cf72d7..9184a54 100644 --- a/Makefile +++ b/Makefile @@ -134,7 +134,7 @@ clean: # the daemon against workers that are not plugin hosts at all (tests/fake-host speaks the same protocol and can be # made to die on cue): placement, forwarding, ledger, replay, attribution, the storm bound; no jack, no plugin -test: test-pin test-relay test-alloc test-format test-daemon check-generated check-schema test-probe-schema test-probe test-contract test-consumer test-perturbation check-readme check-own-host-page check-no-pins test-mod-host-json test-socket +test: test-pin test-relay test-alloc test-format test-daemon check-generated check-schema test-probe-schema test-probe test-contract test-consumer test-perturbation check-readme check-own-host-page check-no-pins check-ci-guards test-mod-host-json test-socket MOD_HOST_DIR=$(MOD_HOST_DIR) python3 tests/verbs_contract.py # the same without the verb table, which is read from a mod-host checkout's README @@ -279,6 +279,10 @@ check-own-host-page: check-no-pins: python3 tests/no_pins.py +# a workflow step the runner may be unable to run fails where it cannot run it (#107) +check-ci-guards: + python3 tests/ci_guards.py + # mod-host.json follows its headers: a moved verb, code or message in a scratch copy moves the JSON, and a header it # cannot read is refused test-mod-host-json: diff --git a/tests/ci_guards.py b/tests/ci_guards.py new file mode 100644 index 0000000..b549d65 --- /dev/null +++ b/tests/ci_guards.py @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: GPL-3.0-or-later +# A step a runner may be unable to run fails where it cannot run it. The JACK scenarios need a private user and pid +# namespace with its own /proc: every `if unshare ...; then` in a workflow has an else branch, and that branch exits 1, +# so a runner that refuses the namespace fails the job instead of passing it with a warning (#107). A step with no +# else at all is an error of its own: the shell takes it as a quiet skip. +# Usage: ci_guards.py [root]. The scan proves it can see a planted warn-only branch before it trusts an empty result. +import os, re, shutil, sys, tempfile + +ROOT = sys.argv[1] if len(sys.argv) > 1 else os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +IF_UNSHARE = re.compile(r"^(\s*)if unshare .*; then\s*$") + + +def workflow_files(root): + d = os.path.join(root, ".github", "workflows") + return [os.path.join(d, f) for f in sorted(os.listdir(d))] if os.path.isdir(d) else [] + + +def branches(lines): + """every (line number, else lines) of an `if unshare` at the same indent; else lines is None without an else""" + out = [] + for n, line in enumerate(lines): + m = IF_UNSHARE.match(line) + if not m: + continue + indent = m.group(1) + body, in_else = None, False + for later in lines[n + 1:]: + if later == indent + "fi" or later.startswith(indent + "fi "): + break + if later == indent + "else": + body, in_else = [], True + elif in_else: + body.append(later.strip()) + out.append((n + 1, body)) + return out + + +def scan(root): + """every finding as 'path:line what'""" + found = [] + for path in workflow_files(root): + lines = open(path).read().split("\n") + for n, body in branches(lines): + where = "%s:%d" % (os.path.relpath(path, root), n) + if body is None: + found.append("%s an if unshare with no else: a runner that refuses the namespace skips the step quietly" % where) + elif not any(b == "exit 1" or b.startswith("exit 1 ") for b in body): + found.append("%s the else branch of an if unshare does not exit 1: a runner that refuses the namespace passes with a warning" % where) + return found + + +def main(): + tmp = tempfile.mkdtemp(prefix="ci-guards.", dir=os.environ.get("TMPDIR") or ROOT) + try: + scratch = os.path.join(tmp, "t") + os.makedirs(os.path.join(scratch, ".github", "workflows")) + plant = (" run: |\n if unshare --user --map-root-user true; then\n make test-jack\n else\n" + " echo \"::warning::skipped\"\n fi\n" + " if unshare --user true; then\n make test-jack2\n fi\n" + " if unshare --user true; then\n make x\n else\n echo no\n exit 1\n fi\n") + open(os.path.join(scratch, ".github", "workflows", "ci.yml"), "w").write(plant) + seen = scan(scratch) + if len(seen) != 2 or "warning" not in seen[0] or "no else" not in seen[1]: + print("FAIL the scan misses a planted branch: saw %s" % seen) + return 1 + finally: + shutil.rmtree(tmp, ignore_errors=True) + print("ok the scan sees a planted warn-only else and a planted missing else, and passes one that exits 1") + failed = 0 + for f in scan(ROOT): + print("FAIL %s" % f) + failed += 1 + n = sum(len(branches(open(p).read().split("\n"))) for p in workflow_files(ROOT)) + if not failed: + print("ok every if unshare in the workflows fails where the runner refuses the namespace (%d of them)" % n) + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/sabotage.py b/tests/sabotage.py index e7be1a9..aa4bda0 100644 --- a/tests/sabotage.py +++ b/tests/sabotage.py @@ -13,8 +13,11 @@ # an LV2 bundle (MOD_HOST, LV2_DIR, LV2_URI, LV2_BUNDLE, LV2_PARAM) and is skipped without them; "pin" breaks # include/plugin-hostd/pin.h and runs tests/pin_test.c; "readme" breaks README.md or BUILDING.md and runs # tests/readme_build.py, with no daemon to build; "probeschema" breaks protocol/probe-output.schema.json and runs -# tests/probe_schema.py; "relay" breaks src/relay.c and runs tests/relay_test.c; "alloc" breaks src/relay.c or src/lv2.c and runs make test-alloc; "contract" breaks the probe runner or the generator and runs tests/contract_derived.py; "consumer" breaks the header and compiles tests/consumer.c against it; "probe" breaks src/probe.c and runs tests/probe_test.py; "nopins" breaks the packaging or a workflow and runs tests/no_pins.py +# tests/probe_schema.py; "relay" breaks src/relay.c and runs tests/relay_test.c; "alloc" breaks src/relay.c or src/lv2.c and runs make test-alloc; "contract" breaks the probe runner or the generator and runs tests/contract_derived.py; "consumer" breaks the header and compiles tests/consumer.c against it; "probe" breaks src/probe.c and runs tests/probe_test.py; "nopins" breaks the packaging or a workflow and runs tests/no_pins.py; "ciguards" breaks a workflow and runs tests/ci_guards.py SABOTAGE = [ + ("a runner that refuses unshare passes build-and-test with a warning", ".github/workflows/ci.yml", + ' echo "::error::test-jack did not run: this runner refuses a private user and pid namespace with its own /proc"\n exit 1\n', + ' echo "::warning::test-jack SKIPPED"\n', "every if unshare in the workflows fails", "ciguards"), ("the verb tail is not replayed", "src/supervisor.c", "if (replay_line(w, i->tail[m].line) != RPC_OK)", "if (0 && replay_line(w, i->tail[m].line) != RPC_OK)", "own_crash_respawns_only_that_instance_and_replays_it"), @@ -571,8 +574,9 @@ def run_test(tree, name, jack=False): p = subprocess.run([sys.executable, os.path.join(ROOT, "tests", "probe_schema.py"), tree], capture_output=True, text=True, timeout=60) return p.returncode, p.stdout - if jack == "nopins": - p = subprocess.run([sys.executable, os.path.join(ROOT, "tests", "no_pins.py"), tree], capture_output=True, + if jack in ("nopins", "ciguards"): + script = {"nopins": "no_pins.py", "ciguards": "ci_guards.py"}[jack] + p = subprocess.run([sys.executable, os.path.join(ROOT, "tests", script), tree], capture_output=True, text=True, timeout=60) return p.returncode, p.stdout env = dict(os.environ, ONLY=name, PLUGIN_HOSTD=os.path.join(tree, "plugin-hostd"), FAKE_HOST=FAKE) @@ -635,6 +639,8 @@ def run_test(tree, name, jack=False): for f in ("packaging/plugin-hostd.spec", "debian/control", ".github/workflows/ci.yml"): os.makedirs(os.path.dirname(os.path.join(tree, f)), exist_ok=True) shutil.copy(os.path.join(ROOT, f), os.path.join(tree, f)) + elif jack == "ciguards": + shutil.copytree(os.path.join(ROOT, ".github", "workflows"), os.path.join(tree, ".github", "workflows")) else: shutil.copytree(os.path.join(ROOT, "src"), os.path.join(tree, "src")) shutil.copytree(os.path.join(ROOT, "include"), os.path.join(tree, "include")) @@ -653,7 +659,7 @@ def run_test(tree, name, jack=False): open(os.path.join(tree, path), "w").write(src.replace(text, repl)) if jack == "probe": build_probe(tree) - elif jack not in ("readme", "nopins", "format", "relay", "contract", "consumer", "probeschema", "alloc"): + elif jack not in ("readme", "nopins", "ciguards", "format", "relay", "contract", "consumer", "probeschema", "alloc"): for f in os.listdir(os.path.join(tree, "src")): if f.endswith(".o"): os.unlink(os.path.join(tree, "src", f))