chore(security): patch dependency advisories and close cache-poisoning alerts - #166
chore(security): patch dependency advisories and close cache-poisoning alerts#166panos-xyz wants to merge 3 commits into
Conversation
Dependabot flags two advisories against the lockfile: - GHSA-4w2j-m93h-cj5j (high) quinn-proto < 0.11.15 -> 0.11.17 - GHSA-7gcf-g7xr-8hxj (moderate) serde_with < 3.21.0 -> 3.22.0 Both are transitive, so this is a lockfile-only bump. `serde_with` is reachable from the built binary via alloy-consensus; `quinn-proto` resolves in the lockfile but is not in the compiled graph, and is bumped to clear the alert rather than because it was exploitable. The added lockfile entries (jiff*, defmt*, bitflags 1.x, rand_pcg, portable-atomic-util) are optional dependencies of the two upgraded crates and are recorded by the resolver without being compiled. `cargo check --workspace --all-targets` passes. The third open Dependabot alert, GHSA-xwfj-jgwm-7wp5 (low) against tracing-subscriber 0.2.25, is deliberately not addressed here. That version is an optional dependency of ark-relations 0.5.1 (reached via revm-precompile -> ark-bn254 -> ark-r1cs-std), its feature is not enabled, so it never compiles into morph-reth. The advisory is fixed in 0.3.20, and ark-relations pins `version = "0.2"`, so no in-range patch exists.
|
Warning Review limit reached
Next review available in: 46 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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. Comment |
Code scanning reports three high-severity
`actions/cache-poisoning/poisonable-step` alerts against release.yml, at the
`cargo metadata`, `cargo install cross`, and `make build-*` steps.
The workflow accepts a `tag` input on workflow_dispatch and feeds it to
`actions/checkout` as `needs.extract-version.outputs.ref`. Dispatch runs hold
write access to the default-branch cache scope, so an arbitrary ref could
build and write a cache entry that later privileged workflows restore.
The untrusted checkout is the source; the cache is the sink. Removing
`Swatinem/rust-cache` removes the sink and keeps workflow_dispatch, `dry_run`
and `profile` intact. This is also CodeQL's first recommendation ("avoid using
caching in workflows that handle sensitive operations like releases") and
matches upstream reth, whose release workflow uses the same
`inputs.ref || github.ref` checkout pattern and caches nothing.
Cost is a cold build per release. Releases are infrequent — ten runs over the
past five months, all tag pushes — so the isolation is worth more than the
build time.
The other cached workflows are unaffected: test/lint/build run on
`pull_request` or `push: main` and check out `github.ref`, and docker.yml's
workflow_dispatch takes no inputs, so none of them combine an untrusted ref
with a cache.
cargo-deny started failing on every PR after RUSTSEC-2026-0247 was published:
the bitmaps repository was archived on 2026-05-03, so the crate is flagged
unmaintained with all versions affected and no upgrade available.
It reaches us only transitively, and only through upstream reth:
bitmaps 3.2.1 <- imbl 7.0.0 <- reth-transaction-pool 2.4.0 <- morph-*
There is nothing to bump on our side, and unmaintained is not a vulnerability.
Upstream reth ignores the same advisory with the same reasoning, so follow it
rather than pinning around a dependency we do not own.
This matches the three unmaintained-crate entries already in the ignore list.
Clears the open security alerts on
main: 2 of 3 Dependabot advisories and all 3 code-scanning alerts.quinn-proto < 0.11.15serde_with < 3.21.0tracing-subscriber 0.2.25actions/cache-poisoning/poisonable-stepbitmapsunmaintained1. Dependency advisories (
Cargo.lockonly)Both are transitive, so this is a lockfile-only bump.
serde_withis reachable from the built binary viaalloy-consensus, so this alert was real.quinn-protoresolves in the lockfile butcargo tree -i quinn-protofinds nothing — it is not in the compiled graph. Bumped to clear the alert rather than because it was exploitable.The new lockfile entries (
jiff*,defmt*,bitflags1.x,rand_pcg,portable-atomic-util) are optional dependencies of the two upgraded crates, recorded by the resolver without being compiled.The low-severity one is not fixable
GHSA-xwfj-jgwm-7wp5 against
tracing-subscriber 0.2.25has no in-range patch and is unreachable:ark-relationsdeclares it optional and the enabling feature is off, socargo tree -i tracing-subscriber@0.2.25returns nothing — it never compiles intomorph-reth.ark-relationsrequiresversion = "0.2", so no patch exists in range.Suggest dismissing that alert as not-affected rather than forcing a resolution.
2. Cache poisoning in
release.ymlVerify Cargo.toml version matches tag(cargo metadata)Install cross main(cargo install cross --git)Build binary(make build-*)The workflow takes a
taginput onworkflow_dispatchand feeds it toactions/checkoutasneeds.extract-version.outputs.ref. Dispatch runs hold write access to the default-branch cache scope, so an arbitrary ref could be built and its output written into a cache entry that privileged workflows later restore.The untrusted checkout is the source; the cache is the sink. This PR removes the sink.
Why remove the cache rather than the dispatch input
workflow_dispatch,dry_runandprofileall stay.Removing
workflow_dispatchinstead would also clear the alerts but would drop the manual dry-run andmaxperfreference-build path.Cost: a cold build per release. Releases are infrequent — ten runs over the past five months, all tag pushes — so the isolation is worth more than the build time.
Other workflows are not affected
test.yml,lint.yml,build.yml:pull_request/push: [main], and they check outgithub.ref. PR runs are scoped to the PR branch cache.docker.yml: itsworkflow_dispatchtakes no inputs and its checkout has noref:override.None combine an untrusted ref with a cache, which matches code scanning flagging only
release.yml.3. cargo-deny: RUSTSEC-2026-0247
Unrelated to the two changes above, but it blocks this PR (and every other open PR).
The advisory was published after
mainlast ran CI:bitmaps 3.2.1is unchanged frommain; it is not one of the entries this PR addsto the lockfile. It reaches us only transitively, and only through upstream reth:
There is nothing to bump on our side, and unmaintained is not a vulnerability.
Upstream reth already ignores this advisory with the same reasoning, so this
follows it rather than pinning around a dependency we do not own. The entry
matches the three unmaintained-crate entries already in
deny.toml.Verification
CodeQL on this branch reports zero cache-poisoning alerts, so all three are resolved — including #46, which sits in
check-versionand has no cache step of its own. That confirms the query's cache condition is evaluated per workflow, not per job, so removing the single caching action clears every flagged step. The default-branch alerts close once this merges.Follow-up, not in this PR
Upstream also pins actions by SHA and declares workflow-level
permissions: {}with explicit per-job grants; we use floating@v7/@v2tags. Upstream additionally audits workflows with zizmor rather than CodeQL. Worth doing, but out of scope here.