diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index a0335d4ce1..86962ddfff 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -10421,7 +10421,7 @@ pub fn load_subagent_transcript_artifact( fn remove_subagent_transcript_artifact(state_root: &Path, agent_id: &str) -> Result { let state_root = normalize_subagent_workspace(state_root); let path = checked_subagent_transcript_artifact_path(&state_root, agent_id)?; - reject_root_relative_symlinks(&state_root, &path)?; + reject_state_path_symlinks(&state_root, &path)?; let metadata = match fs::symlink_metadata(&path) { Ok(metadata) => metadata, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), @@ -10480,6 +10480,10 @@ fn checked_subagent_state_path(state_root: &Path, path: &Path) -> Result Result/.codewhale` a junction (or a +/// symlink), which resolves the child outside the state root while landing +/// exactly where that user put the state. When the link exists and the child +/// resolves inside its target, the target is the root the containment check +/// and the link walk must use; otherwise the state root is returned unchanged. +/// +/// A child that resolves anywhere else keeps the stricter state root, so this +/// re-roots one known, user-owned link and does not widen the boundary. +fn effective_state_root(state_root: &Path, state_path: &Path) -> PathBuf { + let linked = state_root.join(codewhale_config::CODEWHALE_APP_DIR); + let Ok(metadata) = fs::symlink_metadata(&linked) else { + return state_root.to_path_buf(); + }; + // Windows junctions are reparse points without the symlink tag, so the + // shared predicate is required here; `FileType::is_symlink` alone would + // miss exactly the case this exists for. + if !crate::plugins::metadata_is_link_or_reparse(&metadata) { + return state_root.to_path_buf(); + } + match resolve_path_for_containment(&linked) { + Ok(resolved) if path_is_within_state_root(state_path, &resolved) => resolved, + _ => state_root.to_path_buf(), + } +} + +/// [`reject_root_relative_symlinks`] for a path that may sit under a state +/// root the user relocated behind a link. Every guard that runs after +/// `checked_subagent_state_path` must reach the same root decision it did, or +/// a relocated root passes that check and fails the write; roots outside this +/// module's state layout come back unchanged from [`effective_state_root`]. +fn reject_state_path_symlinks(state_root: &Path, path: &Path) -> Result<()> { + let root = effective_state_root(state_root, path); + reject_root_relative_symlinks(&root, path) +} + /// Canonicalize `path` when it exists; otherwise canonicalize the deepest /// existing ancestor and re-append the missing tail. Keeps containment checks /// on the same identity spelling as [`normalize_subagent_workspace`]. @@ -10733,7 +10776,7 @@ const MAX_SUBAGENT_STATE_BYTES: u64 = 16 * 1024 * 1024; fn read_subagent_state_file(state_root: &Path, path: &Path) -> Result { let state_root = normalize_subagent_workspace(state_root); - reject_root_relative_symlinks(&state_root, path)?; + reject_state_path_symlinks(&state_root, path)?; let metadata = fs::symlink_metadata(path)?; let file_type = metadata.file_type(); if file_type.is_symlink() || !file_type.is_file() { @@ -10773,14 +10816,14 @@ fn open_subagent_state_file(path: &Path) -> Result { } fn prepare_subagent_transcript_parent(state_root: &Path, path: &Path) -> Result<()> { - reject_root_relative_symlinks(state_root, path)?; + reject_state_path_symlinks(state_root, path)?; let parent = path .parent() .ok_or_else(|| anyhow!("sub-agent transcript artifact must have a parent directory"))?; fs::create_dir_all(parent)?; // Re-check after creation so a pre-existing component cannot redirect the // private transcript outside the state root. - reject_root_relative_symlinks(state_root, path)?; + reject_state_path_symlinks(state_root, path)?; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -10803,7 +10846,7 @@ fn append_private_subagent_transcript( bytes: &[u8], durable: bool, ) -> Result<()> { - reject_root_relative_symlinks(state_root, path)?; + reject_state_path_symlinks(state_root, path)?; let mut file = open_private_subagent_transcript(path, true)?; if !bytes.is_empty() { file.write_all(bytes)?; @@ -10874,7 +10917,7 @@ static STATE_PUBLISH_SEQUENCES: std::sync::OnceLock Result<()> { let state_root = normalize_subagent_workspace(state_root); - reject_root_relative_symlinks(&state_root, path)?; + reject_state_path_symlinks(&state_root, path)?; if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; } diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 07cc44196d..3ab51affb2 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -11612,6 +11612,142 @@ fn persist_state_rejects_state_path_outside_state_root() { assert!(format!("{err:#}").contains("must stay within state root")); } +/// A user who keeps application state on another volume makes the workspace's +/// `.codewhale` a junction, so `.codewhale/state/...` resolves inside that +/// target. That target is where the state lives; refusing it failed every +/// child at step 0. +#[cfg(windows)] +#[test] +fn state_path_accepts_a_state_root_relocated_behind_a_junction() { + let tmp = tempdir().expect("tempdir"); + let workspace = tmp.path().join("workspace"); + let relocated = tmp.path().join("relocated-state"); + std::fs::create_dir_all(&workspace).expect("mkdir workspace"); + std::fs::create_dir_all(&relocated).expect("mkdir relocated"); + let link = workspace.join(".codewhale"); + let output = Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(&link) + .arg(&relocated) + .output() + .expect("invoke Windows junction creation"); + assert!( + output.status.success(), + "failed to create junction: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let state_path = checked_subagent_state_path( + &workspace, + &std::path::Path::new(".codewhale") + .join("state") + .join(SUBAGENT_STATE_FILE), + ) + .expect("a relocated state root must pass containment"); + + let resolved_target = relocated.canonicalize().expect("canonical target"); + assert!( + state_path.starts_with(&resolved_target), + "state must land in the junction target {}: {}", + resolved_target.display(), + state_path.display() + ); + + // The re-root covers that one user-owned link, not the boundary: a path + // that resolves outside both roots is still refused. + let escaped = std::path::Path::new("..") + .join("escaped") + .join(SUBAGENT_STATE_FILE); + assert!( + checked_subagent_state_path(&workspace, &escaped).is_err(), + "an escape past both roots must still fail" + ); + + // `mklink /J` accepts a target that does not exist yet, and creating the + // junction before the first run is a normal setup order. Both sides then + // resolve to the link's own spelling, so containment holds until the + // target exists. + let dangling_workspace = tmp.path().join("dangling"); + let dangling_target = tmp.path().join("not-created-yet"); + std::fs::create_dir_all(&dangling_workspace).expect("mkdir dangling workspace"); + let output = Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(dangling_workspace.join(".codewhale")) + .arg(&dangling_target) + .output() + .expect("invoke Windows junction creation"); + assert!( + output.status.success(), + "failed to create dangling junction: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !dangling_target.exists(), + "the probe needs a target that does not exist yet" + ); + checked_subagent_state_path( + &dangling_workspace, + &std::path::Path::new(".codewhale") + .join("state") + .join(SUBAGENT_STATE_FILE), + ) + .expect("a junction whose target is not created yet must pass containment"); +} + +/// The guards that run after `checked_subagent_state_path` must reach the same +/// root decision it did. Before they did, the containment check passed and the +/// write still failed with the same message on the child's first step. +#[cfg(windows)] +#[test] +fn state_writes_follow_a_relocated_state_root() { + let tmp = tempdir().expect("tempdir"); + let workspace = tmp.path().join("workspace"); + let relocated = tmp.path().join("relocated-state"); + std::fs::create_dir_all(&workspace).expect("mkdir workspace"); + std::fs::create_dir_all(&relocated).expect("mkdir relocated"); + let link = workspace.join(".codewhale"); + let output = Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(&link) + .arg(&relocated) + .output() + .expect("invoke Windows junction creation"); + assert!( + output.status.success(), + "failed to create junction: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // The manager state file: write_json_atomic and read_subagent_state_file. + let mut manager = SubAgentManager::new(workspace.clone(), 1); + manager + .persist_state_synchronously() + .expect("persist under a relocated state root"); + manager + .load_state() + .expect("load under a relocated state root"); + + // The child's transcript: the pre-create and append guards. + let state_root = normalize_subagent_workspace(&workspace); + let writer = SubAgentTranscriptArtifactWriter::create(&state_root, "agent_relocated") + .expect("transcript create under a relocated state root"); + append_private_subagent_transcript( + &state_root, + &writer.path, + b"{\"kind\":\"message\"}\n", + true, + ) + .expect("transcript append under a relocated state root"); + + let resolved_target = relocated.canonicalize().expect("canonical target"); + assert!( + writer.path.starts_with(&resolved_target), + "the transcript must land in the junction target {}: {}", + resolved_target.display(), + writer.path.display() + ); +} + #[test] fn explicit_state_roots_isolate_managers_for_the_same_execution_workspace() { let tmp = tempdir().expect("tempdir");