Build release binaries with cargo auditable to embed dependency data - #937

Claude finished @shashial's task in 3m 37s —— View job

Review: cargo auditable release builds

Reviewed the diff against origin/develop, plus Makefile, both Dockerfiles, release.yaml, and the surrounding build paths. The approach is sound; findings below are about the guarantees the PR claims versus what the code enforces. Five inline comments posted.

Blocking-ish

1. make audit-bin passes vacuously — Makefile:163-172. The target doesn't depend on build, and an unmatched glob leaves the literal target/.../*, so -f fails, the loop never runs, and it exits 0. Running it on a fresh checkout, before make build, or with a BUILD_PROFILE/BUILD_TARGET different from the one built, reports success having scanned nothing. Suggested a count guard inline. (Fix this →)

2. The 0.7.5 pin isn't actually enforced — Makefile:74-77. command -v cargo-auditable >/dev/null || cargo install ...@0.7.5 skips the install if any version is already on PATH. The PR's stated goal ("pinned to 0.7.5 in all build paths so every artifact embeds identically formatted data") doesn't hold for the make path on a machine or runner that already has 0.6.x. Gate on version instead. Same applies to install-cargo-audit. (Fix this →)

Worth addressing

3. No verification that .dep-v0 is actually present in any artifact. This is the structural gap behind 1 and 2: nothing in CI fails if a binary ships without embedded data. The Dockerfile.rbuilder path is the most exposed (docker/Dockerfile.rbuilder:76) — it inherits RUSTC_WRAPPER=sccache from the base stage while cargo-auditable injects via RUSTC_WORKSPACE_WRAPPER, and cargo chef cook (line 66) builds deps unwrapped against a persistent sccache mount. That chain is supposed to work, but if it doesn't, the result is a released image with no audit data and a green build. A cargo audit bin / readelf -S | grep dep-v0 assertion after each build turns a silent regression into a hard failure.

4. Reproducible builds not re-verified — docker/Dockerfile.reproducible:13. This image backs the Linux release artifacts and the repo works hard for bit-identical output (-C metadata='', --build-id=none, --remap-path-prefix, SOURCE_DATE_EPOCH). cargo-auditable links a generated object produced in a temp dir; worth an actual two-run hash comparison rather than trusting the "sorted and timestamp-free" claim in the new Makefile comment. Secondary: everything above that line is deliberately pinned to a Debian snapshot repo, and cargo install from crates.io is now the one un-snapshotted network fetch in that builder.

5. Version pinned in four independent places — Makefile:77, docker/Dockerfile.rbuilder:39, docker/Dockerfile.reproducible:13, .github/workflows/release.yaml:151. Given the goal is identical embedded data across paths, silent drift between these is the exact failure the PR is trying to prevent. At minimum a Makefile variable plus Docker ARGs.

6. README sections now conflict — the "Reproducible builds" section (lines 157–176) still instructs plain cargo build --release with a fixed sha256sum, which produces a binary without audit data and a hash matching nothing make build emits.

Minor

• docker/Dockerfile.rbuilder-operator:16 builds via make build-reproducible, which isn't a target in the Makefile — so that path is already broken and isn't covered by this PR. Pre-existing, flagging only because it's another release-ish build path that won't get audit data.
• cargo install cargo-auditable --locked compiles from source with no registry cache mount in both Dockerfiles and on every Mac release run; cargo-binstall or taiki-e/install-action would cut that.
• Dangling comment at Makefile:74 reads as a claim about the installer rather than about the build.

Not run: make lint / make test — Bash is restricted in this environment, so I reviewed statically. The shell-semantics claim in finding 1 (unmatched glob → exit 0) is standard POSIX behavior and doesn't depend on execution.
· branch cargo-auditable-builds

Claude finished @shashial's task in 4m 23s —— View job

Re-review: cargo auditable release builds

• Read full diff vs origin/develop
• Verify prior findings
• Review the clippy-lint commit
• Post review (4 inline comments)

Prior round's findings are mostly addressed: audit-bin now fails on zero binaries (Makefile:169-183), both installers gate on version via cargo install --list instead of command -v, .dep-v0 is asserted after every Docker and Mac build, --locked was added to the Mac build, and the README reproducible/auditable sections were reconciled. Remaining items below.

Worth addressing

1. Linux release artifacts are only partially verified — docker/Dockerfile.reproducible:17-20. The loop checks rbuilder-operator, rbuilder-rebalancer, bid-scraper, but this image produces all six binaries that release.yaml renames and uploads (release.yaml:181-231). rbuilder, reth-rbuilder, and tbv-bidding-service can ship without embedded data and the build stays green — and rbuilder is the artifact most consumers will scan. Inline suggestion posted.

