Skip to content

chore(ci): harden the workflows and check licences - #326

Merged
LKSNDRTMLKV merged 3 commits into
mainfrom
chore/ci-parity-with-the-engine
Sep 21, 2026
Merged

LKSNDRTMLKV merged 3 commits into
mainfrom
chore/ci-parity-with-the-engine

Conversation

@LKSNDRTMLKV

Copy link
Copy Markdown
Member

P1–P6 and P9 of the check-coverage audit. Stacked on #325 (which is stacked on
#322) because all three touch .github/workflows/ci.yml. Retarget to main
as the parents merge.

Every item here is something the platform repo already does. None of it reached
this repo — which is the finding underneath the findings.

P1 — CI ran twice per push, and the obvious fix would have broken stacked PRs

push: ["**"] alongside pull_request ran the whole suite twice for every
push to a branch with an open PR: two runners, same commit, simultaneously. Two
independent samples of every timing-sensitive gate, so a flaky-by-timing job had
two chances to trip per push.

🚨 The trap: pull_request: branches: [main, develop] filters on the branch
a PR merges into, so it excludes every stacked PR. That is masked today by
push: ["**"] firing on all branches — I confirmed it live, since #325 is
stacked and its checks come from the push trigger, not the PR one. Fixing the
double-run by narrowing push without also dropping the branch filter would
have silently left stacked PRs with no CI at all.

Both halves land together: push: [main], pull_request: with no filter, plus
workflow_dispatch for the pre-PR case push: ["**"] was really serving.
develop is gone — no such branch exists on this remote.

Applied to all five affected workflows.

P2 — Cargo.lock is now tracked

Without it --locked means nothing, and cargo audit scans a graph resolved
fresh on each run — so a finding can appear or vanish between two runs over an
identical tree. For a library other people vendor, "what was this built from"
should have an answer.

