diff --git a/.agents/skills/generate-sandbox-policy/SKILL.md b/.agents/skills/generate-sandbox-policy/SKILL.md index 953302724..7149bbd61 100644 --- a/.agents/skills/generate-sandbox-policy/SKILL.md +++ b/.agents/skills/generate-sandbox-policy/SKILL.md @@ -266,6 +266,33 @@ network_policies: - { path: } ``` +### Deny Rules + +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. + +```yaml +# Example: Allow full access to GitHub but block admin operations +github_api: + name: github_api + endpoints: + - host: api.github.com + port: 443 + protocol: rest + enforcement: enforce + access: read-write + deny_rules: + - method: POST + path: "/repos/*/pulls/*/reviews" + - method: PUT + path: "/repos/*/branches/*/protection" + - method: "*" + path: "/repos/*/rulesets" + binaries: + - { path: /usr/bin/curl } +``` + +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. + ### Private IP Destinations 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: diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 84358ea67..e3c26061a 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -15,7 +15,7 @@ use std::path::Path; use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::proto::{ - FilesystemPolicy, L7Allow, L7QueryMatcher, L7Rule, LandlockPolicy, NetworkBinary, + FilesystemPolicy, L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, LandlockPolicy, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, ProcessPolicy, SandboxPolicy, }; use serde::{Deserialize, Serialize}; @@ -100,6 +100,8 @@ struct NetworkEndpointDef { rules: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] allowed_ips: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + deny_rules: Vec, } fn is_zero(v: &u16) -> bool { @@ -139,6 +141,19 @@ struct QueryAnyDef { any: Vec, } +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct L7DenyRuleDef { + #[serde(default, skip_serializing_if = "String::is_empty")] + method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + path: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + command: String, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + query: BTreeMap, +} + #[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct NetworkBinaryDef { @@ -214,6 +229,31 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { }) .collect(), allowed_ips: e.allowed_ips, + deny_rules: e + .deny_rules + .into_iter() + .map(|d| L7DenyRule { + method: d.method, + path: d.path, + command: d.command, + query: d + .query + .into_iter() + .map(|(key, matcher)| { + let proto = match matcher { + QueryMatcherDef::Glob(glob) => { + L7QueryMatcher { glob, any: vec![] } + } + QueryMatcherDef::Any(any) => L7QueryMatcher { + glob: String::new(), + any: any.any, + }, + }; + (key, proto) + }) + .collect(), + }) + .collect(), } }) .collect(), @@ -330,6 +370,29 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { }) .collect(), allowed_ips: e.allowed_ips.clone(), + deny_rules: e + .deny_rules + .iter() + .map(|d| L7DenyRuleDef { + method: d.method.clone(), + path: d.path.clone(), + command: d.command.clone(), + query: d + .query + .iter() + .map(|(key, matcher)| { + let yaml_matcher = if !matcher.any.is_empty() { + QueryMatcherDef::Any(QueryAnyDef { + any: matcher.any.clone(), + }) + } else { + QueryMatcherDef::Glob(matcher.glob.clone()) + }; + (key.clone(), yaml_matcher) + }) + .collect(), + }) + .collect(), } }) .collect(), @@ -1323,6 +1386,113 @@ network_policies: ); } + #[test] + fn parse_deny_rules_from_yaml() { + let yaml = r#" +version: 1 +network_policies: + github: + name: github + endpoints: + - host: api.github.com + port: 443 + protocol: rest + access: read-write + deny_rules: + - method: POST + path: "/repos/*/pulls/*/reviews" + - method: PUT + path: "/repos/*/branches/*/protection" + binaries: + - path: /usr/bin/curl +"#; + let proto = parse_sandbox_policy(yaml).expect("parse failed"); + let ep = &proto.network_policies["github"].endpoints[0]; + assert_eq!(ep.deny_rules.len(), 2); + assert_eq!(ep.deny_rules[0].method, "POST"); + assert_eq!(ep.deny_rules[0].path, "/repos/*/pulls/*/reviews"); + assert_eq!(ep.deny_rules[1].method, "PUT"); + assert_eq!(ep.deny_rules[1].path, "/repos/*/branches/*/protection"); + } + + #[test] + fn round_trip_preserves_deny_rules() { + let yaml = r#" +version: 1 +network_policies: + github: + name: github + endpoints: + - host: api.github.com + port: 443 + protocol: rest + access: full + deny_rules: + - method: POST + path: "/repos/*/pulls/*/reviews" + - method: DELETE + path: "/repos/*/branches/*/protection" + query: + force: "true" + binaries: + - path: /usr/bin/curl +"#; + let proto1 = parse_sandbox_policy(yaml).expect("parse failed"); + let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed"); + let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); + + let ep1 = &proto1.network_policies["github"].endpoints[0]; + let ep2 = &proto2.network_policies["github"].endpoints[0]; + assert_eq!(ep1.deny_rules.len(), ep2.deny_rules.len()); + assert_eq!(ep2.deny_rules[0].method, "POST"); + assert_eq!(ep2.deny_rules[0].path, "/repos/*/pulls/*/reviews"); + assert_eq!(ep2.deny_rules[1].method, "DELETE"); + assert_eq!(ep2.deny_rules[1].query["force"].glob, "true"); + } + + #[test] + fn parse_deny_rules_with_query_any() { + let yaml = r#" +version: 1 +network_policies: + test: + name: test + endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: full + deny_rules: + - method: POST + path: /action + query: + type: + any: ["admin-*", "root-*"] + binaries: + - path: /usr/bin/curl +"#; + let proto = parse_sandbox_policy(yaml).expect("parse failed"); + let deny = &proto.network_policies["test"].endpoints[0].deny_rules[0]; + assert_eq!(deny.query["type"].any, vec!["admin-*", "root-*"]); + } + + #[test] + fn parse_rejects_unknown_fields_in_deny_rule() { + let yaml = r#" +version: 1 +network_policies: + test: + endpoints: + - host: example.com + port: 443 + deny_rules: + - method: POST + path: /foo + bogus: true +"#; + assert!(parse_sandbox_policy(yaml).is_err()); + } + #[test] fn rejects_port_above_65535() { let yaml = r#" diff --git a/crates/openshell-sandbox/data/sandbox-policy.rego b/crates/openshell-sandbox/data/sandbox-policy.rego index ce4400a66..939c2e5cb 100644 --- a/crates/openshell-sandbox/data/sandbox-policy.rego +++ b/crates/openshell-sandbox/data/sandbox-policy.rego @@ -183,19 +183,118 @@ _policy_allows_l7(policy) if { request_allowed_for_endpoint(input.request, ep) } -# L7 request allowed if any matching L4 policy also allows the L7 request. +# L7 request allowed if any matching L4 policy also allows the L7 request +# AND no deny rule blocks it. Deny rules take precedence over allow rules. allow_request if { some name policy := data.network_policies[name] endpoint_allowed(policy, input.network) binary_allowed(policy, input.exec) _policy_allows_l7(policy) + not deny_request +} + +# --- L7 deny rules --- +# +# Deny rules are evaluated after allow rules and take precedence. +# If a request matches any deny rule on any matching endpoint, it is blocked +# even if it would otherwise be allowed. + +default deny_request = false + +# Per-policy helper: true when this policy has at least one endpoint matching +# the L4 request whose deny_rules also match the specific L7 request. +_policy_denies_l7(policy) if { + some ep + ep := policy.endpoints[_] + endpoint_matches_request(ep, input.network) + request_denied_for_endpoint(input.request, ep) +} + +deny_request if { + some name + policy := data.network_policies[name] + endpoint_allowed(policy, input.network) + binary_allowed(policy, input.exec) + _policy_denies_l7(policy) +} + +# --- L7 deny rule matching: REST method + path + query --- + +request_denied_for_endpoint(request, endpoint) if { + some deny_rule + deny_rule := endpoint.deny_rules[_] + deny_rule.method + method_matches(request.method, deny_rule.method) + path_matches(request.path, deny_rule.path) + deny_query_params_match(request, deny_rule) +} + +# --- L7 deny rule matching: SQL command --- + +request_denied_for_endpoint(request, endpoint) if { + some deny_rule + deny_rule := endpoint.deny_rules[_] + deny_rule.command + command_matches(request.command, deny_rule.command) +} + +# Deny query matching: fail-closed semantics. +# If no query rules on the deny rule, match unconditionally (any query params). +# If query rules present, trigger the deny if ANY value for a configured key +# matches the matcher. This is the inverse of allow-side semantics where ALL +# values must match. For deny logic, a single matching value is enough to block. +deny_query_params_match(request, deny_rule) if { + deny_query_rules := object.get(deny_rule, "query", {}) + count(deny_query_rules) == 0 +} + +deny_query_params_match(request, deny_rule) if { + deny_query_rules := object.get(deny_rule, "query", {}) + count(deny_query_rules) > 0 + not deny_query_key_missing(request, deny_query_rules) + not deny_query_value_mismatch_all(request, deny_query_rules) +} + +# A configured deny query key is missing from the request entirely. +# Missing key means the deny rule doesn't apply (fail-open on absence). +deny_query_key_missing(request, query_rules) if { + some key + query_rules[key] + request_query := object.get(request, "query_params", {}) + values := object.get(request_query, key, null) + values == null +} + +# ALL values for a configured key fail to match the matcher. +# If even one value matches, deny fires. This rule checks the opposite: +# true when NO value matches (i.e., every value is a mismatch). +deny_query_value_mismatch_all(request, query_rules) if { + some key + matcher := query_rules[key] + request_query := object.get(request, "query_params", {}) + values := object.get(request_query, key, []) + count(values) > 0 + not deny_any_value_matches(values, matcher) +} + +# True if at least one value in the list matches the matcher. +deny_any_value_matches(values, matcher) if { + some i + query_value_matches(values[i], matcher) } # --- L7 deny reason --- request_deny_reason := reason if { input.request + deny_request + reason := sprintf("%s %s blocked by deny rule", [input.request.method, input.request.path]) +} + +request_deny_reason := reason if { + input.request + not deny_request not allow_request reason := sprintf("%s %s not permitted by policy", [input.request.method, input.request.path]) } diff --git a/crates/openshell-sandbox/src/l7/mod.rs b/crates/openshell-sandbox/src/l7/mod.rs index af403aead..d354f562b 100644 --- a/crates/openshell-sandbox/src/l7/mod.rs +++ b/crates/openshell-sandbox/src/l7/mod.rs @@ -331,6 +331,174 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< // The old warning about missing `tls: terminate` is no longer needed // because TLS termination is now automatic. + // Validate deny_rules + let has_deny_rules = ep + .get("deny_rules") + .and_then(|v| v.as_array()) + .is_some_and(|a| !a.is_empty()); + if has_deny_rules { + // deny_rules require L7 inspection + if protocol.is_empty() { + errors.push(format!( + "{loc}: deny_rules require protocol (L7 inspection must be enabled)" + )); + } + + // deny_rules require some allow base (access or rules) + if !has_rules && access.is_empty() { + errors.push(format!( + "{loc}: deny_rules require rules or access to define the base allow set" + )); + } + + if let Some(deny_rules) = ep.get("deny_rules").and_then(|v| v.as_array()) { + for (deny_idx, deny_rule) in deny_rules.iter().enumerate() { + let deny_loc = format!("{loc}.deny_rules[{deny_idx}]"); + + // Validate method + if let Some(method) = deny_rule.get("method").and_then(|m| m.as_str()) + && !method.is_empty() + && protocol == "rest" + { + let valid_methods = [ + "GET", "HEAD", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "*", + ]; + if !valid_methods.contains(&method.to_ascii_uppercase().as_str()) { + warnings.push(format!( + "{deny_loc}: Unknown HTTP method '{method}'. Standard methods: GET, HEAD, POST, PUT, DELETE, PATCH, OPTIONS." + )); + } + } + + // Validate path glob syntax + if let Some(path) = deny_rule.get("path").and_then(|p| p.as_str()) { + if let Some(warning) = check_glob_syntax(path) { + warnings.push(format!("{deny_loc}.path: {warning}")); + } + } + + // Validate query matchers — mirrors allow-side validation exactly + if let Some(query) = deny_rule.get("query").filter(|v| !v.is_null()) { + let Some(query_obj) = query.as_object() else { + errors.push(format!( + "{deny_loc}.query: expected map of query matchers" + )); + continue; + }; + + for (param, matcher) in query_obj { + if let Some(glob_str) = matcher.as_str() { + if let Some(warning) = check_glob_syntax(glob_str) { + warnings + .push(format!("{deny_loc}.query.{param}: {warning}")); + } + continue; + } + + let Some(matcher_obj) = matcher.as_object() else { + errors.push(format!( + "{deny_loc}.query.{param}: expected string glob or object with `any`" + )); + continue; + }; + + let has_any = matcher_obj.get("any").is_some(); + let has_glob = matcher_obj.get("glob").is_some(); + let has_unknown = + matcher_obj.keys().any(|k| k != "any" && k != "glob"); + if has_unknown { + errors.push(format!( + "{deny_loc}.query.{param}: unknown matcher keys; only `glob` or `any` are supported" + )); + continue; + } + + if has_glob && has_any { + errors.push(format!( + "{deny_loc}.query.{param}: matcher cannot specify both `glob` and `any`" + )); + continue; + } + + if !has_glob && !has_any { + errors.push(format!( + "{deny_loc}.query.{param}: object matcher requires `glob` string or non-empty `any` list" + )); + continue; + } + + if has_glob { + match matcher_obj.get("glob").and_then(|v| v.as_str()) { + None => { + errors.push(format!( + "{deny_loc}.query.{param}.glob: expected glob string" + )); + } + Some(g) => { + if let Some(warning) = check_glob_syntax(g) { + warnings.push(format!( + "{deny_loc}.query.{param}.glob: {warning}" + )); + } + } + } + continue; + } + + let any = matcher_obj.get("any").and_then(|v| v.as_array()); + let Some(any) = any else { + errors.push(format!( + "{deny_loc}.query.{param}.any: expected array of glob strings" + )); + continue; + }; + + if any.is_empty() { + errors.push(format!( + "{deny_loc}.query.{param}.any: list must not be empty" + )); + continue; + } + + if any.iter().any(|v| v.as_str().is_none()) { + errors.push(format!( + "{deny_loc}.query.{param}.any: all values must be strings" + )); + } + + for item in any.iter().filter_map(|v| v.as_str()) { + if let Some(warning) = check_glob_syntax(item) { + warnings.push(format!( + "{deny_loc}.query.{param}.any: {warning}" + )); + } + } + } + } + + // SQL command validation + if let Some(command) = deny_rule.get("command").and_then(|c| c.as_str()) { + if !command.is_empty() && protocol == "rest" { + warnings.push(format!( + "{deny_loc}: command is for SQL protocol, not REST" + )); + } + } + } + } + } + + // Empty deny_rules list (explicitly set but empty) + if ep + .get("deny_rules") + .and_then(|v| v.as_array()) + .is_some_and(Vec::is_empty) + { + errors.push(format!( + "{loc}: deny_rules list cannot be empty (would have no effect). Remove it if no denials are needed." + )); + } + // Validate HTTP methods in rules if has_rules && protocol == "rest" { let valid_methods = [ @@ -1161,4 +1329,193 @@ mod tests { "valid query matcher shapes should not error: {errors:?}" ); } + + // --- Deny rules validation tests --- + + #[test] + fn validate_deny_rules_require_protocol() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "deny_rules": [{ "method": "POST", "path": "/admin" }] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("deny_rules require protocol")), + "should require protocol for deny_rules: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_require_allow_base() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "deny_rules": [{ "method": "POST", "path": "/admin" }] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("deny_rules require rules or access")), + "should require rules or access for deny_rules: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_empty_list_rejected() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "access": "full", + "deny_rules": [] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("deny_rules list cannot be empty")), + "should reject empty deny_rules: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_valid_config_accepted() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "access": "read-write", + "deny_rules": [ + { "method": "POST", "path": "/repos/*/pulls/*/reviews" }, + { "method": "PUT", "path": "/repos/*/branches/*/protection" } + ] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors.is_empty(), + "valid deny_rules should not error: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_query_empty_any_rejected() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "access": "full", + "deny_rules": [{ + "method": "POST", + "path": "/admin", + "query": { "type": { "any": [] } } + }] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("any: list must not be empty")), + "should reject empty any list in deny query: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_query_non_string_rejected() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "access": "full", + "deny_rules": [{ + "method": "POST", + "path": "/admin", + "query": { "force": 123 } + }] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors + .iter() + .any(|e| e.contains("expected string glob or object")), + "should reject non-string/non-object matcher in deny query: {errors:?}" + ); + } + + #[test] + fn validate_deny_rules_query_valid_matchers_accepted() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "api.example.com", + "port": 443, + "protocol": "rest", + "access": "full", + "deny_rules": [{ + "method": "POST", + "path": "/admin/**", + "query": { + "force": "true", + "type": { "any": ["admin-*", "root-*"] }, + "scope": { "glob": "org-*" } + } + }] + }], + "binaries": [] + } + } + }); + let (errors, _) = validate_l7_policies(&data); + assert!( + errors.is_empty(), + "valid deny query matchers should not error: {errors:?}" + ); + } } diff --git a/crates/openshell-sandbox/src/opa.rs b/crates/openshell-sandbox/src/opa.rs index 32f1d6c19..0069bcc3e 100644 --- a/crates/openshell-sandbox/src/opa.rs +++ b/crates/openshell-sandbox/src/opa.rs @@ -874,6 +874,43 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St if !e.allowed_ips.is_empty() { ep["allowed_ips"] = e.allowed_ips.clone().into(); } + if !e.deny_rules.is_empty() { + let deny_rules: Vec = e + .deny_rules + .iter() + .map(|d| { + let mut deny = serde_json::json!({}); + if !d.method.is_empty() { + deny["method"] = d.method.clone().into(); + } + if !d.path.is_empty() { + deny["path"] = d.path.clone().into(); + } + if !d.command.is_empty() { + deny["command"] = d.command.clone().into(); + } + let query: serde_json::Map = d + .query + .iter() + .map(|(key, matcher)| { + let mut matcher_json = serde_json::json!({}); + if !matcher.glob.is_empty() { + matcher_json["glob"] = matcher.glob.clone().into(); + } + if !matcher.any.is_empty() { + matcher_json["any"] = matcher.any.clone().into(); + } + (key.clone(), matcher_json) + }) + .collect(); + if !query.is_empty() { + deny["query"] = query.into(); + } + deny + }) + .collect(); + ep["deny_rules"] = deny_rules.into(); + } ep }) .collect(); @@ -1934,6 +1971,267 @@ process: assert_eq!(val, regorus::Value::from(true)); } + // ======================================================================== + // Deny rules tests + // ======================================================================== + + const L7_DENY_TEST_DATA: &str = r#" +network_policies: + github_api: + name: github_api + endpoints: + - host: api.github.com + port: 443 + protocol: rest + enforcement: enforce + access: read-write + deny_rules: + - method: POST + path: "/repos/*/pulls/*/reviews" + - method: PUT + path: "/repos/*/branches/*/protection" + - method: "*" + path: "/repos/*/rulesets" + binaries: + - { path: /usr/bin/curl } + deny_with_query: + name: deny_with_query + endpoints: + - host: api.restricted.com + port: 443 + protocol: rest + enforcement: enforce + access: full + deny_rules: + - method: POST + path: "/admin/**" + query: + force: "true" + binaries: + - { path: /usr/bin/curl } +filesystem_policy: + include_workdir: true + read_only: [] + read_write: [] +landlock: + compatibility: best_effort +process: + run_as_user: sandbox + run_as_group: sandbox +"#; + + fn l7_deny_engine() -> OpaEngine { + OpaEngine::from_strings(TEST_POLICY, L7_DENY_TEST_DATA) + .expect("Failed to load deny test data") + } + + #[test] + fn l7_deny_rule_blocks_allowed_method_path() { + let engine = l7_deny_engine(); + // POST to reviews is allowed by read-write preset but denied by deny rule + let input = l7_input( + "api.github.com", + 443, + "POST", + "/repos/myorg/pulls/123/reviews", + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(false), + "deny rule should block POST to reviews" + ); + } + + #[test] + fn l7_deny_rule_allows_non_matching_requests() { + let engine = l7_deny_engine(); + // GET repos/issues is allowed and not denied + let input = l7_input("api.github.com", 443, "GET", "/repos/myorg/issues"); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(true), + "non-denied GET should be allowed" + ); + } + + #[test] + fn l7_deny_rule_allows_same_method_different_path() { + let engine = l7_deny_engine(); + // POST to issues is allowed (deny only targets reviews) + let input = l7_input("api.github.com", 443, "POST", "/repos/myorg/issues"); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(true), + "POST to issues should be allowed" + ); + } + + #[test] + fn l7_deny_rule_blocks_wildcard_method() { + let engine = l7_deny_engine(); + // GET /repos/myorg/rulesets should be denied (method: "*") + let input = l7_input("api.github.com", 443, "GET", "/repos/myorg/rulesets"); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(false), + "wildcard method deny should block GET" + ); + } + + #[test] + fn l7_deny_rule_blocks_put_protection() { + let engine = l7_deny_engine(); + let input = l7_input( + "api.github.com", + 443, + "PUT", + "/repos/myorg/branches/main/protection", + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(false), + "PUT to branch protection should be denied" + ); + } + + #[test] + fn l7_deny_reason_populated_when_deny_rule_matches() { + let engine = l7_deny_engine(); + let input = l7_input( + "api.github.com", + 443, + "POST", + "/repos/myorg/pulls/123/reviews", + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.request_deny_reason".into()) + .unwrap(); + let reason = match val { + regorus::Value::String(s) => s.to_string(), + _ => String::new(), + }; + assert!( + reason.contains("deny rule"), + "Expected deny rule reason, got: {reason}" + ); + } + + #[test] + fn l7_deny_rule_with_query_blocks_matching_params() { + let engine = l7_deny_engine(); + // POST /admin/settings with force=true should be denied + let input = l7_input_with_query( + "api.restricted.com", + 443, + "POST", + "/admin/settings", + serde_json::json!({"force": ["true"]}), + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(false), + "deny with matching query should block" + ); + } + + #[test] + fn l7_deny_rule_with_query_allows_non_matching_params() { + let engine = l7_deny_engine(); + // POST /admin/settings with force=false should be allowed (query doesn't match deny) + let input = l7_input_with_query( + "api.restricted.com", + 443, + "POST", + "/admin/settings", + serde_json::json!({"force": ["false"]}), + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(true), + "deny with non-matching query should allow" + ); + } + + #[test] + fn l7_deny_rule_with_query_blocks_when_any_value_matches() { + let engine = l7_deny_engine(); + // POST /admin/settings with force=true&force=false should STILL be denied + // because at least one value ("true") matches the deny rule. + // This is fail-closed: any matching value triggers the deny. + let input = l7_input_with_query( + "api.restricted.com", + 443, + "POST", + "/admin/settings", + serde_json::json!({"force": ["true", "false"]}), + ); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(false), + "deny should fire when ANY value matches, even with mixed values" + ); + } + + #[test] + fn l7_deny_rule_without_matching_query_key_allows() { + let engine = l7_deny_engine(); + // POST /admin/settings with no query params -- deny rule has query.force=true, + // so no match (key not present) and request should be allowed + let input = l7_input("api.restricted.com", 443, "POST", "/admin/settings"); + let mut eng = engine.engine.lock().unwrap(); + eng.set_input_json(&input.to_string()).unwrap(); + let val = eng + .eval_rule("data.openshell.sandbox.allow_request".into()) + .unwrap(); + assert_eq!( + val, + regorus::Value::from(true), + "deny without matching query key should allow" + ); + } + // ======================================================================== // Overlapping policies (duplicate host:port) — regression tests // ======================================================================== diff --git a/docs/reference/policy-schema.mdx b/docs/reference/policy-schema.mdx index e9795475d..3e505cf3e 100644 --- a/docs/reference/policy-schema.mdx +++ b/docs/reference/policy-schema.mdx @@ -159,6 +159,7 @@ Each endpoint defines a reachable destination and optional inspection rules. | `enforcement` | string | No | `enforce` actively blocks disallowed requests. `audit` logs violations but allows traffic through. | | `access` | string | No | HTTP access level. One of `read-only`, `read-write`, or `full`. Mutually exclusive with `rules`. | | `rules` | list of rule objects | No | Fine-grained per-method, per-path allow rules. Mutually exclusive with `access`. | +| `deny_rules` | list of deny rule objects | No | L7 deny rules that block specific requests even when allowed by `access` or `rules`. Deny rules take precedence over allow rules. | | `allowed_ips` | list of string | No | CIDR or IP allowlist for SSRF override. Entries overlapping loopback (`127.0.0.0/8`), link-local (`169.254.0.0/16`), or unspecified (`0.0.0.0`) are rejected at load time. | #### Access Levels @@ -198,6 +199,38 @@ rules: any: ["v1.*", "v2.*"] ``` +#### Deny Rule Object + +Blocks specific operations on endpoints that otherwise have broad access. Each deny rule +matches a method and path (and optionally query parameters or SQL commands). Deny rules are +evaluated after allow rules and take precedence -- if a request matches any deny rule, it is +blocked regardless of what the allow rules or access preset permit. + +| Field | Type | Required | Description | +|---|---|---|---| +| `method` | string | No | HTTP method to deny (for example, `POST`, `DELETE`). `*` matches any method. | +| `path` | string | No | URL path pattern. Same glob syntax as allow rules. | +| `command` | string | No | SQL command to deny (`SELECT`, `INSERT`, etc.). For `protocol: sql` only. | +| `query` | map | No | Query parameter matchers. Same syntax as allow rule `query`. | + +Example -- grant read-write access to GitHub but block admin operations: + +```yaml showLineNumbers={false} +endpoints: + - host: api.github.com + port: 443 + protocol: rest + enforcement: enforce + access: read-write + deny_rules: + - method: POST + path: "/repos/*/pulls/*/reviews" + - method: PUT + path: "/repos/*/branches/*/protection" + - method: "*" + path: "/repos/*/rulesets" +``` + ### Binary Object Identifies an executable that is permitted to use the associated endpoints. diff --git a/proto/sandbox.proto b/proto/sandbox.proto index 61948a527..e9b81363e 100644 --- a/proto/sandbox.proto +++ b/proto/sandbox.proto @@ -85,6 +85,25 @@ message NetworkEndpoint { // If `port` is set and `ports` is empty, `port` is normalized to `ports: [port]`. // If both are set, `ports` takes precedence. repeated uint32 ports = 9; + // Explicit L7 deny rules. When present, requests matching any deny rule + // are blocked even if they match an allow rule or access preset. + // Deny rules take precedence over allow rules. + repeated L7DenyRule deny_rules = 10; +} + +// An L7 deny rule that blocks specific requests. +// Mirrors L7Allow — same fields, same matching semantics, inverted effect. +// Deny rules are evaluated after allow rules and take precedence. +message L7DenyRule { + // HTTP method (REST): GET, POST, etc. or "*" for any. + string method = 1; + // URL path glob pattern (REST): "/repos/*/pulls/*/reviews", "**" for any. + string path = 2; + // SQL command (SQL): SELECT, INSERT, etc. or "*" for any. + string command = 3; + // Query parameter matcher map (REST). + // Same semantics as L7Allow.query. + map query = 4; } // An L7 policy rule (allow-only).