From 24be69918d459336bd8bc846ea18721b98cd5030 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 29 Sep 2026 05:53:32 +0900 Subject: [PATCH] feat: add interactive read-write external workdir sharing --- crates/fs-operation/src/lib.rs | 169 +++++++ crates/fs-operation/src/local.rs | 48 +- crates/server-api/src/lib.rs | 36 +- crates/workdir/src/external.rs | 82 +++- crates/workdir/src/external_local_tests.rs | 121 +++++ crates/workdir/src/lib.rs | 8 + crates/workdir/src/local.rs | 101 +++- crates/workspace-server/src/latest_schema.sql | 4 +- crates/workspace-server/src/server.rs | 439 ++++++++++++++---- crates/workspace-server/src/store.rs | 160 ++++++- crates/yoi/src/main.rs | 168 ++++--- crates/yoi/src/workdir_share.rs | 416 ++++++++++++++++- 12 files changed, 1510 insertions(+), 242 deletions(-) diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index 51f4cd7ea..971958418 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -154,6 +154,105 @@ pub fn open_beneath_no_symlinks(_root: &Path, _path: &Path) -> std::io::Result std::io::Result<()>, +) -> std::io::Result<()> { + use std::ffi::{CString, OsString}; + use std::io::Write; + use std::os::fd::{AsRawFd, FromRawFd, RawFd}; + use std::os::unix::ffi::OsStrExt; + + if relative.is_absolute() { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "path is outside provider root", + )); + } + let mut components = relative.components().peekable(); + let mut parent = root.try_clone()?; + let mut file_name = None::; + while let Some(component) = components.next() { + let std::path::Component::Normal(name) = component else { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "path is outside provider root", + )); + }; + if components.peek().is_none() { + file_name = Some(name.to_os_string()); + break; + } + let name = CString::new(name.as_bytes()).map_err(|_| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, "path contains NUL") + })?; + // SAFETY: `name` is a valid C string and `parent` remains open. + let mut fd: RawFd = unsafe { + libc::openat( + parent.as_raw_fd(), + name.as_ptr(), + libc::O_PATH | libc::O_DIRECTORY | libc::O_CLOEXEC | libc::O_NOFOLLOW, + ) + }; + if fd < 0 { + let error = std::io::Error::last_os_error(); + if error.kind() != std::io::ErrorKind::NotFound { + return Err(error); + } + // SAFETY: same valid directory descriptor and component C string. + let created = unsafe { libc::mkdirat(parent.as_raw_fd(), name.as_ptr(), 0o755) }; + if created < 0 { + let create_error = std::io::Error::last_os_error(); + if create_error.kind() != std::io::ErrorKind::AlreadyExists { + return Err(create_error); + } + } + // SAFETY: same as the first openat; a racing symlink is rejected by O_NOFOLLOW. + fd = unsafe { + libc::openat( + parent.as_raw_fd(), + name.as_ptr(), + libc::O_PATH | libc::O_DIRECTORY | libc::O_CLOEXEC | libc::O_NOFOLLOW, + ) + }; + if fd < 0 { + return Err(std::io::Error::last_os_error()); + } + } + // SAFETY: fd is a fresh descriptor returned by openat. + parent = unsafe { std::fs::File::from_raw_fd(fd) }; + } + let file_name = file_name.ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, "file path is empty") + })?; + let parent_path = PathBuf::from(format!("/proc/self/fd/{}", parent.as_raw_fd())); + let mut temporary = tempfile::NamedTempFile::new_in(&parent_path)?; + temporary.write_all(content)?; + temporary.flush()?; + temporary.as_file().sync_all()?; + before_commit()?; + temporary + .persist(parent_path.join(file_name)) + .map_err(|error| error.error)?; + Ok(()) +} + +#[cfg(not(target_os = "linux"))] +pub fn atomic_write_beneath_no_symlinks_at( + _root: &std::fs::File, + _relative: &Path, + _content: &[u8], + _before_commit: impl FnOnce() -> std::io::Result<()>, +) -> std::io::Result<()> { + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "root-confined no-symlink write is unavailable on this platform", + )) +} + /// Keeps a descriptor-backed traversal path alive for the duration of a walk. #[derive(Debug)] pub struct FsTraversalRoot { @@ -218,6 +317,30 @@ pub trait FsAccessPolicy: Send + Sync { std::fs::metadata(resolved) } + /// Atomically replace or create an already-authorized writable file. + /// Capability providers override this to keep parent traversal descriptor- + /// confined through the final rename. + fn atomic_write_file( + &self, + _logical: &Path, + resolved: &Path, + content: &[u8], + ) -> std::io::Result<()> { + use std::io::Write; + + let parent = resolved.parent().ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, "file path has no parent") + })?; + std::fs::create_dir_all(parent)?; + let mut temporary = tempfile::NamedTempFile::new_in(parent)?; + temporary.write_all(content)?; + temporary.flush()?; + temporary.as_file().sync_all()?; + self.check_cancelled()?; + temporary.persist(resolved).map_err(|error| error.error)?; + Ok(()) + } + /// Open an already-authorized directory for bounded enumeration. /// Capability providers override this to bind traversal to a confined /// directory descriptor rather than reopening a mutable path. @@ -1026,6 +1149,52 @@ mod tests { )); } + #[cfg(target_os = "linux")] + #[test] + fn descriptor_confined_atomic_write_rejects_symlinks_and_survives_root_rename() { + use std::os::unix::fs::symlink; + + let parent = tempfile::tempdir().unwrap(); + let selected = parent.path().join("selected"); + std::fs::create_dir(&selected).unwrap(); + let outside = tempfile::tempdir().unwrap(); + std::fs::write(outside.path().join("secret.txt"), "secret").unwrap(); + symlink(outside.path(), selected.join("escape")).unwrap(); + let root = open_root_no_symlinks(&selected).unwrap(); + + let error = atomic_write_beneath_no_symlinks_at( + &root, + Path::new("escape/secret.txt"), + b"escaped", + || Ok(()), + ) + .unwrap_err(); + assert!(matches!( + error.kind(), + std::io::ErrorKind::NotADirectory | std::io::ErrorKind::PermissionDenied + )); + assert_eq!( + std::fs::read_to_string(outside.path().join("secret.txt")).unwrap(), + "secret" + ); + + std::fs::rename(&selected, parent.path().join("approved-original")).unwrap(); + std::fs::create_dir(&selected).unwrap(); + atomic_write_beneath_no_symlinks_at( + &root, + Path::new("nested/created.txt"), + b"approved", + || Ok(()), + ) + .unwrap(); + assert_eq!( + std::fs::read_to_string(parent.path().join("approved-original/nested/created.txt")) + .unwrap(), + "approved" + ); + assert!(!selected.join("nested/created.txt").exists()); + } + #[test] fn grep_content_groups_lines_by_file_and_marks_matches() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/fs-operation/src/local.rs b/crates/fs-operation/src/local.rs index 553271171..48ffa4d37 100644 --- a/crates/fs-operation/src/local.rs +++ b/crates/fs-operation/src/local.rs @@ -1,6 +1,6 @@ use std::ffi::OsString; use std::fs; -use std::io::{BufReader, Read, Write}; +use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use sha2::{Digest, Sha256}; @@ -208,21 +208,32 @@ pub fn run_write( let created = !path.exists(); if path.exists() { let target = require_access(&path, &logical, access, true, false)?; - let metadata = fs::metadata(&target).map_err(|error| map_io(&logical, error))?; + let metadata = access + .read_metadata(&path, &target) + .map_err(|error| map_io(&logical, error))?; if metadata.is_dir() { return Err(FsError::IsDirectory(PathBuf::from(logical.as_str()))); } - let actual = hash_bytes(&fs::read(&target).map_err(|error| map_io(&logical, error))?); + let mut bytes = Vec::new(); + access + .open_read_file(&path, &target) + .and_then(|mut file| file.read_to_end(&mut bytes)) + .map_err(|error| map_io(&logical, error))?; + let actual = hash_bytes(&bytes); if request.expected_hash != Some(actual) { return Err(FsError::Conflict(logical.as_str().to_string())); } - atomic_write(&target, &request.content, &logical)?; + access + .atomic_write_file(&path, &target, &request.content) + .map_err(|error| map_io(&logical, error))?; } else { if request.expected_hash.is_some() { return Err(FsError::Conflict(logical.as_str().to_string())); } let target = require_access(&path, &logical, access, true, true)?; - atomic_write(&target, &request.content, &logical)?; + access + .atomic_write_file(&path, &target, &request.content) + .map_err(|error| map_io(&logical, error))?; } Ok(WriteResult { bytes_written: request.content.len(), @@ -241,7 +252,11 @@ pub fn run_edit( .map_err(|error| map_io(&logical, error))?; let path = resolve(root, &logical)?; let target = require_access(&path, &logical, access, true, false)?; - let bytes = fs::read(&target).map_err(|error| map_io(&logical, error))?; + let mut bytes = Vec::new(); + access + .open_read_file(&path, &target) + .and_then(|mut file| file.read_to_end(&mut bytes)) + .map_err(|error| map_io(&logical, error))?; let actual_hash = hash_bytes(&bytes); if actual_hash != request.expected_hash { return Err(FsError::Conflict(logical.as_str().to_string())); @@ -265,7 +280,9 @@ pub fn run_edit( } else { content.replacen(&request.old_string, &request.new_string, 1) }; - atomic_write(&target, edited.as_bytes(), &logical)?; + access + .atomic_write_file(&path, &target, edited.as_bytes()) + .map_err(|error| map_io(&logical, error))?; Ok(EditResult { replacements: if request.replace_all { occurrences } else { 1 }, bytes_written: edited.len(), @@ -464,23 +481,6 @@ pub fn resolve_access_path(path: &Path) -> std::io::Result { } } -fn atomic_write(path: &Path, content: &[u8], logical: &FsPath) -> Result<(), FsError> { - let parent = path - .parent() - .ok_or_else(|| FsError::InvalidArgument(format!("{} has no parent", logical.as_str())))?; - fs::create_dir_all(parent).map_err(|error| map_io(logical, error))?; - let mut temporary = - tempfile::NamedTempFile::new_in(parent).map_err(|error| map_io(logical, error))?; - temporary - .write_all(content) - .map_err(|error| map_io(logical, error))?; - temporary.flush().map_err(|error| map_io(logical, error))?; - temporary - .persist(path) - .map_err(|error| map_io(logical, error.error))?; - Ok(()) -} - fn hash_bytes(content: &[u8]) -> ContentHash { Sha256::digest(content).into() } diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index 0ae2d1de6..4790c3f4e 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -5241,19 +5241,32 @@ impl From for WorkingDirectorySumma } } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] -#[serde(deny_unknown_fields)] -pub struct ExternalWorkdirGrantCreateRequest { - pub provider_instance_id: String, - pub display_name: String, - #[schemars(range(min = 0, max = 9_007_199_254_740_991_u64))] - pub ttl_seconds: u64, - pub read_only: bool, +#[serde(rename_all = "snake_case")] +pub enum ExternalWorkdirPermission { + #[default] + ReadOnly, + ReadWrite, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(optional_fields = nullable))] +#[serde(deny_unknown_fields)] +pub struct ExternalWorkdirGrantCreateRequest { + pub provider_instance_id: String, + pub display_name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "typescript", ts(optional, type = "number | null"))] + #[schemars(range(min = 0, max = 9_007_199_254_740_991_u64))] + pub ttl_seconds: Option, + pub permission: ExternalWorkdirPermission, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[cfg_attr(feature = "typescript", ts(optional_fields = nullable))] #[serde(deny_unknown_fields)] pub struct ExternalWorkdirGrantResponse { pub grant_id: String, @@ -5261,8 +5274,11 @@ pub struct ExternalWorkdirGrantResponse { pub working_directory_id: String, pub provider_instance_id: String, pub display_name: String, - pub permissions: String, - pub expires_at: String, + pub permission: ExternalWorkdirPermission, + /// `None` means the grant has no automatic expiry. It remains bounded by + /// explicit revoke, provider disconnect, and provider shutdown cleanup. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub expires_at: Option, #[schemars(range(min = 0, max = 9_007_199_254_740_991_u64))] pub generation: u64, pub status: String, diff --git a/crates/workdir/src/external.rs b/crates/workdir/src/external.rs index fb42e1244..1b48c57ef 100644 --- a/crates/workdir/src/external.rs +++ b/crates/workdir/src/external.rs @@ -25,6 +25,7 @@ const MAX_EXTERNAL_PATTERN_BYTES: usize = 4 * 1024; const MAX_EXTERNAL_RESULT_ITEMS: usize = 10_000; const MAX_EXTERNAL_GREP_CONTEXT: usize = 100; const MAX_EXTERNAL_SCOPE_RULES: usize = 64; +pub const MAX_EXTERNAL_WRITE_BYTES: usize = 512 * 1024; /// Fail-closed wire version for the External Workdir provider protocol. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] @@ -122,7 +123,10 @@ fn is_root_relative(path: &WorkdirPath) -> bool { }) } -fn validate_read_only_operation(operation: &WorkdirSessionOperation) -> Result<(), String> { +/// Validate the filesystem-only subset shared by read-only and read-write +/// External Workdirs. Capability enforcement remains a separate grant/session +/// boundary; command operations never enter the provider protocol. +fn validate_filesystem_operation(operation: &WorkdirSessionOperation) -> Result<(), String> { let path_is_valid = match operation { WorkdirSessionOperation::AuthorizeScope(request) => { is_root_relative(&request.path) @@ -165,13 +169,24 @@ fn validate_read_only_operation(operation: &WorkdirSessionOperation) -> Result<( && request.limit <= MAX_EXTERNAL_RESULT_ITEMS && request.offset <= 1_000_000 } - WorkdirSessionOperation::Write(_) - | WorkdirSessionOperation::Edit(_) - | WorkdirSessionOperation::CommandStart(_) + WorkdirSessionOperation::Write(request) => { + is_root_relative(&request.path) && request.content.len() <= MAX_EXTERNAL_WRITE_BYTES + } + WorkdirSessionOperation::Edit(request) => { + is_root_relative(&request.path) + && request.old_string.len() <= MAX_EXTERNAL_WRITE_BYTES + && request.new_string.len() <= MAX_EXTERNAL_WRITE_BYTES + && request + .old_string + .len() + .checked_add(request.new_string.len()) + .is_some_and(|size| size <= MAX_EXTERNAL_WRITE_BYTES) + } + WorkdirSessionOperation::CommandStart(_) | WorkdirSessionOperation::CommandStatus(_) | WorkdirSessionOperation::CommandOutput(_) | WorkdirSessionOperation::CommandCancel(_) => { - return Err("operation is not available to a read-only External Workdir".to_string()); + return Err("command operations are not available to an External Workdir".to_string()); } }; if path_is_valid { @@ -181,7 +196,7 @@ fn validate_read_only_operation(operation: &WorkdirSessionOperation) -> Result<( } } -/// Validated read-only subset of the shared Workdir operation contract. +/// Validated filesystem-only subset of the shared Workdir operation contract. #[derive(Clone, Debug, PartialEq, Eq, Serialize)] #[serde(transparent)] pub struct ExternalWorkdirOperation(WorkdirSessionOperation); @@ -200,7 +215,7 @@ impl TryFrom for ExternalWorkdirOperation { type Error = String; fn try_from(operation: WorkdirSessionOperation) -> Result { - validate_read_only_operation(&operation)?; + validate_filesystem_operation(&operation)?; Ok(Self(operation)) } } @@ -224,7 +239,7 @@ fn result_paths_fit<'a>(paths: impl IntoIterator) -> boo .is_some_and(|retained| retained <= fs_operation::MAX_RESULT_PATH_BYTES) } -fn validate_read_only_result(result: &WorkdirSessionOperationResult) -> Result<(), String> { +fn validate_filesystem_result(result: &WorkdirSessionOperationResult) -> Result<(), String> { let result_is_valid = match result { WorkdirSessionOperationResult::AuthorizeScope | WorkdirSessionOperationResult::ScopeRulesOverlap { .. } => true, @@ -251,14 +266,19 @@ fn validate_read_only_result(result: &WorkdirSessionOperationResult) -> Result<( && result.match_count <= MAX_EXTERNAL_RESULT_ITEMS && result.matched_files <= MAX_EXTERNAL_RESULT_ITEMS } - WorkdirSessionOperationResult::Write(_) - | WorkdirSessionOperationResult::Edit(_) - | WorkdirSessionOperationResult::CommandStart(_) + WorkdirSessionOperationResult::Write(result) => { + result.bytes_written <= MAX_EXTERNAL_WRITE_BYTES + } + WorkdirSessionOperationResult::Edit(result) => { + result.bytes_written <= MAX_EXTERNAL_WRITE_BYTES + && result.replacements <= MAX_EXTERNAL_RESULT_ITEMS + } + WorkdirSessionOperationResult::CommandStart(_) | WorkdirSessionOperationResult::CommandStatus(_) | WorkdirSessionOperationResult::CommandOutput(_) | WorkdirSessionOperationResult::CommandCancel => { return Err( - "operation result is not available to a read-only External Workdir".to_string(), + "command operation results are not available to an External Workdir".to_string(), ); } }; @@ -269,7 +289,7 @@ fn validate_read_only_result(result: &WorkdirSessionOperationResult) -> Result<( } } -/// Validated read-only subset of the shared Workdir operation result contract. +/// Validated filesystem-only subset of the shared Workdir operation result contract. #[derive(Clone, Debug, PartialEq, Eq, Serialize)] #[serde(transparent)] pub struct ExternalWorkdirOperationResult(WorkdirSessionOperationResult); @@ -288,7 +308,7 @@ impl TryFrom for ExternalWorkdirOperationResult { type Error = String; fn try_from(result: WorkdirSessionOperationResult) -> Result { - validate_read_only_result(&result)?; + validate_filesystem_result(&result)?; Ok(Self(result)) } } @@ -411,7 +431,10 @@ mod tests { use serde_json::Value; use super::*; - use crate::{CommandRequest, GrepResult, ListRequest, ReadRequest, Workdir, WorkdirPath}; + use crate::{ + CommandRequest, EditRequest, GrepResult, ListRequest, ReadRequest, Workdir, WorkdirPath, + WriteRequest, + }; fn assert_no_provider_authority(value: &Value) { match value { @@ -521,7 +544,7 @@ mod tests { } #[test] - fn read_only_protocol_rejects_host_paths_and_command_payloads() { + fn filesystem_protocol_rejects_host_paths_and_commands_but_allows_bounded_mutations() { let absolute = WorkdirSessionOperation::Read(ReadRequest { path: WorkdirPath::new_scoped("/home/operator/private/session.log").unwrap(), offset: 0, @@ -539,6 +562,33 @@ mod tests { tool_call_id: None, }); assert!(ExternalWorkdirOperation::try_from(command).is_err()); + + assert!( + ExternalWorkdirOperation::try_from(WorkdirSessionOperation::Write(WriteRequest { + path: WorkdirPath::new("created.txt").unwrap(), + content: b"bounded".to_vec(), + expected_hash: None, + })) + .is_ok() + ); + assert!( + ExternalWorkdirOperation::try_from(WorkdirSessionOperation::Edit(EditRequest { + path: WorkdirPath::new("item.txt").unwrap(), + old_string: "old".to_string(), + new_string: "new".to_string(), + replace_all: false, + expected_hash: [0; 32], + })) + .is_ok() + ); + assert!( + ExternalWorkdirOperation::try_from(WorkdirSessionOperation::Write(WriteRequest { + path: WorkdirPath::new("large.txt").unwrap(), + content: vec![0; MAX_EXTERNAL_WRITE_BYTES + 1], + expected_hash: None, + })) + .is_err() + ); } #[test] diff --git a/crates/workdir/src/external_local_tests.rs b/crates/workdir/src/external_local_tests.rs index bd962bf89..1ee8d8eac 100644 --- a/crates/workdir/src/external_local_tests.rs +++ b/crates/workdir/src/external_local_tests.rs @@ -79,6 +79,82 @@ async fn external_local_provider_is_strictly_read_only() { ); } +#[tokio::test] +async fn external_local_provider_read_write_is_filesystem_only() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("item.txt"), "original").unwrap(); + let session = LocalWorkdirSession::external_read_write( + Workdir::new("external-workdir-rw"), + root.path(), + BoundedReadLimits::new(4096, 1024).unwrap(), + ) + .unwrap(); + + assert_eq!(session.capabilities(), WorkdirSessionCapabilities::READ_WRITE); + assert!(!session.capabilities().supports(WorkdirSessionCapability::Command)); + + let observed = WorkdirSession::read( + &session, + ReadRequest { + path: WorkdirPath::new("item.txt").unwrap(), + offset: 0, + limit: 10, + max_bytes: 1024, + }, + ) + .await + .unwrap(); + WorkdirSession::edit( + &session, + EditRequest { + path: WorkdirPath::new("item.txt").unwrap(), + old_string: "original".to_string(), + new_string: "changed".to_string(), + replace_all: false, + expected_hash: observed.content_hash, + }, + ) + .await + .unwrap(); + WorkdirSession::write( + &session, + WriteRequest { + path: WorkdirPath::new("created.txt").unwrap(), + content: b"created".to_vec(), + expected_hash: None, + }, + ) + .await + .unwrap(); + + assert_eq!( + std::fs::read_to_string(root.path().join("item.txt")).unwrap(), + "changed" + ); + assert_eq!( + std::fs::read_to_string(root.path().join("created.txt")).unwrap(), + "created" + ); + let command = WorkdirSession::start_command( + &session, + CommandRequest { + command: "touch command-ran".to_string(), + timeout_secs: 1, + output_limit: 1024, + cwd: WorkdirPath::root(), + spill_dir: None, + tool_call_id: None, + }, + ) + .await + .unwrap_err(); + assert!(matches!( + command, + WorkdirError::Unsupported(WorkdirSessionCapability::Command) + )); + assert!(!root.path().join("command-ran").exists()); +} + #[tokio::test] async fn common_dispatcher_preserves_operation_result_pairing_and_capabilities() { let root = tempfile::tempdir().unwrap(); @@ -228,6 +304,51 @@ async fn external_local_provider_rejects_symlink_roots_and_traversal() { assert!(matches!(error, WorkdirError::Denied(_))); } +#[cfg(unix)] +#[tokio::test] +async fn external_read_write_provider_rejects_symlink_mutation_escape() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + std::fs::write(outside.path().join("secret.txt"), "secret").unwrap(); + symlink( + outside.path().join("secret.txt"), + root.path().join("outside.txt"), + ) + .unwrap(); + let session = LocalWorkdirSession::external_read_write( + Workdir::new("external-workdir-rw-symlink"), + root.path(), + BoundedReadLimits::new(4096, 1024).unwrap(), + ) + .unwrap(); + + let error = WorkdirSession::write( + &session, + WriteRequest { + path: WorkdirPath::new("outside.txt").unwrap(), + content: b"escaped".to_vec(), + expected_hash: None, + }, + ) + .await + .unwrap_err(); + assert!( + matches!( + error, + WorkdirError::SymlinkOutOfScope { .. } + | WorkdirError::OutOfScope(_) + | WorkdirError::Denied(_) + ), + "unexpected error: {error:?}" + ); + assert_eq!( + std::fs::read_to_string(outside.path().join("secret.txt")).unwrap(), + "secret" + ); +} + #[cfg(target_os = "linux")] #[tokio::test] async fn external_grep_honors_gitignore_above_a_nested_search_root() { diff --git a/crates/workdir/src/lib.rs b/crates/workdir/src/lib.rs index 62b613d45..70a0b5a62 100644 --- a/crates/workdir/src/lib.rs +++ b/crates/workdir/src/lib.rs @@ -134,6 +134,14 @@ impl WorkdirSessionCapabilities { bits: Self::READ | Self::GLOB | Self::GREP, }; + /// Filesystem read/write authority without process execution. + /// + /// External Workdirs use this capability set so operator-approved file + /// mutation never implicitly grants command or shell authority. + pub const READ_WRITE: Self = Self { + bits: Self::READ | Self::WRITE | Self::EDIT | Self::GLOB | Self::GREP, + }; + pub const fn union(self, other: Self) -> Self { Self { bits: self.bits | other.bits, diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index 819e61f52..14af88322 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -347,6 +347,44 @@ impl fs_operation::FsAccessPolicy for ScopeAccess { } } + fn atomic_write_file( + &self, + _logical: &Path, + resolved: &Path, + content: &[u8], + ) -> std::io::Result<()> { + if !self.reject_symlinks { + use std::io::Write; + + let parent = resolved.parent().ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidInput, "file path has no parent") + })?; + std::fs::create_dir_all(parent)?; + let mut temporary = tempfile::NamedTempFile::new_in(parent)?; + temporary.write_all(content)?; + temporary.flush()?; + temporary.as_file().sync_all()?; + self.check_cancelled()?; + temporary.persist(resolved).map_err(|error| error.error)?; + return Ok(()); + } + let root = self.pinned_root.as_ref().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::Unsupported, + "External Workdir root handle is unavailable", + ) + })?; + let relative = resolved.strip_prefix(&self.root).map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "path is outside provider root", + ) + })?; + fs_operation::atomic_write_beneath_no_symlinks_at(root, relative, content, || { + self.check_cancelled() + }) + } + fn open_read_dir(&self, _logical: &Path, resolved: &Path) -> std::io::Result { if !self.reject_symlinks { return std::fs::read_dir(resolved); @@ -686,11 +724,6 @@ impl LocalWorkdirSession { /// Construct a strictly read-only provider session for an /// authority-assigned Workdir and an operator-selected local directory. - /// - /// The selected directory is atomically validated and pinned for the - /// session lifetime. A symlink selected as the root is rejected, and all - /// operation paths are confined below the pinned root without following - /// symbolic links. Read source and response sizes are provider-bounded. pub fn external_read_only( workdir: Workdir, directory: impl AsRef, @@ -700,20 +733,66 @@ impl LocalWorkdirSession { Self::external_read_only_pinned(workdir, root, read_limits) } - /// Construct a provider session from a root pinned before remote grant - /// creation, preventing any later pathname replacement from changing the - /// approved filesystem authority. + /// Construct a filesystem read/write provider session without command + /// authority for an operator-selected local directory. + pub fn external_read_write( + workdir: Workdir, + directory: impl AsRef, + read_limits: BoundedReadLimits, + ) -> Result { + let root = ExternalWorkdirRoot::pin(directory)?; + Self::external_read_write_pinned(workdir, root, read_limits) + } + + /// Construct a read-only provider session from a root pinned before remote + /// grant creation. pub fn external_read_only_pinned( workdir: Workdir, pinned: ExternalWorkdirRoot, read_limits: BoundedReadLimits, + ) -> Result { + Self::external_pinned( + workdir, + pinned, + read_limits, + Permission::Read, + WorkdirSessionCapabilities::READ_ONLY, + ) + } + + /// Construct a filesystem read/write provider session from a root pinned + /// before remote grant creation. Command execution remains unavailable. + pub fn external_read_write_pinned( + workdir: Workdir, + pinned: ExternalWorkdirRoot, + read_limits: BoundedReadLimits, + ) -> Result { + Self::external_pinned( + workdir, + pinned, + read_limits, + Permission::Write, + WorkdirSessionCapabilities::READ_WRITE, + ) + } + + /// The selected directory is atomically validated and pinned for the + /// session lifetime. A symlink selected as the root is rejected, and all + /// operation paths are confined below the pinned root without following + /// symbolic links. Read source and response sizes are provider-bounded. + fn external_pinned( + workdir: Workdir, + pinned: ExternalWorkdirRoot, + read_limits: BoundedReadLimits, + permission: Permission, + capabilities: WorkdirSessionCapabilities, ) -> Result { read_limits.validate().map_err(WorkdirError::from)?; let root = pinned.canonical_path; let scope = Scope::from_config(&ScopeConfig { allow: vec![ScopeRule { target: root.clone(), - permission: Permission::Read, + permission, recursive: true, symlink_policy: SymlinkPolicy::Resolved, }], @@ -721,7 +800,7 @@ impl LocalWorkdirSession { }) .map_err(|_| { WorkdirError::InvalidArgument( - "External Workdir read scope could not be established".to_string(), + "External Workdir filesystem scope could not be established".to_string(), ) })?; let scope = SharedScope::new(scope); @@ -731,7 +810,7 @@ impl LocalWorkdirSession { root, scope.clone(), scope, - WorkdirSessionCapabilities::READ_ONLY, + capabilities, BTreeMap::new(), Vec::new(), Some(pinned.handle), diff --git a/crates/workspace-server/src/latest_schema.sql b/crates/workspace-server/src/latest_schema.sql index b6b8b44c4..674bee27d 100644 --- a/crates/workspace-server/src/latest_schema.sql +++ b/crates/workspace-server/src/latest_schema.sql @@ -697,10 +697,10 @@ CREATE TABLE external_workdir_grants ( workdir_id TEXT NOT NULL, provider_instance_id TEXT NOT NULL, display_name TEXT NOT NULL, - permissions TEXT NOT NULL CHECK (permissions = 'read_only'), + permissions TEXT NOT NULL CHECK (permissions IN ('read_only', 'read_write')), created_by TEXT NOT NULL, created_at TEXT NOT NULL, - expires_at TEXT NOT NULL, + expires_at TEXT, generation INTEGER NOT NULL CHECK (generation > 0), status TEXT NOT NULL CHECK (status IN ('pending', 'online', 'offline', 'revoked', 'expired')), updated_at TEXT NOT NULL, diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 4d19f933b..50f2745f9 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -729,7 +729,7 @@ struct ExternalProviderConnection { workdir_id: String, provider_instance_id: String, generation: u64, - expires_at: chrono::DateTime, + expires_at: Option>, capabilities: workdir::WorkdirSessionCapabilities, admission: Arc, sender: tokio::sync::mpsc::Sender, @@ -861,7 +861,11 @@ impl ExternalProviderWorkdirSession { if self.closed.load(Ordering::Acquire) { return Err(workdir::WorkdirError::SessionClosed); } - if Utc::now() >= self.connection.expires_at { + if self + .connection + .expires_at + .is_some_and(|expires_at| Utc::now() >= expires_at) + { return Err(workdir::WorkdirError::Unavailable( "External Workdir grant has expired".to_string(), )); @@ -877,7 +881,11 @@ impl ExternalProviderWorkdirSession { "External Workdir provider admission is closed".to_string(), ) })?; - if Utc::now() >= self.connection.expires_at { + if self + .connection + .expires_at + .is_some_and(|expires_at| Utc::now() >= expires_at) + { return Err(workdir::WorkdirError::Unavailable( "External Workdir grant has expired".to_string(), )); @@ -1020,19 +1028,42 @@ impl workdir::WorkdirSession for ExternalProviderWorkdirSession { } async fn write( &self, - _request: workdir::WriteRequest, + request: workdir::WriteRequest, ) -> std::result::Result { - Err(workdir::WorkdirError::Unsupported( - workdir::WorkdirSessionCapability::Write, - )) + if !self + .connection + .capabilities + .supports(workdir::WorkdirSessionCapability::Write) + { + return Err(workdir::WorkdirError::Unsupported( + workdir::WorkdirSessionCapability::Write, + )); + } + match self + .operate(WorkdirSessionOperation::Write(request)) + .await? + { + WorkdirSessionOperationResult::Write(value) => Ok(value), + _ => Err(Self::mismatch("write")), + } } async fn edit( &self, - _request: workdir::EditRequest, + request: workdir::EditRequest, ) -> std::result::Result { - Err(workdir::WorkdirError::Unsupported( - workdir::WorkdirSessionCapability::Edit, - )) + if !self + .connection + .capabilities + .supports(workdir::WorkdirSessionCapability::Edit) + { + return Err(workdir::WorkdirError::Unsupported( + workdir::WorkdirSessionCapability::Edit, + )); + } + match self.operate(WorkdirSessionOperation::Edit(request)).await? { + WorkdirSessionOperationResult::Edit(value) => Ok(value), + _ => Err(Self::mismatch("edit")), + } } async fn list( &self, @@ -3415,9 +3446,7 @@ impl WorkspaceApi { code: "external_workdir_unavailable".to_string(), message: "External Workdir grant is unavailable".to_string(), })?; - let grant_expired = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|expires_at| expires_at.with_timezone(&Utc) <= Utc::now()) - .unwrap_or(true); + let grant_expired = external_workdir_grant_is_expired(&grant)?; let connection = self .external_workdir_providers .lock() @@ -3429,7 +3458,9 @@ impl WorkspaceApi { connection.generation == grant.generation && connection.provider_instance_id == grant.provider_instance_id && connection.workdir_id == link.workdir_id - && connection.expires_at > Utc::now() + && connection + .expires_at + .is_none_or(|expires_at| expires_at > Utc::now()) }) { return Err(Error::RuntimeOperationFailed { @@ -3519,7 +3550,7 @@ impl WorkspaceApi { claim.working_directory_id ))) })?; - claim.capabilities = workdir_source_capabilities(&workdir); + claim.capabilities = workdir_source_capabilities(self, &workdir)?; } return Ok(()); }; @@ -3607,7 +3638,7 @@ impl WorkspaceApi { } claim.capabilities = target .capabilities - .intersection(workdir_source_capabilities(&workdir)); + .intersection(workdir_source_capabilities(self, &workdir)?); } let missing = targets @@ -3643,14 +3674,27 @@ struct ValidatedTicketImplementationTarget { capabilities: workdir::WorkdirSessionCapabilities, } +fn external_workdir_capabilities(permissions: &str) -> Result { + match permissions { + "read_only" => Ok(workdir::WorkdirSessionCapabilities::READ_ONLY), + "read_write" => Ok(workdir::WorkdirSessionCapabilities::READ_WRITE), + _ => Err(Error::Store( + "External Workdir grant has invalid persisted permissions".to_string(), + )), + } +} + fn workdir_source_capabilities( + api: &WorkspaceApi, workdir: &WorkdirRegistryRecord, -) -> workdir::WorkdirSessionCapabilities { +) -> Result { match &workdir.source { - WorkdirRegistrySource::Repository { .. } => workdir::WorkdirSessionCapabilities::ALL, - WorkdirRegistrySource::ExternalGrant { .. } => { - workdir::WorkdirSessionCapabilities::READ_ONLY - } + WorkdirRegistrySource::Repository { .. } => Ok(workdir::WorkdirSessionCapabilities::ALL), + WorkdirRegistrySource::ExternalGrant { grant_id } => api + .store + .get_external_workdir_grant(&api.config.workspace_id, grant_id)? + .ok_or_else(|| Error::Store("External Workdir grant is unavailable".to_string())) + .and_then(|grant| external_workdir_capabilities(&grant.permissions)), } } @@ -3750,7 +3794,7 @@ fn ticket_target_capabilities_for_workdir( worker: &RuntimeWorkerRef, workdir: &WorkdirRegistryRecord, ) -> Result { - let source_capabilities = workdir_source_capabilities(workdir); + let source_capabilities = workdir_source_capabilities(api, workdir)?; let Some(assignment) = api .store .get_current_ticket_role_assignment_for_worker(&api.config.workspace_id, worker)? @@ -3887,7 +3931,7 @@ fn validate_manual_ticket_coder_workdir_binding( } let capabilities = link .capabilities - .intersection(workdir_source_capabilities(&workdir)) + .intersection(workdir_source_capabilities(api, &workdir)?) .intersection(target.capabilities); effective_links.push(WorkerWorkdirLinkRecord { capabilities, @@ -13815,7 +13859,7 @@ async fn open_current_worker_workdir_session_locked( })?; let effective_capabilities = link .capabilities - .intersection(workdir_source_capabilities(&workdir)) + .intersection(workdir_source_capabilities(api, &workdir)?) .intersection(ticket_target_capabilities_for_workdir( api, worker, &workdir, )?); @@ -13829,9 +13873,7 @@ async fn open_current_worker_workdir_session_locked( code: "external_workdir_grant_not_found".to_string(), message: "External Workdir grant is unavailable".to_string(), })?; - let expired = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|expires_at| expires_at.with_timezone(&Utc) <= Utc::now()) - .unwrap_or(true); + let expired = external_workdir_grant_is_expired(&grant)?; if expired { grant = fence_external_workdir_grant_expired(api, grant_id) .await @@ -14036,7 +14078,7 @@ fn sync_runtime_worker_workdir_attachments( })?; let capabilities = link .capabilities - .intersection(workdir_source_capabilities(&workdir)) + .intersection(workdir_source_capabilities(api, &workdir)?) .intersection(ticket_target_capabilities_for_workdir( api, worker, &workdir, )?); @@ -14130,9 +14172,7 @@ async fn scoped_attach_current_worker_workdir( code: "external_workdir_unavailable".to_string(), message: "External Workdir grant is unavailable".to_string(), })?; - let grant_expired = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|expires_at| expires_at.with_timezone(&Utc) <= Utc::now()) - .unwrap_or(true); + let grant_expired = external_workdir_grant_is_expired(&grant)?; let online = api .external_workdir_providers .lock() @@ -14142,7 +14182,9 @@ async fn scoped_attach_current_worker_workdir( connection.generation == grant.generation && connection.provider_instance_id == grant.provider_instance_id && connection.workdir_id == workdir.workdir_id - && connection.expires_at > Utc::now() + && connection + .expires_at + .is_none_or(|expires_at| expires_at > Utc::now()) }); if grant.status != "online" || grant_expired || !online { return Err(Error::RuntimeOperationFailed { @@ -16795,13 +16837,32 @@ async fn cleanup_runtime_working_directory( .map_err(ApiError::from) } +fn external_workdir_expires_at( + grant: &ExternalWorkdirGrantRecord, +) -> Result>> { + grant + .expires_at + .as_deref() + .map(|value| { + chrono::DateTime::parse_from_rfc3339(value) + .map(|value| value.with_timezone(&Utc)) + .map_err(|_| Error::Store("External Workdir grant has invalid expiry".to_string())) + }) + .transpose() +} + +fn external_workdir_grant_is_expired(grant: &ExternalWorkdirGrantRecord) -> Result { + Ok(external_workdir_expires_at(grant)?.is_some_and(|expires_at| expires_at <= Utc::now())) +} + fn schedule_external_workdir_expiry( api: &WorkspaceApi, grant: &ExternalWorkdirGrantRecord, ) -> Result<()> { - let expires_at = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map_err(|_| Error::Store("External Workdir grant has invalid expiry".to_string()))? - .with_timezone(&Utc); + let Some(expires_at) = external_workdir_expires_at(grant)? else { + cancel_external_workdir_expiry(api, &grant.grant_id); + return Ok(()); + }; let grant_id = grant.grant_id.clone(); let task_api = api.clone(); let task_grant_id = grant_id.clone(); @@ -16882,9 +16943,9 @@ async fn fence_external_workdir_grant_expired( if matches!(grant.status.as_str(), "revoked" | "expired") { return Ok(grant); } - let expires_at = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|value| value.with_timezone(&Utc)) - .map_err(|_| Error::Store("External Workdir grant has invalid expiry".to_string()))?; + let Some(expires_at) = external_workdir_expires_at(&grant)? else { + return Ok(grant); + }; if expires_at > Utc::now() { return Ok(grant); } @@ -16959,15 +17020,12 @@ async fn scoped_create_external_workdir_grant( "External Workdir display name must be between 1 and 100 bytes and contain no control characters".to_string(), ).into()); } - if !request.read_only { + if request + .ttl_seconds + .is_some_and(|ttl_seconds| !(60..=86_400).contains(&ttl_seconds)) + { return Err(Error::InvalidInput( - "External Workdir grants currently require read_only=true".to_string(), - ) - .into()); - } - if !(60..=86_400).contains(&request.ttl_seconds) { - return Err(Error::InvalidInput( - "External Workdir ttl_seconds must be between 60 and 86400".to_string(), + "External Workdir ttl_seconds must be omitted or between 60 and 86400".to_string(), ) .into()); } @@ -16975,8 +17033,14 @@ async fn scoped_create_external_workdir_grant( .map_err(Error::InvalidInput)?; let now = Utc::now(); let created_at = now.to_rfc3339_opts(SecondsFormat::Nanos, true); - let expires_at = (now + Duration::seconds(request.ttl_seconds as i64)) - .to_rfc3339_opts(SecondsFormat::Nanos, true); + let expires_at = request + .ttl_seconds + .map(|ttl_seconds| now + Duration::seconds(ttl_seconds as i64)) + .map(|expires_at| expires_at.to_rfc3339_opts(SecondsFormat::Nanos, true)); + let permissions = match request.permission { + server_api::ExternalWorkdirPermission::ReadOnly => "read_only", + server_api::ExternalWorkdirPermission::ReadWrite => "read_write", + }; let grant_id = format!("ewg-{}", Uuid::now_v7()); let workdir_id = format!("external-{}", Uuid::now_v7()); let grant = ExternalWorkdirGrantRecord { @@ -16985,7 +17049,7 @@ async fn scoped_create_external_workdir_grant( workdir_id: workdir_id.clone(), provider_instance_id: request.provider_instance_id, display_name: display_name.to_string(), - permissions: "read_only".to_string(), + permissions: permissions.to_string(), created_by: actor.account_id, created_at: created_at.clone(), expires_at: expires_at.clone(), @@ -17029,7 +17093,10 @@ fn external_workdir_grant_response( working_directory_id: grant.workdir_id.clone(), provider_instance_id: grant.provider_instance_id.clone(), display_name: grant.display_name.clone(), - permissions: grant.permissions.clone(), + permission: match grant.permissions.as_str() { + "read_write" => server_api::ExternalWorkdirPermission::ReadWrite, + _ => server_api::ExternalWorkdirPermission::ReadOnly, + }, expires_at: grant.expires_at.clone(), generation: grant.generation, status: grant.status.clone(), @@ -17052,10 +17119,7 @@ async fn scoped_external_workdir_provider_ws( "External Workdir provider connection must use the grant creator's authenticated client".to_string(), ).into()); } - let now = Utc::now(); - let expired = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|expires_at| expires_at.with_timezone(&Utc) <= now) - .unwrap_or(true); + let expired = external_workdir_grant_is_expired(&grant)?; if expired { grant = fence_external_workdir_grant_expired(&api, &grant_id).await?; } @@ -17109,10 +17173,17 @@ async fn serve_external_workdir_provider( let _ = socket.close().await; return; }; + let expected_capabilities = match external_workdir_capabilities(&initial_grant.permissions) { + Ok(capabilities) => capabilities, + Err(_) => { + let _ = socket.close().await; + return; + } + }; if registration.grant_id.as_str() != initial_grant.grant_id || registration.workdir_id.as_str() != initial_grant.workdir_id || registration.provider_instance_id.as_str() != initial_grant.provider_instance_id - || registration.capabilities != workdir::WorkdirSessionCapabilities::READ_ONLY + || registration.capabilities != expected_capabilities || registration.read_limits != workdir::BoundedReadLimits::EXTERNAL_DEFAULT || registration.generation == 0 { @@ -17165,8 +17236,8 @@ async fn serve_external_workdir_provider( let _ = socket.close().await; return; } - let expires_at = match chrono::DateTime::parse_from_rfc3339(&initial_grant.expires_at) { - Ok(value) => value.with_timezone(&Utc), + let expires_at = match external_workdir_expires_at(&initial_grant) { + Ok(value) => value, Err(_) => { let _ = socket.close().await; return; @@ -17419,11 +17490,8 @@ async fn scoped_get_external_workdir_grant( ) .into()); } - let now = Utc::now(); let expired = !matches!(grant.status.as_str(), "revoked" | "expired") - && chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map(|expires_at| expires_at.with_timezone(&Utc) <= now) - .unwrap_or(true); + && external_workdir_grant_is_expired(&grant)?; if expired { grant = fence_external_workdir_grant_expired(&api, &grant_id).await?; } @@ -21856,7 +21924,7 @@ fn finalize_spawned_worker_workdir_attachments( worker_record, attachment.alias.as_str(), &summary.working_directory_id, - workdir_source_capabilities(&workdir_record), + workdir_source_capabilities(api, &workdir_record)?, None, ), )?; @@ -26074,7 +26142,7 @@ mod tests { workdir_id: "external-a".to_string(), provider_instance_id: "provider-a".to_string(), generation: 4, - expires_at: Utc::now() + Duration::minutes(1), + expires_at: Some(Utc::now() + Duration::minutes(1)), capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY, admission: Arc::new(tokio::sync::Semaphore::new(16)), sender, @@ -26130,6 +26198,70 @@ mod tests { )); } + #[tokio::test] + async fn external_provider_read_write_session_routes_files_but_never_commands() { + let (sender, mut commands) = tokio::sync::mpsc::channel(1); + let connection = Arc::new(ExternalProviderConnection { + grant_id: "grant-rw".to_string(), + workdir_id: "external-rw".to_string(), + provider_instance_id: "provider-rw".to_string(), + generation: 2, + expires_at: None, + capabilities: workdir::WorkdirSessionCapabilities::READ_WRITE, + admission: Arc::new(tokio::sync::Semaphore::new(16)), + sender, + }); + let session = Arc::new(ExternalProviderWorkdirSession::new(connection, None)); + let writer = { + let session = session.clone(); + tokio::spawn(async move { + session + .write(workdir::WriteRequest { + path: workdir::WorkdirPath::new("created.txt").unwrap(), + content: b"created".to_vec(), + expected_hash: None, + }) + .await + }) + }; + let Some(ExternalProviderCommand::Operation { + operation, + response, + .. + }) = commands.recv().await + else { + panic!("expected provider write operation"); + }; + assert!(matches!(operation, WorkdirSessionOperation::Write(_))); + response + .send(Ok(WorkdirSessionOperationResult::Write( + workdir::WriteResult { + bytes_written: 7, + created: true, + }, + ))) + .unwrap(); + assert!(writer.await.unwrap().unwrap().created); + + let command = session + .start_command(workdir::CommandRequest { + command: "touch forbidden".to_string(), + timeout_secs: 1, + output_limit: 1024, + cwd: workdir::WorkdirPath::root(), + spill_dir: None, + tool_call_id: None, + }) + .await; + assert!(matches!( + command, + Err(workdir::WorkdirError::Unsupported( + workdir::WorkdirSessionCapability::Command + )) + )); + assert!(commands.try_recv().is_err()); + } + #[tokio::test] async fn backend_expiry_fences_offline_grant_and_persists_cleanup_completion() { let dir = tempfile::tempdir().unwrap(); @@ -26146,8 +26278,10 @@ mod tests { permissions: "read_only".to_string(), created_by: test_browser_request_actor().account_id, created_at: created_at.clone(), - expires_at: (now + Duration::milliseconds(50)) - .to_rfc3339_opts(SecondsFormat::Nanos, true), + expires_at: Some( + (now + Duration::milliseconds(50)) + .to_rfc3339_opts(SecondsFormat::Nanos, true), + ), generation: 1, status: "offline".to_string(), updated_at: created_at.clone(), @@ -26246,7 +26380,9 @@ mod tests { permissions: "read_only".to_string(), created_by: test_browser_request_actor().account_id, created_at: created_at.clone(), - expires_at: (now + Duration::minutes(5)).to_rfc3339_opts(SecondsFormat::Nanos, true), + expires_at: Some( + (now + Duration::minutes(5)).to_rfc3339_opts(SecondsFormat::Nanos, true), + ), generation: 1, status: "online".to_string(), updated_at: created_at.clone(), @@ -26451,7 +26587,7 @@ mod tests { permissions: "read_only".to_string(), created_by: test_browser_request_actor().account_id, created_at: now.clone(), - expires_at: expires_at.to_rfc3339_opts(SecondsFormat::Nanos, true), + expires_at: Some(expires_at.to_rfc3339_opts(SecondsFormat::Nanos, true)), generation: 1, status: "pending".to_string(), updated_at: now.clone(), @@ -26498,7 +26634,7 @@ mod tests { workdir_id: grant.workdir_id.clone(), provider_instance_id: grant.provider_instance_id.clone(), generation: 1, - expires_at, + expires_at: Some(expires_at), capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY, admission: Arc::new(tokio::sync::Semaphore::new(16)), sender, @@ -26590,8 +26726,8 @@ mod tests { Json(ExternalWorkdirGrantCreateRequest { provider_instance_id: "provider-a".to_string(), display_name: "Session logs".to_string(), - ttl_seconds: 600, - read_only: true, + ttl_seconds: Some(600), + permission: server_api::ExternalWorkdirPermission::ReadWrite, }), ) .await @@ -26620,7 +26756,7 @@ mod tests { .id() .clone(), generation, - capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY, + capabilities: workdir::WorkdirSessionCapabilities::READ_WRITE, read_limits: workdir::BoundedReadLimits::EXTERNAL_DEFAULT, }, }) @@ -26716,14 +26852,27 @@ mod tests { .list_worker_workdir_links(TEST_WORKSPACE_ID, &worker) .unwrap(); assert_eq!(links.len(), 2); - assert!( - links.iter().all(|link| { - link.capabilities == workdir::WorkdirSessionCapabilities::READ_ONLY - }) + let repository_capabilities = links + .iter() + .find(|link| link.alias == "repository") + .unwrap() + .capabilities; + let external_capabilities = links + .iter() + .find(|link| link.alias == "session-logs") + .unwrap() + .capabilities; + assert_eq!( + repository_capabilities, + workdir::WorkdirSessionCapabilities::READ_ONLY + ); + assert_eq!( + external_capabilities, + workdir::WorkdirSessionCapabilities::READ_WRITE ); let provider_root = tempfile::tempdir().unwrap(); fs::write(provider_root.path().join("events.jsonl"), b"one\ntwo\n").unwrap(); - let local_session = workdir::LocalWorkdirSession::external_read_only( + let local_session = workdir::LocalWorkdirSession::external_read_write( workdir::Workdir::new(&grant.working_directory_id), provider_root.path(), workdir::BoundedReadLimits::EXTERNAL_DEFAULT, @@ -26737,7 +26886,7 @@ mod tests { .unwrap(); assert_eq!( broker_session.capabilities(), - workdir::WorkdirSessionCapabilities::READ_ONLY + workdir::WorkdirSessionCapabilities::READ_WRITE ); drop(session_guard); let read = { @@ -26787,7 +26936,137 @@ mod tests { )) .await .unwrap(); - assert_eq!(read.await.unwrap().unwrap().bytes, b"one\ntwo\n"); + let read_result = read.await.unwrap().unwrap(); + assert_eq!(read_result.bytes, b"one\ntwo\n"); + + let edit = { + let broker_session = broker_session.clone(); + let expected_hash = read_result.content_hash; + tokio::spawn(async move { + broker_session + .edit(workdir::EditRequest { + path: workdir::WorkdirPath::new("events.jsonl").unwrap(), + old_string: "two".to_string(), + new_string: "changed".to_string(), + replace_all: false, + expected_hash, + }) + .await + }) + }; + let (operation_id, operation) = loop { + let text = second.next().await.unwrap().unwrap().into_text().unwrap(); + let frame = serde_json::from_str::(&text).unwrap(); + if let ExternalWorkdirServerMessage::Operation { + generation: 2, + operation_id, + operation, + } = frame.message + { + break (operation_id, operation); + } + }; + assert!(matches!(operation.as_inner(), WorkdirSessionOperation::Edit(_))); + let result = workdir::http::dispatch_workdir_session_operation( + &local_session, + operation.into_inner(), + ) + .await + .unwrap() + .try_into() + .unwrap(); + second + .send(Message::Text( + serde_json::to_string(&ExternalWorkdirProviderFrame::current( + ExternalWorkdirProviderMessage::OperationResult { + generation: 2, + operation_id, + outcome: ExternalWorkdirOperationOutcome::Completed { result }, + }, + )) + .unwrap() + .into(), + )) + .await + .unwrap(); + assert_eq!(edit.await.unwrap().unwrap().replacements, 1); + + let write = { + let broker_session = broker_session.clone(); + tokio::spawn(async move { + broker_session + .write(workdir::WriteRequest { + path: workdir::WorkdirPath::new("created.txt").unwrap(), + content: b"created".to_vec(), + expected_hash: None, + }) + .await + }) + }; + let (operation_id, operation) = loop { + let text = second.next().await.unwrap().unwrap().into_text().unwrap(); + let frame = serde_json::from_str::(&text).unwrap(); + if let ExternalWorkdirServerMessage::Operation { + generation: 2, + operation_id, + operation, + } = frame.message + { + break (operation_id, operation); + } + }; + assert!(matches!(operation.as_inner(), WorkdirSessionOperation::Write(_))); + let result = workdir::http::dispatch_workdir_session_operation( + &local_session, + operation.into_inner(), + ) + .await + .unwrap() + .try_into() + .unwrap(); + second + .send(Message::Text( + serde_json::to_string(&ExternalWorkdirProviderFrame::current( + ExternalWorkdirProviderMessage::OperationResult { + generation: 2, + operation_id, + outcome: ExternalWorkdirOperationOutcome::Completed { result }, + }, + )) + .unwrap() + .into(), + )) + .await + .unwrap(); + assert!(write.await.unwrap().unwrap().created); + assert_eq!( + fs::read_to_string(provider_root.path().join("events.jsonl")).unwrap(), + "one\nchanged\n" + ); + assert_eq!( + fs::read_to_string(provider_root.path().join("created.txt")).unwrap(), + "created" + ); + let command_error = broker_session + .start_command(workdir::CommandRequest { + command: "touch forbidden".to_string(), + timeout_secs: 1, + output_limit: 1024, + cwd: workdir::WorkdirPath::root(), + spill_dir: None, + tool_call_id: None, + }) + .await + .unwrap_err(); + assert!( + matches!( + command_error, + workdir::WorkdirError::Unsupported(workdir::WorkdirSessionCapability::Command) + | workdir::WorkdirError::UnsupportedOperation(_) + | workdir::WorkdirError::Denied(_) + ), + "unexpected command error: {command_error:?}" + ); let Json(revoked) = scoped_revoke_external_workdir_grant( State(api.clone()), @@ -26829,7 +27108,7 @@ mod tests { workdir_id: "external-a".to_string(), provider_instance_id: "provider-a".to_string(), generation: 2, - expires_at: Utc::now() + Duration::minutes(1), + expires_at: Some(Utc::now() + Duration::minutes(1)), capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY, admission: Arc::new(tokio::sync::Semaphore::new(16)), sender, diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index bf65ff4e2..d729a59d9 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -21,7 +21,7 @@ use crate::workspace_deletion::WorkspaceDeletionStore; use crate::{Error, Result}; const OLDEST_SCHEMA_VERSION: i64 = 50; -const LATEST_SCHEMA_VERSION: i64 = 66; +const LATEST_SCHEMA_VERSION: i64 = 67; const SCHEMA_BASELINE_NAME: &str = "workspace schema baseline"; const WORKSPACE_RUNTIME_BINDINGS_MIGRATION_NAME: &str = "workspace runtime bindings"; const RUNTIME_BINDING_AUDIT_MIGRATION_NAME: &str = "workspace Runtime binding revision and audit"; @@ -48,6 +48,8 @@ const EXTERNAL_WORKDIR_CLEANUP_MIGRATION_NAME: &str = "durable External Workdir expiry cleanup state"; const TICKET_TARGETS_AND_WORKDIR_CAPABILITIES_MIGRATION_NAME: &str = "Ticket target collection and Workdir attachment capabilities"; +const EXTERNAL_WORKDIR_ACCESS_AND_OPTIONAL_EXPIRY_MIGRATION_NAME: &str = + "External Workdir read-write access and optional expiry"; const TICKET_SCHEMA_VERSION_WITH_TARGETS: i64 = 7; const TICKET_SCHEMA_BASELINE_NAME: &str = "ticket schema baseline"; const WORKER_REGISTRY_PROJECTION_SCHEMA: &str = r#" @@ -175,6 +177,11 @@ const MIGRATIONS: &[Migration] = &[ name: TICKET_TARGETS_AND_WORKDIR_CAPABILITIES_MIGRATION_NAME, apply: migrate_ticket_targets_and_workdir_capabilities_v65_to_v66, }, + Migration { + version: 67, + name: EXTERNAL_WORKDIR_ACCESS_AND_OPTIONAL_EXPIRY_MIGRATION_NAME, + apply: migrate_external_workdir_access_and_optional_expiry_v66_to_v67, + }, ]; #[derive(Clone, Copy)] @@ -870,7 +877,7 @@ pub struct ExternalWorkdirGrantRecord { pub permissions: String, pub created_by: String, pub created_at: String, - pub expires_at: String, + pub expires_at: Option, pub generation: u64, pub status: String, pub updated_at: String, @@ -7754,7 +7761,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore { WHERE workspace_id = ?1 AND grant_id = ?2 AND provider_instance_id = ?3 AND ((status = 'pending' AND generation = ?4) OR (status = 'offline' AND generation + 1 = ?4)) - AND expires_at > ?5"#, + AND (expires_at IS NULL OR expires_at > ?5)"#, params![ workspace_id, grant_id, @@ -7827,12 +7834,13 @@ impl ControlPlaneStore for SqliteWorkspaceStore { SET status = 'expired', updated_at = ?2, cleanup_state = 'pending', cleanup_error = NULL, cleanup_updated_at = ?2 WHERE workspace_id = ?1 AND status IN ('pending', 'online', 'offline') - AND expires_at <= ?2", + AND expires_at IS NOT NULL AND expires_at <= ?2", params![workspace_id, now], )?; tx.execute( "UPDATE external_workdir_grants SET status = 'offline', updated_at = ?2 - WHERE workspace_id = ?1 AND status = 'online' AND expires_at > ?2", + WHERE workspace_id = ?1 AND status = 'online' + AND (expires_at IS NULL OR expires_at > ?2)", params![workspace_id, now], )?; tx.execute( @@ -9130,9 +9138,12 @@ fn encode_workdir_link_capabilities( Ok("all") } else if capabilities == workdir::WorkdirSessionCapabilities::READ_ONLY { Ok("read_only") + } else if capabilities == workdir::WorkdirSessionCapabilities::READ_WRITE { + Ok("read_write") } else { Err(Error::InvalidInput( - "Workdir attachment capabilities must be exactly all or read_only".to_string(), + "Workdir attachment capabilities must be exactly all, read_only, or read_write" + .to_string(), )) } } @@ -9144,6 +9155,7 @@ fn decode_workdir_link_capabilities( match value { "all" => Ok(workdir::WorkdirSessionCapabilities::ALL), "read_only" => Ok(workdir::WorkdirSessionCapabilities::READ_ONLY), + "read_write" => Ok(workdir::WorkdirSessionCapabilities::READ_WRITE), _ => Err(rusqlite::Error::FromSqlConversionFailure( column, rusqlite::types::Type::Text, @@ -11957,6 +11969,122 @@ fn apply_ticket_targets_and_workdir_capabilities_schema(conn: &Connection) -> Re } } +fn migrate_external_workdir_access_and_optional_expiry_v66_to_v67( + conn: &Connection, +) -> Result<()> { + let current = current_schema_version(conn)?; + if current != 66 { + return Err(Error::Store(format!( + "expected schema version 66 before {EXTERNAL_WORKDIR_ACCESS_AND_OPTIONAL_EXPIRY_MIGRATION_NAME} migration, found {current}" + ))); + } + let foreign_keys_enabled = + conn.query_row("PRAGMA foreign_keys", [], |row| row.get::<_, i64>(0))? != 0; + if foreign_keys_enabled { + conn.execute_batch("PRAGMA foreign_keys = OFF;")?; + } + let result = (|| { + let tx = rusqlite::Transaction::new_unchecked(conn, TransactionBehavior::Exclusive)?; + tx.execute_batch( + r#" + CREATE TABLE external_workdir_grants_v67 ( + grant_id TEXT NOT NULL, + workspace_id TEXT NOT NULL, + workdir_id TEXT NOT NULL, + provider_instance_id TEXT NOT NULL, + display_name TEXT NOT NULL, + permissions TEXT NOT NULL CHECK (permissions IN ('read_only', 'read_write')), + created_by TEXT NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT, + generation INTEGER NOT NULL CHECK (generation > 0), + status TEXT NOT NULL CHECK (status IN ('pending', 'online', 'offline', 'revoked', 'expired')), + updated_at TEXT NOT NULL, + cleanup_state TEXT NOT NULL DEFAULT 'not_required' CHECK (cleanup_state IN ('not_required', 'pending', 'retry_required', 'completed')), + cleanup_error TEXT, + cleanup_updated_at TEXT, + PRIMARY KEY (workspace_id, grant_id), + UNIQUE (workspace_id, workdir_id), + FOREIGN KEY (workspace_id) REFERENCES workspaces(workspace_id) ON DELETE CASCADE, + FOREIGN KEY (workspace_id, workdir_id) + REFERENCES workdir_registry(workspace_id, workdir_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED + ); + INSERT INTO external_workdir_grants_v67 ( + grant_id, workspace_id, workdir_id, provider_instance_id, display_name, + permissions, created_by, created_at, expires_at, generation, status, updated_at, + cleanup_state, cleanup_error, cleanup_updated_at + ) SELECT grant_id, workspace_id, workdir_id, provider_instance_id, display_name, + permissions, created_by, created_at, expires_at, generation, status, updated_at, + cleanup_state, cleanup_error, cleanup_updated_at + FROM external_workdir_grants; + DROP TABLE external_workdir_grants; + ALTER TABLE external_workdir_grants_v67 RENAME TO external_workdir_grants; + CREATE INDEX idx_external_workdir_grants_status_expiry + ON external_workdir_grants(workspace_id, status, expires_at); + CREATE TABLE worker_workdir_links_v67 ( + workspace_id TEXT NOT NULL, + runtime_id TEXT NOT NULL, + worker_id TEXT NOT NULL, + workdir_id TEXT NOT NULL, + alias TEXT NOT NULL, + capabilities TEXT NOT NULL CHECK (capabilities IN ('all', 'read_only', 'read_write')), + linked_at TEXT NOT NULL, + unlinked_at TEXT, + PRIMARY KEY (workspace_id, worker_id, workdir_id, alias), + FOREIGN KEY (workspace_id, worker_id) + REFERENCES worker_registry(workspace_id, worker_id) ON DELETE CASCADE, + FOREIGN KEY (workspace_id, workdir_id) + REFERENCES workdir_registry(workspace_id, workdir_id) ON DELETE CASCADE + ); + INSERT INTO worker_workdir_links_v67 ( + workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, + linked_at, unlinked_at + ) SELECT workspace_id, runtime_id, worker_id, workdir_id, alias, capabilities, + linked_at, unlinked_at + FROM worker_workdir_links; + DROP TABLE worker_workdir_links; + ALTER TABLE worker_workdir_links_v67 RENAME TO worker_workdir_links; + CREATE UNIQUE INDEX worker_workdir_links_active_workdir_unique + ON worker_workdir_links(workspace_id, workdir_id) WHERE unlinked_at IS NULL; + CREATE UNIQUE INDEX worker_workdir_links_active_alias_unique + ON worker_workdir_links(workspace_id, worker_id, alias) WHERE unlinked_at IS NULL; + CREATE INDEX worker_workdir_links_workdir + ON worker_workdir_links(workspace_id, workdir_id); + "#, + )?; + let violations = tx.query_row( + "SELECT COUNT(*) FROM pragma_foreign_key_check", + [], + |row| row.get::<_, i64>(0), + )?; + if violations != 0 { + return Err(Error::Store(format!( + "schema-67 migration left {violations} foreign-key violation(s)" + ))); + } + tx.execute( + "INSERT INTO __yoi_schema_migrations (version, name) VALUES (?1, ?2)", + params![ + 67_i64, + EXTERNAL_WORKDIR_ACCESS_AND_OPTIONAL_EXPIRY_MIGRATION_NAME + ], + )?; + tx.commit()?; + Ok(()) + })(); + let restore = if foreign_keys_enabled { + conn.execute_batch("PRAGMA foreign_keys = ON;") + .map_err(Error::from) + } else { + Ok(()) + }; + match (result, restore) { + (Err(error), _) => Err(error), + (Ok(()), Err(error)) => Err(error), + (Ok(()), Ok(())) => Ok(()), + } +} + fn create_latest_workspace_schema(conn: &Connection) -> Result<()> { conn.execute_batch("DROP TABLE IF EXISTS typed_ticket_targets;")?; conn.execute_batch(include_str!("latest_schema.sql"))?; @@ -12019,7 +12147,7 @@ fn create_latest_workspace_schema(conn: &Connection) -> Result<()> { ON typed_ticket_targets(workspace_id, repository_key, ticket_id); ALTER TABLE worker_workdir_links ADD COLUMN capabilities TEXT NOT NULL - CHECK (capabilities IN ('all', 'read_only')); + CHECK (capabilities IN ('all', 'read_only', 'read_write')); "#, )?; ticket::verify_sqlite_ticket_schema(conn).map_err(|error| { @@ -12745,10 +12873,10 @@ mod tests { workdir_id: "external-a".to_string(), provider_instance_id: "provider-a".to_string(), display_name: "Session logs".to_string(), - permissions: "read_only".to_string(), + permissions: "read_write".to_string(), created_by: "owner".to_string(), created_at: "2026-01-01T00:00:00Z".to_string(), - expires_at: "2099-01-01T00:00:00Z".to_string(), + expires_at: None, generation: 1, status: "pending".to_string(), updated_at: "2026-01-01T00:00:00Z".to_string(), @@ -12775,6 +12903,12 @@ mod tests { store .create_external_workdir_grant(&grant, &workdir) .unwrap(); + let persisted_grant = store + .get_external_workdir_grant("workspace-a", "grant-a") + .unwrap() + .unwrap(); + assert_eq!(persisted_grant.permissions, "read_write"); + assert_eq!(persisted_grant.expires_at, None); store .record_external_workdir_audit( "workspace-a", @@ -12858,6 +12992,14 @@ mod tests { .unwrap() .is_empty() ); + assert_eq!( + store + .get_external_workdir_grant("workspace-a", "grant-a") + .unwrap() + .unwrap() + .status, + "offline" + ); assert!( !store .activate_external_workdir_grant( diff --git a/crates/yoi/src/main.rs b/crates/yoi/src/main.rs index b46b54a6d..e92b5cf1f 100644 --- a/crates/yoi/src/main.rs +++ b/crates/yoi/src/main.rs @@ -622,6 +622,8 @@ fn parse_workdir_args( let mut display_name = None; let mut ttl = None; let mut read_only = false; + let mut read_write = false; + let mut non_interactive = false; let mut index = 0; while index < args.len() { match args[index].as_str() { @@ -658,9 +660,10 @@ fn parse_workdir_args( if ttl.is_some() { return Err(ParseError("--ttl may only be provided once".to_string())); } - ttl = Some(parse_share_ttl(required_option_value( - args, index, "--ttl", - )?)?); + ttl = Some( + workdir_share::parse_ttl(required_option_value(args, index, "--ttl")?) + .map_err(|error| ParseError(format!("--ttl {error}")))?, + ); index += 2; } "--read-only" => { @@ -672,6 +675,24 @@ fn parse_workdir_args( read_only = true; index += 1; } + "--read-write" => { + if read_write { + return Err(ParseError( + "--read-write may only be provided once".to_string(), + )); + } + read_write = true; + index += 1; + } + "--non-interactive" => { + if non_interactive { + return Err(ParseError( + "--non-interactive may only be provided once".to_string(), + )); + } + non_interactive = true; + index += 1; + } "--help" | "-h" => return Ok(Mode::WorkdirHelp), option if option.starts_with('-') => { return Err(ParseError(format!( @@ -689,49 +710,33 @@ fn parse_workdir_args( } } } - if !read_only { + if read_only && read_write { return Err(ParseError( - "yoi workdir share requires --read-only".to_string(), + "--read-only conflicts with --read-write".to_string(), )); } - let workspace_id = workspace_id - .ok_or_else(|| ParseError("yoi workdir share requires --workspace-id".to_string()))?; - let backend_url = resolve_backend_url(backend_url, Some(&workspace_id))?; + if non_interactive && workspace_id.is_none() { + return Err(ParseError( + "--non-interactive requires --workspace-id".to_string(), + )); + } + let backend_url = resolve_backend_url(backend_url, workspace_id.as_deref())?; Ok(Mode::WorkdirShare(workdir_share::WorkdirShareOptions { path: path .ok_or_else(|| ParseError("yoi workdir share requires a directory path".to_string()))?, workspace_id, backend_url, - display_name: display_name - .ok_or_else(|| ParseError("yoi workdir share requires --display-name".to_string()))?, - ttl: ttl.ok_or_else(|| ParseError("yoi workdir share requires --ttl".to_string()))?, + display_name: display_name.unwrap_or_else(|| "External Workdir".to_string()), + ttl, + permission: if read_write { + server_api::ExternalWorkdirPermission::ReadWrite + } else { + server_api::ExternalWorkdirPermission::ReadOnly + }, + non_interactive, })) } -fn parse_share_ttl(value: &str) -> Result { - let (number, multiplier) = if let Some(value) = value.strip_suffix('h') { - (value, 60 * 60) - } else if let Some(value) = value.strip_suffix('m') { - (value, 60) - } else if let Some(value) = value.strip_suffix('s') { - (value, 1) - } else { - return Err(ParseError( - "--ttl must use an h, m, or s suffix (for example 1h)".to_string(), - )); - }; - let amount = number.parse::().map_err(|_| { - ParseError("--ttl must contain a positive integer and h, m, or s suffix".to_string()) - })?; - let seconds = amount - .checked_mul(multiplier) - .ok_or_else(|| ParseError("--ttl is too large".to_string()))?; - if !(60..=86_400).contains(&seconds) { - return Err(ParseError("--ttl must be between 60s and 24h".to_string())); - } - Ok(Duration::from_secs(seconds)) -} - fn parse_init_args( args: &[String], explicit_backend_url: Option, @@ -1857,7 +1862,7 @@ Usage: yoi --backend [--workspace-id ] panel yoi [--backend ] init --display-name --repository-key [--repository ] [--default-ref ] yoi [--backend ] login [--no-wait] - yoi [--backend ] workdir share --workspace-id --read-only --ttl --display-name + yoi [--backend ] workdir share [--workspace-id ] [--read-only|--read-write] [--ttl ] [--display-name ] [--non-interactive] yoi [OPTIONS] Target selection: @@ -1883,7 +1888,7 @@ Console options: Host commands: yoi init Register the current Git repository as a new Backend Workspace. - yoi workdir share Share a temporary read-only client-hosted External Workdir. + yoi workdir share Interactively share a client-hosted External Workdir. keys Manage local model/API keys setup-model Configure a local model provider worker [WORKER_OPTIONS] Run the direct Worker process entrypoint @@ -1903,7 +1908,7 @@ Options: fn print_workdir_help() { println!( - "yoi workdir share\n\nUsage:\n yoi [--backend ] workdir share --workspace-id --read-only --ttl --display-name \n\nThe foreground CLI provides one local directory over an authenticated outbound Backend connection.\nNo host path is sent to the Backend. TTL accepts s, m, or h suffixes from 60s through 24h.\nCtrl-C explicitly revokes the grant before exit.\n" + "yoi workdir share\n\nUsage:\n yoi [--backend ] workdir share [OPTIONS]\n\nThe foreground CLI provides one local directory over an authenticated outbound Backend connection.\nInteractive mode is the default: choose an accessible Workspace, permission, and TTL, then explicitly confirm before any grant is created.\nRead-only is the default permission. --read-write enables existing-file Write/Edit only; Bash, command, and exec remain disabled.\nOmitting --ttl means unlimited automatic lifetime, but stop, disconnect, Ctrl-C, and revoke still fail closed. TTL accepts s, m, or h suffixes from 60s through 24h.\nNo host path is sent to the Backend. Ctrl-C explicitly revokes the grant before exit.\n\nOptions:\n --workspace-id Preselect a Workspace; required with --non-interactive\n --read-only Explicitly select the default read-only permission\n --read-write Allow existing-file Write/Edit (never commands)\n --ttl Automatic expiry; omission is unlimited\n --display-name Backend-visible label (default: External Workdir)\n --non-interactive Never prompt; requires --workspace-id\n -h, --help Print help\n" ); } @@ -1963,11 +1968,12 @@ mod tests { let args = vec![ "share".to_string(), "./session-log".to_string(), - "--read-only".to_string(), + "--read-write".to_string(), "--ttl".to_string(), "90m".to_string(), "--display-name".to_string(), "session-analysis".to_string(), + "--non-interactive".to_string(), ]; let target = TargetSelection { backend_url: Some("https://backend.example".to_string()), @@ -1978,56 +1984,82 @@ mod tests { panic!("expected Workdir share mode"); }; assert_eq!(options.path, PathBuf::from("./session-log")); - assert_eq!(options.workspace_id, "workspace-a"); + assert_eq!(options.workspace_id.as_deref(), Some("workspace-a")); assert_eq!(options.backend_url, "https://backend.example"); assert_eq!(options.display_name, "session-analysis"); - assert_eq!(options.ttl, Duration::from_secs(5_400)); + assert_eq!(options.ttl, Some(Duration::from_secs(5_400))); + assert_eq!( + options.permission, + server_api::ExternalWorkdirPermission::ReadWrite + ); + assert!(options.non_interactive); } #[test] - fn workdir_share_fails_closed_without_explicit_read_only_workspace_or_valid_ttl() { - let base = vec![ - "share".to_string(), - ".".to_string(), - "--ttl".to_string(), - "1h".to_string(), - "--display-name".to_string(), - "logs".to_string(), - ]; + fn workdir_share_defaults_to_interactive_read_only_and_unlimited() { + let args = vec!["share".to_string(), ".".to_string()]; + let target = TargetSelection { + backend_url: Some("https://backend.example".to_string()), + ..TargetSelection::default() + }; + let Mode::WorkdirShare(options) = parse_workdir_args(&args, &target).unwrap() else { + panic!("expected Workdir share mode"); + }; + assert_eq!(options.workspace_id, None); + assert_eq!(options.display_name, "External Workdir"); + assert_eq!(options.ttl, None); + assert_eq!( + options.permission, + server_api::ExternalWorkdirPermission::ReadOnly + ); + assert!(!options.non_interactive); + } + + #[test] + fn workdir_share_rejects_invalid_or_conflicting_automation_options() { let target = TargetSelection { backend_url: Some("https://backend.example".to_string()), workspace_id: Some("workspace-a".to_string()), ..TargetSelection::default() }; - assert!( - parse_workdir_args(&base, &target) - .err() - .unwrap() - .0 - .contains("--read-only") - ); - - let mut invalid_ttl = base.clone(); - invalid_ttl.push("--read-only".to_string()); - invalid_ttl[3] = "25h".to_string(); + let invalid_ttl = vec![ + "share".to_string(), + ".".to_string(), + "--ttl".to_string(), + "25h".to_string(), + ]; assert!( parse_workdir_args(&invalid_ttl, &target) - .err() - .unwrap() + .unwrap_err() .0 .contains("24h") ); + let conflict = vec![ + "share".to_string(), + ".".to_string(), + "--read-only".to_string(), + "--read-write".to_string(), + ]; + assert!( + parse_workdir_args(&conflict, &target) + .unwrap_err() + .0 + .contains("conflicts") + ); + let missing_workspace = TargetSelection { backend_url: Some("https://backend.example".to_string()), ..TargetSelection::default() }; - let mut valid = base; - valid.push("--read-only".to_string()); + let automation = vec![ + "share".to_string(), + ".".to_string(), + "--non-interactive".to_string(), + ]; assert!( - parse_workdir_args(&valid, &missing_workspace) - .err() - .unwrap() + parse_workdir_args(&automation, &missing_workspace) + .unwrap_err() .0 .contains("--workspace-id") ); diff --git a/crates/yoi/src/workdir_share.rs b/crates/yoi/src/workdir_share.rs index 1786e7524..885e5525d 100644 --- a/crates/yoi/src/workdir_share.rs +++ b/crates/yoi/src/workdir_share.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io::{BufRead, IsTerminal, Write}; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; @@ -20,16 +21,17 @@ use workdir::external::{ use workdir::http::{WorkdirTransportError, dispatch_workdir_session_operation}; use workdir::{ BoundedReadLimits, ExternalWorkdirRoot, LocalWorkdirSession, Workdir, WorkdirSession, - WorkdirSessionCapabilities, }; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct WorkdirShareOptions { pub path: PathBuf, - pub workspace_id: String, + pub workspace_id: Option, pub backend_url: String, pub display_name: String, - pub ttl: Duration, + pub ttl: Option, + pub permission: server_api::ExternalWorkdirPermission, + pub non_interactive: bool, } const PROVIDER_OPERATION_TIMEOUT: Duration = Duration::from_secs(30); @@ -126,7 +128,249 @@ fn authenticated_server_api_client( Ok(ServerApiClient::with_client(base_url, http_client)) } +fn permission_label(permission: server_api::ExternalWorkdirPermission) -> &'static str { + match permission { + server_api::ExternalWorkdirPermission::ReadOnly => "read-only", + server_api::ExternalWorkdirPermission::ReadWrite => "read-write (files only; commands disabled)", + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ResolvedWorkdirShareOptions { + path: PathBuf, + workspace_id: String, + backend_url: String, + display_name: String, + ttl: Option, + permission: server_api::ExternalWorkdirPermission, +} + +pub(crate) fn parse_ttl(value: &str) -> Result { + let (number, multiplier) = if let Some(value) = value.strip_suffix('h') { + (value, 60 * 60) + } else if let Some(value) = value.strip_suffix('m') { + (value, 60) + } else if let Some(value) = value.strip_suffix('s') { + (value, 1) + } else { + return Err("TTL must use an h, m, or s suffix (for example 1h)".to_string()); + }; + let amount = number + .parse::() + .map_err(|_| "TTL must contain a positive integer and h, m, or s suffix".to_string())?; + let seconds = amount + .checked_mul(multiplier) + .ok_or_else(|| "TTL is too large".to_string())?; + if !(60..=86_400).contains(&seconds) { + return Err("TTL must be between 60s and 24h".to_string()); + } + Ok(Duration::from_secs(seconds)) +} + +fn ttl_label(ttl: Option) -> String { + match ttl { + None => "unlimited (ends on stop, disconnect, or revoke)".to_string(), + Some(ttl) => format!("{}s", ttl.as_secs()), + } +} + +fn read_prompt_line( + input: &mut impl BufRead, + output: &mut impl Write, + prompt: &str, +) -> Result, String> { + write!(output, "{prompt}").map_err(|error| format!("failed to write prompt: {error}"))?; + output + .flush() + .map_err(|error| format!("failed to flush prompt: {error}"))?; + let mut line = String::new(); + let read = input + .read_line(&mut line) + .map_err(|error| format!("failed to read operator input: {error}"))?; + if read == 0 { + return Ok(None); + } + Ok(Some(line.trim().to_string())) +} + +fn configure_interactively( + mut options: WorkdirShareOptions, + workspaces: &[server_api::WorkspaceSummary], + input: &mut impl BufRead, + output: &mut impl Write, +) -> Result, String> { + if workspaces.is_empty() { + return Err("Backend returned no accessible Workspaces".to_string()); + } + writeln!(output, "Select a Workspace:") + .map_err(|error| format!("failed to write prompt: {error}"))?; + for (index, workspace) in workspaces.iter().enumerate() { + writeln!( + output, + " {}) {} ({})", + index + 1, + workspace.display_name, + workspace.workspace_id + ) + .map_err(|error| format!("failed to write prompt: {error}"))?; + } + let initial_index = options + .workspace_id + .as_deref() + .map(|workspace_id| { + workspaces + .iter() + .position(|workspace| workspace.workspace_id == workspace_id) + .ok_or_else(|| { + format!( + "Workspace `{workspace_id}` is not present in the accessible Backend catalog" + ) + }) + }) + .transpose()? + .unwrap_or(0); + let Some(selection) = read_prompt_line( + input, + output, + &format!("Workspace [{}; q to cancel]: ", initial_index + 1), + )? else { + return Ok(None); + }; + if matches!(selection.as_str(), "q" | "quit" | "cancel") { + return Ok(None); + } + let selected_index = if selection.is_empty() { + initial_index + } else { + selection + .parse::() + .ok() + .and_then(|index| index.checked_sub(1)) + .filter(|index| *index < workspaces.len()) + .ok_or_else(|| "Workspace selection is invalid".to_string())? + }; + options.workspace_id = Some(workspaces[selected_index].workspace_id.clone()); + + let current_permission = match options.permission { + server_api::ExternalWorkdirPermission::ReadOnly => "r", + server_api::ExternalWorkdirPermission::ReadWrite => "w", + }; + writeln!( + output, + "Permission: read-only or read-write files. Read-write never enables Bash/command/exec." + ) + .map_err(|error| format!("failed to write prompt: {error}"))?; + let Some(permission) = read_prompt_line( + input, + output, + &format!("Permission [r/w; current {current_permission}; q to cancel]: "), + )? else { + return Ok(None); + }; + options.permission = match permission.as_str() { + "" => options.permission, + "r" | "read-only" | "read_only" => server_api::ExternalWorkdirPermission::ReadOnly, + "w" | "read-write" | "read_write" => server_api::ExternalWorkdirPermission::ReadWrite, + "q" | "quit" | "cancel" => return Ok(None), + _ => return Err("Permission must be read-only (`r`) or read-write (`w`)".to_string()), + }; + + writeln!(output, "TTL: {}", ttl_label(options.ttl)) + .map_err(|error| format!("failed to write prompt: {error}"))?; + let Some(ttl) = read_prompt_line( + input, + output, + "TTL [empty keeps current; `none` is unlimited; q to cancel]: ", + )? else { + return Ok(None); + }; + options.ttl = match ttl.as_str() { + "" => options.ttl, + "none" | "unlimited" => None, + "q" | "quit" | "cancel" => return Ok(None), + value => Some(parse_ttl(value)?), + }; + + let workspace_id = options + .workspace_id + .clone() + .expect("interactive selection always assigns a Workspace"); + writeln!(output, "Share configuration:") + .and_then(|()| writeln!(output, " Workspace: {workspace_id}")) + .and_then(|()| { + writeln!( + output, + " Permission: {}", + permission_label(options.permission) + ) + }) + .and_then(|()| writeln!(output, " TTL: {}", ttl_label(options.ttl))) + .map_err(|error| format!("failed to write confirmation: {error}"))?; + let Some(confirm) = read_prompt_line(input, output, "Connect and create grant? [y/N]: ")? + else { + return Ok(None); + }; + if !matches!(confirm.as_str(), "y" | "yes") { + return Ok(None); + } + Ok(Some(ResolvedWorkdirShareOptions { + path: options.path, + workspace_id, + backend_url: options.backend_url, + display_name: options.display_name, + ttl: options.ttl, + permission: options.permission, + })) +} + +fn ensure_interactive_tty(stdin_is_tty: bool, stderr_is_tty: bool) -> Result<(), String> { + if stdin_is_tty && stderr_is_tty { + Ok(()) + } else { + Err( + "interactive sharing requires a TTY; use --non-interactive with --workspace-id for automation" + .to_string(), + ) + } +} + +async fn resolve_share_options( + options: WorkdirShareOptions, +) -> Result, String> { + if options.non_interactive { + let workspace_id = options.workspace_id.clone().ok_or_else(|| { + "non-interactive sharing requires --workspace-id; no Workspace is inferred from cwd or path" + .to_string() + })?; + return Ok(Some(ResolvedWorkdirShareOptions { + path: options.path, + workspace_id, + backend_url: options.backend_url, + display_name: options.display_name, + ttl: options.ttl, + permission: options.permission, + })); + } + ensure_interactive_tty( + std::io::stdin().is_terminal(), + std::io::stderr().is_terminal(), + )?; + let target = client::BackendWorkspaceCatalogTarget::new(options.backend_url.clone()); + let workspaces = client::list_backend_workspaces(&target) + .await + .map_err(|error| format!("failed to query Backend Workspace catalog: {error}"))?; + let stdin = std::io::stdin(); + let stderr = std::io::stderr(); + let mut input = stdin.lock(); + let mut output = stderr.lock(); + configure_interactively(options, &workspaces, &mut input, &mut output) +} + pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> { + let Some(options) = resolve_share_options(options).await? else { + eprintln!("External Workdir share cancelled; no grant was created."); + return Ok(()); + }; // Pin before creating remote authority: the Backend grant can never outlive // a failed local approval/open race, and subsequent path replacement cannot // redirect the provider. @@ -142,30 +386,50 @@ pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> { ExternalWorkdirGrantCreateRequest { provider_instance_id: provider_instance_id.clone(), display_name: options.display_name.clone(), - ttl_seconds: options.ttl.as_secs(), - read_only: true, + ttl_seconds: options.ttl.map(|ttl| ttl.as_secs()), + permission: options.permission, }, ) .await .map_err(|error| format!("External Workdir grant request failed: {error}"))?; let limits = BoundedReadLimits::EXTERNAL_DEFAULT; - let session = LocalWorkdirSession::external_read_only_pinned( - Workdir::new(&grant.working_directory_id), - pinned_root, - limits, - ) + let session = match options.permission { + server_api::ExternalWorkdirPermission::ReadOnly => { + LocalWorkdirSession::external_read_only_pinned( + Workdir::new(&grant.working_directory_id), + pinned_root, + limits, + ) + } + server_api::ExternalWorkdirPermission::ReadWrite => { + LocalWorkdirSession::external_read_write_pinned( + Workdir::new(&grant.working_directory_id), + pinned_root, + limits, + ) + } + } .map_err(|error| error.to_string())?; println!("External Workdir grant: {}", grant.grant_id); println!("Logical Workdir: {}", grant.working_directory_id); println!("Display name: {}", grant.display_name); - println!("Permission: {}", grant.permissions); - println!("Expires at: {}", grant.expires_at); + println!("Permission: {}", permission_label(grant.permission)); + println!( + "Expires at: {}", + grant.expires_at.as_deref().unwrap_or("never (until stopped or revoked)") + ); - let expires_at = chrono::DateTime::parse_from_rfc3339(&grant.expires_at) - .map_err(|_| "Backend returned an invalid External Workdir expiry".to_string())? - .with_timezone(&Utc); + let expires_at = grant + .expires_at + .as_deref() + .map(|expires_at| { + chrono::DateTime::parse_from_rfc3339(expires_at) + .map(|expires_at| expires_at.with_timezone(&Utc)) + .map_err(|_| "Backend returned an invalid External Workdir expiry".to_string()) + }) + .transpose()?; let mut generation = grant.generation; loop { let outcome = serve_provider_connection( @@ -181,7 +445,7 @@ pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> { let outcome = match outcome { Ok(outcome) => outcome, Err(connection_error) => { - if Utc::now() >= expires_at { + if expires_at.is_some_and(|expires_at| Utc::now() >= expires_at) { return Err(format!( "External Workdir grant expired after provider connection failure: {connection_error}" )); @@ -257,7 +521,7 @@ pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> { return Ok(()); } ProviderEnd::Disconnected => { - if Utc::now() >= expires_at { + if expires_at.is_some_and(|expires_at| Utc::now() >= expires_at) { return Err("External Workdir grant expired while disconnected".to_string()); } generation = generation @@ -278,7 +542,7 @@ pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> { async fn serve_provider_connection( backend_client: &BackendApiClient, server_api_client: &ServerApiClient, - options: &WorkdirShareOptions, + options: &ResolvedWorkdirShareOptions, grant: &ExternalWorkdirGrantResponse, provider_instance_id: &str, session: &LocalWorkdirSession, @@ -314,7 +578,7 @@ async fn serve_provider_connection( .map_err(|error| error.to_string())?, workdir_id: session.workdir().id().clone(), generation, - capabilities: WorkdirSessionCapabilities::READ_ONLY, + capabilities: session.capabilities(), read_limits: BoundedReadLimits::EXTERNAL_DEFAULT, }, }); @@ -646,6 +910,114 @@ mod tests { use std::net::{TcpListener, TcpStream}; use std::sync::mpsc; + fn share_options() -> WorkdirShareOptions { + WorkdirShareOptions { + path: PathBuf::from("."), + workspace_id: None, + backend_url: "https://backend.example".to_string(), + display_name: "External Workdir".to_string(), + ttl: None, + permission: server_api::ExternalWorkdirPermission::ReadOnly, + non_interactive: false, + } + } + + fn workspace(workspace_id: &str, display_name: &str) -> server_api::WorkspaceSummary { + server_api::WorkspaceSummary { + workspace_id: workspace_id.to_string(), + owner_account_id: "account-a".to_string(), + display_name: display_name.to_string(), + state: "active".to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } + } + + #[test] + fn interactive_share_defaults_to_first_workspace_read_only_and_unlimited() { + let workspaces = [workspace("workspace-a", "Alpha")]; + let mut input = std::io::Cursor::new(b"\n\n\nyes\n".to_vec()); + let mut output = Vec::new(); + + let selected = configure_interactively( + share_options(), + &workspaces, + &mut input, + &mut output, + ) + .unwrap() + .unwrap(); + + assert_eq!(selected.workspace_id, "workspace-a"); + assert_eq!( + selected.permission, + server_api::ExternalWorkdirPermission::ReadOnly + ); + assert_eq!(selected.ttl, None); + assert!( + String::from_utf8(output) + .unwrap() + .contains("Connect and create grant? [y/N]") + ); + } + + #[test] + fn interactive_share_requires_explicit_read_write_selection_and_confirmation() { + let workspaces = [ + workspace("workspace-a", "Alpha"), + workspace("workspace-b", "Beta"), + ]; + let mut input = std::io::Cursor::new(b"2\nw\n15m\ny\n".to_vec()); + let mut output = Vec::new(); + + let selected = configure_interactively( + share_options(), + &workspaces, + &mut input, + &mut output, + ) + .unwrap() + .unwrap(); + + assert_eq!(selected.workspace_id, "workspace-b"); + assert_eq!( + selected.permission, + server_api::ExternalWorkdirPermission::ReadWrite + ); + assert_eq!(selected.ttl, Some(Duration::from_secs(900))); + } + + #[test] + fn interactive_share_cancellation_yields_no_grant_configuration() { + let workspaces = [workspace("workspace-a", "Alpha")]; + let mut declined = std::io::Cursor::new(b"\n\n\nno\n".to_vec()); + let mut output = Vec::new(); + assert!( + configure_interactively( + share_options(), + &workspaces, + &mut declined, + &mut output, + ) + .unwrap() + .is_none() + ); + + let mut eof = std::io::Cursor::new(Vec::::new()); + assert!( + configure_interactively(share_options(), &workspaces, &mut eof, &mut Vec::new()) + .unwrap() + .is_none() + ); + } + + #[test] + fn interactive_share_fails_closed_without_input_and_prompt_ttys() { + assert!(ensure_interactive_tty(true, true).is_ok()); + assert!(ensure_interactive_tty(false, true).unwrap_err().contains("--non-interactive")); + assert!(ensure_interactive_tty(true, false).unwrap_err().contains("--non-interactive")); + } + fn test_server_api_client(base_url: &str) -> ServerApiClient { let mut headers = reqwest::header::HeaderMap::new(); headers.insert( @@ -726,7 +1098,7 @@ mod tests { "working_directory_id": "workdir-a", "provider_instance_id": "provider-a", "display_name": "Shared files", - "permissions": "read_only", + "permission": "read_only", "expires_at": "2026-09-23T03:00:00Z", "generation": 7, "status": "pending" @@ -745,8 +1117,8 @@ mod tests { ExternalWorkdirGrantCreateRequest { provider_instance_id: "provider-a".to_string(), display_name: "Shared files".to_string(), - ttl_seconds: 600, - read_only: true, + ttl_seconds: Some(600), + permission: server_api::ExternalWorkdirPermission::ReadOnly, }, ) .await