fix: assorted small validation and robustness fixes

crates/openshell-cli/src/commands/common.rs

@@ -174,7 +174,13 @@ pub fn truncate_display(value: &str, max_width: usize) -> String {
 }
 
 pub fn short_hash(hash: &str) -> &str {
- if hash.len() >= 12 { &hash[..12] } else { hash }
+ if hash.chars().count() <= 12 {
+ return hash;
+ }
+ // Slice at the 13th character boundary so multi-byte UTF-8 cannot panic.
+ hash.char_indices()
+ .nth(12)
+ .map_or(hash, |(idx, _)| &hash[..idx])
 }
 
 pub fn non_empty_or<'a>(value: &'a str, fallback: &'a str) -> &'a str {
@@ -975,4 +981,30 @@ mod tests {
  let err = parse_duration_to_ms("\u{20ac}").expect_err("missing number should error");
  assert!(err.to_string().contains("invalid duration"));
  }
+
+ #[test]
+ fn short_hash_returns_first_12_characters() {
+ assert_eq!(short_hash("abcdefghijkl"), "abcdefghijkl");
+ assert_eq!(short_hash("abcdefghijklm"), "abcdefghijkl");
+ }
+
+ #[test]
+ fn short_hash_leaves_short_input_unchanged() {
+ assert_eq!(short_hash("abc"), "abc");
+ assert_eq!(short_hash(""), "");
+ }
+
+ #[test]
+ fn short_hash_handles_multibyte_characters() {
+ // 12 'a' + one 2-byte 'é' is 14 bytes and 13 chars; byte 12 is exactly
+ // the 'é' boundary, so even the old byte-slice would not panic here.
+ assert_eq!(short_hash("aaaaaaaaaaaaé"), "aaaaaaaaaaaa");
+ // 11 'a' + 2-byte 'é' + 'x': a byte-slice at 12 would split 'é' and
+ // panic; slicing at the 13th character boundary keeps 'é' intact.
+ assert_eq!(short_hash("aaaaaaaaaaaéx"), "aaaaaaaaaaaé");
+ // A 12-char hash ending in a multi-byte char is returned unchanged.
+ assert_eq!(short_hash("aaaaaaaaaaaé"), "aaaaaaaaaaaé");
+ // All-multi-byte input still slices on a character boundary.
+ assert_eq!(short_hash("ééééééééééééé"), "éééééééééééé");
+ }
 }

crates/openshell-cli/src/run.rs

@@ -5443,11 +5443,7 @@ pub async fn sandbox_policy_set_global(
  eprintln!(
  "{} Global policy configured (hash: {}, settings revision: {})",
  "✓".green().bold(),
- if response.policy_hash.len() >= 12 {
- &response.policy_hash[..12]
- } else {
- &response.policy_hash
- },
+ short_hash(&response.policy_hash),
  response.settings_revision,
  );
  Ok(())
@@ -5805,7 +5801,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(());
  }
