Skip to content

fix(tool-sandbox): clean up runtime dir with sealed shims - #1492

Merged
SequeI merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/tool-sandbox-dir-leak
Jul 27, 2026
Merged

fix(tool-sandbox): clean up runtime dir with sealed shims#1492
SequeI merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/tool-sandbox-dir-leak

Conversation

@kipz

@kipz kipz commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1491

Summary

The per-invocation tool-sandbox runtime directory leaked on every run. Its shims/ subdirectory is sealed to 0o500, and guarded_remove_runtime_dir called fs::remove_dir_all without restoring write on it — so unlinking the sealed shim copies failed with EACCES and the whole tree was left behind (the error was only logged at debug). This restores owner-write recursively across the runtime tree before removal, on macOS and Linux, without changing the sealing behaviour itself.

Agent Disclosure (if applicable)

  • This PR was generated by an automated coding agent.
  • Files changed: crates/nono-cli/src/tool-sandbox/mod.rs, .../platform/macos.rs, .../platform/linux.rs.
  • Complies with repository coding/security rules: no unwrap/expect, NonoError used, symlinks never followed during the chmod walk.

Test Plan

  • New regression test on macOS and Linux (guarded_remove_deletes_runtime_dir_with_sealed_shims): seals a shims/ dir and asserts the runtime dir is removed. Verified it fails without the fix and passes with it.
  • make ci (clippy -D warnings, fmt-check, tests, doc lint) clean.
  • Verified downstream against a real profile with mediated commands: normal exit and SIGTERM/SIGINT runs now leave zero nono-tool-sandbox-* residue (previously leaked on every run).

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 (N/A — no reused/adapted code)
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths (N/A — no new path canonicalization; symlinks never followed)
  • This PR matches the approved or disclosed issue scope

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

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

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +108
Lines removed -6
Total changed 114
Classification Medium (50–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 23, 2026 13:35
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

kipz added a commit to kipz/nono that referenced this pull request Jul 23, 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

No blocking issues; 0 suggestions.

Findings: none flagged in scope.

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

kipz added a commit to kipz/nono that referenced this pull request Jul 23, 2026
Signed-off-by: James Carnegie <me@kipz.org>
@kipz
kipz force-pushed the kipz/tool-sandbox-dir-leak branch from b72dfc3 to 4181ab8 Compare July 24, 2026 15:31
@lukehinds lukehinds added this to the 0.70 milestone Jul 27, 2026

@SequeI SequeI left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm, thanks!

@SequeI
SequeI merged commit 6dfcf27 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 size/medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool-sandbox runtime directory leaks on every run (sealed shims dir blocks cleanup)

3 participants