diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 394310d..98ee937 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -26,3 +26,42 @@ 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 + + # 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/.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/.github/workflows/release.yml b/.github/workflows/release.yml index dd46620..cb23ba9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -92,7 +92,20 @@ 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. The package set is + # checked inside build_command, which runs before the tag exists. - 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 + uv run semantic-release publish --tag "$after" 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..7f1c417 --- /dev/null +++ b/.opengrep/agentx-ifstack-rules.yaml @@ -0,0 +1,58 @@ +# 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() + # 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] + 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() + # 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 $_ { ... } 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..471e172 --- /dev/null +++ b/.opengrep/tests/agentx-try-wait-outside-finish.rs @@ -0,0 +1,41 @@ +// 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); + } + } +} + +// 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 new file mode 100644 index 0000000..176b457 --- /dev/null +++ b/.opengrep/tests/agentx-unwrap-outside-tests.rs @@ -0,0 +1,41 @@ +// 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); + } +} + +// 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() + } + } +} + +// 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/CLAUDE.md b/CLAUDE.md index f9570b7..aea541d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,8 +45,18 @@ 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`. + +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/Cargo.toml b/Cargo.toml index 96bcc14..df529c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,17 @@ 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] +# 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] +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/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/release-build.sh b/packaging/release-build.sh index 174a66e..0848a0e 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 -type f -name "*.${suffix}" -print -quit)" ] || { + echo "dist holds no .${suffix}, refusing to release an incomplete package set" >&2 + exit 1 + } +done 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 cb9960e..8a42587 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.""" @@ -116,6 +115,86 @@ 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"] + # build.sh writes into dist/, so a glob elsewhere uploads nothing. + for suffix in (".deb", ".rpm"): + self.assertTrue( + 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"] + 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. 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. + + 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(files, directories=()): + 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 files) + + "".join(f"mkdir -p dist/pkg{suffix}\n" for suffix in directories) + ) + 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 files, missing in (([".deb"], ".rpm"), ([".rpm"], ".deb"), ([], "both")): + self.assertNotEqual( + 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.""" bumped = "9.9.9" @@ -148,6 +227,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" @@ -229,6 +342,43 @@ 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(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)) + # 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 + 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"], + 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 = [] @@ -244,6 +394,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"