Skip to content

Commit 9321d16

Browse files
authored
fix(network): align REST and WebSocket endpoint handling (#5336)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent c162ccc commit 9321d16

14 files changed

Lines changed: 374 additions & 47 deletions

File tree

‎crates/openshell-policy-schema/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ pub struct NetworkEndpoint {
285285
/// Defaults to false (strict).
286286
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
287287
pub allow_encoded_slash: bool,
288-
/// When true, client-to-server WebSocket text messages on this REST
288+
/// When true, client-to-server WebSocket text messages on a `protocol: websocket`
289289
/// endpoint rewrite credential placeholders after an allowed 101 upgrade.
290290
/// Defaults to false.
291291
#[serde(default, skip_serializing_if = "std::ops::Not::not")]

‎crates/openshell-policy/src/l7_validate.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,9 @@ pub struct L7EndpointFields<'a> {
223223
/// `true` when the endpoint has a non-empty rules list.
224224
pub has_rules: bool,
225225

226+
/// Whether WebSocket credential rewriting was enabled.
227+
pub websocket_credential_rewrite: bool,
228+
226229
/// `true` when the endpoint has a non-empty `deny_rules` list.
227230
pub has_deny_rules: bool,
228231

@@ -244,6 +247,9 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec<String>
244247
let mut errors = Vec::new();
245248
let protocol = ep.protocol;
246249
let l7_protocol = L7Protocol::parse(protocol);
250+
if matches!(l7_protocol, Some(L7Protocol::Rest)) && ep.websocket_credential_rewrite {
251+
errors.push("websocket_credential_rewrite is not supported for protocol rest; use protocol: websocket with explicit message rules".to_string());
252+
}
247253
let explicit_tcp = is_explicit_tcp_protocol(protocol);
248254
let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family);
249255
let is_mcp = matches!(l7_protocol, Some(L7Protocol::Mcp));
@@ -335,6 +341,7 @@ mod tests {
335341

336342
fn valid_rest_endpoint() -> L7EndpointFields<'static> {
337343
L7EndpointFields {
344+
websocket_credential_rewrite: false,
338345
protocol: "rest",
339346
access: "read-only",
340347
has_rules: false,
@@ -344,6 +351,27 @@ mod tests {
344351
}
345352
}
346353

354+
#[test]
355+
fn rest_rejects_websocket_credential_rewrite_with_migration_hint() {
356+
let mut endpoint = valid_rest_endpoint();
357+
endpoint.websocket_credential_rewrite = true;
358+
for protocol in ["rest", "REST", "Rest"] {
359+
endpoint.protocol = protocol;
360+
let errors = validate_l7_endpoint_semantics(&endpoint);
361+
assert!(
362+
errors.iter().any(|e| e.contains("use protocol: websocket")),
363+
"{protocol}: {errors:?}"
364+
);
365+
}
366+
for protocol in ["websocket", "WEBSOCKET", "WebSocket"] {
367+
endpoint.protocol = protocol;
368+
assert!(
369+
validate_l7_endpoint_semantics(&endpoint).is_empty(),
370+
"{protocol}"
371+
);
372+
}
373+
}
374+
347375
#[test]
348376
fn valid_endpoint_produces_no_errors() {
349377
let errors = validate_l7_endpoint_semantics(&valid_rest_endpoint());
@@ -383,6 +411,7 @@ mod tests {
383411
#[test]
384412
fn rejects_unknown_protocol() {
385413
let ep = L7EndpointFields {
414+
websocket_credential_rewrite: false,
386415
protocol: "ftp",
387416
access: "",
388417
has_rules: false,
@@ -397,6 +426,7 @@ mod tests {
397426
#[test]
398427
fn rejects_rules_and_access_together() {
399428
let ep = L7EndpointFields {
429+
websocket_credential_rewrite: false,
400430
protocol: "rest",
401431
access: "full",
402432
has_rules: true,
@@ -411,6 +441,7 @@ mod tests {
411441
#[test]
412442
fn rejects_jsonrpc_with_access_presets() {
413443
let ep = L7EndpointFields {
444+
websocket_credential_rewrite: false,
414445
protocol: "json-rpc",
415446
access: "full",
416447
has_rules: false,
@@ -429,6 +460,7 @@ mod tests {
429460
#[test]
430461
fn rejects_mcp_with_access_presets() {
431462
let ep = L7EndpointFields {
463+
websocket_credential_rewrite: false,
432464
protocol: "mcp",
433465
access: "full",
434466
has_rules: false,
@@ -447,6 +479,7 @@ mod tests {
447479
#[test]
448480
fn rejects_jsonrpc_without_rules() {
449481
let ep = L7EndpointFields {
482+
websocket_credential_rewrite: false,
450483
protocol: "json-rpc",
451484
access: "",
452485
has_rules: false,
@@ -461,6 +494,7 @@ mod tests {
461494
#[test]
462495
fn rejects_protocol_without_rules_or_access() {
463496
let ep = L7EndpointFields {
497+
websocket_credential_rewrite: false,
464498
protocol: "rest",
465499
access: "",
466500
has_rules: false,
@@ -479,6 +513,7 @@ mod tests {
479513
#[test]
480514
fn rejects_mcp_without_rules_when_allow_all_false() {
481515
let ep = L7EndpointFields {
516+
websocket_credential_rewrite: false,
482517
protocol: "mcp",
483518
access: "",
484519
has_rules: false,
@@ -493,6 +528,7 @@ mod tests {
493528
#[test]
494529
fn accepts_mcp_with_allow_all_true() {
495530
let ep = L7EndpointFields {
531+
websocket_credential_rewrite: false,
496532
protocol: "mcp",
497533
access: "",
498534
has_rules: false,
@@ -507,6 +543,7 @@ mod tests {
507543
#[test]
508544
fn rejects_rules_that_deny_all() {
509545
let ep = L7EndpointFields {
546+
websocket_credential_rewrite: false,
510547
protocol: "rest",
511548
access: "",
512549
has_rules: true,
@@ -521,6 +558,7 @@ mod tests {
521558
#[test]
522559
fn mcp_allow_all_with_empty_allow_not_rejected() {
523560
let ep = L7EndpointFields {
561+
websocket_credential_rewrite: false,
524562
protocol: "mcp",
525563
access: "",
526564
has_rules: true,
@@ -538,6 +576,7 @@ mod tests {
538576
#[test]
539577
fn rejects_deny_rules_without_protocol() {
540578
let ep = L7EndpointFields {
579+
websocket_credential_rewrite: false,
541580
protocol: "",
542581
access: "",
543582
has_rules: false,
@@ -556,6 +595,7 @@ mod tests {
556595
#[test]
557596
fn rejects_deny_rules_without_allow_base() {
558597
let ep = L7EndpointFields {
598+
websocket_credential_rewrite: false,
559599
protocol: "rest",
560600
access: "",
561601
has_rules: false,
@@ -574,6 +614,7 @@ mod tests {
574614
#[test]
575615
fn no_protocol_no_errors() {
576616
let ep = L7EndpointFields {
617+
websocket_credential_rewrite: false,
577618
protocol: "",
578619
access: "",
579620
has_rules: false,
@@ -588,6 +629,7 @@ mod tests {
588629
#[test]
589630
fn explicit_tcp_is_valid_without_l7_fields() {
590631
let ep = L7EndpointFields {
632+
websocket_credential_rewrite: false,
591633
protocol: "tcp",
592634
access: "",
593635
has_rules: false,
@@ -604,6 +646,7 @@ mod tests {
604646
#[test]
605647
fn explicit_tcp_rejects_l7_fields() {
606648
let ep = L7EndpointFields {
649+
websocket_credential_rewrite: false,
607650
protocol: "tcp",
608651
access: "full",
609652
has_rules: false,
@@ -648,6 +691,7 @@ mod tests {
648691
#[test]
649692
fn jsonrpc_with_access_emits_single_diagnostic() {
650693
let ep = L7EndpointFields {
694+
websocket_credential_rewrite: false,
651695
protocol: "json-rpc",
652696
access: "full",
653697
has_rules: false,
@@ -669,6 +713,7 @@ mod tests {
669713
#[test]
670714
fn jsonrpc_without_rules_or_access_emits_single_diagnostic() {
671715
let ep = L7EndpointFields {
716+
websocket_credential_rewrite: false,
672717
protocol: "json-rpc",
673718
access: "",
674719
has_rules: false,

‎crates/openshell-policy/src/lib.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,6 +1562,7 @@ fn validate_sandbox_policy_with_mcp_presence(
15621562
})
15631563
});
15641564
let fields = L7EndpointFields {
1565+
websocket_credential_rewrite: ep.websocket_credential_rewrite,
15651566
protocol: &ep.protocol,
15661567
access,
15671568
has_rules: !ep.rules.is_empty(),
@@ -4786,6 +4787,18 @@ network_policies:
47864787
);
47874788
}
47884789

4790+
#[test]
4791+
fn validate_rest_rejects_websocket_credential_rewrite() {
4792+
for protocol in ["rest", "REST", "Rest"] {
4793+
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";
4794+
let yaml = yaml.replace("protocol: rest", &format!("protocol: {protocol}"));
4795+
let policy = parse_sandbox_policy(&yaml).unwrap();
4796+
let errors = validate_sandbox_policy(&policy)
4797+
.expect_err("REST cannot rewrite WebSocket messages");
4798+
assert!(format!("{errors:?}").contains("use protocol: websocket"));
4799+
}
4800+
}
4801+
47894802
#[test]
47904803
fn round_trip_preserves_websocket_credential_rewrite() {
47914804
let yaml = r"
@@ -4796,7 +4809,7 @@ network_policies:
47964809
endpoints:
47974810
- host: gateway.example.com
47984811
port: 443
4799-
protocol: rest
4812+
protocol: websocket
48004813
enforcement: enforce
48014814
access: full
48024815
websocket_credential_rewrite: true
@@ -4808,7 +4821,7 @@ network_policies:
48084821
let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed");
48094822

48104823
let ep = &proto2.network_policies["discord_gateway"].endpoints[0];
4811-
assert_eq!(ep.protocol, "rest");
4824+
assert_eq!(ep.protocol, "websocket");
48124825
assert!(ep.websocket_credential_rewrite);
48134826
assert!(yaml_out.contains("websocket_credential_rewrite: true"));
48144827
}

‎crates/openshell-policy/src/merge.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3818,8 +3818,8 @@ mod tests {
38183818
/// agent is told its approved rule never landed.
38193819
#[test]
38203820
fn merged_policy_covers_the_rule_it_merged() {
3821-
// Every field `merge_endpoint` widens rather than replaces, set on the
3822-
// existing endpoint and absent from the proposal.
3821+
// REST-compatible fields that `merge_endpoint` widens rather than
3822+
// replaces, set on the existing endpoint and absent from the proposal.
38233823
let existing_endpoint = NetworkEndpoint {
38243824
protocol: "rest".to_string(),
38253825
rules: vec![rest_rule("GET", "/health")],
@@ -3830,7 +3830,6 @@ mod tests {
38303830
}],
38313831
allowed_ips: vec!["10.0.0.1".to_string()],
38323832
allow_encoded_slash: true,
3833-
websocket_credential_rewrite: true,
38343833
request_body_credential_rewrite: true,
38353834
advisor_proposed: true,
38363835
..endpoint("api.example.com", 443)

‎crates/openshell-providers/src/profiles.rs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2759,6 +2759,7 @@ pub fn validate_profile_set(
27592759
}
27602760

27612761
let l7_fields = L7EndpointFields {
2762+
websocket_credential_rewrite: endpoint.websocket_credential_rewrite,
27622763
protocol: &endpoint.protocol,
27632764
access: &endpoint.access,
27642765
has_rules: endpoint.rules.as_ref().is_some_and(|r| !r.is_empty()),
@@ -6330,6 +6331,42 @@ binaries: ["", /usr/bin/broken]
63306331
assert!(messages.contains(&"binary path must not be empty"));
63316332
}
63326333

6334+
#[test]
6335+
fn validate_profile_rejects_rest_websocket_credential_rewrite() {
6336+
for protocol in ["rest", "REST", "Rest"] {
6337+
let profile = parse_profile_yaml(
6338+
r"
6339+
id: rest-rewrite
6340+
display_name: REST rewrite
6341+
credentials:
6342+
- name: api_key
6343+
env_vars: [API_KEY]
6344+
auth_style: bearer
6345+
header_name: authorization
6346+
discovery:
6347+
credentials: [api_key]
6348+
endpoints:
6349+
- host: api.example.com
6350+
port: 443
6351+
protocol: rest
6352+
access: read-only
6353+
websocket_credential_rewrite: true
6354+
binaries:
6355+
- /usr/bin/curl
6356+
"
6357+
.replace("protocol: rest", &format!("protocol: {protocol}"))
6358+
.as_str(),
6359+
)
6360+
.unwrap();
6361+
let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]);
6362+
assert!(
6363+
diagnostics
6364+
.iter()
6365+
.any(|d| d.message.contains("use protocol: websocket"))
6366+
);
6367+
}
6368+
}
6369+
63336370
#[test]
63346371
fn validate_profile_set_rejects_noncanonical_profile_ids() {
63356372
let profiles = [

‎crates/openshell-supervisor-network/src/l7/mod.rs‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,9 @@ pub fn parse_l7_config(val: &regorus::Value) -> Option<L7EndpointConfig> {
379379
let allow_encoded_slash = get_object_bool(val, "allow_encoded_slash").unwrap_or(false);
380380
let websocket_credential_rewrite =
381381
get_object_bool(val, "websocket_credential_rewrite").unwrap_or(false);
382+
if protocol == L7Protocol::Rest && websocket_credential_rewrite {
383+
return None;
384+
}
382385
let request_body_credential_rewrite =
383386
get_object_bool(val, "request_body_credential_rewrite").unwrap_or(false);
384387
let allow_uninspected_credentials =
@@ -1393,6 +1396,10 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
13931396
.and_then(serde_json::Value::as_bool)
13941397
.unwrap_or(false);
13951398
let l7_fields = L7EndpointFields {
1399+
websocket_credential_rewrite: ep
1400+
.get("websocket_credential_rewrite")
1401+
.and_then(serde_json::Value::as_bool)
1402+
.unwrap_or(false),
13961403
protocol,
13971404
access,
13981405
has_rules,
@@ -1504,7 +1511,7 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
15041511
&& protocol != "websocket"
15051512
{
15061513
warnings.push(format!(
1507-
"{loc}: websocket_credential_rewrite is ignored unless protocol is rest or websocket"
1514+
"{loc}: websocket_credential_rewrite is ignored unless protocol is websocket"
15081515
));
15091516
}
15101517

@@ -2470,10 +2477,24 @@ mod tests {
24702477
assert!(!config.websocket_credential_rewrite);
24712478
}
24722479

2480+
#[test]
2481+
fn rest_websocket_credential_rewrite_fails_validation_and_materialization() {
2482+
let endpoint = serde_json::json!({
2483+
"protocol": "rest", "host": "gateway.example.com", "port": 443,
2484+
"access": "read-only", "websocket_credential_rewrite": true
2485+
});
2486+
let val = regorus::Value::from_json_str(&endpoint.to_string()).unwrap();
2487+
assert!(parse_l7_config(&val).is_none());
2488+
let (errors, _) = validate_l7_policies(&serde_json::json!({
2489+
"network_policies": {"test": {"endpoints": [endpoint], "binaries": []}}
2490+
}));
2491+
assert!(errors.iter().any(|e| e.contains("use protocol: websocket")));
2492+
}
2493+
24732494
#[test]
24742495
fn parse_l7_config_websocket_credential_rewrite_opt_in() {
24752496
let val = regorus::Value::from_json_str(
2476-
r#"{"protocol": "rest", "host": "gateway.example.com", "port": 443, "websocket_credential_rewrite": true}"#,
2497+
r#"{"protocol": "websocket", "host": "gateway.example.com", "port": 443, "websocket_credential_rewrite": true}"#,
24772498
)
24782499
.unwrap();
24792500
let config = parse_l7_config(&val).unwrap();
@@ -2521,7 +2542,7 @@ mod tests {
25212542
}
25222543

25232544
#[test]
2524-
fn validate_websocket_credential_rewrite_warns_unless_rest_or_websocket() {
2545+
fn validate_websocket_credential_rewrite_warns_without_inspected_protocol() {
25252546
let data = serde_json::json!({
25262547
"network_policies": {
25272548
"test": {

0 commit comments

Comments
 (0)