Skip to content

Commit dfb0605

Browse files
committed
fix(policy): align request authorization with route protocol
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent e9ba8d9 commit dfb0605

8 files changed

Lines changed: 461 additions & 7 deletions

File tree

‎crates/openshell-core/src/endpoint_path.rs‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,46 @@ pub fn matches(pattern: &str, path: &str) -> bool {
6868
EndpointPathPattern::new(pattern).matches(path)
6969
}
7070

71+
/// Select the most specific matching endpoint, retaining the last match on ties.
72+
///
73+
/// Specificity counts non-asterisk characters in the endpoint's path selector.
74+
/// Callers filter candidates for protocol and connection eligibility first.
75+
#[must_use]
76+
pub fn select_most_specific<'a, T: 'a>(
77+
endpoints: impl IntoIterator<Item = &'a T>,
78+
path: &str,
79+
selector: impl Fn(&T) -> &str,
80+
) -> Option<&'a T> {
81+
endpoints
82+
.into_iter()
83+
.filter(|endpoint| matches(selector(endpoint), path))
84+
.max_by_key(|endpoint| selector(endpoint).chars().filter(|c| *c != '*').count())
85+
}
86+
7187
#[cfg(test)]
7288
mod tests {
73-
use super::{EndpointPathPattern, matches};
89+
use super::{EndpointPathPattern, matches, select_most_specific};
90+
91+
#[test]
92+
fn route_selection_preserves_specificity_and_ties() {
93+
let endpoints = [
94+
("rest", ""),
95+
("graphql", "/unrelated/exact"),
96+
("json-rpc", "/services/**"),
97+
("mcp", "/services/tools/**"),
98+
("mcp-last", "/services/tools/**"),
99+
];
100+
for (path, protocol) in [
101+
("/public", "rest"),
102+
("/services", "json-rpc"),
103+
("/services/tools/read", "mcp-last"),
104+
("/unrelated/exact", "graphql"),
105+
] {
106+
let selected = select_most_specific(&endpoints, path, |endpoint| endpoint.1).unwrap();
107+
assert_eq!(selected.0, protocol, "route {path}");
108+
}
109+
assert!(select_most_specific(&endpoints[1..], "/public", |endpoint| endpoint.1).is_none());
110+
}
74111

75112
#[test]
76113
fn matches_canonical_endpoint_patterns() {

‎crates/openshell-prover/tests/runtime_parity.rs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,40 @@ fn runtime_engine_with_identity(policy: &str, require_binary_identity: bool) ->
4949
}),
5050
)
5151
.expect("runtime endpoint path matcher should register");
52+
engine
53+
.add_extension(
54+
"openshell.selected_l7_protocol".into(),
55+
2,
56+
Box::new(|params: Vec<Value>| {
57+
// These fixtures use validated endpoint modes. Share route
58+
// matching, ranking, and tie behavior with the runtime without
59+
// depending on the supervisor's networking implementation.
60+
let configs = params[0]
61+
.as_array()?
62+
.iter()
63+
.filter_map(|config| {
64+
let protocol = config["protocol"].as_string().ok()?.to_ascii_lowercase();
65+
if !matches!(
66+
protocol.as_str(),
67+
"rest" | "websocket" | "graphql" | "sql" | "json-rpc" | "mcp"
68+
) {
69+
return None;
70+
}
71+
let path = config["path"]
72+
.as_string()
73+
.map_or("", |value| value.as_ref());
74+
Some((protocol, path))
75+
})
76+
.collect::<Vec<_>>();
77+
let selected = openshell_core::endpoint_path::select_most_specific(
78+
&configs,
79+
params[1].as_string()?,
80+
|config| config.1,
81+
);
82+
Ok(selected.map_or(Value::Undefined, |config| Value::from(config.0.as_str())))
83+
}),
84+
)
85+
.expect("runtime route protocol selector should register");
5286
engine
5387
.add_policy("sandbox-policy.rego".into(), SANDBOX_POLICY_REGO.into())
5488
.expect("runtime Rego should compile");

