ci: add cargo-audit weekly + PR-on-Cargo-touch#52
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated GitHub Actions workflow to run cargo audit via rustsec/audit-check, providing a first-party RustSec advisory baseline on a weekly cadence and on dependency-changing PRs.
Changes:
- Introduces
.github/workflows/audit.ymlwithschedule,workflow_dispatch, and path-filteredpull_requesttriggers for Cargo files. - Checks out the repo with submodules enabled and runs
rustsec/audit-check.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| submodules: true | ||
|
|
||
| - name: Run cargo-audit | ||
| uses: rustsec/audit-check@v2 |
There was a problem hiding this comment.
rustsec/audit-check is referenced by tag (@v2) rather than being pinned to a commit SHA. Other workflows in this repo pin actions to a full SHA for supply-chain integrity; please pin this action similarly (and keep the # vX.Y.Z comment).
Suggested change
| uses: rustsec/audit-check@v2 | |
| uses: rustsec/audit-check@8d7d1b1c1f9d9f2d7d4f6e3e6b6f3d7a4c2b1e90 # v2.0.0 |
Comment on lines
+43
to
+46
| - name: Run cargo-audit | ||
| uses: rustsec/audit-check@v2 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
This workflow runs cargo-audit only once at the repo root, but the repo also has a separate cmtrace-wasm/Cargo.lock (self-hosting workspace) that can pull in distinct Rust dependencies. Since the workflow is triggered by changes to any **/Cargo.lock, a PR updating cmtrace-wasm/Cargo.lock would currently still only audit the root lockfile; consider adding a second audit step/job targeting cmtrace-wasm/ (or explicitly selecting lockfiles) so both lockfiles are covered.
Suggested change
| - name: Run cargo-audit | |
| uses: rustsec/audit-check@v2 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Install cargo-audit | |
| run: cargo install cargo-audit --locked | |
| - name: Run cargo-audit (root Cargo.lock) | |
| run: cargo audit --file Cargo.lock | |
| - name: Run cargo-audit (cmtrace-wasm/Cargo.lock) | |
| run: cargo audit --file cmtrace-wasm/Cargo.lock |
adamgell
added a commit
that referenced
this pull request
Apr 22, 2026
Two fixes rolled into PR #37 to unblock the cascade: 1. **Uploader missing Debug**: `tests/upload_integration.rs` and the half-set client-auth unit test call `expect_err()` which requires `Debug` on the `Ok` variant. Derive Debug on `Uploader` (both `reqwest::Client` and `UploaderConfig` are already Debug). 2. **cargo audit on RUSTSEC-2023-0071**: the new `cargo-audit` workflow (PR #52) flags Marvin attack on `rsa` v0.9.10 (transitively pulled by jwt-simple for JWT verification). No patched version exists. Add `audit.toml` ignore with explanation: api-server only verifies RSA-signed JWTs (no private-key ops), so the timing-sidechannel surface doesn't apply. Revisit when a fixed `rsa` lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adamgell
added a commit
that referenced
this pull request
Apr 22, 2026
* build(agent): add rustls + aws-lc-rs deps for real TLS PR #32 shipped the agent without TLS because both `rustls-tls` (pulls ring) and `aws-lc-rs` (needs cmake + NASM) tripped the dev box. CI runners have those toolchains, so wire rustls properly here and document the build-dep requirement so contributors can install it themselves. Reqwest now uses the `*-no-provider` feature variants (`rustls-tls-manual-roots-no-provider` + `rustls-tls-native-roots-no-provider`) — the non-suffixed variants transitively enable `__rustls-ring`, which would pull `ring` back into the tree alongside `aws-lc-rs`. With the no-provider features, `cargo tree -p agent | grep -iE "^ ring v|^ openssl v"` is empty. Wave 3 mTLS readiness: `rustls-pemfile` is in deps now so the follow-up uploader rewrite can load PEM-encoded client certs without another deps churn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent): add TLS config knobs (client cert, key, CA bundle) Adds three optional `Option<PathBuf>` fields to `AgentConfig`: * `tls_client_cert_pem` (env: `CMTRACE_TLS_CLIENT_CERT`) — PEM client cert chain for Wave 3 mTLS. None today = no client cert. * `tls_client_key_pem` (env: `CMTRACE_TLS_CLIENT_KEY`) — matching PEM private key. PKCS#8 / SEC1 / PKCS#1 all accepted by rustls-pemfile. Required when `tls_client_cert_pem` is set. * `tls_ca_bundle_pem` (env: `CMTRACE_TLS_CA_BUNDLE`) — optional override of the trusted root CA set. None = use OS native roots via `rustls-native-certs`. All three default to None, so existing TOML configs and env configurations keep their current behavior (native roots, no client auth). The uploader rewrite in the next commit consumes these fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(agent): real TLS in uploader via rustls + aws-lc-rs Replaces the plaintext-HTTP stop-gap from PR #32 with a real TLS layer. The uploader now hands reqwest a pre-built `rustls::ClientConfig` via `Client::builder().use_preconfigured_tls(..)`, keeping the choice of crypto provider in our hands rather than inheriting reqwest's default. New module `agent::tls`: * `install_default_crypto_provider()` — idempotent, OnceLock-guarded install of `rustls::crypto::aws_lc_rs::default_provider()` as the process-wide default. Called from `Uploader::new` so tests, the integration suite, and any future embedders all share one provider. * `TlsClientOptions` — mirror of the three TLS fields on `AgentConfig`. Decouples the uploader from the full config struct. * `build_client_config(opts)` — assembles a `rustls::ClientConfig`: - root store: custom CA bundle if `ca_bundle_pem` set, else OS native roots via `rustls-native-certs`. - client auth: `with_client_auth_cert(chain, key)` when both `client_cert_pem` and `client_key_pem` are set; otherwise `with_no_client_auth()`. Half-set is rejected loudly with `TlsConfigError::PartialClientAuth` so a misconfiguration surfaces at startup, not on the first chunk PUT. Unit tests in `tls.rs`: * `builds_without_client_cert` — default options yield a working ClientConfig against the OS native trust store. * `partial_client_auth_is_rejected` — half-set client auth errors. * `missing_cert_file_surfaces_path` — Io errors carry the bad path for ops grepability. * `empty_pem_file_surfaces_no_certs_error` — a PEM file with no certs yields `NoCerts` rather than a silent empty chain. A throwaway-cert generation test (rcgen) was deliberately skipped — rcgen pulls ring transitively, which would defeat the no-ring-in-the-tree posture this PR establishes. A full mTLS round trip is gated on Wave 3 server-side mTLS landing; a TODO comment in the integration test calls this out. `UploaderConfig` grows a `tls: TlsClientOptions` field with a builder-style `with_tls(..)` setter; `UploaderError` grows a `Tls` variant for surfacing TLS-config errors. `main.rs` threads the three `tls_*_pem` fields from `AgentConfig` through to `UploaderConfig::with_tls`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(agent): wire TlsClientOptions through end-to-end upload test Fills in the new `tls: TlsClientOptions::default()` field on the `UploaderConfig` literal so the existing end-to-end test compiles against the rewritten uploader. Default options (native roots, no client cert) are correct for the test: it talks to a loopback api-server over `http://`, where rustls is bypassed entirely and the agent client falls through to plain TCP. Adds a `TODO(wave-3)` marker for the missing mutual-TLS integration test — that one needs server-side mTLS enforcement (deferred to Wave 3) plus a throwaway cert generator (rcgen, which we don't want to pull yet because it brings ring back into the tree). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): derive Debug on Uploader + ignore RSA Marvin advisory Two fixes rolled into PR #37 to unblock the cascade: 1. **Uploader missing Debug**: `tests/upload_integration.rs` and the half-set client-auth unit test call `expect_err()` which requires `Debug` on the `Ok` variant. Derive Debug on `Uploader` (both `reqwest::Client` and `UploaderConfig` are already Debug). 2. **cargo audit on RUSTSEC-2023-0071**: the new `cargo-audit` workflow (PR #52) flags Marvin attack on `rsa` v0.9.10 (transitively pulled by jwt-simple for JWT verification). No patched version exists. Add `audit.toml` ignore with explanation: api-server only verifies RSA-signed JWTs (no private-key ops), so the timing-sidechannel surface doesn't apply. Revisit when a fixed `rsa` lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): install rustls provider in tests that build reqwest direct After PR #46 unified workspace reqwest features to rustls (no provider), test code that constructs `reqwest::Client::new()` directly (bypassing `Uploader::new`'s install hook) panics with "No provider set". Three unit tests in `uploader.rs` and `start_server` in `upload_integration.rs` all needed an explicit `install_default_crypto_provider()` call. The function is `OnceLock`- guarded so calling it from many places is fine. The api-server crate is also affected — its router-construction path constructs a reqwest client for the JWKS cache, which is why `start_server()` (which builds the api-server router) panics before `Uploader::new` even runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(api+ci): install rustls provider in JwksCache + ignore RSA via action input Two fixes to unblock the cascade after #46 reqwest rustls migration: 1. **api-server JwksCache::with_ttl**: builds a `reqwest::Client` which panics with "No provider set" because reqwest's rustls features need the global crypto provider installed first. main.rs installs eagerly at startup; tests that construct JwksCache directly bypass main. Move the install into `with_ttl` so any code path constructing the cache gets the provider for free. Idempotent (rustls install_default no-ops if a provider is already set). Fixes 9 api-server test failures: routes::status::tests::*, auth::tests::*, auth::device_identity::tests::*. 2. **cargo-audit workflow**: `audit.toml` at repo root isn't being honored by `rustsec/audit-check@v2` (the action's settings dump shows ignore=[] even when audit.toml has the entry). Pass `ignore: RUSTSEC-2023-0071` as workflow input instead. The rationale stays in audit.toml for human readers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
GitHub keeps surfacing a low-severity Dependabot alert on every push (visible at https://github.com/adamgell/cmtraceopen-web/security/dependabot/1) and we currently have no first-party baseline for catching new RustSec advisories. Dependabot lags advisory-db, doesn't fail CI, and only fires on its own cadence. This wires the official rustsec/audit-check action into CI so:
Cargo.toml/Cargo.lockand pulls in a known-vulnerable crate fails the check instead of merging silently.workflow_dispatchafter fixing an advisory.Triggers + permissions
Single job (
ubuntu-latest) runningrustsec/audit-check@v2afteractions/checkout@v6withsubmodules: true(the rootCargo.lockpath-deps on./cmtraceopen/crates/cmtraceopen-parser, mirroringci.yml).Currently-open Dependabot alerts
Pulled via
gh api repos/adamgell/cmtraceopen-web/dependabot/alerts:randrand::rng()Will the first run fail? Probably not.
GHSA-cq8v-f236-94qcis a soundness advisory onrand(PR #1763), and as of writing it does not appear to have a corresponding entry in rustsec/advisory-db (Dependabot consumes the GHSA database, which is broader than RustSec).cargo-auditonly fires on RustSec advisories, so this specific alert is expected to be silent on the first run.If a RustSec advisory does land for it later (or for any other dep in our lockfile), the orchestrator can either:
audit.tomlat the repo root with an[advisories] ignore = ["RUSTSEC-XXXX-XXXX"]block and a justification comment.No
audit.tomlis being added preemptively — let's see what the first real run reports.Test plan
Cargo.toml/Cargo.lock; this PR touches neither, soauditwill be skipped here — that's fine, it'll wake up on the cron or on the next dep-bump PR).workflow_dispatchthe new workflow once from the Actions tab to confirm it runs end-to-end againstmainand reports any RustSec findings.