harivansh-afk/sandbox-agent
1
0
Fork 0
mirror of https://github.com/harivansh-afk/sandbox-agent.git synced 2026-04-15 11:02:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: address review issues in process management API

Browse source 
- Add doc comments to all 13 new #[utoipa::path] handlers (CLAUDE.md compliance)
- Fix send_signal ESRCH check: use raw_os_error() == Some(libc::ESRCH) instead of ErrorKind::NotFound
- Add max_input_bytes_per_request enforcement in WebSocket terminal handler
- URL-decode access_token query parameter for WebSocket auth
- Replace fragile string prefix matching with proper SandboxError::NotFound variant

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
Nathan Flurry 2026-03-05 15:51:07 -08:00
parent 38efa316a2
commit 8de9d8ee80
4 changed files with 124 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

13
server/packages/error/src/lib.rs
View file

@ -17,6 +17,7 @@ pub enum ErrorType {
PermissionDenied,
NotAcceptable,
UnsupportedMediaType,
NotFound,
SessionNotFound,
SessionAlreadyExists,
ModeNotSupported,
@ -37,6 +38,7 @@ impl ErrorType {
Self::PermissionDenied => "urn:sandbox-agent:error:permission_denied",
Self::NotAcceptable => "urn:sandbox-agent:error:not_acceptable",
Self::UnsupportedMediaType => "urn:sandbox-agent:error:unsupported_media_type",
Self::NotFound => "urn:sandbox-agent:error:not_found",
Self::SessionNotFound => "urn:sandbox-agent:error:session_not_found",
Self::SessionAlreadyExists => "urn:sandbox-agent:error:session_already_exists",
Self::ModeNotSupported => "urn:sandbox-agent:error:mode_not_supported",
@ -57,6 +59,7 @@ impl ErrorType {
Self::PermissionDenied => "Permission Denied",
Self::NotAcceptable => "Not Acceptable",
Self::UnsupportedMediaType => "Unsupported Media Type",
Self::NotFound => "Not Found",
Self::SessionNotFound => "Session Not Found",
Self::SessionAlreadyExists => "Session Already Exists",
Self::ModeNotSupported => "Mode Not Supported",
@ -77,6 +80,7 @@ impl ErrorType {
Self::PermissionDenied => 403,
Self::NotAcceptable => 406,
Self::UnsupportedMediaType => 415,
Self::NotFound => 404,
Self::SessionNotFound => 404,
Self::SessionAlreadyExists => 409,
Self::ModeNotSupported => 400,
@ -155,6 +159,8 @@ pub enum SandboxError {
NotAcceptable { message: String },
#[error("unsupported media type: {message}")]
UnsupportedMediaType { message: String },
#[error("not found: {resource} {id}")]
NotFound { resource: String, id: String },
#[error("session not found: {session_id}")]
SessionNotFound { session_id: String },
#[error("session already exists: {session_id}")]
@ -180,6 +186,7 @@ impl SandboxError {
Self::PermissionDenied { .. } => ErrorType::PermissionDenied,
Self::NotAcceptable { .. } => ErrorType::NotAcceptable,
Self::UnsupportedMediaType { .. } => ErrorType::UnsupportedMediaType,
Self::NotFound { .. } => ErrorType::NotFound,
Self::SessionNotFound { .. } => ErrorType::SessionNotFound,
Self::SessionAlreadyExists { .. } => ErrorType::SessionAlreadyExists,
Self::ModeNotSupported { .. } => ErrorType::ModeNotSupported,
@ -264,6 +271,12 @@ impl SandboxError {
map.insert("message".to_string(), Value::String(message.clone()));
(None, None, Some(Value::Object(map)))
}
Self::NotFound { resource, id } => {
let mut map = Map::new();
map.insert("resource".to_string(), Value::String(resource.clone()));
map.insert("id".to_string(), Value::String(id.clone()));
(None, None, Some(Value::Object(map)))
}
Self::SessionNotFound { session_id } => (None, Some(session_id.clone()), None),
Self::SessionAlreadyExists { session_id } => (None, Some(session_id.clone()), None),
Self::ModeNotSupported { agent, mode } => {

7
server/packages/sandbox-agent/src/process_runtime.rs
View file

@ -502,8 +502,9 @@ impl ProcessRuntime {
async fn lookup_process(&self, id: &str) -> Result<Arc<ManagedProcess>, SandboxError> {
let process = self.inner.processes.read().await.get(id).cloned();
process.ok_or_else(|| SandboxError::InvalidRequest {
message: format!("process not found: {id}"),
process.ok_or_else(|| SandboxError::NotFound {
resource: "process".to_string(),
id: id.to_string(),
})
}
@ -970,7 +971,7 @@ fn send_signal(pid: u32, signal: i32) -> Result<(), SandboxError> {
}
let err = std::io::Error::last_os_error();
if err.kind() == std::io::ErrorKind::NotFound {
if err.raw_os_error() == Some(libc::ESRCH) {
return Ok(());
}

61
server/packages/sandbox-agent/src/router.rs
View file

@ -1151,6 +1151,10 @@ async fn post_v1_fs_upload_batch(
}))
}
/// Get process runtime configuration.
///
/// Returns the current runtime configuration for the process management API,
/// including limits for concurrency, timeouts, and buffer sizes.
#[utoipa::path(
get,
path = "/v1/processes/config",
@ -1171,6 +1175,10 @@ async fn get_v1_processes_config(
Ok(Json(map_process_config(config)))
}
/// Update process runtime configuration.
///
/// Replaces the runtime configuration for the process management API.
/// Validates that all values are non-zero and clamps default timeout to max.
#[utoipa::path(
post,
path = "/v1/processes/config",
@ -1197,6 +1205,10 @@ async fn post_v1_processes_config(
Ok(Json(map_process_config(updated)))
}
/// Create a long-lived managed process.
///
/// Spawns a new process with the given command and arguments. Supports both
/// pipe-based and PTY (tty) modes. Returns the process descriptor on success.
#[utoipa::path(
post,
path = "/v1/processes",
@ -1232,6 +1244,10 @@ async fn post_v1_processes(
Ok(Json(map_process_snapshot(snapshot)))
}
/// Run a one-shot command.
///
/// Executes a command to completion and returns its stdout, stderr, exit code,
/// and duration. Supports configurable timeout and output size limits.
#[utoipa::path(
post,
path = "/v1/processes/run",
@ -1274,6 +1290,10 @@ async fn post_v1_processes_run(
}))
}
/// List all managed processes.
///
/// Returns a list of all processes (running and exited) currently tracked
/// by the runtime, sorted by process ID.
#[utoipa::path(
get,
path = "/v1/processes",
@ -1296,6 +1316,10 @@ async fn get_v1_processes(
}))
}
/// Get a single process by ID.
///
/// Returns the current state of a managed process including its status,
/// PID, exit code, and creation/exit timestamps.
#[utoipa::path(
get,
path = "/v1/processes/{id}",
@ -1321,6 +1345,10 @@ async fn get_v1_process(
Ok(Json(map_process_snapshot(snapshot)))
}
/// Send SIGTERM to a process.
///
/// Sends SIGTERM to the process and optionally waits up to `waitMs`
/// milliseconds for the process to exit before returning.
#[utoipa::path(
post,
path = "/v1/processes/{id}/stop",
@ -1351,6 +1379,10 @@ async fn post_v1_process_stop(
Ok(Json(map_process_snapshot(snapshot)))
}
/// Send SIGKILL to a process.
///
/// Sends SIGKILL to the process and optionally waits up to `waitMs`
/// milliseconds for the process to exit before returning.
#[utoipa::path(
post,
path = "/v1/processes/{id}/kill",
@ -1381,6 +1413,10 @@ async fn post_v1_process_kill(
Ok(Json(map_process_snapshot(snapshot)))
}
/// Delete a process record.
///
/// Removes a stopped process from the runtime. Returns 409 if the process
/// is still running; stop or kill it first.
#[utoipa::path(
delete,
path = "/v1/processes/{id}",
@ -1407,6 +1443,11 @@ async fn delete_v1_process(
Ok(StatusCode::NO_CONTENT)
}
/// Fetch process logs.
///
/// Returns buffered log entries for a process. Supports filtering by stream
/// type, tail count, and sequence-based resumption. When `follow=true`,
/// returns an SSE stream that replays buffered entries then streams live output.
#[utoipa::path(
get,
path = "/v1/processes/{id}/logs",
@ -1506,6 +1547,11 @@ async fn get_v1_process_logs(
.into_response())
}
/// Write input to a process.
///
/// Sends data to a process's stdin (pipe mode) or PTY writer (tty mode).
/// Data can be encoded as base64, utf8, or text. Returns 413 if the decoded
/// payload exceeds the configured `maxInputBytesPerRequest` limit.
#[utoipa::path(
post,
path = "/v1/processes/{id}/input",
@ -1546,6 +1592,10 @@ async fn post_v1_process_input(
Ok(Json(ProcessInputResponse { bytes_written }))
}
/// Resize a process terminal.
///
/// Sets the PTY window size (columns and rows) for a tty-mode process and
/// sends SIGWINCH so the child process can adapt.
#[utoipa::path(
post,
path = "/v1/processes/{id}/terminal/resize",
@ -1581,6 +1631,12 @@ async fn post_v1_process_terminal_resize(
}))
}
/// Open an interactive WebSocket terminal session.
///
/// Upgrades the connection to a WebSocket for bidirectional PTY I/O. Accepts
/// `access_token` query param for browser-based auth (WebSocket API cannot
/// send custom headers). Streams raw PTY output as binary frames and accepts
/// JSON control frames for input, resize, and close.
#[utoipa::path(
get,
path = "/v1/processes/{id}/terminal/ws",
@ -1677,6 +1733,11 @@ async fn process_terminal_ws_session(
continue;
}
};
let max_input = runtime.max_input_bytes().await;
if input.len() > max_input {
let _ = send_ws_error(&mut socket, &format!("input payload exceeds maxInputBytesPerRequest ({max_input})")).await;
continue;
}
if let Err(err) = runtime.write_input(&id, &input).await {
let _ = send_ws_error(&mut socket, &err.to_string()).await;
}

