patchloom · SebTardif · Jul 4, 2026 · Jul 4, 2026 · Jul 4, 2026
@@ -157,7 +157,7 @@ Patchloom runs with the privileges of the invoking user and treats all inputs (c
 What this means in practice:
 
 - **Plans can execute arbitrary shell commands.** The `format` and `validate` lifecycle steps pass their `cmd` field to `sh -c` (or `cmd /C` on Windows) with the user's full privileges. Only load plans you trust.
-- **CLI file operations are unrestricted by default.** `create`, `delete`, `read`, `replace`, `patch`, and all `tx` operations accept any path the invoking user can access (including `../` escapes from `--cwd`). `--cwd` only sets the default base for relative paths; it is **not** a containment boundary unless you also pass **`--contain`**, which enables the same PathGuard rejection rules as MCP.
+- **CLI file operations are unrestricted by default.** `create`, `delete`, `read`, `replace`, `patch`, `rename`, and all `tx` operations accept any path the invoking user can access (including `../` escapes from `--cwd`). `--cwd` only sets the default base for relative paths; it is **not** a containment boundary unless you also pass **`--contain`**, which enables the same PathGuard rejection rules as MCP on every CLI write path (engine-backed and binary/case-only rename).
 - **MCP and the library PathGuard are sandboxed.** The MCP server and embedders that pass a `PathGuard` reject paths that escape the workspace root (via `../` or symlinks). Prefer MCP tool calls when an agent must stay inside a workspace, or use `patchloom --cwd <ws> --contain …` for CLI agents.
 - **Plan `cwd` overrides the working directory.** A plan's `cwd` field changes the working directory for all subsequent operations and lifecycle steps. Relative values resolve from the invocation root, not from the plan file location. In normal CLI use this still runs with the invoking user's filesystem access; in MCP mode the resolved directory must stay under the server root.
 

@@ -140,7 +140,7 @@ These flags affect how Patchloom reports results or chooses which files to touch
 <!-- ref:global-flag:contain -->
 ### `--contain`
 
-- **What it does:** Rejects file paths that escape the working directory (via `../` or symlinks that resolve outside the workspace). Mirrors MCP / library `PathGuard` behavior.
+- **What it does:** Rejects file paths that escape the working directory (via `../`, absolute paths outside the policy, or symlinks that resolve outside the workspace). Mirrors MCP / library `PathGuard` behavior for **all CLI write paths**, including engine-backed commands (`create`, `delete`, `replace`, `patch`, `md`, `doc`, `tidy`, text `rename`, `tx` / `batch`) and callback-only mutations (binary and case-only `rename`).
 - **Use when:** An agent or automation should not be able to write outside `--cwd` (or the process cwd). Pair with `--cwd` for a workspace root.
 - **Default:** Off. CLI remains unrestricted for human scripts (same trust model as `make` / `sh`).
 - **Prefer instead:** Use the MCP server when the agent already has MCP tools; containment is always on there.

@@ -5,7 +5,7 @@
 //! [`rewrite_function_signature`] with [`FunctionSigEdit`] for structured
 //! multi-language edits (visibility, parameters, return type).
 //!
-//! size-waiver: domain bulk, see #1376. Multi-language function signature rewrite domain; tests co-located in module.
+//! size-waiver: domain bulk, see #1408. Multi-language function signature rewrite domain; tests co-located in module.
 
 use super::symbol_extract::innermost_qualified_name;
 use super::{Language, child_text_by_kind, child_text_by_kinds, parse_source};

@@ -1,4 +1,4 @@
-// size-waiver: domain bulk, see #1376. MCP integration unit tests co-located with server module.
+// size-waiver: co-located MCP unit tests, see #1408. Intentionally co-located with the server module for shared test helpers; not unfinished Phase 4 work.
 use super::*;
 use rmcp::ServiceExt;
 

@@ -46,6 +46,17 @@ pub fn run(args: RenameArgs, global: &GlobalFlags) -> anyhow::Result<u8> {
         args.force
     );
     let cwd = global.resolve_cwd()?;
+    // Enforce --contain for all rename paths (engine-backed text and
+    // binary/case-only callback via write_dispatch). Engine-backed ops also
+    // check declared_paths in the tx engine; this early check covers the
+    // callback path that never reaches the engine (#1409).
+    if let Some(guard) = global.workspace_guard(&cwd)? {
+        for p in [&args.from, &args.to] {
+            guard
+                .check_path(p)
+                .map_err(|e| anyhow::anyhow!("path rejected by workspace guard: {e}"))?;
+        }
+    }
     let src = cwd.join(&args.from);
     let dst = cwd.join(&args.to);
 
@@ -451,6 +462,152 @@ mod tests {
         assert_eq!(fs::read(&dst).unwrap(), b"\x00\x01\x02\xff\xfe");
     }
 
+    #[test]
+    fn rename_with_contain_rejects_parent_escape_on_to() {
+        let dir = TempDir::new().unwrap();
+        fs::write(dir.path().join("inside.txt"), "keep\n").unwrap();
+
+        let mut global = GlobalFlags::test_with_cwd(dir.path());
+        global.apply = true;
+        global.contain = true;
+
+        let args = RenameArgs {
+            from: "inside.txt".into(),
+            to: "../escape-renamed.txt".into(),
+            force: false,
+            write: Default::default(),
+        };
+
+        let err = run(args, &global).unwrap_err();
+        let msg = err.to_string();
+        assert!(
+            msg.contains("escapes") || msg.contains("rejected"),
+            "expected containment error, got: {msg}"
+        );
+        assert!(
+            dir.path().join("inside.txt").exists(),
+            "source must remain when containment rejects destination"
+        );
+        assert!(!dir.path().join("../escape-renamed.txt").exists());
+    }
+
+    #[test]
+    fn rename_with_contain_rejects_parent_escape_on_from() {
+        let dir = TempDir::new().unwrap();
+        let outside = dir.path().parent().unwrap().join(format!(
+            "patchloom-rename-from-escape-{}.txt",
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_millis()
+        ));
+        fs::write(&outside, "outside\n").unwrap();
+
+        let mut global = GlobalFlags::test_with_cwd(dir.path());
+        global.apply = true;
+        global.contain = true;
+
+        let args = RenameArgs {
+            from: format!("../{}", outside.file_name().unwrap().to_string_lossy()),
+            to: "pulled-in.txt".into(),
+            force: false,
+            write: Default::default(),
+        };
+
+        let err = run(args, &global).unwrap_err();
+        let msg = err.to_string();
+        assert!(
+            msg.contains("escapes") || msg.contains("rejected"),
+            "expected containment error, got: {msg}"
+        );
+        assert!(outside.exists(), "outside source must not be moved");
+        assert!(!dir.path().join("pulled-in.txt").exists());
+        let _ = fs::remove_file(&outside);
+    }
+
+    #[test]
+    fn rename_binary_with_contain_rejects_parent_escape() {
+        // Binary rename uses write_dispatch (not the tx engine). Containment
+        // must still apply (#1409).
+        let dir = TempDir::new().unwrap();
+        fs::write(dir.path().join("image.bin"), b"\x00\x01\x02\xff\xfe").unwrap();
+
+        let mut global = GlobalFlags::test_with_cwd(dir.path());
+        global.apply = true;
+        global.contain = true;
+
+        let args = RenameArgs {
+            from: "image.bin".into(),
+            to: "../escaped.bin".into(),
+            force: false,
+            write: Default::default(),
+        };
+
+        let err = run(args, &global).unwrap_err();
+        let msg = err.to_string();
+        assert!(
+            msg.contains("escapes") || msg.contains("rejected"),
+            "expected containment error on binary rename escape, got: {msg}"
+        );
+        assert!(dir.path().join("image.bin").exists());
+        assert!(!dir.path().join("../escaped.bin").exists());
+    }
+
+    #[test]
+    fn rename_with_contain_allows_in_workspace_relative_path() {
+        let dir = TempDir::new().unwrap();
+        fs::write(dir.path().join("old.txt"), "hello\n").unwrap();
+
+        let mut global = GlobalFlags::test_with_cwd(dir.path());
+        global.apply = true;
+        global.contain = true;
+
+        let args = RenameArgs {
+            from: "old.txt".into(),
+            to: "new.txt".into(),
+            force: false,
+            write: Default::default(),
+        };
+
+        let code = run(args, &global).unwrap();
+        assert_eq!(code, exit::SUCCESS);
+        assert!(!dir.path().join("old.txt").exists());
+        assert_eq!(
+            fs::read_to_string(dir.path().join("new.txt")).unwrap(),
+            "hello\n"
+        );
+    }
+
+    #[test]
+    fn rename_without_contain_allows_parent_escape() {
+        let dir = TempDir::new().unwrap();
+        fs::write(dir.path().join("inside.txt"), "move me\n").unwrap();
+        let name = format!(
+            "patchloom-rename-escape-{}.txt",
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_millis()
+        );
+
+        let mut global = GlobalFlags::test_with_cwd(dir.path());
+        global.apply = true;
+        // contain defaults to false
+
+        let args = RenameArgs {
+            from: "inside.txt".into(),
+            to: format!("../{name}"),
+            force: false,
+            write: Default::default(),
+        };
+
+        let code = run(args, &global).unwrap();
+        assert_eq!(code, exit::SUCCESS);
+        let outside = dir.path().parent().unwrap().join(&name);
+        assert_eq!(fs::read_to_string(&outside).unwrap(), "move me\n");
+        let _ = fs::remove_file(&outside);
+    }
+
     #[test]
     fn rename_creates_parent_dirs() {
         let dir = TempDir::new().unwrap();

@@ -57,7 +57,7 @@
 //! assert!(result.valid);
 //! ```
 //!
-//! size-waiver: domain bulk, see #1376. Multi-strategy edit recovery chain is one conceptual unit.
+//! size-waiver: domain bulk, see #1408. Multi-strategy edit recovery chain is one conceptual unit.
 
 use serde::{Deserialize, Serialize};
 

@@ -1,4 +1,4 @@
-// size-waiver: domain bulk, see #1376. YAML comment-preserving splice is cohesive domain logic; further split risks dual writers.
+// size-waiver: domain bulk, see #1408. YAML comment-preserving splice is cohesive domain logic; further split risks dual writers.
 /// Check if there are any arrays that grew between `old` and `new`.
 pub(super) fn has_array_growth_diffs(old: &serde_json::Value, new: &serde_json::Value) -> bool {
     if old == new {

@@ -603,3 +603,85 @@ fn test_create_default_mode_exits_2() {
         "file should not be created in default (preview) mode"
     );
 }
+
+// ---------------------------------------------------------------------------
+// --contain on create (engine-backed path) (#1406)
+// ---------------------------------------------------------------------------
+
+#[test]
+fn test_create_contain_rejects_parent_escape() {
+    let dir = TempDir::new().unwrap();
+
+    Command::cargo_bin("patchloom")
+        .unwrap()
+        .args(["--cwd"])
+        .arg(dir.path())
+        .args([
+            "--contain",
+            "create",
+            "../escape-outside.txt",
+            "--content",
+            "nope\n",
+            "--apply",
+        ])
+        .assert()
+        .failure()
+        .stderr(
+            predicate::str::contains("escapes")
+                .or(predicate::str::contains("rejected"))
+                .or(predicate::str::contains("workspace guard")),
+        );
+
+    assert!(!dir.path().join("../escape-outside.txt").exists());
+}
+
+#[test]
+fn test_create_without_contain_allows_parent_escape() {
+    let dir = TempDir::new().unwrap();
+    let name = format!(
+        "patchloom-create-escape-{}.txt",
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_millis()
+    );
+
+    Command::cargo_bin("patchloom")
+        .unwrap()
+        .args(["--cwd"])
+        .arg(dir.path())
+        .arg("create")
+        .arg(format!("../{name}"))
+        .args(["--content", "escaped\n", "--apply"])
+        .assert()
+        .code(0);
+
+    let outside = dir.path().parent().unwrap().join(&name);
+    assert_eq!(fs::read_to_string(&outside).unwrap(), "escaped\n");
+    let _ = fs::remove_file(&outside);
+}
+
+#[test]
+fn test_create_contain_allows_in_workspace() {
+    let dir = TempDir::new().unwrap();
+
+    Command::cargo_bin("patchloom")
+        .unwrap()
+        .args(["--cwd"])
+        .arg(dir.path())
+        .args([
+            "--contain",
+            "create",
+            "inside.txt",
+            "--content",
+            "ok\n",
+            "--apply",
+        ])
+        .assert()
+        .code(0);
+
+    assert_eq!(
+        fs::read_to_string(dir.path().join("inside.txt")).unwrap(),
+        "ok\n"
+    );
+}
@@ -1,5 +1,5 @@
 //! Module layout and size-policy hygiene for the structural rewrite program
-//! (#1372 / #1376).
+//! (#1372 / #1376). Remaining domain monofiles are tracked in #1408.
 
 use std::fs;
 use std::path::PathBuf;
@@ -9,11 +9,14 @@ fn repo_root() -> PathBuf {
 }
 
 /// Production sources (excluding co-located `tests.rs` / `*_tests.rs`) over
-/// 1000 lines must carry an explicit size-waiver linked to #1376.
+/// 1000 lines must carry an explicit `size-waiver:` linked to an open tracker
+/// issue (currently #1408; historically #1376).
 #[test]
 fn large_production_src_files_have_size_waiver() {
     let src_root = repo_root().join("src");
     let mut offenders = Vec::new();
+    // Require a GitHub issue reference so waivers stay tracked (not free-form).
+    let issue_ref = regex::Regex::new(r"size-waiver:.*#\d+").unwrap();
 
     for entry in walkdir_rs_files(&src_root) {
         let path = entry;
@@ -26,9 +29,9 @@ fn large_production_src_files_have_size_waiver() {
         if lines <= 1000 {
             continue;
         }
-        if !text.contains("size-waiver:") || !text.contains("#1376") {
+        if !issue_ref.is_match(&text) {
             offenders.push(format!(
-                "{} ({} lines) missing size-waiver #1376",
+                "{} ({} lines) missing `size-waiver: ... #NNNN`",
                 path.strip_prefix(repo_root()).unwrap().display(),
                 lines
             ));
@@ -37,7 +40,7 @@ fn large_production_src_files_have_size_waiver() {
 
     assert!(
         offenders.is_empty(),
-        "large production files need `size-waiver: ... #1376`:\n{}",
+        "large production files need `size-waiver: ... #NNNN` (see #1408):\n{}",
         offenders.join("\n")
     );
 }