Skip to content

Commit eea495e

Browse files
authored
fix: remediate 9 security findings from external audit (OS-15 through OS-23) (#744)
* 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 * 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 * 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 * 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 * 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 * fix(sandbox): add byte cap and idle timeout to streaming inference relay Prevents resource exhaustion from upstream inference endpoints that stream indefinitely or hold connections open. Adds a 32 MiB total body limit and 30-second per-chunk idle timeout (CWE-400). OS-21 * fix(policy): narrow port field from u32 to u16 to reject invalid values Prevents meaningless port values >65535 from being accepted in policy YAML definitions. The proto field remains uint32 (protobuf has no u16) with validation at the conversion boundary. OS-22 * fix(deps): migrate from archived serde_yaml to serde_yml Replaces serde_yaml 0.9 (archived, RUSTSEC-2024-0320) with serde_yml 0.0.12, a maintained API-compatible fork. All import sites updated across openshell-policy, openshell-sandbox, and openshell-router. OS-19 * fix(server): re-validate sandbox-submitted security_notes and cap hit_count The gateway now re-runs security heuristics on proposed policy chunks instead of trusting sandbox-provided security_notes, validates host wildcards, caps hit_count at 100, and clamps confidence to [0,1]. The TUI approve-all path is updated to use ApproveAllDraftChunks RPC which respects the security_notes filtering gate (CWE-284, confused deputy). OS-15 * chore: apply cargo fmt and update Cargo.lock for serde_yml --------- Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
1 parent 77e55ea commit eea495e

File tree

15 files changed

+326
-115
lines changed

15 files changed

+326
-115
lines changed

Cargo.lock

Lines changed: 35 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ nix = { version = "0.29", features = ["signal", "process", "user", "fs", "term"]
6464
# Serialization
6565
serde = { version = "1", features = ["derive"] }
6666
serde_json = "1"
67-
serde_yaml = "0.9"
67+
serde_yml = "0.0.12"
6868

6969
# HTTP client
7070
reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }

crates/openshell-policy/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ repository.workspace = true
1313
[dependencies]
1414
openshell-core = { path = "../openshell-core" }
1515
serde = { workspace = true }
16-
serde_yaml = { workspace = true }
16+
serde_yml = { workspace = true }
1717
miette = { workspace = true }
1818

1919
[lints]

crates/openshell-policy/src/lib.rs

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,12 @@ struct NetworkEndpointDef {
8282
#[serde(default, skip_serializing_if = "String::is_empty")]
8383
host: String,
8484
/// Single port (backwards compat). Mutually exclusive with `ports`.
85+
/// Uses `u16` to reject invalid values >65535 at parse time.
8586
#[serde(default, skip_serializing_if = "is_zero")]
86-
port: u32,
87+
port: u16,
8788
/// Multiple ports. When non-empty, this endpoint covers all listed ports.
8889
#[serde(default, skip_serializing_if = "Vec::is_empty")]
89-
ports: Vec<u32>,
90+
ports: Vec<u16>,
9091
#[serde(default, skip_serializing_if = "String::is_empty")]
9192
protocol: String,
9293
#[serde(default, skip_serializing_if = "String::is_empty")]
@@ -101,7 +102,7 @@ struct NetworkEndpointDef {
101102
allowed_ips: Vec<String>,
102103
}
103104

104-
fn is_zero(v: &u32) -> bool {
105+
fn is_zero(v: &u16) -> bool {
105106
*v == 0
106107
}
107108

@@ -169,10 +170,10 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy {
169170
.map(|e| {
170171
// Normalize port/ports: ports takes precedence, else
171172
// single port is promoted to ports array.
172-
let normalized_ports = if !e.ports.is_empty() {
173-
e.ports
173+
let normalized_ports: Vec<u32> = if !e.ports.is_empty() {
174+
e.ports.into_iter().map(u32::from).collect()
174175
} else if e.port > 0 {
175-
vec![e.port]
176+
vec![u32::from(e.port)]
176177
} else {
177178
vec![]
178179
};
@@ -285,10 +286,12 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
285286
.map(|e| {
286287
// Use compact form: if ports has exactly 1 element,
287288
// emit port (scalar). If >1, emit ports (array).
289+
// Proto uses u32; YAML uses u16. Clamp at boundary.
290+
let clamp = |v: u32| -> u16 { v.min(65535) as u16 };
288291
let (port, ports) = if e.ports.len() > 1 {
289-
(0, e.ports.clone())
292+
(0, e.ports.iter().map(|&p| clamp(p)).collect())
290293
} else {
291-
(e.ports.first().copied().unwrap_or(e.port), vec![])
294+
(clamp(e.ports.first().copied().unwrap_or(e.port)), vec![])
292295
};
293296
NetworkEndpointDef {
294297
host: e.host.clone(),
@@ -358,7 +361,7 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
358361

359362
/// Parse a sandbox policy from a YAML string.
360363
pub fn parse_sandbox_policy(yaml: &str) -> Result<SandboxPolicy> {
361-
let raw: PolicyFile = serde_yaml::from_str(yaml)
364+
let raw: PolicyFile = serde_yml::from_str(yaml)
362365
.into_diagnostic()
363366
.wrap_err("failed to parse sandbox policy YAML")?;
364367
Ok(to_proto(raw))
@@ -371,7 +374,7 @@ pub fn parse_sandbox_policy(yaml: &str) -> Result<SandboxPolicy> {
371374
/// and is round-trippable through `parse_sandbox_policy`.
372375
pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result<String> {
373376
let yaml_repr = from_proto(policy);
374-
serde_yaml::to_string(&yaml_repr)
377+
serde_yml::to_string(&yaml_repr)
375378
.into_diagnostic()
376379
.wrap_err("failed to serialize policy to YAML")
377380
}
@@ -1207,4 +1210,20 @@ network_policies:
12071210
proto2.network_policies["test"].endpoints[0].host
12081211
);
12091212
}
1213+
1214+
#[test]
1215+
fn rejects_port_above_65535() {
1216+
let yaml = r#"
1217+
version: 1
1218+
network_policies:
1219+
test:
1220+
endpoints:
1221+
- host: example.com
1222+
port: 70000
1223+
"#;
1224+
assert!(
1225+
parse_sandbox_policy(yaml).is_err(),
1226+
"port >65535 should fail to parse"
1227+
);
1228+
}
12101229
}

crates/openshell-router/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ serde_json = { workspace = true }
1919
thiserror = { workspace = true }
2020
tracing = { workspace = true }
2121
tokio = { workspace = true }
22-
serde_yaml = { workspace = true }
22+
serde_yml = { workspace = true }
2323
uuid = { workspace = true }
2424

2525
[dev-dependencies]

crates/openshell-router/src/config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl RouterConfig {
7575
path.display()
7676
))
7777
})?;
78-
let config: Self = serde_yaml::from_str(&content).map_err(|e| {
78+
let config: Self = serde_yml::from_str(&content).map_err(|e| {
7979
RouterError::Internal(format!(
8080
"failed to parse router config {}: {e}",
8181
path.display()

crates/openshell-sandbox/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ ipnet = "2"
6060

6161
# Serialization
6262
serde_json = { workspace = true }
63-
serde_yaml = { workspace = true }
63+
serde_yml = { workspace = true }
6464

6565
# Logging
6666
tracing = { workspace = true }

crates/openshell-sandbox/src/opa.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ fn parse_process_policy(val: &regorus::Value) -> ProcessPolicy {
511511

512512
/// Preprocess YAML policy data: parse, normalize, validate, expand access presets, return JSON.
513513
fn preprocess_yaml_data(yaml_str: &str) -> Result<String> {
514-
let mut data: serde_json::Value = serde_yaml::from_str(yaml_str)
514+
let mut data: serde_json::Value = serde_yml::from_str(yaml_str)
515515
.map_err(|e| miette::miette!("failed to parse YAML data: {e}"))?;
516516

517517
// Normalize port → ports for all endpoints so Rego always sees "ports" array.

crates/openshell-sandbox/src/proxy.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ use tracing::{debug, info, warn};
2323
const MAX_HEADER_BYTES: usize = 8192;
2424
const INFERENCE_LOCAL_HOST: &str = "inference.local";
2525

26+
/// Maximum total bytes for a streaming inference response body (32 MiB).
27+
const MAX_STREAMING_BODY: usize = 32 * 1024 * 1024;
28+
29+
/// Idle timeout per chunk when relaying streaming inference responses.
30+
const CHUNK_IDLE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
31+
2632
/// Result of a proxy CONNECT policy decision.
2733
struct ConnectDecision {
2834
action: NetworkAction,
@@ -1045,18 +1051,35 @@ async fn route_inference_request(
10451051
let header_bytes = format_http_response_header(resp.status, &resp_headers);
10461052
write_all(tls_client, &header_bytes).await?;
10471053

1048-
// Stream body chunks as they arrive from the upstream.
1054+
// Stream body chunks with byte cap and idle timeout.
1055+
let mut total_bytes: usize = 0;
10491056
loop {
1050-
match resp.next_chunk().await {
1051-
Ok(Some(chunk)) => {
1057+
match tokio::time::timeout(CHUNK_IDLE_TIMEOUT, resp.next_chunk()).await {
1058+
Ok(Ok(Some(chunk))) => {
1059+
total_bytes += chunk.len();
1060+
if total_bytes > MAX_STREAMING_BODY {
1061+
warn!(
1062+
total_bytes = total_bytes,
1063+
limit = MAX_STREAMING_BODY,
1064+
"streaming response exceeded byte limit, truncating"
1065+
);
1066+
break;
1067+
}
10521068
let encoded = format_chunk(&chunk);
10531069
write_all(tls_client, &encoded).await?;
10541070
}
1055-
Ok(None) => break,
1056-
Err(e) => {
1071+
Ok(Ok(None)) => break,
1072+
Ok(Err(e)) => {
10571073
warn!(error = %e, "error reading upstream response chunk");
10581074
break;
10591075
}
1076+
Err(_) => {
1077+
warn!(
1078+
idle_timeout_secs = CHUNK_IDLE_TIMEOUT.as_secs(),
1079+
"streaming response chunk idle timeout, closing"
1080+
);
1081+
break;
1082+
}
10601083
}
10611084
}
10621085

0 commit comments

Comments
 (0)