2. Reproducibility still isn't verified for this change. reproducible-build-test (.github/workflows/reprotest.yml) runs only on schedule (cron, every 2 days) and workflow_dispatch — not on PRs. So nothing in this PR's CI confirms that linking cargo-auditable's generated object still yields bit-identical output under -C metadata='' / --build-id=none / --remap-path-prefix. It's cheap to settle: dispatch that workflow on this branch before merging. Note it also only compares rbuilder-operator and rbuilder-rebalancer, so it won't cover the other four either.

Related: cargo install cargo-auditable@0.7.5 --locked (Dockerfile.reproducible:13) is now the one crates.io fetch in a builder whose apt sources are deliberately pinned to a Debian snapshot. Pinned by version + lockfile, so it should resolve identically, but the reproducible image's inputs no longer come from a single snapshot. Worth a conscious call rather than an accident.

3. Version pinned in four places — Makefile:75, docker/Dockerfile.rbuilder:39, docker/Dockerfile.reproducible:13, release.yaml:151. The Makefile variable helps, but the three hardcoded copies are where drift will actually happen, and drift is the exact failure this PR is trying to prevent. ARG CARGO_AUDITABLE_VERSION=0.7.5 in the Dockerfiles at minimum.

4. Mac verify step can't be exercised before a release — release.yaml:172-177. Only runs on tag push / manual dispatch, and greps otool -l output for a section name. Should work (__DATA,.dep-v0), but a failure here means a broken release rather than a missed check. One workflow_dispatch run with build-mac: true / draft-release: false would confirm it.

Rust changes (clippy commit)

5. Over-broad allow — crates/rbuilder-operator/src/bidding_service_wrapper/mod.rs:1. The inner #![allow(clippy::result_large_err)] silences the lint for client, server, and any future submodule, not just the generated bidding_service. The bloxroute_grpc.rs:16 change in the same commit scopes it correctly to the generated module — do the same here. Inline suggestion posted.

as_chunks::<N>().0 (crates/eth-sparse-mpt/src/utils.rs:129) is behaviour-preserving — same min(len) zip, same N-multiple offset, and the only instantiation is N = 8, so no zero-N panic path. Fine. The #[allow] on bid-scraper's connect (best_bid_ws_connector.rs:88) papers over a large tungstenite::Error; boxing would be the real fix, but it's a hand-written signature and not worth blocking on.

Minor

• audit-bin doesn't distinguish binaries with .dep-v0 from ones without: cargo audit bin falls back to guessing versions from panic strings and reports low-confidence results without failing, so a stale non-auditable artifact left in target/ mixes silently into the output. Given the target now guarantees it scanned something, guaranteeing it scanned something auditable is the natural follow-up.
• docker/Dockerfile.rbuilder-operator:16 still builds via make build-reproducible, which is not a target in the Makefile. Pre-existing and out of scope, but it's another release-ish path that won't get audit data.
• Makefile:75 — the comment # The cargo-auditable embedded data is sorted and timestamp-free now sits above CARGO_AUDITABLE_VERSION, where it reads as a claim about the version variable. It belongs next to build or in the reproducibility docs.
• grep -q '^cargo-auditable v0.7.5:' — the .s are regex wildcards. Harmless in practice; grep -qF or escaping is tidier.

Not run: make lint / make test — Bash is restricted here to read-only inspection, so this is a static review. The Makefile shell-semantics and Docker/workflow claims above are from reading the files, not from executing them.
· branch cargo-auditable-builds

Claude finished @shashial's task in 4m 45s —— View job

Re-review: cargo auditable release builds

• Read full diff vs origin/develop
• Verify prior findings
• Review the single-source cargo-auditable versions commit
• Post review (4 inline comments)

Round-2 findings are addressed: .dep-v0 is now asserted for all six binaries in Dockerfile.reproducible:18-21, the version is single-sourced through .tool-versions, and bidding_service_wrapper/mod.rs scopes result_large_err per-module instead of at module-tree level. Remaining items below.

Worth addressing

1. .tool-versions collides with asdf/mise — new in this commit. That filename is asdf's config file and mise reads it too, so it now applies to every contributor with either installed. There's no asdf plugin named cargo-auditable, so asdf install in the repo fails outright, and mise will try to resolve both entries through its own registry rather than cargo install. It also reads as a second, partial toolchain source next to rust-toolchain.toml (no rust entry). Nothing here is actually asdf-managed — all four consumers awk the file by hand — so a neutral name (build-tools.txt) sidesteps it. Inline comment has the five call sites. (Fix this →)

