From e2bd78f21874251e4051390bb72b119e70e51c04 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 21 Jul 2026 23:19:13 -0500 Subject: [PATCH 1/3] fix(server): reject non-ASCII characters in Kubernetes label validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate_label_key and validate_label_value used Unicode-aware char::is_alphanumeric(), so values like 'café' or '日本語' passed gateway validation even though the Kubernetes label spec (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? is ASCII-only and the API server rejects such labels later. The same functions already validate the key prefix with ASCII-only checks. Use is_ascii_alphanumeric() and add regression tests for Unicode keys and values. Signed-off-by: Andrew White --- .../openshell-server/src/grpc/validation.rs | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 2f0ad8d139..10ce121bfe 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -494,7 +494,7 @@ pub(super) fn validate_label_key(key: &str) -> Result<(), Status> { // Name must contain only alphanumeric, hyphens, underscores, and dots if !name .chars() - .all(|c| c.is_alphanumeric() || c == '-' || c == '_' || c == '.') + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') { return Err(Status::invalid_argument(format!( "label key name segment contains invalid characters (must be alphanumeric, '-', '_', or '.'): '{key}'" @@ -504,12 +504,12 @@ pub(super) fn validate_label_key(key: &str) -> Result<(), Status> { // Name must start and end with alphanumeric let first = name.chars().next().unwrap(); // safe: we checked !is_empty() let last = name.chars().last().unwrap(); - if !first.is_alphanumeric() { + if !first.is_ascii_alphanumeric() { return Err(Status::invalid_argument(format!( "label key name segment must start with alphanumeric character: '{key}'" ))); } - if !last.is_alphanumeric() { + if !last.is_ascii_alphanumeric() { return Err(Status::invalid_argument(format!( "label key name segment must end with alphanumeric character: '{key}'" ))); @@ -585,7 +585,7 @@ pub(super) fn validate_label_value(value: &str) -> Result<(), Status> { // Must contain only alphanumeric, hyphens, underscores, and dots if !value .chars() - .all(|c| c.is_alphanumeric() || c == '-' || c == '_' || c == '.') + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') { return Err(Status::invalid_argument(format!( "label value contains invalid characters (must be alphanumeric, '-', '_', or '.'): '{value}'" @@ -595,12 +595,12 @@ pub(super) fn validate_label_value(value: &str) -> Result<(), Status> { // Must start and end with alphanumeric let first = value.chars().next().unwrap(); // safe: we checked !is_empty() let last = value.chars().last().unwrap(); - if !first.is_alphanumeric() { + if !first.is_ascii_alphanumeric() { return Err(Status::invalid_argument(format!( "label value must start with alphanumeric character: '{value}'" ))); } - if !last.is_alphanumeric() { + if !last.is_ascii_alphanumeric() { return Err(Status::invalid_argument(format!( "label value must end with alphanumeric character: '{value}'" ))); @@ -1480,6 +1480,17 @@ mod tests { assert!(err.message().contains("invalid characters")); } + #[test] + fn validate_label_key_rejects_unicode_characters() { + // The Kubernetes label spec is ASCII-only; Unicode alphanumerics + // must not pass gateway validation. + let err = validate_label_key("日本語").unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + + let err = validate_label_key("café").unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + } + // ---- Label value validation ---- #[test] @@ -1547,6 +1558,14 @@ mod tests { assert!(err.message().contains("invalid characters")); } + #[test] + fn validate_label_value_rejects_unicode_characters() { + // The Kubernetes label spec is ASCII-only; Unicode alphanumerics + // must not pass gateway validation. + let err = validate_label_value("café").unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + } + // ---- Label selector validation ---- #[test] From afa5d1305835a72c551ffd877c50146e1167b6a1 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 21 Jul 2026 23:19:13 -0500 Subject: [PATCH 2/3] fix(cli): guard server-supplied string truncation against panics Two display paths in sandbox policy commands sliced server-supplied strings without guarding: - sandbox_policy_set used &resp.policy_hash[..12] unconditionally; an empty or short proto3 hash field would panic. Use the existing short_hash() helper, as nearby call sites already do. - The policy revision table truncated load_error at a fixed byte index (&rev.load_error[..40]), panicking on multi-byte UTF-8 in server error messages. Back off to a char boundary instead. Signed-off-by: Andrew White --- crates/openshell-cli/src/run.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index a2ad725a4c..aaef4f9240 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -6930,7 +6930,7 @@ pub async fn sandbox_policy_set( "{} Policy unchanged (version {}, hash: {})", "·".dimmed(), resp.version, - &resp.policy_hash[..12] + short_hash(&resp.policy_hash) ); return Ok(()); } @@ -6939,7 +6939,7 @@ pub async fn sandbox_policy_set( "{} Policy version {} submitted (hash: {})", "✓".green().bold(), resp.version, - &resp.policy_hash[..12] + short_hash(&resp.policy_hash) ); if !wait { @@ -7638,7 +7638,13 @@ fn print_policy_revision_table(revisions: &[openshell_core::proto::SandboxPolicy &rev.policy_hash }; let error_short = if rev.load_error.len() > 40 { - format!("{}...", &rev.load_error[..40]) + // Back off to a char boundary: byte-index slicing panics on + // multi-byte UTF-8 in server-supplied error messages. + let mut end = 40; + while !rev.load_error.is_char_boundary(end) { + end -= 1; + } + format!("{}...", &rev.load_error[..end]) } else { rev.load_error.clone() }; From 949b5b6be5d0634296691cf826d0c81e4f48c7f1 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 21 Jul 2026 23:19:13 -0500 Subject: [PATCH 3/3] fix(sandbox): include port 49152 in ephemeral range note The IANA dynamic/private (ephemeral) port range is 49152-65535 inclusive, but the check used port > 49152, silently omitting the advisory note for port 49152 itself. Signed-off-by: Andrew White --- crates/openshell-sandbox/src/mechanistic_mapper.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/openshell-sandbox/src/mechanistic_mapper.rs b/crates/openshell-sandbox/src/mechanistic_mapper.rs index 8ee2fc37f9..f49004d895 100644 --- a/crates/openshell-sandbox/src/mechanistic_mapper.rs +++ b/crates/openshell-sandbox/src/mechanistic_mapper.rs @@ -308,7 +308,8 @@ fn generate_security_notes(host: &str, port: u16, is_ssrf: bool) -> String { } // High port numbers may indicate ephemeral services. - if port > 49152 { + // The IANA dynamic/private (ephemeral) range is 49152-65535 inclusive. + if port >= 49152 { notes.push(format!( "Port {port} is in the ephemeral range — \ this may be a temporary service."