@@ -5814,7 +5810,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 {
@@ -6500,23 +6496,28 @@ pub async fn sandbox_policy_list_global(
  Ok(())
 }
 
+fn truncate_error_message(error: &str, max_bytes: usize) -> String {
+ if error.len() <= max_bytes {
+ return error.to_string();
+ }
+ // Back off to a char boundary: byte-index slicing panics on multi-byte
+ // UTF-8 in server-supplied error messages.
+ let mut end = max_bytes;
+ while !error.is_char_boundary(end) {
+ end -= 1;
+ }
+ format!("{}...", &error[..end])
+}
+
 fn print_policy_revision_table(revisions: &[openshell_core::proto::SandboxPolicyRevision]) {
  println!(
  "{:<8} {:<14} {:<12} {:<24} ERROR",
  "VERSION", "HASH", "STATUS", "CREATED"
  );
  for rev in revisions {
  let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified);
- let hash_short = if rev.policy_hash.len() >= 12 {
- &rev.policy_hash[..12]
- } else {
- &rev.policy_hash
- };
- let error_short = if rev.load_error.len() > 40 {
- format!("{}...", &rev.load_error[..40])
- } else {
- rev.load_error.clone()
- };
+ let hash_short = short_hash(&rev.policy_hash);
+ let error_short = truncate_error_message(&rev.load_error, 40);
  println!(
  "{:<8} {:<14} {:<12} {:<24} {}",
  rev.version,
@@ -6791,7 +6792,7 @@ pub async fn sandbox_draft_approve(
  "{} Chunk approved. Policy version: {}, hash: {}",
  "OK".green().bold(),
  inner.policy_version,
- &inner.policy_hash[..12.min(inner.policy_hash.len())]
+short_hash(&inner.policy_hash)
  );
 
  Ok(())
@@ -8386,4 +8387,28 @@ mod tests {
  assert!(json["revision"].is_null());
  assert!(json["policy"].is_null());
  }
+
+ #[test]
+ fn truncate_error_message_leaves_short_input_unchanged() {
+ assert_eq!(
+ super::truncate_error_message("short error", 40),
+ "short error"
+ );
+ }
+
+ #[test]
+ fn truncate_error_message_backs_off_to_char_boundary() {
+ // 39 'a' + one 2-byte 'é' means byte 40 splits the multibyte char.
+ let error = "a".repeat(39) + "é";
+ let truncated = super::truncate_error_message(&error, 40);
+ assert!(
+ truncated.ends_with("..."),
+ "expected ellipsis, got {truncated:?}"
+ );
+ assert!(
+ !truncated.contains("é"),
+ "expected char boundary backoff, got {truncated:?}"
+ );
+ assert_eq!(truncated.len(), 39 + 3); // 39 ASCII chars + "..."
+ }
 }

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."
@@ -472,6 +473,15 @@ mod tests {
  assert!(notes.contains("SSRF"));
  }
 
+ #[test]
+ fn test_security_notes_ephemeral_range_inclusive_boundary() {
+ // 49151 is just below the IANA ephemeral range; 49152 is the first
+ // ephemeral port and must be flagged.
+ assert!(!generate_security_notes("api.example.com", 49151, false).contains("ephemeral"));
+ assert!(generate_security_notes("api.example.com", 49152, false).contains("ephemeral"));
+ assert!(generate_security_notes("api.example.com", 65535, false).contains("ephemeral"));
+ }
+
  #[test]
  fn test_security_notes_internal_ip_uses_canonical_classifier() {
  // RFC 1918 is 172.16.0.0/12 only: the old starts_with("172.") prefix

crates/openshell-server/src/grpc/policy.rs

@@ -3881,7 +3881,7 @@ fn generate_security_notes(rule: &NetworkPolicyRule) -> String {
  };
  for raw_port in ports {
  let port = u16::try_from(*raw_port).unwrap_or(0);
- if port > 49152 {
+ if port >= 49152 {
  notes.push(format!(
  "Port {port} is in the ephemeral range — this may be a temporary service."
  ));
@@ -4822,6 +4822,36 @@ mod tests {
  assert!(!notes.contains("Port 3306"));
  }
 
+ #[test]
+ fn security_notes_ephemeral_range_boundary_49152_inclusive() {
+ // IANA dynamic/private range is 49152-65535 inclusive.
+ let below = generate_security_notes(&NetworkPolicyRule {
+ endpoints: vec![NetworkEndpoint {
+ host: "api.example.com".to_string(),
+ port: 49151,
+ ..Default::default()
+ }],
+ ..Default::default()
+ });
+ assert!(
+ !below.contains("ephemeral"),
+ "49151 should not be flagged: {below}"
+ );
+
+ let at = generate_security_notes(&NetworkPolicyRule {
+ endpoints: vec![NetworkEndpoint {
+ host: "api.example.com".to_string(),
+ port: 49152,
+ ..Default::default()
+ }],
+ ..Default::default()
+ });
+ assert!(
+ at.contains("Port 49152 is in the ephemeral range"),
+ "49152 should be flagged: {at}"
+ );
+ }
+
  #[test]
  fn sandbox_caller_update_validation_allows_sandbox_policy_sync() {
  let req = UpdateConfigRequest {

crates/openshell-server/src/grpc/validation.rs

@@ -513,7 +513,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}'"
@@ -523,12 +523,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}'"
  )));
@@ -604,7 +604,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}'"
@@ -614,12 +614,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}'"
  )));
@@ -1506,6 +1506,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]
@@ -1573,6 +1584,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]