Skip to content

build: add a 14-day publish-age cooldown to dependency resolution - #607

Merged
MegaRedHand merged 3 commits into
mainfrom
build/min-publish-age-cooldown
Sep 4, 2026
Merged

build: add a 14-day publish-age cooldown to dependency resolution#607
MegaRedHand merged 3 commits into
mainfrom
build/min-publish-age-cooldown

Conversation

@pablodeymo

Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

Ports Commit-Boost/commit-boost-client#492. Cargo's unstable min-publish-age (rust-lang/cargo#17009) lets the resolver skip crate versions published less than N days ago, a cooldown against freshly compromised releases. This enables it at 14 days, adds the Makefile path for resolving under it, and makes every build --locked so stable cargo cannot silently re-resolve around the policy.

The feature is nightly-only; stable 1.97.1 ignores the tables silently. Only the pinned nightly-2026-06-21 resolver enforces the cooldown, so --locked everywhere is what makes an unresolved manifest change fail loudly instead of resolving on stable.

What Changed

File Change
.cargo/config.toml [unstable] min-publish-age = true, [registry] global-min-publish-age = "14 days"
Makefile make update (resolve under the cooldown), make update-allow PACKAGE= VERSION= (escape hatch, bypasses the whole resolution), make cooldown-check (advisory). make lint/make test now --locked
.github/workflows/ci.yml cargo check/clippy run --locked; new cooldown job annotates the run with a warning, never fails
Dockerfile cargo chef cook honors $LOCKED like the final build; the shadow variant still sets it empty
CONTRIBUTING.md, CLAUDE.md Document the workflow

Correctness / Behavior Guarantees

  • No manifest or lockfile change. Locked builds are byte-for-byte what they were.
  • A cargo update --workspace --dry-run does not flag too-new locked crates, so the check has to be the full cargo update --dry-run -Z min-publish-age, grepping Downgrading|is too new. That run also refreshes git dependencies (leanSig, leanVM, ethrex, rust-libp2p) to their branch heads with no cooldown; git revs have no publish age and must be reviewed by hand.
  • Known state today: a full re-resolution under any window of 8+ days fails with a misleading rand ^0.10 conflict. Real cause: rand 0.10 needs chacha20 0.10, whose 0.10.0/0.10.1 are yanked and whose only live release (0.10.2) was published 2026-08-27. make update errors and the CI job emits a "probe failed" warning until 2026-09-10; make update-allow covers an urgent bump before then. The committed lockfile already pins the yanked chacha20 0.10.0, which any future re-resolution will move.

Tests Added / Run

  • make lint passes with --locked.
  • Stable cargo check --locked and cargo update --workspace --dry-run --locked with the new tables: no warnings, no changes.
  • Temporarily pinned smallvec 1.16.0 (3 days old) and ran the 7-day dry-run: Downgrading smallvec v1.16.0 -> v1.15.2 (...), confirming the grep pattern. Lockfile restored identical.
  • Ran the CI step script locally and make cooldown-check: both report the chacha20-driven probe failure as a warning with exit 0.

Related Issues / PRs

✅ Verification Checklist

  • Ran make fmt — clean (no Rust changes)
  • Ran make lint (clippy with -D warnings) — clean
  • Ran make test — not run; no Rust or fixture changes, only the --locked flag was added

…--locked everywhere

