Repository navigation
feat(ipc)!: restrict IPC eval to an allowlist and confirm visible code with the human - #303
Merged
Merged
Conversation
`arf ipc eval` accepted arbitrary R code from any program that could reach the session socket, and `arf ipc send` injected code into an interactive REPL without asking the person sitting at it. Evaluation now parses the code with tree-sitter-r and rejects it unless every call target is allowlisted. The allowlist is empty by default and is configured through `[ipc.eval].allowed_functions` or the repeatable `--ipc-eval-allow-function` startup option; `--ipc-eval-unrestricted` restores the previous behaviour and is available at startup only. Parse errors, computed callees, assignments, control flow, `:::`, and the native pipe are rejected. Rejections happen before R evaluation, so they leave no history entry. This is a syntactic policy, not an R sandbox. Interactive `send` now previews the code and waits for an explicit `y`. Anything else, including Ctrl+C, Ctrl+D, and a client that has gone away, declines. `:ipc send-policy allow` lifts the prompt for the running process only. Headless `send` is unchanged, since there is nobody to ask. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The confirmation printed `IPC send [1]? [y/N]: ` directly after arf's R prompt, so the transcript read as though someone had typed R code at the prompt. Every line of the confirmation now starts with `# [arf] `, matching the status line `:ipc send-policy` already prints, which keeps a copied transcript valid as R comments. The old `[y/N]:` also implied that Enter was required, when the answer is a single keypress; the instruction now says so, on its own line so it stays visible when the preview is long. A decline is not echoed in raw mode and previously left no trace, so it now reports `# [arf] IPC send declined.` Non-key terminal events also declined the request, which meant that merely resizing the window while the confirmation was up rejected the send. Those events are now ignored and the confirmation keeps waiting for a key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`arf ipc eval '1'` was rejected. The top level of the policy accepted only calls and operators, so a bare literal or a bare identifier fell into its catch-all error, even though the same nodes were already accepted as arguments to an allowlisted call. Only the top level was inconsistent. Reading a value is a main use of `eval` — an editor's debugger inspects objects — and it does not need a call. Literals have no side effects at all. Evaluating an identifier can force a promise or run an active binding, but IPC eval never permits assignment, so a caller cannot create one, and formatting the result dispatches the same methods an allowlisted call's return value already would. Extraction with `$`, `[`, and `[[` stays allowlisted: those dispatch to user-defined methods, so a session that wants debugger-style inspection opts into that reach at startup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`--visible` exists to close a gap in `send`: `send` runs code in the session but returns no output, so a caller cannot see the result. It is send with the output captured, and it was being checked against the eval allowlist while `send` itself was not. What decides the policy is whether a person watching the session can see the operation. Silent evaluation leaves no trace, so it keeps the allowlist. `send` and `eval --visible` both surface in the session, so both are now confirmed by the human instead. A headless session has nobody to ask, so `eval --visible` runs immediately there, exactly as headless `send` already did. Approval also has to report whether it emitted its own CRLF rather than letting callers assume it. Visible evaluate would otherwise print one newline too many, and `:ipc send-policy allow` -- which approves without printing anything -- dropped the newline the echoed line needed, losing the `agent>` line entirely. The second one predates this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README quickstart and the `arf ipc eval` help still started a plain `arf headless` and then evaluated `1 + 1`, which now fails because `+` is not allowlisted. Both show the startup flag instead, and the README also shows `--visible`, which needs no allowlist because the code is shown where a person can see it. The `R_EVAL_NOT_ALLOWED` hint listed only the allowlist and the unrestricted flag, all of which need a server restart. It now offers `--visible` first, since that is the only route that works against the session already running. Also record two things the docs did not say: evaluating a bare identifier can run code by forcing a promise or triggering an active binding, and visible evaluation returns a different response shape in each mode -- interactive leaves `value` and `error` null because it runs through normal REPL evaluation, while headless populates them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The examples were written before evaluation was allowlisted, and most of them no longer worked: piping `1 + 1` over stdin, `getwd()` with `--pid`, and the `jq` example all fail with `R_EVAL_NOT_ALLOWED`. Two of them had grown their own `arf headless` startup line, so it was also unclear which server any given example was talking to. One startup line now heads the block and allowlists what the examples call, and the rest run against it. The `--timeout` example is back, since what that flag does not do -- cancel the evaluation -- is the easiest thing here to get wrong. All six were run against a real headless session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `R_EVAL_NOT_ALLOWED` hint offered `--visible` as a way to avoid a restart, which reads as advice to bypass the policy. It skips the allowlist for a reason: the code runs where the session shows it, and what the allowlist protects is the silent path nobody sees. The hint and the help caption now say that, and the hint notes that the allowlist is settable only at startup rather than describing that as a restart. No behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The policy walk rejected any named node it did not recognise, and the R grammar exposes both argument separators and comments as named nodes. Only comments directly under the program root were skipped, so a comment anywhere inside an expression was refused — and, more seriously, every call with more than one argument was refused too, because the separating comma reached the catch-all arm. Neither node carries execution semantics, so skip both wherever the traversal walks named children. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The eval subcommand's own examples were fixed earlier, but the quick start on the parent commands and the block in the IPC guide still started a default restricted server and then evaluated code no allowlist entry covered, so copying them returned R_EVAL_NOT_ALLOWED. Start each block with the entries its examples need, and describe `--visible` by what it does rather than by the check it skips. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adding the eval allowlist options grew HeadlessArgs past the point where clippy's large_enum_variant fires on the subcommand enum. The lint is platform-sensitive and only tripped on Windows, but the enum is parsed once per process, so boxing the largest variant costs nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens arf’s IPC surface by preventing unattended clients from running arbitrary R in a session: silent arf ipc eval is restricted by a server-side syntactic allowlist, while visible operations (send and eval --visible) require explicit human approval in interactive sessions (and remain immediate in headless sessions).
Changes:
- Add a tree-sitter-based syntactic policy for silent IPC eval, configurable via
[ipc.eval].allowed_functions,--ipc-eval-allow-function, or--ipc-eval-unrestricted(startup-only). - Add interactive approval prompting for IPC
sendand routeeval --visiblethrough the same approval path. - Update docs, CLI help/completions/snapshots, and expand PTY/headless/integration test coverage for the new behaviors.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates IPC examples to reflect allowlisted silent eval and visible eval behavior. |
| docs/ipc.md | Documents new eval policy, new error codes, and interactive approval semantics. |
| docs/configuration.md | Adds [ipc.eval].allowed_functions to documented config. |
| crates/arf-console/tests/pty_ipc_tests.rs | Adds PTY tests for approval prompt and new send-policy behavior; adjusts expectations for visible eval. |
| crates/arf-console/tests/ipc_tests.rs | Updates IPC transport tests for approval prompting; adds PTY output waiting helper. |
| crates/arf-console/tests/headless/support.rs | Defaults headless test harness to unrestricted unless a policy flag is supplied. |
| crates/arf-console/tests/headless/eval.rs | Adds headless allowlist test and verifies visible vs silent behavior under restriction. |
| crates/arf-console/src/snapshots/arf__cli__tests__help_long.snap | Updates long help snapshot with new CLI flags. |
| crates/arf-console/src/snapshots/arf__cli__tests__help_headless.snap | Updates headless help snapshot and examples for allowlisting. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_zsh.snap | Adds new flags to zsh completions snapshot. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_powershell.snap | Adds new flags to PowerShell completions snapshot. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_fish.snap | Adds new flags to fish completions snapshot. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_bash.snap | Adds new flags to bash completions snapshot. |
| crates/arf-console/src/repl/read_console.rs | Routes visible IPC ops through approval, and avoids extra blank line when approval already emitted CRLF. |
| crates/arf-console/src/repl/meta_command.rs | Adds `:ipc send-policy prompt |
| crates/arf-console/src/main.rs | Initializes eval policy from config + CLI at startup. |
| crates/arf-console/src/ipc/protocol.rs | Adds IPC error codes for policy rejection and unapproved interactive input. |
| crates/arf-console/src/ipc/policy.rs | Implements syntactic allowlist validation for silent eval using tree-sitter-r. |
| crates/arf-console/src/ipc/mod.rs | Enforces policy for silent eval; adds interactive approval prompt logic and send-policy state. |
| crates/arf-console/src/ipc/client.rs | Adds user-facing mappings for new IPC error codes. |
| crates/arf-console/src/config/snapshots/arf__config__tests__schema_tests__default_config.snap | Updates default config snapshot to include [ipc.eval]. |
| crates/arf-console/src/config/snapshots/arf__config__tests__schema_tests__config_schema.snap | Extends config schema snapshot with IPC eval config. |
| crates/arf-console/src/config/mod.rs | Wires IPC config into Config and adds parsing test. |
| crates/arf-console/src/config/ipc.rs | Defines IPC eval configuration structs and schema metadata. |
| crates/arf-console/src/completion/meta.rs | Adds completion support for :ipc send-policy … and updates descriptions. |
| crates/arf-console/src/cli/mod.rs | Adds global CLI flags and boxes Headless subcommand args to reduce enum size. |
| crates/arf-console/src/cli/ipc.rs | Updates arf ipc eval CLI help to explain allowlist vs visible execution. |
| crates/arf-console/src/cli/headless.rs | Adds headless CLI flags to configure the eval policy at startup. |
| crates/arf-console/src/app/headless.rs | Initializes eval policy for headless sessions from config + CLI at startup. |
| CHANGELOG.md | Documents breaking changes and new IPC policy/approval behavior. |
| artifacts/arf.schema.json | Updates published JSON schema to include IPC eval config. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The user_input test slept a fixed 300ms before writing the approval keystroke, which would flake on a loaded CI runner. Wait for the prompt on the PTY the way the visible-evaluate test already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"is now confirmed like send rather than checked against the eval allowlist" described a transition that never reached a release: the allowlist arrives in this same entry, so there is no earlier state in which visible evaluation was checked against it. Say which of the two mechanisms applies instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Approval for `send` and for `eval --visible` is one mechanism, so listing them separately read as two independent new restrictions. Merge them, and trim both IPC entries to the facts a user has to act on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
arf ipc evallet any program with access to the session socket run arbitrary R code in a human's session, silently. This restricts it.Two mechanisms, selected by whether a human can see the code run — not by whether the session is headless or interactive:
send, andeval --visible) is guarded by human approval. An interactive session prompts before running the code; a headless session has no human to prompt, so it runs immediately, assendhas always done.The allowlist is empty by default. Literals and bare identifiers evaluate without configuration so that a debugger can still read values; every call, operator, and index operation needs an explicit entry.
What changed
Eval policy.
evaluaterequests are parsed with tree-sitter and walked. Each call target, operator,$,[, and[[must appear in the allowlist. Assignment, function definitions, and control flow are never permitted, with or without an allowlist entry. The walk fails closed: an unrecognised grammar node is rejected rather than ignored. This is a syntactic filter, not an R sandbox — it does not promise that an allowed function is non-mutating, and evaluating a bare identifier can itself run code by forcing a promise or triggering an active binding. The documentation says so.Configure with
[ipc.eval].allowed_functionsinarf.tomlor--ipc-eval-allow-function(repeatable) at startup.--ipc-eval-unrestrictedopts a session out entirely and is available only at startup — a client cannot ask for it.Send approval. In an interactive REPL,
arf ipc sendprints the code and asks for confirmation before running it. No configuration file setting or CLI flag disables this.:ipc send-policy allow, typed by the human at their own prompt, suspends it for the current session only; a restart returns to prompting. Headless sessions have no human to ask and runsendimmediately, as before.eval --visible. Because it runs the code where the person can see it, it counts as visible evaluation: the approval above applies to it, and the allowlist does not. Its response fields therefore differ between the two visible paths, whichdocs/ipc.mdspells out.Design notes for reviewers
Two aspects look like gaps but are deliberate.
Headless
eval --visibleis not allowlisted. A client with socket access to a headless session can already run arbitrary R throughsend, and could before this change. What--visibleadds oversendis output capture, not execution capability, so requiring an allowlist for it would not close a hole — it would only make the two visible paths inconsistent with each other. A headless session is owned by the agent that started it.Bare identifiers are allowed by default. Reading an identifier can execute code, via a promise or an active binding. It is permitted anyway because IPC evaluation never allows assignment, so a caller cannot plant the binding it would need; the risk is confined to state the session already had. Requiring an allowlist entry to read a variable would also make the feature useless for its main purpose, which is inspection.
Breaking change
This is a breaking change to an experimental interface. Existing callers of
arf ipc evalwill getR_EVAL_NOT_ALLOWEDuntil the session is started with the entries they need, or with--ipc-eval-unrestricted. Callers ofarf ipc sendagainst an interactive session will block until the human answers the prompt.Test plan
:ipc send-policymeta command, and the display of the pending request.arf ipc eval --help,arf ipc --help,arf headless --help, anddocs/ipc.mdrun against a real server and confirmed to succeed as written.cargo fmt,cargo clippy --workspace --all-targets -- -D warnings, andcargo test -p arf-consoleclean.🤖 Generated with Claude Code