Skip to content

Harden CI supply chain and add grouped Dependabot config - #300

Open
vpetersson wants to merge 7 commits into
masterfrom
chore/dependabot-grouped-updates
Open

Harden CI supply chain and add grouped Dependabot config#300
vpetersson wants to merge 7 commits into
masterfrom
chore/dependabot-grouped-updates

Conversation

@vpetersson

@vpeterssonvpetersson commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

CI supply-chain hardening, plus the dependency-update plumbing that prompted it.

Background

Five Dependabot PRs (#280, #283, #288, #289, #290) sat open for up to three months. All five turned out to be already superseded by #297 (7531baa) and #299 (5dad863) — every target version was at or below what Cargo.lock already had. They've been closed. This PR addresses why they accumulated, and what turned up while looking.

Changes

1. Add .github/dependabot.yml

The repo had no Dependabot config at all, so security updates ran with default settings and opened one PR per advisory.

  • Cargo — security updates grouped into a single PR. Groups default to version updates, so applies-to: security-updates is required.
  • Cargo — version updates stay disabled via open-pull-requests-limit: 0. This repo takes CVE-driven bumps only; enabling routine version updates would add PR volume rather than reduce it. Deliberate no-change to current behavior.
  • GitHub Actions — grouped, both version and security. Not previously covered at all.

2. Drop unmaintained / third-party actions

  • actions-rs/clippy-check@v1.0.7cargo clippy directly. The actions-rs org has been archived since 2021 and the action runs on the deprecated Node 12 runtime.
  • dtolnay/rust-toolchainrustup, in docs.yml, release.yml, and fmt.yml. The runners have rustup preinstalled. No dtolnay references remain.

Permissions on the lint job drop to contents: readpull-requests: write and checks: write existed only so the action could post check annotations.

3. Pin all third-party actions to commit SHAs

Nothing was digest-pinned. Four actions ran from mutable branch refsdtolnay/rust-toolchain@master, @nightly, sbomify/github-action@master, DeterminateSystems/flakehub-cache-action@main — meaning any upstream push executed automatically. Two of those run in release.yml and sbom.yml, which build and attest release artifacts.

Every third-party reference is now pinned to a full SHA with the version as a trailing comment. Dependabot updates digest pins and maintains those comments, which is what the grouping in (1) is for.

screenly/cli@master is intentionally left unpinned: actions.yml exercises the CLI at master by design.

4. Upgrade outdated actions

  • actions/checkout v3 → v4 in docs.yml and release.yml, matching the other workflows. v3 runs on deprecated Node 16.
  • softprops/action-gh-release v1 → v3.0.2. Also a Node 16 action; v3 runs Node 24. Input-compatible — every input this workflow uses (files, plus the GITHUB_TOKEN env) still exists in v3, with only additions.
  • sbomify/github-actionmasterv26.7.0. action.yml is byte-identical between the two, so the interface is unchanged. Note this moves back 10 commits — from an untagged branch tip onto the latest release, which is the point.
  • flakehub-cache-action needed no change: its pinned SHA is already identical to v3.21.7, so only the trailing comment was corrected.

5. Fix 19 clippy warnings and enforce the gate

useless_borrows_in_formatting has fired since clippy 1.97 but went unnoticed, because the old actions-rs step only annotated warnings rather than failing on them. Removed 19 redundant & in format!/debug!/println! args across six files via cargo clippy --fix — 19 insertions, 19 deletions, inert since formatting traits auto-deref.

With the tree clean, the clippy step now runs -D warnings so these can't silently accumulate again.

6. Path-filter fix

lint.yml was added to its own paths filter. Previously, edits to the lint workflow didn't run it, so a broken lint config could merge unnoticed.

Verification

All 9 checks pass. Locally, against clippy/rustc 1.97.1 (matching the 1.97 the runner ships):

  • cargo clippy --all-targets -- -D warnings — clean
  • cargo test — 200 passed, 0 failed
  • cargo fmt --all --check — clean
  • cargo build --all-targets — clean
  • All workflow YAML parses

Reviewer notes — please read before merging

release.yml and sbom.yml trigger only on v* tags, so no PR check exercises them. Green CI on this PR says nothing about either. This branch changes the release pipeline in three unverified ways:

  1. dtolnay/rust-toolchainrustup across the 8-target build matrix
  2. actions/checkout v3 → v4
  3. action-gh-release v1 → v3.0.2 (two major versions)

A throwaway pre-release tag is the only way to confirm these before a real release depends on them. This is the main risk in the PR.

Pinning sbomify/github-action is largely cosmetic. Its action.yml runs docker://ghcr.io/sbomify/sbomify-action:latest — a mutable Docker tag. Whatever ref is pinned here, the executed code is whatever :latest resolves to at run time. sbom.yml therefore still has an unpinned supply chain, and it can't be fixed from this side; mitigating it means forking or replacing the action. This is the one real gap remaining.

rustfmt.toml's unstable options are now inert.group_imports and imports_granularity only applied under nightly; fmt.yml now uses stable. Ordinary formatting drift is still caught, but import ordering is no longer enforced. Either delete them as dead config or keep them for developers who run nightly by choice.

-D warnings with a floating toolchain will break again when a future clippy adds lints — that's exactly how the 19 warnings here surfaced. It's the tradeoff for the gate being meaningful; the alternative is pinning the clippy toolchain.

🤖 Generated with Claude Code

vpeterssonand others added 2 commits July 20, 2026 11:07
The repo had no dependabot.yml, so Dependabot security updates ran with
default settings and opened one PR per advisory. Five such PRs (#280,
#283, #288, #289, #290) accumulated over three months and were only
closed after the bumps had already landed via #297 and #299.
Group Cargo security updates into a single PR. Version updates stay off
(open-pull-requests-limit: 0) to preserve current behavior -- this repo
takes CVE-driven bumps only, so enabling them would add PR volume rather
than reduce it.
Also add grouped GitHub Actions updates, which were not covered before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The actions-rs org has been archived since 2021 and clippy-check@v1.0.7
runs on the deprecated Node 12 runtime. Run clippy directly via rustup,
which is preinstalled on the runner -- no third-party action involved.
Behavior changes:
- Adds -D warnings, so lints now fail the job rather than only annotating.
Verified clean: `cargo clippy --all-targets --all-features -- -D warnings`
passes on master.
- Adds --all-targets, extending coverage to tests and benches.
- Drops pull-requests: write and checks: write. Those existed so the
action could post check annotations; a plain cargo step needs neither,
leaving contents: read.
- Adds lint.yml to its own paths filter so changes to this workflow are
exercised by it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vpetersson
vpeterssonforce-pushed the chore/dependabot-grouped-updates branch from 0925f56 to 948c3b1CompareJuly 20, 2026 11:42
vpeterssonand others added 4 commits July 20, 2026 11:44
No action in the repo was pinned by digest. Four ran from mutable branch
refs -- dtolnay/rust-toolchain@master, @nightly, sbomify/github-action@master,
and DeterminateSystems/flakehub-cache-action@main -- meaning any upstream
push executed automatically. Two of those run in release.yml and sbom.yml,
which build and attest release artifacts.
Pin every third-party action to a full commit SHA with the previous ref
retained as a trailing comment. Dependabot updates digest pins and
maintains those comments, and the github-actions grouping added earlier
in this branch keeps the churn to one PR.
Versions are unchanged -- each SHA is the current tip of the ref already
in use, so this is a hardening change with no behavior difference.
screenly/cli@master is intentionally left unpinned: actions.yml is an
integration test that exercises the CLI at master by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the third-party toolchain action from docs.yml and release.yml in
favor of rustup, which is preinstalled on the ubuntu-22.04 and macos-15
runners these jobs use.
The action's toolchain/target inputs map directly onto rustup commands,
so the matrix in release.yml keeps installing the same per-target
toolchains it did before.
fmt.yml still uses the action, since that job needs a nightly toolchain
for the unstable options in rustfmt.toml.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop dtolnay/rust-toolchain from fmt.yml, the last remaining use. The
job now runs stable rustfmt via rustup. The unstable options in
rustfmt.toml (group_imports, imports_granularity, format_generated_files)
are ignored on stable with a warning; `cargo fmt --all --check` passes
either way, so the check still catches ordinary formatting drift.
Also revert the -D warnings flag added earlier in this branch. The CI
runner ships clippy 1.97, which has useless_borrows_in_formatting; that
lint fires 19 times in src/ and turned the lint job red. Those warnings
predate this branch -- the old actions-rs step only annotated them.
Restoring the non-blocking behavior keeps this branch to CI plumbing
rather than mixing in src/ changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove 19 redundant `&` in format!, debug!, and println! arguments across
six files, applied via `cargo clippy --fix`. Formatting traits auto-deref,
so these are inert -- 19 insertions, 19 deletions, no behavior change.
The lint (useless_borrows_in_formatting) has been firing since clippy
1.97 but went unnoticed because the old actions-rs step only annotated
warnings rather than failing on them.
With the tree clean, restore -D warnings on the clippy step so these
cannot silently accumulate again. Verified against clippy 1.97.1, which
matches the 1.97 the runner ships.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vpeterssonvpetersson changed the title Add dependabot config to group dependency update PRsHarden CI supply chain and add grouped Dependabot configJul 20, 2026
- actions/checkout v3 -> v4 in docs.yml and release.yml, matching the
other workflows. v3 runs on deprecated Node 16.
- softprops/action-gh-release v1 -> v3.0.2. Also a Node 16 action; v3
runs Node 24. Input-compatible: every input this workflow uses (files,
plus the GITHUB_TOKEN env) still exists in v3.
- sbomify/github-action master -> v26.7.0, moving off an untagged branch
tip onto the latest release. action.yml is byte-identical between the
two, so the interface is unchanged.
- flakehub-cache-action comment corrected to # v3.21.7. The pinned SHA is
already identical to that tag, so only the comment was wrong.
Note that pinning sbomify/github-action is largely cosmetic: its
action.yml runs docker://ghcr.io/sbomify/sbomify-action:latest, a mutable
tag, so the executed code is whatever :latest resolves to regardless of
the ref pinned here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the repository’s CI supply chain by adding a Dependabot configuration, removing unmaintained/third-party actions, and pinning GitHub Actions to immutable commit SHAs; it also updates CI workflows to run Rust tooling directly and applies clippy-driven formatting fixes across the Rust codebase.

Changes:

  • Add grouped Dependabot configuration for Cargo security updates and GitHub Actions updates.
  • Pin third-party GitHub Actions to commit SHAs, upgrade outdated actions, and remove unmaintained actions in favor of direct rustup/cargo usage.
  • Apply clippy autofixes across multiple Rust command/API modules and enforce clippy warnings as errors in CI.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
src/commands/screen.rsClippy formatting tweak for URL construction.
src/commands/mod.rsClippy formatting tweaks; response logging line updated in get().
src/commands/edge_app/utils.rsRemove redundant borrow in debug logging.
src/commands/edge_app/app.rsRemove redundant borrows in debug logging and URL construction.
src/commands/asset.rsRemove redundant borrow in URL construction and debug logging.
src/api/edge_app/setting.rsRemove redundant borrows in debug logging for payloads.
.github/workflows/sbom.ymlPin actions to SHAs and pin sbomify action reference.
.github/workflows/rust.ymlPin checkout/cache actions to SHAs.
.github/workflows/release.ymlReplace rust-toolchain action with rustup, pin actions, upgrade release action.
.github/workflows/nix.ymlPin nix-related actions to SHAs (installer + cache).
.github/workflows/lint.ymlEnsure workflow runs on its own edits, drop elevated permissions, replace clippy action with cargo clippy -D warnings, pin checkout.
.github/workflows/fmt.ymlReplace rust-toolchain action with rustup component add rustfmt, pin checkout.
.github/workflows/docs.ymlPin actions and replace rust-toolchain action with rustup installs.
.github/workflows/actions.ymlPin checkout action to SHA (CLI action remains intentionally on master).
.github/dependabot.ymlNew Dependabot config with grouping for Cargo security and GitHub Actions updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/commands/mod.rs
Comment on lines 191 to 194
if status != StatusCode::OK {
println!("Response: {:?}", &response.text());
println!("Response: {:?}", response.text());
return Err(CommandError::WrongResponseStatus(status.as_u16()));
}
Comment on lines 660 to 663
let status = response.status();
if status != StatusCode::CREATED {
debug!("Response: {:?}", &response.text());
debug!("Response: {:?}", response.text());
return Err(CommandError::WrongResponseStatus(status.as_u16()));

@sergey-borovkovsergey-borovkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verification: all 9 pinned SHAs resolve to their claimed tags (including softprops/action-gh-release v3.0.2, whose annotated tag derefs to 3d0d988). No wrong or unknown digests. The Rust changes are pure no-ops — & before a formatting arg is inert since format traits auto-deref.

Solid, well-scoped hardening pass, and the description is unusually honest about its own gaps. Most findings below are things the pass stopped just short of.


1. fmt.yml: the regression is bigger than the description says

The description flags that group_imports/imports_granularity go inert. The CI log from this PR shows all fiverustfmt.toml options are dropped:

Warning: can't set `imports_granularity = Module` ...
Warning: can't set `group_imports = StdExternalCrate` ...
Warning: can't set `format_generated_files = false` ...
Warning: can't set `unstable_features = true` ...
Warning: can't set `ignore = IgnoreList { path_set: {"furiousfilter/**"} } ...

So in CI, rustfmt.toml is now effectively an empty file. That's a semantic change to a merge gate riding along in a supply-chain PR — and it isn't required to reach the PR's goal, since dropping dtolnay doesn't require dropping nightly:

- name: Install nightly rustfmtrun: rustup toolchain install nightly --profile minimal --component rustfmt
- name: Run rustfmt checkrun: cargo +nightly fmt --all --manifest-path Cargo.toml --check

This removes the third-party action and preserves behavior exactly. I'd prefer it over the "delete the dead config" option — deleting it is a real policy change that deserves its own PR rather than being a side effect of pinning actions.

2. actions/attest-build-provenance left three majors behind

Pinned to ef24412 = v1.4.4, commented # v1; latest is v4.1.1. This is the action producing provenance attestations for release artifacts and the SBOM — the one place in the repo where staleness has security meaning. The PR upgrades checkout and action-gh-release on exactly this rationale but skips this one.

Related: this PR's own CI log emits

Node.js 20 is deprecated ... actions/checkout@34e114876b... forced to run on Node.js 24

checkout v4.3.1 targets Node 20, now deprecated — the same argument used for v3→v4 applies to v4→v5. Either go to v5, or state explicitly that v4 is the intended floor.

3. rust.yml runs rust:latest — a fixable instance of the gap you called out

build_and_test runs docker run ... rust:${{ matrix.rust }} with matrix.rust: [latest, 1.88.0]. That's a mutable Docker tag executing against PR-authored code on every push — structurally the same problem as sbomify's docker://...:latest, except this one is fixable from this repo (pin a digest, or name a concrete version). Relatedly, dependabot.yml has no docker ecosystem entry despite the repo shipping a Dockerfile.

4. The path-filter fix was applied to lint.yml only

fmt.yml has the identical bug — its paths filter is **/*.rs, so edits to fmt.yml don't run it. Same one-line fix.

fmt.yml also still carries permissions: pull-requests: write, which — like the lint permissions this PR correctly drops — nothing in the job uses.

Minor: lint.yml's paths covers **.rs and Cargo.toml but not Cargo.lock, so a lockfile-only dependency bump — i.e. exactly what the new grouped Dependabot PRs will be — skips clippy entirely.

5. Nits

  • Version-comment precision is inconsistent.# v4 is really v4.3.1, # v3 is v3.0.3, # v1 is v1.4.4 — while # v3.0.2, # v26.7.0, # v3.21.7 are exact. The comment is the only human-readable check on a digest, and Dependabot rewrites these to exact versions on update anyway, so I'd make them exact now rather than let the style drift.
  • Dead config worth a follow-up: there's no .gitmodules and no furiousfilter path in this repo, so rustfmt.toml's ignore is already vestigial — as are submodules: recursive and ssh-key: ${{ secrets.SSH_PRIVATE_KEY }} in the fmt.yml/lint.yml checkouts. That last one hands a private SSH key to two jobs that don't need it, which is odd company for a hardening PR.
  • Pre-existing, but adjacent to touched lines:debug!("Response: {:?}", response.text()?) in src/commands/mod.rs (post, delete, patch) puts a ? inside a log macro. debug! only evaluates its args when the level is enabled, so whether that error propagates depends on the log level.

dependabot.yml — correct as written

open-pull-requests-limit: 0 does disable version updates without affecting security updates, and applies-to: security-updates is genuinely required for the group to apply to them. No issues.

On the release-pipeline risk

Your reviewer note is right. The rustup replacement looks faithful to what dtolnay/rust-toolchain did (install with --profile minimal, set default, add target), and rustup is preinstalled on both ubuntu-22.04 and macos-15 — but "looks faithful" is doing real work across an 8-target matrix plus two unverified action major bumps. A throwaway v*-rc tag is cheap insurance.

Recommendation: address (1) before merge — three lines, and it's a gate weakening. (2), (3), (4) are worth folding in now since the PR is already in those files. The rest can be follow-ups.

@sergey-borovkovsergey-borovkov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking on finding (1) in my review above: fmt.yml's switch to stable rustfmt silently disables all five rustfmt.toml options (confirmed in this PR's own CI log), which weakens a merge gate as a side effect of a supply-chain PR. The three-line cargo +nightly fmt fix keeps the dtolnay removal while preserving behavior exactly.

Everything else in the review is non-blocking.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vpetersson@sergey-borovkov