Fix/nightly bench sysroot

[jotabulacios]

This PR fixes the nightly ethrex bench (c-kzg couldn't find stdlib.h). prepare-sysroot now detects a partial sysroot (checks include/stdlib.h) and the .elf rules depend on it, so the guest ELF rebuild always provisions first. The bench now uses a user-writable $HOME/.lambda-vm-sysroot (no sudo, no /opt), and a new step fails the job when the bench fails

[github-actions]

Codex Code Review

Findings

• High Security: Makefile and Makefile now run rm -rf "$(SYSROOT_DIR)", including with sudo in the fallback path. Since SYSROOT_DIR is user/environment overrideable, a typo or hostile value like /usr, /opt, $HOME, or another important directory could be recursively deleted before extraction. Add a strict guard before any removal, e.g. reject empty/root/top-level paths and only allow known managed paths such as /opt/lambda-vm-sysroot or $HOME/.lambda-vm-sysroot, or extract to a temporary directory and replace only after validating the target is the managed sysroot path.

No other substantive issues found in the PR diff.

[claude]

Review

TL;DR: Solid, well-reasoned infrastructure fix. No critical/high/medium issues. One low-priority nit below.

What the PR does

• Fixes a broken nightly bench: the RISC-V sysroot at /opt/lambda-vm-sysroot on the self-hosted runner was root-owned and partially provisioned (missing libc headers), so c-kzg couldn't find stdlib.h and the ELF rebuild failed silently under continue-on-error.
• Three interlocking fixes: (1) better sysroot completeness check (stdlib.h file instead of include/ dir), (2) self-healing re-extract that wipes and re-provisions a partial sysroot, (3) export SYSROOT_DIR="$HOME/.lambda-vm-sysroot" in the workflow step so no sudo is needed.
• Restores CI red state by adding id: ethrex_bench + an explicit fail step at the end (correct: uses outcome, not conclusion, which is the right field when continue-on-error: true is set).
• Order-only | prepare-sysroot prerequisite on both .elf pattern rules is correctly used — .PHONY targets as normal prereqs would force a rebuild every time; order-only avoids that while still guaranteeing the sysroot is provisioned first.

Low: curl without --fail (pre-existing, not introduced here)

curl -L "$(SYSROOT_URL)" -o "$(SYSROOT_TARBALL)";

If the server returns a 4xx/5xx, curl exits 0 and writes the error body to the tarball. tar then fails with a confusing error like "not in gzip format" rather than a clear HTTP error. Worth adding --fail-with-body when touching this area next time — not a blocker for this PR.

Everything else looks correct

• rm -rf "$(SYSROOT_DIR)" && mkdir -p "$(SYSROOT_DIR)" in the writable path and sudo rm -rf ... in the sudo path: correct self-healing sequence.
• Error handlers on both paths call exit 1, so rm "$(SYSROOT_TARBALL)" at line 99 is only reached on success and the file always exists there.
• if: always() && steps.ethrex_bench.outcome == 'failure' correctly re-surfaces the failure after continue-on-error kept the job running through artifact upload and Slack notification.

LGTM.

[github-actions]

Codex Code Review

Findings

• High - destructive rm -rf on caller-controlled SYSROOT_DIR: Makefile and Makefile now remove $(SYSROOT_DIR) before re-extracting. Since SYSROOT_DIR is explicitly overrideable from the environment/CLI, a typo or injected value like SYSROOT_DIR=/opt would run sudo rm -rf /opt on the fallback path. Add a hard guard before any deletion, e.g. reject empty/root/broad paths and require a dedicated basename such as lambda-vm-sysroot or .lambda-vm-sysroot, then delete only after that validation.

No Rust/VM/crypto issues were present in this diff.

[claude]

Review: Fix/nightly bench sysroot

The fix is correct and well-reasoned. The three-part approach (partial-sysroot detection in prepare-sysroot, | prepare-sysroot order-only dependency on .elf rules, and SYSROOT_DIR override in the workflow) properly addresses the root cause.

Issues

[Low – Security] No integrity check on the downloaded sysroot tarball

The sysroot tarball is fetched with a bare curl -L and extracted directly into the build toolchain. There is no SHA256 checksum verification. The sysroot contains C headers and libraries compiled into guest programs; a compromised server or MITM could inject malicious content silently.

Consider pinning a checksum in the Makefile, e.g.:

SYSROOT_SHA256 := <expected-hash>
...
curl -L "$(SYSROOT_URL)" -o "$(SYSROOT_TARBALL)"
echo "$(SYSROOT_SHA256)  $(SYSROOT_TARBALL)" | sha256sum -c - || { rm -f "$(SYSROOT_TARBALL)"; exit 1; }

---

[Low – Docs] run_ethrex.sh prerequisite comment is now stale

Line 13 of bench_vs/run_ethrex.sh still reads:

Sysroot present at /opt/lambda-vm-sysroot (run make prepare-sysroot first)

With this PR the .elf pattern rules carry | prepare-sysroot as an order-only dependency, so the sysroot is auto-provisioned at whatever SYSROOT_DIR is set to; manual pre-provisioning is no longer required.

---

Everything else looks good

• The writable-directory check → rm -rf → re-extract flow is correct and the error-handler (|| { rm -rf ...; exit 1; }) cleans up on failure.
• SYSROOT_TARBALL lives at /tmp/, so the rm -rf "$(SYSROOT_DIR)" can't accidentally delete it.
• Order-only | prepare-sysroot is the right choice for a .PHONY target — prevents spurious .elf rebuilds while still running the idempotent recipe each time.
• The continue-on-error: true + explicit fail-step pattern in the workflow is the correct GHA idiom and is well-commented.

[jotabulacios]

/claude codex

[claude]

Code Review

Summary: Fixes nightly CI sysroot provisioning — c-kzg couldn't find stdlib.h because the runner had a partial sysroot (directories present, headers missing). The fix: detect partial sysroots via include/stdlib.h presence, re-provision from scratch when incomplete, use a user-writable $HOME/.lambda-vm-sysroot to avoid sudo, and wire prepare-sysroot as an order-only prereq on each .elf pattern rule so direct invocations don't skip it.

The changes are clean and targeted. A few observations:

---

Medium — No tarball integrity check

The downloaded tarball (SYSROOT_TARBALL) is extracted and its headers are passed directly to the compiler via --sysroot. There is no SHA256/checksum verification, so a MitM or compromised CDN can silently inject arbitrary headers/libs into every guest ELF build. Pre-existing, but the PR touches this line and worth addressing.

Consider adding a pinned hash:

SYSROOT_SHA256 := <expected-hash>
# After curl:
echo "$(SYSROOT_SHA256)  $(SYSROOT_TARBALL)" | sha256sum -c - || { rm -f "$(SYSROOT_TARBALL)"; exit 1; }

---

Low — Basename guard intent vs. actual protection

The case basename ... guard correctly prevents SYSROOT_DIR=/opt from being wiped with rm -rf. However, a path like /some/dangerous/path/lambda-vm-sysroot also passes (basename matches). This is fine — the guard is accident-prevention, not a security boundary — but the comment could clarify that to avoid a future reviewer thinking it provides stronger guarantees.

---

Correct patterns worth confirming

• Order-only prereq (| prepare-sysroot): correct choice since prepare-sysroot is .PHONY — a normal prereq would force rebuilds every time; order-only runs the recipe once and the idempotent guard short-circuits immediately.
• continue-on-error + re-fail step: standard GitHub Actions idiom for letting artifact upload and Slack notification run after a failing step while still failing the job. Correct.
• curl -fL: adding -f makes curl exit non-zero on HTTP errors instead of silently downloading an error page. Good improvement.
• Tarball cleanup on success: rm "$(SYSROOT_TARBALL)" at the end of the target is preserved; each error-path handler already cleans up before exit 1, so there's no double-rm risk.

---

Overall: solid, targeted fix. The SHA256 check is the only meaningful hardening gap. Everything else is correct.