From 721f4bdca7feb72653a740a93a7ae21a68e5b0df Mon Sep 17 00:00:00 2001 From: John Myers Date: Thu, 2 Apr 2026 15:28:26 -0700 Subject: [PATCH 01/10] fix(install): restrict tar extraction to expected binary member Prevents CWE-22 path traversal by extracting only the expected APP_NAME member instead of the full archive contents. Adds --no-same-owner and --no-same-permissions for defense-in-depth. OS-20 --- install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install.sh b/install.sh index 565c48c0..0ad6eee6 100755 --- a/install.sh +++ b/install.sh @@ -277,7 +277,7 @@ main() { # Extract info "extracting..." - tar -xzf "${_tmpdir}/${_filename}" -C "${_tmpdir}" + tar -xzf "${_tmpdir}/${_filename}" -C "${_tmpdir}" --no-same-owner --no-same-permissions "${APP_NAME}" # Install mkdir -p "$_install_dir" 2>/dev/null || true From b8307dc7e3dc5c93a40a94b83b8d4330679a8938 Mon Sep 17 00:00:00 2001 From: John Myers Date: Thu, 2 Apr 2026 15:29:22 -0700 Subject: [PATCH 02/10] fix(deploy): quote registry credentials in YAML heredocs Wraps username/password values with a yaml_quote helper to prevent YAML injection from special characters in registry credentials (CWE-94). Applied to all three heredoc blocks that emit registries.yaml auth. OS-23 --- deploy/docker/cluster-entrypoint.sh | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/deploy/docker/cluster-entrypoint.sh b/deploy/docker/cluster-entrypoint.sh index 367665db..86d61e06 100644 --- a/deploy/docker/cluster-entrypoint.sh +++ b/deploy/docker/cluster-entrypoint.sh @@ -25,6 +25,15 @@ set -e +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- +# Escape a value for safe embedding as a YAML single-quoted scalar. +# Single quotes are the only character that needs escaping (' -> ''). +yaml_quote() { + printf "'%s'" "$(printf '%s' "$1" | sed "s/'/''/g")" +} + # --------------------------------------------------------------------------- # Select iptables backend # --------------------------------------------------------------------------- @@ -269,8 +278,8 @@ REGEOF configs: "${REGISTRY_HOST}": auth: - username: ${REGISTRY_USERNAME} - password: ${REGISTRY_PASSWORD} + username: $(yaml_quote "${REGISTRY_USERNAME}") + password: $(yaml_quote "${REGISTRY_PASSWORD}") REGEOF fi @@ -284,8 +293,8 @@ REGEOF cat >> "$REGISTRIES_YAML" <> "$REGISTRIES_YAML" < Date: Thu, 2 Apr 2026 15:30:01 -0700 Subject: [PATCH 03/10] fix(server): redact session token in SSH tunnel rate-limit log Logs only the last 4 characters of bearer tokens to prevent credential exposure in log aggregation systems (CWE-532). OS-18 --- crates/openshell-server/src/ssh_tunnel.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/ssh_tunnel.rs b/crates/openshell-server/src/ssh_tunnel.rs index 5dbff9b5..899f413b 100644 --- a/crates/openshell-server/src/ssh_tunnel.rs +++ b/crates/openshell-server/src/ssh_tunnel.rs @@ -28,6 +28,15 @@ const PREFACE_MAGIC: &str = "NSSH1"; /// Maximum concurrent SSH tunnel connections per session token. const MAX_CONNECTIONS_PER_TOKEN: u32 = 3; +/// Redact a bearer token for safe logging — show only the last 4 characters. +fn redact_token(token: &str) -> String { + if token.len() <= 4 { + "****".to_string() + } else { + format!("****{}", &token[token.len() - 4..]) + } +} + /// Maximum concurrent SSH tunnel connections per sandbox. const MAX_CONNECTIONS_PER_SANDBOX: u32 = 20; @@ -116,7 +125,7 @@ async fn ssh_connect( let mut counts = state.ssh_connections_by_token.lock().unwrap(); let count = counts.entry(token.clone()).or_insert(0); if *count >= MAX_CONNECTIONS_PER_TOKEN { - warn!(token = %token, "SSH tunnel: per-token connection limit reached"); + warn!(token = %redact_token(&token), "SSH tunnel: per-token connection limit reached"); return StatusCode::TOO_MANY_REQUESTS.into_response(); } *count += 1; From 2164677853e5aa0d77578ac1ca99129bcb8bf6c7 Mon Sep 17 00:00:00 2001 From: John Myers Date: Thu, 2 Apr 2026 15:30:20 -0700 Subject: [PATCH 04/10] fix(server): escape gateway_display in auth connect page Applies html_escape() to the Host/X-Forwarded-Host header value before rendering it into the HTML template, preventing HTML injection (CWE-79). OS-17 --- crates/openshell-server/src/auth.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/auth.rs b/crates/openshell-server/src/auth.rs index 5a3229ff..3e7e8afd 100644 --- a/crates/openshell-server/src/auth.rs +++ b/crates/openshell-server/src/auth.rs @@ -68,9 +68,11 @@ async fn auth_connect( .and_then(|v| v.to_str().ok()) .map_or_else(|| state.config.bind_address.to_string(), String::from); + let safe_gateway = html_escape(&gateway_display); + match cf_token { Some(token) => Html(render_connect_page( - &gateway_display, + &safe_gateway, params.callback_port, &token, ¶ms.code, From 3c0d7e8d11ed0e7af4a1afc96b92769f0e34ad19 Mon Sep 17 00:00:00 2001 From: John Myers Date: Thu, 2 Apr 2026 15:34:59 -0700 Subject: [PATCH 05/10] fix(server): prevent XSS via code param with validation and proper JS escaping Adds server-side validation rejecting confirmation codes that do not match the CLI-generated format, replaces manual JS string escaping with serde_json serialization (handling U+2028/U+2029 line terminators), and adds a Content-Security-Policy header with nonce-based script-src. OS-16 --- crates/openshell-server/src/auth.rs | 138 +++++++++++++++++++++------- 1 file changed, 106 insertions(+), 32 deletions(-) diff --git a/crates/openshell-server/src/auth.rs b/crates/openshell-server/src/auth.rs index 3e7e8afd..9fe7e9bb 100644 --- a/crates/openshell-server/src/auth.rs +++ b/crates/openshell-server/src/auth.rs @@ -22,11 +22,28 @@ use axum::{ response::{Html, IntoResponse}, routing::get, }; +use http::header; use serde::Deserialize; use std::sync::Arc; use crate::ServerState; +/// Validate that a confirmation code matches the CLI-generated format. +/// +/// Codes are 3 alphanumeric characters, a dash, then 4 alphanumeric characters +/// (e.g., "AB7-X9KM"). The CLI generates these from the charset `[A-Z2-9]`. +fn is_valid_code(code: &str) -> bool { + let bytes = code.as_bytes(); + bytes.len() == 8 + && bytes[3] == b'-' + && bytes[..3] + .iter() + .all(|b| b.is_ascii_uppercase() || b.is_ascii_digit()) + && bytes[4..] + .iter() + .all(|b| b.is_ascii_uppercase() || b.is_ascii_digit()) +} + #[derive(Deserialize)] struct ConnectParams { callback_port: u16, @@ -54,6 +71,15 @@ async fn auth_connect( Query(params): Query, headers: HeaderMap, ) -> impl IntoResponse { + // Reject codes that don't match the CLI-generated format to prevent + // reflected XSS via crafted URLs. + if !is_valid_code(¶ms.code) { + return Html( + "

Invalid confirmation code format.

".to_string(), + ) + .into_response(); + } + let cf_token = headers .get("cookie") .and_then(|v| v.to_str().ok()) @@ -71,13 +97,32 @@ async fn auth_connect( let safe_gateway = html_escape(&gateway_display); match cf_token { - Some(token) => Html(render_connect_page( - &safe_gateway, - params.callback_port, - &token, - ¶ms.code, - )), - None => Html(render_waiting_page(params.callback_port, ¶ms.code)), + Some(token) => { + let nonce = uuid::Uuid::new_v4().to_string(); + let csp = format!( + "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src http://127.0.0.1:*" + ); + ( + [(header::CONTENT_SECURITY_POLICY, csp)], + Html(render_connect_page( + &safe_gateway, + params.callback_port, + &token, + ¶ms.code, + &nonce, + )), + ) + .into_response() + } + None => { + let csp = + "default-src 'none'; style-src 'unsafe-inline'".to_string(); + ( + [(header::CONTENT_SECURITY_POLICY, csp)], + Html(render_waiting_page(params.callback_port, ¶ms.code)), + ) + .into_response() + } } } @@ -106,22 +151,27 @@ fn render_connect_page( callback_port: u16, cf_token: &str, code: &str, + nonce: &str, ) -> String { - // Escape the token for safe embedding in a JS string literal. - let escaped_token = cf_token - .replace('\\', "\\\\") - .replace('\'', "\\'") - .replace('"', "\\\"") - .replace('<', "\\x3c") - .replace('>', "\\x3e"); + // Use JSON serialization for JS-safe string embedding — handles all + // edge cases including \n, \r, U+2028, U+2029 that break JS string + // literals. serde_json::to_string produces a quoted JSON string + // (e.g., "value") which is a valid JS string literal. + // + // We additionally escape < and > to \u003c / \u003e because while + // they're valid in JSON, they're dangerous inside an HTML before the JS parser runs). + let json_token = serde_json::to_string(cf_token) + .unwrap_or_else(|_| "\"\"".to_string()) + .replace('<', "\\u003c") + .replace('>', "\\u003e"); + let json_code = serde_json::to_string(code) + .unwrap_or_else(|_| "\"\"".to_string()) + .replace('<', "\\u003c") + .replace('>', "\\u003e"); - // Escape the code the same way (it's alphanumeric + dash, but be safe). - let escaped_code = code - .replace('\\', "\\\\") - .replace('\'', "\\'") - .replace('"', "\\\"") - .replace('<', "\\x3c") - .replace('>', "\\x3e"); + // HTML-safe version of the code for display in the page body. + let html_code = html_escape(code); let version = openshell_core::VERSION; @@ -252,7 +302,7 @@ fn render_connect_page(
Connect to Gateway
Confirmation Code
-
{escaped_code}
+
{html_code}
Verify this matches the code shown in your terminal
@@ -273,9 +323,9 @@ fn render_connect_page(
- ", "ABC-1234"); - // < and > should be escaped + let html = render_connect_page( + "gw", + 1234, + "token", + "ABC-1234", + "nonce", + ); + // < and > should be escaped via JSON encoding (\u003c) assert!(!html.contains("