Skip to content

Commit 28e1ff7

Browse files
authored
feat(policy): add deny rules to network policy schema (#822)
* feat(policy): add deny rules to network policy schema Closes #565 Add L7 deny rules that block specific requests even when allowed by access presets or explicit allow rules. Deny rules mirror the full capability set of allow rules (method, path, query params, SQL command) and take precedence -- if a request matches any deny rule, it is blocked regardless of allow rules. This enables the "allow everything except these specific operations" pattern without enumerating every allowed endpoint. For example, granting read-write access to GitHub while blocking PR approvals, branch protection changes, and ruleset modifications. * fix(policy): deny query matching fails closed, mirror allow-side validation Addresses PR review findings P1 and P3: P1: Deny-side query matching now uses fail-closed semantics. If ANY value for a query key matches the deny matcher, the deny fires. The previous implementation reused allow-side "all values must match" logic which allowed ?force=true&force=false to bypass a deny on force=true. P3: Deny-side query validation now mirrors the full allow-side checks: empty any lists, non-string matcher values, glob+any mutual exclusion, glob type checks, and glob syntax warnings are all validated.
1 parent e0db01e commit 28e1ff7

File tree

7 files changed

+1005
-2
lines changed

7 files changed

+1005
-2
lines changed

.agents/skills/generate-sandbox-policy/SKILL.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,33 @@ network_policies:
266266
- { path: <binary_path> }
267267
```
268268

269+
### Deny Rules
270+
271+
Use `deny_rules` to block specific dangerous operations while allowing broad access. Deny rules are evaluated after allow rules and take precedence. This is the inverse of the `rules` approach — instead of enumerating every allowed operation, you grant broad access and block a small set of dangerous ones.
272+
273+
```yaml
274+
# Example: Allow full access to GitHub but block admin operations
275+
github_api:
276+
name: github_api
277+
endpoints:
278+
- host: api.github.com
279+
port: 443
280+
protocol: rest
281+
enforcement: enforce
282+
access: read-write
283+
deny_rules:
284+
- method: POST
285+
path: "/repos/*/pulls/*/reviews"
286+
- method: PUT
287+
path: "/repos/*/branches/*/protection"
288+
- method: "*"
289+
path: "/repos/*/rulesets"
290+
binaries:
291+
- { path: /usr/bin/curl }
292+
```
293+
294+
Deny rules support the same matching capabilities as allow rules: `method`, `path`, `command` (SQL), and `query` parameter matchers. When generating policies, prefer deny rules when the user needs broad access with a small set of blocked operations — it produces a shorter, more maintainable policy than enumerating 60+ allow rules.
295+
269296
### Private IP Destinations
270297

271298
When the endpoint resolves to a private IP (RFC 1918), the proxy's SSRF protection blocks the connection by default. Use `allowed_ips` to selectively allow specific private IP ranges:

crates/openshell-policy/src/lib.rs

Lines changed: 171 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::path::Path;
1515

1616
use miette::{IntoDiagnostic, Result, WrapErr};
1717
use openshell_core::proto::{
18-
FilesystemPolicy, L7Allow, L7QueryMatcher, L7Rule, LandlockPolicy, NetworkBinary,
18+
FilesystemPolicy, L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, LandlockPolicy, NetworkBinary,
1919
NetworkEndpoint, NetworkPolicyRule, ProcessPolicy, SandboxPolicy,
2020
};
2121
use serde::{Deserialize, Serialize};
@@ -100,6 +100,8 @@ struct NetworkEndpointDef {
100100
rules: Vec<L7RuleDef>,
101101
#[serde(default, skip_serializing_if = "Vec::is_empty")]
102102
allowed_ips: Vec<String>,
103+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
104+
deny_rules: Vec<L7DenyRuleDef>,
103105
}
104106

105107
fn is_zero(v: &u16) -> bool {
@@ -139,6 +141,19 @@ struct QueryAnyDef {
139141
any: Vec<String>,
140142
}
141143

144+
#[derive(Debug, Serialize, Deserialize)]
145+
#[serde(deny_unknown_fields)]
146+
struct L7DenyRuleDef {
147+
#[serde(default, skip_serializing_if = "String::is_empty")]
148+
method: String,
149+
#[serde(default, skip_serializing_if = "String::is_empty")]
150+
path: String,
151+
#[serde(default, skip_serializing_if = "String::is_empty")]
152+
command: String,
153+
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
154+
query: BTreeMap<String, QueryMatcherDef>,
155+
}
156+
142157
#[derive(Debug, Serialize, Deserialize)]
143158
#[serde(deny_unknown_fields)]
144159
struct NetworkBinaryDef {
@@ -214,6 +229,31 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy {
214229
})
215230
.collect(),
216231
allowed_ips: e.allowed_ips,
232+
deny_rules: e
233+
.deny_rules
234+
.into_iter()
235+
.map(|d| L7DenyRule {
236+
method: d.method,
237+
path: d.path,
238+
command: d.command,
239+
query: d
240+
.query
241+
.into_iter()
242+
.map(|(key, matcher)| {
243+
let proto = match matcher {
244+
QueryMatcherDef::Glob(glob) => {
245+
L7QueryMatcher { glob, any: vec![] }
246+
}
247+
QueryMatcherDef::Any(any) => L7QueryMatcher {
248+
glob: String::new(),
249+
any: any.any,
250+
},
251+
};
252+
(key, proto)
253+
})
254+
.collect(),
255+
})
256+
.collect(),
217257
}
218258
})
219259
.collect(),
@@ -330,6 +370,29 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
330370
})
331371
.collect(),
332372
allowed_ips: e.allowed_ips.clone(),
373+
deny_rules: e
374+
.deny_rules
375+
.iter()
376+
.map(|d| L7DenyRuleDef {
377+
method: d.method.clone(),
378+
path: d.path.clone(),
379+
command: d.command.clone(),
380+
query: d
381+
.query
382+
.iter()
383+
.map(|(key, matcher)| {
384+
let yaml_matcher = if !matcher.any.is_empty() {
385+
QueryMatcherDef::Any(QueryAnyDef {
386+
any: matcher.any.clone(),
387+
})
388+
} else {
389+
QueryMatcherDef::Glob(matcher.glob.clone())
390+
};
391+
(key.clone(), yaml_matcher)
392+
})
393+
.collect(),
394+
})
395+
.collect(),
333396
}
334397
})
335398
.collect(),
@@ -1323,6 +1386,113 @@ network_policies:
13231386
);
13241387
}
13251388

1389+
#[test]
1390+
fn parse_deny_rules_from_yaml() {
1391+
let yaml = r#"
1392+
version: 1
1393+
network_policies:
1394+
github:
1395+
name: github
1396+
endpoints:
1397+
- host: api.github.com
1398+
port: 443
1399+
protocol: rest
1400+
access: read-write
1401+
deny_rules:
1402+
- method: POST
1403+
path: "/repos/*/pulls/*/reviews"
1404+
- method: PUT
1405+
path: "/repos/*/branches/*/protection"
1406+
binaries:
1407+
- path: /usr/bin/curl
1408+
"#;
1409+
let proto = parse_sandbox_policy(yaml).expect("parse failed");
1410+
let ep = &proto.network_policies["github"].endpoints[0];
1411+
assert_eq!(ep.deny_rules.len(), 2);
1412+
assert_eq!(ep.deny_rules[0].method, "POST");
1413+
assert_eq!(ep.deny_rules[0].path, "/repos/*/pulls/*/reviews");
1414+
assert_eq!(ep.deny_rules[1].method, "PUT");
1415+
assert_eq!(ep.deny_rules[1].path, "/repos/*/branches/*/protection");
1416+
}
1417+
1418+
#[test]
1419+
fn round_trip_preserves_deny_rules() {
1420+
let yaml = r#"
1421+
version: 1
1422+
network_policies:
1423+
github:
1424+
name: github
1425+
endpoints:
1426+
- host: api.github.com
1427+
port: 443
1428+
protocol: rest
1429+
access: full
1430+
deny_rules:
1431+
- method: POST
1432+
path: "/repos/*/pulls/*/reviews"
1433+
- method: DELETE
1434+
path: "/repos/*/branches/*/protection"
1435+
query:
1436+
force: "true"
1437+
binaries:
1438+
- path: /usr/bin/curl
1439+
"#;
1440+
let proto1 = parse_sandbox_policy(yaml).expect("parse failed");
1441+
let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed");
1442+
let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed");
1443+
1444+
let ep1 = &proto1.network_policies["github"].endpoints[0];
1445+
let ep2 = &proto2.network_policies["github"].endpoints[0];
1446+
assert_eq!(ep1.deny_rules.len(), ep2.deny_rules.len());
1447+
assert_eq!(ep2.deny_rules[0].method, "POST");
1448+
assert_eq!(ep2.deny_rules[0].path, "/repos/*/pulls/*/reviews");
1449+
assert_eq!(ep2.deny_rules[1].method, "DELETE");
1450+
assert_eq!(ep2.deny_rules[1].query["force"].glob, "true");
1451+
}
1452+
1453+
#[test]
1454+
fn parse_deny_rules_with_query_any() {
1455+
let yaml = r#"
1456+
version: 1
1457+
network_policies:
1458+
test:
1459+
name: test
1460+
endpoints:
1461+
- host: api.example.com
1462+
port: 443
1463+
protocol: rest
1464+
access: full
1465+
deny_rules:
1466+
- method: POST
1467+
path: /action
1468+
query:
1469+
type:
1470+
any: ["admin-*", "root-*"]
1471+
binaries:
1472+
- path: /usr/bin/curl
1473+
"#;
1474+
let proto = parse_sandbox_policy(yaml).expect("parse failed");
1475+
let deny = &proto.network_policies["test"].endpoints[0].deny_rules[0];
1476+
assert_eq!(deny.query["type"].any, vec!["admin-*", "root-*"]);
1477+
}
1478+
1479+
#[test]
1480+
fn parse_rejects_unknown_fields_in_deny_rule() {
1481+
let yaml = r#"
1482+
version: 1
1483+
network_policies:
1484+
test:
1485+
endpoints:
1486+
- host: example.com
1487+
port: 443
1488+
deny_rules:
1489+
- method: POST
1490+
path: /foo
1491+
bogus: true
1492+
"#;
1493+
assert!(parse_sandbox_policy(yaml).is_err());
1494+
}
1495+
13261496
#[test]
13271497
fn rejects_port_above_65535() {
13281498
let yaml = r#"

crates/openshell-sandbox/data/sandbox-policy.rego

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,19 +183,118 @@ _policy_allows_l7(policy) if {
183183
request_allowed_for_endpoint(input.request, ep)
184184
}
185185

186-
# L7 request allowed if any matching L4 policy also allows the L7 request.
186+
# L7 request allowed if any matching L4 policy also allows the L7 request
187+
# AND no deny rule blocks it. Deny rules take precedence over allow rules.
187188
allow_request if {
188189
some name
189190
policy := data.network_policies[name]
190191
endpoint_allowed(policy, input.network)
191192
binary_allowed(policy, input.exec)
192193
_policy_allows_l7(policy)
194+
not deny_request
195+
}
196+
197+
# --- L7 deny rules ---
198+
#
199+
# Deny rules are evaluated after allow rules and take precedence.
200+
# If a request matches any deny rule on any matching endpoint, it is blocked
201+
# even if it would otherwise be allowed.
202+
203+
default deny_request = false
204+
205+
# Per-policy helper: true when this policy has at least one endpoint matching
206+
# the L4 request whose deny_rules also match the specific L7 request.
207+
_policy_denies_l7(policy) if {
208+
some ep
209+
ep := policy.endpoints[_]
210+
endpoint_matches_request(ep, input.network)
211+
request_denied_for_endpoint(input.request, ep)
212+
}
213+
214+
deny_request if {
215+
some name
216+
policy := data.network_policies[name]
217+
endpoint_allowed(policy, input.network)
218+
binary_allowed(policy, input.exec)
219+
_policy_denies_l7(policy)
220+
}
221+
222+
# --- L7 deny rule matching: REST method + path + query ---
223+
224+
request_denied_for_endpoint(request, endpoint) if {
225+
some deny_rule
226+
deny_rule := endpoint.deny_rules[_]
227+
deny_rule.method
228+
method_matches(request.method, deny_rule.method)
229+
path_matches(request.path, deny_rule.path)
230+
deny_query_params_match(request, deny_rule)
231+
}
232+
233+
# --- L7 deny rule matching: SQL command ---
234+
235+
request_denied_for_endpoint(request, endpoint) if {
236+
some deny_rule
237+
deny_rule := endpoint.deny_rules[_]
238+
deny_rule.command
239+
command_matches(request.command, deny_rule.command)
240+
}
241+
242+
# Deny query matching: fail-closed semantics.
243+
# If no query rules on the deny rule, match unconditionally (any query params).
244+
# If query rules present, trigger the deny if ANY value for a configured key
245+
# matches the matcher. This is the inverse of allow-side semantics where ALL
246+
# values must match. For deny logic, a single matching value is enough to block.
247+
deny_query_params_match(request, deny_rule) if {
248+
deny_query_rules := object.get(deny_rule, "query", {})
249+
count(deny_query_rules) == 0
250+
}
251+
252+
deny_query_params_match(request, deny_rule) if {
253+
deny_query_rules := object.get(deny_rule, "query", {})
254+
count(deny_query_rules) > 0
255+
not deny_query_key_missing(request, deny_query_rules)
256+
not deny_query_value_mismatch_all(request, deny_query_rules)
257+
}
258+
259+
# A configured deny query key is missing from the request entirely.
260+
# Missing key means the deny rule doesn't apply (fail-open on absence).
261+
deny_query_key_missing(request, query_rules) if {
262+
some key
263+
query_rules[key]
264+
request_query := object.get(request, "query_params", {})
265+
values := object.get(request_query, key, null)
266+
values == null
267+
}
268+
269+
# ALL values for a configured key fail to match the matcher.
270+
# If even one value matches, deny fires. This rule checks the opposite:
271+
# true when NO value matches (i.e., every value is a mismatch).
272+
deny_query_value_mismatch_all(request, query_rules) if {
273+
some key
274+
matcher := query_rules[key]
275+
request_query := object.get(request, "query_params", {})
276+
values := object.get(request_query, key, [])
277+
count(values) > 0
278+
not deny_any_value_matches(values, matcher)
279+
}
280+
281+
# True if at least one value in the list matches the matcher.
282+
deny_any_value_matches(values, matcher) if {
283+
some i
284+
query_value_matches(values[i], matcher)
193285
}
194286

195287
# --- L7 deny reason ---
196288

197289
request_deny_reason := reason if {
198290
input.request
291+
deny_request
292+
reason := sprintf("%s %s blocked by deny rule", [input.request.method, input.request.path])
293+
}
294+
295+
request_deny_reason := reason if {
296+
input.request
297+
not deny_request
199298
not allow_request
200299
reason := sprintf("%s %s not permitted by policy", [input.request.method, input.request.path])
201300
}

0 commit comments

Comments
 (0)