2. Empty version from awk fails confusingly — Makefile:75, 162, plus the same unguarded pattern in both Dockerfiles and release.yaml:152. If the file is missing or the key is renamed, $(shell awk ...) returns empty silently; the guard greps for ^cargo-auditable v: (no match) and you get cargo install cargo-auditable@ --locked, which errors pointing at cargo rather than at the missing file. Now that the version is load-bearing from exactly one place, asserting the read succeeded is worth four lines. Suggestion inline, along with the unescaped . in the grep pattern.

3. audit-bin is the one path that doesn't assert .dep-v0 — Makefile:169-183. Both Dockerfiles and the Mac job check the section; this doesn't. cargo audit bin on a binary without embedded data doesn't fail — it guesses versions from panic strings and reports low confidence, silently mixed in with real results. Stale artifacts from a pre-cargo-auditable build stay in target/x86_64-unknown-linux-gnu/reproducible/ since nothing cleans it. Given the target now guarantees it scanned something, guaranteeing it scanned something auditable closes the loop. Suggestion inline (note readelf is Linux-only; cargo audit bin --deny warnings is the portable version).

4. Reproducibility still isn't verified for this change — carried from round 2, still the highest-stakes open item. reproducible-build-test (.github/workflows/reprotest.yml:3-6) runs on schedule and workflow_dispatch only, not on PRs, so nothing here confirms that linking cargo-auditable's generated object still yields bit-identical output under -C metadata='' / --build-id=none / --remap-path-prefix. The risk is lower than it looked — profile.reproducible inherits release, so there's no debuginfo to carry the generated object's temp path — but the README now asserts bit-identical output, and one workflow_dispatch run on this branch settles it. It also only diffs rbuilder-operator and rbuilder-rebalancer, so it won't cover the other four.

5. README overstates the release build — README.md:158-161. "sets SOURCE_DATE_EPOCH from the last commit timestamp" holds for a local make build (Makefile:52), but docker/Dockerfile.reproducible:19 overrides it with SOURCE_DATE=1730000000. Following the README on a release commit produces a self-consistent hash that won't match the published artifact — the same mismatch the old section had, relocated rather than removed.

Minor

• cargo install cargo-auditable --locked compiles from source with no registry cache mount in either Dockerfile and on every Mac release run. cargo-binstall / taiki-e/install-action would cut a couple of minutes per cold build.
• docker/Dockerfile.reproducible:14 is still the only crates.io fetch in a builder whose apt sources are deliberately pinned to a Debian snapshot. Pinned by version + --locked, so it should resolve identically — flagging so it's a conscious call.
• Mac verify (release.yaml:174-179) only executes during an actual release, so a failure there breaks a release rather than a PR. cargo audit bin would be name-agnostic and double as the scan.
• docker/Dockerfile.rbuilder-operator:16 still builds via make build-reproducible, which is not a target in the Makefile. Pre-existing, out of scope, but it's another release-ish path with no audit data.
• Makefile:74 — the comment # The cargo-auditable embedded data is sorted and timestamp-free still sits above CARGO_AUDITABLE_VERSION, where it reads as a claim about the variable rather than about the build.

Checked and fine: tbv-bidding-service does exist (crates/rbuilder-operator/src/bin/tbv-bidding-service.rs, auto-discovered), so the unguarded six-binary loop in Dockerfile.reproducible won't spuriously fail. install-cargo-audit's ^cargo-audit v pattern doesn't false-match the cargo-auditable line. readelf is present in both rust:*-bookworm and rust:*-trixie.

Not run: make lint / make test — Bash here is restricted to read-only inspection, so this is a static review. The Makefile shell semantics, Docker, and workflow claims come from reading the files, not executing them.
· branch cargo-auditable-builds

Claude finished @shashial's task in 4m 54s —— View job

Re-review: cargo auditable release builds

• Read full diff vs origin/develop
• Verify prior round-3 findings
• Review the fix review commit
• Post review (4 inline comments)

Round-3 findings are addressed: .tool-versions is gone (replaced by the neutral .cargo-auditable-version), the empty-version case is now guarded in all four consumers, the grep uses -qxF, and the README no longer contradicts itself on which command produces the released artifact.

Two things changed in fix review that weren't asked for and go the wrong way:

Worth addressing

