Skip to content

fix(tool-sandbox): grant command interpreter read of its script - #1467

Merged
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/shebang-interp-script-read
Jul 27, 2026
Merged

fix(tool-sandbox): grant command interpreter read of its script#1467
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/shebang-interp-script-read

Conversation

@kipz

@kipz kipz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1466

Summary

On Linux, when the tool-sandbox is engaged, a #!/usr/bin/env <interp> script's interpreter is launched to run the script inside a per-exec child sandbox that grants the interpreter + its runtime baseline but not the script — which is an argv path, not the launched binary — so the interpreter gets EACCES reading its own script (and its module tree), even when the caller's session already grants the script's directory. This grants the child read of argv paths the agent's outer sandbox already permits, re-using the covering outer read grant so dependency trees resolve; bounded by outer_caps, so the child gains nothing the caller lacked. When such a script is matched, it also grants the session TLS-intercept trust bundle (otherwise given only to commands with their own proxy route), so the interpreter can make the caller's HTTPS calls.

Agent Disclosure (if applicable)

  • This PR was authored by an AI agent (Claude) on behalf of the repository owner.
  • Areas changed: crates/nono-cli/src/tool-sandbox/platform/linux.rs (build_child_caps + new add_interpreted_script_read/_inner); reviewed CLAUDE.md coding/security rules and the sibling handling from fix(tool-sandbox): grant env-shebang scripts their re-exec interpreter #1394.
  • The grant is bounded by outer_caps (path_covered_with_access) so it never widens the child beyond the caller's own read set; argv paths are canonicalized before use.

Test Plan

  • New unit test interpreted_script_read_grants_covered_script_and_trust_bundle (Linux): asserts a covered script's enclosing outer read grant + the trust bundle are granted, and that an argv path outside outer_caps is not granted. Passes (cargo test -p nono-cli, Linux container).
  • Built nono for Linux and verified manually: a #!/usr/bin/env node script run under a profile with command policies now executes (was EACCES on the script before); reads outside the caller's grants remain denied.
  • Change is #[cfg(target_os = "linux")]; host make ci clippy/fmt/tests pass (one unrelated attestation-signing test is a known parallel-run flake — passes in isolation).

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates
  • Release note has been added to CHANGELOG.md if needed

Agent Compliance Check (Required for AI/Automated PRs)

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

@github-actions github-actions Bot added bug Something isn't working nono-cli size/medium labels Jul 20, 2026
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +306
Lines removed -2
Total changed 308
Classification Large (> 300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@kipz
kipz marked this pull request as ready for review July 20, 2026 19:34

@nogent-nolabs-ai nogent-nolabs-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nogent code review

1 medium severity bug found.

Findings (not tied to a changed line):

  • 🐛 [LOW · bug] crates/nono-cli/src/tool-sandbox/platform/macos.rs:2590 — There is a platform divergence where shebang script interpreter read support (add_interpreted_script_read) is entirely absent on macOS. This causes shebang script execution to fail with EACCES on macOS under restricted policies even when the agent's outer sandbox has read access. Implement equivalent shebang script read capability handling in macOS build_child_caps.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026
@kipz kipz changed the title fix(tool-sandbox): grant mediated interpreter read of its script fix(tool-sandbox): grant command interpreter read of its script Jul 20, 2026
@kipz
kipz marked this pull request as draft July 20, 2026 19:51
@kipz
kipz force-pushed the kipz/shebang-interp-script-read branch from ab12a45 to ae22df6 Compare July 20, 2026 20:03
@kipz

kipz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed the findings:

  • [HIGH · security] CA/grant leaked to any command via argv file path. Real concern, fixed. Note the suggested binary.shape.kind == ShebangScript gate wouldn't work here: the launched binary is the interpreter (/usr/bin/node, kind == Elf) with the script as an argv path, so that gate would disable the fix entirely. Instead the grant now fires only when an argv path is a shebang script whose resolved interpreter is the launched binary (classify_executable_shape + env_shebang_target_interpreter, basename match). curl /path/file (plain file) and curl script.sh (shebang for a different interpreter) both match nothing, so they get neither the read grant nor the trust bundle. Covered by new negative test cases.

  • [MEDIUM · bug] path_covered_with_access precheck ignores file capabilities. Removed the precheck entirely; coverage is now decided in the per-capability loop, which handles both file and directory caps.

  • [MEDIUM · design] called for all commands. The call stays in build_child_caps, but the shebang-interpreter check above scopes it precisely — ordinary (non-interpreter) commands match nothing, so no argv read grant or trust bundle is added for them.

  • [LOW · bug] macOS divergence. This is Linux/Landlock-specific — under macOS Seatbelt the interpreter reads its script without this failure in our testing, so no macOS change is included here. Happy to add the equivalent if a macOS repro surfaces.

@kipz
kipz marked this pull request as ready for review July 20, 2026 20:06
kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026

@nogent-nolabs-ai nogent-nolabs-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nogent code review

2 medium findings: relative path skip bug and missing macOS platform implementation.

Findings (not tied to a changed line):

  • 🏗️ [MEDIUM · design] crates/nono-cli/src/tool-sandbox/platform/macos.rs:2590 — The script read grant and trust bundle delegation is only implemented for Linux. Sibling handling is missing on macOS under crates/nono-cli/src/tool-sandbox/platform/macos.rs, causing interpreted scripts to still fail with EACCES on macOS.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
Comment thread crates/nono-cli/src/tool-sandbox/platform/linux.rs Outdated
@kipz

kipz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Fixed both — the coverage check now runs before the file is opened/classified (is_shebang_script_for is only called once a covering outer_caps grant is found), and relative argv paths are now resolved against request.cwd before canonicalizing instead of being skipped. Added regression tests for both (interpreted_script_read_grants_nothing_for_an_uncovered_script, interpreted_script_read_resolves_relative_argv_against_cwd).

kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 23, 2026
… read of its script

# Conflicts:
#	crates/nono-cli/src/tool-sandbox/platform/linux.rs
kipz added 2 commits July 24, 2026 16:41
…ript

A #!/usr/bin/env <interp> script re-exec's <interp> (e.g. node), which the
exec gate launches as its own binary with the script as an argv path, not
binary.canonical_path. Its child domain never grants the script, so module
runtimes fail to read the script and its dependency tree (EACCES).

Grant the child read only when an argv path is a shebang script whose
resolved interpreter is the launched binary — excluding ordinary file
arguments to non-interpreter commands (e.g. curl /path/file). For a matched
script re-grant the covering outer read grant so the dependency tree resolves
(bounded by outer_caps: nothing the caller could not already read), plus the
session TLS-intercept trust bundle so the interpreter can make the caller's
HTTPS calls.

Signed-off-by: James Carnegie <me@kipz.org>
@kipz
kipz force-pushed the kipz/shebang-interp-script-read branch from c784192 to 3084615 Compare July 24, 2026 16:01
@lukehinds
lukehinds merged commit b4dbd4f into nolabs-ai:main Jul 27, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nono-cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sandboxed #!/usr/bin/env <interp> script can't read itself under the tool-sandbox (EACCES)

2 participants