Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
15 changes: 14 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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"
39 changes: 39 additions & 0 deletions .opengrep/README.md
Original file line number Diff line number Diff line change
@@ -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: <rule-id>`.
58 changes: 58 additions & 0 deletions .opengrep/agentx-ifstack-rules.yaml
Original file line number Diff line number Diff line change
@@ -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<ExitStatus> { ... }

- 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 $_ { ... }
41 changes: 41 additions & 0 deletions .opengrep/tests/agentx-try-wait-outside-finish.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Fixture for agentx-try-wait-outside-finish. Contains rule-violating code on purpose.

impl IpCommand {
fn finish(&mut self) -> Result<ExitStatus> {
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<ExitStatus> {
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<ExitStatus> {
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")),
}
}
}
41 changes: 41 additions & 0 deletions .opengrep/tests/agentx-unwrap-outside-tests.rs
Original file line number Diff line number Diff line change
@@ -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::<u32>().unwrap()
}

fn production_ok(input: &str) -> Result<u32> {
// ok: agentx-unwrap-outside-tests
input.parse::<u32>().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::<u32>().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::<u32>().unwrap();
}
}
10 changes: 10 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,18 @@ cargo test <test_name> # 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.

Expand Down
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment thread
coderabbitai[bot] marked this conversation as resolved.

[lints.clippy]
all = { level = "deny", priority = -1 }

[dependencies]
agentx = "0.1.1"
serde = { version = "1", features = ["derive"] }
Expand Down
31 changes: 31 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 1 addition & 1 deletion packaging/changelog
Original file line number Diff line number Diff line change
Expand Up @@ -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 <[email protected]> Fri, 11 Sep 2026 18:05:06 GMT
-- Marcin Zieba <[email protected]> Fri, 11 Sep 2026 18:05:06 +0000

agentx-ifstack (0.1.0-1) unstable; urgency=medium

Expand Down
9 changes: 9 additions & 0 deletions packaging/release-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion packaging/sync-version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
Loading