1. Mac .dep-v0 verification was removed — release.yaml:167. The Verify audit data embedded (Mac) step added in the previous commit is gone, so the darwin release artifacts are now the only build path in this PR without the assertion (Dockerfile.rbuilder:80 and Dockerfile.reproducible:20 both have it). That's the wrong path to drop it from: the two Docker checks cover ELF, and Mach-O is the one object format nothing here exercises. Inline comment has the re-add plus the cargo audit bin alternative if the concern was that it can only fail during a real release. (Fix this →)

2. make audit-bin and install-cargo-audit were deleted. The README was updated consistently, so this looks deliberate — but the PR description still lists "New make audit-bin target scans built binaries against the RustSec advisory database" as a bullet, and the repo now has no way to actually consume the data it embeds. If the removal was intentional, update the PR body; if it was collateral from the .tool-versions rename, it needs to come back (the hardened version from the last round). Leftover: BUILD_OUTPUT_DIR (Makefile:120) now exists only to dedupe three deb paths — harmless, just noting it came from that target.

3. Version guard checks the wrong thing — Makefile:82. cargo install --list reports $CARGO_HOME/.crates2.json; cargo auditable dispatches to the first cargo-auditable on PATH. An externally-installed copy earlier in PATH isn't listed, so the guard installs 0.7.5 into ~/.cargo/bin and the build uses the other one anyway. cargo-auditable --version reflects what will actually run. Also Makefile:75 — the cat path is cwd-relative and the $(error) is parse-time, so make -f /abs/path/Makefile help from another directory hard-fails every target. Both inline.

4. Reproducibility still isn't verified for this change — carried from rounds 2 and 3. reprotest.yml:3-6 runs on schedule/workflow_dispatch only, never on PRs, so nothing confirms that linking cargo-auditable's generated object stays bit-identical under -C metadata='' / --build-id=none / --remap-path-prefix. The risk is genuinely low — profile.reproducible inherits release (Cargo.toml:65-70) with no debug override, so there's no debuginfo to carry the generated object's temp path — but the README now asserts bit-identical output and one workflow_dispatch run on this branch settles it. It also only diffs rbuilder-operator and rbuilder-rebalancer.

5. README reproducible section still contradicts the release image — README.md:159-160. "sets SOURCE_DATE_EPOCH (last commit timestamp)" is true for a local make build, but Dockerfile.reproducible:19 pins SOURCE_DATE=1730000000 for the published Linux artifacts. Same trap as the old section, relocated. Carried from round 3, inline suggestion posted.

Minor

• Dockerfile.rbuilder:79 — the .dep-v0 check resolves last round's correctness concern, but RUSTC_WRAPPER=sccache + cargo-auditable's RUSTC_WORKSPACE_WRAPPER now means workspace crates compile through chained wrappers. Deps from cargo chef cook are unaffected; worth confirming the workspace hit rate off one build log. Flagged as unverified.
• Dockerfile.rbuilder:39 / Dockerfile.reproducible:13 leave /tmp/cargo-auditable-version in the image; a bind mount avoids the layer.
• cargo install cargo-auditable --locked still compiles from source with no registry cache mount in both Dockerfiles and on every Mac release run, and is still the only crates.io fetch in a builder whose apt sources are pinned to a Debian snapshot.
• Makefile:74 — the comment # The cargo-auditable embedded data is sorted and timestamp-free still sits above CARGO_AUDITABLE_VERSION, reading as a claim about the variable.
• docker/Dockerfile.rbuilder-operator:16 still builds via make build-reproducible, not a target in the Makefile. Pre-existing, out of scope.

Rust changes

bidding_service_wrapper/mod.rs is fixed — per-module #[allow] instead of the module-tree #![allow]. The client one still covers a whole subdirectory rather than the two Result-returning fns in bidding_service_client_adapter.rs, but that's a much smaller blast radius and not worth another round.

Checked and fine: .dockerignore doesn't exclude .cargo-auditable-version, so both COPYs resolve. grep -qxF 'cargo-auditable v0.7.5:' matches cargo's registry-install line format exactly and won't false-match. The six-binary loop in Dockerfile.reproducible is unguarded, so a missing binary fails loudly — intended.

Not run: make lint / make test — Bash is restricted to read-only inspection here, so this is a static review. The Makefile, Docker, and workflow claims come from reading the files, not executing them.
· branch cargo-auditable-builds

Claude finished @shashial's task in 2m 41s —— View job

Re-review: cargo auditable release builds

• Read full diff vs origin/develop
• Verify prior round-4 findings
• Review the new Update Makefile commit
• Post review (3 inline comments)