Ports Commit-Boost/commit-boost-client#492. Cargo's unstable min-publish-age
(rust-lang/cargo#17009) excludes crate versions published less than N days ago
from resolution, a cooldown against freshly compromised releases. The policy
lives in .cargo/config.toml; stable cargo 1.97.1 ignores the tables silently, so
only the pinned nightly-2026-06-21 resolver behind `make update` enforces it.
That is why `make lint`, `make test`, CI check/clippy and the Docker cook step
now pass --locked: an unresolved manifest change fails loudly instead of being
re-resolved on stable around the cooldown.

`make update-allow PACKAGE= VERSION=` is the escape hatch for a version younger
than the window; it bypasses the whole resolution, so the lockfile diff must be
reviewed. `make cooldown-check` and a new advisory CI job surface lockfile
entries younger than the window via `cargo update --dry-run`, which is the only
form that flags them (`--workspace` does not), and never fail the build.

As of today a full re-resolution under any window of 8+ days fails: rand 0.10
needs chacha20 0.10, whose 0.10.0/0.10.1 are yanked and whose only live release
was published 2026-08-27. `make update` errors and the CI job warns until
2026-09-10. The lockfile is unchanged and still pins the yanked chacha20 0.10.0.
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

I'll review this PR which adds a supply-chain security mechanism (publish-age cooldown for dependencies) to the ethlambda project.

Overall Assessment

This is a well-designed supply-chain security feature with good documentation and tooling. However, I found several issues ranging from minor improvements to potential reliability concerns.


Detailed Findings

1. Hardcoded Future Date for Nightly Toolchain (Critical Reliability Issue)

File: .github/workflows/ci.yml (lines 101, 102), Makefile (line 23)

The nightly toolchain nightly-2026-06-21 is a future date (June 21, 2026). This is problematic:

  • Line 101: rustup toolchain install nightly-2026-06-21 --profile minimal
  • Line 23 in Makefile: RESOLVER_TOOLCHAIN := nightly-2026-06-21

This appears to be a placeholder or typo. As of early 2025, this date is in the future. Rust nightlies are only available for ~167 days before being pruned from the release servers. When this date passes or if it's invalid, the entire update, update-allow, and cooldown-check workflows will fail.

Recommendation: Use a recent, stable nightly date or track a specific version via rust-toolchain.toml. Consider using a manifest approach:

# In a separate toolchain file or documented procedure
[toolchain]
channel = "nightly-2025-01-15"  # Use actual available nightly
components = ["cargo"]

2. Silent Failures in CI Cooldown Job (Medium Severity)

File: .github/workflows/ci.yml (lines 100-104)

if ! rustup toolchain install nightly-2026-06-21 --profile minimal; then
    echo "::warning title=Publish-age cooldown check skipped::toolchain install failed"
    exit 0
fi

The exit 0 on failure means this job never fails. While documented as "this job never fails the build," this creates a blind spot:

  • If the toolchain install fails for reasons other than the future date (network issues, rustup bugs), the supply-chain check is silently skipped
  • An attacker who compromises a dependency could potentially also disrupt this check without raising alarms

Recommendation: Add a scheduled job or alternative verification path that can fail. At minimum, track skip frequency via metrics.


3. Inconsistent Error Handling Between Makefile and CI

File: Makefile (lines 42, 52, 60)

The Makefile suppresses rustup errors with > /dev/null 2>&1:

rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal > /dev/null 2>&1 && \

If this fails, the && chain continues with whatever cargo is available, potentially using stable cargo which silently ignores the feature. The CI version at least warns on failure.

Recommendation: Match CI behavior—surface warnings on toolchain install failure:

@rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal || { echo "WARNING: failed to install $(RESOLVER_TOOLCHAIN)" >&2; exit 0; }

4. Shell Quoting Vulnerability in update-allow Target

File: Makefile (line 49)

cargo +$(RESOLVER_TOOLCHAIN) update -Z min-publish-age -p $(PACKAGE) --precise $(VERSION)

PACKAGE and VERSION are passed unquoted to the shell. While Make variables aren't directly user-facing, this could break with special characters:

# Hypothetical: VERSION="0.4.16 && rm -rf /"
make update-allow PACKAGE=h2 VERSION="0.4.16; echo pwned"

Recommendation: Quote variables:

cargo +$(RESOLVER_TOOLCHAIN) update -Z min-publish-age -p "$(PACKAGE)" --precise "$(VERSION)"

5. Missing --locked in cargo chef cook Context

File: Dockerfile (line 41)

RUN cargo chef cook --profile $BUILD_PROFILE $NO_DEFAULT_FEATURES --features "$FEATURES" $LOCKED --recipe-path recipe.json

The $LOCKED variable defaults to "--locked" (line 38), but this is not the same as the Makefile/CI enforcement. The Dockerfile uses shell variable expansion which:

  • Could be empty if LOCKED is unset (though ENV sets default)
  • Is less explicit than hardcoding --locked

More importantly, cargo chef cook generates a recipe.json from Cargo.lock. If the lockfile is modified between prepare and cook stages, the --locked ensures consistency. This looks correct but verify that cargo chef prepare also preserves lockfile integrity.


6. CI cooldown Job Uses actions/checkout@v6 (Future Version)

File: .github/workflows/ci.yml (line 96)

- uses: actions/checkout@v6

As of early 2025, actions/checkout@v4 is the latest stable release. v6 does not exist yet. This will cause the job to fail immediately.

Recommendation: Use actions/checkout@v4 or verify if this is intentional for a future deployment timeline.


7. Race Condition in make update / make update-allow

File: Makefile (lines 42, 52)

Multiple parallel invocations of make update could race on rustup toolchain install:

rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal > /dev/null 2>&1 && \

rustup is generally safe for concurrent installs, but the output redirection and && chaining could lead to confusing states.

Recommendation: Add .NOTPARALLEL: for these targets or use a lockfile mechanism if this becomes an issue in CI.


8. Documentation Inconsistency: make lint vs make test Lock Behavior

File: CLAUDE.md (lines 93-95), CONTRIBUTING.md (lines 99-105)

The documentation states make lint/make test build --locked, but verify this is actually true after changes. Looking at the diff:

lint: ## 🔍 Run clippy on all workspace crates
	cargo clippy --locked --workspace --all-targets -- -D warnings

test: leanSpec/fixtures ## 🧪 Run all tests
	cargo test --locked --workspace --profile release-fast

This is correct in the Makefile. However, CONTRIBUTING.md says "A plain cargo update or cargo add on stable bypasses the cooldown; make lint/make test and CI build --locked".

Minor issue: The cargo add case isn't fully covered. cargo add on stable will resolve without the cooldown and modify Cargo.toml, but --locked only enforces Cargo.lock, not Cargo.toml manifest changes. A malicious or compromised cargo add could still introduce new dependencies.

Recommendation: Clarify that cargo add should also go through the nightly resolver, or add a make add target:

add: ## ➕ Add a dependency under the publish-age cooldown (PACKAGE=... [FEATURES=...])
	@test -n "$(PACKAGE)" || { echo "usage: make add PACKAGE=<crate> [FEATURES=feat1,feat2]" >&2; exit 1; }
	rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal > /dev/null 2>&1 && \
	cargo +$(RESOLVER_TOOLCHAIN) add $(PACKAGE) $(if $(FEATURES),--features $(FEATURES)) -Z min-publish-age

9. grep Pattern Reliability in Cooldown Check

File: .github/workflows/ci.yml (lines 111, 112)

hits=$(grep -E "Downgrading|is too new" cooldown.txt || true)

This depends on Cargo's exact error message strings, which are not stable API. A future Cargo update could change "is too new" to "published too recently" or similar, breaking detection silently.

Recommendation: Add a comment noting this fragility, and consider pinning the nightly cargo version more strictly (though this conflicts with the future date issue in Point 1).


10. Missing CARGO_NET_OFFLINE or Similar for cargo chef cook

File: Dockerfile (line 41)

The cargo chef cook with --locked should ideally also use --offline in production builds to prevent any network-based resolution surprises. However, this may break if the recipe needs to fetch crates.

Recommendation: Evaluate if CARGO_NET_OFFLINE=true is appropriate for your build environment, or document why network access is required.


Positive Aspects (Acknowledged)

  1. Good documentation: The comments in .cargo/config.toml clearly explain the unstable feature and migration path
  2. Defense in depth: --locked enforcement in CI, Makefile, and Dockerfile creates multiple checkpoints
  3. Escape hatch design: update-allow with explicit PACKAGE/VERSION and warning about full resolution review is well-thought-out
  4. Non-blocking CI job: The cooldown job as annotation-only is appropriate for gradual adoption
  5. SSZ/consensus code untouched: This PR is purely build/tooling infrastructure with no consensus-critical changes

Summary Table

Priority Item File Line
🔴 Critical Future nightly date .github/workflows/ci.yml, Makefile 101, 23
🔴 Critical Non-existent actions/checkout@v6 .github/workflows/ci.yml 96
🟡 Medium Silent CI failures on toolchain install .github/workflows/ci.yml 100-104
🟡 Medium Suppressed errors in Makefile Makefile 42, 52, 60
🟡 Medium Shell quoting in update-allow Makefile 49
🟢 Low Missing make add target CONTRIBUTING.md
🟢 Low Fragile grep patterns .github/workflows/ci.yml 111

The 🔴 Critical items will cause immediate failures and must be fixed before merge.


Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

  1. The cooldown policy does not cover the repo’s second lockfile, so fresh dependencies can still enter through tooling/event-monitor without any of the new checks catching them. tooling/event-monitor is its own workspace with its own committed lockfile at tooling/event-monitor/Cargo.toml, but the new make update, update-allow, and cooldown-check targets only operate at the root Makefile, and the new CI cooldown job only probes the root lockfile at ci.yml. That leaves a supply-chain gap exactly where this PR is trying to tighten controls.

  2. The benchmark smoke step still runs unlocked, so CI can silently re-resolve dependencies there and bypass the lockfile/cooldown enforcement this PR adds elsewhere. See ci.yml: cargo run --profile release-fast --bin ethlambda ... should also pass --locked, otherwise a stale or missing root lockfile entry can still go green in the test job.

No consensus-layer logic changed here, so fork choice, attestation processing, finalization, STF, XMSS, and SSZ correctness were not materially reviewable from this diff. The Dockerfile change to pass $LOCKED into cargo chef cook looks consistent with the stated intent.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

Confirmed the exact lines. Here's my review.

Review: build/min-publish-age-cooldown (PR 607)

Pure tooling/CI change (no Rust code touched), well documented and consistent with the existing $LOCKED pattern already used in the Dockerfile's final cargo build step. Overall solid; one real gap and a couple of minor suggestions below.

Findings

1. Makefile:47cooldown-check fails opaquely if the nightly toolchain install fails, unlike its CI counterpart

cooldown-check: ## 🔎 Warn about lockfile entries younger than the publish-age cooldown
	@rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal > /dev/null 2>&1 && \
	if ! out=$$(cargo +$(RESOLVER_TOOLCHAIN) update --dry-run -Z min-publish-age 2>&1); then \

rustup toolchain install ... && if ! out=$(...) means a failed rustup install short-circuits the &&, so the if ! out=...; then ... exit 0; fi fallback (which is what makes this "advisory only") never runs. The recipe line's exit status becomes rustup's non-zero status, and since the line is @-prefixed, make cooldown-check fails with a bare Error 1 and zero diagnostic output — both rustup's stdout and stderr were redirected to /dev/null.

Compare with the CI job (.github/workflows/ci.yml), which explicitly guards this exact failure mode:

if ! rustup toolchain install nightly-2026-06-21 --profile minimal; then
  echo "::warning title=Publish-age cooldown check skipped::toolchain install failed"
  exit 0
fi

The Makefile's update (line 31) and update-allow (line 40) targets have the same silent-swallow issue, though for those a hard failure is arguably intentional (they're not meant to be "advisory"). For cooldown-check specifically — meant to be a soft, non-blocking probe — this inconsistency means a contributor on an offline machine or behind a proxy that blocks rustup gets a confusing failure instead of the intended warning. Suggest mirroring the CI guard:

