fix: harden runtime config bundle boundary
This commit is contained in:
parent
abab1af2f0
commit
4867ab21bf
|
|
@ -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()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ConfigBundleAvailability, RuntimeError> {
|
||||
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!(
|
||||
|
|
|
|||
|
|
@ -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::<RuntimeHttpConfigBundleAvailabilityResponse>(&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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user