Merge pull request #7227 from asto18089/up/subagent-absolute-write-paths

fix(subagent): accept absolute write paths that resolve inside the claimed workspace

crates/engine/src/core/authority.rs

@@ -592,8 +592,10 @@ fn carve_out_target_allowed(workspace: &Path, workspace_canonical: &Path, raw: &
 
 /// Canonicalize the deepest existing ancestor of `candidate` and re-append
 /// the not-yet-existing tail, so write targets that do not exist yet still
-/// get a real-path check.
-fn resolve_deepest_existing(candidate: &Path) -> Option<PathBuf> {
+/// get a real-path check. Shared crate-wide so the subagent write-claim lane
+/// resolves spellings through the same walk as the carve-out boundary and
+/// cannot drift from it on symlinked or partially-existing targets.
+pub(crate) fn resolve_deepest_existing(candidate: &Path) -> Option<PathBuf> {
     let mut ancestor = candidate;
     let mut suffix: Vec<&OsStr> = Vec::new();
     loop {

crates/engine/src/tools/subagent/engine.rs

@@ -399,7 +399,7 @@ impl ChildAuthority {
                 .is_some_and(|action| matches!(action, "write" | "edit" | "patch")))
             || (name == "pandoc_convert" && input.get("output_path").is_some());
         if scope_aware_write && self.enforce_write_claim {
-            let paths = mutation_paths(name, input)?;
+            let paths = mutation_paths(name, input, &self.runtime.context.workspace)?;
             if paths.is_empty() {
                 return Err(admission_denied(format!(
                     "Write tool {name} did not expose a bounded repo-relative target for coordination"
@@ -465,7 +465,7 @@ impl ChildAuthority {
             }
         }
         let observed_paths = if scope_aware_write {
-            mutation_paths(name, input)?
+            mutation_paths(name, input, &self.runtime.context.workspace)?
         } else {
             Vec::new()
         };

crates/engine/src/tools/subagent/mod.rs

@@ -21997,7 +21997,7 @@ fn raw_mutation_target_paths(name: &str, input: &Value) -> Vec<String> {
         .unwrap_or_default()
 }
 
-fn mutation_paths(name: &str, input: &Value) -> Result<Vec<String>> {
+fn mutation_paths(name: &str, input: &Value, workspace: &Path) -> Result<Vec<String>> {
     let raw_paths = if name == "apply_patch"
         || (name == "File" && input.get("action").and_then(Value::as_str) == Some("patch"))
     {
@@ -22017,12 +22017,48 @@ fn mutation_paths(name: &str, input: &Value) -> Result<Vec<String>> {
     } else {
         Vec::new()
     };
+    // One normalization per batch, not per path: every absolute spelling
+    // below strips against the same canonical workspace.
+    let workspace = normalize_subagent_workspace(workspace);
     raw_paths
         .into_iter()
-        .map(|path| normalize_claim_path(&path).map_err(anyhow::Error::msg))
+        .map(|path| {
+            let path = workspace_relative_mutation_path(&workspace, &path);
+            normalize_claim_path(&path).map_err(anyhow::Error::msg)
+        })
         .collect()
 }
 
+/// An absolute target inside the child's workspace names the same file as
+/// its relative form, and that is what the write tool would resolve it to;
+/// the claim ledger speaks repo-relative paths, so strip the workspace
+/// prefix before scope validation instead of refusing the spelling. A target
+/// outside the workspace is handed to the validator untouched and refused
+/// there, as before. The string is judged exactly as the tool receives it —
+/// no trimming: a padded spelling is not absolute, so it stays on the
+/// validator's existing lane (which trims and then refuses the absolute
+/// form, as before this helper existed). The deepest-existing walk delegates
+/// to the core's single implementation
+/// ([`crate::core::authority::resolve_deepest_existing`]) — the same one
+/// behind the write carve-out's real-path check — so this lane cannot drift
+/// from it on symlinked or partially-existing spellings.
+///
+/// `workspace` arrives already normalized from [`mutation_paths`].
+fn workspace_relative_mutation_path(workspace: &Path, path: &str) -> String {
+    let candidate = Path::new(path);
+    if !candidate.is_absolute() {
+        return path.to_string();
+    }
+    // The walk always composes a path, so this only guards a future
+    // signature change of the shared core walk.
+    let resolved = crate::core::authority::resolve_deepest_existing(candidate)
+        .unwrap_or_else(|| candidate.to_path_buf());
+    match resolved.strip_prefix(workspace) {
+        Ok(relative) => relative.to_string_lossy().into_owned(),
+        Err(_) => path.to_string(),
+    }
+}
+
 fn reject_subagent_terminal_takeover(name: &str, input: &Value) -> Result<()> {
     let wants_interactive_shell = matches!(name, "bash" | "Bash" | "exec_shell")
         && input

crates/engine/src/tools/subagent/tests.rs

@@ -4399,6 +4399,125 @@ fn declared_write_scope_is_normalized_and_rejects_traversal() {
     );
 }
 
+/// A write-capable child that spells its target as an absolute path inside
+/// its own workspace is writing the same file the relative spelling names,
+/// and that is the path the write tool would resolve it to — the claim lane
+/// must treat both spellings alike. Absolute paths outside the workspace,
+/// `..` traversal (relative or absolute), and a symlinked ancestor that
+/// resolves outside the workspace stay refused, exactly as before.
+#[test]
+fn scoped_mutation_paths_accept_absolute_targets_inside_the_workspace() {
+    let tmp = tempdir().expect("tempdir");
+    let workspace = tmp.path().join("workspace");
+    std::fs::create_dir_all(workspace.join("src")).expect("workspace tree");
+    let canonical = workspace.canonicalize().expect("canonical workspace");
+
+    // Existing target, canonical absolute spelling.
+    std::fs::write(workspace.join("src/lib.rs"), "x").expect("seed");
+    let paths = mutation_paths(
+        "write",
+        &json!({ "path": canonical.join("src/lib.rs").to_string_lossy() }),
+        &workspace,
+    )
+    .expect("absolute in-workspace path is admitted");
+    assert_eq!(paths, vec!["src/lib.rs".to_string()]);
+
+    // Not-yet-existing target (the write creates it), spelled through the
+    // un-canonicalized workspace path.
+    let paths = mutation_paths(
+        "write",
+        &json!({ "path": workspace.join("src/new.txt").to_string_lossy() }),
+        &workspace,
+    )
+    .expect("absolute path to a file the write will create is admitted");
+    assert_eq!(paths, vec!["src/new.txt".to_string()]);
+
+    // Relative spelling is unchanged.
+    let paths = mutation_paths("write", &json!({ "path": "src/new.txt" }), &workspace)
+        .expect("relative path");
+    assert_eq!(paths, vec!["src/new.txt".to_string()]);
+
+    // The two spellings land in the same claimed scope: a child holding the
+    // `src` root admits both the relative form and the absolute form's
+    // workspace-relative equivalent.
+    let mut manager = SubAgentManager::new(tmp.path().to_path_buf(), 4);
+    let child_owner = manager.insert_test_running_agent("abs_child", &workspace);
+    let claim = manager
+        .namespace_write_claim(
+            &workspace,
+            false,
+            WriteScopeClaim {
+                owner: child_owner.clone(),
+                roots: vec!["src".into()],
+                exact_files: Vec::new(),
+                contracts: Vec::new(),
+            },
+        )
+        .expect("child claim namespace");
+    manager
+        .coordination
+        .register_claim(claim, false, |_| true)
+        .expect("child claim");
+    manager
+        .validate_write_scope(&child_owner, &["src/lib.rs".into()])
+        .expect("relative spelling admitted by the claimed root scope");
+    let absolute_paths = mutation_paths(
+        "write",
+        &json!({ "path": canonical.join("src/lib.rs").to_string_lossy() }),
+        &workspace,
+    )
+    .expect("absolute spelling strips to its workspace-relative equivalent");
+    assert_eq!(absolute_paths, vec!["src/lib.rs".to_string()]);
+    manager
+        .validate_write_scope(&child_owner, &absolute_paths)
+        .expect("absolute spelling of the same target is admitted by the same scope");
+
+    // Outside the workspace: refused as before.
+    let err = mutation_paths(
+        "write",
+        &json!({ "path": tmp.path().join("escape.txt").to_string_lossy() }),
+        &workspace,
+    )
+    .expect_err("absolute path outside the workspace is refused")
+    .to_string();
+    assert!(err.contains("repo-relative"), "{err}");
+
+    // Traversal: refused as before, relative or absolute.
+    for path in ["../escape.txt", "src/../../escape.txt"] {
+        let err = mutation_paths("write", &json!({ "path": path }), &workspace)
+            .expect_err("traversal is refused")
+            .to_string();
+        assert!(err.contains("traversal"), "{path}: {err}");
+    }
+    let err = mutation_paths(
+        "write",
+        &json!({ "path": workspace.join("src/../../escape.txt").to_string_lossy() }),
+        &workspace,
+    )
+    .expect_err("absolute traversal out of the workspace is refused")
+    .to_string();
+    assert!(err.contains("repo-relative"), "{err}");
+
+    // A symlinked ancestor that resolves outside the workspace stays
+    // refused: the deepest-existing walk resolves through the link, the
+    // strip fails, and the raw absolute spelling is refused by the
+    // validator — the same posture as the write boundary's real-path check.
+    let outside = tempdir().expect("outside tempdir");
+    std::fs::write(outside.path().join("outside.txt"), "x").expect("outside seed");
+    #[cfg(unix)]
+    {
+        std::os::unix::fs::symlink(outside.path(), workspace.join("link")).expect("symlink");
+        let err = mutation_paths(
+            "write",
+            &json!({ "path": workspace.join("link/outside.txt").to_string_lossy() }),
+            &workspace,
+        )
+        .expect_err("symlink hop out of the workspace is refused")
+        .to_string();
+        assert!(err.contains("repo-relative"), "{err}");
+    }
+}
+
 #[test]
 fn shared_child_cwd_claims_use_one_root_namespace_for_collisions_and_mutations() {
     let repo = tempdir().expect("repo");