Repository navigation
fix(tool-sandbox): fall back to the interpreter's directory for RPATH-less ELF dependencies - #1650
Conversation
PR Review SummarySize
Affected crates
Blast radius — ContainedThis PR touches: source code Updated automatically on each push to this PR. |
|
Hey I tried to build nono from this PR. It fixed the command_policies failing, but I hit then another bug. When having a any #!-script on Not sure if fixing this should be part of this PR or a new issue. |
|
Fixing both issues in one if the scope doesnt expand greatly would be very appreciated! |
|
@Ekleog Is that something you would be willing to look into? I'm not well versed in Rust and would not feel comfortable vibe coding and submitting a PR that I would not thoroughly understand. |
|
Confirming this fixes #1649 on NixOS with nono 0.74.0 — the issue was filed against 0.73.0, so it's still current. The patch applies to tag v0.74.0 with offsets only and leaves Cargo.lock alone, so carrying it in nixpkgs is a one-line overrideAttrs. Would you consider un-drafting it? |
|
I just undrafted, but unfortunately it looks like CI is failing and I'll probably not be able to get back to this for the next few weeks (maybe until early next year depending on how things evolve on my personal side). If someone could take the PR over and push it to completion, it'd be awesome. Sorry for being unable to pass the finish line! |
|
If I have free time tmwr, I will take care of this; will write it down on my todo's! |
…-less ELF dependencies Closes nolabs-ai#1649 `resolve_shared_library`'s ELF dependency-closure walk only searched a `NEEDED` library's own `DT_RPATH`/`DT_RUNPATH` plus a hardcoded FHS default list (`/lib`, `/usr/lib`, etc.). On Nix/NixOS this fails whenever a shared object has no RPATH/RUNPATH of its own — e.g. `libgcc_s.so.1` (from the `gcc-*-libgcc` package) needing `libc.so.6` by bare soname — because none of the FHS defaults exist as such there, so `nono run` refused to start any command whose dependency closure reached that library. The real dynamic linker resolves this case fine: a Nix-built `ld.so` has its own directory compiled in as its default search path, so an RPATH-less `NEEDED` entry still resolves through it. This PR threads the *top-level* binary's own ELF interpreter (`PT_INTERP`) directory through the recursive closure walk as an extra fallback search directory — tried after the referencing object's own RPATH/RUNPATH and before the FHS defaults — so `resolve_shared_library` mirrors that real resolution order instead of hardcoding anything Nix-specific. On FHS/non-Nix systems this is a no-op: the interpreter directory there is normally `/lib` or `/lib64`, already covered by the existing default list. The `ELF_LIB_CACHE` memoization key was widened to include the interpreter directory so a resolution computed under one binary's interpreter doesn't leak into a different binary's closure within the same launch. Verified directly against `libgcc_s.so.1` on NixOS with `readelf -d` (empty `RUNPATH`) and `ldd` (resolves via the interpreter's own directory). - `cargo test -p nono-cli --bins tool_sandbox::linux` — 34 passed, including 6 new/changed tests covering the fallback, its precedence under RPATH, the not-found error path, and the real `PT_INTERP`-parsing path via `std::env::current_exe()`. - `cargo fmt --all -- --check` and `cargo build --release -p nono-cli` — clean. - Manual repro: `nono run --profile claude --allow-cwd -- ~/.nix-profile/bin/claude --dangerously-skip-permissions --help` on NixOS no longer fails with the ELF-resolution error. - [x] An issue exists and is linked above - [x] All commits are signed-off, using [DCO](https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin) - [x] All new code follows the project's coding standards ([CLAUDE.md](CLAUDE.md)) and is covered by tests - [x] Public-facing changes are paired with documentation updates — n/a, this is internal sandbox-initialization behavior with no public API or documented contract change This PR was prepared with AI assistance (Claude Code). Files/sections consulted: `crates/nono-cli/src/tool-sandbox/platform/linux.rs` (`elf_dependency_closure`, `resolve_elf_recursive`, `resolve_shared_library`, `parse_elf`/`parse_dynamic`, and the surrounding `ELF_*_CACHE` memoization), this repository's `AGENTS.md` (Coding Agent Contribution Policy, coding standards, security considerations), and the linked issue. - [x] I am not prohibited from contributing under this policy - [x] An issue already exists - [x] I described my intent and approach in the issue discussion - [x] I reviewed repository coding and security rules for the affected area - [x] I provided required attribution for reused or adapted code — n/a, no reused/adapted code - [x] I did not use forbidden patterns such as unwrap/expect - [x] I used NonoError where required - [x] I validated and canonicalized all relevant paths - [x] This PR matches the approved or disclosed issue scope Signed-off-by: Léo Gaspard <leo@gaspard.io>
4d688bf to
b5ff77f
Compare
|
@jocelynthode Yup, just orienting myself around this right now. Will tag you once again to give it a whirl when the fix is up |
Grant and validate shebang interpreter closures for trusted executable directories, including /usr/bin/env targets. Add Nix-style wrapper and writable-interpreter regressions. Signed-off-by: Aleksy Siek <aleksy@nolabs.ai>
Signed-off-by: Aleksy Siek <aleksy@nolabs.ai>
|
@jocelynthode Let me know if both issues are resolved now off of this PR please |
|
@SequeI I did a quick test running on this branch with pi worked fine. I also have command policies in this profile (for gh and kubectl) and both worked fine, so from my cursory test it appears to be fixed as these 2 tests where failing before! Thanks! |
Linked Issue
Closes #1649
Closes #1724
Summary
resolve_shared_library's ELF dependency-closure walk only searched aNEEDEDlibrary's ownDT_RPATH/DT_RUNPATHplus a hardcoded FHS defaultlist (
/lib,/usr/lib, etc.). On Nix/NixOS this fails whenever a sharedobject has no RPATH/RUNPATH of its own — e.g.
libgcc_s.so.1(from thegcc-*-libgccpackage) needinglibc.so.6by bare soname — because none ofthe FHS defaults exist as such there, so
nono runrefused to start anycommand whose dependency closure reached that library.
The real dynamic linker resolves this case fine: a Nix-built
ld.sohas itsown directory compiled in as its default search path, so an RPATH-less
NEEDEDentry still resolves through it. This PR threads the top-levelbinary's own ELF interpreter (
PT_INTERP) directory through the recursiveclosure walk as an extra fallback search directory — tried after the
referencing object's own RPATH/RUNPATH and before the FHS defaults — so
resolve_shared_librarymirrors that real resolution order instead ofhardcoding anything Nix-specific.
On FHS/non-Nix systems this is a no-op: the interpreter directory there is
normally
/libor/lib64, already covered by the existing default list.The
ELF_LIB_CACHEmemoization key was widened to include the interpreterdirectory so a resolution computed under one binary's interpreter doesn't
leak into a different binary's closure within the same launch.
Verified directly against
libgcc_s.so.1on NixOS withreadelf -d(emptyRUNPATH) andldd(resolves via the interpreter's own directory).Agent Disclosure (if applicable)
This PR was prepared with AI assistance (Claude Code). Files/sections
consulted:
crates/nono-cli/src/tool-sandbox/platform/linux.rs(
elf_dependency_closure,resolve_elf_recursive,resolve_shared_library,parse_elf/parse_dynamic, and the surroundingELF_*_CACHEmemoization),this repository's
AGENTS.md(Coding Agent Contribution Policy, codingstandards, security considerations), and the linked issue.
Test Plan
cargo test -p nono-cli --bins tool_sandbox::linux— 34 passed, including6 new/changed tests covering the fallback, its precedence under RPATH,
the not-found error path, and the real
PT_INTERP-parsing path viastd::env::current_exe().cargo fmt --all -- --checkandcargo build --release -p nono-cli— clean.nono run --profile claude --allow-cwd -- ~/.nix-profile/bin/claude --dangerously-skip-permissions --helpon NixOS no longer fails with the ELF-resolution error.Checklist
Agent Compliance Check (Required for AI/Automated PRs)