Repository navigation
Conversation
…y set The text dry run is written for people. A program that has to decide on the same answer (a launcher checking that a sandbox stays away from some directory, say) can only parse it, and a path can hold any character the text's framing uses: a line break, a bracket, a word the summary prints as a heading. Resource limits print as a framed row too. Add `--json` to `nono run`, valid with `--dry-run` only. It prints one JSON document on stdout from the same preparation the text dry run uses: every filesystem and Unix-socket capability the run would apply (the resolved path and the path as written, access, file or directory, and the source: user, profile, system or a named group), whether the network is blocked, the memory and process limits as numbers, and every sandbox a brokered command can run under (its session sandbox, each other caller's `from` edge, each intercept rule's override), resolved by the functions that build that sandbox at launch, for a command run from the working directory. Entries written relative to the command's live working directory are named, since they resolve wherever the command runs. A path that is not UTF-8 is given as its bytes in hex, never approximated. The platform tool-sandbox modules gain resolve_policy_grants, which runs add_policy_fs, add_policy_unix_sockets and (on Linux) resolve_exec_paths for one policy, so the report and the launch share one resolver. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
…es what it cannot model From a review of the first version: - Brokered commands are listed as the runtime selects them: a from.<caller> edge only when the caller is a command that lists the callee in can_use, intercept overrides only for a command that can run at all. - Each command reports its resolved executable, the credential paths its launch hands it (a raw file, a local socket), and every path its policy declares as the launch expands it before checking it exists, so a declared path that is absent today is still reported. - network.mode is the mode the run applies: the proxy is decided after preparation, as the text summary decides it (now one function, proxy_pending). - On Linux a read grant of /proc/self is reported as /proc, as the launch widens it for the child. - `incomplete` names what the report cannot model (raw Seatbelt rules, a credential path it cannot resolve); a program that decides on the report treats a non-empty one as "cannot be shown". - --json conflicts with --detached, whose launch discards stdout. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
🟡 Changes recommended
The report omits command-network grants and contains several inaccurate command and macOS representations.
5 open findings
What changed in this PR
This review-only PR adds machine-readable sandbox inspection through nono run --dry-run --json.
Changes:
- Adds JSON reporting for resolved capabilities, network, resources, and brokered commands.
- Shares policy-grant and proxy resolution with launch behavior.
- Adds CLI documentation, test helpers, and integration coverage.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
docs/cli/usage/flags.mdx |
Documents the JSON schema and limitations. |
crates/nono-test-support/src/cli.rs |
Adds dry-run JSON test helpers. |
crates/nono-cli/tests/dry_run_json.rs |
Tests report resolution and CLI constraints. |
crates/nono-cli/src/tool-sandbox/platform/macos.rs |
Resolves macOS command policy grants. |
crates/nono-cli/src/tool-sandbox/platform/linux.rs |
Resolves Linux command policy grants. |
crates/nono-cli/src/tool-sandbox/mod.rs |
Defines shared resolved-grant types. |
crates/nono-cli/src/sandbox_prepare.rs |
Shares proxy-startup determination. |
crates/nono-cli/src/main.rs |
Registers the reporting module. |
crates/nono-cli/src/launch_runtime.rs |
Updates test argument construction. |
crates/nono-cli/src/dry_run_report.rs |
Builds the JSON report. |
crates/nono-cli/src/command_runtime.rs |
Emits JSON during dry runs. |
crates/nono-cli/src/cli.rs |
Adds and validates --json. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…es, admission and declarations as the launch has them From a second review: - network.mode comes from prepare_proxy_launch_options, which decides the proxy without starting it: a credential route defined but not enabled starts no proxy, and deny_domain alone does. The text summary's predicate is left as it was. - `incomplete` names only the profile's raw Seatbelt rules, not the rules nono generates itself, so an ordinary macOS report is complete. - A declared entry whose dynamic token expands to nothing from the working directory is still listed (expanded: null), and cwd_dependent is read from the policy as written, not from the expansions. - The admission tests now cover a command that lacks can_use and a denied command's intercept override; the network test covers a defined-only credential route. - The documentation names what a brokered command's launch derives from its binary (interpreter, ELF closure, an exec intercept's helper), which the report does not list. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
A respond rule launches nothing and is skipped. An exec rule launches its helper, under the rule's sandbox or else the session's, and the entry's executable is that helper. A command's own network policy is documented as not listed. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57dd3a1a50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
…iled report A local-socket credential naming a regular file leaves the credential unavailable at launch; the report names it in the entry's incomplete list instead of failing. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6ddb18a4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…es, and classifies entries as the launch expands them An exec rule without a sandbox of its own runs its helper under each sandbox the callers select; each is listed. cwd_dependent is decided on the entry as its variables leave it, so $UID/x counts. A raw-file credential that is no regular file is incomplete. With no proxy or block decided, the network mode is the prepared set's own. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0918edeb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The report no longer resolves anything itself. The session's capability set, network mode and resource limits come from prepare_run_launch_plan, the plan `nono run` builds. Each brokered command's sandbox comes from the command-mediation runtime the launch prepares: build_child_launch_spec (or, for an exec intercept, build_child_launch_spec_for_binary with its helper) for every caller select_effective_policy admits, read back from the launch spec the child applies. The parallel resolution (policy grants, credential paths, caller selection, exec helper lookup) is gone; declared_policy_paths keeps only the expansion of declared paths, which a later run resolves again. The contract is that capability set. What else a run does is named in out_of_scope. What would widen filesystem access beyond the list (capability elevation, raw or generated Seatbelt allow rules) is named in incomplete. A new test runs the same profile (a glob, a variable, a symlink, a brokered command) for real and checks the report's filesystem against the capability state the run writes for itself. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
…ts own A command sandbox that goes through a scoped proxy needs that proxy's environment to be built. The dry run starts no proxy: it gives every scope start_command_scoped_proxies would serve (the same selection) a stand-in loopback address, so the launch code builds the sandbox as a run does. Its network stays out of the report's contract. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59b291a04c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A command whose policy the runtime refuses (resources it cannot enforce) launches nothing: its entry says why, with no grants. A catch-all intercept rule matches every run, so the command's own launch is not listed. The docs name the script read a brokered interpreter gets, which the session's read grants already bound. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
…nce and start directory Raw Seatbelt rules count as incomplete only when the prepared set still applies them (never on Linux, never once a trust policy strips them). Brokered entries carry no source: the launch spec a child applies has none, so none is claimed. workdir is where the program starts, decided by execution_start_dir as the launch decides it. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0e61e3040
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A catch-all intercept rule that launches the command as it is (no sandbox of its own, no helper) runs it under the selected sandbox; the report lists that launch. Only a respond, exec or sandboxed catch-all replaces it. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06f2675066
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…adows Rules match in order. An args rule whose own arguments an earlier args rule already matches (as the runtime's matcher decides) never runs, and is not listed. A rule whose args list is empty is a catch-all whatever its predicate says, as the matcher treats it. How the network mode is enforced (external, Landlock alone, WSL2's unenforced proxy) is named out of the contract. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6c6103110
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…and keeps going past a bad declaration An invocation policy that denies by default and allows or approves nothing refuses every run before any launch: that edge is not listed. A declared path the launch expands only when the command runs no longer fails the whole report when it does not expand: its entry names it. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`nono run --dry-run --json` no longer materializes credentials: no keystore or password manager is asked, no credential command runs, nothing prompts. The plan is otherwise built as `run` builds it, with each credential planned as declared (an env credential's destination, a proxied credential's route, both with no value). Command-policy credentials are listed as declared under credentials.commands, a file or socket credential with its declared path and where it resolves. One that doesn't resolve is named in incomplete and left out of the runtime the dry run prepares, so the rest is still built. On macOS, whether the run grants Claude Code LaunchServices depends on its Keychain login; the dry run leaves that undecided and says so in incomplete. A test gives a profile a command-backed credential whose command would leave a marker: the dry run leaves none, and its filesystem equals the capability state the run writes (the run then runs the command). Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2361d586c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… prompt The switch a JSON dry run sets is now plan-only, and covers more than credential sources: a missing profile is not looked up or pulled, the trust scan asks no keystore for a key (a dry run that would need one stops rather than prompt), and the working directory is shared only with --allow-cwd, as on a non-interactive terminal. Where a person would have been asked, incomplete says so. credentials.proxy names the session proxy's credential routes (a --credential service, a custom credential) as declared. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c360a2e9ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c360a2e9ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A working directory a run would ask the person to share is planned without it, named in incomplete, instead of stopping the dry run. A brokered grant the launch would resolve for the supervisor's own process can't be carried to the command: its entry is incomplete and shows the self-relative spelling. Provider routes are listed with the session proxy's credentials. The docs say read grants allow execute, and name Unix-socket enforcement out of the contract. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
…ation runs Any rule that mentions allow counts as a widening, however it is spaced or commented; a deny whose path says allow errs toward incomplete. The docs no longer say nothing runs: the program isn't launched, and the launch's own preparation still runs what it runs to plan (the trust scan, git for @git: tokens). Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df8913b58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ped trust key A registry profile that isn't installed stops the dry run instead of being pulled. A trust key the keystore would supply to verify an instruction file isn't read, and incomplete names it, whatever the trust policy's enforcement mode. The docs name the profile's own setup a dry run still does as a run does. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17e6586022
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…mote, and flags macOS resource limits A plan-only run whose trust scan failed only because it declined to read a key from the keystore leaves those files unverified and ungranted, named in incomplete, instead of stopping; any other failure stops it as it stops a run. --json conflicts with --remote, which would launch remotely instead of planning here. On macOS, where a run refuses resource limits, the report marks them incomplete. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8780f6939b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…through, and judge each blocked trust result The command runs as it is when no catch-all exists, or when any rule, catch-all or not, launches it with no sandbox of its own and no helper; a pass-through rule before a replacing catch-all no longer hides it. A plan-only run goes past a trust scan only when every result that blocks is a declined keystore lookup. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… error The --dry-run diagnostic was asserted on the --remote case after the first result was shadowed; each case now asserts its own exit code 2 and diagnostic. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The execute gate's allowlist (an ELF dependency closure over every executable on PATH, ~4,400 paths and nearly all of a dry run's cost) is used only to apply the gate, which a dry run never does; neither the session's reported caps nor any brokered launch spec reads it. A plan-only run leaves it unbuilt, and asking the runtime for the gate then is an error. out_of_scope and the docs name it. On macOS, the rules the runtime adds to the session are sorted: the gate's process-exec rule is the execute allowlist (out of the contract), the working directory's ancestor metadata reads are listed in metadata_paths, and any other allow is still named in incomplete. A test builds the allowlist anyway (NONO_DRY_RUN_BUILD_EXEC_GATE=1, honored only in a plan-only run) and shows the report is the same, the runtime directory's per-run name aside, and prints both timings. Signed-off-by: Jeremy Daer <jeremy@37signals.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 321a5f69ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review only. Don't merge. This shows the commits of topic branch
feat/dry-run-jsonagainst upstreammain(9edf3ea, pushed asreview-base/upstream-9edf3ea9), so the code can be reviewed. The fork carries the topic through the carry PR #5, which adds it to.github/integration/topics. Close this PR when that one merges, and deletereview-base/upstream-9edf3ea9then.What it adds
nono run --dry-run --jsonprints, as one JSON document on stdout, the capability set the run would apply, read from the launch's own plan rather than from a resolver of its own:prepare_run_launch_plan(the plannono runbuilds);out_of_scope, and a test shows the report is the same when it is built;incompletewhat only a source or a person would decide;select_effective_policyadmits, each intercept that launches something), built by the command-mediation runtime the launch prepares (build_child_launch_spec) and read back from the launch spec the child applies. No proxy is started: each command-scoped proxy the run would start gets a stand-in loopback address. The runtime's directory is removed afterwards.The contract is that set. What else a run does is named in
out_of_scope: brokered commands' network policy, external enforcement, PATH resolution, host session hooks, environment credentials,open_urls, signal, process-info and IPC scope, nono's own per-launch files. What would widen filesystem access beyond the list setsincompletewith a named reason: capability elevation, raw or generated Seatbelt allow rules, and a brokered launch the runtime wouldn't build.declaredandcwd_dependentkeep each brokered policy's declared paths as the launch expands them, for a consumer judging a later run. The format and itsversionare documented indocs/cli/usage/flags.mdx.Code
crates/nono-cli/src/dry_run_report.rsis new and serializes the plan.command_runtime.rs: the JSON branch builds the plan withprepare_run_launch_plan;execution_runtime.rsaddsdry_run_children, which prepares the command-mediation runtime the wayexecute_sandboxeddoes.tool-sandbox/platform/{linux,macos}.rs:PreparedToolSandboxRuntime::dry_run_childrenruns the launch's selection andbuild_child_launch_specfor each sandbox;declared_policy_pathsexpands declared paths with the launch's functions.proxy_runtime.rs:dry_run_scoped_proxy_env_varsgives the scopesstart_command_scoped_proxieswould serve a stand-in address.cli.rs:--jsonrequires--dry-runand conflicts with--detached.nono-test-supportgainsdry_run_json(),memory()andread(). The tests are intests/dry_run_json.rs, among them one that runs the same profile for real and compares the report's filesystem with the capability state the run writes for itself.