From 20093625d4076a319deeb1e47055ea4a6129d8bc Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 20:27:13 +0200 Subject: [PATCH 1/9] ci: attach the packages to the release and gate the dependency tree v0.0.2 shipped a tag with no packages. packaging/build.sh writes dist/*.deb and dist/*.rpm, and pyproject.toml already declared dist_glob_patterns and upload_to_vcs_release, but only `semantic-release publish` reads that block. The workflow ran `semantic-release version` alone, so the packages were built on the runner and discarded with it. The release now runs publish as well. publish defaults to the latest release, which would attach this run's packages to the previous tag when nothing was bumped, so the step compares the tag before and after the version step, exits when it did not change, and passes the exact new tag to publish. It also refuses to publish when dist holds no .deb or no .rpm, because a release with half the artifacts is worse than a failed job. The policy test asserts that the release runs both commands, names a tag, and declares a glob for each package format. It fails against the workflow that shipped v0.0.2. deny.toml adds a supply-chain gate as a separate job in checks.yml, so it also gates releases without lengthening the critical path. Dependabot raises version bumps but does not report whether the locked tree carries a known advisory. The licence allowlist matters beyond the legal question here, because the packages carry a copyright file and a copyleft crate arriving transitively is a packaging problem. The list was taken from the resolved musl tree, not guessed, and was checked by removing MIT, which fails the gate. Cargo.toml gains a [lints] table. Strictness lived only in the CI flag, so a local cargo clippy was more permissive than CI and the difference surfaced on push. Verified: with the table a plain `cargo clippy` reports a needless return as an error, without it only as a warning. --- .github/workflows/checks.yml | 14 ++++++++++++++ .github/workflows/release.yml | 22 +++++++++++++++++++++- CLAUDE.md | 5 +++++ Cargo.toml | 8 ++++++++ deny.toml | 31 +++++++++++++++++++++++++++++++ packaging/test_policy.py | 30 ++++++++++++++++++++++++++++++ 6 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 deny.toml diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 394310d..1b4e971 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -26,3 +26,17 @@ jobs: run: | python3 -m pip install PyYAML==6.0.3 python3 packaging/test_policy.py + + # Dependabot raises version bumps. This reports whether the tree as locked carries a + # known advisory, and whether a transitive crate brings a licence the packages cannot + # ship. It runs beside `check` so it does not lengthen the critical path. + deny: + name: Supply chain + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: EmbarkStudios/cargo-deny-action@3c6349835b2b7b196a839186cb8b78e02f7b5f25 # v2.1.1 + with: + command: check diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index dd46620..25fb532 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -92,7 +92,27 @@ jobs: cargo install --locked --version 3.8.0 cargo-deb cargo install --locked --version 0.21.0 cargo-generate-rpm uv sync --group dev + # `version` builds dist/ through build_command and creates the release, but it + # never uploads dist_glob_patterns. Only `publish` does that. Publish the exact new + # tag, because `publish` otherwise defaults to "latest" and would attach the + # packages to the previous release when this run bumped nothing. - name: Run semantic-release env: GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} - run: uv run semantic-release version --changelog --push --vcs-release + run: | + before=$(git describe --tags --abbrev=0 2>/dev/null || echo none) + uv run semantic-release version --changelog --push --vcs-release + after=$(git describe --tags --abbrev=0 2>/dev/null || echo none) + if [ "$after" = "$before" ]; then + echo "No new tag, so there is nothing to publish." + exit 0 + fi + test -n "$(find dist -name '*.deb' -print -quit)" || { + echo "dist holds no .deb, refusing to publish an incomplete release" >&2 + exit 1 + } + test -n "$(find dist -name '*.rpm' -print -quit)" || { + echo "dist holds no .rpm, refusing to publish an incomplete release" >&2 + exit 1 + } + uv run semantic-release publish --tag "$after" diff --git a/CLAUDE.md b/CLAUDE.md index f9570b7..fa1029e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,8 +45,13 @@ cargo test # single test cargo test -- --nocapture # keep test stdout cargo clippy --all-targets -- -D warnings cargo fmt --check +cargo deny check # advisories, licences, banned and duplicate crates ``` +Lint levels live in `Cargo.toml` under `[lints]`, so a plain `cargo clippy` fails on the +same code CI rejects. `cargo deny` needs `cargo install --locked cargo-deny`; CI runs it +as a separate job in `checks.yml`. + `rust-toolchain.toml` pins the toolchain, but a `RUSTUP_TOOLCHAIN` environment variable overrides it. Check that variable before blaming a build failure on the code. diff --git a/Cargo.toml b/Cargo.toml index 96bcc14..9a27ae1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,14 @@ readme = "README.md" keywords = ["snmp", "agentx", "if-mib", "ifstacktable", "linux"] categories = ["network-programming", "command-line-utilities"] +# Lint levels belong here, not only in the CI flag, so a local cargo clippy and CI +# reject the same code. +[lints.rust] +unsafe_op_in_unsafe_fn = "deny" + +[lints.clippy] +all = { level = "deny", priority = -1 } + [dependencies] agentx = "0.1.1" serde = { version = "1", features = ["derive"] } diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..819ef58 --- /dev/null +++ b/deny.toml @@ -0,0 +1,31 @@ +# Supply-chain gate for the dependency tree. Dependabot raises version bumps, but it +# does not report whether the tree as locked carries a known advisory. + +[graph] +# The release binary is built for musl, so resolve the tree that actually ships. +targets = [{ triple = "x86_64-unknown-linux-musl" }] + +[advisories] +version = 2 +# RUSTSEC advisories fail the build. Add an id here only with a comment saying why. +ignore = [] + +[licenses] +version = 2 +# The package ships as a .deb and .rpm with a copyright file, so a copyleft crate +# arriving through a transitive dependency is a packaging problem, not only a legal one. +allow = [ + "MIT", + + "Apache-2.0", + "Unlicense", + "Unicode-3.0", +] + +[bans] +multiple-versions = "warn" +wildcards = "deny" + +[sources] +unknown-registry = "deny" +unknown-git = "deny" diff --git a/packaging/test_policy.py b/packaging/test_policy.py index cb9960e..418e9e0 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -116,6 +116,36 @@ def test_semantic_release_writes_the_manifest_and_syncs_the_rest(self): self.assertIs(release["major_on_zero"], False) self.assertIs(release["allow_zero_version"], True) + def test_the_release_uploads_both_package_formats(self): + """`version` builds dist/ and creates the release but uploads nothing from it. + + Only `publish` uploads dist_glob_patterns, so a release that runs `version` + alone ships a tag with no packages attached. + """ + config = tomllib.loads((ROOT / "pyproject.toml").read_text()) + publish = config["tool"]["semantic_release"]["publish"] + self.assertIs(publish["upload_to_vcs_release"], True) + globs = publish["dist_glob_patterns"] + for suffix in (".deb", ".rpm"): + self.assertTrue( + any(glob.endswith(suffix) for glob in globs), + f"no dist glob matches {suffix}: {globs}", + ) + workflow = yaml.safe_load((ROOT / ".github/workflows/release.yml").read_text()) + steps = workflow["jobs"]["semantic-release"]["steps"] + commands = "\n".join(str(step.get("run", "")) for step in steps) + self.assertIn( + "semantic-release version", commands, "the release must compute a version" + ) + self.assertIn( + "semantic-release publish", + commands, + "run semantic-release publish, or the packages never reach the release", + ) + # publish defaults to the latest release, which would attach this run's packages + # to the previous tag when nothing was bumped. + self.assertIn("--tag", commands, "publish must name the tag it uploads to") + def test_the_sync_script_carries_a_bump_into_every_version_source(self): """Run the real script on a real copy: a stub would not catch cargo drift.""" bumped = "9.9.9" From 4cf483ba2254086ac3ec97e2d24f694629e892db Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 20:35:24 +0200 Subject: [PATCH 2/9] fix: write a Debian changelog trailer dpkg accepts CI has been red on main since the v0.0.2 release. lintian reported two warnings and the debian 12 and 13 package jobs failed: W: syntax-error-in-debian-changelog "badly formatted trailer line" W: syntax-error-in-debian-changelog "found start of entry where expected more change data or trailer" Both come from one line. sync-version.sh built the trailer date with email.utils.formatdate(stamp, usegmt=True), which writes "GMT". A Debian trailer needs a numeric offset, so dpkg fails to parse the line and then reports the next stanza header as unexpected. Use email.utils.format_datetime on a timezone aware UTC datetime, which writes "+0000". The hand written stanza already used "+0000", so the file was valid until the first release generated one. The generator had never been checked against dpkg or lintian, only its output shape. Also repair the 0.0.2 stanza the release wrote, because fixing the generator does not rewrite what is already committed. The test runs the real script and parses the result with dpkg-parsechangelog, then asserts it wrote nothing to stderr. dpkg exits 0 on this fault and only warns, which is why a returncode check would have missed it. It skips when dpkg-parsechangelog is absent. --- packaging/changelog | 2 +- packaging/sync-version.sh | 7 ++++++- packaging/test_policy.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packaging/changelog b/packaging/changelog index 7f452d0..3d03c82 100644 --- a/packaging/changelog +++ b/packaging/changelog @@ -2,7 +2,7 @@ agentx-ifstack (0.0.2-1) unstable; urgency=medium * Release 0.0.2. See CHANGELOG.md for the change list. - -- Marcin Zieba Fri, 11 Sep 2026 18:05:06 GMT + -- Marcin Zieba Fri, 11 Sep 2026 18:05:06 +0000 agentx-ifstack (0.1.0-1) unstable; urgency=medium diff --git a/packaging/sync-version.sh b/packaging/sync-version.sh index 52537ed..bd6fe56 100755 --- a/packaging/sync-version.sh +++ b/packaging/sync-version.sh @@ -6,6 +6,7 @@ set -eu python3 - <<'PY' """Carry the version semantic-release just wrote into the Debian changelog.""" +import datetime import email.utils import os import pathlib @@ -58,7 +59,11 @@ existing = changelog.read_text() if not re.match(rf"^agentx-ifstack \({re.escape(version)}-\d+\) ", existing): # Honour SOURCE_DATE_EPOCH so a rebuild of the same release is reproducible. stamp = int(os.environ.get("SOURCE_DATE_EPOCH", time.time())) - released = email.utils.formatdate(stamp, usegmt=True) + # A Debian trailer needs a numeric offset. usegmt writes "GMT", which dpkg and + # lintian both reject as a badly formatted trailer line. + released = email.utils.format_datetime( + datetime.datetime.fromtimestamp(stamp, datetime.timezone.utc) + ) entry = ( f"agentx-ifstack ({version}-1) unstable; urgency=medium\n" f"\n" diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 418e9e0..2f26708 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -178,6 +178,40 @@ def test_the_sync_script_carries_a_bump_into_every_version_source(self): self.assertIn(f"agentx-ifstack ({manifest_version()}-1) ", (work / "packaging/changelog").read_text()) + def test_the_generated_changelog_trailer_parses(self): + """Debian trailers need a numeric timezone offset, and lintian fails on a warning. + + Parse the generated file with dpkg itself rather than a regex: the hand written + stanza was valid, so nothing caught the generator until the first real release. + """ + parser = shutil.which("dpkg-parsechangelog") + if parser is None: + self.skipTest("dpkg-parsechangelog is not installed") + with tempfile.TemporaryDirectory() as directory: + work = Path(directory) + for name in ("Cargo.toml", "Cargo.lock", "rust-toolchain.toml"): + shutil.copy(ROOT / name, work / name) + shutil.copytree(ROOT / "src", work / "src") + (work / "packaging").mkdir() + for name in ("changelog", "sync-version.sh"): + shutil.copy(ROOT / "packaging" / name, work / "packaging" / name) + manifest = (work / "Cargo.toml").read_text() + (work / "Cargo.toml").write_text( + manifest.replace(f'version = "{manifest_version()}"', 'version = "9.9.9"', 1) + ) + subprocess.run( + ["sh", "packaging/sync-version.sh"], cwd=work, check=True, + capture_output=True, text=True, + ) + parsed = subprocess.run( + [parser, "-l", str(work / "packaging/changelog")], + capture_output=True, text=True, check=True, + ) + self.assertEqual( + parsed.stderr.strip(), "", "dpkg rejected the generated changelog" + ) + self.assertIn("Version: 9.9.9-1", parsed.stdout) + def test_the_sync_script_finishes_a_half_applied_run(self): """A retry after a crash between the two writes must still fix Cargo.lock.""" bumped = "9.9.9" From e052ad671078ddf7a8e7fcb0c7035db9255ccc40 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 20:55:02 +0200 Subject: [PATCH 3/9] ci: run the checks on pull requests only push on every branch and pull_request both fired for one push to a branch with an open pull request, so every job ran twice: two check runs, two supply chain runs, two package builds and two container installs per push. Drop the push trigger. The main ruleset requires a pull request, so nothing reaches main without this workflow running first, and pull_request builds the merge commit, which is the result that lands. A branch with no pull request now runs no CI, which is what the operator asked for. release.yml keeps its own push trigger for main, because that is how a release starts. Its gate calls checks.yml, as before. The test that pinned ci.yml to every branch now pins the absence of a push trigger. A second test rejects any workflow that has both a pull_request trigger and a push trigger for branches other than main, so the duplication cannot come back through another workflow. --- .github/workflows/ci.yml | 5 +++-- packaging/test_policy.py | 27 ++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 209b3da..30ce03c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,8 +1,9 @@ name: CI +# Pull requests only. push and pull_request both fired for one push to a PR branch, so +# every job ran twice. The main ruleset requires a pull request, so nothing reaches main +# without this running first, and pull_request builds the merge commit that lands. on: - push: - branches: ['**'] pull_request: permissions: diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 2f26708..6e7488e 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -68,9 +68,8 @@ def test_releases_come_from_main_and_not_from_a_pushed_tag(self): release_push = workflow("release.yml")["on"]["push"] self.assertEqual(release_push.get("branches"), ["main"]) self.assertNotIn("tags", release_push) - ci_push = workflow("ci.yml")["on"]["push"] - self.assertEqual(ci_push.get("branches"), ["**"]) - self.assertNotIn("tags", ci_push) + # CI runs on pull requests only, so it has no push trigger to carry a tag. + self.assertNotIn("push", workflow("ci.yml")["on"]) def test_the_release_job_waits_for_the_shared_checks(self): """A red test gate must stop the release before it tags and publishes.""" @@ -293,6 +292,28 @@ def test_third_party_actions_are_pinned_to_full_commit_shas(self): unpinned.append(f"{path.name} {reference}") self.assertEqual(unpinned, [], "third-party actions must be pinned to a SHA") + def test_no_workflow_runs_twice_for_one_push(self): + """push on every branch plus pull_request runs every job twice on a PR branch. + + pull_request builds the merge commit, which is the result that matters for a + pull request, so push stays on main for post-merge validation. + """ + for path in sorted((ROOT / ".github/workflows").glob("*.yml")): + workflow = yaml.safe_load(path.read_text()) + # PyYAML follows YAML 1.1, where a bare `on:` key parses as the boolean True. + triggers = workflow.get("on", workflow.get(True)) + if not isinstance(triggers, dict) or "pull_request" not in triggers: + continue + push = triggers.get("push") + if push is None: + continue + self.assertEqual( + push.get("branches"), + ["main"], + f"{path.name}: push and pull_request both fire on a PR branch, " + "so every job runs twice; limit push to main", + ) + def test_no_workflow_checkout_persists_its_credential(self): """actions/checkout leaves the token in .git/config, where any later step reads it.""" persisting = [] From ef6c4a733a49185bfc553ffccfa96f7dc158d5e2 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 21:01:50 +0200 Subject: [PATCH 4/9] fix: refuse an incomplete package set before the release exists The artifact check ran after `semantic-release version`, which has already committed, tagged, pushed and created the GitHub release by then. Failing there turned a missing format into exactly the half-published release the check was meant to prevent, because a published release cannot be un-published. Move both checks into packaging/release-build.sh. semantic-release runs build_command after it stamps the version and before it creates the tag, and build_distributions raises BuildDistributionsError when the command fails, so a missing .deb or .rpm now stops the release before anything is published. The copy in the workflow is removed rather than kept alongside. The test runs the real release-build.sh against a stubbed build that produces a chosen subset, and asserts a complete set succeeds while each incomplete one fails. It exercises the guard rather than the text of the script. --- .github/workflows/release.yml | 11 ++--------- packaging/release-build.sh | 9 +++++++++ packaging/test_policy.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 25fb532..cb23ba9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -95,7 +95,8 @@ jobs: # `version` builds dist/ through build_command and creates the release, but it # never uploads dist_glob_patterns. Only `publish` does that. Publish the exact new # tag, because `publish` otherwise defaults to "latest" and would attach the - # packages to the previous release when this run bumped nothing. + # packages to the previous release when this run bumped nothing. The package set is + # checked inside build_command, which runs before the tag exists. - name: Run semantic-release env: GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} @@ -107,12 +108,4 @@ jobs: echo "No new tag, so there is nothing to publish." exit 0 fi - test -n "$(find dist -name '*.deb' -print -quit)" || { - echo "dist holds no .deb, refusing to publish an incomplete release" >&2 - exit 1 - } - test -n "$(find dist -name '*.rpm' -print -quit)" || { - echo "dist holds no .rpm, refusing to publish an incomplete release" >&2 - exit 1 - } uv run semantic-release publish --tag "$after" diff --git a/packaging/release-build.sh b/packaging/release-build.sh index 174a66e..c794e6d 100755 --- a/packaging/release-build.sh +++ b/packaging/release-build.sh @@ -5,3 +5,12 @@ set -eu sh packaging/sync-version.sh sh packaging/build.sh + +# semantic-release runs this before it commits, tags and pushes, so failing here stops +# the release. A release that is already published cannot be un-published. +for suffix in deb rpm; do + [ -n "$(find dist -name "*.${suffix}" -print -quit)" ] || { + echo "dist holds no .${suffix}, refusing to release an incomplete package set" >&2 + exit 1 + } +done diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 6e7488e..9678ccc 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -145,6 +145,38 @@ def test_the_release_uploads_both_package_formats(self): # to the previous tag when nothing was bumped. self.assertIn("--tag", commands, "publish must name the tag it uploads to") + def test_the_build_command_refuses_an_incomplete_package_set(self): + """build_command runs before the tag, so a missing format must stop the release. + + Checking after `semantic-release version` is too late: it has already committed, + tagged, pushed and created the release by then. Run the real script against a + stubbed build so the guard itself is exercised. + """ + def run(produce): + with tempfile.TemporaryDirectory() as directory: + work = Path(directory) + (work / "packaging").mkdir() + shutil.copy( + ROOT / "packaging/release-build.sh", work / "packaging/release-build.sh" + ) + (work / "packaging/sync-version.sh").write_text("#!/bin/sh\n") + (work / "packaging/build.sh").write_text( + "#!/bin/sh\nset -eu\nmkdir -p dist\n" + + "".join(f"touch dist/pkg{suffix}\n" for suffix in produce) + ) + return subprocess.run( + ["sh", "packaging/release-build.sh"], cwd=work, + capture_output=True, text=True, check=False, + ) + + self.assertEqual(run([".deb", ".rpm"]).returncode, 0, "a complete set must build") + for produce, missing in (([".deb"], ".rpm"), ([".rpm"], ".deb"), ([], "both")): + result = run(produce) + self.assertNotEqual( + result.returncode, 0, + f"the build command accepted a package set missing {missing}", + ) + def test_the_sync_script_carries_a_bump_into_every_version_source(self): """Run the real script on a real copy: a stub would not catch cargo drift.""" bumped = "9.9.9" From 88f4f7502fe68a570df74869b5e44636ae755798 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 21:46:12 +0200 Subject: [PATCH 5/9] ci: gate the invariants clippy cannot express clippy is type aware and covers idiomatic Rust well, but it cannot say "this method is only allowed inside this function". CodeQL covers broad dataflow SAST. Neither encodes a project invariant, so a fixed bug class returns as soon as the commit message that explained it scrolls out of view. Two rules, both drawn from faults this repository actually had: - try_wait reaps the child and frees its pid, and that pid is also the process group id, so reaping before the group kill lets kill(-pgid) reach an unrelated group. Reaping is allowed only in IpCommand::finish, which kills first. - unwrap panics, and a panic aborts the daemon while systemd counts the restart. Production code currently has none; every unwrap in src is inside a test module. The ruleset filename is deliberate. CodeRabbit adopts a file named opengrep.yml or semgrep.yml as its config and then runs it INSTEAD OF its default packs, so such a name would silently replace that coverage. The ruleset carries a name CodeRabbit does not adopt and is passed with --config, so both rulesets apply. A policy test asserts no adopted name exists in the tree. Rules are covered by opengrep's own rule-tests: fixtures under .opengrep/tests carry `// ruleid:` and `// ok:` markers, and the same policy test requires a fixture per rule, so a rule cannot be added without one or silently stop matching. CI fetches the opengrep binary over the network, so it is pinned by version and sha256. The pinned digest matches the binary these rules were developed against. --- .github/workflows/checks.yml | 25 ++++++++++ .opengrep/README.md | 39 +++++++++++++++ .opengrep/agentx-ifstack-rules.yaml | 48 +++++++++++++++++++ .../tests/agentx-try-wait-outside-finish.rs | 29 +++++++++++ .../tests/agentx-unwrap-outside-tests.rs | 22 +++++++++ CLAUDE.md | 5 ++ packaging/test_policy.py | 34 +++++++++++++ scripts/opengrep-scan.sh | 30 ++++++++++++ scripts/opengrep-test.sh | 33 +++++++++++++ 9 files changed, 265 insertions(+) create mode 100644 .opengrep/README.md create mode 100644 .opengrep/agentx-ifstack-rules.yaml create mode 100644 .opengrep/tests/agentx-try-wait-outside-finish.rs create mode 100644 .opengrep/tests/agentx-unwrap-outside-tests.rs create mode 100755 scripts/opengrep-scan.sh create mode 100755 scripts/opengrep-test.sh diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 1b4e971..98ee937 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -40,3 +40,28 @@ jobs: - uses: EmbarkStudios/cargo-deny-action@3c6349835b2b7b196a839186cb8b78e02f7b5f25 # v2.1.1 with: command: check + + # Project invariants clippy cannot express. Not a security scan: CodeQL already + # analyses this crate, and CodeRabbit keeps running its own opengrep packs because the + # ruleset is deliberately not named so CodeRabbit adopts it. + rules: + name: Custom rules + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Install opengrep + env: + OPENGREP_VERSION: v1.30.0 + OPENGREP_SHA256: 35779bdd72e92129c8df2a77f0c55e8c08356801ea92591ef32108d6b28d564c + run: | + curl -fsSL -o /usr/local/bin/opengrep \ + "https://github.com/opengrep/opengrep/releases/download/${OPENGREP_VERSION}/opengrep_manylinux_x86" + echo "${OPENGREP_SHA256} /usr/local/bin/opengrep" | sha256sum -c - + chmod +x /usr/local/bin/opengrep + opengrep --version + - name: Run the rule-tests + run: ./scripts/opengrep-test.sh + - name: Run the custom ruleset + run: ./scripts/opengrep-scan.sh diff --git a/.opengrep/README.md b/.opengrep/README.md new file mode 100644 index 0000000..beb67b7 --- /dev/null +++ b/.opengrep/README.md @@ -0,0 +1,39 @@ +# opengrep ruleset + +Custom [opengrep](https://github.com/opengrep/opengrep) rules that encode this project's +`CLAUDE.md` correctness invariants as machine-checked gates, so the same classes of bug +stop coming back review after review. + +## Why opengrep, and not just clippy or CodeQL + +- **clippy** is type aware and covers idiomatic Rust far better than a syntactic matcher, + but it cannot express "this method is only allowed inside this function". +- **CodeQL** (`CodeQL/Analyze (rust)`) covers broad dataflow SAST. +- **opengrep** fills the gap: cheap, readable patterns for *our* invariants, and it is the + same engine CodeRabbit runs. + +## Relationship to CodeRabbit + +CodeRabbit auto-detects an opengrep config only when it is named `opengrep.yml` or +`semgrep.yml` (and a few variants), and when it finds one it runs *that* **instead of** its +default packs. This ruleset deliberately avoids those names, so CodeRabbit keeps running +its own packs while these rules are enforced separately by `scripts/opengrep-scan.sh` and +the CI job. Both rulesets apply. + +## Layout + +| Path | Purpose | +| --- | --- | +| `.opengrep/agentx-ifstack-rules.yaml` | The ruleset, and the single source of truth. Named so CodeRabbit does not adopt it. | +| `.opengrep/tests/*.rs` | Rule-test fixtures. `// ruleid:` must match, `// ok:` must not. They violate the rules on purpose and are not part of the crate. | +| `scripts/opengrep-scan.sh` | Scan `src/`. Exits non-zero on any finding. | +| `scripts/opengrep-test.sh` | Run the rule-tests against the ruleset. | + +## Rules + +| Rule | Invariant | +| --- | --- | +| `agentx-try-wait-outside-finish` | `try_wait` reaps the child and frees its pid, and that pid is the process group id, so reaping before the group kill lets `kill(-pgid)` reach an unrelated group. Reap only in `IpCommand::finish`. | +| `agentx-unwrap-outside-tests` | `unwrap` panics, and a panic aborts the daemon while systemd counts the restart. | + +Suppress a deliberate exception on the line with `// nosemgrep: `. diff --git a/.opengrep/agentx-ifstack-rules.yaml b/.opengrep/agentx-ifstack-rules.yaml new file mode 100644 index 0000000..416e95d --- /dev/null +++ b/.opengrep/agentx-ifstack-rules.yaml @@ -0,0 +1,48 @@ +# Custom opengrep ruleset encoding this project's CLAUDE.md correctness invariants. +# These run ON TOP OF CodeRabbit's default opengrep packs: the file is intentionally +# NOT named opengrep.yml or semgrep.yml, because CodeRabbit treats such a file as its +# config and runs it INSTEAD OF its own packs. Here it is passed explicitly with +# --config by scripts/opengrep-scan.sh and the CI job, so both rulesets apply. +# +# Rule-test fixtures live in .opengrep/tests/; run them with scripts/opengrep-test.sh. +rules: + - id: agentx-try-wait-outside-finish + languages: [rust] + severity: ERROR + message: >- + try_wait reaps the child and frees its pid, and that pid is also the process + group id, so a later kill(-pgid) can reach an unrelated group. Reap only in + IpCommand::finish, which kills the group first. If a call is deliberately + outside that order, suppress it on the line with + `// nosemgrep: agentx-try-wait-outside-finish`. + metadata: + category: correctness + confidence: HIGH + references: + - "CLAUDE.md: kill the process group before the single reap" + paths: + exclude: + - "tests/**" + patterns: + - pattern: $CHILD.try_wait() + - pattern-not-inside: | + fn finish(&mut self) -> Result { ... } + + - id: agentx-unwrap-outside-tests + languages: [rust] + severity: ERROR + message: >- + unwrap panics, and a panic aborts the daemon while systemd counts the restart. + Return an error, or use expect with a message when the invariant is local and + genuinely cannot fail. + metadata: + category: reliability + confidence: HIGH + references: + - "CLAUDE.md: validate at boundaries and fail fast, no silent panics" + paths: + exclude: + - "tests/**" + patterns: + - pattern: $VALUE.unwrap() + - pattern-not-inside: mod tests { ... } diff --git a/.opengrep/tests/agentx-try-wait-outside-finish.rs b/.opengrep/tests/agentx-try-wait-outside-finish.rs new file mode 100644 index 0000000..b813f6b --- /dev/null +++ b/.opengrep/tests/agentx-try-wait-outside-finish.rs @@ -0,0 +1,29 @@ +// Fixture for agentx-try-wait-outside-finish. Contains rule-violating code on purpose. + +impl IpCommand { + fn finish(&mut self) -> Result { + let mut child = self.child.take().expect("unreaped ip child"); + kill_group(child.id()); + // ok: agentx-try-wait-outside-finish + match child.try_wait() { + Ok(Some(status)) => Ok(status), + _ => Err(Error::other("not reaped")), + } + } + + fn reap_early(&mut self) -> Result<()> { + let child = self.child.as_mut().expect("unreaped ip child"); + // ruleid: agentx-try-wait-outside-finish + let _ = child.try_wait()?; + Ok(()) + } +} + +fn wait_bounded(child: &mut Child) -> Result { + loop { + // ruleid: agentx-try-wait-outside-finish + if let Some(status) = child.try_wait()? { + return Ok(status); + } + } +} diff --git a/.opengrep/tests/agentx-unwrap-outside-tests.rs b/.opengrep/tests/agentx-unwrap-outside-tests.rs new file mode 100644 index 0000000..e581ec8 --- /dev/null +++ b/.opengrep/tests/agentx-unwrap-outside-tests.rs @@ -0,0 +1,22 @@ +// Fixture for agentx-unwrap-outside-tests. Contains rule-violating code on purpose. + +fn production(input: &str) -> u32 { + // ruleid: agentx-unwrap-outside-tests + input.parse::().unwrap() +} + +fn production_ok(input: &str) -> Result { + // ok: agentx-unwrap-outside-tests + input.parse::().map_err(Error::other) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses() { + // ok: agentx-unwrap-outside-tests + assert_eq!(production_ok("7").unwrap(), 7); + } +} diff --git a/CLAUDE.md b/CLAUDE.md index fa1029e..aea541d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,6 +52,11 @@ Lint levels live in `Cargo.toml` under `[lints]`, so a plain `cargo clippy` fail same code CI rejects. `cargo deny` needs `cargo install --locked cargo-deny`; CI runs it as a separate job in `checks.yml`. +Project invariants that clippy cannot express live in `.opengrep/agentx-ifstack-rules.yaml`. +Run `scripts/opengrep-scan.sh` to check the source and `scripts/opengrep-test.sh` to check +the rules themselves. Every rule needs a fixture in `.opengrep/tests/`, which the packaging +policy tests enforce. See `.opengrep/README.md` for why the filename matters. + `rust-toolchain.toml` pins the toolchain, but a `RUSTUP_TOOLCHAIN` environment variable overrides it. Check that variable before blaming a build failure on the code. diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 9678ccc..8ca3716 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -361,6 +361,40 @@ def test_no_workflow_checkout_persists_its_credential(self): persisting.append(f"{path.name}:{job_name}") self.assertEqual(persisting, [], "checkout must not persist credentials") + def test_the_custom_ruleset_adds_to_coderabbit_instead_of_replacing_it(self): + """CodeRabbit runs a detected opengrep config INSTEAD OF its default packs. + + A file named opengrep.yml or semgrep.yml would silently replace that coverage, + so the ruleset carries a name CodeRabbit does not adopt and is passed with + --config instead. + """ + for name in ( + ".semgrep.yaml", ".semgrep.yml", "semgrep.yaml", "semgrep.yml", + ".opengrep.yaml", ".opengrep.yml", "opengrep.yaml", "opengrep.yml", + ): + self.assertFalse( + (ROOT / name).exists(), + f"{name} would replace CodeRabbit's own opengrep packs", + ) + ruleset = ROOT / ".opengrep/agentx-ifstack-rules.yaml" + self.assertTrue(ruleset.exists(), "the custom ruleset is missing") + + # Every rule needs a fixture, or it can silently stop matching. + rules = yaml.safe_load(ruleset.read_text())["rules"] + self.assertTrue(rules, "the ruleset declares no rules") + for rule in rules: + fixture = ROOT / ".opengrep/tests" / f"{rule['id']}.rs" + self.assertTrue( + fixture.exists(), f"rule {rule['id']} has no rule-test fixture" + ) + + steps = workflow("checks.yml")["jobs"]["rules"]["steps"] + commands = "\n".join(str(step.get("run", "")) for step in steps) + self.assertIn("opengrep-test.sh", commands, "CI must run the rule-tests") + self.assertIn("opengrep-scan.sh", commands, "CI must run the ruleset") + # The binary is fetched over the network, so pin it by digest. + self.assertIn("sha256sum -c -", commands, "pin the opengrep binary by checksum") + def test_package_scripts_use_private_temporary_files(self): """A predictable temporary path lets a local user redirect a root-run write.""" offenders = [] diff --git a/scripts/opengrep-scan.sh b/scripts/opengrep-scan.sh new file mode 100755 index 0000000..2a7372c --- /dev/null +++ b/scripts/opengrep-scan.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Run the custom opengrep ruleset over the source tree. CodeRabbit keeps running its own +# opengrep packs, because the ruleset is deliberately not named so CodeRabbit adopts it +# as its config. See .opengrep/README.md. +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +opengrep_bin="${OPENGREP_BIN:-}" +if [[ -z "$opengrep_bin" ]]; then + if command -v opengrep >/dev/null 2>&1; then + opengrep_bin="$(command -v opengrep)" + else + echo "error: opengrep not found. Install it from https://github.com/opengrep/opengrep" >&2 + echo " (or set OPENGREP_BIN=/path/to/opengrep)." >&2 + exit 1 + fi +fi + +# Scan explicit targets if given, otherwise the crate source. The fixtures under +# .opengrep/tests/ violate the rules on purpose and are not part of the crate. +targets=("$@") +if [[ ${#targets[@]} -eq 0 ]]; then + targets=("$repo_root/src") +fi + +exec "$opengrep_bin" scan \ + --config "$repo_root/.opengrep/agentx-ifstack-rules.yaml" \ + --error \ + "${targets[@]}" diff --git a/scripts/opengrep-test.sh b/scripts/opengrep-test.sh new file mode 100755 index 0000000..12dbed1 --- /dev/null +++ b/scripts/opengrep-test.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Run opengrep rule-tests for .opengrep/agentx-ifstack-rules.yaml against the annotated +# fixtures in .opengrep/tests/. Each fixture carries `// ruleid:` and `// ok:` markers +# asserting which lines must and must not match. +# +# `opengrep test` pairs a .yaml rule file with a same-stem .rs fixture in one +# directory. To keep one source of truth, stage a temporary directory pairing a copy of +# the ruleset with each fixture, then run the test there. +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +opengrep_bin="${OPENGREP_BIN:-}" +if [[ -z "$opengrep_bin" ]]; then + if command -v opengrep >/dev/null 2>&1; then + opengrep_bin="$(command -v opengrep)" + else + echo "error: opengrep not found. Install it from https://github.com/opengrep/opengrep" >&2 + echo " (or set OPENGREP_BIN=/path/to/opengrep)." >&2 + exit 1 + fi +fi + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +for fixture in "$repo_root"/.opengrep/tests/*.rs; do + stem="$(basename "$fixture" .rs)" + cp "$repo_root/.opengrep/agentx-ifstack-rules.yaml" "$tmp/$stem.yaml" + cp "$fixture" "$tmp/$stem.rs" +done + +exec "$opengrep_bin" test "$tmp" From 00df5133195658bede6a41c6e1a6246561ef3fb8 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 21:55:35 +0200 Subject: [PATCH 6/9] test: apply the duplicate-run policy to .yaml workflows too The test globbed *.yml, while workflow_paths() covers both suffixes and the two neighbouring tests already use it. A workflow named .yaml with push on every branch and a pull_request trigger would have passed the check it exists to make. Confirmed by adding such a workflow: the test passed before this change and fails after it. --- packaging/test_policy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 8ca3716..dfe7e13 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -330,7 +330,7 @@ def test_no_workflow_runs_twice_for_one_push(self): pull_request builds the merge commit, which is the result that matters for a pull request, so push stays on main for post-merge validation. """ - for path in sorted((ROOT / ".github/workflows").glob("*.yml")): + for path in sorted(workflow_paths()): workflow = yaml.safe_load(path.read_text()) # PyYAML follows YAML 1.1, where a bare `on:` key parses as the boolean True. triggers = workflow.get("on", workflow.get(True)) From f513c6230b2b263fb7869de3d69b269556c8b8c7 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 11 Sep 2026 23:03:37 +0200 Subject: [PATCH 7/9] fix: close three gaps in the guards added by this branch All three findings are against checks this branch introduced. packaging/release-build.sh accepted a directory whose name ends in .deb or .rpm, because find matched any entry type. A build could satisfy the guard without producing a package. Require a regular file with -type f. Confirmed: `find dist -name "*.deb"` prints a directory named pkg.deb, and `-type f` does not. The try_wait exemption matched the signature alone, so any same-shaped finish added to another type inherited it. Scope it to IpCommand::finish by requiring both enclosing contexts. The unwrap exemption accepted any module named tests, so a plain `mod tests` holding production code was exempt. Require the #[cfg(test)] attribute. Both rules gain a negative fixture for the bypass they missed. Checked by running the rule-tests against the previous rules: they fail on the new lines and pass against these, so the fixtures test the exemption rather than restating it. The release test asserted only that a --tag option appeared, which a stale literal or an unrelated variable would satisfy. Assert the whole publish command. The build-command test now also runs with a directory carrying each suffix, and failed against the previous script. --- .opengrep/agentx-ifstack-rules.yaml | 15 ++++++++-- .../tests/agentx-try-wait-outside-finish.rs | 12 ++++++++ .../tests/agentx-unwrap-outside-tests.rs | 10 +++++++ packaging/release-build.sh | 2 +- packaging/test_policy.py | 28 ++++++++++++++----- 5 files changed, 56 insertions(+), 11 deletions(-) diff --git a/.opengrep/agentx-ifstack-rules.yaml b/.opengrep/agentx-ifstack-rules.yaml index 416e95d..fc8a429 100644 --- a/.opengrep/agentx-ifstack-rules.yaml +++ b/.opengrep/agentx-ifstack-rules.yaml @@ -25,8 +25,14 @@ rules: - "tests/**" patterns: - pattern: $CHILD.try_wait() - - pattern-not-inside: | - fn finish(&mut self) -> Result { ... } + # Scope the exemption to IpCommand::finish. Matching the signature alone would + # exempt any same-shaped method added elsewhere. + - pattern-not: + patterns: + - pattern-inside: | + impl IpCommand { ... } + - pattern-inside: | + fn finish(&mut self) -> Result { ... } - id: agentx-unwrap-outside-tests languages: [rust] @@ -45,4 +51,7 @@ rules: - "tests/**" patterns: - pattern: $VALUE.unwrap() - - pattern-not-inside: mod tests { ... } + # Require the attribute: a plain `mod tests` is not a test module. + - pattern-not-inside: | + #[cfg(test)] + mod tests { ... } diff --git a/.opengrep/tests/agentx-try-wait-outside-finish.rs b/.opengrep/tests/agentx-try-wait-outside-finish.rs index b813f6b..471e172 100644 --- a/.opengrep/tests/agentx-try-wait-outside-finish.rs +++ b/.opengrep/tests/agentx-try-wait-outside-finish.rs @@ -27,3 +27,15 @@ fn wait_bounded(child: &mut Child) -> Result { } } } + +// A method with the same signature in another type must not inherit the exemption. +impl SomethingElse { + fn finish(&mut self) -> Result { + let mut child = self.child.take().expect("unreaped child"); + // ruleid: agentx-try-wait-outside-finish + match child.try_wait() { + Ok(Some(status)) => Ok(status), + _ => Err(Error::other("not reaped")), + } + } +} diff --git a/.opengrep/tests/agentx-unwrap-outside-tests.rs b/.opengrep/tests/agentx-unwrap-outside-tests.rs index e581ec8..eeff6be 100644 --- a/.opengrep/tests/agentx-unwrap-outside-tests.rs +++ b/.opengrep/tests/agentx-unwrap-outside-tests.rs @@ -20,3 +20,13 @@ mod tests { assert_eq!(production_ok("7").unwrap(), 7); } } + +// A module literally named tests, but without #[cfg(test)], is production code. +mod outer { + mod tests { + fn helper(input: &str) -> u32 { + // ruleid: agentx-unwrap-outside-tests + input.parse::().unwrap() + } + } +} diff --git a/packaging/release-build.sh b/packaging/release-build.sh index c794e6d..0848a0e 100755 --- a/packaging/release-build.sh +++ b/packaging/release-build.sh @@ -9,7 +9,7 @@ sh packaging/build.sh # semantic-release runs this before it commits, tags and pushes, so failing here stops # the release. A release that is already published cannot be un-published. for suffix in deb rpm; do - [ -n "$(find dist -name "*.${suffix}" -print -quit)" ] || { + [ -n "$(find dist -type f -name "*.${suffix}" -print -quit)" ] || { echo "dist holds no .${suffix}, refusing to release an incomplete package set" >&2 exit 1 } diff --git a/packaging/test_policy.py b/packaging/test_policy.py index dfe7e13..88cdb25 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -142,8 +142,13 @@ def test_the_release_uploads_both_package_formats(self): "run semantic-release publish, or the packages never reach the release", ) # publish defaults to the latest release, which would attach this run's packages - # to the previous tag when nothing was bumped. - self.assertIn("--tag", commands, "publish must name the tag it uploads to") + # to the previous tag when nothing was bumped. Pin the whole command: a stale + # literal or an unrelated variable would satisfy a bare "--tag" check. + self.assertIn( + 'semantic-release publish --tag "$after"', + commands, + "publish must upload to the tag this run created", + ) def test_the_build_command_refuses_an_incomplete_package_set(self): """build_command runs before the tag, so a missing format must stop the release. @@ -152,7 +157,7 @@ def test_the_build_command_refuses_an_incomplete_package_set(self): tagged, pushed and created the release by then. Run the real script against a stubbed build so the guard itself is exercised. """ - def run(produce): + def run(files, directories=()): with tempfile.TemporaryDirectory() as directory: work = Path(directory) (work / "packaging").mkdir() @@ -162,7 +167,8 @@ def run(produce): (work / "packaging/sync-version.sh").write_text("#!/bin/sh\n") (work / "packaging/build.sh").write_text( "#!/bin/sh\nset -eu\nmkdir -p dist\n" - + "".join(f"touch dist/pkg{suffix}\n" for suffix in produce) + + "".join(f"touch dist/pkg{suffix}\n" for suffix in files) + + "".join(f"mkdir -p dist/pkg{suffix}\n" for suffix in directories) ) return subprocess.run( ["sh", "packaging/release-build.sh"], cwd=work, @@ -170,12 +176,20 @@ def run(produce): ) self.assertEqual(run([".deb", ".rpm"]).returncode, 0, "a complete set must build") - for produce, missing in (([".deb"], ".rpm"), ([".rpm"], ".deb"), ([], "both")): - result = run(produce) + for files, missing in (([".deb"], ".rpm"), ([".rpm"], ".deb"), ([], "both")): self.assertNotEqual( - result.returncode, 0, + run(files).returncode, 0, f"the build command accepted a package set missing {missing}", ) + # A directory carrying the suffix is not a package. + for files, directories, shape in ( + ([".rpm"], [".deb"], "a directory named *.deb"), + ([".deb"], [".rpm"], "a directory named *.rpm"), + ): + self.assertNotEqual( + run(files, directories).returncode, 0, + f"the build command accepted {shape}", + ) def test_the_sync_script_carries_a_bump_into_every_version_source(self): """Run the real script on a real copy: a stub would not catch cargo drift.""" From 0988e38e529cb3306d9cc7ff7cb536ea96cf3aa7 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sat, 12 Sep 2026 00:07:19 +0200 Subject: [PATCH 8/9] fix: close three more gaps in this branch's own guards The unwrap rule excluded the module named tests rather than test modules. A `#[cfg(test)] mod unit_tests` was reported, so a correctly written test module failed CI. Exclude any module carrying the attribute. `mod $_ { ... }` matches a module of any name. A named metavariable, `mod $M { ... }`, does not match a Rust module declaration at all, which leaves nothing excluded, and a bare `...` excludes the whole file including production code. Both were measured before choosing `$_`. The attribute is still required, so a plain `mod tests` holding production code stays reported. The release test accepted any glob ending in .deb or .rpm, so `artifacts/*.deb` or `*.rpm` passed while build.sh writes into dist/ and publish would upload nothing. Require the dist/ prefix. The duplicate-run test skipped list-form triggers, so `on: [push, pull_request]` passed the check it exists to make. Handle the list form. Each fix was confirmed against the previous code: the fixture for a differently named test module reported an unexpected finding, and probes using a non-dist glob and a list-form trigger both passed before and fail now. --- .opengrep/agentx-ifstack-rules.yaml | 5 +++-- .opengrep/tests/agentx-unwrap-outside-tests.rs | 9 +++++++++ packaging/test_policy.py | 16 ++++++++++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.opengrep/agentx-ifstack-rules.yaml b/.opengrep/agentx-ifstack-rules.yaml index fc8a429..7f1c417 100644 --- a/.opengrep/agentx-ifstack-rules.yaml +++ b/.opengrep/agentx-ifstack-rules.yaml @@ -51,7 +51,8 @@ rules: - "tests/**" patterns: - pattern: $VALUE.unwrap() - # Require the attribute: a plain `mod tests` is not a test module. + # The attribute makes a test module, not its name. `$_` matches any module name; + # a named metavariable does not match a Rust module declaration here. - pattern-not-inside: | #[cfg(test)] - mod tests { ... } + mod $_ { ... } diff --git a/.opengrep/tests/agentx-unwrap-outside-tests.rs b/.opengrep/tests/agentx-unwrap-outside-tests.rs index eeff6be..176b457 100644 --- a/.opengrep/tests/agentx-unwrap-outside-tests.rs +++ b/.opengrep/tests/agentx-unwrap-outside-tests.rs @@ -30,3 +30,12 @@ mod outer { } } } + +// A test module may carry any name; the attribute is what makes it a test module. +#[cfg(test)] +mod unit_tests { + fn helper() { + // ok: agentx-unwrap-outside-tests + let _ = "7".parse::().unwrap(); + } +} diff --git a/packaging/test_policy.py b/packaging/test_policy.py index 88cdb25..f208c05 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -125,10 +125,14 @@ def test_the_release_uploads_both_package_formats(self): publish = config["tool"]["semantic_release"]["publish"] self.assertIs(publish["upload_to_vcs_release"], True) globs = publish["dist_glob_patterns"] + # build.sh writes into dist/, so a glob elsewhere uploads nothing. for suffix in (".deb", ".rpm"): self.assertTrue( - any(glob.endswith(suffix) for glob in globs), - f"no dist glob matches {suffix}: {globs}", + any( + glob.startswith("dist/") and glob.endswith(suffix) + for glob in globs + ), + f"no dist/ glob matches {suffix}: {globs}", ) workflow = yaml.safe_load((ROOT / ".github/workflows/release.yml").read_text()) steps = workflow["jobs"]["semantic-release"]["steps"] @@ -348,6 +352,14 @@ def test_no_workflow_runs_twice_for_one_push(self): workflow = yaml.safe_load(path.read_text()) # PyYAML follows YAML 1.1, where a bare `on:` key parses as the boolean True. triggers = workflow.get("on", workflow.get(True)) + # GitHub Actions also accepts `on: [push, pull_request]`. + if isinstance(triggers, list): + self.assertFalse( + "push" in triggers and "pull_request" in triggers, + f"{path.name}: push and pull_request both fire on a PR branch, " + "so every job runs twice; limit push to main", + ) + continue if not isinstance(triggers, dict) or "pull_request" not in triggers: continue push = triggers.get("push") From e5674a585aee1cee9b7dfdac7ac0f849aa33fc86 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Sat, 12 Sep 2026 06:28:14 +0200 Subject: [PATCH 9/9] fix: deny rustc warnings in the manifest, and reject a bare push trigger The [lints] table denied clippy lints but not rustc warnings, while CI passes -D warnings, so the parity the table exists to provide was only half there. Measured with an unused function: a plain cargo clippy exits 0 and reports a warning, while the CI form exits 101. Adding the warnings group makes both exit 101, so the local command now rejects what CI rejects. The duplicate-run test read the push trigger with triggers.get("push") and skipped a None result, but GitHub Actions allows an event with no configuration and PyYAML loads a bare `push:` as None. That trigger fires on every branch, so the workflow it was meant to catch passed. Treat a present but empty push trigger as the violation it is. Both were confirmed against the previous code: the unused function passed a plain clippy, and a workflow with a bare `push:` alongside `pull_request:` passed the test. Both now fail. --- Cargo.toml | 3 +++ packaging/test_policy.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9a27ae1..df529c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,9 @@ categories = ["network-programming", "command-line-utilities"] # Lint levels belong here, not only in the CI flag, so a local cargo clippy and CI # reject the same code. [lints.rust] +# The CI flag is -D warnings, which denies rustc warnings as well as clippy lints. +# Without this group a dead_code warning passes a plain cargo clippy and fails CI. +warnings = "deny" unsafe_op_in_unsafe_fn = "deny" [lints.clippy] diff --git a/packaging/test_policy.py b/packaging/test_policy.py index f208c05..8a42587 100644 --- a/packaging/test_policy.py +++ b/packaging/test_policy.py @@ -362,9 +362,16 @@ def test_no_workflow_runs_twice_for_one_push(self): continue if not isinstance(triggers, dict) or "pull_request" not in triggers: continue - push = triggers.get("push") - if push is None: + if "push" not in triggers: continue + push = triggers["push"] + # GitHub Actions allows an event with no configuration, which PyYAML loads + # as None. A bare `push:` fires on every branch. + self.assertIsNotNone( + push, + f"{path.name}: a bare push trigger fires on every branch, so every job " + "runs twice on a PR branch; limit push to main", + ) self.assertEqual( push.get("branches"), ["main"],