51
server/packages/sandbox-agent/src/router/support.rs
View file

@ -56,7 +56,47 @@ fn query_param(query: &str, key: &str) -> Option<String> {
query
.split('&')
.filter_map(|part| part.split_once('='))
.find_map(|(k, v)| if k == key { Some(v.to_string()) } else { None })
.find_map(|(k, v)| {
if k == key {
Some(percent_decode(v))
} else {
None
}
})
}
fn percent_decode(input: &str) -> String {
let mut output = Vec::with_capacity(input.len());
let bytes = input.as_bytes();
let mut i = 0;
while i < bytes.len() {
if bytes[i] == b'%' && i + 2 < bytes.len() {
if let (Some(hi), Some(lo)) = (
hex_nibble(bytes[i + 1]),
hex_nibble(bytes[i + 2]),
) {
output.push((hi << 4) | lo);
i += 3;
continue;
}
}
if bytes[i] == b'+' {
output.push(b' ');
} else {
output.push(bytes[i]);
}
i += 1;
}
String::from_utf8(output).unwrap_or_else(|_| input.to_string())
}
fn hex_nibble(b: u8) -> Option<u8> {
match b {
b'0'..=b'9' => Some(b - b'0'),
b'a'..=b'f' => Some(b - b'a' + 10),
b'A'..=b'F' => Some(b - b'A' + 10),
_ => None,
}
}
pub(super) type PinBoxSseStream = crate::acp_proxy_runtime::PinBoxSseStream;
@ -515,16 +555,17 @@ pub(super) fn problem_from_sandbox_error(error: &SandboxError) -> ProblemDetails
match error {
SandboxError::InvalidRequest { message } => {
if message.starts_with("process not found:") {
problem.status = 404;
problem.title = "Not Found".to_string();
} else if message.starts_with("input payload exceeds maxInputBytesPerRequest") {
if message.starts_with("input payload exceeds maxInputBytesPerRequest") {
problem.status = 413;
problem.title = "Payload Too Large".to_string();
} else {
problem.status = 400;
}
}
SandboxError::NotFound { .. } => {
problem.status = 404;
problem.title = "Not Found".to_string();
}
SandboxError::Timeout { .. } => {
problem.status = 504;
}