Homebrew 7 Fix - #26
Conversation
rustsec/audit-check's findOrInstall() falls back to `cargo install cargo-audit` without --locked. Unpinned resolution now picks kstring 2.0.5, which requires rustc 1.96.0, while rust-toolchain.toml pins 1.93.1, so cargo-audit fails to compile and the job exits 101. No advisory was actually reported. The same audit job is a hard needs: gate for prepare-matrix and build-app-assets in S2, so this blocks the 3.1.1 release. Installing the prebuilt binary via taiki-e/install-action (already used here for cargo-tarpaulin) makes the fallback a no-op. Also mark the S1 beta matrix leg continue-on-error: beta clippy's double_must_use now fires on every async fn in the async_trait Provider trait, which is upstream lint churn we do not own and which does not exist in S2's stable-only matrix.
With cargo-audit actually running again, the audit gate reports real findings. All of them resolve in the lockfile, no manifest changes: RUSTSEC-2026-0258 h2 0.4.15 -> 0.4.19 RUSTSEC-2026-0195 quick-xml 0.39.4 -> 0.42.0 (plist 1.9.0 -> 1.10.1) RUSTSEC-2026-0194 quick-xml (same bump) RUSTSEC-2026-0285 rustls 0.23.40 -> 0.23.45 RUSTSEC-2026-0253 lru 0.18.0 -> 0.18.4 (unsound warning) quick-xml is reached through plist via Tauri and is capped at ^0.39.2 by plist 1.9.0, so it needs the plist bump to move. rustls needed --precise: plain update stopped at 0.23.43 to avoid pulling aws-lc-sys 0.45. cargo test --workspace passes.
async_trait rewrites each async fn into a #[must_use] boxed future, which beta clippy's new double_must_use then flags as redundant. The attribute comes from the macro expansion, not from our source, so there is nothing to remove; an allow on the trait is the only lever. Verified with cargo +beta clippy --workspace --all-targets --all-features -- -D warnings (clean). Stable clippy and fmt unchanged. The continue-on-error canary on the beta matrix leg stays: it keeps the next round of beta lint churn from blocking the release path.
Reviewer's GuideThe release CI audit gate now receives a preinstalled cargo-audit compatible with the pinned Rust toolchain, while beta S1 checks remain non-blocking for upstream lint churn. The lockfile is refreshed to resolve the reported RustSec advisories, and the provider trait suppresses a beta clippy warning produced by async_trait. Sequence diagram for the release RustSec audit gatesequenceDiagram
participant CI as Release CI
participant Install as taiki-e/install-action
participant Audit as rustsec/audit-check
participant Toolchain as Pinned Rust toolchain
participant Lockfile as Cargo.lock
CI->>Install: install cargo-audit
Install-->>CI: cargo-audit in CARGO_HOME/bin
CI->>Audit: run audit-check
Audit->>Toolchain: use pinned toolchain
Audit->>Lockfile: audit locked dependencies
Lockfile-->>Audit: resolved advisory-free versions
Audit-->>CI: pass or report RustSec findings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe workflows now tolerate beta test failures, install ChangesCI toolchain compatibility
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit reads each line, Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/S1-Test-CI.yml" line_range="19" />
<code_context>
runs-on: ${{ matrix.os }}
+ # The beta leg is an early-warning canary for upcoming compiler and lint
+ # changes, not a gate. Beta clippy churn must not turn CI red.
+ continue-on-error: ${{ matrix.rust == 'beta' }}
strategy:
fail-fast: false
</code_context>
<issue_to_address>
**issue (broader_impact):** The beta `test` job is allowed to succeed when any step fails, including formatting, compilation, or runtime tests; those failures are indistinguishable from tolerated beta Clippy lint churn and therefore do not block downstream CI.
**Triggers:** When the beta compiler exposes a real regression unrelated to upstream lint changes.
**Suggested fix:** Allow only the beta Clippy step to fail, or split beta Clippy into a separate continue-on-error job while keeping beta formatting, compilation, and tests as required checks.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the new install action changes the release audit path, and the lockfile updates cryptographic and TLS dependencies; if either is wrong, a release could pass an ineffective audit or ship altered security behavior. Reverting restores future CI behavior, but it cannot undo a release artifact or security decision made while this was active.
Blocking findings: .github/workflows/S1-Test-CI.yml:19
| runs-on: ${{ matrix.os }} | ||
| # The beta leg is an early-warning canary for upcoming compiler and lint | ||
| # changes, not a gate. Beta clippy churn must not turn CI red. | ||
| continue-on-error: ${{ matrix.rust == 'beta' }} |
There was a problem hiding this comment.
issue (broader_impact): The beta test job is allowed to succeed when any step fails, including formatting, compilation, or runtime tests; those failures are indistinguishable from tolerated beta Clippy lint churn and therefore do not block downstream CI.
Triggers: When the beta compiler exposes a real regression unrelated to upstream lint changes.
Suggested fix: Allow only the beta Clippy step to fail, or split beta Clippy into a separate continue-on-error job while keeping beta formatting, compilation, and tests as required checks.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Fixes RustSec audit gating and beta Clippy behavior while updating vulnerable locked dependencies.
Changes:
- Preinstalls
cargo-auditin S1 and S2. - Makes the beta CI canary non-blocking.
- Suppresses the beta Clippy false positive.
- Refreshes affected dependencies.
| File | Description |
|---|---|
crates/git-same-core/src/provider/traits.rs |
Suppresses beta Clippy’s double_must_use false positive. |
Cargo.lock |
Refreshes audited dependency versions. |
.github/workflows/S2-Release-GitHub.yml |
Installs cargo-audit for release auditing. |
.github/workflows/S1-Test-CI.yml |
Configures beta CI and installs cargo-audit. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/git-same-core/src/provider/traits.rs">
<violation number="1" location="crates/git-same-core/src/provider/traits.rs:178">
P3: The `#[allow(clippy::double_must_use)]` only covers the `Provider` trait definition, but `#[async_trait]` generates the same `#[must_use]`-annotated boxed futures for impl blocks. Since clippy 1.82+ (rust-clippy#16633) treats `Pin<Box<dyn Future + Send>>` as must_use via the `Future` trait, the beta clippy that fires this lint flags the two `#[async_trait] impl Provider` blocks in `crates/git-same-core/src/provider/github/client.rs:114` and `crates/git-same-core/src/provider/mock.rs:118` as well, so the beta canary stays red (now silently swallowed by `continue-on-error`). Cover all three generated-method sites — e.g. a file-level `#![allow(clippy::double_must_use)]` in each file (the pattern async-trait's own test suite applies: "async_trait marks its generated boxed futures as must_use. Newer Clippy versions otherwise report those generated attributes as redundant on every async trait method") or the same attribute on each impl block.</violation>
</file>
<file name=".github/workflows/S2-Release-GitHub.yml">
<violation number="1" location=".github/workflows/S2-Release-GitHub.yml:183">
P3: The audit tool version is left floating: `taiki-e/install-action@v2` (a moving major tag) installs the latest `cargo-audit` release on every run, and `rustsec/audit-check@v2` then runs whatever binary is on PATH. Since `cargo-audit` post-0.12 changed its JSON warning shape (which audit-check's `main.ts` parses with a format check) and can change which advisories are reported or how `--ignore` is handled, a new upstream release can flip the security gate from green to red without any dependency change. This is worth pinning or at least acknowledging, since the whole point of this step is to make audit-check deterministic on the pinned toolchain.</violation>
</file>
<file name=".github/workflows/S1-Test-CI.yml">
<violation number="1" location=".github/workflows/S1-Test-CI.yml:19">
P1: The job-level `continue-on-error` allows beta formatting, compilation, tests, and audit failures to pass as well as Clippy lint churn. Remove this job-level setting and apply `continue-on-error` only to the beta Clippy step so real beta regressions remain required checks.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| runs-on: ${{ matrix.os }} | ||
| # The beta leg is an early-warning canary for upcoming compiler and lint | ||
| # changes, not a gate. Beta clippy churn must not turn CI red. | ||
| continue-on-error: ${{ matrix.rust == 'beta' }} |
There was a problem hiding this comment.
P1: The job-level continue-on-error allows beta formatting, compilation, tests, and audit failures to pass as well as Clippy lint churn. Remove this job-level setting and apply continue-on-error only to the beta Clippy step so real beta regressions remain required checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/S1-Test-CI.yml, line 19:
<comment>The job-level `continue-on-error` allows beta formatting, compilation, tests, and audit failures to pass as well as Clippy lint churn. Remove this job-level setting and apply `continue-on-error` only to the beta Clippy step so real beta regressions remain required checks.</comment>
<file context>
@@ -14,6 +14,9 @@ jobs:
runs-on: ${{ matrix.os }}
+ # The beta leg is an early-warning canary for upcoming compiler and lint
+ # changes, not a gate. Beta clippy churn must not turn CI red.
+ continue-on-error: ${{ matrix.rust == 'beta' }}
strategy:
fail-fast: false
</file context>
| // `async_trait` rewrites each `async fn` into a `#[must_use]` boxed future, | ||
| // which beta clippy's `double_must_use` then flags as redundant. The attribute | ||
| // is generated by the macro, not written here, so there is nothing to remove. | ||
| #[allow(clippy::double_must_use)] |
There was a problem hiding this comment.
P3: The #[allow(clippy::double_must_use)] only covers the Provider trait definition, but #[async_trait] generates the same #[must_use]-annotated boxed futures for impl blocks. Since clippy 1.82+ (rust-clippy#16633) treats Pin<Box<dyn Future + Send>> as must_use via the Future trait, the beta clippy that fires this lint flags the two #[async_trait] impl Provider blocks in crates/git-same-core/src/provider/github/client.rs:114 and crates/git-same-core/src/provider/mock.rs:118 as well, so the beta canary stays red (now silently swallowed by continue-on-error). Cover all three generated-method sites — e.g. a file-level #![allow(clippy::double_must_use)] in each file (the pattern async-trait's own test suite applies: "async_trait marks its generated boxed futures as must_use. Newer Clippy versions otherwise report those generated attributes as redundant on every async trait method") or the same attribute on each impl block.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/git-same-core/src/provider/traits.rs, line 178:
<comment>The `#[allow(clippy::double_must_use)]` only covers the `Provider` trait definition, but `#[async_trait]` generates the same `#[must_use]`-annotated boxed futures for impl blocks. Since clippy 1.82+ (rust-clippy#16633) treats `Pin<Box<dyn Future + Send>>` as must_use via the `Future` trait, the beta clippy that fires this lint flags the two `#[async_trait] impl Provider` blocks in `crates/git-same-core/src/provider/github/client.rs:114` and `crates/git-same-core/src/provider/mock.rs:118` as well, so the beta canary stays red (now silently swallowed by `continue-on-error`). Cover all three generated-method sites — e.g. a file-level `#![allow(clippy::double_must_use)]` in each file (the pattern async-trait's own test suite applies: "async_trait marks its generated boxed futures as must_use. Newer Clippy versions otherwise report those generated attributes as redundant on every async trait method") or the same attribute on each impl block.</comment>
<file context>
@@ -172,6 +172,10 @@ impl DiscoveryProgress for NoProgress {
+// `async_trait` rewrites each `async fn` into a `#[must_use]` boxed future,
+// which beta clippy's `double_must_use` then flags as redundant. The attribute
+// is generated by the macro, not written here, so there is nothing to remove.
+#[allow(clippy::double_must_use)]
#[async_trait]
pub trait Provider: Send + Sync {
</file context>
| # pinned in rust-toolchain.toml. Dropping a prebuilt binary into | ||
| # $CARGO_HOME/bin first makes that fallback a no-op. | ||
| - name: Install cargo-audit | ||
| uses: taiki-e/install-action@v2 |
There was a problem hiding this comment.
P3: The audit tool version is left floating: taiki-e/install-action@v2 (a moving major tag) installs the latest cargo-audit release on every run, and rustsec/audit-check@v2 then runs whatever binary is on PATH. Since cargo-audit post-0.12 changed its JSON warning shape (which audit-check's main.ts parses with a format check) and can change which advisories are reported or how --ignore is handled, a new upstream release can flip the security gate from green to red without any dependency change. This is worth pinning or at least acknowledging, since the whole point of this step is to make audit-check deterministic on the pinned toolchain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/S2-Release-GitHub.yml, line 183:
<comment>The audit tool version is left floating: `taiki-e/install-action@v2` (a moving major tag) installs the latest `cargo-audit` release on every run, and `rustsec/audit-check@v2` then runs whatever binary is on PATH. Since `cargo-audit` post-0.12 changed its JSON warning shape (which audit-check's `main.ts` parses with a format check) and can change which advisories are reported or how `--ignore` is handled, a new upstream release can flip the security gate from green to red without any dependency change. This is worth pinning or at least acknowledging, since the whole point of this step is to make audit-check deterministic on the pinned toolchain.</comment>
<file context>
@@ -175,6 +175,15 @@ jobs:
+ # pinned in rust-toolchain.toml. Dropping a prebuilt binary into
+ # $CARGO_HOME/bin first makes that fallback a no-op.
+ - name: Install cargo-audit
+ uses: taiki-e/install-action@v2
+ with:
+ tool: cargo-audit
</file context>
Preinstall the
cargo-auditbinary in S1 and S2 so RustSec checks work with the pinned Rust toolchain. Keep the S1 beta compiler canary informative without allowing upstream lint churn to block CI. Suppress beta Clippy’s macro-generateddouble_must_usefalse positive on theProvidertrait. Update locked dependencies to resolve the reported h2, quick-xml, rustls, and lru advisories.Summary by cubic
Fixes the RustSec audit gate in S1 and S2 workflows, which failed when
rustsec/audit-checkfell back to compilingcargo-auditwith dependencies that need a newer Rust than the pinned toolchain. Keeps the S1 beta compiler leg as an early-warning canary rather than a gate.Bug Fixes
cargo-auditviataiki-e/install-actionso the unpinned fallback install is never used.continue-on-errorto the beta matrix leg so upcoming beta clippy churn stays informational.double_must_useonProvider; the#[must_use]future is emitted by theasync_traitmacro.Dependencies
Cargo.lockto clear the h2, quick-xml, rustls, and lru RustSec advisories.plistto unlockquick-xmlpast the vulnerable version; no manifest changes required.Written for commit 4f4e9dc. Summary will update on new commits.
Summary by CodeRabbit