diff --git a/crates/worker-runtime/src/config_bundle.rs b/crates/worker-runtime/src/config_bundle.rs index 91837878..18ea771a 100644 --- a/crates/worker-runtime/src/config_bundle.rs +++ b/crates/worker-runtime/src/config_bundle.rs @@ -1,8 +1,7 @@ -use crate::catalog::ProfileSelector; +use crate::catalog::{ConfigBundleRef, ProfileSelector}; use crate::error::RuntimeError; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use std::path::Path; pub const CONFIG_BUNDLE_DIGEST_ALGORITHM: &str = "sha256"; @@ -169,13 +168,14 @@ pub struct ConfigBundleSummary { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct ConfigBundleAvailability { - pub reference: crate::catalog::ConfigBundleRef, + pub reference: ConfigBundleRef, pub summary: ConfigBundleSummary, } pub(crate) fn validate_config_bundle(bundle: &ConfigBundle) -> Result<(), RuntimeError> { - validate_non_empty("config bundle id", &bundle.metadata.id)?; + validate_config_bundle_id(&bundle.metadata.id)?; validate_non_empty("config bundle digest", &bundle.metadata.digest)?; + validate_digest("config bundle digest", &bundle.metadata.digest)?; validate_non_empty("config bundle revision", &bundle.metadata.revision)?; validate_non_empty("config bundle workspace id", &bundle.metadata.workspace_id)?; validate_non_empty("config bundle created_at", &bundle.metadata.created_at)?; @@ -212,9 +212,7 @@ pub(crate) fn validate_config_bundle(bundle: &ConfigBundle) -> Result<(), Runtim for declaration in &bundle.declarations { validate_non_empty("config declaration name", &declaration.name)?; - validate_non_empty("config declaration reference", &declaration.reference)?; validate_boundary_text("config declaration name", &declaration.name)?; - validate_boundary_text("config declaration reference", &declaration.reference)?; if declaration.kind == ConfigDeclarationKind::Unsupported { return Err(RuntimeError::UnsupportedConfigDeclaration { bundle_id: bundle.metadata.id.clone(), @@ -222,10 +220,18 @@ pub(crate) fn validate_config_bundle(bundle: &ConfigBundle) -> Result<(), Runtim name: declaration.name.clone(), }); } + validate_declaration_reference(&bundle.metadata.id, declaration)?; } Ok(()) } +pub(crate) fn validate_config_bundle_ref(reference: &ConfigBundleRef) -> Result<(), RuntimeError> { + validate_config_bundle_id(&reference.id)?; + validate_non_empty("config bundle reference digest", &reference.digest)?; + validate_digest("config bundle reference digest", &reference.digest)?; + Ok(()) +} + pub(crate) fn validate_profile_selector( selector: ProfileSelector, bundle_id: Option<&str>, @@ -263,6 +269,117 @@ fn validate_non_empty(label: &'static str, value: &str) -> Result<(), RuntimeErr } } +fn validate_config_bundle_id(value: &str) -> Result<(), RuntimeError> { + validate_non_empty("config bundle id", value)?; + let trimmed = value.trim(); + if trimmed.len() > 128 { + return Err(RuntimeError::InvalidRequest( + "config bundle id is too large".to_string(), + )); + } + if trimmed != value { + return Err(RuntimeError::InvalidRequest( + "config bundle id must not contain surrounding whitespace".to_string(), + )); + } + if !trimmed + .bytes() + .next() + .is_some_and(|byte| byte.is_ascii_alphanumeric()) + || !trimmed + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b':')) + { + return Err(RuntimeError::InvalidRequest( + "config bundle id must be a path-safe stable identifier".to_string(), + )); + } + Ok(()) +} + +fn validate_digest(label: &'static str, value: &str) -> Result<(), RuntimeError> { + let trimmed = value.trim(); + if trimmed != value + || trimmed.len() != 64 + || !trimmed.bytes().all(|byte| byte.is_ascii_hexdigit()) + { + return Err(RuntimeError::InvalidRequest(format!( + "{label} must be a 64-character lowercase sha256 hex digest" + ))); + } + if !trimmed + .bytes() + .all(|byte| byte.is_ascii_digit() || matches!(byte, b'a'..=b'f')) + { + return Err(RuntimeError::InvalidRequest(format!( + "{label} must be a 64-character lowercase sha256 hex digest" + ))); + } + Ok(()) +} + +fn validate_declaration_reference( + bundle_id: &str, + declaration: &ConfigDeclaration, +) -> Result<(), RuntimeError> { + validate_non_empty("config declaration reference", &declaration.reference)?; + validate_ref_boundary_text("config declaration reference", &declaration.reference)?; + let allowed_prefixes: &[&str] = match declaration.kind { + ConfigDeclarationKind::SecretRef => &["secret:", "secret-ref:", "vault:", "keyring:"], + ConfigDeclarationKind::MountGrant => &["mount:", "mount-grant:"], + ConfigDeclarationKind::NetworkPolicy => &["network:", "network-policy:"], + ConfigDeclarationKind::ShellPolicy => &["shell:", "shell-policy:"], + ConfigDeclarationKind::GitPolicy => &["git:", "git-policy:"], + ConfigDeclarationKind::CapabilityGrant => &["capability:", "capability-grant:"], + ConfigDeclarationKind::Unsupported => &[], + }; + if !allowed_prefixes.iter().any(|prefix| { + declaration.reference.starts_with(prefix) && declaration.reference.len() > prefix.len() + }) { + return Err(RuntimeError::UnsupportedConfigDeclaration { + bundle_id: bundle_id.to_string(), + declaration_kind: declaration.kind.canonical_name().to_string(), + name: declaration.name.clone(), + }); + } + Ok(()) +} + +fn validate_ref_boundary_text(label: &'static str, value: &str) -> Result<(), RuntimeError> { + let trimmed = value.trim(); + validate_boundary_text(label, trimmed)?; + if trimmed != value + || trimmed.contains('/') + || trimmed.contains('\\') + || trimmed.contains('?') + || trimmed.contains('&') + || trimmed.contains('#') + || trimmed.contains('%') + || trimmed.contains('=') + || trimmed.chars().any(char::is_whitespace) + || !trimmed.bytes().all(|byte| { + byte.is_ascii_alphanumeric() || matches!(byte, b':' | b'-' | b'_' | b'.' | b'@' | b'+') + }) + { + return Err(RuntimeError::InvalidRequest(format!( + "{label} must be a typed ref/grant/policy token, not a secret value or path" + ))); + } + let lower = trimmed.to_ascii_lowercase(); + if lower.contains(".cache") + || lower.contains(".yoi") + || lower.contains(".sock") + || lower.contains("socket=") + || lower.contains("session_path") + || lower.contains("cache_path") + { + return Err(RuntimeError::InvalidRequest(format!( + "{label} must not contain host-local cache/session/socket material" + ))); + } + Ok(()) +} + fn validate_boundary_text(label: &'static str, value: &str) -> Result<(), RuntimeError> { let trimmed = value.trim(); if trimmed.len() > 2048 { @@ -275,16 +392,23 @@ fn validate_boundary_text(label: &'static str, value: &str) -> Result<(), Runtim "{label} must not contain control characters" ))); } - if Path::new(trimmed).is_absolute() + let lower = trimmed.to_ascii_lowercase(); + if trimmed.starts_with('/') || trimmed.starts_with('~') - || trimmed.contains("/.cache") - || trimmed.contains("\\.cache") - || trimmed.contains("/run/") - || trimmed.contains("\\run\\") - || trimmed.contains(".sock") - || trimmed.contains("socket=") - || trimmed.contains("session_path") - || trimmed.contains("cache_path") + || trimmed.contains(":\\") + || lower.contains(".cache") + || lower.contains(".yoi/sessions") + || lower.contains(".yoi\\sessions") + || lower.contains("/sessions/") + || lower.contains("\\sessions\\") + || lower.contains("/run/") + || lower.contains("\\run\\") + || lower.contains(".sock") + || lower.contains("/sock") + || lower.contains("\\sock") + || lower.contains("socket=") + || lower.contains("session_path") + || lower.contains("cache_path") { return Err(RuntimeError::InvalidRequest(format!( "{label} must be a stable ref/grant/policy declaration, not a host-local path" @@ -317,3 +441,79 @@ fn hex_digest(bytes: &[u8]) -> String { } out } + +#[cfg(test)] +mod tests { + use super::*; + + fn bundle_with_declaration(reference: &str) -> ConfigBundle { + ConfigBundle { + metadata: ConfigBundleMetadata { + id: "bundle-1".to_string(), + digest: String::new(), + revision: "rev-1".to_string(), + workspace_id: "workspace-1".to_string(), + created_at: "2026-06-26T00:00:00Z".to_string(), + provenance: ConfigBundleProvenance { + source: "test".to_string(), + detail: None, + }, + }, + profiles: vec![ConfigProfileDescriptor { + selector: ProfileSelector::Builtin("builtin:coder".to_string()), + label: None, + }], + declarations: vec![ConfigDeclaration { + kind: ConfigDeclarationKind::SecretRef, + name: "credential".to_string(), + reference: reference.to_string(), + }], + } + .with_computed_digest() + } + + #[test] + fn rejects_host_local_cache_session_socket_and_plaintext_secret_refs() { + for reference in [ + ".cache/yoi", + ".yoi/sessions/foo.jsonl", + "pods/foo/sock", + "password=hunter2", + "hunter2-secret-value", + ] { + let error = validate_config_bundle(&bundle_with_declaration(reference)).unwrap_err(); + assert!( + matches!( + error, + RuntimeError::InvalidRequest(_) + | RuntimeError::UnsupportedConfigDeclaration { .. } + ), + "unexpected error for {reference}: {error:?}" + ); + } + } + + #[test] + fn accepts_typed_secret_refs() { + validate_config_bundle(&bundle_with_declaration("secret:github-token")).unwrap(); + validate_config_bundle(&bundle_with_declaration("vault:team.api-key")).unwrap(); + } + + #[test] + fn rejects_unsafe_bundle_ids_and_refs() { + for id in ["bundle/1", "bundle?x", "bundle&x", "bundle#x", " bundle"] { + let mut bundle = bundle_with_declaration("secret:github-token"); + bundle.metadata.id = id.to_string(); + bundle = bundle.with_computed_digest(); + assert!(validate_config_bundle(&bundle).is_err(), "accepted id {id}"); + } + + assert!( + validate_config_bundle_ref(&ConfigBundleRef { + id: "bundle/1".to_string(), + digest: "0".repeat(64), + }) + .is_err() + ); + } +} diff --git a/crates/worker-runtime/src/runtime.rs b/crates/worker-runtime/src/runtime.rs index 7fa559ea..1cec1f33 100644 --- a/crates/worker-runtime/src/runtime.rs +++ b/crates/worker-runtime/src/runtime.rs @@ -4,7 +4,7 @@ use crate::catalog::{ }; use crate::config_bundle::{ ConfigBundle, ConfigBundleAvailability, ConfigBundleSummary, validate_config_bundle, - validate_profile_selector, + validate_config_bundle_ref, validate_profile_selector, }; use crate::diagnostics::{DiagnosticSeverity, RuntimeDiagnostic}; use crate::error::RuntimeError; @@ -864,11 +864,7 @@ impl RuntimeState { &self, reference: &ConfigBundleRef, ) -> Result { - if reference.id.trim().is_empty() || reference.digest.trim().is_empty() { - return Err(RuntimeError::InvalidRequest( - "config bundle reference id and digest must not be empty".to_string(), - )); - } + validate_config_bundle_ref(reference)?; let bundle = self.config_bundles.get(&reference.id).ok_or_else(|| { RuntimeError::ConfigBundleMissing { bundle_id: reference.id.clone(), @@ -1274,7 +1270,7 @@ mod tests { let mismatch = runtime .check_config_bundle(&ConfigBundleRef { id: bundle.metadata.id.clone(), - digest: "bad-digest".to_string(), + digest: "0".repeat(64), }) .unwrap_err(); assert!(matches!( diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index 4094be3a..bfa68575 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -1451,6 +1451,14 @@ impl RemoteWorkerRuntime { format!("{}{}", self.base_url, path) } + fn bundle_availability_path(reference: &ConfigBundleRef) -> String { + format!( + "/v1/config-bundles/{}/availability?digest={}", + url_path_segment_encode(&reference.id), + url_query_value_encode(&reference.digest) + ) + } + fn ws_endpoint(&self, worker_id: &str) -> String { let mut base = self.base_url.clone(); if let Some(rest) = base.strip_prefix("https://") { @@ -1782,10 +1790,7 @@ impl WorkspaceWorkerRuntime for RemoteWorkerRuntime { } fn check_config_bundle(&self, reference: ConfigBundleRef) -> ConfigBundleCheckResult { - let path = format!( - "/v1/config-bundles/{}/availability?digest={}", - reference.id, reference.digest - ); + let path = Self::bundle_availability_path(&reference); match self.get_json::(&path) { Ok(response) => ConfigBundleCheckResult { state: WorkerOperationState::Accepted, @@ -2428,6 +2433,30 @@ fn host_id_for_remote_runtime(runtime_id: &str) -> String { bounded_backend_identifier("remote-", runtime_id) } +fn url_path_segment_encode(input: &str) -> String { + percent_encode(input, |byte| { + byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b'~' | b':') + }) +} + +fn url_query_value_encode(input: &str) -> String { + percent_encode(input, |byte| { + byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b'~') + }) +} + +fn percent_encode(input: &str, keep: impl Fn(u8) -> bool) -> String { + let mut encoded = String::with_capacity(input.len()); + for byte in input.bytes() { + if keep(byte) { + encoded.push(byte as char); + } else { + encoded.push_str(&format!("%{byte:02X}")); + } + } + encoded +} + fn remote_runtime_capabilities(limit: usize, available: bool) -> RuntimeCapabilitySummary { RuntimeCapabilitySummary { can_list_hosts: true, @@ -2483,10 +2512,6 @@ fn remote_http_status_diagnostic( .as_ref() .map(|error| error.error.code.as_str()) .unwrap_or("remote_http_error"); - let remote_message = error - .as_ref() - .map(|error| error.error.message.clone()) - .unwrap_or_else(|| format!("remote Runtime returned HTTP {status}")); let (code, severity) = match status { StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => { ("remote_runtime_auth_failed", DiagnosticSeverity::Error) @@ -2501,7 +2526,9 @@ fn remote_http_status_diagnostic( diagnostic( code, severity, - format!("Remote Runtime '{runtime_id}' rejected request ({remote_code}): {remote_message}"), + format!( + "Remote Runtime '{runtime_id}' rejected request ({remote_code}, HTTP {status}); internal details were sanitized" + ), ) } @@ -3415,6 +3442,76 @@ mod tests { assert!(browser_payload.contains("worker_id")); } + #[test] + fn remote_config_bundle_sync_and_check_diagnostics_are_sanitized_and_path_safe() { + let leaked_store_path = "/var/lib/yoi/runtime/bundles/bundle-1.json"; + let leaked_session_path = ".yoi/sessions/session.jsonl"; + let digest = "0".repeat(64); + let (base_url, server) = serve_mock_http(vec![ + mock_response( + "POST", + "/v1/config-bundles", + true, + 500, + json!({ + "error": { + "code": "store_io", + "message": format!("failed to write {leaked_store_path}") + } + }) + .to_string(), + ), + mock_response( + "GET", + "/v1/config-bundles/bundle%2F1%3Fx/availability?digest=0000000000000000000000000000000000000000000000000000000000000000", + true, + 400, + json!({ + "error": { + "code": "invalid_request", + "message": format!("invalid path {leaked_session_path}") + } + }) + .to_string(), + ), + ]); + let mut registry = RuntimeRegistry::new(Vec::new()); + registry.register( + RemoteWorkerRuntime::new(RemoteRuntimeConfig::new( + "remote:primary", + "Remote Primary", + base_url, + Some("secret-token".to_string()), + )) + .unwrap(), + ); + + let sync = registry + .sync_config_bundle("remote:primary", test_config_bundle()) + .unwrap(); + assert_eq!(sync.state, WorkerOperationState::Rejected); + let sync_payload = serde_json::to_string(&sync).unwrap(); + assert!(!sync_payload.contains(leaked_store_path), "{sync_payload}"); + + let check = registry + .check_config_bundle( + "remote:primary", + ConfigBundleRef { + id: "bundle/1?x".to_string(), + digest, + }, + ) + .unwrap(); + assert_eq!(check.state, WorkerOperationState::Rejected); + let check_payload = serde_json::to_string(&check).unwrap(); + assert!( + !check_payload.contains(leaked_session_path), + "{check_payload}" + ); + assert!(!check_payload.contains(".yoi/sessions"), "{check_payload}"); + server.join().expect("mock remote server finished"); + } + #[test] fn remote_runtime_auth_errors_map_to_typed_backend_error() { let (base_url, server) = serve_mock_http(vec![mock_response(