diff --git a/README.md b/README.md index 07aaa466..801ef016 100644 --- a/README.md +++ b/README.md @@ -362,19 +362,33 @@ preflight, before any action starts. Create the configured target first or prote an existing ancestor. Merry never creates host paths to install these masks; optional, absent development mounts remain skippable. -`ssh_agent`, `gpg_agent`, and `dbus` independently enable outer-sandbox forwarding; -they do not preauthorize inner actions. An inner action must request the matching -host-integration capability and receive runtime approval before using the forwarded -socket or automatically imported client files. Explicit trusted path rules may -separately expose regular files, but do not approve protected agent sockets. -A missing agent does not prevent ordinary actions -from starting; explicitly requesting an unavailable endpoint reports an error. Native GPG -socket discovery uses `gpgconf --list-dirs` and honors `GNUPGHOME`; it does not -assume sockets live in `.gnupg` or `/run`. Outer scaffolding preserves private -socket-directory permissions without importing their other contents. - -After approval, the SSH integration also exposes `~/.ssh/known_hosts` and -`known_hosts2` read-only to inner actions, +`ssh_agent`, `gpg_agent`, and `dbus` independently enable outer-sandbox forwarding +and preauthorize the matching inner capability: when the validated endpoint exists, +ordinary actions use the forwarded socket and automatically imported client files +without a separate request, like trusted `readonly_paths` and `readwrite_paths`. +`review_paths` still mask a configured endpoint until its exact path is approved +for that action, and `deny_paths` always mask it; explicit trusted path rules that +expose regular files do not thereby approve protected agent sockets. An +integration that trusted configuration did not enable can still be requested for +one permissioned action when its endpoint is visible. A missing agent does not +prevent ordinary actions from starting; explicitly requesting an unavailable +endpoint reports an error. Native GPG socket discovery uses `gpgconf --list-dirs` +and honors `GNUPGHOME`; it does not assume sockets live in `.gnupg` or `/run`. Outer +scaffolding preserves private socket-directory permissions without importing their +other contents. + +Path policy decides whether an enabled endpoint is reachable, not whether it is +announced. `SSH_AUTH_SOCK`, `DBUS_SESSION_BUS_ADDRESS`, and `GNUPGHOME` name the +configured endpoints for every action, while a `deny_paths` or `review_paths` +entry covering the socket, the keyring, or one of their parent directories masks +the mount itself. A broad deny therefore also hides the agent even though +`ssh_agent`/`gpg_agent`/`dbus` is enabled: the client sees the endpoint and fails +when it connects. Keep those denies narrow (or outside the endpoint tree) when the +integration should stay usable, and remember that `deny_paths` is never reopened +by an approval. + +Once the SSH integration is available, `~/.ssh/known_hosts` and +`known_hosts2` are exposed read-only to inner actions, without granting access to private keys, `~/.ssh/config`, or the network. These files still obey `deny_paths` and `review_paths`; an explicit deny of the entire `.ssh` directory blocks them as well. Existing host identities can be checked, @@ -420,13 +434,14 @@ Runtime owns the session capability store and retention decisions. Process adapt consume read-only snapshots and report normalized path constraints; they do not record grants. Each preparation captures a fresh mount-alias view, shared by path review, masking, and client-resource discovery, rather than caching the filesystem -for an entire session. Approved host integrations may be retained within that -session; configuration flags alone never create a runtime grant. Separately -reviewed paths still require per-action approval. - -`gpg_agent = true` makes the conventional public-key stores available for reviewed -use. After GPG integration approval, inner actions import `pubring.kbx` and legacy -`pubring.gpg` read-only, without additional `readonly_paths`. Each approved inner +for an entire session. Trusted configuration flags are the preauthorized baseline +for paths and host integrations; capabilities approved through a request may be +retained within that session. Separately reviewed paths still require per-action +approval. + +`gpg_agent = true` makes the conventional public-key stores available to inner +actions. When the integration is available, inner actions import `pubring.kbx` and +legacy `pubring.gpg` read-only, without additional `readonly_paths`. Each such action gets a private, temporary `GNUPGHOME` view at the original path for locks and a fresh trust database. These client writes never modify the host keyring, even when the host directory is declared read-only. diff --git a/crates/merry-cli/src/coding/tests/composition.rs b/crates/merry-cli/src/coding/tests/composition.rs index f85a6d1c..df220cba 100644 --- a/crates/merry-cli/src/coding/tests/composition.rs +++ b/crates/merry-cli/src/coding/tests/composition.rs @@ -103,6 +103,10 @@ async fn headless_runtime_uses_coding_agent_profile() { .collect::>() .join("\n"); assert!(request_text.contains("Coding file capabilities")); + assert!( + request_text.contains("Network is withheld from every action that does not request it") + ); + assert!(request_text.contains("Action PATH search tools:")); assert!(request_text.contains("user's current input language")); assert!( request diff --git a/crates/merry-cli/src/config/mod.rs b/crates/merry-cli/src/config/mod.rs index f672700a..1a750499 100644 --- a/crates/merry-cli/src/config/mod.rs +++ b/crates/merry-cli/src/config/mod.rs @@ -234,8 +234,13 @@ impl MerryConfig { } /// Returns host IPC integrations explicitly enabled by trusted global - /// configuration. These form the outer sandbox capability ceiling and are - /// forwarded to inner process sandboxes when their endpoints are present. + /// configuration. + /// + /// The same flags are the outer sandbox capability ceiling and the inner + /// action preauthorization: when a validated endpoint exists, ordinary + /// inner actions use it without another permission request. `deny_paths` + /// and `review_paths` still mask a configured endpoint until the exact + /// path is approved for that action. pub fn host_integrations(&self) -> Vec { let Some(permissions) = self.raw.permissions.as_ref() else { return Vec::new(); diff --git a/crates/merry-cli/src/config/tests/permissions.rs b/crates/merry-cli/src/config/tests/permissions.rs index 62d044de..213e9b43 100644 --- a/crates/merry-cli/src/config/tests/permissions.rs +++ b/crates/merry-cli/src/config/tests/permissions.rs @@ -123,7 +123,7 @@ fn rejects_removed_no_sandbox_review_key() { } #[test] -fn parses_host_integrations_for_outer_sandbox_ceiling() { +fn parses_host_integrations_for_outer_ceiling_and_inner_preauthorization() { let paths = XdgPaths::from_parts(home(), None, None); let config = MerryConfig::load_optional_from_text( Some( diff --git a/crates/merry-cli/src/runtime_config.rs b/crates/merry-cli/src/runtime_config.rs index 29693ed7..bb2c0dac 100644 --- a/crates/merry-cli/src/runtime_config.rs +++ b/crates/merry-cli/src/runtime_config.rs @@ -91,7 +91,14 @@ pub(crate) fn main_reasoning_effort( .and_then(|provider| provider.reasoning_effort)) } -pub(crate) fn action_process_backend_options( +/// Builds the action backend inputs that need no asynchronous discovery. +/// +/// GnuPG socket discovery has to run on the async runtime, so this is the +/// synchronous base for [`prepared_action_process_backend_options`], which every +/// product surface uses. Trusted global configuration preauthorizes the inner +/// action sandbox here; the per-action endpoint checks still happen in the +/// process backend. +fn action_process_backend_options( config: Option<&MerryConfig>, ) -> Result { let home = config @@ -124,7 +131,14 @@ pub(crate) fn action_process_backend_options( Ok(ActionProcessBackendOptions::new() .with_path_rules(path_rules) .with_network_requests_allowed(config.is_none_or(MerryConfig::network_requests_allowed)) - .with_environment_overrides(environment_overrides)) + .with_environment_overrides(environment_overrides) + // Trusted global config is the user's own preauthorization for the + // inner action sandbox; the endpoints are still validated per action. + .with_host_integrations( + config + .map(MerryConfig::host_integrations) + .unwrap_or_default(), + )) } pub(crate) fn configured_runtime_builder( @@ -189,7 +203,9 @@ mod tests { use std::{fs, path::PathBuf, sync::Arc}; #[test] - fn configured_host_integrations_do_not_preauthorize_inner_actions() { + fn configured_host_integrations_preauthorize_inner_actions() { + use merry_runtime::HostIntegration; + let paths = XdgPaths::from_parts(PathBuf::from("/home/alice"), None, None); let config = MerryConfig::load_optional_from_text( Some("[permissions]\nssh_agent = true\ngpg_agent = true\ndbus = true\n"), @@ -197,9 +213,27 @@ mod tests { ) .unwrap() .unwrap(); - assert_eq!(config.host_integrations().len(), 3); + // `MerryConfig::host_integrations()` parsing is covered by the config + // module; this test owns the mapping into the action backend options. let options = action_process_backend_options(Some(&config)).unwrap(); - assert!(options.host_integrations().is_empty()); + assert_eq!( + options.host_integrations(), + [ + HostIntegration::SshAgent, + HostIntegration::SessionBus, + HostIntegration::GpgAgent, + ] + ); + + let unconfigured = + MerryConfig::load_optional_from_text(Some("[permissions]\nnetwork = true\n"), &paths) + .unwrap() + .unwrap(); + let options = action_process_backend_options(Some(&unconfigured)).unwrap(); + assert!( + options.host_integrations().is_empty(), + "only integrations enabled by trusted config may be preauthorized" + ); } #[test] diff --git a/crates/merry-cli/src/sandbox/tests.rs b/crates/merry-cli/src/sandbox/tests.rs index dc03bfe1..a1f40ad4 100644 --- a/crates/merry-cli/src/sandbox/tests.rs +++ b/crates/merry-cli/src/sandbox/tests.rs @@ -1,3 +1,5 @@ +#[cfg(target_os = "linux")] +use crate::{config::MerryConfig, sandbox::host::current_process_uid}; use crate::{ config::XdgPaths, sandbox::{ @@ -7,10 +9,13 @@ use crate::{ os, plan_bootstrap_with_file_exists, plan_bootstrap_with_probe, }, }; +use merry_runtime::{PathAccess, PathAccessRule, PathAccessRuleSource}; use std::{ collections::BTreeMap, path::{Path, PathBuf}, }; +#[cfg(target_os = "linux")] +use std::{env, fs}; #[derive(Default)] struct FakeHostProbe { @@ -94,6 +99,45 @@ fn path_is_fake_bwrap(path: &Path) -> bool { path == Path::new("/custom/bin/bwrap") } +/// Builds the host fixture an integration test re-enters with: a private home +/// holding `permissions`, the current test binary as the sandbox command, and +/// the same config the child half reloads through its own XDG paths. +#[cfg(target_os = "linux")] +fn integration_host(home: &Path, workspace: &Path, permissions: &str) -> Host { + let mut host = sandbox_host(); + host.cwd = workspace.to_path_buf(); + host.current_exe = env::current_exe().unwrap(); + host.path = Some(os("/usr/bin:/bin")); + host.args.clear(); + host.current_uid = current_process_uid().unwrap(); + host.xdg_paths = XdgPaths::from_parts(home.to_path_buf(), None, None); + fs::create_dir_all(host.xdg_paths.config_dir()).unwrap(); + fs::write(host.xdg_paths.config_file(), permissions).unwrap(); + let config = MerryConfig::load_optional(&host.xdg_paths) + .unwrap() + .unwrap(); + host.host_integrations = config.host_integrations(); + host.trusted_path_rules = config.trusted_global_path_rules().unwrap(); + host.trusted_path_rules.push(PathAccessRule::new( + &host.current_exe, + PathAccess::ReadOnly, + PathAccessRuleSource::TrustedGlobalConfig, + )); + host +} + +/// Re-enters the test binary inside `plan`'s sandbox and asserts the named child +/// test reported exactly one passing test. +#[cfg(target_os = "linux")] +fn assert_sandbox_child_ran(plan: &mut Plan, host: &Host, marker: &str, test_path: &str) { + plan.args.extend(reentry::sandboxed_reentry_arguments( + marker, + &host.current_exe, + test_path, + )); + reentry::assert_child_passed(&reentry::run_plan(plan), test_path); +} + fn plan_sandbox(with_sandbox: bool, host: &Host) -> Result { plan_bootstrap_with_file_exists(with_sandbox, host, path_is_fake_bwrap) } @@ -154,6 +198,9 @@ mod runtime_evidence; #[cfg(target_os = "linux")] mod mount_execution; +#[cfg(target_os = "linux")] +mod reentry; + #[cfg(target_os = "linux")] mod gpg_public; diff --git a/crates/merry-cli/src/sandbox/tests/gpg_public.rs b/crates/merry-cli/src/sandbox/tests/gpg_public.rs index d1356de4..9656851c 100644 --- a/crates/merry-cli/src/sandbox/tests/gpg_public.rs +++ b/crates/merry-cli/src/sandbox/tests/gpg_public.rs @@ -1,25 +1,25 @@ -use super::sandbox_host; +use super::{assert_sandbox_child_ran, integration_host, reentry}; use crate::{ coding::ProcessExecutionMode, config::{MerryConfig, XdgPaths}, runtime_config::prepared_action_process_backend_options, - sandbox::{Bootstrap, ClipboardAccess, host::current_process_uid, os, plan_bootstrap}, + sandbox::{Bootstrap, ClipboardAccess, plan_bootstrap}, }; use merry_core::{PendingToolCall, ToolCallArguments, ToolCallId, ToolName}; use merry_process::{GpgAgentSockets, LocalProcessBackend, ProcessBackend, ProcessBackendMode}; use merry_runtime::{ - HostIntegration, PathAccess, PathAccessRule, PathAccessRuleSource, PermissionedAction, - ProcessRunnerContext, parse_permission_request, + HostIntegration, PermissionedAction, ProcessRunnerContext, parse_permission_request, }; -use std::{env, ffi::OsStr, fs, os::unix::net::UnixListener, process::Command}; +use std::{env, fs, os::unix::net::UnixListener, process::Command}; use tokio_util::sync::CancellationToken; -const CHILD_ENV: &str = "MERRY_GPG_PUBLIC_TEST_CHILD"; +const CHILD_MARKER: &str = "MERRY_GPG_PUBLIC_TEST_CHILD"; +const CHILD_TEST: &str = "sandbox::tests::gpg_public::gpg_agent_config_preauthorizes_inner_actions_through_both_sandboxes"; const PUBLIC_IDENTITY: &str = "Merry Sandbox Fixture "; #[test] -fn gpg_agent_config_requires_review_through_both_sandboxes() { - if env::var_os(CHILD_ENV).as_deref() == Some(OsStr::new("1")) { +fn gpg_agent_config_preauthorizes_inner_actions_through_both_sandboxes() { + if reentry::is_child(CHILD_MARKER) { tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -49,63 +49,17 @@ fn gpg_agent_config_requires_review_through_both_sandboxes() { "not a private key", ) .unwrap(); - let mut host = sandbox_host(); - host.cwd = workspace; - host.current_exe = env::current_exe().unwrap(); - host.path = Some(os("/usr/bin:/bin")); - host.args.clear(); - host.current_uid = current_process_uid().unwrap(); - host.xdg_paths = XdgPaths::from_parts(home, None, None); - fs::create_dir_all(host.xdg_paths.config_dir()).unwrap(); - fs::write( - host.xdg_paths.config_file(), - "[permissions]\ngpg_agent = true\n", - ) - .unwrap(); - let config = MerryConfig::load_optional(&host.xdg_paths) - .unwrap() - .unwrap(); - host.host_integrations = config.host_integrations(); + let mut host = integration_host(&home, &workspace, "[permissions]\ngpg_agent = true\n"); assert_eq!(host.host_integrations, vec![HostIntegration::GpgAgent]); host.host_integration_environment.gpg_agent_sockets = Some(GpgAgentSockets::new(&keyring, agent).unwrap()); - host.trusted_path_rules = vec![PathAccessRule::new( - &host.current_exe, - PathAccess::ReadOnly, - PathAccessRuleSource::TrustedGlobalConfig, - )]; let Bootstrap::Reexec(mut plan) = plan_bootstrap(true, ClipboardAccess::Disabled, &host).unwrap() else { panic!("expected outer sandbox"); }; - let command_index = plan - .args - .iter() - .rposition(|argument| argument == host.current_exe.as_os_str()) - .unwrap(); - plan.args.truncate(command_index); - plan.args.extend([ - os("--setenv"), - os(CHILD_ENV), - os("1"), - host.current_exe.into_os_string(), - os("--exact"), - os("sandbox::tests::gpg_public::gpg_agent_config_requires_review_through_both_sandboxes"), - os("--nocapture"), - ]); - let output = Command::new(plan.program) - .args(plan.args) - .env_clear() - .envs(plan.env) - .output() - .expect("bubblewrap test dependency"); - assert!( - output.status.success(), - "{}\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); + reentry::truncate_before_command(&mut plan, &host.current_exe); + assert_sandbox_child_ran(&mut plan, &host, CHILD_MARKER, CHILD_TEST); assert_eq!(fs::read(keyring.join("pubring.kbx")).unwrap(), original); assert_eq!( fs::read_to_string(keyring.join("trustdb.gpg")).unwrap(), @@ -145,11 +99,14 @@ async fn assert_public_key_client() { let PermissionedAction::Process(intent) = request.action(); let factory = session.permissioned_factory(); assert!( - !factory + factory .request_capabilities_are_satisfied(&request) - .unwrap() + .unwrap(), + "trusted config must preauthorize the configured gpg agent" ); - let before = session + // No permission request is needed: the configured agent and its public + // keyring are already part of the inner action baseline. + let output = session .runner() .run( intent.clone(), @@ -157,29 +114,9 @@ async fn assert_public_key_client() { ) .await .unwrap(); - assert!( - !before.stdout_text().contains(PUBLIC_IDENTITY), - "{before:?}" - ); - factory.validate_request(&request).unwrap(); - let output = factory - .runner_for(&request) - .run( - intent.clone(), - ProcessRunnerContext::new(CancellationToken::new()), - ) - .await - .unwrap(); assert!(output.ok(), "{output:?}"); assert!(output.stdout_text().contains(PUBLIC_IDENTITY), "{output:?}"); assert!(!keyring.join("trustdb.gpg").exists()); - assert!( - !backend - .new_session() - .permissioned_factory() - .request_capabilities_are_satisfied(&request) - .unwrap() - ); } /// Returns only an anonymous public keybox; private keys stay on disposable tmpfs. diff --git a/crates/merry-cli/src/sandbox/tests/host_integrations.rs b/crates/merry-cli/src/sandbox/tests/host_integrations.rs index 0e4d526a..ab039bd7 100644 --- a/crates/merry-cli/src/sandbox/tests/host_integrations.rs +++ b/crates/merry-cli/src/sandbox/tests/host_integrations.rs @@ -49,7 +49,7 @@ fn non_tui_sandbox_ignores_valid_graphical_endpoints() { } #[test] -fn sandbox_exposes_configured_host_integrations_as_outer_ceiling() { +fn outer_sandbox_forwards_configured_host_integrations() { let mut host = sandbox_host(); host.host_integrations = vec![HostIntegration::SshAgent, HostIntegration::SessionBus]; host.host_integration_environment = HostIntegrationEnvironment { diff --git a/crates/merry-cli/src/sandbox/tests/mount_execution.rs b/crates/merry-cli/src/sandbox/tests/mount_execution.rs index 05d359f3..29ec2b26 100644 --- a/crates/merry-cli/src/sandbox/tests/mount_execution.rs +++ b/crates/merry-cli/src/sandbox/tests/mount_execution.rs @@ -1,3 +1,4 @@ +use super::reentry; use crate::sandbox::{ mounts::{MountOrigin, MountPlan, MountPlanError}, os, @@ -458,7 +459,9 @@ fn cyclic_links_in_an_imported_parent_fail_during_planning() { #[test] fn outer_and_inner_preserve_system_reads_and_git_admission() { const CHILD_ENV: &str = "MERRY_BWRAP_MOUNT_TEST_CHILD"; - if std::env::var_os(CHILD_ENV).as_deref() == Some(std::ffi::OsStr::new("1")) { + const CHILD_TEST: &str = + "sandbox::tests::mount_execution::outer_and_inner_preserve_system_reads_and_git_admission"; + if reentry::is_child(CHILD_ENV) { tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -493,20 +496,11 @@ fn outer_and_inner_preserve_system_reads_and_git_admission() { false, MountOrigin::Workspace, ); - let command = vec![ - executable.into_os_string(), - os("--exact"), - os( - "sandbox::tests::mount_execution::outer_and_inner_preserve_system_reads_and_git_admission", - ), - os("--nocapture"), - ]; - assert_success( - outer_command(mounts, &command) - .env(CHILD_ENV, "1") - .output() - .expect("outer sandbox"), - ); + let output = outer_command(mounts, &reentry::reentry_arguments(&executable, CHILD_TEST)) + .env(CHILD_ENV, "1") + .output() + .expect("outer sandbox"); + reentry::assert_child_passed(&output, CHILD_TEST); assert_eq!( fs::read_to_string(workspace.join(".git/config")).unwrap(), "approved" diff --git a/crates/merry-cli/src/sandbox/tests/reentry.rs b/crates/merry-cli/src/sandbox/tests/reentry.rs new file mode 100644 index 00000000..f741f552 --- /dev/null +++ b/crates/merry-cli/src/sandbox/tests/reentry.rs @@ -0,0 +1,143 @@ +//! Shared scaffolding for the sandbox tests that re-enter this test binary. +//! +//! These tests have two halves. The parent half prepares a sandbox plan or +//! command, replaces the re-executed command with [`sandboxed_reentry_arguments`] +//! or [`reentry_arguments`], runs it through [`run_plan`], and then checks the +//! result with [`assert_child_passed`]. The child half recognizes itself with +//! [`is_child`] and does the work that only makes sense inside the sandbox. +//! +//! The parent must not settle for the child's exit status. The test harness +//! exits successfully when `--exact` matches no test, so a renamed test or a +//! stale filter would pass as an empty run; [`assert_child_passed`] requires the +//! harness to report the named test as one passing test. + +use crate::sandbox::{Plan, os}; +use std::{ + ffi::{OsStr, OsString}, + path::Path, + process::{Command, Output}, +}; + +/// Returns true when the current process is the sandboxed child test. +pub(super) fn is_child(marker: &str) -> bool { + std::env::var_os(marker).as_deref() == Some(OsStr::new("1")) +} + +/// Runner arguments that select exactly one test in the test binary at +/// `executable`, with no sandbox prefix. Use +/// [`sandboxed_reentry_arguments`] when bubblewrap has to install the marker. +pub(super) fn reentry_arguments(executable: &Path, test_path: &str) -> Vec { + vec![ + executable.into(), + os("--exact"), + os(test_path), + os("--nocapture"), + ] +} + +/// Removes the command a prepared plan would have run, keeping its sandbox +/// arguments, so the caller can supply the real command. +pub(super) fn truncate_before_command(plan: &mut Plan, executable: &Path) { + let command_index = plan + .args + .iter() + .rposition(|argument| argument == executable.as_os_str()) + .expect("outer sandbox plan re-executes the test binary"); + plan.args.truncate(command_index); +} + +/// Bubblewrap arguments that re-enter the test binary at `executable`, running +/// exactly `test_path` with `marker` set. +/// +/// The marker reaches the child through bubblewrap's `--setenv`, so the child +/// half can recognize itself without inheriting parent state. +pub(super) fn sandboxed_reentry_arguments( + marker: &str, + executable: &Path, + test_path: &str, +) -> Vec { + let mut command = vec![os("--setenv"), os(marker), os("1")]; + command.extend(reentry_arguments(executable, test_path)); + command +} + +/// Runs a prepared plan with the environment and descriptors the sandbox +/// command expects. +pub(super) fn run_plan(plan: &Plan) -> Output { + let mut command = Command::new(&plan.program); + command + .args(&plan.args) + .env_clear() + .envs(plan.env.iter().cloned()); + plan.ssh_config + .configure_command(&mut command) + .expect("SSH configuration snapshot"); + command.output().expect("bubblewrap test dependency") +} + +/// Asserts the sandboxed child ran exactly `test_path` and reported success. +/// +/// Fails closed on the empty selection produced by a stale test path, and +/// surfaces the child's own harness output when the test itself failed. +pub(super) fn assert_child_passed(output: &Output, test_path: &str) { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stdout}\n{stderr}"); + assert!( + stdout.contains(&format!("test {test_path} ... ok")), + "the sandboxed child did not run {test_path}; check that the test name still exists: {stdout}\n{stderr}" + ); + assert!( + stdout + .lines() + .any(|line| line.starts_with("test result: ok. 1 passed; 0 failed;")), + "the sandboxed child did not report exactly one passing test: {stdout}\n{stderr}" + ); +} + +#[cfg(test)] +mod assert_child_passed_tests { + use super::assert_child_passed; + use std::{ + os::unix::process::ExitStatusExt, + process::{ExitStatus, Output}, + }; + + const TEST_PATH: &str = "sandbox::tests::group::expected_child"; + + fn harness_output(status: i32, stdout: &str) -> Output { + Output { + status: ExitStatus::from_raw(status), + stdout: stdout.as_bytes().to_vec(), + stderr: Vec::new(), + } + } + + /// `cargo test --exact ` exits successfully after running zero + /// tests, so a renamed child must not look like a passing one. + #[test] + #[should_panic(expected = "did not run")] + fn empty_selection_is_not_a_pass() { + assert_child_passed( + &harness_output( + 0, + "running 0 tests\n\ntest result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 630 filtered out; finished in 0.00s\n", + ), + TEST_PATH, + ); + } + + /// A substring check would accept any count ending in `1`, so the count is + /// matched from the start of the harness line. + #[test] + #[should_panic(expected = "did not report exactly one passing test")] + fn more_than_one_passing_test_is_not_a_single_child() { + assert_child_passed( + &harness_output( + 0, + "test sandbox::tests::group::expected_child ... ok\n\ntest result: ok. 31 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s\n", + ), + TEST_PATH, + ); + } +} diff --git a/crates/merry-cli/src/sandbox/tests/ssh.rs b/crates/merry-cli/src/sandbox/tests/ssh.rs index f340c536..9a4b6ec5 100644 --- a/crates/merry-cli/src/sandbox/tests/ssh.rs +++ b/crates/merry-cli/src/sandbox/tests/ssh.rs @@ -1,9 +1,9 @@ -use super::sandbox_host; +use super::{assert_sandbox_child_ran, integration_host, reentry}; use crate::{ coding::ProcessExecutionMode, config::{MerryConfig, XdgPaths}, runtime_config::prepared_action_process_backend_options, - sandbox::{Bootstrap, ClipboardAccess, host::current_process_uid, os, plan_bootstrap}, + sandbox::{Bootstrap, ClipboardAccess, plan_bootstrap}, }; use merry_core::{PendingToolCall, ToolCallArguments, ToolCallId, ToolName}; use merry_process::{LocalProcessBackend, ProcessBackend, ProcessBackendMode}; @@ -11,15 +11,17 @@ use merry_runtime::{ PathAccess, PathAccessRule, PathAccessRuleSource, PermissionedAction, ProcessRunnerContext, parse_permission_request, }; -use std::{env, ffi::OsStr, fs, os::unix::net::UnixListener, process::Command}; +use std::{env, fs, os::unix::net::UnixListener, process::Command}; use tokio_util::sync::CancellationToken; -const CHILD_ENV: &str = "MERRY_SSH_CONFIG_TEST_CHILD"; +const CHILD_MARKER: &str = "MERRY_SSH_CONFIG_TEST_CHILD"; +const CHILD_TEST: &str = + "sandbox::tests::ssh::ssh_agent_config_preauthorizes_inner_actions_through_both_sandboxes"; const HOST_KEY: &str = "sandbox.example ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"; #[test] -fn ssh_agent_config_requires_review_through_both_sandboxes() { - if env::var_os(CHILD_ENV).as_deref() == Some(OsStr::new("1")) { +fn ssh_agent_config_preauthorizes_inner_actions_through_both_sandboxes() { + if reentry::is_child(CHILD_MARKER) { tokio::runtime::Builder::new_current_thread() .enable_all() .build() @@ -50,68 +52,27 @@ fn assert_ssh_sandboxes(expose_etc: bool) { fs::write(ssh.join("config"), "invalid-host-only-config").unwrap(); let original = fs::read("/etc/ssh/ssh_config").expect("openssh-client configuration"); let original_metadata = fs::metadata("/etc/ssh/ssh_config").unwrap(); - let mut host = sandbox_host(); - host.cwd = workspace; - host.current_exe = env::current_exe().unwrap(); - host.path = Some(os("/usr/bin:/bin")); - host.args.clear(); - host.current_uid = current_process_uid().unwrap(); - host.xdg_paths = XdgPaths::from_parts(home, None, None); - fs::create_dir_all(host.xdg_paths.config_dir()).unwrap(); - fs::write( - host.xdg_paths.config_file(), + let mut host = integration_host( + &home, + &workspace, if expose_etc { "[permissions]\nssh_agent = true\nreadonly_paths = [\"/etc\"]\n" } else { "[permissions]\nssh_agent = true\n" }, - ) - .unwrap(); - let config = MerryConfig::load_optional(&host.xdg_paths) - .unwrap() - .unwrap(); - host.host_integrations = config.host_integrations(); + ); host.host_integration_environment.ssh_agent_socket = Some(agent); - host.trusted_path_rules = config.trusted_global_path_rules().unwrap(); - host.trusted_path_rules.push(PathAccessRule::new( - &host.current_exe, - PathAccess::ReadOnly, - PathAccessRuleSource::TrustedGlobalConfig, - )); let Bootstrap::Reexec(mut plan) = plan_bootstrap(true, ClipboardAccess::Disabled, &host).unwrap() else { panic!("outer sandbox plan"); }; - let command_index = plan - .args - .iter() - .rposition(|argument| argument == host.current_exe.as_os_str()) - .unwrap(); - plan.args.truncate(command_index); + reentry::truncate_before_command(&mut plan, &host.current_exe); assert_ssh_bootstrap(&plan, expose_etc); if !expose_etc { assert_ssh_bootstrap_restrictions(&host); } - plan.args.extend([ - os("--setenv"), - os(CHILD_ENV), - os("1"), - host.current_exe.into_os_string(), - os("--exact"), - os("sandbox::tests::ssh::ssh_agent_config_requires_review_through_both_sandboxes"), - os("--nocapture"), - ]); - let mut command = Command::new(plan.program); - command.args(plan.args).env_clear().envs(plan.env); - plan.ssh_config.configure_command(&mut command).unwrap(); - let output = command.output().unwrap(); - assert!( - output.status.success(), - "{}\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); + assert_sandbox_child_ran(&mut plan, &host, CHILD_MARKER, CHILD_TEST); assert_eq!(fs::read("/etc/ssh/ssh_config").unwrap(), original); assert_eq!( fs::metadata("/etc/ssh/ssh_config").unwrap().permissions(), @@ -177,12 +138,7 @@ fn assert_ssh_bootstrap_restrictions(host: &crate::sandbox::host::Host) { else { panic!("expected sandbox reexec plan"); }; - let command_index = plan - .args - .iter() - .rposition(|argument| argument == host.current_exe.as_os_str()) - .unwrap(); - plan.args.truncate(command_index); + reentry::truncate_before_command(&mut plan, &host.current_exe); let output = bootstrap_output( &plan, r#" @@ -267,11 +223,14 @@ async fn assert_ssh_client() { let PermissionedAction::Process(intent) = request.action(); let factory = session.permissioned_factory(); assert!( - !factory + factory .request_capabilities_are_satisfied(&request) - .unwrap() + .unwrap(), + "trusted config must preauthorize the configured ssh agent" ); - let before = session + // No permission request is needed: the configured agent is already part of + // the inner action baseline. + let output = session .runner() .run( intent.clone(), @@ -279,16 +238,6 @@ async fn assert_ssh_client() { ) .await .unwrap(); - assert!(!before.ok(), "{before:?}"); - factory.validate_request(&request).unwrap(); - let output = factory - .runner_for(&request) - .run( - intent.clone(), - ProcessRunnerContext::new(CancellationToken::new()), - ) - .await - .unwrap(); assert!(output.ok(), "{output:?}"); assert!( output.stdout_text().contains("sandbox.example ssh-ed25519"), @@ -301,11 +250,4 @@ async fn assert_ssh_client() { )), "{output:?}" ); - assert!( - !backend - .new_session() - .permissioned_factory() - .request_capabilities_are_satisfied(&request) - .unwrap() - ); } diff --git a/crates/merry-coding/src/lib.rs b/crates/merry-coding/src/lib.rs index 1d3e3d82..252514ec 100644 --- a/crates/merry-coding/src/lib.rs +++ b/crates/merry-coding/src/lib.rs @@ -10,6 +10,7 @@ mod child_runtime; mod project_capabilities; mod project_rules; mod runtime; +mod search_tools; mod workspace; #[cfg(test)] @@ -55,9 +56,9 @@ pub const CODING_AGENT_DYNAMIC_CONTEXT_LAYOUT: &str = pub const CODING_AGENT_POLICY_PROMPT: &str = r#" This is a coding-agent run. Inspect the repository and its governing rules before changing files. Keep runtime state, task progress, artifacts, checkpoints, permissions, and tool results in their owning runtime contracts; do not treat a raw transcript as the source of truth. -Use the registered file and process tools according to their typed schemas. Use `read_text` for a bounded line range from a known text file; never request complete-file content when a focused range is enough. Use `run_process` for repository discovery and verification when it is available, preferring bounded commands such as `rg --files`, a focused `rg` search, or `sed -n ',p'`. Avoid broad recursive output, `cat` on large files, and repeated exploratory calls. Use `apply_patch` for edits, keep hunks localized, and include only the smallest unique context needed. Permission, phase, role, and path scope are runtime admission decisions; do not invent tools or request broader capability than the exact action needs. +Use the registered file and process tools according to their typed schemas. Use `read_text` for a bounded line range from a known text file; never request complete-file content when a focused range is enough. Use `run_process` for repository discovery and verification when it is available, preferring the installed modern search tools the workspace capability facts report, bounded commands such as `rg --files`, a focused literal `rg` search, or `sed -n ',p'`. Avoid broad recursive output, `cat` on large files, and repeated exploratory calls. Use `apply_patch` for edits, keep hunks localized, and include only the smallest unique context needed. Permission, phase, role, and path scope are runtime admission decisions; do not invent tools or request broader capability than the exact action needs. -If a process command is known to need network, extra filesystem, or host-integration access, include all required capabilities in that same `run_process` call under `permissions`; Merry reviews them before execution and runs the exact command through the permissioned backend when approved. Use `reason` to explain the minimum required access. Use `request_permissions` only when the needed capability is discovered after a failed sandboxed attempt or when the action is not being retried through `run_process`. +A sandboxed action starts with no network access and no access beyond what trusted global configuration already granted. Decide what a command needs before running it rather than after it fails: a command that authenticates, installs, downloads, publishes, or otherwise reaches a remote service needs network requested in the same `run_process` call, while a command that only touches the workspace and the configured local baseline needs no additional capability. Paths and host integrations enabled by trusted global configuration are already available to sandboxed commands. If a process command needs access that is still missing, such as network, a reviewed path, or an unconfigured host integration, include all required capabilities in that same `run_process` call under `permissions`; Merry reviews them before execution and runs the exact command through the permissioned backend when approved. Use `reason` to explain the minimum required access. Use `request_permissions` only when the needed capability is discovered after a failed sandboxed attempt or when the action is not being retried through `run_process`. Treat the sandbox as the first explanation when a process command fails: a capability it withheld often surfaces as a credentials, authentication, or connectivity error, so re-run the same action with the missing capability before concluding anything about the user's machine, account, or local setup. When a tool fails, preserve the failure evidence, determine whether the cause is validation, missing permission, unavailable capability, or an implementation error, and then either make a bounded recovery attempt or report the blocker. Do not repeat an identical failed action without new evidence or an explicit reviewed admission. diff --git a/crates/merry-coding/src/search_tools.rs b/crates/merry-coding/src/search_tools.rs new file mode 100644 index 00000000..5ca1d4b8 --- /dev/null +++ b/crates/merry-coding/src/search_tools.rs @@ -0,0 +1,94 @@ +//! Action-PATH probe for the search tools the coding prompt prefers. +//! +//! The prompt asks for modern search tools because their output is easier to +//! scope and bound than `grep -r` or `find`. Merry probes the PATH that +//! sandboxed actions inherit while it composes the coding profile, so the model +//! is told which of those tools this machine actually has instead of spending +//! an action on a binary that is not installed. + +use merry_process::action_process_path; +use std::{env, ffi::OsStr, path::Path}; + +/// Program names probed on the action PATH, in prompt preference order. +const RIPGREP_PROGRAM: &str = "rg"; +const FD_PROGRAMS: [&str; 2] = ["fd", "fdfind"]; + +/// Which preferred search tools the action PATH resolves to. +/// +/// The fields are private so callers describe availability through +/// [`SearchToolAvailability::summary_line`] instead of re-deriving prompt text. +#[derive(Debug, Clone, Copy)] +pub(crate) struct SearchToolAvailability { + ripgrep: bool, + fd: bool, +} + +impl SearchToolAvailability { + /// Probes `path` for the preferred search tools. + /// + /// `file_exists` decides whether one candidate is runnable, which keeps the + /// probe deterministic in tests and lets callers reuse the host file check + /// they already rely on. + pub(crate) fn detect(path: &OsStr, mut file_exists: impl FnMut(&Path) -> bool) -> Self { + Self { + ripgrep: program_is_on_path(path, &[RIPGREP_PROGRAM], &mut file_exists), + fd: program_is_on_path(path, &FD_PROGRAMS, &mut file_exists), + } + } + + /// Probes the PATH that sandboxed actions inherit from this process. + pub(crate) fn probe_action_path() -> Self { + Self::detect(&action_process_path(), is_executable_file) + } + + /// Model-visible facts line appended to the workspace capability summary. + /// + /// The line always starts with the same marker so the prompt reports a + /// definite answer for this host, including when no modern tool is present. + pub(crate) fn summary_line(&self) -> String { + match (self.ripgrep, self.fd) { + (true, true) => format!( + "Action PATH search tools: `{RIPGREP_PROGRAM}` and `fd` are installed; use them for content search and path lookup." + ), + (true, false) => format!( + "Action PATH search tools: `{RIPGREP_PROGRAM}` is installed; use it for content and file search. Neither `fd` nor `fdfind` is installed here, so use bounded `find` commands for path lookup by name." + ), + (false, true) => format!( + "Action PATH search tools: `fd` is installed; use it for path lookup by name. `{RIPGREP_PROGRAM}` is not installed here, so use bounded `grep -r` commands for content search." + ), + (false, false) => format!( + "Action PATH search tools: neither `{RIPGREP_PROGRAM}` nor `fd` is installed on the action PATH; use bounded `grep -r` and `find` commands and keep their output small." + ), + } + } +} + +fn program_is_on_path( + path: &OsStr, + names: &[&str], + file_exists: &mut impl FnMut(&Path) -> bool, +) -> bool { + env::split_paths(path) + .filter(|directory| !directory.as_os_str().is_empty()) + .any(|directory| names.iter().any(|name| file_exists(&directory.join(name)))) +} + +/// Whether one PATH candidate is a file this process could execute. +fn is_executable_file(candidate: &Path) -> bool { + let Ok(metadata) = candidate.metadata() else { + return false; + }; + if !metadata.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + metadata.permissions().mode() & 0o111 != 0 + } + #[cfg(not(unix))] + { + true + } +} diff --git a/crates/merry-coding/src/tests.rs b/crates/merry-coding/src/tests.rs index f16813cb..df09c50f 100644 --- a/crates/merry-coding/src/tests.rs +++ b/crates/merry-coding/src/tests.rs @@ -55,3 +55,5 @@ mod composition; mod process_policy; mod profile_contract; + +mod search_tools; diff --git a/crates/merry-coding/src/tests/profile_contract.rs b/crates/merry-coding/src/tests/profile_contract.rs index b88aa889..2f24e29f 100644 --- a/crates/merry-coding/src/tests/profile_contract.rs +++ b/crates/merry-coding/src/tests/profile_contract.rs @@ -88,9 +88,30 @@ fn coding_agent_profile_owns_process_permission_and_patch_order() { .description() .contains("runtime will review before running this exact command") ); + assert!(process.spec().description().contains( + "Paths and host integrations enabled by trusted global configuration are already available" + )); + assert!( + process + .spec() + .description() + .contains("Network is never granted implicitly") + ); let schema = process.spec().input_schema().as_schema().as_value(); assert_eq!(schema["required"], json!(["command"])); assert!(schema["properties"]["permissions"].is_object()); + assert!( + schema["properties"]["permissions"]["properties"]["host_integrations"]["description"] + .as_str() + .expect("host integration description should be text") + .contains("Integrations enabled by trusted global configuration are already available") + ); + assert!( + schema["properties"]["permissions"]["properties"]["network"]["description"] + .as_str() + .expect("network description should be text") + .contains("Network is never granted implicitly") + ); } #[test] @@ -224,6 +245,21 @@ fn coding_agent_profile_owns_the_coding_prompt_and_hashes_its_exact_text() { .text() .contains("discovered after a failed sandboxed attempt") ); + assert!( + prompt.stable_blocks()[0] + .text() + .contains("already available to sandboxed commands") + ); + assert!( + prompt.stable_blocks()[0] + .text() + .contains("starts with no network access") + ); + assert!( + prompt.stable_blocks()[0] + .text() + .contains("preferring the installed modern search tools") + ); } #[test] diff --git a/crates/merry-coding/src/tests/search_tools.rs b/crates/merry-coding/src/tests/search_tools.rs new file mode 100644 index 00000000..6084a00a --- /dev/null +++ b/crates/merry-coding/src/tests/search_tools.rs @@ -0,0 +1,112 @@ +use crate::{coding_agent, search_tools::SearchToolAvailability}; +use std::{ + ffi::OsStr, + fs, + path::{Path, PathBuf}, +}; + +/// Builds a temporary PATH-style directory holding the requested programs. +fn tool_directory(programs: &[&str]) -> (tempfile::TempDir, String) { + let temp = tempfile::tempdir().expect("tempdir should be created"); + let bin = temp.path().join("bin"); + fs::create_dir_all(&bin).expect("bin directory should be created"); + for program in programs { + let path = bin.join(program); + fs::write(&path, b"#!/bin/sh\n").expect("program stub should be written"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)) + .expect("program stub should be executable"); + } + } + let path = bin.to_string_lossy().into_owned(); + (temp, path) +} + +#[test] +fn probe_reports_the_modern_search_tools_on_the_action_path() { + let (_temp, path) = tool_directory(&["rg", "fd"]); + let availability = SearchToolAvailability::detect(OsStr::new(&path), Path::exists); + let summary = availability.summary_line(); + + assert!(summary.contains("`rg` and `fd` are installed"), "{summary}"); + assert!(!summary.contains("not installed"), "{summary}"); +} + +#[test] +fn probe_accepts_the_distribution_name_of_fd() { + let (_temp, path) = tool_directory(&["rg", "fdfind"]); + let availability = SearchToolAvailability::detect(OsStr::new(&path), Path::exists); + + assert!( + availability + .summary_line() + .contains("`rg` and `fd` are installed") + ); +} + +#[test] +fn probe_names_the_missing_tool_instead_of_promising_it() { + let (_temp, path) = tool_directory(&["rg"]); + let summary = SearchToolAvailability::detect(OsStr::new(&path), Path::exists).summary_line(); + + assert!(summary.contains("`rg` is installed"), "{summary}"); + assert!( + summary.contains("Neither `fd` nor `fdfind` is installed"), + "{summary}" + ); +} + +#[test] +fn probe_reports_that_no_modern_search_tool_is_installed() { + let (_temp, path) = tool_directory(&[]); + let summary = SearchToolAvailability::detect(OsStr::new(&path), Path::exists).summary_line(); + + assert!( + summary.starts_with("Action PATH search tools: neither `rg` nor `fd` is installed"), + "{summary}" + ); +} + +#[test] +fn probe_builds_candidates_only_from_named_path_directories() { + let (_temp, path) = tool_directory(&["rg"]); + let mut probed = Vec::::new(); + let mixed_path = format!("::{path}:"); + let availability = SearchToolAvailability::detect(OsStr::new(&mixed_path), |candidate| { + probed.push(candidate.to_path_buf()); + candidate.exists() + }); + + assert!( + availability.summary_line().contains("`rg` is installed"), + "{probed:?}" + ); + assert_eq!( + probed, + vec![ + Path::new(&path).join("rg"), + Path::new(&path).join("fd"), + Path::new(&path).join("fdfind"), + ] + ); +} + +#[test] +fn coding_profile_reports_search_tool_availability_for_this_host() { + let temp = tempfile::tempdir().expect("tempdir should be created"); + let profile = coding_agent(temp.path()) + .build() + .expect("coding-agent profile should build"); + let summary = profile + .runtime_profile() + .initial_context_summaries() + .get("project-capabilities") + .expect("profile should seed the project capability summary") + .clone(); + + assert!(summary.contains("Coding file capabilities:"), "{summary}"); + assert!(summary.contains("Action PATH search tools:"), "{summary}"); +} diff --git a/crates/merry-coding/src/workspace.rs b/crates/merry-coding/src/workspace.rs index 44b48038..d6842f5c 100644 --- a/crates/merry-coding/src/workspace.rs +++ b/crates/merry-coding/src/workspace.rs @@ -1,4 +1,7 @@ -use crate::{CODING_LOOP_PROCESS_TOOL, project_capabilities::project_capability_summary_for_root}; +use crate::{ + CODING_LOOP_PROCESS_TOOL, project_capabilities::project_capability_summary_for_root, + search_tools::SearchToolAvailability, +}; use merry_core::ToolName; use merry_llm::ModelRetryPolicy; use merry_process::ProcessSession; @@ -15,8 +18,16 @@ use std::{path::PathBuf, sync::Arc}; use thiserror::Error; const PROJECT_CAPABILITY_CONTEXT_ID: &str = "project-capabilities"; -const CODING_WORKSPACE_CAPABILITY_SUMMARY: &str = "\ -Coding file capabilities:\n- `read_text` reads a bounded one-based line range from a known UTF-8 text path. Omit the range only to use the small configured default; use multiple focused reads instead of requesting a whole file. Paths are relative to configured roots, and skill/resource roots are read-only and separate from write scope.\n- `apply_patch` is the only file-edit tool. Use one patch envelope with localized Add File or Update File hunks; do not submit whole-file content for a small edit. Runtime admission, write scope, forbidden paths, and current-file preimages are enforced before writes.\n- `run_process` is the discovery and verification lane when configured. Prefer `rg --files`, focused literal `rg` searches, and bounded `sed -n ',p'` reads. Avoid broad recursive output and unbounded file reads.\n- Process execution runs through Merry runtime policy and the configured sandbox/profile, so filesystem and network access may be intentionally restricted; environment and host IPC access may also be intentionally restricted. If a command is known to need extra network, filesystem, or host-integration access, put all minimum required capabilities in that same `run_process` call under `permissions`; runtime reviews before executing the exact command through the permissioned backend.\n- If a capability is discovered only after a sandboxed failure, call `request_permissions` for that exact action before retrying it. Approved paths and host integrations remain available for later actions in this runtime session; network access must be requested again for every action that needs it.\n- Linux Unix sockets are filesystem paths. If a host resource is not represented by a named integration, request its exact socket/file path through `permissions.paths` or `requested.paths`; the outer sandbox must already expose the path.\n- `request_permissions` must name the exact planned action and request only the minimum needed capability; the runtime may approve, deny, or fail the request."; +const CODING_WORKSPACE_CAPABILITY_SUMMARY: &str = concat!( + "Coding file capabilities:\n", + "- `read_text` reads a bounded one-based line range from a known UTF-8 text path. Omit the range only to use the small configured default; use multiple focused reads instead of requesting a whole file. Paths are relative to configured roots, and skill/resource roots are read-only and separate from write scope.\n", + "- `apply_patch` is the only file-edit tool. Use one patch envelope with localized Add File or Update File hunks; do not submit whole-file content for a small edit. Runtime admission, write scope, forbidden paths, and current-file preimages are enforced before writes.\n", + "- `run_process` is the discovery and verification lane when configured. Prefer the modern search tools the environment facts below report: `rg --files` to list files, a focused literal `rg` search for content, and `fd`/`fdfind` for paths by name; fall back to `grep -r` and `find` only for a tool the facts say is missing. Scope each search to the directories that own the behavior instead of the repository root, and exclude build output such as `target/`, `node_modules/`, and `.venv/`. Read files with bounded `sed -n ',p'`. Avoid broad recursive output, `cat` on large files, and repeated exploratory calls.\n", + "- Process execution runs through Merry runtime policy and the configured sandbox/profile, so filesystem and network access may be intentionally restricted; environment and host IPC access may also be intentionally restricted. Network is withheld from every action that does not request it, so a command that reaches a remote service needs `network: true` in the same call's `permissions`. Paths and host integrations enabled by trusted global configuration are already available to actions. If a command needs access that is still missing, such as network, a reviewed path, or an unconfigured endpoint, put all minimum required capabilities in that same `run_process` call under `permissions`; runtime reviews before executing the exact command through the permissioned backend.\n", + "- If a capability is discovered only after a sandboxed failure, call `request_permissions` for that exact action before retrying it. Approved paths and host integrations remain available for later actions in this runtime session; network access must be requested again for every action that needs it.\n", + "- Linux Unix sockets are filesystem paths. If a host resource is not represented by a named integration, request its exact socket/file path through `permissions.paths` or `requested.paths`; the outer sandbox must already expose the path.\n", + "- `request_permissions` must name the exact planned action and request only the minimum needed capability; the runtime may approve, deny, or fail the request.", +); #[derive(Clone)] pub(crate) enum WorkspaceProcessRunnerConfig { @@ -140,10 +151,22 @@ impl WorkspaceCodingProfileBuilder { .find_map(|root| project_capability_summary_for_root(root)); let workspace_tools = WorkspaceTools::new(config)?; - let capability_summary = project_summary.map_or_else( - || CODING_WORKSPACE_CAPABILITY_SUMMARY.to_owned(), - |facts| format!("{CODING_WORKSPACE_CAPABILITY_SUMMARY}\n{facts}"), + // The environment facts name what this host actually provides so the + // static summary can keep asking for modern search tools without + // promising a tool the sandbox does not have. + let search_tools = SearchToolAvailability::probe_action_path(); + tracing::debug!( + event = "coding.search_tools.probe", + availability = ?search_tools, + "coding profile probed action-PATH search tools" ); + let environment_facts = search_tools.summary_line(); + let mut summary_sections = vec![CODING_WORKSPACE_CAPABILITY_SUMMARY.to_owned()]; + if let Some(project_summary) = project_summary { + summary_sections.push(project_summary); + } + summary_sections.push(environment_facts); + let capability_summary = summary_sections.join("\n"); builder = builder .model_retry_policy(ModelRetryPolicy::coding_agent_default()) .progress_commentary(true) @@ -183,7 +206,7 @@ impl WorkspaceCodingProfileBuilder { let process_tool = process_command_tool( ToolName::new(CODING_LOOP_PROCESS_TOOL)?, - "Run one shell command through Merry's configured process runner. Provide command as one JSON string; omit cwd for the current workspace directory, or use null or a workspace-relative directory such as \".\". Do not add a bash field or pass argv. The runtime validates command and cwd byte/control-character limits. Workspace files and directories are writable in the sandbox; .git, external paths, network, and host integrations begin restricted and may require a reviewed capability. If the command is known to need extra access, include the minimum capabilities in permissions and runtime will review before running this exact command. If the need is discovered only after failure, use request_permissions for the exact same action before retrying it. Network access must be requested again for each action that needs it. Linux Unix sockets can be requested as exact filesystem paths when no named integration applies.", + "Run one shell command through Merry's configured process runner. Provide command as one JSON string; omit cwd for the current workspace directory, or use null or a workspace-relative directory such as \".\". Do not add a bash field or pass argv. The runtime validates command and cwd byte/control-character limits. Workspace files and directories are writable in the sandbox. Paths and host integrations enabled by trusted global configuration are already available; .git, network, and anything not configured remain restricted and may require a reviewed capability. Network is never granted implicitly: a command that reaches a remote service, such as one that authenticates, installs, downloads, or publishes, must include network in this call's permissions. If the command needs access that is still missing, include the minimum capabilities in permissions and runtime will review before running this exact command. If the need is discovered only after failure, use request_permissions for the exact same action before retrying it. Network access must be requested again for each action that needs it. Linux Unix sockets can be requested as exact filesystem paths when no named integration applies.", )?; builder = builder .register_tool(process_tool) diff --git a/crates/merry-process/src/lib.rs b/crates/merry-process/src/lib.rs index 2fd3b676..c3c80964 100644 --- a/crates/merry-process/src/lib.rs +++ b/crates/merry-process/src/lib.rs @@ -28,7 +28,7 @@ pub use sandbox_path::{SandboxPathError, resolve_sandbox_path}; #[cfg(target_os = "linux")] pub use ssh_config::BwrapSshConfigFiles; -pub use process_runner::TokioProcessRunner; +pub use process_runner::{TokioProcessRunner, action_process_path}; #[cfg(target_os = "linux")] pub use process_runner::{ @@ -165,7 +165,7 @@ pub struct ProcessBackendOptions { path_rules: Vec, /// Isolated-session capability ceiling, not a grant to an ordinary runner. network_requests_allowed: bool, - /// Host integrations already approved by the embedding application. + /// Host integrations the embedding application authorizes for isolated actions. host_integrations: Vec, gpg_agent_sockets: Option, /// Environment assignments validated and applied by the host backend. @@ -209,8 +209,15 @@ impl ProcessBackendOptions { self } - /// Sets host integrations already approved by the embedding application. - /// Availability or outer-sandbox forwarding alone is not an approval. + /// Sets host integrations the embedding application authorizes. + /// + /// The embedding application owns this decision; the backend treats the + /// listed integrations as part of the ordinary action baseline instead of + /// requiring a per-action request for them. Merry's CLI derives the list + /// from user-trusted global configuration, which preauthorizes the inner + /// action sandbox; project-local configuration never does. Endpoint + /// validation and path policy still apply per action, so a `deny_paths` or + /// `review_paths` entry can keep a listed integration masked. #[must_use] pub fn with_host_integrations( mut self, diff --git a/crates/merry-process/src/process_runner.rs b/crates/merry-process/src/process_runner.rs index 624037c6..e5ebcf05 100644 --- a/crates/merry-process/src/process_runner.rs +++ b/crates/merry-process/src/process_runner.rs @@ -14,7 +14,7 @@ mod review; mod sandbox; mod tokio_runner; -pub use environment::BwrapProcessEnvironment; +pub use environment::{BwrapProcessEnvironment, action_process_path}; pub use permissions::{ BwrapPermissionedProcessRunnerFactory, BwrapProcessRunner, BwrapSessionPermissions, }; diff --git a/crates/merry-process/src/process_runner/environment.rs b/crates/merry-process/src/process_runner/environment.rs index b6ba439d..621259f0 100644 --- a/crates/merry-process/src/process_runner/environment.rs +++ b/crates/merry-process/src/process_runner/environment.rs @@ -10,6 +10,22 @@ pub(super) const BWRAP_PROGRAM: &str = "bwrap"; pub(super) const ACTION_SANDBOX_HOME_FALLBACK: &str = "/home/merry"; pub(super) const ACTION_SANDBOX_TMPDIR: &str = "/tmp"; pub(super) const ACTION_SANDBOX_PATH_FALLBACK: &str = "/usr/local/bin:/usr/bin:/bin"; + +/// PATH value that Merry passes to sandboxed action commands. +/// +/// The value is the current process `PATH` when it is set and non-empty, +/// otherwise the action sandbox fallback used when a bubblewrap action +/// environment is built. Callers that describe the action environment, such as +/// a prompt naming the command-line tools a sandboxed command may use, resolve +/// the value through here so the description follows the PATH the sandbox +/// actually passes instead of guessing. +#[must_use] +pub fn action_process_path() -> OsString { + env::var_os("PATH") + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| OsString::from(ACTION_SANDBOX_PATH_FALLBACK)) +} + /// Host-derived paths used to construct one action sandbox. /// /// The HOME, PATH, and non-policy environment variables preserve the caller's @@ -31,6 +47,19 @@ pub struct BwrapProcessEnvironment { pub(super) gpg_agent_sockets: Option, } +/// One configured host integration whose endpoint validated for this action. +/// +/// The socket is the object a sandbox mounts read-only, and the environment +/// value is what the client needs next to it: the session bus address for +/// `dbus`, the private GnuPG home for `gpg-agent`, and nothing for the SSH +/// agent, whose client reads the socket path directly. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct HostIntegrationBinding { + pub(super) integration: HostIntegration, + pub(super) socket: PathBuf, + pub(super) environment_value: Option, +} + impl BwrapProcessEnvironment { /// Builds an environment layout from the current process environment. /// @@ -39,9 +68,7 @@ impl BwrapProcessEnvironment { /// validated process defaults and configured assignments. #[must_use] pub fn from_current_process() -> Self { - let path = env::var_os("PATH") - .filter(|value| !value.is_empty()) - .unwrap_or_else(|| OsString::from(ACTION_SANDBOX_PATH_FALLBACK)); + let path = action_process_path(); let home = absolute_env_path("HOME", ACTION_SANDBOX_HOME_FALLBACK); let tmp_source = absolute_env_path("TMPDIR", ACTION_SANDBOX_TMPDIR); Self { @@ -210,26 +237,30 @@ impl BwrapProcessEnvironment { ))); } } - for (_, socket, _) in self + for binding in self .host_integration_candidates() .into_iter() - .filter(|(integration, _, _)| integrations.contains(integration)) + .filter(|binding| integrations.contains(&binding.integration)) { - validate_host_socket(&socket)?; + validate_host_socket(&binding.socket)?; } Ok(()) } - pub(super) fn host_integration_candidates( - &self, - ) -> Vec<(HostIntegration, PathBuf, Option)> { + /// Returns every configured integration endpoint this environment can name, + /// whether or not it is enabled or its socket currently validates. + pub(super) fn host_integration_candidates(&self) -> Vec { let mut candidates = Vec::new(); if let Some(path) = self .ssh_agent_socket .as_ref() .filter(|path| is_clean_absolute_path(path)) { - candidates.push((HostIntegration::SshAgent, path.clone(), None)); + candidates.push(HostIntegrationBinding { + integration: HostIntegration::SshAgent, + socket: path.clone(), + environment_value: None, + }); } if let Some((socket, address)) = self .session_bus_address @@ -237,28 +268,38 @@ impl BwrapProcessEnvironment { .and_then(|address| session_bus_socket_path(address).map(|socket| (socket, address))) .filter(|(socket, _)| is_clean_absolute_path(socket)) { - candidates.push((HostIntegration::SessionBus, socket, Some(address.clone()))); + candidates.push(HostIntegrationBinding { + integration: HostIntegration::SessionBus, + socket, + environment_value: Some(address.clone()), + }); } if let Some(sockets) = &self.gpg_agent_sockets { - candidates.push(( - HostIntegration::GpgAgent, - sockets.agent().to_path_buf(), - Some(sockets.home().as_os_str().to_owned()), - )); + candidates.push(HostIntegrationBinding { + integration: HostIntegration::GpgAgent, + socket: sockets.agent().to_path_buf(), + environment_value: Some(sockets.home().as_os_str().to_owned()), + }); } candidates } - pub(super) fn host_integration_hidden_paths(&self) -> Vec { - let allowed = self - .host_integration_bindings() - .into_iter() - .map(|(_, path, _)| crate::resolve_bwrap_path(&path)) + /// Returns the masked paths for candidates this action did not enable. + /// + /// `bindings` is the result of [`Self::host_integration_bindings`] for the + /// same action, so one plan validates each endpoint exactly once. + pub(super) fn host_integration_hidden_paths( + &self, + bindings: &[HostIntegrationBinding], + ) -> Vec { + let allowed = bindings + .iter() + .map(|binding| crate::resolve_bwrap_path(&binding.socket)) .collect::>(); let mut hidden = self .host_integration_candidates() .into_iter() - .map(|(_, path, _)| path) + .map(|binding| binding.socket) .collect::>(); if let Some(sockets) = &self.gpg_agent_sockets { hidden.extend(sockets.paths().map(Path::to_path_buf)); @@ -269,13 +310,15 @@ impl BwrapProcessEnvironment { hidden } - pub(super) fn host_integration_bindings( - &self, - ) -> Vec<(HostIntegration, PathBuf, Option)> { + /// Returns the enabled integrations whose endpoints validated, in a stable + /// order. One plan resolves this once and shares it between mounts and the + /// client environment. + pub(super) fn host_integration_bindings(&self) -> Vec { self.host_integration_candidates() .into_iter() - .filter(|(integration, socket, _)| { - self.host_integrations.contains(integration) && validate_host_socket(socket).is_ok() + .filter(|binding| { + self.host_integrations.contains(&binding.integration) + && validate_host_socket(&binding.socket).is_ok() }) .collect() } diff --git a/crates/merry-process/src/process_runner/path_view.rs b/crates/merry-process/src/process_runner/path_view.rs index 065e55ac..3217e66d 100644 --- a/crates/merry-process/src/process_runner/path_view.rs +++ b/crates/merry-process/src/process_runner/path_view.rs @@ -49,11 +49,7 @@ impl ActionPathView { for rule in rules.iter().filter(|rule| { rule.access() == PathAccess::ReadOnly && !rule.review_required() - && matches!( - rule.source(), - PathAccessRuleSource::TrustedGlobalConfig - | PathAccessRuleSource::TrustedGlobalConfigWritableCeiling - ) + && rule.source() == PathAccessRuleSource::TrustedGlobalConfig }) { if rule.path().exists() { bindings.extend( diff --git a/crates/merry-process/src/process_runner/permissions.rs b/crates/merry-process/src/process_runner/permissions.rs index 549e7ee0..e72eb9ca 100644 --- a/crates/merry-process/src/process_runner/permissions.rs +++ b/crates/merry-process/src/process_runner/permissions.rs @@ -459,11 +459,7 @@ fn path_rule_covers( && rules.iter().any(|rule| { path_matches_rule(requested_path, rule.path()) && rule.access() == PathAccess::ReadOnly - && matches!( - rule.source(), - PathAccessRuleSource::TrustedGlobalConfig - | PathAccessRuleSource::TrustedGlobalConfigWritableCeiling - ) + && rule.source() == PathAccessRuleSource::TrustedGlobalConfig }) { return false; @@ -492,13 +488,7 @@ fn normalize_path_rules(rules: Vec) -> Vec { action_rules.push(rule); continue; } - let access = if rule.source() == PathAccessRuleSource::TrustedGlobalConfigWritableCeiling - && rule.access() == PathAccess::ReadWrite - { - PathAccess::ReadOnly - } else { - rule.access() - }; + let access = rule.access(); let entry = merged .entry(rule.path().to_path_buf()) .or_insert((access, rule.source())); diff --git a/crates/merry-process/src/process_runner/restricted_mounts.rs b/crates/merry-process/src/process_runner/restricted_mounts.rs index e1633f63..0582b1f3 100644 --- a/crates/merry-process/src/process_runner/restricted_mounts.rs +++ b/crates/merry-process/src/process_runner/restricted_mounts.rs @@ -38,11 +38,7 @@ pub(super) fn append( for rule in rules.iter().filter(|rule| { rule.access() == PathAccess::ReadOnly && !rule.review_required() - && matches!( - rule.source(), - PathAccessRuleSource::TrustedGlobalConfig - | PathAccessRuleSource::TrustedGlobalConfigWritableCeiling - ) + && rule.source() == PathAccessRuleSource::TrustedGlobalConfig }) { for (path, source) in aliases.action_paths(rule.path(), tmp_source) { let path = view.destination(&path)?; diff --git a/crates/merry-process/src/process_runner/sandbox.rs b/crates/merry-process/src/process_runner/sandbox.rs index 6f80607a..226cb902 100644 --- a/crates/merry-process/src/process_runner/sandbox.rs +++ b/crates/merry-process/src/process_runner/sandbox.rs @@ -154,13 +154,23 @@ pub(crate) fn bwrap_process_plan_with_environment( if let Some(sockets) = environment.gpg_client() { super::gpg_client::append(&mut args, sockets, cwd_root, &view)?; } - for (_, socket, _) in environment.host_integration_bindings() { - if view.visible(&socket) && view.visible(&resolve_bwrap_path(&socket)) { - append_bwrap_host_integration_mount_args(&mut args, &socket, &view)?; + // Path policy governs whether the endpoint is reachable, not whether an + // enabled integration is announced. A deny or review rule over the socket + // or one of its parent directories leaves the mount masked, while the + // client environment below still names the configured endpoint, so the + // failure appears at connect time instead of the capability silently + // disappearing. + // + // Resolving the enabled bindings once serves both the mounts below and the + // client environment, so each endpoint is validated exactly once per plan. + let host_integration_bindings = environment.host_integration_bindings(); + for binding in &host_integration_bindings { + if view.visible(&binding.socket) && view.visible(&resolve_bwrap_path(&binding.socket)) { + append_bwrap_host_integration_mount_args(&mut args, &binding.socket, &view)?; } } let aliases = &view.aliases; - for path in environment.host_integration_hidden_paths() { + for path in environment.host_integration_hidden_paths(&host_integration_bindings) { for (path, source) in aliases.action_paths(&path, &environment.tmp_source) { if source.exists() && view.visible(&path) { append_bwrap_hidden_host_integration_args(&mut args, &view.destination(&path)?); @@ -199,12 +209,12 @@ pub(crate) fn bwrap_process_plan_with_environment( os("--unsetenv"), os("DBUS_SESSION_BUS_ADDRESS"), ]); - for (integration, socket, address) in environment.host_integration_bindings() { + for binding in &host_integration_bindings { append_bwrap_host_integration_environment_args( &mut args, - integration, - &socket, - address.as_deref(), + binding.integration, + &binding.socket, + binding.environment_value.as_deref(), ); } args.push(os("--")); diff --git a/crates/merry-process/src/process_runner/tests/permissions.rs b/crates/merry-process/src/process_runner/tests/permissions.rs index b865f742..6edb85f6 100644 --- a/crates/merry-process/src/process_runner/tests/permissions.rs +++ b/crates/merry-process/src/process_runner/tests/permissions.rs @@ -1,666 +1,14 @@ -use super::{contains_sequence, intent, os_args, permission_request, request_process_intent}; -use crate::UnrestrictedPermissionedProcessRunnerFactory; -use crate::process_runner::{ - BwrapPermissionedProcessRunnerFactory, BwrapProcessEnvironment, BwrapProcessRunner, - BwrapSessionPermissions, TokioProcessRunner, bwrap_process_plan, -}; -use merry_runtime::{ - PathAccess, PathAccessRule, PathAccessRuleSource, PermissionedProcessRunnerFactory, - ProcessRunner, StaticPermissionedProcessRunnerFactory, -}; -use serde_json::json; -use std::path::PathBuf; -use std::sync::Arc; - -#[test] -fn bwrap_permissioned_factory_allows_network_only_when_requested() { - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap"); - let request_without_network = permission_request(json!({ - "requested": { - "paths": [{ "path": "/workspace/merry", "access": "rw" }] - }, - "for_action": { "command": "cargo test", "cwd": "." } - })); - let request_with_network = permission_request(json!({ - "requested": { "network": true }, - "for_action": { "command": "cargo test", "cwd": "." } - })); - - let runner_without_network = factory.build_runner(&request_without_network); - let plan_without_network = bwrap_process_plan( - request_process_intent(&request_without_network), - &runner_without_network.cwd_root, - runner_without_network.network_allowed, - &runner_without_network.path_rules, - &runner_without_network.bwrap_program, - ); - let runner_with_network = factory.build_runner(&request_with_network); - let plan_with_network = bwrap_process_plan( - request_process_intent(&request_with_network), - &runner_with_network.cwd_root, - runner_with_network.network_allowed, - &runner_with_network.path_rules, - &runner_with_network.bwrap_program, - ); - - assert!(os_args(&plan_without_network.args).contains(&"--unshare-net".to_owned())); - assert!(!os_args(&plan_with_network.args).contains(&"--unshare-net".to_owned())); -} - -#[cfg(unix)] -#[test] -fn bwrap_permissioned_factory_matches_rules_through_symlink_aliases() { - use std::os::unix::fs::symlink; - - let temp = tempfile::tempdir().expect("temporary path"); - let real = temp.path().join("real"); - let link = temp.path().join("link"); - std::fs::create_dir_all(&real).expect("real directory"); - symlink(&real, &link).expect("directory symlink"); - - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_path_rules([PathAccessRule::new( - real.clone(), - PathAccess::ReadWrite, - PathAccessRuleSource::TrustedGlobalConfig, - )]); - let request = permission_request(json!({ - "requested": { - "paths": [{ - "path": link.to_str().expect("UTF-8 test path"), - "access": "rw" - }] - }, - "for_action": { "command": "touch", "cwd": "." } - })); - - assert!( - factory - .request_capabilities_are_satisfied(&request) - .expect("path capability should be evaluated") - ); - - let denied_factory = - BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_path_rules([PathAccessRule::new( - real, - PathAccess::Deny, - PathAccessRuleSource::TrustedGlobalConfig, - )]); - assert!( - !denied_factory - .request_capabilities_are_satisfied(&request) - .expect("denied path capability should be evaluated") - ); -} - -#[test] -fn bwrap_permissioned_factory_preserves_trusted_path_rules() { - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_path_rules([PathAccessRule::new( - PathBuf::from("/var/log"), - PathAccess::ReadOnly, - PathAccessRuleSource::TrustedGlobalConfig, - )]); - let request = permission_request(json!({ - "requested": { "network": true }, - "for_action": { "command": "cargo test", "cwd": "." } - })); - - let runner = factory.build_runner(&request); - let plan = bwrap_process_plan( - request_process_intent(&request), - &runner.cwd_root, - runner.network_allowed, - &runner.path_rules, - &runner.bwrap_program, - ); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &["--ro-bind-try", "/var/log", "/var/log"] - )); -} - -#[test] -fn bwrap_permissioned_factory_materializes_requested_path_rules() { - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap"); - let request = permission_request(json!({ - "requested": { - "paths": [{ "path": "deps/cache", "access": "rw" }] - }, - "for_action": { "command": "cargo test", "cwd": "." } - })); - - let runner = factory.build_runner(&request); - let plan = bwrap_process_plan( - request_process_intent(&request), - &runner.cwd_root, - runner.network_allowed, - &runner.path_rules, - &runner.bwrap_program, - ); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &[ - "--bind", - "/workspace/merry/deps/cache", - "/workspace/merry/deps/cache" - ] - )); -} - -#[test] -fn bwrap_permissioned_factory_keeps_approved_paths_for_later_actions() { - let session_permissions = BwrapSessionPermissions::new(); - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions.clone()); - let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions); - let first_request = permission_request(json!({ - "requested": { - "paths": [ - { "path": "/tmp", "access": "rw" }, - { "path": "/var/lib/merry-demo.txt", "access": "ro" } - ] - }, - "for_action": { "command": "touch /var/lib/merry-demo.txt", "cwd": null } - })); - let later_request = permission_request(json!({ - "requested": { "network": true }, - "for_action": { "command": "cat /var/lib/merry-demo.txt", "cwd": null } - })); - let narrower_request = permission_request(json!({ - "requested": { - "paths": [{ "path": "/tmp", "access": "ro" }] - }, - "for_action": { "command": "ls /tmp", "cwd": null } - })); - - let _ = factory.runner_for(&first_request); - let _ = factory.runner_for(&narrower_request); - let plan = base_runner - .plan_for(request_process_intent(&later_request)) - .expect("later ordinary process plan should build"); - let args = os_args(&plan.args); - - assert!(contains_sequence(&args, &["--bind", "/tmp", "/tmp"])); - assert!(!contains_sequence(&args, &["--ro-bind", "/tmp", "/tmp"])); - assert!(contains_sequence( - &args, - &[ - "--ro-bind", - "/var/lib/merry-demo.txt", - "/var/lib/merry-demo.txt" - ] - )); - assert!(!contains_sequence( - &args, - &["--bind-try", "/var/tmp", "/var/tmp"] - )); - assert!(args.iter().any(|arg| arg == "--unshare-net")); -} - -#[test] -fn bwrap_permissioned_factory_reuses_parent_path_grants_for_descendants() { - let session_permissions = BwrapSessionPermissions::new(); - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions.clone()); - let parent_request = permission_request(json!({ - "requested": { - "paths": [{ "path": "/tmp", "access": "rw" }] - }, - "for_action": { "command": "mkdir -p /tmp/work", "cwd": null } - })); - let descendant_read_write_request = permission_request(json!({ - "requested": { - "paths": [{ "path": "/tmp/hello_world.txt", "access": "rw" }] - }, - "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } - })); - let descendant_read_only_request = permission_request(json!({ - "requested": { - "paths": [{ "path": "/tmp/hello_world.txt", "access": "ro" }] - }, - "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } - })); - let network_request = permission_request(json!({ - "requested": { "network": true }, - "for_action": { "command": "curl https://example.invalid", "cwd": null } - })); - - let _ = factory.runner_for(&parent_request); - let descendant_runner = factory - .backend() - .build_runner(&descendant_read_write_request); - let descendant_plan = descendant_runner - .plan_for(request_process_intent(&descendant_read_write_request)) - .expect("covered descendant process plan should build"); - let descendant_args = os_args(&descendant_plan.args); - assert!(contains_sequence( - &descendant_args, - &["--bind", "/tmp", "/tmp"] - )); - assert!(!contains_sequence( - &descendant_args, - &["--bind", "/tmp/hello_world.txt", "/tmp/hello_world.txt"] - )); - assert!( - factory - .request_capabilities_are_satisfied(&descendant_read_write_request) - .expect("descendant write capability query should succeed") - ); - assert!( - factory - .request_capabilities_are_satisfied(&descendant_read_only_request) - .expect("descendant read capability query should succeed") - ); - assert!( - !factory - .request_capabilities_are_satisfied(&network_request) - .expect("network capability query should succeed") - ); - - let read_only_session = BwrapSessionPermissions::new(); - let read_only_factory = - BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(read_only_session); - let read_only_parent_request = permission_request(json!({ - "requested": { - "paths": [{ "path": "/tmp", "access": "ro" }] - }, - "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } - })); - - let _ = read_only_factory.runner_for(&read_only_parent_request); - assert!( - read_only_factory - .request_capabilities_are_satisfied(&descendant_read_only_request) - .expect("read-only descendant capability query should succeed") - ); - assert!( - !read_only_factory - .request_capabilities_are_satisfied(&descendant_read_write_request) - .expect("read-write upgrade capability query should succeed") - ); -} - -#[test] -fn bwrap_permissioned_factory_keeps_network_scoped_to_current_action() { - let session_permissions = BwrapSessionPermissions::new(); - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions.clone()); - let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions); - let network_request = permission_request(json!({ - "requested": { "network": true }, - "for_action": { "command": "curl https://example.invalid", "cwd": null } - })); - - let _ = factory.runner_for(&network_request); - let approved_runner = factory.backend().build_runner(&network_request); - let approved_plan = approved_runner - .plan_for(request_process_intent(&network_request)) - .expect("approved network process plan should build"); - let approved_args = os_args(&approved_plan.args); - assert!(!approved_args.iter().any(|arg| arg == "--unshare-net")); - - let plan = base_runner - .plan_for(&intent(None)) - .expect("later process plan should build"); - let args = os_args(&plan.args); - - assert!(args.iter().any(|arg| arg == "--unshare-net")); -} - -#[test] -fn bwrap_permissioned_factory_requires_git_write_per_action() { - let external_root = tempfile::tempdir().expect("external root should be created"); - let external_git = external_root.path().join(".git"); - std::fs::create_dir(&external_git).expect("external git directory should be created"); - let external_root_path = external_root - .path() - .to_str() - .expect("root path should be utf-8") - .to_owned(); - let external_git_path = external_git - .to_str() - .expect("git path should be utf-8") - .to_owned(); - let session_permissions = BwrapSessionPermissions::new(); - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions.clone()); - let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_session_permissions(session_permissions); - let parent_request = permission_request(json!({ - "requested": { - "paths": [{ "path": external_root_path.clone(), "access": "rw" }] - }, - "for_action": { "command": "cp a external/out", "cwd": null } - })); - let git_request = permission_request(json!({ - "requested": { - "paths": [{ "path": external_git_path.clone(), "access": "rw" }] - }, - "for_action": { "command": "git checkout -- README.md", "cwd": null } - })); - - let _ = factory.runner_for(&parent_request); - let _ = factory.runner_for(&git_request); - - let current_git_runner = factory.backend().build_runner(&git_request); - let current_git_plan = current_git_runner - .plan_for(request_process_intent(&git_request)) - .expect("the reviewed git action should build"); - let current_git_args = os_args(¤t_git_plan.args); - assert!(contains_sequence( - ¤t_git_args, - &[ - "--bind", - external_git_path.as_str(), - external_git_path.as_str() - ] - )); - assert!(!contains_sequence( - ¤t_git_args, - &[ - "--ro-bind", - external_git_path.as_str(), - external_git_path.as_str() - ] - )); - - let later_plan = base_runner - .plan_for(request_process_intent(&parent_request)) - .expect("later ordinary action should build"); - let later_args = os_args(&later_plan.args); - assert!(contains_sequence( - &later_args, - &[ - "--bind", - external_root_path.as_str(), - external_root_path.as_str() - ] - )); - assert!(contains_sequence( - &later_args, - &[ - "--ro-bind", - external_git_path.as_str(), - external_git_path.as_str() - ] - )); - assert!(!contains_sequence( - &later_args, - &[ - "--bind", - external_git_path.as_str(), - external_git_path.as_str() - ] - )); -} - -#[test] -fn bwrap_git_baseline_cannot_be_upgraded_by_unreviewed_configured_write() { - let runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_path_rules([PathAccessRule::new( - PathBuf::from("/pathA/.git"), - PathAccess::ReadWrite, - PathAccessRuleSource::TrustedGlobalConfigWritableCeiling, - )]); - let plan = runner - .plan_for(&intent(None)) - .expect("configured path rules should build"); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &["--ro-bind-try", "/pathA/.git", "/pathA/.git"] - )); - assert!(!contains_sequence( - &args, - &["--bind-try", "/pathA/.git", "/pathA/.git"] - )); -} - -#[test] -fn bwrap_git_baseline_checks_only_workspace_and_requested_git_paths() { - let workspace = tempfile::tempdir().expect("workspace should be created"); - let workspace_git = workspace.path().join(".git"); - std::fs::create_dir_all(&workspace_git).expect("workspace git directory should be created"); - let nested_git = workspace.path().join("nested-repo/.git"); - std::fs::create_dir_all(&nested_git).expect("nested git directory should be created"); - let nested_missing_git = workspace.path().join("nested-worktree/.git"); - std::fs::create_dir_all( - nested_missing_git - .parent() - .expect("nested missing git parent should exist"), - ) - .expect("nested missing git parent should be created"); - let unrequested_git = workspace.path().join("unrequested-repo/.git"); - std::fs::create_dir_all(&unrequested_git).expect("unrequested git directory should be created"); - let requested_repo = workspace.path().join("nested-repo"); - let workspace_git_path = workspace_git - .to_str() - .expect("workspace git path should be utf-8"); - let nested_git_path = nested_git - .to_str() - .expect("nested git path should be utf-8"); - let nested_missing_git_path = nested_missing_git - .to_str() - .expect("nested missing git path should be utf-8"); - let unrequested_git_path = unrequested_git - .to_str() - .expect("unrequested git path should be utf-8"); - - let runner = BwrapProcessRunner::new_at_workspace_root(workspace.path()) - .with_bwrap_program("/custom/bin/bwrap") - .with_path_rules([PathAccessRule::new( - requested_repo, - PathAccess::ReadWrite, - PathAccessRuleSource::PermissionReview, - )]); - let plan = runner - .plan_for(&intent(None)) - .expect("git metadata plan should build"); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &["--ro-bind", workspace_git_path, workspace_git_path] - )); - assert!(contains_sequence( - &args, - &["--ro-bind", nested_git_path, nested_git_path] - )); - assert!(!contains_sequence( - &args, - &["--bind", nested_git_path, nested_git_path] - )); - assert!(!contains_sequence( - &args, - &["--tmpfs", nested_missing_git_path] - )); - assert!(!contains_sequence( - &args, - &[ - "--ro-bind", - nested_missing_git_path, - nested_missing_git_path - ] - )); - assert!(!contains_sequence( - &args, - &["--ro-bind", unrequested_git_path, unrequested_git_path] - )); -} - -#[test] -#[cfg(unix)] -fn bwrap_permissioned_factory_keeps_approved_host_integrations_for_later_actions() { - let directory = tempfile::tempdir().unwrap(); - let socket = directory.path().join("agent.sock"); - let _listener = std::os::unix::net::UnixListener::bind(&socket).unwrap(); - let socket_path = socket.to_str().unwrap(); - let mut environment = - BwrapProcessEnvironment::new("/custom/bin:/usr/bin", "/home/alice", "/tmp") - .expect("environment layout should validate"); - environment.ssh_agent_socket = Some(socket.clone()); - let session_permissions = BwrapSessionPermissions::new(); - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_environment(environment.clone()) - .with_session_permissions(session_permissions.clone()); - let request = permission_request(json!({ - "requested": { "host_integrations": ["ssh-agent"] }, - "for_action": { "command": "ssh -T git@example.test", "cwd": null } - })); - - factory - .validate_request(&request) - .expect("configured host integration should be materializable"); - let _ = factory.runner_for(&request); - - let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") - .with_bwrap_program("/custom/bin/bwrap") - .with_environment(environment) - .with_session_permissions(session_permissions); - let later_request = permission_request(json!({ - "requested": { "paths": [{ "path": ".", "access": "rw" }] }, - "for_action": { "command": "ssh -T git@example.test", "cwd": null } - })); - let plan = base_runner - .plan_for(request_process_intent(&later_request)) - .expect("later process plan should build"); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &["--ro-bind", socket_path, socket_path] - )); -} - -#[test] -fn bwrap_permissioned_factory_caps_requested_write_to_trusted_read_only_rule() { - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_path_rules([PathAccessRule::new( - PathBuf::from("/workspace/merry/deps"), - PathAccess::ReadOnly, - PathAccessRuleSource::TrustedGlobalConfig, - )]); - let request = permission_request(json!({ - "requested": { - "paths": [{ "path": "deps", "access": "rw" }] - }, - "for_action": { "command": "cargo test", "cwd": "." } - })); - - factory - .validate_request(&request) - .expect("read-only policy should cap rather than reject a write request"); - let runner = factory.build_runner(&request); - let plan = bwrap_process_plan( - request_process_intent(&request), - &runner.cwd_root, - runner.network_allowed, - &runner.path_rules, - &runner.bwrap_program, - ); - let args = os_args(&plan.args); - - assert!(contains_sequence( - &args, - &[ - "--ro-bind-try", - "/workspace/merry/deps", - "/workspace/merry/deps" - ] - )); - assert!(!contains_sequence( - &args, - &[ - "--bind-try", - "/workspace/merry/deps", - "/workspace/merry/deps" - ] - )); -} - -#[test] -fn bwrap_permissioned_factory_rejects_requested_path_under_configured_deny() { - let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") - .with_path_rules([PathAccessRule::new( - PathBuf::from("/workspace/merry/secrets"), - PathAccess::Deny, - PathAccessRuleSource::TrustedGlobalConfig, - )]); - let request = permission_request(json!({ - "requested": { - "paths": [{ "path": "secrets/token", "access": "ro" }] - }, - "for_action": { "command": "cat secrets/token", "cwd": null } - })); - - let error = factory - .validate_request(&request) - .expect_err("configured deny must be a hard policy boundary"); - assert!( - error - .to_string() - .contains("denied by configured path policy") - ); -} - -#[test] -fn static_permissioned_factory_rejects_requested_path_capabilities() { - let factory = StaticPermissionedProcessRunnerFactory::new(Arc::new( - BwrapProcessRunner::new_at_workspace_root("/workspace/merry"), - )); - let request = permission_request(json!({ - "requested": { - "paths": [{ "path": "deps/cache", "access": "rw" }] - }, - "for_action": { "command": "cargo test", "cwd": null } - })); - - let error = factory - .validate_request(&request) - .expect_err("static runner must not silently ignore path capabilities"); - assert!( - error - .to_string() - .contains("cannot enforce requested path capabilities") - ); -} - -#[test] -fn unrestricted_permissioned_factory_accepts_host_capabilities() { - let runner: Arc = Arc::new(TokioProcessRunner::new()); - let factory = UnrestrictedPermissionedProcessRunnerFactory::new(runner); - let request = permission_request(json!({ - "requested": { - "network": true, - "paths": [{ "path": "/var/lib/merry-demo.txt", "access": "rw" }], - "host_integrations": ["dbus"] - }, - "for_action": { "command": "gh auth status", "cwd": null } - })); - - factory - .validate_request(&request) - .expect("unrestricted host mode should not reject already-host-visible capabilities"); -} +//! Permission admission tests for the sandbox runner factories, grouped by the +//! policy each group owns. +//! +//! `path_rules` covers trusted rules, review materialization, session +//! retention, and the deny/read-only ceilings; `git_baseline` the automatic Git +//! metadata protection; `host_integrations` the trusted-config +//! preauthorization; `network` the network ceiling; and `factory_capabilities` +//! how the non-sandbox factories answer requested capabilities. + +mod factory_capabilities; +mod git_baseline; +mod host_integrations; +mod network; +mod path_rules; diff --git a/crates/merry-process/src/process_runner/tests/permissions/factory_capabilities.rs b/crates/merry-process/src/process_runner/tests/permissions/factory_capabilities.rs new file mode 100644 index 00000000..6dbcc53d --- /dev/null +++ b/crates/merry-process/src/process_runner/tests/permissions/factory_capabilities.rs @@ -0,0 +1,51 @@ +//! How the non-sandbox factories answer requested capabilities: neither +//! silently ignores them nor invents a restriction the host mode does not have. + +use crate::UnrestrictedPermissionedProcessRunnerFactory; +use crate::process_runner::tests::permission_request; +use crate::process_runner::{BwrapProcessRunner, TokioProcessRunner}; +use merry_runtime::{ + PermissionedProcessRunnerFactory, ProcessRunner, StaticPermissionedProcessRunnerFactory, +}; +use serde_json::json; +use std::sync::Arc; + +#[test] +fn static_permissioned_factory_rejects_requested_path_capabilities() { + let factory = StaticPermissionedProcessRunnerFactory::new(Arc::new( + BwrapProcessRunner::new_at_workspace_root("/workspace/merry"), + )); + let request = permission_request(json!({ + "requested": { + "paths": [{ "path": "deps/cache", "access": "rw" }] + }, + "for_action": { "command": "cargo test", "cwd": null } + })); + + let error = factory + .validate_request(&request) + .expect_err("static runner must not silently ignore path capabilities"); + assert!( + error + .to_string() + .contains("cannot enforce requested path capabilities") + ); +} + +#[test] +fn unrestricted_permissioned_factory_accepts_host_capabilities() { + let runner: Arc = Arc::new(TokioProcessRunner::new()); + let factory = UnrestrictedPermissionedProcessRunnerFactory::new(runner); + let request = permission_request(json!({ + "requested": { + "network": true, + "paths": [{ "path": "/var/lib/merry-demo.txt", "access": "rw" }], + "host_integrations": ["dbus"] + }, + "for_action": { "command": "gh auth status", "cwd": null } + })); + + factory + .validate_request(&request) + .expect("unrestricted host mode should not reject already-host-visible capabilities"); +} diff --git a/crates/merry-process/src/process_runner/tests/permissions/git_baseline.rs b/crates/merry-process/src/process_runner/tests/permissions/git_baseline.rs new file mode 100644 index 00000000..d5a1a9e6 --- /dev/null +++ b/crates/merry-process/src/process_runner/tests/permissions/git_baseline.rs @@ -0,0 +1,207 @@ +//! Automatic Git metadata protection: `.git` stays read-only even when a +//! configured or reviewed rule covers its parent, and only the workspace and +//! requested repositories are considered. + +use crate::process_runner::tests::{ + contains_sequence, intent, os_args, permission_request, request_process_intent, +}; +use crate::process_runner::{ + BwrapPermissionedProcessRunnerFactory, BwrapProcessRunner, BwrapSessionPermissions, +}; +use merry_runtime::{ + PathAccess, PathAccessRule, PathAccessRuleSource, PermissionedProcessRunnerFactory, +}; +use serde_json::json; + +#[test] +fn bwrap_permissioned_factory_requires_git_write_per_action() { + let external_root = tempfile::tempdir().expect("external root should be created"); + let external_git = external_root.path().join(".git"); + std::fs::create_dir(&external_git).expect("external git directory should be created"); + let external_root_path = external_root + .path() + .to_str() + .expect("root path should be utf-8") + .to_owned(); + let external_git_path = external_git + .to_str() + .expect("git path should be utf-8") + .to_owned(); + let session_permissions = BwrapSessionPermissions::new(); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions.clone()); + let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions); + let parent_request = permission_request(json!({ + "requested": { + "paths": [{ "path": external_root_path.clone(), "access": "rw" }] + }, + "for_action": { "command": "cp a external/out", "cwd": null } + })); + let git_request = permission_request(json!({ + "requested": { + "paths": [{ "path": external_git_path.clone(), "access": "rw" }] + }, + "for_action": { "command": "git checkout -- README.md", "cwd": null } + })); + + let _ = factory.runner_for(&parent_request); + let _ = factory.runner_for(&git_request); + + let current_git_runner = factory.backend().build_runner(&git_request); + let current_git_plan = current_git_runner + .plan_for(request_process_intent(&git_request)) + .expect("the reviewed git action should build"); + let current_git_args = os_args(¤t_git_plan.args); + assert!(contains_sequence( + ¤t_git_args, + &[ + "--bind", + external_git_path.as_str(), + external_git_path.as_str() + ] + )); + assert!(!contains_sequence( + ¤t_git_args, + &[ + "--ro-bind", + external_git_path.as_str(), + external_git_path.as_str() + ] + )); + + let later_plan = base_runner + .plan_for(request_process_intent(&parent_request)) + .expect("later ordinary action should build"); + let later_args = os_args(&later_plan.args); + assert!(contains_sequence( + &later_args, + &[ + "--bind", + external_root_path.as_str(), + external_root_path.as_str() + ] + )); + assert!(contains_sequence( + &later_args, + &[ + "--ro-bind", + external_git_path.as_str(), + external_git_path.as_str() + ] + )); + assert!(!contains_sequence( + &later_args, + &[ + "--bind", + external_git_path.as_str(), + external_git_path.as_str() + ] + )); +} + +#[test] +fn bwrap_git_baseline_still_protects_metadata_under_a_configured_write() { + let root = tempfile::tempdir().expect("configured root should be created"); + let git = root.path().join(".git"); + std::fs::create_dir(&git).expect("git directory should be created"); + let root_path = root + .path() + .to_str() + .expect("configured root path should be utf-8"); + let git_path = git.to_str().expect("git path should be utf-8"); + let runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_path_rules([PathAccessRule::new( + root.path(), + PathAccess::ReadWrite, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + let plan = runner + .plan_for(&intent(None)) + .expect("configured path rules should build"); + let args = os_args(&plan.args); + + // `readwrite_paths` is preauthorized, but Git metadata inside it keeps the + // automatic read-only baseline: a separate per-action grant is required. + assert!(contains_sequence( + &args, + &["--bind-try", root_path, root_path] + )); + assert!(contains_sequence(&args, &["--ro-bind", git_path, git_path])); + assert!(!contains_sequence(&args, &["--bind", git_path, git_path])); +} + +#[test] +fn bwrap_git_baseline_checks_only_workspace_and_requested_git_paths() { + let workspace = tempfile::tempdir().expect("workspace should be created"); + let workspace_git = workspace.path().join(".git"); + std::fs::create_dir_all(&workspace_git).expect("workspace git directory should be created"); + let nested_git = workspace.path().join("nested-repo/.git"); + std::fs::create_dir_all(&nested_git).expect("nested git directory should be created"); + let nested_missing_git = workspace.path().join("nested-worktree/.git"); + std::fs::create_dir_all( + nested_missing_git + .parent() + .expect("nested missing git parent should exist"), + ) + .expect("nested missing git parent should be created"); + let unrequested_git = workspace.path().join("unrequested-repo/.git"); + std::fs::create_dir_all(&unrequested_git).expect("unrequested git directory should be created"); + let requested_repo = workspace.path().join("nested-repo"); + let workspace_git_path = workspace_git + .to_str() + .expect("workspace git path should be utf-8"); + let nested_git_path = nested_git + .to_str() + .expect("nested git path should be utf-8"); + let nested_missing_git_path = nested_missing_git + .to_str() + .expect("nested missing git path should be utf-8"); + let unrequested_git_path = unrequested_git + .to_str() + .expect("unrequested git path should be utf-8"); + + let runner = BwrapProcessRunner::new_at_workspace_root(workspace.path()) + .with_bwrap_program("/custom/bin/bwrap") + .with_path_rules([PathAccessRule::new( + requested_repo, + PathAccess::ReadWrite, + PathAccessRuleSource::PermissionReview, + )]); + let plan = runner + .plan_for(&intent(None)) + .expect("git metadata plan should build"); + let args = os_args(&plan.args); + + assert!(contains_sequence( + &args, + &["--ro-bind", workspace_git_path, workspace_git_path] + )); + assert!(contains_sequence( + &args, + &["--ro-bind", nested_git_path, nested_git_path] + )); + assert!(!contains_sequence( + &args, + &["--bind", nested_git_path, nested_git_path] + )); + assert!(!contains_sequence( + &args, + &["--tmpfs", nested_missing_git_path] + )); + assert!(!contains_sequence( + &args, + &[ + "--ro-bind", + nested_missing_git_path, + nested_missing_git_path + ] + )); + assert!(!contains_sequence( + &args, + &["--ro-bind", unrequested_git_path, unrequested_git_path] + )); +} diff --git a/crates/merry-process/src/process_runner/tests/permissions/host_integrations.rs b/crates/merry-process/src/process_runner/tests/permissions/host_integrations.rs new file mode 100644 index 00000000..943a6803 --- /dev/null +++ b/crates/merry-process/src/process_runner/tests/permissions/host_integrations.rs @@ -0,0 +1,144 @@ +//! Trusted-config host integration preauthorization inside the inner sandbox. + +use crate::process_runner::tests::{ + contains_sequence, os_args, permission_request, request_process_intent, +}; +use crate::process_runner::{ + BwrapPermissionedProcessRunnerFactory, BwrapProcessEnvironment, BwrapProcessRunner, + BwrapSessionPermissions, +}; +use merry_runtime::{HostIntegration, PermissionedProcessRunnerFactory}; +use serde_json::json; + +#[test] +#[cfg(unix)] +fn bwrap_permissioned_factory_preauthorizes_configured_host_integrations() { + let directory = tempfile::tempdir().unwrap(); + let ssh = directory.path().join("ssh.sock"); + let bus = directory.path().join("bus.sock"); + let _ssh_listener = std::os::unix::net::UnixListener::bind(&ssh).unwrap(); + let _bus_listener = std::os::unix::net::UnixListener::bind(&bus).unwrap(); + let ssh_path = ssh.to_str().unwrap(); + let bus_path = bus.to_str().unwrap(); + let mut environment = + BwrapProcessEnvironment::new("/custom/bin:/usr/bin", "/home/alice", "/tmp") + .expect("environment layout should validate"); + environment.ssh_agent_socket = Some(ssh.clone()); + environment.session_bus_address = Some(format!("unix:path={bus_path}").into()); + // This mirrors the CLI mapping of trusted global configuration into the + // inner backend options. + let ssh_environment = environment + .clone() + .with_host_integrations([HostIntegration::SshAgent]); + let bus_environment = environment.with_host_integrations([HostIntegration::SessionBus]); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_environment(ssh_environment); + + let configured = permission_request(json!({ + "requested": { "host_integrations": ["ssh-agent"] }, + "for_action": { "command": "ssh -T git@example.test", "cwd": null } + })); + let unconfigured = permission_request(json!({ + "requested": { "host_integrations": ["dbus"] }, + "for_action": { "command": "secret-tool lookup key value", "cwd": null } + })); + assert!( + factory + .request_capabilities_are_satisfied(&configured) + .expect("configured integration should be evaluated"), + "an integration enabled by trusted config must not require another review" + ); + assert!( + !factory + .request_capabilities_are_satisfied(&unconfigured) + .expect("unconfigured integration should be evaluated"), + "an integration trusted config did not enable still requires review" + ); + + let unrelated = permission_request(json!({ + "requested": { "paths": [{ "path": ".", "access": "rw" }] }, + "for_action": { "command": "true", "cwd": null } + })); + let plan = factory + .build_runner(&unrelated) + .plan_for(request_process_intent(&unrelated)) + .expect("preauthorized integration should build a sandbox plan"); + let args = os_args(&plan.args); + + assert!(contains_sequence(&args, &["--ro-bind", ssh_path, ssh_path])); + assert!(contains_sequence( + &args, + &["--setenv", "SSH_AUTH_SOCK", ssh_path] + )); + assert!(!contains_sequence( + &args, + &["--ro-bind", bus_path, bus_path] + )); + + let bus_factory = + BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_environment(bus_environment); + let plan = bus_factory + .build_runner(&unrelated) + .plan_for(request_process_intent(&unrelated)) + .expect("preauthorized session bus should build a sandbox plan"); + let args = os_args(&plan.args); + let address = format!("unix:path={bus_path}"); + + assert!(contains_sequence(&args, &["--ro-bind", bus_path, bus_path])); + assert!(contains_sequence( + &args, + &["--setenv", "DBUS_SESSION_BUS_ADDRESS", &address] + )); + assert!(!contains_sequence( + &args, + &["--ro-bind", ssh_path, ssh_path] + )); +} + +#[test] +#[cfg(unix)] +fn bwrap_permissioned_factory_keeps_approved_host_integrations_for_later_actions() { + let directory = tempfile::tempdir().unwrap(); + let socket = directory.path().join("agent.sock"); + let _listener = std::os::unix::net::UnixListener::bind(&socket).unwrap(); + let socket_path = socket.to_str().unwrap(); + let mut environment = + BwrapProcessEnvironment::new("/custom/bin:/usr/bin", "/home/alice", "/tmp") + .expect("environment layout should validate"); + environment.ssh_agent_socket = Some(socket.clone()); + let session_permissions = BwrapSessionPermissions::new(); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_environment(environment.clone()) + .with_session_permissions(session_permissions.clone()); + let request = permission_request(json!({ + "requested": { "host_integrations": ["ssh-agent"] }, + "for_action": { "command": "ssh -T git@example.test", "cwd": null } + })); + + factory + .validate_request(&request) + .expect("configured host integration should be materializable"); + let _ = factory.runner_for(&request); + + let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_environment(environment) + .with_session_permissions(session_permissions); + let later_request = permission_request(json!({ + "requested": { "paths": [{ "path": ".", "access": "rw" }] }, + "for_action": { "command": "ssh -T git@example.test", "cwd": null } + })); + let plan = base_runner + .plan_for(request_process_intent(&later_request)) + .expect("later process plan should build"); + let args = os_args(&plan.args); + + assert!(contains_sequence( + &args, + &["--ro-bind", socket_path, socket_path] + )); +} diff --git a/crates/merry-process/src/process_runner/tests/permissions/network.rs b/crates/merry-process/src/process_runner/tests/permissions/network.rs new file mode 100644 index 00000000..752f8cb2 --- /dev/null +++ b/crates/merry-process/src/process_runner/tests/permissions/network.rs @@ -0,0 +1,75 @@ +//! The network ceiling: a request can widen one action, never the session. + +use crate::process_runner::tests::{intent, os_args, permission_request, request_process_intent}; +use crate::process_runner::{ + BwrapPermissionedProcessRunnerFactory, BwrapProcessRunner, BwrapSessionPermissions, + bwrap_process_plan, +}; +use merry_runtime::PermissionedProcessRunnerFactory; +use serde_json::json; + +#[test] +fn bwrap_permissioned_factory_allows_network_only_when_requested() { + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap"); + let request_without_network = permission_request(json!({ + "requested": { + "paths": [{ "path": "/workspace/merry", "access": "rw" }] + }, + "for_action": { "command": "cargo test", "cwd": "." } + })); + let request_with_network = permission_request(json!({ + "requested": { "network": true }, + "for_action": { "command": "cargo test", "cwd": "." } + })); + + let runner_without_network = factory.build_runner(&request_without_network); + let plan_without_network = bwrap_process_plan( + request_process_intent(&request_without_network), + &runner_without_network.cwd_root, + runner_without_network.network_allowed, + &runner_without_network.path_rules, + &runner_without_network.bwrap_program, + ); + let runner_with_network = factory.build_runner(&request_with_network); + let plan_with_network = bwrap_process_plan( + request_process_intent(&request_with_network), + &runner_with_network.cwd_root, + runner_with_network.network_allowed, + &runner_with_network.path_rules, + &runner_with_network.bwrap_program, + ); + + assert!(os_args(&plan_without_network.args).contains(&"--unshare-net".to_owned())); + assert!(!os_args(&plan_with_network.args).contains(&"--unshare-net".to_owned())); +} + +#[test] +fn bwrap_permissioned_factory_keeps_network_scoped_to_current_action() { + let session_permissions = BwrapSessionPermissions::new(); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions.clone()); + let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions); + let network_request = permission_request(json!({ + "requested": { "network": true }, + "for_action": { "command": "curl https://example.invalid", "cwd": null } + })); + + let _ = factory.runner_for(&network_request); + let approved_runner = factory.backend().build_runner(&network_request); + let approved_plan = approved_runner + .plan_for(request_process_intent(&network_request)) + .expect("approved network process plan should build"); + let approved_args = os_args(&approved_plan.args); + assert!(!approved_args.iter().any(|arg| arg == "--unshare-net")); + + let plan = base_runner + .plan_for(&intent(None)) + .expect("later process plan should build"); + let args = os_args(&plan.args); + + assert!(args.iter().any(|arg| arg == "--unshare-net")); +} diff --git a/crates/merry-process/src/process_runner/tests/permissions/path_rules.rs b/crates/merry-process/src/process_runner/tests/permissions/path_rules.rs new file mode 100644 index 00000000..ebdd870d --- /dev/null +++ b/crates/merry-process/src/process_runner/tests/permissions/path_rules.rs @@ -0,0 +1,332 @@ +//! Path capability admission: trusted rules, review materialization, session +//! retention, symlink aliases, and the read-only and deny ceilings. + +use crate::process_runner::tests::{ + contains_sequence, os_args, permission_request, request_process_intent, +}; +use crate::process_runner::{ + BwrapPermissionedProcessRunnerFactory, BwrapProcessRunner, BwrapSessionPermissions, + bwrap_process_plan, +}; +use merry_runtime::{ + PathAccess, PathAccessRule, PathAccessRuleSource, PermissionedProcessRunnerFactory, +}; +use serde_json::json; +use std::path::PathBuf; + +#[cfg(unix)] +#[test] +fn bwrap_permissioned_factory_matches_rules_through_symlink_aliases() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().expect("temporary path"); + let real = temp.path().join("real"); + let link = temp.path().join("link"); + std::fs::create_dir_all(&real).expect("real directory"); + symlink(&real, &link).expect("directory symlink"); + + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_path_rules([PathAccessRule::new( + real.clone(), + PathAccess::ReadWrite, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + let request = permission_request(json!({ + "requested": { + "paths": [{ + "path": link.to_str().expect("UTF-8 test path"), + "access": "rw" + }] + }, + "for_action": { "command": "touch", "cwd": "." } + })); + + assert!( + factory + .request_capabilities_are_satisfied(&request) + .expect("path capability should be evaluated") + ); + + let denied_factory = + BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_path_rules([PathAccessRule::new( + real, + PathAccess::Deny, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + assert!( + !denied_factory + .request_capabilities_are_satisfied(&request) + .expect("denied path capability should be evaluated") + ); +} + +#[test] +fn bwrap_permissioned_factory_preserves_trusted_path_rules() { + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_path_rules([PathAccessRule::new( + PathBuf::from("/var/log"), + PathAccess::ReadOnly, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + let request = permission_request(json!({ + "requested": { "network": true }, + "for_action": { "command": "cargo test", "cwd": "." } + })); + + let runner = factory.build_runner(&request); + let plan = bwrap_process_plan( + request_process_intent(&request), + &runner.cwd_root, + runner.network_allowed, + &runner.path_rules, + &runner.bwrap_program, + ); + let args = os_args(&plan.args); + + assert!(contains_sequence( + &args, + &["--ro-bind-try", "/var/log", "/var/log"] + )); +} + +#[test] +fn bwrap_permissioned_factory_materializes_requested_path_rules() { + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap"); + let request = permission_request(json!({ + "requested": { + "paths": [{ "path": "deps/cache", "access": "rw" }] + }, + "for_action": { "command": "cargo test", "cwd": "." } + })); + + let runner = factory.build_runner(&request); + let plan = bwrap_process_plan( + request_process_intent(&request), + &runner.cwd_root, + runner.network_allowed, + &runner.path_rules, + &runner.bwrap_program, + ); + let args = os_args(&plan.args); + + assert!(contains_sequence( + &args, + &[ + "--bind", + "/workspace/merry/deps/cache", + "/workspace/merry/deps/cache" + ] + )); +} + +#[test] +fn bwrap_permissioned_factory_keeps_approved_paths_for_later_actions() { + let session_permissions = BwrapSessionPermissions::new(); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions.clone()); + let base_runner = BwrapProcessRunner::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions); + let first_request = permission_request(json!({ + "requested": { + "paths": [ + { "path": "/tmp", "access": "rw" }, + { "path": "/var/lib/merry-demo.txt", "access": "ro" } + ] + }, + "for_action": { "command": "touch /var/lib/merry-demo.txt", "cwd": null } + })); + let later_request = permission_request(json!({ + "requested": { "network": true }, + "for_action": { "command": "cat /var/lib/merry-demo.txt", "cwd": null } + })); + let narrower_request = permission_request(json!({ + "requested": { + "paths": [{ "path": "/tmp", "access": "ro" }] + }, + "for_action": { "command": "ls /tmp", "cwd": null } + })); + + let _ = factory.runner_for(&first_request); + let _ = factory.runner_for(&narrower_request); + let plan = base_runner + .plan_for(request_process_intent(&later_request)) + .expect("later ordinary process plan should build"); + let args = os_args(&plan.args); + + assert!(contains_sequence(&args, &["--bind", "/tmp", "/tmp"])); + assert!(!contains_sequence(&args, &["--ro-bind", "/tmp", "/tmp"])); + assert!(contains_sequence( + &args, + &[ + "--ro-bind", + "/var/lib/merry-demo.txt", + "/var/lib/merry-demo.txt" + ] + )); + assert!(!contains_sequence( + &args, + &["--bind-try", "/var/tmp", "/var/tmp"] + )); + assert!(args.iter().any(|arg| arg == "--unshare-net")); +} + +#[test] +fn bwrap_permissioned_factory_reuses_parent_path_grants_for_descendants() { + let session_permissions = BwrapSessionPermissions::new(); + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(session_permissions.clone()); + let parent_request = permission_request(json!({ + "requested": { + "paths": [{ "path": "/tmp", "access": "rw" }] + }, + "for_action": { "command": "mkdir -p /tmp/work", "cwd": null } + })); + let descendant_read_write_request = permission_request(json!({ + "requested": { + "paths": [{ "path": "/tmp/hello_world.txt", "access": "rw" }] + }, + "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } + })); + let descendant_read_only_request = permission_request(json!({ + "requested": { + "paths": [{ "path": "/tmp/hello_world.txt", "access": "ro" }] + }, + "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } + })); + let network_request = permission_request(json!({ + "requested": { "network": true }, + "for_action": { "command": "curl https://example.invalid", "cwd": null } + })); + + let _ = factory.runner_for(&parent_request); + let descendant_runner = factory + .backend() + .build_runner(&descendant_read_write_request); + let descendant_plan = descendant_runner + .plan_for(request_process_intent(&descendant_read_write_request)) + .expect("covered descendant process plan should build"); + let descendant_args = os_args(&descendant_plan.args); + assert!(contains_sequence( + &descendant_args, + &["--bind", "/tmp", "/tmp"] + )); + assert!(!contains_sequence( + &descendant_args, + &["--bind", "/tmp/hello_world.txt", "/tmp/hello_world.txt"] + )); + assert!( + factory + .request_capabilities_are_satisfied(&descendant_read_write_request) + .expect("descendant write capability query should succeed") + ); + assert!( + factory + .request_capabilities_are_satisfied(&descendant_read_only_request) + .expect("descendant read capability query should succeed") + ); + assert!( + !factory + .request_capabilities_are_satisfied(&network_request) + .expect("network capability query should succeed") + ); + + let read_only_session = BwrapSessionPermissions::new(); + let read_only_factory = + BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_bwrap_program("/custom/bin/bwrap") + .with_session_permissions(read_only_session); + let read_only_parent_request = permission_request(json!({ + "requested": { + "paths": [{ "path": "/tmp", "access": "ro" }] + }, + "for_action": { "command": "cat /tmp/hello_world.txt", "cwd": null } + })); + + let _ = read_only_factory.runner_for(&read_only_parent_request); + assert!( + read_only_factory + .request_capabilities_are_satisfied(&descendant_read_only_request) + .expect("read-only descendant capability query should succeed") + ); + assert!( + !read_only_factory + .request_capabilities_are_satisfied(&descendant_read_write_request) + .expect("read-write upgrade capability query should succeed") + ); +} + +#[test] +fn bwrap_permissioned_factory_caps_requested_write_to_trusted_read_only_rule() { + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_path_rules([PathAccessRule::new( + PathBuf::from("/workspace/merry/deps"), + PathAccess::ReadOnly, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + let request = permission_request(json!({ + "requested": { + "paths": [{ "path": "deps", "access": "rw" }] + }, + "for_action": { "command": "cargo test", "cwd": "." } + })); + + factory + .validate_request(&request) + .expect("read-only policy should cap rather than reject a write request"); + let runner = factory.build_runner(&request); + let plan = bwrap_process_plan( + request_process_intent(&request), + &runner.cwd_root, + runner.network_allowed, + &runner.path_rules, + &runner.bwrap_program, + ); + let args = os_args(&plan.args); + + assert!(contains_sequence( + &args, + &[ + "--ro-bind-try", + "/workspace/merry/deps", + "/workspace/merry/deps" + ] + )); + assert!(!contains_sequence( + &args, + &[ + "--bind-try", + "/workspace/merry/deps", + "/workspace/merry/deps" + ] + )); +} + +#[test] +fn bwrap_permissioned_factory_rejects_requested_path_under_configured_deny() { + let factory = BwrapPermissionedProcessRunnerFactory::new_at_workspace_root("/workspace/merry") + .with_path_rules([PathAccessRule::new( + PathBuf::from("/workspace/merry/secrets"), + PathAccess::Deny, + PathAccessRuleSource::TrustedGlobalConfig, + )]); + let request = permission_request(json!({ + "requested": { + "paths": [{ "path": "secrets/token", "access": "ro" }] + }, + "for_action": { "command": "cat secrets/token", "cwd": null } + })); + + let error = factory + .validate_request(&request) + .expect_err("configured deny must be a hard policy boundary"); + assert!( + error + .to_string() + .contains("denied by configured path policy") + ); +} diff --git a/crates/merry-process/src/process_runner/tests/sandbox.rs b/crates/merry-process/src/process_runner/tests/sandbox.rs index 567bf5bb..92eae74c 100644 --- a/crates/merry-process/src/process_runner/tests/sandbox.rs +++ b/crates/merry-process/src/process_runner/tests/sandbox.rs @@ -194,6 +194,47 @@ fn bwrap_process_plan_hides_unapproved_host_integrations() { )); } +#[test] +#[cfg(unix)] +fn bwrap_process_plan_keeps_denied_host_integration_endpoint_in_the_environment() { + let directory = tempfile::tempdir().unwrap(); + let socket = directory.path().join("agent.sock"); + let _listener = std::os::unix::net::UnixListener::bind(&socket).unwrap(); + let socket_path = socket.to_str().unwrap(); + let mut environment = + BwrapProcessEnvironment::new("/custom/bin:/usr/bin", "/home/alice", "/tmp") + .expect("environment layout should validate"); + environment.ssh_agent_socket = Some(socket.clone()); + let environment = environment.with_host_integrations([HostIntegration::SshAgent]); + let denied = PathAccessRule::new( + directory.path().to_path_buf(), + PathAccess::Deny, + PathAccessRuleSource::TrustedGlobalConfig, + ); + let plan = bwrap_process_plan_with_environment( + &intent(None), + Path::new("/workspace/merry"), + &environment, + true, + &[denied], + Path::new("/custom/bin/bwrap"), + ) + .expect("sandbox plan"); + let args = os_args(&plan.args); + + assert!(!contains_sequence( + &args, + &["--ro-bind", socket_path, socket_path] + )); + // A deny rule commonly covers a parent directory the user wanted hidden, + // not the agent itself, so the configured integration stays announced and + // the client reports the unreachable endpoint at connect time. + assert!(contains_sequence( + &args, + &["--setenv", "SSH_AUTH_SOCK", socket_path] + )); +} + #[test] fn bwrap_process_plan_preserves_action_tmp_when_host_socket_is_under_it() { let mut environment = diff --git a/crates/merry-runtime/src/permission.rs b/crates/merry-runtime/src/permission.rs index 63054020..871e649c 100644 --- a/crates/merry-runtime/src/permission.rs +++ b/crates/merry-runtime/src/permission.rs @@ -104,9 +104,10 @@ pub enum RequestedCapability { } /// Host integration and supporting client files exposed to inner process actions. -/// The outer sandbox remains the capability ceiling; an enabled integration can -/// be forwarded to the inner action sandbox, while an explicit request can add -/// one for a permissioned action when the backend supports it. +/// Trusted global configuration is the capability ceiling and preauthorizes the +/// inner action sandbox; an integration the configuration did not enable can +/// still be requested for one permissioned action when the backend supports it +/// and its endpoint is visible. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum HostIntegration { /// The user's SSH authentication agent socket and read-only known-hosts files. diff --git a/crates/merry-runtime/src/permission/input.rs b/crates/merry-runtime/src/permission/input.rs index 220c1027..db332cc2 100644 --- a/crates/merry-runtime/src/permission/input.rs +++ b/crates/merry-runtime/src/permission/input.rs @@ -19,15 +19,23 @@ use serde::Deserialize; use serde_json::{Value, json}; use std::{collections::BTreeMap, path::Path, sync::Arc}; +// Model-visible schema text for requested capabilities. The derive attributes +// below and the hand-written JSON schema in `requested_capabilities_schema_json` +// describe the same fields, so both read these constants instead of repeating +// the wording and drifting apart. +const REQUESTED_PATHS_DESCRIPTION: &str = "Exact filesystem paths to authorize. Configured review_paths stay hidden until explicitly requested and are approved only for this action, as are Git metadata writes. Ordinary path grants may be retained by a session-aware backend. Each item specifies a path and ro, rw, or deny access; approval never overrides a configured read-only ceiling or denial."; +const REQUESTED_PATH_DESCRIPTION: &str = "Path requested for additional filesystem access."; +const REQUESTED_ACCESS_DESCRIPTION: &str = "Requested access for this path: ro, rw, or deny."; +const REQUESTED_NETWORK_DESCRIPTION: &str = "Set true to request network capability for the exact action. Network is never granted implicitly: a command that authenticates, installs, downloads, publishes, or otherwise reaches a remote service needs this set in the same action, while a command that only touches the workspace and the configured local baseline does not."; +const REQUESTED_HOST_INTEGRATIONS_DESCRIPTION: &str = "Host integrations: ssh-agent, dbus, or gpg-agent. Integrations enabled by trusted global configuration are already available and need no request; request one here only when it is not configured but its endpoint is present. SSH includes its agent and read-only known_hosts; GPG includes read-only public keys and its native agent, not the SSH socket. File sources still obey path restrictions, including deny_paths and review_paths; neither integration authorizes networking. dbus is the session bus used by keyring clients."; +const PERMISSION_REASON_DESCRIPTION: &str = "Optional short explanation of why the current task needs the requested capability. Null is treated as omitted; a provided string must be non-blank and within the byte limit."; + #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] pub(crate) struct RequestedPathInput { - #[schemars( - description = "Path requested for additional filesystem access.", - length(min = 1) - )] + #[schemars(description = REQUESTED_PATH_DESCRIPTION, length(min = 1))] pub(crate) path: String, - #[schemars(description = "Requested access for this path: ro, rw, or deny.")] + #[schemars(description = REQUESTED_ACCESS_DESCRIPTION)] pub(crate) access: String, } @@ -35,17 +43,13 @@ pub(crate) struct RequestedPathInput { #[serde(deny_unknown_fields)] pub(crate) struct RequestedCapabilitiesInput { #[serde(default)] - #[schemars(description = "Set true to request network capability for the exact action.")] + #[schemars(description = REQUESTED_NETWORK_DESCRIPTION)] pub(crate) network: bool, #[serde(default)] - #[schemars( - description = "Exact filesystem paths to authorize. Configured review_paths stay hidden until explicitly requested and are approved only for this action, as are Git metadata writes. Ordinary path grants may be retained by a session-aware backend. Each item specifies a path and ro, rw, or deny access; approval never overrides a configured read-only ceiling or denial." - )] + #[schemars(description = REQUESTED_PATHS_DESCRIPTION)] pub(crate) paths: Vec, #[serde(default)] - #[schemars( - description = "Explicitly configured host integrations: ssh-agent, dbus, or gpg-agent. SSH includes its agent and read-only known_hosts; GPG includes read-only public keys and its native agent, not the SSH socket. File sources still obey path restrictions; neither integration authorizes networking. dbus is the session bus used by keyring clients." - )] + #[schemars(description = REQUESTED_HOST_INTEGRATIONS_DESCRIPTION)] pub(crate) host_integrations: Vec, } @@ -89,7 +93,7 @@ impl<'de> Deserialize<'de> for PermissionedProcessInput { #[tool( crate = "crate", name = "request_permissions", - description = "Request additional filesystem, network, or explicitly configured host-integration capability for one exact planned action." + description = "Request additional filesystem, network, or host-integration capability that trusted configuration did not already enable for one exact planned action." )] #[derive(Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] @@ -97,7 +101,7 @@ struct RequestPermissionsInput { #[serde(default)] #[schemars( schema_with = "permission_reason_schema_for_schemars", - description = "Optional short explanation of why the current task needs the requested capability. Null is treated as omitted; a provided string must be non-blank and within the byte limit." + description = PERMISSION_REASON_DESCRIPTION )] reason: Option, #[schemars(schema_with = "requested_capabilities_schema_for_schemars")] @@ -111,7 +115,7 @@ struct RequestPermissionsInput { pub fn request_permissions_tool() -> Result { let spec = RequestPermissionsInput::tool_spec_with( REQUEST_PERMISSIONS_TOOL_NAME, - "Request additional filesystem, network, or explicitly configured host-integration capability for one exact planned action. A configured session-aware process backend retains approved paths and host integrations for later actions in the current runtime session, but network access must be requested again for every action. When one command needs multiple capabilities, request them together.", + "Request filesystem, network, or host-integration capability that trusted configuration has not already enabled for one exact planned action. Paths and host integrations enabled by trusted global configuration are already available, so request only what is still missing. A configured session-aware process backend retains approved paths and host integrations for later actions in the current runtime session, but network access must be requested again for every action. When one command needs multiple capabilities, request them together.", ) .map_err(|error| PermissionAdmissionError::Core { source: error.into(), @@ -203,7 +207,7 @@ impl ToolExecutor for RequestPermissionsToolExecutor { fn permission_reason_schema_json() -> Value { json!({ - "description": "Optional short explanation of why the current task needs the requested capability. Null is treated as omitted; a provided string must be non-blank and within the byte limit.", + "description": PERMISSION_REASON_DESCRIPTION, "anyOf": [ { "type": "null" }, { @@ -219,15 +223,15 @@ fn requested_capabilities_schema_json() -> Value { json!({ "type": "object", "additionalProperties": false, - "description": "Capabilities to add for this exact action after approval. Use network for network access, paths for filesystem paths, or host_integrations for SSH agent, native GPG agent, or D-Bus access. A session-aware backend may retain ordinary path and host-integration grants, but network, configured review_paths, and Git metadata writes require approval for every action. Include every capability the same command needs in one request.", + "description": "Capabilities to add for this exact action after approval. Use network for network access, paths for filesystem paths, or host_integrations for SSH agent, native GPG agent, or D-Bus access. Paths and host integrations enabled by trusted global configuration are already available to every action, so request only what is still missing; network is never granted implicitly, so a command that reaches a remote service must request it in the same action. A session-aware backend may retain ordinary path and host-integration grants, but network, configured review_paths, and Git metadata writes require approval for every action. Include every capability the same command needs in one request.", "properties": { "network": { "type": "boolean", - "description": "Set true to request network capability for the exact action." + "description": REQUESTED_NETWORK_DESCRIPTION }, "paths": { "type": "array", - "description": "Exact filesystem paths to authorize. Configured review_paths stay hidden until explicitly requested and are approved only for this action, as are Git metadata writes. Ordinary path grants may be retained by a session-aware backend. Each item specifies a path and ro, rw, or deny access; approval never overrides a configured read-only ceiling or denial.", + "description": REQUESTED_PATHS_DESCRIPTION, "items": { "type": "object", "additionalProperties": false, @@ -235,12 +239,12 @@ fn requested_capabilities_schema_json() -> Value { "path": { "type": "string", "minLength": 1, - "description": "Path requested for additional filesystem access." + "description": REQUESTED_PATH_DESCRIPTION }, "access": { "type": "string", "enum": ["ro", "rw", "deny"], - "description": "Requested access for this path: ro, rw, or deny." + "description": REQUESTED_ACCESS_DESCRIPTION } }, "required": ["path", "access"] @@ -248,7 +252,7 @@ fn requested_capabilities_schema_json() -> Value { }, "host_integrations": { "type": "array", - "description": "Explicitly configured host integrations: ssh-agent, dbus, or gpg-agent. SSH includes its agent and read-only known_hosts; GPG includes read-only public keys and its native agent, not the SSH socket. File sources still obey path restrictions; neither integration authorizes networking. dbus is the session bus used by keyring clients.", + "description": REQUESTED_HOST_INTEGRATIONS_DESCRIPTION, "items": { "type": "string", "enum": ["ssh-agent", "dbus", "gpg-agent"] diff --git a/crates/merry-runtime/src/profile.rs b/crates/merry-runtime/src/profile.rs index 94a11ec7..4710eba0 100644 --- a/crates/merry-runtime/src/profile.rs +++ b/crates/merry-runtime/src/profile.rs @@ -60,9 +60,6 @@ pub enum PathAccessRuleSource { /// This is intentionally higher trust than project-local configuration, /// because normal coding-agent runs may edit files inside the project. TrustedGlobalConfig, - /// Rule exposed by trusted global configuration as an outer writable - /// ceiling while the inner action baseline remains read-only. - TrustedGlobalConfigWritableCeiling, /// Product-owned configuration, credentials, and state unavailable to actions. ProductPrivate, /// Automatically applied read-only protection for Git metadata paths. diff --git a/crates/merry-runtime/src/runtime/process_execution.rs b/crates/merry-runtime/src/runtime/process_execution.rs index 782daaca..0ae70904 100644 --- a/crates/merry-runtime/src/runtime/process_execution.rs +++ b/crates/merry-runtime/src/runtime/process_execution.rs @@ -387,7 +387,7 @@ fn process_output_artifact_content( if !output.ok() { payload["guidance"] = serde_json::json!({ "kind": "process_action_recovery", - "message": "The process action ran inside the sandbox and failed. If the needed network, filesystem path, or host integration was known before execution, include every minimum required capability under permissions in the next run_process call so runtime can review before execution. If the need was discovered only from this failure, call request_permissions for the exact same action before retrying it. An unmodeled Linux Unix socket may be requested as its exact filesystem path.", + "message": "The process action ran inside the sandbox and failed. Treat the sandbox as the first suspect: a withheld network, filesystem, or host integration capability often surfaces as a credentials, authentication, or connectivity error, so check whether the command needed access it did not request. Include every minimum required capability under permissions in the next run_process call so runtime can review before execution. If the need was discovered only from this failure, call request_permissions for the exact same action before retrying it. An unmodeled Linux Unix socket may be requested as its exact filesystem path.", }); } diff --git a/examples/config.toml b/examples/config.toml index 2e4bed74..a3166af5 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -61,19 +61,30 @@ approval_policy = "model_then_human" network = true # Optional host integrations. These are explicit outer-sandbox ceilings and -# forwarding switches, not inner preauthorizations. Inner actions must request -# the matching host integration and pass runtime review before using its socket -# or automatically imported client files. Approved grants follow runtime session -# retention; review_paths still requires approval for each action. +# inner preauthorizations: when the endpoint exists, ordinary inner actions use +# the socket and automatically imported client files without another request, +# like readonly_paths / readwrite_paths. review_paths still mask a configured +# endpoint until the exact path is approved for that action, and deny_paths +# always mask it. An integration that is not enabled here can still be requested +# per action when its endpoint is visible. +# +# Path policy decides reachability, not announcement: SSH_AUTH_SOCK, +# DBUS_SESSION_BUS_ADDRESS, and GNUPGHOME still name the configured endpoints, +# while a deny_paths or review_paths entry covering the socket, the keyring, or +# one of their parent directories masks the mount itself. A broad deny (for +# example a whole ~/.ssh or ~/.gnupg deny) therefore also hides the agent even +# though ssh_agent / gpg_agent / dbus is enabled: the client sees the endpoint +# and fails when it connects. Keep such denies narrow if the integration should +# stay usable; deny_paths is never reopened by an approval. # dbus is the D-Bus session bus, including Secret Service/keyring access. # ssh_agent = true # gpg_agent = true # dbus = true -# After SSH approval, inner actions also get ~/.ssh/known_hosts and known_hosts2 -# read-only, not private keys or ~/.ssh/config. This does not enable networking, -# accept new host keys, or bypass deny_paths / review_paths. A blanket ~/.ssh -# deny still blocks these trust files; choose narrower deny paths if they should -# remain readable. +# Once the SSH integration is available, inner actions get ~/.ssh/known_hosts +# and known_hosts2 read-only, not private keys or ~/.ssh/config. This does not +# enable networking, accept new host keys, or bypass deny_paths / review_paths. +# A blanket ~/.ssh deny still blocks these trust files; choose narrower deny +# paths if they should remain readable. # ssh_agent also imports /etc/passwd and /etc/group for client account lookup, # plus /etc/ssh/ssh_config and /etc/ssh/ssh_config.d, all read-only. No blanket # /etc grant is needed. These imports obey deny_paths and do not import SSH @@ -84,10 +95,10 @@ network = true # read-only snapshots to retain OpenSSH ownership checks across user namespaces. # Host files are never changed. # gpg_agent enables the native GPG socket discovered with gpgconf --list-dirs, -# independently of ssh_agent (including GPG's SSH endpoint). After GPG approval, -# inner actions also import pubring.kbx / pubring.gpg read-only; no extra -# readonly_paths is needed. Approved inner actions use a private temporary -# GNUPGHOME view for locks and a fresh trustdb. +# independently of ssh_agent (including GPG's SSH endpoint). When the +# integration is available, inner actions import pubring.kbx / pubring.gpg +# read-only; no extra readonly_paths is needed. Such inner actions use a private +# temporary GNUPGHOME view for locks and a fresh trustdb. # Host private keys, configuration, and trust state are not imported. Public-key # reads still obey deny_paths / review_paths. Keyboxd databases are not yet # supported and produce an explicit error, not an implicit host write grant. diff --git a/sdks/python/README.md b/sdks/python/README.md index 9adb9f35..83727c9b 100644 --- a/sdks/python/README.md +++ b/sdks/python/README.md @@ -152,8 +152,12 @@ assert result.status is merry.RunStatus.COMPLETED `ToolCallBatch` is an exclusive Rust-owned lease. The complete result set must be submitted before reading the next message. `result()` is valid after EOF; -`run.cancel()` and `close()` wait for a durable cancelled terminal result. The -Python async task may be cancelled; the SDK requests Rust cancellation and +`run.cancel()` and `close()` wait for the durable terminal result. Cancellation +is cooperative, so a run that reached its terminal state first keeps that +result: `close()` requests cancellation only while the run is unfinished, and +`cancel()` returns the completed result instead of rewriting its status. Both +wait for Rust to stop provider and tool work, so the returned result is durable. +The Python async task may be cancelled; the SDK requests Rust cancellation and re-raises `asyncio.CancelledError`. `AgentBuilder` is single-use for `build()` and `resume()`. A native operation diff --git a/sdks/python/merry/_run.py b/sdks/python/merry/_run.py index 01672f96..302fbef1 100644 --- a/sdks/python/merry/_run.py +++ b/sdks/python/merry/_run.py @@ -126,7 +126,12 @@ async def result(self) -> RunResult[OutputT]: return result async def cancel(self) -> RunResult[OutputT]: - """Cancel the run and return its durable terminal result.""" + """Cancel the run and return its durable terminal result. + + Cancellation is cooperative: a run that reached its terminal state + before the request arrived keeps that result, so the returned status is + `CANCELLED` only when cancellation reached the runtime first. + """ if self._result is not None: return self._result @@ -151,7 +156,11 @@ async def cancel(self) -> RunResult[OutputT]: return result async def close(self) -> None: - """Cancel an unfinished run and wait for its terminal result.""" + """Cancel an unfinished run and wait for its terminal result. + + A run that already reached EOF or a terminal result keeps that result; + its status is not rewritten to `CANCELLED`. + """ if self._terminal_error is not None: self._raise_terminal_error() diff --git a/sdks/python/tests/test_run.py b/sdks/python/tests/test_run.py index 9bd52bfb..56c942d4 100644 --- a/sdks/python/tests/test_run.py +++ b/sdks/python/tests/test_run.py @@ -334,8 +334,9 @@ async def scenario() -> str: def test_close_cancels_an_unfinished_run_and_keeps_terminal_result() -> None: async def scenario() -> merry.RunResult[BaseModel]: - run = fake_agent(session_id="close-run").stream("Close me.") - await run.close() + native = pending_native_agent("close-run") + run = merry.Agent._from_native(native).stream("Close me.") + await asyncio.wait_for(run.close(), timeout=5) return await run.result() result = asyncio.run(scenario()) @@ -343,6 +344,23 @@ async def scenario() -> merry.RunResult[BaseModel]: assert result.status is merry.RunStatus.CANCELLED +def test_close_after_the_run_reached_eof_keeps_the_completed_result() -> None: + async def scenario() -> tuple[ + merry.RunResult[BaseModel], merry.RunResult[BaseModel] + ]: + run = fake_agent(session_id="close-completed").stream("Close me.") + while await run.next() is not None: + pass + before = await run.result() + await asyncio.wait_for(run.close(), timeout=5) + return before, await run.result() + + before, after = asyncio.run(scenario()) + + assert before.status is merry.RunStatus.COMPLETED + assert after.status is before.status + + def test_direct_next_cancellation_persists_a_terminal_result() -> None: async def scenario() -> merry.RunResult[BaseModel]: native = pending_native_agent("direct-next-cancel")