‎crates/openshell-supervisor-network/data/sandbox-policy.rego‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,27 @@ _policy_allowed_l7_endpoints(policy) := [ep |
231231
some ep
232232
ep := policy.endpoints[_]
233233
endpoint_matches_l7_request(ep, input.network, input.request)
234+
endpoint_uses_selected_request_protocol(ep)
234235
request_allowed_for_endpoint(input.request, ep)
235236
]
236237

238+
# HTTP authorization uses the same parser selection as the relay. A sibling
239+
# endpoint can contribute an allow only when it uses the selected protocol;
240+
# all matching endpoints of that protocol retain additive allow semantics.
241+
# The native selector shares config parsing and path ranking with Rust.
242+
_selected_l7_request_protocol := openshell.selected_l7_protocol(_matching_endpoint_configs, object.get(input.request, "path", ""))
243+
244+
endpoint_uses_selected_request_protocol(endpoint) if {
245+
object.get(input.request, "command", "") == ""
246+
lower(endpoint.protocol) == _selected_l7_request_protocol
247+
}
248+
249+
# SQL commands do not use HTTP request-path routing.
250+
endpoint_uses_selected_request_protocol(_) if {
251+
command := object.get(input.request, "command", "")
252+
command != ""
253+
}
254+
237255
# Forwarding and credential selection share endpoint admission. A deny rule
238256
# on any matching endpoint removes every otherwise admitted endpoint.
239257
_admitted_l7_endpoints := {ep |

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

Lines changed: 153 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,14 +1662,11 @@ where
16621662
}
16631663
}
16641664

1665-
fn select_l7_config_for_path<'a>(
1665+
pub(crate) fn select_l7_config_for_path<'a>(
16661666
configs: &'a [L7EndpointConfig],
16671667
path: &str,
16681668
) -> Option<&'a L7EndpointConfig> {
1669-
configs
1670-
.iter()
1671-
.filter(|config| config.matches_path(path))
1672-
.max_by_key(|config| config.path_specificity())
1669+
openshell_core::endpoint_path::select_most_specific(configs, path, |config| &config.path)
16731670
}
16741671

16751672
fn emit_l7_request_log(
@@ -10565,6 +10562,157 @@ network_policies:
1056510562
let _ = tokio::time::timeout(std::time::Duration::from_secs(1), relay).await;
1056610563
}
1056710564

