From fbb7055b9020e505cd1bf903b1b1c301ba8f3eef Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:58:31 -0600 Subject: [PATCH] fix(runner): keep activity FIFOs out of Codex writable roots (#18886) ## Thinking Path > - Paperclip manages agents and their work. > - Runner owns each provider process and its activity channel. > - Codex Plan mode uses a restricted filesystem policy. > - Runner adds its activity FIFO as a writable filesystem root. > - Codex adds protected metadata paths beneath each writable root. > - A FIFO cannot contain those paths, so the Linux helper fails before useful work starts. > - This pull request grants access through the private activity directory and keeps discovery files read-only. ## Linked Issues or Issue Description Refs #18050. The activity channel came from that change. No existing FIFO-mount repair was found. #18853 repairs interrupted preparation recovery; it does not change this provider filesystem policy. **What happened?** A native Codex Plan session can fail while loading AGENTS.md instructions. The filesystem helper tries to inspect `activity.fifo/.aws` and fails with `Not a directory`. This happens before the provider can produce the requested plan. **Expected behavior** The sandbox must load permitted instructions and allow activity-channel writes. Plan mode must still reject workspace writes and private state access. **Steps to reproduce** Use the ignored `read_only_codex_opens_with_runtime_activity` regression in the Runner Rust suite with the source-pinned Linux Codex executable and its matching bubblewrap resource. The fixture calls the actual activity channel producer and policy projection. On the original producer it exits 101 with the FIFO metadata-mount panic. With this change it exits 0. **Paperclip version or commit** Original producer reproduced at 208bda1a0ea36b0500e86606eedc32fc1244aa03. The local native artifact is official Codex 0.160.0 for Linux arm64, which matches the source pin. The failing hosted executable version was not independently available from its normal run records. **Deployment mode** Native Runner with Linux Codex. Verification used an isolated local Linux container with no network, credentials, or model call. ## What Changed - Grant writes to the Runner-created private activity-channel directory instead of using its FIFO as a filesystem root. - Keep the lock, descriptor, and helper read-only. Keep the existing network, workspace, environment, and active-run authority policies. - Add actual channel isolation coverage and a pinned executable regression. The latter also checks FIFO bytes, instruction reads, workspace write denial, private-state and synthetic credential-home denial, descriptor and lock write denial, lock removal denial, and ambient-variable filtering. - Document the native sandbox check and the separate fixture-export path. Export alone does not verify the sandbox. ## Verification - Actual producer plus official Linux Codex 0.160.0 and matching bubblewrap: original producer exits 101 with the FIFO `.aws` panic; changed producer exits 0 with all permission checks and exact FIFO output verified. - `cargo test --manifest-path packages/paperclip-runner/runner/Cargo.toml runtime_notification --lib --offline`: 8 passed, 2 manual sandbox fixtures ignored. - `cargo test --manifest-path packages/paperclip-runner/runner/Cargo.toml codex_provider::tests --lib --offline`: 30 passed. - `cargo build --manifest-path packages/paperclip-runner/runner/Cargo.toml --bin paperclip-runnerd --offline`: passed. - Cargo format and diff checks passed. - A broader optional local Rust library sweep did not pass in the restricted macOS host. Three filesystem-monitor tests failed and another did not finish; the owned sweep was bounded and stopped. A socket-identity fixture also failed with EPERM. A representative monitor failure and the socket failure were repeated with the unchanged original producer. Their failures are retained and are not claimed fixed by this PR. - The existing ignored `publisher_crosses_real_codex_tool_boundary_without_runner_state_access` fixture was not executed successfully on the final head. Its stale channel-directory denial was corrected after Greptile review. The separate actual-producer Linux sandbox fixture verifies exact FIFO bytes and permission denials; it does not call `record-activity`. - Full local pnpm typecheck/test/build was not run for this Rust-only change. Required technical CI passes on fd4da0bf5ef2ea7dfc3da566567444ca3e87d1a2. Greptile is 5/5. The automatic Paperclip Review remains queued; merge is held for that review. No hosted task recovery or provider turn is claimed. ## Risks The writable grant now covers the process-private channel directory. The descriptor remains outside that grant. The lock has an explicit read-only mount inside it. Native sandbox checks verify the lock cannot be written or removed. Existing tests retain authority rotation, stale-context rejection, malformed input bounds, and fixed-path cleanup after endpoint replacement. The retained startup error also mentions a loopback permission error. This change fixes the independently reproduced FIFO-root defect. It does not prove that every hosted namespace capability is healthy. Preconfigured-home and custom policies retain their existing explicit deny rules. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) in Codex, with source editing, terminal execution, and tool use. The host did not report its context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- packages/paperclip-runner/README.md | 23 +++ .../crates/runner-core/src/codex_provider.rs | 16 +- .../runner-core/src/runtime_notifications.rs | 5 +- .../tests/codex_sandbox.rs | 157 ++++++++++++++++++ 4 files changed, 195 insertions(+), 6 deletions(-) create mode 100644 packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications/tests/codex_sandbox.rs diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index a7e77abe79b..13c354a89c0 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -726,6 +726,29 @@ actual shell discovery, successful durable activity recording, and denial of unr Runner state, descriptor writes and ambient secrets. The normal suite covers bounded framing, concurrent publishers, fixed-path cleanup and attachment fencing. +Codex receives write access to Runner's private activity-channel directory, with +the lock file explicitly read-only. The FIFO itself must not be a writable +filesystem root: Codex adds protected metadata children to writable roots, which +cannot exist beneath a FIFO. The helper and discovery descriptor remain read-only; +the activity grant does not expose Runner state or change workspace/network policy. + +To check restrictive Plan mode without credentials or a model call, run the +actual channel/policy producer against the pinned Linux Codex sandbox: + +```sh +CODEX_NOTIFICATION_SANDBOX_COMMAND=/absolute/path/to/pinned/codex \ +CODEX_NOTIFICATION_SANDBOX_RESOURCES=/absolute/path/to/pinned/executable/resources \ +cargo test --manifest-path runner/Cargo.toml --locked -p paperclip-runner-core --lib \ + read_only_codex_opens_with_runtime_activity -- --ignored +``` + +This checks instruction reads, FIFO writes, and denial of workspace writes, +unrelated Runner-state reads, descriptor writes, and lock writes/removal. An +isolated non-Linux host can instead set `CODEX_NOTIFICATION_FIXTURE_EXPORT` to a +JSON file and omit `CODEX_NOTIFICATION_SANDBOX_COMMAND`; that only exports a +synthetic producer fixture for a separately controlled Linux sandbox. Exporting +the fixture does not execute or verify the sandbox. + ## Direct live protocol qualification The canonical direct live protocol suite lives in the separate diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs index fefedb0501a..1687b1ad23a 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs @@ -61,8 +61,13 @@ pub(crate) fn runtime_notification_codex_args( let Some(filesystem) = config["filesystem"].as_inline_table_mut() else { continue; }; - for (path, mode) in paths.iter().zip(["read", "read", "read", "write"]) { - filesystem.insert(path.to_string_lossy().as_ref(), mode.into()); + let Some(channel_directory) = paths[3].parent().filter(|path| path.parent().is_some()) + else { + continue; + }; + filesystem.insert(channel_directory.to_string_lossy().as_ref(), "write".into()); + for path in &paths[..3] { + filesystem.insert(path.to_string_lossy().as_ref(), "read".into()); } extra.extend(["-c".into(), format!("{prefix}{filesystem}")]); } @@ -4522,8 +4527,8 @@ mod tests { let paths = [ PathBuf::from("/runtime/runnerd"), PathBuf::from("/state/private/runtime-notifications.json"), - PathBuf::from("/state/private/runtime-activity.lock"), - PathBuf::from("/tmp/prn-channel"), + PathBuf::from("/tmp/prn-channel/activity.lock"), + PathBuf::from("/tmp/prn-channel/activity.fifo"), ]; let extra = runtime_notification_codex_args(&args, "paperclip-runner-workspace-only", &paths); @@ -4562,7 +4567,7 @@ mod tests { ("/state/private", "none"), ("/runtime/runnerd", "read"), ("/state/private/runtime-notifications.json", "read"), - ("/state/private/runtime-activity.lock", "read"), + ("/tmp/prn-channel/activity.lock", "read"), ("/tmp/prn-channel", "write"), ] { assert_eq!( @@ -4570,6 +4575,7 @@ mod tests { Some(mode) ); } + assert!(filesystem.get("/tmp/prn-channel/activity.fifo").is_none()); } assert!(!serialized.contains("network.")); assert!(!serialized.contains("PAPERCLIP_RUN_ID")); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications.rs b/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications.rs index d1722519fb1..5b05061290c 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications.rs @@ -666,6 +666,8 @@ mod tests { assert_eq!(fs::read(nested.join("protected")).unwrap(), b"unchanged"); fs::remove_dir_all(pipe.parent().unwrap()).unwrap(); } + include!("runtime_notifications/tests/codex_sandbox.rs"); + #[test] #[ignore = "requires CODEX_NOTIFICATION_SANDBOX_COMMAND and PAPERCLIP_RUNTIME_NOTIFICATION_HELPER"] fn publisher_crosses_real_codex_tool_boundary_without_runner_state_access() { @@ -709,7 +711,8 @@ mod tests { if cat "$1" >/dev/null 2>&1; then exit 71; fi if /bin/sh -c ': >> "$PAPERCLIP_RUNTIME_NOTIFICATION_CONTEXT"' 2>/dev/null; then exit 72; fi if touch "$2/probe" 2>/dev/null; then exit 75; fi -if touch "$3/probe" 2>/dev/null; then exit 76; fi +touch "$3/probe" || exit 76 +rm "$3/probe" || exit 76 if /bin/sh -c ': >> "$1"' ignored "$4" 2>/dev/null; then exit 77; fi context=$("$PAPERCLIP_RUNTIME_NOTIFICATION_HELPER" notification-context) || exit 73 "$PAPERCLIP_RUNTIME_NOTIFICATION_HELPER" record-activity "$context" example.runtime desktop.used || exit 74 diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications/tests/codex_sandbox.rs b/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications/tests/codex_sandbox.rs new file mode 100644 index 00000000000..1638b7d84c5 --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/src/runtime_notifications/tests/codex_sandbox.rs @@ -0,0 +1,157 @@ +#[test] +fn codex_activity_grant_uses_only_its_own_private_channel_directory() { + let (_directory, config, _store, _state, server) = setup(); + let (_other_directory, _other_config, _other_store, _other_state, other) = setup(); + let channel = server.pipe.0.parent().unwrap(); + let other_channel = other.pipe.0.parent().unwrap(); + assert_ne!(channel, other_channel); + verify_private_directory(channel).unwrap(); + assert!(fs::symlink_metadata(&server.pipe.0) + .unwrap() + .file_type() + .is_fifo()); + let paths = [ + PathBuf::from("/runtime/runnerd"), + context_path(&config.state_dir), + server.pipe.0.with_extension("lock"), + server.pipe.0.clone(), + ]; + let profile = "paperclip-runner-workspace-read-only"; + let args = vec![ + "shell_environment_policy.include_only=[]".into(), + format!("permissions.{profile}.filesystem={{\":root\"=\"none\"}}"), + ]; + let projected = crate::codex_provider::runtime_notification_codex_args(&args, profile, &paths); + let prefix = format!("permissions.{profile}.filesystem="); + let value = projected + .iter() + .find_map(|arg| arg.strip_prefix(&prefix)) + .unwrap(); + let config = format!("filesystem={value}") + .parse::() + .unwrap(); + let filesystem = config["filesystem"].as_inline_table().unwrap(); + assert_eq!( + filesystem + .get(channel.to_str().unwrap()) + .and_then(toml_edit::Value::as_str), + Some("write") + ); + assert!(filesystem.get(other_channel.to_str().unwrap()).is_none()); + assert!(filesystem + .get(paths[1].parent().unwrap().to_str().unwrap()) + .is_none()); +} + +#[test] +#[ignore = "requires source-pinned Linux CODEX_NOTIFICATION_SANDBOX_COMMAND or CODEX_NOTIFICATION_FIXTURE_EXPORT"] +fn read_only_codex_opens_with_runtime_activity_without_widening_file_access() { + use std::process::Command; + + let directory = TestDirectory::create(&std::env::temp_dir()).unwrap(); + let config = config(directory.directory.join("state")); + fs::DirBuilder::new() + .mode(0o700) + .create(&config.state_dir) + .unwrap(); + let server = RuntimeNotifications::bind(&config.state_dir).unwrap(); + let workspace = directory.directory.join("workspace"); + let home = directory.directory.join("home"); + fs::create_dir(&workspace).unwrap(); + fs::create_dir(&home).unwrap(); + fs::write( + home.join("credential-fixture"), + b"synthetic private credential", + ) + .unwrap(); + fs::write( + workspace.join("AGENTS.md"), + b"Read-only fixture instructions.\n", + ) + .unwrap(); + fs::write( + config.state_dir.join("private-state"), + b"must remain inaccessible", + ) + .unwrap(); + let helper = config.state_dir.join("helper"); + fs::write(&helper, b"synthetic executable resource\n").unwrap(); + let paths = [ + helper, + context_path(&config.state_dir), + server.pipe.0.with_extension("lock"), + server.pipe.0.clone(), + ]; + let profile = "paperclip-runner-workspace-read-only"; + let resources = std::env::var("CODEX_NOTIFICATION_SANDBOX_RESOURCES") + .expect("set the source-pinned Codex executable resource directory"); + let resources = serde_json::to_string(&resources).unwrap(); + let args = vec![ + "-c".into(), + "shell_environment_policy.include_only=[\"PATH\"]".into(), + "-c".into(), + format!("permissions.{profile}.filesystem={{\":root\"=\"none\",\":minimal\"=\"read\",\":tmpdir\"=\"none\",{resources}=\"read\",\"{}\"=\"none\",\"{}\"=\"read\"}}", home.display(), workspace.display()), + "-c".into(), + format!("permissions.{profile}.network.enabled=false"), + ]; + let extra = crate::codex_provider::runtime_notification_codex_args(&args, profile, &paths); + let script = r#" +set -eu +test -z "${UNRELATED_SECRET:-}" +test "$(cat "$4/AGENTS.md")" = 'Read-only fixture instructions.' +if touch "$4/forbidden" 2>/dev/null; then exit 71; fi +if cat "$1/private-state" >/dev/null 2>&1; then exit 72; fi +if sh -c ': >> "$1"' ignored "$2" 2>/dev/null; then exit 73; fi +if sh -c ': >> "$1"' ignored "$3" 2>/dev/null; then exit 74; fi +if rm "$3" 2>/dev/null; then exit 75; fi +if cat "$6/credential-fixture" >/dev/null 2>&1; then exit 76; fi +cat "$2" >/dev/null +cat "$3" >/dev/null +printf 'synthetic notification frame\n' > "$5" +"#; + let mut args = args; + args.extend(extra); + args.extend(["sandbox", "-P", profile, "-C"].map(str::to_owned)); + args.push(workspace.to_string_lossy().into_owned()); + args.extend(["--", "/bin/sh", "-c", script, "notification-sandbox"].map(str::to_owned)); + args.extend( + [ + &config.state_dir, + &paths[1], + &paths[2], + &workspace, + &paths[3], + &home, + ] + .map(|path| path.to_string_lossy().into_owned()), + ); + if let Some(path) = std::env::var_os("CODEX_NOTIFICATION_FIXTURE_EXPORT") { + fs::write( + path, + serde_json::to_vec_pretty(&json!({ + "args": args, "stateDirectory": config.state_dir, + "pipe": server.pipe.0, "home": home, "workspace": workspace, + "descriptor": fs::read_to_string(&paths[1]).unwrap(), + })) + .unwrap(), + ) + .unwrap(); + return; + } + let command = std::env::var_os("CODEX_NOTIFICATION_SANDBOX_COMMAND").unwrap(); + let output = Command::new(command) + .args(args) + .current_dir(&workspace) + .env_clear() + .env("PATH", std::env::var_os("PATH").unwrap()) + .env("HOME", &home) + .env("CODEX_HOME", &home) + .env("UNRELATED_SECRET", "must-not-reach-command") + .output() + .unwrap(); + assert!( + output.status.success(), + "source-pinned sandbox failed: {}", + String::from_utf8_lossy(&output.stderr) + ); +}