From 7b26195d4400ce8e32425ec545cce6bbacb9b9f4 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Fri, 6 Nov 2026 07:24:04 -0800 Subject: [PATCH] fix(network): align REST and WebSocket endpoint handling Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --- crates/openshell-policy-schema/src/lib.rs | 2 +- crates/openshell-policy/src/l7_validate.rs | 45 +++++ crates/openshell-policy/src/lib.rs | 17 +- crates/openshell-policy/src/merge.rs | 5 +- crates/openshell-providers/src/profiles.rs | 37 ++++ .../src/l7/mod.rs | 27 ++- .../src/l7/relay.rs | 180 ++++++++++++++++-- .../src/l7/rest.rs | 30 +-- .../openshell-supervisor-network/src/opa.rs | 6 +- docs/how-it-works/policies/network-rules.mdx | 5 + docs/how-it-works/policies/schema.mdx | 4 +- docs/how-it-works/providers/overview.mdx | 3 +- e2e/rust/tests/forward_proxy_l7_bypass.rs | 59 ++++++ skills/generate-sandbox-policy/SKILL.md | 1 + 14 files changed, 374 insertions(+), 47 deletions(-) diff --git a/crates/openshell-policy-schema/src/lib.rs b/crates/openshell-policy-schema/src/lib.rs index 656433b794..422ac13440 100644 --- a/crates/openshell-policy-schema/src/lib.rs +++ b/crates/openshell-policy-schema/src/lib.rs @@ -285,7 +285,7 @@ pub struct NetworkEndpoint { /// Defaults to false (strict). #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub allow_encoded_slash: bool, - /// When true, client-to-server WebSocket text messages on this REST + /// When true, client-to-server WebSocket text messages on a `protocol: websocket` /// endpoint rewrite credential placeholders after an allowed 101 upgrade. /// Defaults to false. #[serde(default, skip_serializing_if = "std::ops::Not::not")] diff --git a/crates/openshell-policy/src/l7_validate.rs b/crates/openshell-policy/src/l7_validate.rs index ac995fa65b..74be95cc82 100644 --- a/crates/openshell-policy/src/l7_validate.rs +++ b/crates/openshell-policy/src/l7_validate.rs @@ -223,6 +223,9 @@ pub struct L7EndpointFields<'a> { /// `true` when the endpoint has a non-empty rules list. pub has_rules: bool, + /// Whether WebSocket credential rewriting was enabled. + pub websocket_credential_rewrite: bool, + /// `true` when the endpoint has a non-empty `deny_rules` list. pub has_deny_rules: bool, @@ -244,6 +247,9 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec let mut errors = Vec::new(); let protocol = ep.protocol; let l7_protocol = L7Protocol::parse(protocol); + if matches!(l7_protocol, Some(L7Protocol::Rest)) && ep.websocket_credential_rewrite { + errors.push("websocket_credential_rewrite is not supported for protocol rest; use protocol: websocket with explicit message rules".to_string()); + } let explicit_tcp = is_explicit_tcp_protocol(protocol); let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family); let is_mcp = matches!(l7_protocol, Some(L7Protocol::Mcp)); @@ -335,6 +341,7 @@ mod tests { fn valid_rest_endpoint() -> L7EndpointFields<'static> { L7EndpointFields { + websocket_credential_rewrite: false, protocol: "rest", access: "read-only", has_rules: false, @@ -344,6 +351,27 @@ mod tests { } } + #[test] + fn rest_rejects_websocket_credential_rewrite_with_migration_hint() { + let mut endpoint = valid_rest_endpoint(); + endpoint.websocket_credential_rewrite = true; + for protocol in ["rest", "REST", "Rest"] { + endpoint.protocol = protocol; + let errors = validate_l7_endpoint_semantics(&endpoint); + assert!( + errors.iter().any(|e| e.contains("use protocol: websocket")), + "{protocol}: {errors:?}" + ); + } + for protocol in ["websocket", "WEBSOCKET", "WebSocket"] { + endpoint.protocol = protocol; + assert!( + validate_l7_endpoint_semantics(&endpoint).is_empty(), + "{protocol}" + ); + } + } + #[test] fn valid_endpoint_produces_no_errors() { let errors = validate_l7_endpoint_semantics(&valid_rest_endpoint()); @@ -383,6 +411,7 @@ mod tests { #[test] fn rejects_unknown_protocol() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "ftp", access: "", has_rules: false, @@ -397,6 +426,7 @@ mod tests { #[test] fn rejects_rules_and_access_together() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "rest", access: "full", has_rules: true, @@ -411,6 +441,7 @@ mod tests { #[test] fn rejects_jsonrpc_with_access_presets() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "json-rpc", access: "full", has_rules: false, @@ -429,6 +460,7 @@ mod tests { #[test] fn rejects_mcp_with_access_presets() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "mcp", access: "full", has_rules: false, @@ -447,6 +479,7 @@ mod tests { #[test] fn rejects_jsonrpc_without_rules() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "json-rpc", access: "", has_rules: false, @@ -461,6 +494,7 @@ mod tests { #[test] fn rejects_protocol_without_rules_or_access() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "rest", access: "", has_rules: false, @@ -479,6 +513,7 @@ mod tests { #[test] fn rejects_mcp_without_rules_when_allow_all_false() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "mcp", access: "", has_rules: false, @@ -493,6 +528,7 @@ mod tests { #[test] fn accepts_mcp_with_allow_all_true() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "mcp", access: "", has_rules: false, @@ -507,6 +543,7 @@ mod tests { #[test] fn rejects_rules_that_deny_all() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "rest", access: "", has_rules: true, @@ -521,6 +558,7 @@ mod tests { #[test] fn mcp_allow_all_with_empty_allow_not_rejected() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "mcp", access: "", has_rules: true, @@ -538,6 +576,7 @@ mod tests { #[test] fn rejects_deny_rules_without_protocol() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "", access: "", has_rules: false, @@ -556,6 +595,7 @@ mod tests { #[test] fn rejects_deny_rules_without_allow_base() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "rest", access: "", has_rules: false, @@ -574,6 +614,7 @@ mod tests { #[test] fn no_protocol_no_errors() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "", access: "", has_rules: false, @@ -588,6 +629,7 @@ mod tests { #[test] fn explicit_tcp_is_valid_without_l7_fields() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "tcp", access: "", has_rules: false, @@ -604,6 +646,7 @@ mod tests { #[test] fn explicit_tcp_rejects_l7_fields() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "tcp", access: "full", has_rules: false, @@ -648,6 +691,7 @@ mod tests { #[test] fn jsonrpc_with_access_emits_single_diagnostic() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "json-rpc", access: "full", has_rules: false, @@ -669,6 +713,7 @@ mod tests { #[test] fn jsonrpc_without_rules_or_access_emits_single_diagnostic() { let ep = L7EndpointFields { + websocket_credential_rewrite: false, protocol: "json-rpc", access: "", has_rules: false, diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index bad8532cdf..a84bca7868 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -1562,6 +1562,7 @@ fn validate_sandbox_policy_with_mcp_presence( }) }); let fields = L7EndpointFields { + websocket_credential_rewrite: ep.websocket_credential_rewrite, protocol: &ep.protocol, access, has_rules: !ep.rules.is_empty(), @@ -4786,6 +4787,18 @@ network_policies: ); } + #[test] + fn validate_rest_rejects_websocket_credential_rewrite() { + for protocol in ["rest", "REST", "Rest"] { + let yaml = "version: 1\nnetwork_policies:\n api:\n endpoints:\n - { host: api.example.com, port: 443, protocol: rest, access: read-only, websocket_credential_rewrite: true }\n"; + let yaml = yaml.replace("protocol: rest", &format!("protocol: {protocol}")); + let policy = parse_sandbox_policy(&yaml).unwrap(); + let errors = validate_sandbox_policy(&policy) + .expect_err("REST cannot rewrite WebSocket messages"); + assert!(format!("{errors:?}").contains("use protocol: websocket")); + } + } + #[test] fn round_trip_preserves_websocket_credential_rewrite() { let yaml = r" @@ -4796,7 +4809,7 @@ network_policies: endpoints: - host: gateway.example.com port: 443 - protocol: rest + protocol: websocket enforcement: enforce access: full websocket_credential_rewrite: true @@ -4808,7 +4821,7 @@ network_policies: let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); let ep = &proto2.network_policies["discord_gateway"].endpoints[0]; - assert_eq!(ep.protocol, "rest"); + assert_eq!(ep.protocol, "websocket"); assert!(ep.websocket_credential_rewrite); assert!(yaml_out.contains("websocket_credential_rewrite: true")); } diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 4639af6123..c010e6a723 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -3818,8 +3818,8 @@ mod tests { /// agent is told its approved rule never landed. #[test] fn merged_policy_covers_the_rule_it_merged() { - // Every field `merge_endpoint` widens rather than replaces, set on the - // existing endpoint and absent from the proposal. + // REST-compatible fields that `merge_endpoint` widens rather than + // replaces, set on the existing endpoint and absent from the proposal. let existing_endpoint = NetworkEndpoint { protocol: "rest".to_string(), rules: vec![rest_rule("GET", "/health")], @@ -3830,7 +3830,6 @@ mod tests { }], allowed_ips: vec!["10.0.0.1".to_string()], allow_encoded_slash: true, - websocket_credential_rewrite: true, request_body_credential_rewrite: true, advisor_proposed: true, ..endpoint("api.example.com", 443) diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 3adfc9c278..b56dc2c7e4 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -2759,6 +2759,7 @@ pub fn validate_profile_set( } let l7_fields = L7EndpointFields { + websocket_credential_rewrite: endpoint.websocket_credential_rewrite, protocol: &endpoint.protocol, access: &endpoint.access, has_rules: endpoint.rules.as_ref().is_some_and(|r| !r.is_empty()), @@ -6330,6 +6331,42 @@ binaries: ["", /usr/bin/broken] assert!(messages.contains(&"binary path must not be empty")); } + #[test] + fn validate_profile_rejects_rest_websocket_credential_rewrite() { + for protocol in ["rest", "REST", "Rest"] { + let profile = parse_profile_yaml( + r" +id: rest-rewrite +display_name: REST rewrite +credentials: + - name: api_key + env_vars: [API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: read-only + websocket_credential_rewrite: true +binaries: + - /usr/bin/curl +" + .replace("protocol: rest", &format!("protocol: {protocol}")) + .as_str(), + ) + .unwrap(); + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("use protocol: websocket")) + ); + } + } + #[test] fn validate_profile_set_rejects_noncanonical_profile_ids() { let profiles = [ diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index 37849b9c0c..4737e5c1d7 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -379,6 +379,9 @@ pub fn parse_l7_config(val: ®orus::Value) -> Option { let allow_encoded_slash = get_object_bool(val, "allow_encoded_slash").unwrap_or(false); let websocket_credential_rewrite = get_object_bool(val, "websocket_credential_rewrite").unwrap_or(false); + if protocol == L7Protocol::Rest && websocket_credential_rewrite { + return None; + } let request_body_credential_rewrite = get_object_bool(val, "request_body_credential_rewrite").unwrap_or(false); let allow_uninspected_credentials = @@ -1392,6 +1395,10 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< .and_then(serde_json::Value::as_bool) .unwrap_or(false); let l7_fields = L7EndpointFields { + websocket_credential_rewrite: ep + .get("websocket_credential_rewrite") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), protocol, access, has_rules, @@ -1503,7 +1510,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< && protocol != "websocket" { warnings.push(format!( - "{loc}: websocket_credential_rewrite is ignored unless protocol is rest or websocket" + "{loc}: websocket_credential_rewrite is ignored unless protocol is websocket" )); } @@ -2469,10 +2476,24 @@ mod tests { assert!(!config.websocket_credential_rewrite); } + #[test] + fn rest_websocket_credential_rewrite_fails_validation_and_materialization() { + let endpoint = serde_json::json!({ + "protocol": "rest", "host": "gateway.example.com", "port": 443, + "access": "read-only", "websocket_credential_rewrite": true + }); + let val = regorus::Value::from_json_str(&endpoint.to_string()).unwrap(); + assert!(parse_l7_config(&val).is_none()); + let (errors, _) = validate_l7_policies(&serde_json::json!({ + "network_policies": {"test": {"endpoints": [endpoint], "binaries": []}} + })); + assert!(errors.iter().any(|e| e.contains("use protocol: websocket"))); + } + #[test] fn parse_l7_config_websocket_credential_rewrite_opt_in() { let val = regorus::Value::from_json_str( - r#"{"protocol": "rest", "host": "gateway.example.com", "port": 443, "websocket_credential_rewrite": true}"#, + r#"{"protocol": "websocket", "host": "gateway.example.com", "port": 443, "websocket_credential_rewrite": true}"#, ) .unwrap(); let config = parse_l7_config(&val).unwrap(); @@ -2520,7 +2541,7 @@ mod tests { } #[test] - fn validate_websocket_credential_rewrite_warns_unless_rest_or_websocket() { + fn validate_websocket_credential_rewrite_warns_without_inspected_protocol() { let data = serde_json::json!({ "network_policies": { "test": { diff --git a/crates/openshell-supervisor-network/src/l7/relay.rs b/crates/openshell-supervisor-network/src/l7/relay.rs index b4acb5e52f..352da45ab6 100644 --- a/crates/openshell-supervisor-network/src/l7/relay.rs +++ b/crates/openshell-supervisor-network/src/l7/relay.rs @@ -1985,8 +1985,7 @@ pub(crate) fn upgrade_options<'a>( engine: Option<&'a TunnelPolicyEngine>, ) -> UpgradeRelayOptions<'a> { let websocket_credential_rewrite = - matches!(config.protocol, L7Protocol::Rest | L7Protocol::Websocket) - && config.websocket_credential_rewrite; + config.protocol == L7Protocol::Websocket && config.websocket_credential_rewrite; let deny_uninspected_credentials = config.provider_credentialed && !config.allow_uninspected_credentials; let websocket_message_policy = if config.protocol == L7Protocol::Websocket { @@ -2030,7 +2029,6 @@ pub(crate) fn websocket_extension_mode( ) -> WebSocketExtensionMode { if inspecting_middleware_session || config.protocol == L7Protocol::Websocket - || (config.protocol == L7Protocol::Rest && config.websocket_credential_rewrite) || (config.provider_credentialed && !config.allow_uninspected_credentials) { WebSocketExtensionMode::PermessageDeflate @@ -2358,6 +2356,12 @@ where websocket_permessage_deflate, websocket_subprotocol, } => { + if crate::l7::rest::upgrade_refusal_for_protocol(config.protocol).is_some() { + if let Some(session) = middleware_session.take() { + session.end(openshell_core::proto::MiddlewareSessionEndReason::ProtocolError).await; + } + return Ok(()); + } let mut options = upgrade_options( config, ctx, @@ -5397,13 +5401,16 @@ network_policies: endpoints: - host: api.example.test port: 8080 - protocol: rest + protocol: websocket token_grant_owner: test-owner enforcement: enforce rules: - allow: method: GET path: "/v1/**" + - allow: + method: WEBSOCKET_TEXT + path: "/v1/**" binaries: - { path: /usr/bin/curl } "#; @@ -5459,7 +5466,10 @@ network_policies: ) .await .expect("send upgrade request"); - seen_rx.recv().await.expect("preflight reached middleware"); + tokio::time::timeout(std::time::Duration::from_secs(2), seen_rx.recv()) + .await + .expect("preflight timed out") + .expect("preflight reached middleware"); fixture.assert_no_requests(); release.notify_one(); @@ -10002,7 +10012,7 @@ network_policies: } #[tokio::test] - async fn rest_websocket_middleware_inspects_compressed_wss_messages() { + async fn websocket_middleware_inspects_compressed_wss_messages() { let data = r#" network_middlewares: redact: @@ -10016,12 +10026,15 @@ network_policies: endpoints: - host: api.example.test port: 8080 - protocol: rest + protocol: websocket enforcement: enforce rules: - allow: method: GET path: "/ws" + - allow: + method: WEBSOCKET_TEXT + path: "/ws" binaries: - { path: /usr/bin/node } "#; @@ -10110,7 +10123,7 @@ network_policies: endpoints: - host: gateway.example.test port: 443 - protocol: rest + protocol: websocket enforcement: enforce rules: - allow: @@ -10124,7 +10137,7 @@ network_policies: .clone_engine_for_tunnel(engine.current_generation()) .unwrap(); let configs = vec![L7EndpointConfig { - protocol: L7Protocol::Rest, + protocol: L7Protocol::Websocket, endpoint_id: String::new(), policy_hash: String::new(), path: "/ws".into(), @@ -10929,9 +10942,140 @@ network_policies: } #[tokio::test] - async fn route_selected_rest_websocket_upgrade_still_relays_beside_mcp() { - // The refusal follows the selected endpoint's protocol: a REST - // upgrade on the same host and port keeps its documented raw relay. + async fn rest_upgrade_refusal_emits_ocsf_denial() { + let (configs, _, ctx) = jsonrpc_and_rest_route_configs("mcp", "enforce"); + let config = configs + .iter() + .find(|c| c.protocol == L7Protocol::Rest) + .unwrap(); + let request = crate::l7::provider::L7Request { + action: "GET".into(), + target: "/api/ws".into(), + query_params: std::collections::HashMap::new(), + raw_header: b"GET /api/ws HTTP/1.1\r\nHost: mcp.example.test:8000\r\nUpgrade: websocket\r\nConnection: Upgrade\r\n\r\n".to_vec(), + body_length: crate::l7::provider::BodyLength::None, + }; + let capture = openshell_ocsf::test_support::OcsfCapture::default(); + let _guard = capture.install(); + let (_app, mut client) = tokio::io::duplex(8192); + assert!( + deny_unsupported_upgrade_if_requested(&request, config, &ctx, None, &mut client) + .await + .unwrap() + ); + let events = capture.json_events(); + assert!( + events.iter().any(|event| event["action_id"] == 2 + && event["status_detail"] == crate::l7::rest::UNSUPPORTED_REST_UPGRADE_DETAIL), + "{events:?}" + ); + } + + #[tokio::test] + async fn rest_upgrades_are_refused_in_both_modes_and_relay_paths() { + for enforcement in [EnforcementMode::Enforce, EnforcementMode::Audit] { + for route_selected in [false, true] { + for upgrade in ["websocket", "custom"] { + let (mut configs, engine, ctx) = + jsonrpc_and_rest_route_configs("mcp", "enforce"); + let config = configs + .iter_mut() + .find(|c| c.protocol == L7Protocol::Rest) + .unwrap(); + config.enforcement = enforcement; + let config = config.clone(); + let request = format!( + "GET /api/ws HTTP/1.1\r\nHost: mcp.example.test:8000\r\nConnection: keep-alive, Upgrade\r\nUpgrade: {upgrade}\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\nSec-WebSocket-Version: 13\r\n\r\n" + ); + let frame = br#"{"action":"delete"}"#; + let scenario = run_upgrade_scenario( + request.as_bytes(), + frame, + move |mut client, mut upstream| { + tokio::spawn(async move { + if route_selected { + relay_with_route_selection( + &configs, + engine, + &mut client, + &mut upstream, + &ctx, + ) + .await + } else { + relay_with_inspection( + &config, + engine, + &mut client, + &mut upstream, + &ctx, + ) + .await + } + }) + }, + ) + .await; + assert_upgrade_refused_before_forwarding( + &scenario, + frame, + crate::l7::rest::UNSUPPORTED_REST_UPGRADE_DETAIL, + ); + } + } + } + } + + #[tokio::test] + async fn rest_upgrade_after_ordinary_request_is_refused() { + for route_selected in [false, true] { + let (configs, engine, ctx) = jsonrpc_and_rest_route_configs("mcp", "enforce"); + let config = configs + .iter() + .find(|c| c.protocol == L7Protocol::Rest) + .unwrap() + .clone(); + let (mut app, mut client) = tokio::io::duplex(8192); + let (mut upstream, mut server) = tokio::io::duplex(8192); + let relay = tokio::spawn(async move { + if route_selected { + relay_with_route_selection(&configs, engine, &mut client, &mut upstream, &ctx) + .await + } else { + relay_with_inspection(&config, engine, &mut client, &mut upstream, &ctx).await + } + }); + app.write_all(b"GET /api/tickets HTTP/1.1\r\nHost: mcp.example.test:8000\r\n\r\n") + .await + .unwrap(); + let forwarded = read_http_headers(&mut server).await; + assert!(String::from_utf8_lossy(&forwarded).starts_with("GET /api/tickets ")); + server + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") + .await + .unwrap(); + let response = read_http_headers(&mut app).await; + assert!(String::from_utf8_lossy(&response).starts_with("HTTP/1.1 200")); + app.write_all(b"GET /api/ws HTTP/1.1\r\nHost: mcp.example.test:8000\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\nSec-WebSocket-Version: 13\r\n\r\n").await.unwrap(); + let mut denied = Vec::new(); + tokio::time::timeout( + std::time::Duration::from_secs(2), + app.read_to_end(&mut denied), + ) + .await + .unwrap() + .unwrap(); + assert!(String::from_utf8_lossy(&denied).starts_with("HTTP/1.1 403")); + relay.await.unwrap().unwrap(); + let mut extra = Vec::new(); + server.read_to_end(&mut extra).await.unwrap(); + assert!(extra.is_empty(), "upgrade must not reach the upstream"); + } + } + + #[tokio::test] + async fn route_selected_rest_websocket_upgrade_is_refused_beside_mcp() { + // Both per-request protocols refuse upgrades on their selected path. let (configs, tunnel_engine, ctx) = jsonrpc_and_rest_route_configs("mcp", "enforce"); let frame = br#"{"type":"ping"}"#; let scenario = run_upgrade_scenario( @@ -10951,15 +11095,11 @@ network_policies: }, ) .await; - assert!( - scenario.response.starts_with("HTTP/1.1 101"), - "REST upgrade should still switch protocols, got: {}", - scenario.response + assert_upgrade_refused_before_forwarding( + &scenario, + frame, + crate::l7::rest::UNSUPPORTED_REST_UPGRADE_DETAIL, ); - assert!(contains_bytes( - &scenario.upstream_seen, - &masked_text_frame(frame) - )); } #[tokio::test] diff --git a/crates/openshell-supervisor-network/src/l7/rest.rs b/crates/openshell-supervisor-network/src/l7/rest.rs index 7a6a0d6236..9ff440639c 100644 --- a/crates/openshell-supervisor-network/src/l7/rest.rs +++ b/crates/openshell-supervisor-network/src/l7/rest.rs @@ -88,6 +88,7 @@ pub(crate) const UNSUPPORTED_H2C_UPGRADE_DETAIL: &str = pub(crate) const UNSUPPORTED_JSONRPC_UPGRADE_DETAIL: &str = "HTTP upgrade is not supported for JSON-RPC or MCP endpoints"; pub(crate) const UNSUPPORTED_GRAPHQL_UPGRADE_DETAIL: &str = "HTTP upgrade is not supported for GraphQL endpoints; serve GraphQL over WebSocket from a separate protocol: websocket endpoint on another path or port"; +pub(crate) const UNSUPPORTED_REST_UPGRADE_DETAIL: &str = "HTTP upgrade is not supported for REST endpoints; use a separate protocol: websocket endpoint with explicit message rules"; const MIN_HTTP2_PREFACE_DETECTION_BYTES: usize = 8; /// Idle timeout for `relay_until_eof`. If no data arrives within this window @@ -2441,11 +2442,11 @@ pub(crate) fn unsupported_upgrade_detail( /// individual HTTP requests, or `None` for a protocol that may relay an /// allowed upgrade. /// -/// JSON-RPC, MCP and GraphQL rules inspect each HTTP request body or query. +/// REST, JSON-RPC, MCP and GraphQL rules inspect individual HTTP requests. /// After an upgrade the relay would copy frames that no rule of these /// protocols evaluates, so they never upgrade; GraphQL over WebSocket is /// served by separate `protocol: websocket` endpoints with GraphQL operation -/// rules. REST and WebSocket endpoints relay allowed upgrades, and SQL +/// rules. WebSocket endpoints relay allowed upgrades, and SQL /// endpoints keep their existing upgrade behavior. The match is exhaustive so /// a new protocol must choose. pub(crate) fn upgrade_refusal_for_protocol( @@ -2455,7 +2456,8 @@ pub(crate) fn upgrade_refusal_for_protocol( match protocol { L7Protocol::JsonRpc | L7Protocol::Mcp => Some(UNSUPPORTED_JSONRPC_UPGRADE_DETAIL), L7Protocol::Graphql => Some(UNSUPPORTED_GRAPHQL_UPGRADE_DETAIL), - L7Protocol::Rest | L7Protocol::Websocket | L7Protocol::Sql => None, + L7Protocol::Rest => Some(UNSUPPORTED_REST_UPGRADE_DETAIL), + L7Protocol::Websocket | L7Protocol::Sql => None, } } @@ -9253,16 +9255,10 @@ mod tests { let websocket = format!( "GET /ws HTTP/1.1\r\nHost: example.com\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Key: {VALID_WS_KEY}\r\nSec-WebSocket-Version: 13\r\n\r\n" ); - for protocol in [ - crate::l7::L7Protocol::Rest, - crate::l7::L7Protocol::Websocket, - ] { - assert_eq!( - unsupported_upgrade_detail(websocket.as_bytes(), protocol), - None, - "{protocol:?}" - ); - } + assert_eq!( + unsupported_upgrade_detail(websocket.as_bytes(), crate::l7::L7Protocol::Websocket), + None, + ); // Streamable HTTP requests, a look-alike header name, and a stray // `Connection: upgrade` without an `Upgrade` header stay allowed; @@ -9275,6 +9271,7 @@ mod tests { ]; for raw in ordinary { for protocol in [ + crate::l7::L7Protocol::Rest, crate::l7::L7Protocol::JsonRpc, crate::l7::L7Protocol::Mcp, crate::l7::L7Protocol::Graphql, @@ -9325,7 +9322,11 @@ mod tests { upgrade_refusal_for_protocol(L7Protocol::Graphql), Some(UNSUPPORTED_GRAPHQL_UPGRADE_DETAIL) ); - for protocol in [L7Protocol::Rest, L7Protocol::Websocket, L7Protocol::Sql] { + assert_eq!( + upgrade_refusal_for_protocol(L7Protocol::Rest), + Some(UNSUPPORTED_REST_UPGRADE_DETAIL) + ); + for protocol in [L7Protocol::Websocket, L7Protocol::Sql] { assert_eq!(upgrade_refusal_for_protocol(protocol), None, "{protocol:?}"); } } @@ -9363,6 +9364,7 @@ mod tests { "fixture must be an upgrade to the relay: {raw}" ); for protocol in [ + crate::l7::L7Protocol::Rest, crate::l7::L7Protocol::JsonRpc, crate::l7::L7Protocol::Mcp, crate::l7::L7Protocol::Graphql, diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 2d1a73a949..1a44ff6a28 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -4566,6 +4566,10 @@ process: let mut policy = valid.clone(); policy["version"] = 1.into(); policy["network_policies"]["admin"]["endpoints"][0][field] = control; + if field == "websocket_credential_rewrite" { + policy["network_policies"]["admin"]["endpoints"][0]["protocol"] = + "websocket".into(); + } openshell_policy::parse_sandbox_policy(&policy.to_string()).expect("typed valid twin"); OpaEngine::from_strings(TEST_POLICY, &policy.to_string()).expect("raw valid twin"); policy["network_policies"]["admin"]["endpoints"][0][field] = invalid; @@ -8277,7 +8281,7 @@ network_policies: endpoints: vec![NetworkEndpoint { host: "gateway.example.com".to_string(), port: 443, - protocol: "rest".to_string(), + protocol: "websocket".to_string(), enforcement: openshell_core::proto::NetworkEnforcementMode::Enforce as i32, access: openshell_core::proto::NetworkAccessPreset::Full as i32, websocket_credential_rewrite: true, diff --git a/docs/how-it-works/policies/network-rules.mdx b/docs/how-it-works/policies/network-rules.mdx index 66405e5e5b..d7dec792a6 100644 --- a/docs/how-it-works/policies/network-rules.mdx +++ b/docs/how-it-works/policies/network-rules.mdx @@ -408,6 +408,11 @@ instead of broadening the range when an address changes. ### Allow WebSocket Messages +REST endpoints refuse requests carrying an `Upgrade` header with `403 Forbidden` +in both `enforce` and `audit` mode. To migrate a REST endpoint that previously +relayed WebSocket traffic, configure a separate `protocol: websocket` endpoint +on its own path or port with explicit upgrade and message rules. + Use `protocol: websocket` to control WebSocket connections and the messages that clients send over them. This template requires `/usr/bin/node` and a WebSocket service you control. It permits the `/v1/realtime` upgrade and client diff --git a/docs/how-it-works/policies/schema.mdx b/docs/how-it-works/policies/schema.mdx index 40faf25379..9ab09275d7 100644 --- a/docs/how-it-works/policies/schema.mdx +++ b/docs/how-it-works/policies/schema.mdx @@ -153,7 +153,7 @@ the gateway host. | Field | Type | Default | Description | |---|---|---|---| -| `protocol` | string | None | `rest`, `websocket`, `graphql`, `mcp`, or `json-rpc` for request inspection, or `tcp` for a native TCP connection. `graphql`, `mcp`, and `json-rpc` endpoints refuse requests that carry an `Upgrade` header with `403`. Refer to [Connection and Request Checks](/how-it-works/policies/network-rules#connection-and-request-checks). | +| `protocol` | string | None | `rest`, `websocket`, `graphql`, `mcp`, or `json-rpc` for request inspection, or `tcp` for a native TCP connection. `rest`, `graphql`, `mcp`, and `json-rpc` endpoints refuse requests that carry an `Upgrade` header with `403`. Refer to [Connection and Request Checks](/how-it-works/policies/network-rules#connection-and-request-checks). | | `tls` | string | Automatic | `skip` relays traffic without terminating TLS, so OpenShell cannot inspect it. Do not use it with a request protocol. | | `enforcement` | string | `audit` | `enforce` blocks requests that break the endpoint's rules. `audit` logs them and allows the request. | | `access` | string | None | Access preset: `read-only`, `read-write`, or `full`. Refer to [Access Presets](#access-presets). | @@ -167,7 +167,7 @@ the gateway host. |---|---|---|---| | `credential_binding.provider` | string | None | Binds the static credentials of an attached provider whose profile defines no endpoints. Valid only in a sandbox policy. | | `request_body_credential_rewrite` | bool | `false` | Replaces ordinary static credential placeholders in REST bodies, including those with declared HTTP placement. | -| `websocket_credential_rewrite` | bool | `false` | Replaces ordinary static credential placeholders in client WebSocket text messages, including those with declared HTTP placement. | +| `websocket_credential_rewrite` | bool | `false` | Replaces ordinary static credential placeholders in client WebSocket text messages, including those with declared HTTP placement. Requires `protocol: websocket`. | | `allow_uninspected_credentials` | bool | `false` | Allows a provider-credentialed endpoint to omit request inspection or use `tls: skip`. | | `credential_signing` | string | None | AWS request signing: `sigv4`, `sigv4:body`, or `sigv4:no_body`. | | `signing_service` | string | None | AWS service name, such as `bedrock` or `s3`. Required with `credential_signing`. | diff --git a/docs/how-it-works/providers/overview.mdx b/docs/how-it-works/providers/overview.mdx index a5299802a7..aba6269546 100644 --- a/docs/how-it-works/providers/overview.mdx +++ b/docs/how-it-works/providers/overview.mdx @@ -490,7 +490,8 @@ The proxy does not rewrite cookies, response content, unsupported request bodies, or WebSocket binary frames. Explicit endpoint policies can enable credential rewriting in request bodies -or WebSocket text messages. These flags do not authorize arbitrary query +or WebSocket text messages. WebSocket text message rewriting requires +`protocol: websocket`. These flags do not authorize arbitrary query parameters, path slots, or unrelated headers. See [Profiles](/how-it-works/providers/profiles#understand-static-credential-endpoint-binding) for placement configuration and upgrade requirements. diff --git a/e2e/rust/tests/forward_proxy_l7_bypass.rs b/e2e/rust/tests/forward_proxy_l7_bypass.rs index d726b84721..f274f20e93 100644 --- a/e2e/rust/tests/forward_proxy_l7_bypass.rs +++ b/e2e/rust/tests/forward_proxy_l7_bypass.rs @@ -19,6 +19,13 @@ async fn start_test_server(alias: &str) -> Result { class Handler(BaseHTTPRequestHandler): def do_GET(self): + if self.headers.get("Upgrade"): + self.send_response(101) + self.send_header("Connection", "Upgrade") + self.send_header("Upgrade", "websocket") + self.send_header("Sec-WebSocket-Accept", "s3pPLMBiTxaQ9kYGzzhZRbK+xOo=") + self.end_headers() + return self.send_response(200) self.end_headers() self.wfile.write(b'{"ok":true}') @@ -178,3 +185,55 @@ except Exception as e: guard.create_output ); } + +/// A permitted GET cannot switch a REST endpoint into an uninspected tunnel. +#[tokio::test] +async fn rest_upgrade_is_refused_in_enforce_and_audit_modes() { + let server = start_test_server("rest-upgrade.openshell.test") + .await + .expect("start backend"); + for enforcement in ["enforce", "audit"] { + let source = write_policy_with_l7_rules(&server.host, server.port).unwrap(); + let policy = std::fs::read_to_string(source.path()).unwrap().replace( + "enforcement: enforce", + &format!("enforcement: {enforcement}"), + ); + let mut file = NamedTempFile::new().unwrap(); + file.write_all(policy.as_bytes()).unwrap(); + file.flush().unwrap(); + let policy_path = file.path().to_str().unwrap(); + let script = format!( + r#" +import socket +for target in ["/allowed", "http://{host}:{port}/allowed"]: + s = socket.create_connection(("{host}", {port}), timeout=15) + request = ("GET " + target + " HTTP/1.1\r\nHost: {host}:{port}\r\n" + "Connection: Upgrade\r\nUpgrade: websocket\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\nSec-WebSocket-Version: 13\r\n\r\n") + s.sendall(request.encode()) + response = b"" + while True: + chunk = s.recv(4096) + if not chunk: + break + response += chunk + s.close() + assert response.startswith(b"HTTP/1.1 403"), response + assert b"unsupported_l7_protocol" in response, response + assert b"REST endpoints" in response, response +print("REST_UPGRADE_REFUSED") +"#, + host = server.host, + port = server.port + ); + let guard = + SandboxGuard::create(&["--policy", policy_path, "--", "python3", "-c", &script]) + .await + .expect("sandbox create"); + assert!( + guard.create_output.contains("REST_UPGRADE_REFUSED"), + "{enforcement}: {}", + guard.create_output + ); + } +} diff --git a/skills/generate-sandbox-policy/SKILL.md b/skills/generate-sandbox-policy/SKILL.md index 80690c45d1..3d1f02de27 100644 --- a/skills/generate-sandbox-policy/SKILL.md +++ b/skills/generate-sandbox-policy/SKILL.md @@ -246,6 +246,7 @@ Only needed for the **Moderate** and **Full** tiers. Translate API path paramete | All sub-paths under `/api/v1/` | `/api/v1/**` | In REST rule paths, `*` stops at `/`, while a whole-segment `**` crosses `/`. +REST endpoints refuse HTTP upgrades in both enforce and audit modes. For WebSocket traffic, use a separate `protocol: websocket` endpoint with explicit upgrade and message rules; `websocket_credential_rewrite` requires that protocol. For example, `/api/v1/**` covers descendants but not `/api/v1` itself; add an explicit rule for that path when needed. `?` matches one character except `/`, and bracket classes such as `[0-9]` are supported.