10565+
#[tokio::test]
10566+
async fn route_protocol_confines_rpc_calls_beside_broad_rest() {
10567+
for protocol in ["json-rpc", "mcp"] {
10568+
let rule = if protocol == "mcp" {
10569+
"{ method: tools/call, tool: read_status }"
10570+
} else {
10571+
"{ method: status.get }"
10572+
};
10573+
let yaml = format!(
10574+
r#"
10575+
version: 1
10576+
network_policies:
10577+
rest_api:
10578+
endpoints:
10579+
- host: "*.routes.test"
10580+
port: 8000
10581+
protocol: rest
10582+
enforcement: enforce
10583+
access: read-write
10584+
binaries: [{{ path: /usr/bin/python3 }}]
10585+
rpc_api:
10586+
endpoints:
10587+
- host: api.routes.test
10588+
port: 8000
10589+
path: /services/**
10590+
protocol: {protocol}
10591+
enforcement: enforce
10592+
rules: [{{ allow: {rule} }}]
10593+
binaries: [{{ path: /usr/bin/python3 }}]
10594+
"#
10595+
);
10596+
for (path, body, expected) in [
10597+
(
10598+
"/services",
10599+
if protocol == "mcp" {
10600+
r#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"read_status","arguments":{}}}"#
10601+
} else {
10602+
r#"{"jsonrpc":"2.0","id":1,"method":"status.get"}"#
10603+
},
10604+
"200 OK",
10605+
),
10606+
(
10607+
"/services/catalog",
10608+
if protocol == "mcp" {
10609+
r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"write_status","arguments":{}}}"#
10610+
} else {
10611+
r#"{"jsonrpc":"2.0","id":2,"method":"status.set"}"#
10612+
},
10613+
"403 Forbidden",
10614+
),
10615+
("/public", "{}", "200 OK"),
10616+
] {
10617+
let proto = openshell_policy::parse_sandbox_policy(&yaml).unwrap();
10618+
let engine = OpaEngine::from_proto(&proto).unwrap();
10619+
let input = NetworkInput {
10620+
host: "api.routes.test".into(),
10621+
port: 8000,
10622+
binary_path: PathBuf::from("/usr/bin/python3"),
10623+
binary_sha256: "unused".into(),
10624+
ancestors: vec![],
10625+
cmdline_paths: vec![],
10626+
};
10627+
let (endpoints, generation) = engine
10628+
.query_endpoint_configs_with_generation(&input)
10629+
.unwrap();
10630+
let configs = endpoints
10631+
.iter()
10632+
.filter_map(crate::l7::parse_l7_config)
10633+
.collect::<Vec<_>>();
10634+
let tunnel_engine = engine.clone_engine_for_tunnel(generation).unwrap();
10635+
let ctx = L7EvalContext {
10636+
host: input.host,
10637+
port: 8000,
10638+
request_default_port: Some(8000),
10639+
policy_name: "rpc_api".into(),
10640+
binary_path: "/usr/bin/python3".into(),
10641+
..Default::default()
10642+
};
10643+
let (mut app, mut client) = tokio::io::duplex(8192);
10644+
let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192);
10645+
let relay = tokio::spawn(async move {
10646+
relay_with_route_selection(
10647+
&configs,
10648+
tunnel_engine,
10649+
&mut client,
10650+
&mut relay_upstream,
10651+
&ctx,
10652+
)
10653+
.await
10654+
});
10655+
// An accepting upstream makes a policy bypass produce a 200.
10656+
let server = tokio::spawn(async move {
10657+
let mut received = Vec::new();
10658+
let mut buffer = [0; 4096];
10659+
loop {
10660+
let n = upstream.read(&mut buffer).await.unwrap();
10661+
if n == 0 {
10662+
return received;
10663+
}
10664+
received.extend_from_slice(&buffer[..n]);
10665+
if let Some(head) =
10666+
received.windows(4).position(|window| window == b"\r\n\r\n")
10667+
{
10668+
let headers = std::str::from_utf8(&received[..head]).unwrap();
10669+
let length = headers
10670+
.lines()
10671+
.find_map(|line| {
10672+
let (name, value) = line.split_once(':')?;
10673+
name.eq_ignore_ascii_case("content-length")
10674+
.then(|| value.trim().parse::<usize>().unwrap())
10675+
})
10676+
.unwrap();
10677+
if received.len() >= head + 4 + length {
10678+
upstream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").await.unwrap();
10679+
return received;
10680+
}
10681+
}
10682+
}
10683+
});
10684+
app.write_all(format!(
10685+
"POST {path} HTTP/1.1\r\nHost: api.routes.test:8000\r\nContent-Type: application/json\r\nAccept: application/json, text/event-stream\r\nMCP-Protocol-Version: 2025-11-25\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()
10686+
).as_bytes()).await.unwrap();
10687+
let mut response = Vec::new();
10688+
tokio::time::timeout(
10689+
std::time::Duration::from_secs(5),
10690+
app.read_to_end(&mut response),
10691+
)
10692+
.await
10693+
.expect("relay must answer")
10694+
.unwrap();
10695+
assert!(
10696+
String::from_utf8_lossy(&response).starts_with(&format!("HTTP/1.1 {expected}")),
10697+
"{protocol} {path}: {}",
10698+
String::from_utf8_lossy(&response)
10699+
);
10700+
drop(app);
10701+
tokio::time::timeout(std::time::Duration::from_secs(5), relay)
10702+
.await
10703+
.unwrap()
10704+
.unwrap()
10705+
.unwrap();
10706+
let received = server.await.unwrap();
10707+
assert_eq!(
10708+
received.is_empty(),
10709+
expected == "403 Forbidden",
10710+
"{protocol} {path}: denied calls must not reach upstream"
10711+
);
10712+
}
10713+
}
10714+
}
10715+
1056810716
/// A `tools/call` that no JSON-RPC-family fixture below allows.
1056910717
const UNALLOWED_TOOL_CALL: &[u8] =
1057010718
br#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"delete_resource","arguments":{}}}"#;

0 commit comments

Comments
 (0)