fix(modules): make bin-patch skip runnable binaries, prefer patchelf 🐛

[dohernandez]

Summary

Field report from the pre-clarke validator launch (2026-07-10): three distinct failures in post-install's patch_executable(), all stemming from LIEF's full-file rewrite of the ~291MB executor just to set an RPATH:

1. binary.write() needs a working set several times the binary size — std::bad_alloc on a box with 18GB free.
2. Re-runs after any partial failure wedge: LIEF cannot re-parse an ELF it wrote itself.
3. The rewritten ELF has corrupt section headers — it runs (the loader only uses program headers), but readelf errors out and patchelf refuses the file, so downstream tooling (stripping, signing, scanners) chokes.

Fix

Patching only ever sets interpreter/rpath, so ask the ground-truth question first: when the target platform matches the host, exec <binary> --version (60s timeout) before doing anything. Exit 0 means the loader resolved the interpreter and every needed library — skip patching entirely. This fixes all three issues for standard (FHS) deploys, where the shipped tree's interpreter resolves and $ORIGIN/../lib RUNPATH already works, and makes re-runs idempotent: a binary patched by a previous run executes and is skipped, so LIEF never re-parses its own output. (A static "is $ORIGIN/../lib present" check — the field report's suggestion — would not have sufficed: the code sets two rpath dirs and $ORIGIN/../lib covers only one, so dir-coverage would still "need" patching on every FHS box.)

When patching is genuinely needed (e.g. NixOS-style hosts where the interpreter doesn't resolve):

• Prefer patchelf when the file is ELF and the CLI is on PATH: --print-interpreter, --set-interpreter only if the current one doesn't resolve (same rule as before), and --force-rpath --set-rpath (DT_RPATH, parity with what the LIEF path writes). In-place edit — no memory multiplier, output stays valid for section-parsing tools.
• LIEF demoted to last resort: its import is deferred to the actual rewrite path (it no longer loads at all when patching is skipped or patchelf handles it — also easing the NixOS libstdc++.so.6 venv quirk), and a parse failure now raises with a recovery hint instead of the previous silent return that shipped a possibly-unpatched binary.

Verification

E2E against the real script on three fake bundle trees:

• runnable stand-in binaries at host platform → both bin/genvm-modules and executor/<v>/bin/genvm skipped, exit 0, and LIEF provably never imported (the test python has no lief installed);
• non-runnable non-ELF, no patchelf → falls through to the LIEF import in the expected order;
• fake ELFs, cross-target (host-exec gate bypassed as designed) with a logging patchelf stub on PATH → argv exactly --print-interpreter then --force-rpath --set-rpath <root>/lib[:<exec>/lib] per binary, no --set-interpreter (stub interpreter resolves), no LIEF.

Known limit: the patchelf path was validated against a stub (no linux-ELF + patchelf pair on this dev machine); a real NixOS smoke-run of setup.py is being arranged via the dev-env team once this lands in an rc.

Merges cleanly on top of #323 — different regions of the same file; both add import shutil. In the step flow, #323's platform check runs before this precheck, so a wrong-platform binary is deleted/re-downloaded before anything asks whether it "runs".

🤖 Generated with Claude Code

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 09b93a4d-baf9-4c1e-b85e-75b5109ce628

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/postinstall-lief-patching

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

GenVM PR actions

Tick a box to run it (the box unticks itself when handled). Actions only run while the PR has the ci-safe label.

• Force run full tests
• Rerun full tests
• Merge into dev

_{Full GenVM CI runs only when rtm or run-full-tests is set — "Force run full tests" is a sticky toggle for run-full-tests. Adding rtm marks the PR ready-to-merge and also runs full tests. Merge requires: rtm, green full tests, green E2E, and the branch 0 commits behind.}

[kp2pml30]

it was fixed in lief and manager builds newer version themselves now, but nixpkgs haven't accepted my patch yet NixOS/nixpkgs#538024

also this part was reworked I believe in manager