Update Makefile (05afbfd) fixes the cwd-relative version read — $(dir $(firstword $(MAKEFILE_LIST))) resolves against the Makefile, so make -f /abs/path/Makefile <target> from another directory no longer hard-fails every target, and 2>/dev/null suppresses the raw cat: error. That's the whole commit; everything else is carried forward.

Correction to my last round

The cargo install --list guard is fine — Makefile:82. I said an externally-installed cargo-auditable earlier in PATH would shadow the pinned one. That's backwards: cargo's subcommand lookup searches $CARGO_HOME/bin before PATH, so what --list reports and what cargo auditable dispatches to agree. Withdrawing that finding — no change needed.

Still open

1. Mac .dep-v0 verification is still absent — release.yaml:167. Darwin remains the only build path in this PR without the assertion (Dockerfile.rbuilder:80 and Dockerfile.reproducible:20-23 both check). It's the path where the check is worth the most: the Docker checks cover ELF, and Mach-O is the one object format nothing here exercises. Inline comment has the re-add and the cargo audit bin alternative. (Fix this →)

2. PR description is now stale. make audit-bin and install-cargo-audit are gone from the Makefile and the README was updated to match, so the removal reads as deliberate — but the PR body still lists "New make audit-bin target scans built binaries against the RustSec advisory database" as a bullet. Either drop that bullet or restore the target; as it stands the repo embeds the data with no first-party way to consume it. (BUILD_OUTPUT_DIR, Makefile:120, now exists only to dedupe three deb paths — harmless leftover.)

3. Reproducibility still isn't verified for this change — rounds 2, 3, 4, now 5. reprotest.yml:3-6 runs on schedule/workflow_dispatch only, never on PRs, so nothing confirms that linking cargo-auditable's generated object stays bit-identical under -C metadata='' / --build-id=none / --remap-path-prefix. The risk is low — profile.reproducible inherits release with no debug override, so there's no debuginfo to carry the generated object's temp path — but README.md:172 now asserts "the two hashes are identical", and one workflow_dispatch run on this branch settles it. It also only diffs rbuilder-operator and rbuilder-rebalancer, so it won't cover the other four.

4. README reproducible section still contradicts the release image — README.md:158-159. "sets SOURCE_DATE_EPOCH (last commit timestamp)" holds for a local make build, but Dockerfile.reproducible:19 pins SOURCE_DATE=1730000000 for the published Linux artifacts, so the documented procedure gives two self-consistent hashes that match each other and not the release. Suggestion inline. Also: SOURCE_DATE comes from git log -1, so in a shallow/tarball checkout it expands to empty and SOURCE_DATE_EPOCH= is silently unset.

Minor

• Makefile:75 — $(dir ...) already ends in /, so the extra literal slash yields .//.cargo-auditable-version. Harmless.
• Makefile:74 — the comment # The cargo-auditable embedded data is sorted and timestamp-free still sits above CARGO_AUDITABLE_VERSION, reading as a claim about the variable.
• Dockerfile.rbuilder:81 — the .dep-v0 gate now applies to the test-relay stage too, so a local make docker-image-test-relay fails hard for a release-only reason.
• Dockerfile.rbuilder:39 / Dockerfile.reproducible:13 leave /tmp/cargo-auditable-version in the image; --mount=type=bind avoids the layer.
• cargo install cargo-auditable --locked still compiles from source with no registry cache mount in both Dockerfiles and on every Mac release run, and is still the only crates.io fetch in a builder whose apt sources are pinned to a Debian snapshot.
• Dockerfile.rbuilder:79 — RUSTC_WRAPPER=sccache plus cargo-auditable's RUSTC_WORKSPACE_WRAPPER means workspace crates compile through chained wrappers. Deps from cargo chef cook are unaffected; worth reading sccache --show-stats off one build log. Unverified.
• docker/Dockerfile.rbuilder-operator:16 still builds via make build-reproducible, not a target in the Makefile. Pre-existing, out of scope.

Checked and fine

$(dir $(firstword $(MAKEFILE_LIST))) is correct — no include precedes line 75, so it's the top-level Makefile. grep -qxF 'cargo-auditable v0.7.5:' matches cargo's registry-install line exactly. The six-binary loop in Dockerfile.reproducible is unguarded, so a missing binary fails loudly. RBUILDER_BIN defaults to rbuilder, where package name and binary name coincide, so the readelf path resolves. as_chunks::<8>().0 and the per-module #[allow(clippy::result_large_err)] scoping are unchanged and fine.

Not run: make lint / make test — Bash here is restricted to read-only inspection, so this is a static review. The Makefile, Docker, and workflow claims come from reading the files, not executing them.
· branch cargo-auditable-builds