fix: require explicit workdir command cwd
This commit is contained in:
@@ -7,7 +7,8 @@ use async_trait::async_trait;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use workdir::{
|
||||
CommandHandle, CommandOutputRequest, CommandRequest, WorkdirSessionHandle, WorkdirSessionRouter,
|
||||
CommandHandle, CommandOutputRequest, CommandRequest, WorkdirPath, WorkdirSessionHandle,
|
||||
WorkdirSessionRouter,
|
||||
};
|
||||
|
||||
const DEFAULT_TIMEOUT_SECS: u64 = 120;
|
||||
@@ -19,6 +20,8 @@ struct BashParams {
|
||||
/// Worker-local alias of the Workdir attachment to use.
|
||||
#[serde(default)]
|
||||
target_workdir: Option<String>,
|
||||
/// Workdir-relative directory in which to execute the command.
|
||||
cwd: WorkdirPath,
|
||||
command: String,
|
||||
#[serde(default)]
|
||||
timeout: Option<u64>,
|
||||
@@ -129,7 +132,7 @@ impl Tool for BashTool {
|
||||
command: params.command,
|
||||
timeout_secs,
|
||||
output_limit: INLINE_BYTE_BUDGET,
|
||||
cwd: None,
|
||||
cwd: params.cwd,
|
||||
spill_dir: Some(self.output_dir.clone()),
|
||||
tool_call_id: Some(call_id.clone()),
|
||||
})
|
||||
@@ -341,7 +344,7 @@ mod tests {
|
||||
let command = "i=0; while [ $i -lt 2000 ]; do printf 'line-%04d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'";
|
||||
let result = bash
|
||||
.execute(
|
||||
&serde_json::json!({ "command": command }).to_string(),
|
||||
&serde_json::json!({ "command": command, "cwd": "." }).to_string(),
|
||||
Default::default(),
|
||||
)
|
||||
.await
|
||||
@@ -401,6 +404,24 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn missing_cwd_is_rejected_before_command_start() {
|
||||
let root = TempDir::new().unwrap();
|
||||
let output = TempDir::new().unwrap();
|
||||
let session = session_with_output_scope(&root, &output);
|
||||
let (_, bash) = bash_tool(session, output.path().to_path_buf())();
|
||||
|
||||
let error = bash
|
||||
.execute(
|
||||
&serde_json::json!({ "command": "printf hidden" }).to_string(),
|
||||
Default::default(),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(error.to_string().contains("missing field `cwd`"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn short_output_does_not_leave_a_spill_artifact() {
|
||||
let root = TempDir::new().unwrap();
|
||||
@@ -410,7 +431,7 @@ mod tests {
|
||||
|
||||
let result = bash
|
||||
.execute(
|
||||
&serde_json::json!({ "command": "printf short" }).to_string(),
|
||||
&serde_json::json!({ "command": "printf short", "cwd": "." }).to_string(),
|
||||
Default::default(),
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -534,7 +534,7 @@ mod tests {
|
||||
command: "cat /home/operator/private/session.log".to_string(),
|
||||
timeout_secs: 1,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: Some("/home/operator/private/output".into()),
|
||||
tool_call_id: None,
|
||||
});
|
||||
|
||||
@@ -62,7 +62,7 @@ async fn external_local_provider_is_strictly_read_only() {
|
||||
command: "echo changed > item.txt".to_string(),
|
||||
timeout_secs: 1,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
|
||||
+39
-20
@@ -1192,18 +1192,13 @@ impl WorkdirSession for LocalWorkdirSession {
|
||||
{
|
||||
return Err(WorkdirError::OutOfScope(spill_dir.to_path_buf()));
|
||||
}
|
||||
let cwd = if let Some(logical_cwd) = request.cwd.as_ref() {
|
||||
let cwd = self.resolve(logical_cwd);
|
||||
let scope = self.inner.scope.snapshot();
|
||||
if !scope.is_readable(&cwd)
|
||||
|| !std::fs::metadata(&cwd).is_ok_and(|metadata| metadata.is_dir())
|
||||
{
|
||||
return Err(WorkdirError::OutOfScope(cwd));
|
||||
}
|
||||
cwd
|
||||
} else {
|
||||
self.inner.cwd.clone()
|
||||
};
|
||||
let cwd = self.resolve(&request.cwd);
|
||||
let scope = self.inner.scope.snapshot();
|
||||
if !scope.is_readable(&cwd)
|
||||
|| !std::fs::metadata(&cwd).is_ok_and(|metadata| metadata.is_dir())
|
||||
{
|
||||
return Err(WorkdirError::OutOfScope(cwd));
|
||||
}
|
||||
let id = self.inner.next_command_id.fetch_add(1, Ordering::Relaxed);
|
||||
let handle = CommandHandle(format!("command-{id}"));
|
||||
let (completion_tx, completion) = watch::channel(false);
|
||||
@@ -2080,7 +2075,7 @@ mod tests {
|
||||
command: "sleep 30".to_owned(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -2777,7 +2772,7 @@ mod tests {
|
||||
command: "pwd && printf provider-command".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 4096,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -2866,6 +2861,30 @@ mod tests {
|
||||
assert!(grep.output.contains("FINAL-NEEDLE"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_rejects_explicit_cwd_outside_attachment_scope() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let outside = TempDir::new().unwrap();
|
||||
let workdir = make_fs(&dir);
|
||||
let cwd = WorkdirPath::new_scoped(outside.path().display().to_string()).unwrap();
|
||||
|
||||
let error = WorkdirSession::start_command(
|
||||
&workdir,
|
||||
CommandRequest {
|
||||
command: "printf hidden".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd,
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(matches!(error, WorkdirError::OutOfScope(path) if path == outside.path()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_rejects_spill_directory_without_read_scope() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
@@ -2878,7 +2897,7 @@ mod tests {
|
||||
command: "printf hidden".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: Some(spill.path().to_path_buf()),
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -2918,7 +2937,7 @@ mod tests {
|
||||
command: "i=0; while [ $i -lt 200 ]; do printf 'line-%03d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 64,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: Some(spill.path().to_path_buf()),
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -2965,7 +2984,7 @@ mod tests {
|
||||
command: "printf 'aéz'".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -3191,7 +3210,7 @@ mod tests {
|
||||
command: "printf ready; printf warning >&2; sleep 0.2; printf done".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-7".into()),
|
||||
},
|
||||
@@ -3296,7 +3315,7 @@ mod tests {
|
||||
command: "sleep 30".into(),
|
||||
timeout_secs: 1,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
@@ -3367,7 +3386,7 @@ mod tests {
|
||||
command: "sleep 30".into(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
},
|
||||
|
||||
@@ -16,8 +16,7 @@ pub struct CommandRequest {
|
||||
pub output_limit: usize,
|
||||
/// Workdir-relative command directory. Providers validate it against the
|
||||
/// active session before process start.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub cwd: Option<fs_operation::FsPath>,
|
||||
pub cwd: fs_operation::FsPath,
|
||||
/// Provider-local directory where complete output is retained when the
|
||||
/// inline result exceeds `output_limit`.
|
||||
pub spill_dir: Option<PathBuf>,
|
||||
|
||||
@@ -718,7 +718,7 @@ mod tests {
|
||||
command: "printf done".to_string(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: crate::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
@@ -752,7 +752,7 @@ mod tests {
|
||||
command: "sleep 30".to_string(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: crate::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
@@ -779,7 +779,7 @@ mod tests {
|
||||
command: "sleep 30".to_string(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: crate::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
|
||||
+12
-15
@@ -1094,10 +1094,7 @@ impl WorkdirSession for ScopedWorkdirSession {
|
||||
}
|
||||
let tool_call_id = request.tool_call_id.clone();
|
||||
if self.scope.is_some() {
|
||||
request.cwd = Some(match request.cwd.as_ref() {
|
||||
Some(cwd) => self.resolve_path(cwd)?,
|
||||
None => self.cwd.clone(),
|
||||
});
|
||||
request.cwd = self.resolve_path(&request.cwd)?;
|
||||
}
|
||||
if let Some(tool_call_id) = &tool_call_id {
|
||||
self.starting_tool_calls
|
||||
@@ -1677,7 +1674,7 @@ mod tests {
|
||||
command: command.into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some(tool_call_id.into()),
|
||||
})
|
||||
@@ -1706,7 +1703,7 @@ mod tests {
|
||||
command: "printf ready; sleep 0.2; printf done".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-delegated".into()),
|
||||
})
|
||||
@@ -1773,7 +1770,7 @@ mod tests {
|
||||
command: "pwd".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
@@ -1805,7 +1802,7 @@ mod tests {
|
||||
command: "pwd; sleep 0.2".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 4096,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("first-command".into()),
|
||||
})
|
||||
@@ -1907,7 +1904,7 @@ mod tests {
|
||||
command: "printf denied".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("read-only-command".into()),
|
||||
})
|
||||
@@ -2213,7 +2210,7 @@ mod tests {
|
||||
command: "printf revoked".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("revoked-child-command".into()),
|
||||
})
|
||||
@@ -2315,7 +2312,7 @@ mod tests {
|
||||
command: "sleep 30".into(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some(format!("command-{index}")),
|
||||
})
|
||||
@@ -2328,7 +2325,7 @@ mod tests {
|
||||
command: "sleep 30".into(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("command-over-limit".into()),
|
||||
})
|
||||
@@ -2362,7 +2359,7 @@ mod tests {
|
||||
command: "sleep 30".into(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("racing-command".into()),
|
||||
})
|
||||
@@ -2408,7 +2405,7 @@ mod tests {
|
||||
command: "sleep 30; printf leaked > marker".into(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("owned-command".into()),
|
||||
})
|
||||
@@ -2603,7 +2600,7 @@ mod tests {
|
||||
command: "printf closed".into(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: FsPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("closed-parent-command".into()),
|
||||
})
|
||||
|
||||
@@ -1357,7 +1357,7 @@ mod tests {
|
||||
command: "true".to_string(),
|
||||
timeout_secs: 120,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: Some("/worker-local/bash-output".into()),
|
||||
tool_call_id: Some("call-1".to_string()),
|
||||
})
|
||||
@@ -1370,6 +1370,7 @@ mod tests {
|
||||
serde_json::from_str(requests[0].body.as_deref().unwrap()).unwrap();
|
||||
assert_eq!(body["operation"]["operation"], "command_start");
|
||||
assert_eq!(body["operation"]["request"]["command"], "true");
|
||||
assert_eq!(body["operation"]["request"]["cwd"], "");
|
||||
assert!(body["operation"]["request"]["spill_dir"].is_null());
|
||||
assert_eq!(body["operation"]["request"]["tool_call_id"], "call-1");
|
||||
assert!(
|
||||
@@ -1638,7 +1639,7 @@ mod tests {
|
||||
command: "printf done".to_string(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
|
||||
@@ -345,7 +345,7 @@ async fn shutdown_closes_bound_workdir_session() {
|
||||
command: "sleep 30".to_owned(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
@@ -395,7 +395,7 @@ async fn controller_projects_workdir_command_events_and_snapshot_state() {
|
||||
command: "printf ready; sleep 0.3; printf done".to_owned(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-command-1".into()),
|
||||
})
|
||||
@@ -509,7 +509,7 @@ async fn controller_refreshes_command_snapshot_after_high_output_provider_lag()
|
||||
.to_owned(),
|
||||
timeout_secs: 10,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-high-output".into()),
|
||||
})
|
||||
@@ -591,7 +591,7 @@ async fn controller_startup_failure_closes_bound_workdir_session() {
|
||||
command: "printf unreachable".to_owned(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
|
||||
@@ -27415,7 +27415,7 @@ mod tests {
|
||||
command: "printf ready; sleep 30".to_string(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 4096,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-call-command-session".to_string()),
|
||||
})
|
||||
@@ -27529,7 +27529,7 @@ mod tests {
|
||||
command: "sleep 30".to_string(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 4096,
|
||||
cwd: None,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user