From 39ecd1b239579d5797596666a5ac1d7e737ac92f Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 30 Sep 2026 02:01:12 +0900 Subject: [PATCH] fix: enforce hierarchical external Workdir permissions --- crates/server-api/src/lib.rs | 62 +++- crates/workspace-server/src/server.rs | 333 ++++++++++++------ crates/yoi/src/main.rs | 72 ++-- crates/yoi/src/workdir_share.rs | 123 ++++--- docs/development/external-workdir-sharing.md | 47 ++- openapi/server-api.json | 8 +- .../src/lib/generated/companion-api.ts | 2 +- .../src/lib/generated/repository-api.ts | 2 +- .../src/lib/generated/runtime-api.ts | 2 +- web/workspace/src/lib/generated/ticket-api.ts | 2 +- .../src/lib/generated/worker-launch-api.ts | 2 +- .../src/lib/workspace/api/workdirs.ts | 21 +- .../src/lib/workspace/api/workers.test.ts | 2 +- .../workspace/settings/workdir-permissions.ts | 20 +- .../w/[workspaceId]/workers/new/+page.svelte | 4 +- web/workspace/tests/workdir-api.test.ts | 15 +- .../tests/workdir-permissions.test.ts | 36 +- 17 files changed, 510 insertions(+), 243 deletions(-) diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index e68a33cf2..ac82de16a 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -5262,27 +5262,71 @@ impl From for WorkingDirectorySumma } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExternalWorkdirPermissionLevel { + Read, + Write, + Command, +} + +impl ExternalWorkdirPermissionLevel { + pub const fn permissions(self) -> ExternalWorkdirPermissions { + match self { + Self::Read => ExternalWorkdirPermissions::READ, + Self::Write => ExternalWorkdirPermissions::WRITE, + Self::Command => ExternalWorkdirPermissions::COMMAND, + } + } +} + #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(deny_unknown_fields)] pub struct ExternalWorkdirPermissions { /// Read files and use read-only discovery operations such as Glob and Grep. + /// Every supported External Workdir permission level enables this field. pub read: bool, - /// Create or update files with Write and Edit. Existing-file mutation also - /// requires `read` so callers can satisfy the read-before-write contract. + /// Create or update files with Write and Edit. WRITE includes READ so callers + /// can satisfy the read-before-write contract. pub write: bool, - /// Execute commands as the sharing CLI user on the provider host. This is - /// explicit authority and does not imply either filesystem category. + /// Execute commands as the sharing CLI user on the provider host. COMMAND + /// includes WRITE and READ, remains explicit, and is not sandboxed. pub command: bool, } +impl ExternalWorkdirPermissions { + pub const READ: Self = Self { + read: true, + write: false, + command: false, + }; + pub const WRITE: Self = Self { + read: true, + write: true, + command: false, + }; + pub const COMMAND: Self = Self { + read: true, + write: true, + command: true, + }; + + /// Returns the single hierarchical level represented by a grant permission + /// object. Partial combinations remain valid for attachment/session + /// projections, but are not valid authority for creating an External grant. + pub const fn grant_level(self) -> Option { + match (self.read, self.write, self.command) { + (true, false, false) => Some(ExternalWorkdirPermissionLevel::Read), + (true, true, false) => Some(ExternalWorkdirPermissionLevel::Write), + (true, true, true) => Some(ExternalWorkdirPermissionLevel::Command), + _ => None, + } + } +} + impl Default for ExternalWorkdirPermissions { fn default() -> Self { - Self { - read: true, - write: false, - command: false, - } + Self::READ } } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index abd2b5ec4..31649fcb3 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3905,72 +3905,47 @@ struct ValidatedTicketImplementationTarget { } fn external_workdir_permissions(persisted: &str) -> Result { - let permissions = match persisted { - "read_only" => server_api::ExternalWorkdirPermissions { - read: true, - write: false, - command: false, - }, - "read_write" => server_api::ExternalWorkdirPermissions { - read: true, - write: true, - command: false, - }, - "command_only" => server_api::ExternalWorkdirPermissions { - read: false, - write: false, - command: true, - }, - "read_command" => server_api::ExternalWorkdirPermissions { - read: true, - write: false, - command: true, - }, - "read_write_command" => server_api::ExternalWorkdirPermissions { - read: true, - write: true, - command: true, - }, - _ => { - return Err(Error::Store( - "External Workdir grant has invalid persisted permissions".to_string(), - )); - } - }; - Ok(permissions) + match persisted { + "read_only" => Ok(server_api::ExternalWorkdirPermissions::READ), + "read_write" => Ok(server_api::ExternalWorkdirPermissions::WRITE), + "read_write_command" => Ok(server_api::ExternalWorkdirPermissions::COMMAND), + "command_only" | "read_command" => Err(Error::InvalidInput( + "External Workdir grant uses a legacy non-hierarchical permission combination; revoke it and share the directory again with READ, WRITE, or COMMAND" + .to_string(), + )), + _ => Err(Error::Store( + "External Workdir grant has invalid persisted permissions".to_string(), + )), + } } fn persist_external_workdir_permissions( permissions: server_api::ExternalWorkdirPermissions, ) -> Result<&'static str> { - match (permissions.read, permissions.write, permissions.command) { - (true, false, false) => Ok("read_only"), - (true, true, false) => Ok("read_write"), - (false, false, true) => Ok("command_only"), - (true, false, true) => Ok("read_command"), - (true, true, true) => Ok("read_write_command"), - (false, true, _) => Err(Error::InvalidInput( - "External Workdir WRITE permission requires READ for read-before-write".to_string(), - )), - (false, false, false) => Err(Error::InvalidInput( - "External Workdir sharing requires at least one of READ or COMMAND".to_string(), + match permissions.grant_level() { + Some(server_api::ExternalWorkdirPermissionLevel::Read) => Ok("read_only"), + Some(server_api::ExternalWorkdirPermissionLevel::Write) => Ok("read_write"), + Some(server_api::ExternalWorkdirPermissionLevel::Command) => Ok("read_write_command"), + None => Err(Error::InvalidInput( + "External Workdir permissions must select exactly one level: READ=(true,false,false), WRITE=(true,true,false), or COMMAND=(true,true,true)" + .to_string(), )), } } fn external_workdir_capabilities(permissions: &str) -> Result { - let permissions = external_workdir_permissions(permissions)?; - let mut capabilities = workdir::WorkdirSessionCapabilities::EMPTY; - if permissions.read { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::READ_ONLY); + match external_workdir_permissions(permissions)?.grant_level() { + Some(server_api::ExternalWorkdirPermissionLevel::Read) => { + Ok(workdir::WorkdirSessionCapabilities::READ_ONLY) + } + Some(server_api::ExternalWorkdirPermissionLevel::Write) => { + Ok(workdir::WorkdirSessionCapabilities::READ_WRITE) + } + Some(server_api::ExternalWorkdirPermissionLevel::Command) => { + Ok(workdir::WorkdirSessionCapabilities::ALL) + } + None => unreachable!("persisted External grant permissions were already validated"), } - if permissions.write { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::WRITE_ONLY); - } - if permissions.command { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::COMMAND_ONLY); - } - Ok(capabilities) } fn workdir_permission_summary( @@ -26480,36 +26455,49 @@ mod tests { }; #[test] - fn external_permission_categories_map_to_existing_capability_bits_without_escalation() { + fn external_permission_levels_map_to_existing_bits_and_reject_legacy_partial_grants() { + let bits = |capabilities| { + serde_json::to_value(capabilities).unwrap()["bits"] + .as_u64() + .unwrap() + }; assert_eq!( - external_workdir_capabilities("read_only").unwrap(), - workdir::WorkdirSessionCapabilities::READ_ONLY + bits(external_workdir_capabilities("read_only").unwrap()), + 25 ); assert_eq!( - external_workdir_capabilities("read_write").unwrap(), - workdir::WorkdirSessionCapabilities::READ_WRITE + bits(external_workdir_capabilities("read_write").unwrap()), + 31 ); assert_eq!( - external_workdir_capabilities("command_only").unwrap(), - workdir::WorkdirSessionCapabilities::COMMAND_ONLY + bits(external_workdir_capabilities("read_write_command").unwrap()), + 63 ); - assert_eq!( - external_workdir_capabilities("read_command").unwrap(), - workdir::WorkdirSessionCapabilities::READ_ONLY - .union(workdir::WorkdirSessionCapabilities::COMMAND_ONLY) - ); - assert_eq!( - external_workdir_capabilities("read_write_command").unwrap(), - workdir::WorkdirSessionCapabilities::ALL - ); - assert!( - persist_external_workdir_permissions(server_api::ExternalWorkdirPermissions { + + for legacy in ["command_only", "read_command"] { + let error = external_workdir_capabilities(legacy).unwrap_err(); + assert!(error.to_string().contains("revoke it and share")); + } + for invalid in [ + server_api::ExternalWorkdirPermissions { + read: false, + write: false, + command: true, + }, + server_api::ExternalWorkdirPermissions { read: false, write: true, command: false, - }) - .is_err() - ); + }, + server_api::ExternalWorkdirPermissions { + read: true, + write: false, + command: true, + }, + ] { + let error = persist_external_workdir_permissions(invalid).unwrap_err(); + assert!(error.to_string().contains("exactly one level")); + } } #[test] @@ -26736,7 +26724,41 @@ mod tests { } #[tokio::test] - async fn external_provider_command_only_session_denies_read_and_routes_bounded_lifecycle() { + async fn external_provider_write_level_denies_command_without_provider_fallback() { + let (sender, mut commands) = tokio::sync::mpsc::channel(1); + let connection = Arc::new(ExternalProviderConnection { + grant_id: "grant-write".to_string(), + workdir_id: "external-write".to_string(), + provider_instance_id: "provider-write".to_string(), + generation: 1, + expires_at: None, + capabilities: workdir::WorkdirSessionCapabilities::READ_WRITE, + admission: Arc::new(tokio::sync::Semaphore::new(16)), + shutdown_confirmed: Arc::new(std::sync::atomic::AtomicBool::new(false)), + sender, + }); + let session = ExternalProviderWorkdirSession::new(connection, None); + let denied = session + .start_command(workdir::CommandRequest { + command: "printf forbidden".to_string(), + timeout_secs: 30, + output_limit: 1024, + cwd: workdir::WorkdirPath::root(), + spill_dir: None, + tool_call_id: None, + }) + .await; + assert!(matches!( + denied, + Err(workdir::WorkdirError::Unsupported( + workdir::WorkdirSessionCapability::Command + )) + )); + assert!(commands.try_recv().is_err()); + } + + #[tokio::test] + async fn external_provider_partial_session_capabilities_remain_supported() { let (sender, mut commands) = tokio::sync::mpsc::channel(4); let capabilities = workdir::WorkdirSessionCapabilities::COMMAND_ONLY; let connection = Arc::new(ExternalProviderConnection { @@ -27445,6 +27467,105 @@ mod tests { assert!(audit_actions.contains(&"external_workdir_cleanup_completed".to_string())); } + #[tokio::test] + async fn external_grant_api_persists_only_hierarchical_levels_with_exact_bits() { + let dir = tempfile::tempdir().unwrap(); + init_clean_git_workspace(dir.path()); + let api = test_api(dir.path()).await; + let levels = [ + ( + server_api::ExternalWorkdirPermissions::READ, + "read_only", + 25, + ), + ( + server_api::ExternalWorkdirPermissions::WRITE, + "read_write", + 31, + ), + ( + server_api::ExternalWorkdirPermissions::COMMAND, + "read_write_command", + 63, + ), + ]; + for (index, (permissions, persisted, expected_bits)) in levels.into_iter().enumerate() { + let (_, Json(grant)) = scoped_create_external_workdir_grant( + State(api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + }), + Extension(test_browser_request_actor()), + Json(ExternalWorkdirGrantCreateRequest { + provider_instance_id: format!("provider-{index}"), + display_name: format!("Level {index}"), + ttl_seconds: None, + permissions, + }), + ) + .await + .unwrap(); + assert_eq!(grant.permissions, permissions); + let stored = api + .store + .get_external_workdir_grant(TEST_WORKSPACE_ID, &grant.grant_id) + .unwrap() + .unwrap(); + assert_eq!(stored.permissions, persisted); + let bits = + serde_json::to_value(external_workdir_capabilities(&stored.permissions).unwrap()) + .unwrap()["bits"] + .as_u64() + .unwrap(); + assert_eq!(bits, expected_bits); + } + + for (index, permissions) in [ + server_api::ExternalWorkdirPermissions { + read: false, + write: false, + command: true, + }, + server_api::ExternalWorkdirPermissions { + read: false, + write: true, + command: false, + }, + server_api::ExternalWorkdirPermissions { + read: true, + write: false, + command: true, + }, + ] + .into_iter() + .enumerate() + { + let error = scoped_create_external_workdir_grant( + State(api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + }), + Extension(test_browser_request_actor()), + Json(ExternalWorkdirGrantCreateRequest { + provider_instance_id: format!("invalid-provider-{index}"), + display_name: format!("Invalid {index}"), + ttl_seconds: None, + permissions, + }), + ) + .await + .unwrap_err(); + assert!(error.error.to_string().contains("exactly one level")); + } + assert_eq!( + api.store + .list_external_workdir_grants(TEST_WORKSPACE_ID) + .unwrap() + .len(), + 3 + ); + } + #[tokio::test] async fn external_workdirs_are_launchable_with_real_multiple_attachments() { let dir = tempfile::tempdir().unwrap(); @@ -27460,15 +27581,11 @@ mod tests { for index in 1..=2 { let (permissions, capabilities) = if index == 1 { - ( - "command_only", - workdir::WorkdirSessionCapabilities::COMMAND_ONLY, - ) + ("read_only", workdir::WorkdirSessionCapabilities::READ_ONLY) } else { ( - "read_command", - workdir::WorkdirSessionCapabilities::READ_ONLY - .union(workdir::WorkdirSessionCapabilities::COMMAND_ONLY), + "read_write_command", + workdir::WorkdirSessionCapabilities::ALL, ) }; let grant = ExternalWorkdirGrantRecord { @@ -27557,17 +27674,9 @@ mod tests { .find(|summary| summary.working_directory_id == *workdir_id) .unwrap(); let expected_permissions = if index == 0 { - server_api::ExternalWorkdirPermissions { - read: false, - write: false, - command: true, - } + server_api::ExternalWorkdirPermissions::READ } else { - server_api::ExternalWorkdirPermissions { - read: true, - write: false, - command: true, - } + server_api::ExternalWorkdirPermissions::COMMAND }; assert!(matches!( &summary.source, @@ -27628,10 +27737,9 @@ mod tests { .unwrap(); for (index, receiver) in receivers.iter_mut().enumerate() { let expected_capabilities = if index == 0 { - workdir::WorkdirSessionCapabilities::COMMAND_ONLY - } else { workdir::WorkdirSessionCapabilities::READ_ONLY - .union(workdir::WorkdirSessionCapabilities::COMMAND_ONLY) + } else { + workdir::WorkdirSessionCapabilities::ALL }; let link = links .iter() @@ -27658,20 +27766,37 @@ mod tests { drop(session_guard); assert_eq!(session.capabilities(), expected_capabilities); if index == 0 { - let denied = session - .read(workdir::ReadRequest { + let denied_write = session + .write(workdir::WriteRequest { path: workdir::WorkdirPath::new("forbidden.txt").unwrap(), - offset: 0, - limit: 1, - max_bytes: 1, + content: b"no fallback".to_vec(), + expected_hash: None, }) .await; assert!(matches!( - denied, + denied_write, Err(workdir::WorkdirError::Unsupported( - workdir::WorkdirSessionCapability::Read + workdir::WorkdirSessionCapability::Write )) | Err(workdir::WorkdirError::Denied(_)) )); + let denied_command = session + .start_command(workdir::CommandRequest { + command: "printf forbidden".to_string(), + timeout_secs: 30, + output_limit: 1024, + cwd: workdir::WorkdirPath::root(), + spill_dir: None, + tool_call_id: None, + }) + .await; + assert!(matches!( + denied_command, + Err(workdir::WorkdirError::Unsupported( + workdir::WorkdirSessionCapability::Command + )) | Err(workdir::WorkdirError::Denied(_)) + )); + assert!(receiver.try_recv().is_err()); + continue; } let mut starter = { diff --git a/crates/yoi/src/main.rs b/crates/yoi/src/main.rs index 7a3577091..924f5b849 100644 --- a/crates/yoi/src/main.rs +++ b/crates/yoi/src/main.rs @@ -623,7 +623,7 @@ fn parse_workdir_args( let mut ttl = None; let mut read_only = false; let mut read_write = false; - let mut explicit_permissions: Option = None; + let mut explicit_permission_level = None; let mut non_interactive = false; let mut index = 0; while index < args.len() { @@ -668,29 +668,22 @@ fn parse_workdir_args( index += 2; } "--permission" => { - let category = required_option_value(args, index, "--permission")?; - let permissions = - explicit_permissions.get_or_insert(server_api::ExternalWorkdirPermissions { - read: false, - write: false, - command: false, - }); - let enabled = match category.to_ascii_lowercase().as_str() { - "read" => &mut permissions.read, - "write" => &mut permissions.write, - "command" => &mut permissions.command, + let level = required_option_value(args, index, "--permission")?; + if explicit_permission_level.is_some() { + return Err(ParseError( + "--permission selects one level and may only be provided once".to_string(), + )); + } + explicit_permission_level = Some(match level.to_ascii_lowercase().as_str() { + "read" => server_api::ExternalWorkdirPermissionLevel::Read, + "write" => server_api::ExternalWorkdirPermissionLevel::Write, + "command" => server_api::ExternalWorkdirPermissionLevel::Command, _ => { return Err(ParseError(format!( - "unknown --permission category `{category}`; expected read, write, or command" + "unknown --permission level `{level}`; expected read, write, or command" ))); } - }; - if *enabled { - return Err(ParseError(format!( - "--permission {category} may only be provided once" - ))); - } - *enabled = true; + }); index += 2; } "--read-only" => { @@ -742,21 +735,18 @@ fn parse_workdir_args( "--read-only conflicts with --read-write".to_string(), )); } - if explicit_permissions.is_some() && (read_only || read_write) { + if explicit_permission_level.is_some() && (read_only || read_write) { return Err(ParseError( "--permission conflicts with legacy --read-only/--read-write".to_string(), )); } - let permissions = explicit_permissions.unwrap_or(server_api::ExternalWorkdirPermissions { - read: true, - write: read_write, - command: false, - }); - if permissions.write && !permissions.read { - return Err(ParseError( - "--permission write requires --permission read for read-before-write".to_string(), - )); - } + let permissions = explicit_permission_level + .map(server_api::ExternalWorkdirPermissionLevel::permissions) + .unwrap_or(if read_write { + server_api::ExternalWorkdirPermissions::WRITE + } else { + server_api::ExternalWorkdirPermissions::READ + }); if non_interactive && workspace_id.is_none() { return Err(ParseError( "--non-interactive requires --workspace-id".to_string(), @@ -1900,7 +1890,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 ] [--permission ]... [--read-only|--read-write] [--ttl ] [--display-name ] [--non-interactive] + yoi [--backend ] workdir share [--workspace-id ] [--permission ] [--read-only|--read-write] [--ttl ] [--display-name ] [--non-interactive] yoi [OPTIONS] Target selection: @@ -1946,7 +1936,7 @@ Options: fn print_workdir_help() { println!( - "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, READ/WRITE/COMMAND categories, and TTL, then explicitly confirm before any grant is created.\nREAD enables Read/Glob/Grep and related observation operations. WRITE enables Write/Edit and requires READ for read-before-write. COMMAND is independent, disabled by default, and runs with the sharing CLI user's host authority; it is not sandboxed.\nRepeat --permission to choose explicit categories in automation. Legacy --read-only and --read-write remain file-only aliases and conflict with --permission.\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 --permission Enable read, write, or command; repeat per category\n --read-only Legacy alias for --permission read\n --read-write Legacy alias for read + write (commands remain disabled)\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" + "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, one READ/WRITE/COMMAND permission level, and TTL, then explicitly confirm before any grant is created.\nREAD enables Read/Glob/Grep and related observation operations. WRITE includes READ and adds Write/Edit. COMMAND includes WRITE and READ, adds command start/status/output/cancel, and runs with the sharing CLI user's host authority; it is not sandboxed.\nUse --permission once to choose the level in automation. Legacy --read-only and --read-write retain the READ and WRITE meanings and conflict with --permission.\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 --permission Grant read, write (includes read), or command (includes write + read)\n --read-only Legacy alias for --permission read\n --read-write Legacy alias for --permission write\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" ); } @@ -2038,7 +2028,7 @@ mod tests { } #[test] - fn workdir_share_parses_independent_command_permission() { + fn workdir_share_parses_command_as_the_full_hierarchical_level() { let args = vec![ "share".to_string(), ".".to_string(), @@ -2056,11 +2046,7 @@ mod tests { }; assert_eq!( options.permissions, - server_api::ExternalWorkdirPermissions { - read: false, - write: false, - command: true, - } + server_api::ExternalWorkdirPermissions::COMMAND ); } @@ -2131,17 +2117,19 @@ mod tests { .contains("conflicts") ); - let write_without_read = vec![ + let multiple_levels = vec![ "share".to_string(), ".".to_string(), "--permission".to_string(), "write".to_string(), + "--permission".to_string(), + "command".to_string(), ]; assert!( - parse_workdir_args(&write_without_read, &target) + parse_workdir_args(&multiple_levels, &target) .unwrap_err() .0 - .contains("requires --permission read") + .contains("selects one level") ); let missing_workspace = TargetSelection { diff --git a/crates/yoi/src/workdir_share.rs b/crates/yoi/src/workdir_share.rs index 196863b3f..2330f6419 100644 --- a/crates/yoi/src/workdir_share.rs +++ b/crates/yoi/src/workdir_share.rs @@ -140,47 +140,42 @@ fn authenticated_server_api_client( Ok(ServerApiClient::with_client(base_url, http_client)) } -fn permissions_label(permissions: server_api::ExternalWorkdirPermissions) -> String { - let mut enabled = Vec::new(); - if permissions.read { - enabled.push("READ"); +fn permissions_label(permissions: server_api::ExternalWorkdirPermissions) -> &'static str { + match permissions.grant_level() { + Some(server_api::ExternalWorkdirPermissionLevel::Read) => "READ", + Some(server_api::ExternalWorkdirPermissionLevel::Write) => "WRITE (includes READ)", + Some(server_api::ExternalWorkdirPermissionLevel::Command) => { + "COMMAND (includes WRITE + READ)" + } + None => "INVALID", } - if permissions.write { - enabled.push("WRITE"); - } - if permissions.command { - enabled.push("COMMAND"); - } - enabled.join(", ") } fn validate_permissions(permissions: server_api::ExternalWorkdirPermissions) -> Result<(), String> { - if permissions.write && !permissions.read { + if permissions.grant_level().is_none() { return Err( - "WRITE requires READ so existing files can satisfy read-before-write; select both categories" + "select exactly one permission level: READ, WRITE (includes READ), or COMMAND (includes WRITE + READ)" .to_string(), ); } - if !permissions.read && !permissions.command { - return Err("select at least one of READ or COMMAND".to_string()); - } Ok(()) } fn workdir_capabilities( permissions: server_api::ExternalWorkdirPermissions, ) -> workdir::WorkdirSessionCapabilities { - let mut capabilities = workdir::WorkdirSessionCapabilities::EMPTY; - if permissions.read { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::READ_ONLY); + match permissions.grant_level() { + Some(server_api::ExternalWorkdirPermissionLevel::Read) => { + workdir::WorkdirSessionCapabilities::READ_ONLY + } + Some(server_api::ExternalWorkdirPermissionLevel::Write) => { + workdir::WorkdirSessionCapabilities::READ_WRITE + } + Some(server_api::ExternalWorkdirPermissionLevel::Command) => { + workdir::WorkdirSessionCapabilities::ALL + } + None => workdir::WorkdirSessionCapabilities::EMPTY, } - if permissions.write { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::WRITE_ONLY); - } - if permissions.command { - capabilities = capabilities.union(workdir::WorkdirSessionCapabilities::COMMAND_ONLY); - } - capabilities } #[derive(Debug, Clone, PartialEq, Eq)] @@ -302,14 +297,14 @@ fn configure_interactively( writeln!( output, - "Permissions are independent categories: READ (Read/Glob/Grep), WRITE (Write/Edit), and COMMAND (host command lifecycle). WRITE requires READ for read-before-write. COMMAND runs with this CLI user's host authority and is not sandboxed." + "Permission levels: READ (Read/Glob/Grep), WRITE (includes READ + Write/Edit), or COMMAND (includes WRITE + READ + host command lifecycle). COMMAND runs with this CLI user's host authority and is not sandboxed." ) .map_err(|error| format!("failed to write prompt: {error}"))?; let Some(permission_input) = read_prompt_line( input, output, &format!( - "Permissions [comma-separated READ/WRITE/COMMAND; current {}; q to cancel]: ", + "Permission level [READ/WRITE/COMMAND; current {}; q to cancel]: ", permissions_label(options.permissions) ), )? @@ -320,28 +315,20 @@ fn configure_interactively( return Ok(None); } if !permission_input.is_empty() { - let mut permissions = server_api::ExternalWorkdirPermissions { - read: false, - write: false, - command: false, - }; - for category in permission_input - .split([',', ' ']) - .filter(|category| !category.is_empty()) - { - match category.to_ascii_lowercase().as_str() { - "r" | "read" | "read-only" | "read_only" => permissions.read = true, - "w" | "write" | "read-write" | "read_write" => permissions.write = true, - "c" | "command" => permissions.command = true, - _ => { - return Err(format!( - "unknown permission category `{category}`; use READ, WRITE, or COMMAND" - )); - } + options.permissions = match permission_input.to_ascii_lowercase().as_str() { + "r" | "read" | "read-only" | "read_only" => { + server_api::ExternalWorkdirPermissions::READ } - } - validate_permissions(permissions)?; - options.permissions = permissions; + "w" | "write" | "read-write" | "read_write" => { + server_api::ExternalWorkdirPermissions::WRITE + } + "c" | "command" => server_api::ExternalWorkdirPermissions::COMMAND, + _ => { + return Err(format!( + "unknown permission level `{permission_input}`; use READ, WRITE, or COMMAND" + )); + } + }; } writeln!(output, "TTL: {}", ttl_label(options.ttl)) @@ -1085,12 +1072,12 @@ mod tests { } #[test] - fn interactive_share_requires_explicit_write_and_command_selection_and_confirmation() { + fn interactive_share_selects_one_hierarchical_level_and_requires_confirmation() { let workspaces = [ workspace("workspace-a", "Alpha"), workspace("workspace-b", "Beta"), ]; - let mut input = std::io::Cursor::new(b"2\nREAD,WRITE,COMMAND\n15m\ny\n".to_vec()); + let mut input = std::io::Cursor::new(b"2\nCOMMAND\n15m\ny\n".to_vec()); let mut output = Vec::new(); let selected = @@ -1110,7 +1097,39 @@ mod tests { assert_eq!(selected.ttl, Some(Duration::from_secs(900))); let output = String::from_utf8(output).unwrap(); assert!(output.contains("not sandboxed")); - assert!(output.contains("Permissions: READ, WRITE, COMMAND")); + assert!(output.contains("Permissions: COMMAND (includes WRITE + READ)")); + } + + #[test] + fn share_permission_levels_map_to_exact_existing_capability_bits() { + let bits = |permissions| { + serde_json::to_value(workdir_capabilities(permissions)).unwrap()["bits"] + .as_u64() + .unwrap() + }; + assert_eq!(bits(server_api::ExternalWorkdirPermissions::READ), 25); + assert_eq!(bits(server_api::ExternalWorkdirPermissions::WRITE), 31); + assert_eq!(bits(server_api::ExternalWorkdirPermissions::COMMAND), 63); + + for invalid in [ + server_api::ExternalWorkdirPermissions { + read: false, + write: false, + command: true, + }, + server_api::ExternalWorkdirPermissions { + read: false, + write: true, + command: false, + }, + server_api::ExternalWorkdirPermissions { + read: true, + write: false, + command: true, + }, + ] { + assert!(validate_permissions(invalid).is_err()); + } } #[test] diff --git a/docs/development/external-workdir-sharing.md b/docs/development/external-workdir-sharing.md index dbe980168..1030bc6ef 100644 --- a/docs/development/external-workdir-sharing.md +++ b/docs/development/external-workdir-sharing.md @@ -1,34 +1,49 @@ # External Workdir sharing permissions `yoi workdir share` exposes a client-hosted directory through an authenticated, -outbound Backend connection. The operator grants three independent categories: +outbound Backend connection. The operator selects exactly one permission level: -- **READ**: file reads and read-only discovery operations such as Stat, List, - ViewImage, Glob, and Grep. -- **WRITE**: Write and Edit. WRITE requires READ because updating an existing - file uses the read-before-write content-hash contract. Selecting WRITE never - enables COMMAND. -- **COMMAND**: command start, status, bounded output, and cancellation. COMMAND - does not enable READ or WRITE. It executes on the sharing host with the OS +- **READ** (capability bits `25`): file reads and read-only discovery operations + such as Stat, List, ViewImage, Glob, and Grep. +- **WRITE** (capability bits `31`): READ plus Write and Edit. Including READ + preserves the read-before-write content-hash contract. WRITE never enables + COMMAND. +- **COMMAND** (capability bits `63`): WRITE plus READ plus command start, status, + bounded output, and cancellation. It executes on the sharing host with the OS authority of the user running the CLI; External Workdir command execution is **not a sandbox** and does not confine the process to the shared directory. -READ is enabled by default and COMMAND is disabled by default. Interactive mode -asks for a comma-separated category list and shows the exact grant before it is -created. For automation, repeat `--permission`: +In short, `COMMAND ⇒ WRITE ⇒ READ`. READ is the default. Interactive mode asks +for one level and shows its included permissions before creating the grant. For +automation, pass `--permission` once: ```sh yoi --backend https://example.invalid workdir share ./project \ --workspace-id workspace-a \ - --permission read \ - --permission write \ --permission command \ --non-interactive ``` -`--read-only` remains an alias for READ. `--read-write` remains an alias for -READ+WRITE and never enables COMMAND. These legacy flags conflict with explicit -`--permission` arguments so ambiguous automation fails closed. +`--read-only` remains an alias for the READ level. `--read-write` remains an +alias for the WRITE level and never enables COMMAND. These legacy flags conflict +with `--permission`, and multiple `--permission` arguments are rejected so +ambiguous automation fails closed. + +The grant API retains the boolean projection for compatibility, but accepts only +these combinations when creating new grants: + +| Level | `read` | `write` | `command` | +|---|---:|---:|---:| +| READ | `true` | `false` | `false` | +| WRITE | `true` | `true` | `false` | +| COMMAND | `true` | `true` | `true` | + +Older non-hierarchical grants such as command-only or READ+COMMAND are not +silently widened. The Backend refuses their projection, provider registration, +and subsequent restore/reconnect with guidance to revoke and re-share the +directory at one of the supported levels. General Workdir attachment/session +capability intersections may still produce partial capability sets; that +restriction applies only to External grant authority. Omitting `--ttl` creates no automatic expiry. An explicit TTL must be between 60 seconds and 24 hours. Ctrl-C, revoke, expiry, provider disconnect, attachment diff --git a/openapi/server-api.json b/openapi/server-api.json index 85a0e6c25..3e9bd5063 100644 --- a/openapi/server-api.json +++ b/openapi/server-api.json @@ -2092,15 +2092,15 @@ "additionalProperties": false, "properties": { "command": { - "description": "Execute commands as the sharing CLI user on the provider host. This is\nexplicit authority and does not imply either filesystem category.", + "description": "Execute commands as the sharing CLI user on the provider host. COMMAND\nincludes WRITE and READ, remains explicit, and is not sandboxed.", "type": "boolean" }, "read": { - "description": "Read files and use read-only discovery operations such as Glob and Grep.", + "description": "Read files and use read-only discovery operations such as Glob and Grep.\nEvery supported External Workdir permission level enables this field.", "type": "boolean" }, "write": { - "description": "Create or update files with Write and Edit. Existing-file mutation also\nrequires `read` so callers can satisfy the read-before-write contract.", + "description": "Create or update files with Write and Edit. WRITE includes READ so callers\ncan satisfy the read-before-write contract.", "type": "boolean" } }, @@ -14020,7 +14020,7 @@ "info": { "title": "Yoi Server API", "version": "0.1.0", - "x-yoi-source-digest": "sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c" + "x-yoi-source-digest": "sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052" }, "openapi": "3.1.0", "paths": { diff --git a/web/workspace/src/lib/generated/companion-api.ts b/web/workspace/src/lib/generated/companion-api.ts index df244651f..9135fc09d 100644 --- a/web/workspace/src/lib/generated/companion-api.ts +++ b/web/workspace/src/lib/generated/companion-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-companion-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c) +// Input contract: 0.1.0 (sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052) // Options: companion-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/companion-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_companion_api_types -- --check diff --git a/web/workspace/src/lib/generated/repository-api.ts b/web/workspace/src/lib/generated/repository-api.ts index 04e182360..227916e94 100644 --- a/web/workspace/src/lib/generated/repository-api.ts +++ b/web/workspace/src/lib/generated/repository-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-repository-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c) +// Input contract: 0.1.0 (sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052) // Options: repository-operation-schema-closure,openapi-3.1,format:uint64=safe-number,strict-safe-integers,sorted-output // Output: web/workspace/src/lib/generated/repository-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_repository_openapi_types -- --check diff --git a/web/workspace/src/lib/generated/runtime-api.ts b/web/workspace/src/lib/generated/runtime-api.ts index 83d4c91a0..6359c3604 100644 --- a/web/workspace/src/lib/generated/runtime-api.ts +++ b/web/workspace/src/lib/generated/runtime-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-runtime-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c) +// Input contract: 0.1.0 (sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052) // Options: runtime-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/runtime-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_runtime_api_types -- --check diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index 4e8e5ad7d..58567530f 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-ticket-browser-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c) +// Input contract: 0.1.0 (sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052) // Options: ticket-objective-merge-request-browser-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/ticket-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_ticket_api_types -- --check diff --git a/web/workspace/src/lib/generated/worker-launch-api.ts b/web/workspace/src/lib/generated/worker-launch-api.ts index ce3cb7aa0..d07ed1ff2 100644 --- a/web/workspace/src/lib/generated/worker-launch-api.ts +++ b/web/workspace/src/lib/generated/worker-launch-api.ts @@ -1,6 +1,6 @@ // @generated by yoi-worker-openapi-typescript@1. Do not edit. // Input: openapi/server-api.json -// Input contract: 0.1.0 (sha256:3a298734fd19f3a224e4f1503385fb8a823b612d328224f45f677ebaca28b96c) +// Input contract: 0.1.0 (sha256:7b06d005d3436758c96fc4cfd462df76460f89620fd1b58fdf05009d96bb9052) // Options: worker-operation-schema-closure,openapi-3.1,strict-integer-ranges,sorted-output // Output: web/workspace/src/lib/generated/worker-launch-api.ts // Regenerate/check: cargo run -q -p server-api --example generate_worker_launch_api_types -- --check diff --git a/web/workspace/src/lib/workspace/api/workdirs.ts b/web/workspace/src/lib/workspace/api/workdirs.ts index f22724f07..8df77af4a 100644 --- a/web/workspace/src/lib/workspace/api/workdirs.ts +++ b/web/workspace/src/lib/workspace/api/workdirs.ts @@ -182,7 +182,9 @@ function parseWorkingDirectorySource(value: unknown): WorkingDirectorySource { ); const kind = stringField(source, "kind"); if (kind === "repository") { - if (source.grant_id !== undefined || source.grant_permissions !== undefined) { + if ( + source.grant_id !== undefined || source.grant_permissions !== undefined + ) { throw new Error( "Repository Workdir source must not contain External grant authority", ); @@ -201,7 +203,7 @@ function parseWorkingDirectorySource(value: unknown): WorkingDirectorySource { return { kind, grant_id: stringField(source, "grant_id"), - grant_permissions: parseExternalWorkdirPermissions( + grant_permissions: parseExternalWorkdirGrantPermissions( source.grant_permissions, "External Workdir grant permissions", ), @@ -226,6 +228,21 @@ export function parseExternalWorkdirPermissions( }; } +export function parseExternalWorkdirGrantPermissions( + value: unknown, + label = "External Workdir grant permissions", +): ExternalWorkdirPermissions { + const permissions = parseExternalWorkdirPermissions(value, label); + const hierarchical = permissions.read && + (!permissions.command || permissions.write); + if (!hierarchical) { + throw new Error( + `${label} must be READ=(true,false,false), WRITE=(true,true,false), or COMMAND=(true,true,true)`, + ); + } + return permissions; +} + function parseCleanupTarget(value: unknown): WorkingDirectoryCleanupTarget { const record = exactRecord( value, diff --git a/web/workspace/src/lib/workspace/api/workers.test.ts b/web/workspace/src/lib/workspace/api/workers.test.ts index f6b703159..66b1bada0 100644 --- a/web/workspace/src/lib/workspace/api/workers.test.ts +++ b/web/workspace/src/lib/workspace/api/workers.test.ts @@ -106,7 +106,7 @@ Deno.test("Worker summary parser preserves attachment-effective External permiss source: { kind: "external_grant", grant_id: "grant-1", - grant_permissions: { read: true, write: false, command: true }, + grant_permissions: { read: true, write: true, command: true }, }, materializer_kind: "client_hosted_external", status: "active", diff --git a/web/workspace/src/lib/workspace/settings/workdir-permissions.ts b/web/workspace/src/lib/workspace/settings/workdir-permissions.ts index 7942e324d..4024a21f5 100644 --- a/web/workspace/src/lib/workspace/settings/workdir-permissions.ts +++ b/web/workspace/src/lib/workspace/settings/workdir-permissions.ts @@ -1,6 +1,6 @@ import type { ExternalWorkdirPermissions } from "$lib/generated/workdir-api"; -/** Render independent Workdir authority categories in a stable, non-conflating order. */ +/** Render an effective attachment/session capability intersection without widening it. */ export function formatWorkdirPermissions( permissions: ExternalWorkdirPermissions, ): string { @@ -10,3 +10,21 @@ export function formatWorkdirPermissions( if (permissions.command) categories.push("COMMAND"); return categories.length > 0 ? categories.join(" · ") : "NONE"; } + +/** Render one validated External grant level with its inherited authority. */ +export function formatExternalGrantPermissionLevel( + permissions: ExternalWorkdirPermissions, +): string { + if (permissions.read && permissions.write && permissions.command) { + return "COMMAND (includes WRITE + READ)"; + } + if (permissions.read && permissions.write && !permissions.command) { + return "WRITE (includes READ)"; + } + if (permissions.read && !permissions.write && !permissions.command) { + return "READ"; + } + throw new Error( + "External Workdir grant has a non-hierarchical permission set", + ); +} diff --git a/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte index 3c2e82aa1..da2d95782 100644 --- a/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/workers/new/+page.svelte @@ -12,7 +12,7 @@ parseWorkerLaunchOptionsResponse, } from '$lib/workspace/api/workers'; import { formatCurrentWorkdirRevision } from '$lib/workspace/settings/workdir-revision'; - import { formatWorkdirPermissions } from '$lib/workspace/settings/workdir-permissions'; + import { formatExternalGrantPermissionLevel } from '$lib/workspace/settings/workdir-permissions'; import { buildCreateWorkspaceWorkerRequest, defaultWorkerLaunchForm, @@ -40,7 +40,7 @@ : null; const label = directory.display_name ?? repositoryKey ?? 'External Workdir'; if (directory.source.kind === 'external_grant') { - return `${label} · External · ${formatWorkdirPermissions(directory.source.grant_permissions)}`; + return `${label} · External · ${formatExternalGrantPermissionLevel(directory.source.grant_permissions)}`; } return `${label} · ${formatCurrentWorkdirRevision(directory, provider)}`; } diff --git a/web/workspace/tests/workdir-api.test.ts b/web/workspace/tests/workdir-api.test.ts index abb9d8963..c1d941a44 100644 --- a/web/workspace/tests/workdir-api.test.ts +++ b/web/workspace/tests/workdir-api.test.ts @@ -56,7 +56,7 @@ Deno.test("Workdir REST validation accepts External grant sources without Runtim source: { kind: "external_grant", grant_id: "grant-1", - grant_permissions: { read: true, write: false, command: true }, + grant_permissions: { read: true, write: true, command: true }, }, materializer_kind: "client_hosted_external", status: "active", @@ -72,7 +72,7 @@ Deno.test("Workdir REST validation accepts External grant sources without Runtim } if ( !detail.item.source.grant_permissions.read || - detail.item.source.grant_permissions.write || + !detail.item.source.grant_permissions.write || !detail.item.source.grant_permissions.command ) { throw new Error("External Workdir grant permissions were not preserved"); @@ -83,6 +83,11 @@ Deno.test("Workdir REST validation rejects missing or misplaced External grant p for ( const source of [ { kind: "external_grant", grant_id: "grant-1" }, + { + kind: "external_grant", + grant_id: "grant-1", + grant_permissions: { read: false, write: false, command: true }, + }, { kind: "external_grant", grant_id: "grant-1", @@ -113,7 +118,11 @@ Deno.test("Workdir REST validation rejects missing or misplaced External grant p rejected = true; } if (!rejected) { - throw new Error(`invalid External permission projection was accepted: ${JSON.stringify(source)}`); + throw new Error( + `invalid External permission projection was accepted: ${ + JSON.stringify(source) + }`, + ); } } }); diff --git a/web/workspace/tests/workdir-permissions.test.ts b/web/workspace/tests/workdir-permissions.test.ts index 244cb4e47..db16181d0 100644 --- a/web/workspace/tests/workdir-permissions.test.ts +++ b/web/workspace/tests/workdir-permissions.test.ts @@ -2,9 +2,12 @@ declare const Deno: { test(name: string, fn: () => void | Promise): void; }; -import { formatWorkdirPermissions } from "../src/lib/workspace/settings/workdir-permissions.ts"; +import { + formatExternalGrantPermissionLevel, + formatWorkdirPermissions, +} from "../src/lib/workspace/settings/workdir-permissions.ts"; -Deno.test("Workdir permissions render independent categories in stable order", () => { +Deno.test("effective Workdir permissions render partial intersections without widening", () => { const cases = [ [{ read: true, write: false, command: false }, "READ"], [{ read: true, write: true, command: false }, "READ · WRITE"], @@ -20,3 +23,32 @@ Deno.test("Workdir permissions render independent categories in stable order", ( } } }); + +Deno.test("External grant levels show inherited authority and reject partial grants", () => { + const cases = [ + [{ read: true, write: false, command: false }, "READ"], + [{ read: true, write: true, command: false }, "WRITE (includes READ)"], + [ + { read: true, write: true, command: true }, + "COMMAND (includes WRITE + READ)", + ], + ] as const; + for (const [permissions, expected] of cases) { + const actual = formatExternalGrantPermissionLevel(permissions); + if (actual !== expected) { + throw new Error(`expected ${expected}, got ${actual}`); + } + } + + let rejected = false; + try { + formatExternalGrantPermissionLevel({ + read: false, + write: false, + command: true, + }); + } catch { + rejected = true; + } + if (!rejected) throw new Error("command-only External grant was accepted"); +});