cooldown-check:
	@if ! rustup toolchain install $(RESOLVER_TOOLCHAIN) --profile minimal >/dev/null 2>&1; then \
		echo "WARNING: publish-age cooldown check skipped: toolchain install failed"; exit 0; \
	fi; \
	if ! out=$$(cargo +$(RESOLVER_TOOLCHAIN) update --dry-run -Z min-publish-age 2>&1); then \
	...

Minor / non-blocking

  • .github/workflows/ci.yml cooldown job — runs unconditionally on every CI invocation (installs a whole extra nightly toolchain each time), even for changes that don't touch Cargo.toml/Cargo.lock. Given it's purely advisory, gating it on lockfile/manifest changes (or running it on a schedule instead) would save CI minutes without losing signal. Not required, just an efficiency consideration.
  • Per the PR description, this job is guaranteed to emit a probe failed warning on every run until 2026-09-10 (known rand/chacha20 yanked-version conflict). That's already disclosed and self-resolving, not a defect, but worth flagging to reviewers so nobody mistakes the guaranteed warning for a new regression once this lands.
  • .cargo/config.toml's [unstable]/[registry] tables apply to every contributor's cargo invocation in this checkout. The PR claims stable 1.97.1 silently ignores them (verified per the checklist) — worth a quick sanity check on CI's actual pinned stable version too, since a future stable bump that starts warning/erroring on unrecognized [unstable] keys would be a silent regression here.

