Skip to content

CI hardening backlog: actionlint, release attestation, scheduled advisories, stamp drift #169

Description

@plusky

Findings from a CI gap assessment (2026-08-30), ranked, with the rustdoc
gate split out as #168 because it is the one to do first.

Baseline for calibration: this repo is already well above ecosystem
average — ten required contexts, every action SHA-pinned with
persist-credentials: false, deny.toml covering advisories/licenses/bans/
sources with yanked = "deny", dependency-review at PR time, Dependabot on
cargo+actions+docker, secret scanning and push protection on, digest-pinned
Docker FROMs, unsafe_code = "forbid" workspace-wide, and MSRV/beta jobs
that now assert their own toolchain. What follows are deltas.

Worth doing now

1. actionlint (with embedded shellcheck) on .github/workflows/.
Analyze (actions) does scan the workflows, but for security
anti-patterns — not YAML validity or shell correctness. A malformed
ci.yml self-blocks harmlessly (its required contexts never report), but
release.yml is ~16K of YAML with elaborate bash and is validated by
nothing before a tag push
, which AGENTS.md calls irreversible. A quoting
typo there costs a tag. Honest limit: the three workflow defects that
actually shipped (#137, #160, #102) were semantic — actionlint would have
caught none of them. This buys tag-time bash safety; someone is already
shellchecking by hand (# shellcheck disable=SC2046 in release.yml).

2. Release provenance attestation. Nothing shipped is signed or
attested, and the .sha256 files live in the same release as the artifacts
— integrity, zero authenticity. For a security product people mount in
front of Bugzilla this is the largest genuine gap.
actions/attest-build-provenance on the tarballs/deb/rpm (~10 lines plus
id-token: write), and provenance: mode=max on the existing
docker/build-push-action. crates.io already gets provenance via Trusted
Publishing. Consider cargo auditable so downstreams can run
cargo audit bin on shipped binaries.

3. Scheduled advisory run. rust-deny fires only on push/PR, so a
fresh RustSec advisory announces itself by failing whichever stranger's PR
arrives next. A cron on a deny-advisories-only job fixes the messenger.
Dependabot alerts cover most of it, but RustSec's unmaintained/unsound
classes are its own. (cargo-audit itself: redundant with deny, skip.)

4. DESIGN.md rmcp stamp-drift check. DESIGN.md hard-stamps the rmcp
version in ~5 places; #135 / 3e7eff0 exists because a bump landed and the
stamps went stale. Two sed calls in the house style of the existing
toolchain-drift and rust-assets-drift jobs.

5. Two one-liners. (a) deny.toml has all-features = false, so the
gen-only deps (clap_complete, clap_mangen) sit outside advisory/licence/
bans scanning — yet the gen tool runs in CI and its output ships inside the
deb/rpm. Set all-features = true. (b) rust-msrv runs cargo build
only, so an MSRV break in test code or dev-deps passes CI and breaks a
contributor on 1.88; cargo check --workspace --all-targets --locked is
near-free.

Worth doing later

6. Differential/property tests for the hand-written OTLP encoder.
otel.rs's wire module hand-rolls varints and length prefixes. A wrong
length prefix fails silently at the collector — the audit-adjacent export
goes dark. prost + proptest as dev-deps, round-trip-decode what wire
emits. An afternoon, no CI infrastructure. (Full fuzzing: not worth it —
the byte-level parse surfaces are operator- or Bugzilla-supplied, and the
one attacker-facing parser is small and heavily tested.)

7. Container smoke test. The docker job is build-only; nothing ever
runs the assembled image — distroless, nonroot, env defaults, the
BUGWARDEN_POLICY refuse-to-start path. #114 (SIGTERM) was a shipped
container-behaviour defect; binary_shutdown.rs covers signals at the
binary level, not the image.

8. Automated mutation testing, non-blocking only. The hand-run
discipline works and is the house method; automation would catch lapses,
not replace review. A weekly cargo-mutants run with a curated baseline,
or --in-diff as a non-required PR job. Never a required check — a full
run over server.rs is hours, and unviable/timeout mutants need curation.

Deliberately rejected

  • Coverage as a gate. The defects this repo ships are wrong assertions
    in covered code, which coverage cannot see and mutation testing can.
    Line coverage would read high and gate nothing.
  • cargo-vet / cackle. High maintenance for a solo-maintained repo
    already above average on supply chain. (aws-lc-sys compiles C in
    build.rs and would be flagged forever.)
  • cargo-hack feature matrix. One optional gen feature, which already
    has its own clippy leg. Nothing to matrix.
  • Flaky-test retries. The response to the three transport races was to
    make the tests deterministic; retries would have masked them.

Note: no gate catches "comments that were true once" — that stays a review
problem. Item 4 covers the one mechanically checkable instance.


Split into subtasks (2026-08-31)

This is now a tracking issue. Each item below has its own issue carrying the
reasoning and the honest limits; the prose above is the assessment that produced
them and should not be edited further — amend the subtasks instead.

Worth doing now

subtask
1 #195 — actionlint: nothing validates release.yml's YAML or bash before an irreversible tag push
2 #196 — release provenance attestation (the largest genuine gap)
3 #197 — scheduled advisory run, so a new advisory does not fail a stranger's PR
4 #198 — DESIGN.md rmcp stamp-drift check
5a #199deny.toml all-features = false leaves the gen-only deps unscanned
5b #200rust-msrv builds but does not check tests

Item 5 was split: the two one-liners are different subsystems (supply-chain
scanning vs MSRV coverage) and would be separate PRs.

Worth doing later

subtask
6 #201 — differential/property tests for the hand-written OTLP wire encoder
7 #202 — the docker job never runs the image it builds
8 #203 — automated mutation testing, non-blocking only

The rejected items stay rejected and deliberately have no subtasks: coverage
as a gate, cargo-vet/cackle, a cargo-hack feature matrix, flaky-test retries. The
reasoning is above; re-proposing any of them should start by refuting it.

One update to the assessment's closing note: it said no gate catches "comments
that were true once", and that item 4 covers the only mechanically checkable
instance. Since then the rustdoc gate (#168) shipped and catches a second class —
broken intra-doc links. The rest of the class is still a review problem, and has
since produced #172, #174, #179, #184 and #193.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestgithub_actionsPull requests that update GitHub Actions code

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions