Skip to content

fix(tool-sandbox): let allow_launch_services reach the open shim on macOS - #1464

Merged
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/fix-allow-launch-services-shim
Jul 24, 2026
Merged

fix(tool-sandbox): let allow_launch_services reach the open shim on macOS#1464
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/fix-allow-launch-services-shim

Conversation

@kipz

@kipz kipz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1463

Summary

allow_launch_services: true only worked when no other command in the profile declared open_urls. Once any other command's edge set open_urls, three separate gaps combined to break it for every allow_launch_services-only command:

  1. Exec-gate (add_child_process_exec_gate_with_policy): the browser-open shim's path was only admitted to the exec allowlist for open_urls edges, not allow_launch_services ones — even though a bare open in the mediated $PATH always resolves to the shim (never straight to /usr/bin/open) once any command in the profile needs it.
  2. Env injection (inject_url_open_env): the URL-socket env var that lets the shim recognize itself as the relay was only set for open_urls edges — without it, the shim doesn't even attempt to connect to the supervisor.
  3. Socket capability (add_url_open_caps): connect access to the URL listener socket was, again, only granted for open_urls edges — so even once the shim tried to connect, the sandbox denied the connection.
  4. Origin check (validate_url_open): unconditionally required open_urls to be set, with no allow_launch_services check — so a relayed request would still be rejected.

This PR extends all four gates to also recognize allow_launch_services, scoped entirely to the macOS tool-sandbox path (Linux's allow_launch_services remains a documented no-op, unchanged).

Agent Disclosure

This PR was authored by an AI agent (Claude Code). Files consulted/changed: crates/nono-cli/src/tool-sandbox/platform/macos.rs (add_child_process_exec_gate_with_policy, validate_url_open, add_url_open_caps), crates/nono-cli/src/tool-sandbox/env.rs (inject_url_open_env). Also read (unchanged): crates/nono-cli/src/command_policy.rs (any_command_allows_url_open), crates/nono-cli/src/tool-sandbox/url_shim.rs, crates/nono-cli/src/url_open.rs, and the Linux equivalent (crates/nono-cli/src/tool-sandbox/platform/linux.rs) to confirm allow_launch_services is intentionally a no-op there and left untouched. Consulted CLAUDE.md for the affected area. Confirms compliance with repository requirements.

Test Plan

  • make ci (clippy -D warnings, fmt-check, full test suite) — clean.
  • 8 new unit tests covering each of the four fixed gates (exec-gate, env injection, socket capability, origin check) for both the allow_launch_services-without-open_urls case and the unaffected/no-op case.
  • Reproduced the original failure and confirmed the full fix against a real downstream consumer: built this commit into a full profile with both open_urls (on one command) and allow_launch_services (on another), invoked through genuine tool-sandbox child mediation. Before: fork/exec .../shims/open: operation not permitted, then (after partial fixes) a LaunchServices dispatch failure. After all four fixes: the browser opens successfully. Also re-verified against the original real-world trigger (a CLI tool doing an interactive OAuth browser login through the sandbox) — confirmed the browser opens there too.

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

  • 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/small 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 +237
Lines removed -14
Total changed 251
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.

…acOS

Signed-off-by: James Carnegie <me@kipz.org>
@kipz
kipz force-pushed the kipz/fix-allow-launch-services-shim branch from 49b181a to 93ff260 Compare July 20, 2026 16:45
@kipz
kipz marked this pull request as ready for review July 20, 2026 16:51

@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 found; 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 20, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 20, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 20, 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 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

@lukehinds lukehinds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

note to self, put something in changelog since allow_launch_services is public-facing

@lukehinds
lukehinds merged commit 82aa2bc into nolabs-ai:main Jul 24, 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.

allow_launch_services fails to open a browser when another command in the profile uses open_urls

2 participants