Looks good

  • Reusing the existing ARG LOCKED idiom for the cargo chef cook step (Dockerfile:41) is the right call — keeps the shadow-build's unlocked path consistent between the chef cook and final build steps.
  • make lint/make test going --locked correctly closes the gap where a manifest drift could silently re-resolve on stable and bypass the cooldown.
  • Docs (CLAUDE.md, CONTRIBUTING.md) clearly explain the escape hatch and its blast radius (whole-resolution bypass, review the lockfile diff).

Automated review by Claude (Anthropic) · sonnet · custom prompt

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment on lines +90 to +92
# Stable cargo ignores the publish-age cooldown in .cargo/config.toml, so the
# lockfile can pin too-young crates (or deliberately via `make update-allow`).
# Surface them as annotations; this job never fails the build.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If this doesn't fail the build it's useless

Suggested change
# Stable cargo ignores the publish-age cooldown in .cargo/config.toml, so the
# lockfile can pin too-young crates (or deliberately via `make update-allow`).
# Surface them as annotations; this job never fails the build.
# Stable cargo ignores the publish-age cooldown in .cargo/config.toml, so the
# lockfile can pin too-young crates (or deliberately via `make update-allow`).
# Surface them as annotations and fail the build.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done: the job now exits 1 on a too-young pin in either lockfile, and I narrowed the grep to Downgrading .*published because bare downgrades for MSRV or tightened requirements show up too (event-monitor has three today). Infrastructure failures still only warn, so the root probe, which currently fails on yanked chacha20 releases upstream, doesn't block every PR; it starts gating on 2026-09-10 when chacha20 0.10.2 ages out. One consequence: an allow-bypassed bump keeps CI red until it ages, which I think is the right trade but wanted to flag.