Three lockfiles, one per workspace: root, plugins/, fuzz/. The ten
plugins/*/Cargo.lock files are deliberately excluded — they date from
2026-07-21, before those crates shared a workspace, and no longer govern
anything. Left on disk rather than deleted (not this change is business), but not
committed as though they were policy.

Cost, accepted: a lockfile conflicts on concurrent dependency changes, and
strict-up-to-date already serialises merges here. Regenerate with
cargo update -w, never resolve by hand.

P3 — cargo-deny, and it failed on the first run

cargo audit answers one question. It cannot answer the three that matter for a
published Apache-2.0 crate: licences, duplicate/banned crates, provenance.

🚨 Licences had never been checked, and the first run rejected one.
webpki-root-certs (CDLA-Permissive-2.0) reaches this library as a runtime
dependency via jsonschema -> reqwest -> rustls-platform-verifier. It is
genuinely permissive — the Mozilla CA root store, no reciprocity — and is now
explicitly allowed with that reasoning recorded. The point is that nothing had
ever looked, and a consumer vendoring this crate inherits its graph.

The allow-list is permissive-only and tighter than the platform is, deliberately:
a copyleft dependency arriving transitively changes a consumers obligations
without telling them. cargo deny check advisories is left out rather than run
twice — cargo audit owns that.

P4/P5/P6 — hardening

  • permissions: contents: read on all nine workflows. There were none
    anywhere; every job inherited a writable token.
  • 19 action refs SHA-pinned, resolved from each actions official repo so
    the pin carries provenance and not just immutability — the platform does not
    verify that a SHA belongs to the repo you named. upload-artifact is pinned at
    both v4 and v7 where each was already used; unifying them is a behaviour
    change and not this PRs business, but it is worth knowing they differ.
  • concurrency with cancel-in-progress on every PR-triggered workflow,
    excluding main — the post-merge run is the record for that commit and
    cancelling it would leave a merged commit never fully built. Scheduled
    workflows are left alone; a nightly run has no successor.

P9 — a scheduled advisory run

A new RUSTSEC advisory lands against a lockfile nobody is touching, so no
PR-triggered job fires and the first anyone hears of it is whenever someone next
opens a PR. advisories.yml runs daily against the now-committed lockfile — its
job is to fail on a day when nothing was committed.

Verification

just check green, 1561/1561, bans ok, licenses ok, sources ok. All nine
workflow files parse and carry a permissions block.

Not included

P7 (CI for the infrastructure repo), P8 (PR CI for the control plane) and P10
(the propagation mechanism) are separate repos or a separate decision, and
follow separately.

@LKSNDRTMLKV LKSNDRTMLKV added security Security-relevant issue type/decision Needs a call; analysis alone will not close it urgency/next Blocks work already scheduled surface/docs-only Contained to documentation; no code depends on it labels Sep 19, 2026
@LKSNDRTMLKV LKSNDRTMLKV changed the title chore(ci): bring the gates up to the platform's standard chore(ci): harden the workflows and check licences Sep 19, 2026
@LKSNDRTMLKV
LKSNDRTMLKV force-pushed the chore/gates-that-cannot-pass-vacuously branch from 4bcffd2 to b40a213 Compare September 21, 2026 15:29
Base automatically changed from chore/gates-that-cannot-pass-vacuously to main September 21, 2026 15:32
@LKSNDRTMLKV
LKSNDRTMLKV force-pushed the chore/ci-parity-with-the-engine branch from 0e513d6 to 250b09a Compare September 21, 2026 15:43
@LKSNDRTMLKV

Copy link
Copy Markdown
Member Author

Rebased onto main (#322 and #325 have landed; #326's own delta is conserved exactly — same file/line stat before and after).

Reviewed, and one addition. Verifying the parts that are hard to eyeball:

  • SHA pins. This PR introduces six, all actions/*. Each is the exact tag tip in the official repo — checked via git ls-remote tag targets, not the commits API, because GitHub shares objects across a fork network and repos/{owner}/{repo}/commits/{sha} will happily 200 on a SHA that only exists in a fork. The other four pins (dtolnay, taiki-e, Swatinem) are pre-existing on main and untouched here.
  • Job display names are unchanged, so all eight required contexts in the ruleset still report. Renaming Sector plugin tests here would have blocked every PR silently.
  • deny.toml is permissive-only with no copyleft and no exceptions — consistent with what the body argues.

🚨 One of the three lockfiles was stale when it was committed

fuzz/Cargo.lock still pinned the workspace crates at 0.10.0 (plus jsonschema 0.46.10, fancy-regex 0.18.0, fraction 0.15.4). Root and plugins/ were both in sync; only fuzz/ was not.

It is the same kind of artefact as the ten plugins/*/Cargo.lock files this PR deliberately excludes for being pre-workspace leftovers — this one just happened to sit at a path the exclusion did not cover, so it got committed as though it were policy. Regenerated: Locking 13 packages, four workspace crates 0.10.0 → 0.20.0.

And nothing would have caught it. Committing a lockfile buys reproducible advisory scanning only if something proves the lock still describes the manifests; otherwise a version bumped in a Cargo.toml leaves cargo audit and the new daily advisories.yml run reading the old graph — authoritative-looking and wrong, which is the failure mode this branch is named for. Worth noting the body's "without it --locked means nothing" is set up but not yet cashed in: nothing builds the workspace --locked, so the benefit actually realised is the deterministic audit graph.

Added just lock-check (cargo metadata --locked per workspace), wired into check and into the CI audit job before the scanners. Watched to fail on the real case — restoring the stale fuzz/Cargo.lock gives:

error: cannot update the lock file .../fuzz/Cargo.lock because --locked was passed to prevent this
error: Recipe `lock-check` failed with exit code 101

just check green, 1564/1564, 3 lockfiles match their manifests., bans ok, licenses ok, sources ok.

One note for triage, not a blocker: this PR is labelled surface/docs-only ("Contained to documentation; no code depends on it"), which does not match a change that adds a licence gate, commits three lockfiles and rewrites the triggers on nine workflows. Left alone rather than relabelled.

@LKSNDRTMLKV
LKSNDRTMLKV merged commit 8f13bb7 into main Sep 21, 2026
13 of 14 checks passed
@LKSNDRTMLKV
LKSNDRTMLKV deleted the chore/ci-parity-with-the-engine branch September 21, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Security-relevant issue surface/docs-only Contained to documentation; no code depends on it type/decision Needs a call; analysis alone will not close it urgency/next Blocks work already scheduled

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant