Skip to content

Commit 1797fb9

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

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
@@ -1636,14 +1636,11 @@ where
16361636
}
16371637
}
16381638

1639-
fn select_l7_config_for_path<'a>(
1639+
pub(crate) fn select_l7_config_for_path<'a>(
16401640
configs: &'a [L7EndpointConfig],
16411641
path: &str,
16421642
) -> Option<&'a L7EndpointConfig> {
1643-
configs
1644-
.iter()
1645-
.filter(|config| config.matches_path(path))
1646-
.max_by_key(|config| config.path_specificity())
1643+
openshell_core::endpoint_path::select_most_specific(configs, path, |config| &config.path)
16471644
}
16481645

16491646
fn emit_l7_request_log(
@@ -9968,6 +9965,157 @@ network_policies:
99689965
let _ = tokio::time::timeout(std::time::Duration::from_secs(1), relay).await;
99699966
}
99709967

9968+
#[tokio::test]
9969+
async fn route_protocol_confines_rpc_calls_beside_broad_rest() {
9970+
for protocol in ["json-rpc", "mcp"] {
9971+
let rule = if protocol == "mcp" {
9972+
"{ method: tools/call, tool: read_status }"
9973+
} else {
9974+
"{ method: status.get }"
9975+
};
9976+
let yaml = format!(
9977+
r#"
9978+
version: 1
9979+
network_policies:
9980+
rest_api:
9981+
endpoints:
9982+
- host: "*.routes.test"
9983+
port: 8000
9984+
protocol: rest
9985+
enforcement: enforce
9986+
access: read-write
9987+
binaries: [{{ path: /usr/bin/python3 }}]
9988+
rpc_api:
9989+
endpoints:
9990+
- host: api.routes.test
9991+
port: 8000
9992+
path: /services/**
9993+
protocol: {protocol}
9994+
enforcement: enforce
9995+
rules: [{{ allow: {rule} }}]
9996+
binaries: [{{ path: /usr/bin/python3 }}]
9997+
"#
9998+
);
9999+
for (path, body, expected) in [
10000+
(
10001+
"/services",
10002+
if protocol == "mcp" {
10003+
r#"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"read_status","arguments":{}}}"#
10004+
} else {
10005+
r#"{"jsonrpc":"2.0","id":1,"method":"status.get"}"#
10006+
},
10007+
"200 OK",
10008+
),
10009+
(
10010+
"/services/catalog",
10011+
if protocol == "mcp" {
10012+
r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"write_status","arguments":{}}}"#
10013+
} else {
10014+
r#"{"jsonrpc":"2.0","id":2,"method":"status.set"}"#
10015+
},
10016+
"403 Forbidden",
10017+
),
10018+
("/public", "{}", "200 OK"),
10019+
] {
10020+
let proto = openshell_policy::parse_sandbox_policy(&yaml).unwrap();
10021+
let engine = OpaEngine::from_proto(&proto).unwrap();
10022+
let input = NetworkInput {
10023+
host: "api.routes.test".into(),
10024+
port: 8000,
10025+
binary_path: PathBuf::from("/usr/bin/python3"),
10026+
binary_sha256: "unused".into(),
10027+
ancestors: vec![],
10028+
cmdline_paths: vec![],
10029+
};
10030+
let (endpoints, generation) = engine
10031+
.query_endpoint_configs_with_generation(&input)
10032+
.unwrap();
10033+
let configs = endpoints
10034+
.iter()
10035+
.filter_map(crate::l7::parse_l7_config)
10036+
.collect::<Vec<_>>();
10037+
let tunnel_engine = engine.clone_engine_for_tunnel(generation).unwrap();
10038+
let ctx = L7EvalContext {
10039+
host: input.host,
10040+
port: 8000,
10041+
request_default_port: Some(8000),
10042+
policy_name: "rpc_api".into(),
10043+
binary_path: "/usr/bin/python3".into(),
10044+
..Default::default()
10045+
};
10046+
let (mut app, mut client) = tokio::io::duplex(8192);
10047+
let (mut relay_upstream, mut upstream) = tokio::io::duplex(8192);
10048+
let relay = tokio::spawn(async move {
10049+
relay_with_route_selection(
10050+
&configs,
10051+
tunnel_engine,
10052+
&mut client,
10053+
&mut relay_upstream,
10054+
&ctx,
10055+
)
10056+
.await
10057+
});
10058+
// An accepting upstream makes a policy bypass produce a 200.
10059+
let server = tokio::spawn(async move {
10060+
let mut received = Vec::new();
10061+
let mut buffer = [0; 4096];
10062+
loop {
10063+
let n = upstream.read(&mut buffer).await.unwrap();
10064+
if n == 0 {
10065+
return received;
10066+
}
10067+
received.extend_from_slice(&buffer[..n]);
10068+
if let Some(head) =
10069+
received.windows(4).position(|window| window == b"\r\n\r\n")
10070+
{
10071+
let headers = std::str::from_utf8(&received[..head]).unwrap();
10072+
let length = headers
10073+
.lines()
10074+
.find_map(|line| {
10075+
let (name, value) = line.split_once(':')?;
10076+
name.eq_ignore_ascii_case("content-length")
10077+
.then(|| value.trim().parse::<usize>().unwrap())
10078+
})
10079+
.unwrap();
10080+
if received.len() >= head + 4 + length {
10081+
upstream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").await.unwrap();
10082+
return received;
10083+
}
10084+
}
10085+
}
10086+
});
10087+
app.write_all(format!(
10088+
"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()
10089+
).as_bytes()).await.unwrap();
10090+
let mut response = Vec::new();
10091+
tokio::time::timeout(
10092+
std::time::Duration::from_secs(5),
10093+
app.read_to_end(&mut response),
10094+
)
10095+
.await
10096+
.expect("relay must answer")
10097+
.unwrap();
10098+
assert!(
10099+
String::from_utf8_lossy(&response).starts_with(&format!("HTTP/1.1 {expected}")),
10100+
"{protocol} {path}: {}",
10101+
String::from_utf8_lossy(&response)
10102+
);
10103+
drop(app);
10104+
tokio::time::timeout(std::time::Duration::from_secs(5), relay)
10105+
.await
10106+
.unwrap()
10107+
.unwrap()
10108+
.unwrap();
10109+
let received = server.await.unwrap();
10110+
assert_eq!(
10111+
received.is_empty(),
10112+
expected == "403 Forbidden",
10113+
"{protocol} {path}: denied calls must not reach upstream"
10114+
);
10115+
}
10116+
}
10117+
}
10118+
997110119
/// A `tools/call` that no JSON-RPC-family fixture below allows.
997210120
const UNALLOWED_TOOL_CALL: &[u8] =
997310121
br#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"delete_resource","arguments":{}}}"#;

0 commit comments

Comments
 (0)