Comment thread Makefile Outdated
Comment thread Makefile Outdated
…-allow target

Review follow-up. An advisory annotation nobody has to act on does not protect
anything, so the CI cooldown job now exits non-zero when a lockfile pins a crate
younger than the window, and covers tooling/event-monitor's lockfile as well as
the root one. Toolchain download failures and resolutions that fail for reasons
unrelated to age (today: yanked chacha20 releases upstream) still only warn,
since they are outside the PR's control and would block every PR.

The grep is narrowed to `Downgrading .*published`: a cooldown-driven downgrade
carries the too-young version's publish date, while downgrades for other reasons
(MSRV, a tightened requirement; the event-monitor lockfile shows several) do
not, and a bare `Downgrading` match would have failed the build on those.

`make update-allow` is removed; the escape hatch is the plain cargo env var on
the existing target, `CARGO_RESOLVER_INCOMPATIBLE_PUBLISH_AGE=allow make update
UPDATE_ARGS="-p <crate> --precise <ver>"`, and the docs say the cooldown job
stays red until that version ages past the window. Two explanatory comments
above `make lint` and the CI `cargo check` step are dropped, the benchmark
smoke step builds `--locked` like everything else, and rustup's stderr is no
longer swallowed so a failed toolchain install is visible.
Comment thread CLAUDE.md Outdated
Review follow-up. With the CI cooldown job failing the build on a too-young
pin, a bypass that resolves past the window only produces a lockfile CI will
reject, so there is nothing to document: the env var is removed from
CLAUDE.md, CONTRIBUTING.md, the Makefile comment and the CI job comment.
@MegaRedHand
MegaRedHand added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 0b85d47 Sep 4, 2026
4 checks passed
@MegaRedHand
MegaRedHand deleted the build/min-publish-age-cooldown branch September 4, 2026 21:23
Sign up for free to 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.

2 participants