Skip to content

Commit ceaf8dc

Browse files
johntmyersjohntmyers
committed
fix(policy): reserve provider rule namespace
Closes #1982 Reject user-authored _provider_* network policy keys at gateway boundaries, strip provider-derived rules from sandbox sync, and document the round-trip workflow. Signed-off-by: John Myers <[email protected]>
1 parent d93293a commit ceaf8dc

9 files changed

Lines changed: 472 additions & 32 deletions

File tree

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

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55
66
use openshell_core::proto::{NetworkPolicyRule, SandboxPolicy};
77

8+
pub const PROVIDER_RULE_NAME_PREFIX: &str = "_provider_";
9+
810
#[derive(Debug, Clone, PartialEq)]
911
pub struct ProviderPolicyLayer {
1012
pub rule_name: String,
1113
pub rule: NetworkPolicyRule,
1214
}
1315

16+
#[must_use]
17+
pub fn is_provider_rule_name(rule_name: &str) -> bool {
18+
rule_name.starts_with(PROVIDER_RULE_NAME_PREFIX)
19+
}
20+
1421
#[must_use]
1522
pub fn provider_rule_name(provider_name: &str) -> String {
1623
let sanitized = provider_name
@@ -27,19 +34,27 @@ pub fn provider_rule_name(provider_name: &str) -> String {
2734
.to_string();
2835

2936
if sanitized.is_empty() {
30-
"_provider_unnamed".to_string()
37+
format!("{PROVIDER_RULE_NAME_PREFIX}unnamed")
3138
} else {
32-
format!("_provider_{sanitized}")
39+
format!("{PROVIDER_RULE_NAME_PREFIX}{sanitized}")
3340
}
3441
}
3542

43+
pub fn strip_provider_rule_names(policy: &mut SandboxPolicy) -> bool {
44+
let original_len = policy.network_policies.len();
45+
policy
46+
.network_policies
47+
.retain(|key, _| !is_provider_rule_name(key));
48+
policy.network_policies.len() != original_len
49+
}
50+
3651
/// Compose a normal sandbox policy from user-authored policy plus provider
3752
/// policy layers.
3853
///
3954
/// The returned policy is derived data. It preserves the source policy's
4055
/// static fields and user-authored network policies, then concatenates each
41-
/// provider rule under a reserved `_provider_*` key. Existing user keys are not
42-
/// overwritten; a numeric suffix is added if needed.
56+
/// provider rule under a reserved `_provider_*` key. Existing keys are not
57+
/// overwritten; a numeric suffix is added if provider rule names collide.
4358
#[must_use]
4459
pub fn compose_effective_policy(
4560
source_policy: &SandboxPolicy,
@@ -76,7 +91,10 @@ fn unique_provider_rule_key(policy: &SandboxPolicy, preferred: &str) -> String {
7691

7792
#[cfg(test)]
7893
mod tests {
79-
use super::{ProviderPolicyLayer, compose_effective_policy, provider_rule_name};
94+
use super::{
95+
PROVIDER_RULE_NAME_PREFIX, ProviderPolicyLayer, compose_effective_policy,
96+
is_provider_rule_name, provider_rule_name, strip_provider_rule_names,
97+
};
8098
use openshell_core::proto::{NetworkEndpoint, NetworkPolicyRule, SandboxPolicy};
8199

82100
fn rule(name: &str, host: &str) -> NetworkPolicyRule {
@@ -107,24 +125,58 @@ mod tests {
107125
assert_eq!(provider_rule_name("..."), "_provider_unnamed");
108126
}
109127

128+
#[test]
129+
fn provider_rule_name_prefix_identifies_reserved_keys() {
130+
assert_eq!(PROVIDER_RULE_NAME_PREFIX, "_provider_");
131+
assert!(is_provider_rule_name("_provider_work_github"));
132+
assert!(is_provider_rule_name("_provider_work_github_2"));
133+
assert!(is_provider_rule_name("_provider_"));
134+
assert!(!is_provider_rule_name("provider_work_github"));
135+
assert!(!is_provider_rule_name("custom_provider_work_github"));
136+
}
137+
138+
#[test]
139+
fn strip_provider_rule_names_removes_only_reserved_keys() {
140+
let mut policy = SandboxPolicy::default();
141+
policy.network_policies.insert(
142+
"_provider_work_github".to_string(),
143+
rule("_provider_work_github", "api.github.com"),
144+
);
145+
policy.network_policies.insert(
146+
"sandbox_only".to_string(),
147+
rule("sandbox_only", "sandbox.example.com"),
148+
);
149+
150+
assert!(strip_provider_rule_names(&mut policy));
151+
assert!(
152+
!policy
153+
.network_policies
154+
.contains_key("_provider_work_github")
155+
);
156+
assert!(policy.network_policies.contains_key("sandbox_only"));
157+
assert!(!strip_provider_rule_names(&mut policy));
158+
}
159+
110160
#[test]
111161
fn compose_concatenates_provider_rules_without_overwriting_user_rules() {
112162
let mut source = SandboxPolicy::default();
113163
source.network_policies.insert(
114164
"custom_github".to_string(),
115165
rule("custom_github", "api.github.com"),
116166
);
117-
source.network_policies.insert(
118-
"_provider_work_github".to_string(),
119-
rule("_provider_work_github", "example.com"),
120-
);
121167

122168
let effective = compose_effective_policy(
123169
&source,
124-
&[ProviderPolicyLayer {
125-
rule_name: "_provider_work_github".to_string(),
126-
rule: rule("_provider_work_github", "github.com"),
127-
}],
170+
&[
171+
ProviderPolicyLayer {
172+
rule_name: "_provider_work_github".to_string(),
173+
rule: rule("_provider_work_github", "github.com"),
174+
},
175+
ProviderPolicyLayer {
176+
rule_name: "_provider_work_github".to_string(),
177+
rule: rule("_provider_work_github", "github.example.com"),
178+
},
179+
],
128180
);
129181

130182
assert!(effective.network_policies.contains_key("custom_github"));
@@ -138,7 +190,16 @@ mod tests {
138190
.network_policies
139191
.contains_key("_provider_work_github_2")
140192
);
141-
assert_eq!(source.network_policies.len(), 2);
193+
assert_eq!(
194+
effective
195+
.network_policies
196+
.get("custom_github")
197+
.unwrap()
198+
.endpoints[0]
199+
.host,
200+
"api.github.com"
201+
);
202+
assert_eq!(source.network_policies.len(), 1);
142203
assert_eq!(effective.network_policies.len(), 3);
143204
}
144205
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ use openshell_core::proto::{
2424
};
2525
use serde::{Deserialize, Serialize};
2626

27-
pub use compose::{ProviderPolicyLayer, compose_effective_policy, provider_rule_name};
27+
pub use compose::{
28+
PROVIDER_RULE_NAME_PREFIX, ProviderPolicyLayer, compose_effective_policy,
29+
is_provider_rule_name, provider_rule_name, strip_provider_rule_names,
30+
};
2831
pub use merge::{
2932
PolicyMergeError, PolicyMergeOp, PolicyMergeResult, PolicyMergeWarning, generated_rule_name,
3033
merge_policy, policy_covers_rule,

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use openshell_core::proto::{
77
L7Allow, L7DenyRule, L7Rule, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, SandboxPolicy,
88
};
99

10+
use crate::is_provider_rule_name;
11+
1012
#[derive(Debug, Clone, PartialEq)]
1113
pub enum PolicyMergeOp {
1214
AddRule {
@@ -413,7 +415,7 @@ fn add_rule(
413415
let mut keys: Vec<_> = policy.network_policies.keys().cloned().collect();
414416
keys.sort();
415417
keys.into_iter()
416-
.filter(|k| !k.starts_with("_provider_"))
418+
.filter(|k| !is_provider_rule_name(k))
417419
.find(|key| {
418420
policy
419421
.network_policies
@@ -653,7 +655,7 @@ fn find_endpoint_mut<'a>(
653655
keys.sort();
654656
let target_key = keys
655657
.into_iter()
656-
.filter(|k| !k.starts_with("_provider_"))
658+
.filter(|k| !is_provider_rule_name(k))
657659
.find(|key| {
658660
policy.network_policies.get(key).is_some_and(|rule| {
659661
rule.endpoints

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

Lines changed: 104 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,23 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy)
886886
modified
887887
}
888888

889+
fn strip_proto_provider_policy_entries(proto: &mut openshell_core::proto::SandboxPolicy) -> bool {
890+
openshell_policy::strip_provider_rule_names(proto)
891+
}
892+
893+
fn proto_sync_payload_for_enriched_policy(
894+
proto: &openshell_core::proto::SandboxPolicy,
895+
enriched: bool,
896+
) -> Option<openshell_core::proto::SandboxPolicy> {
897+
if !enriched {
898+
return None;
899+
}
900+
901+
let mut sync_policy = proto.clone();
902+
strip_proto_provider_policy_entries(&mut sync_policy);
903+
Some(sync_policy)
904+
}
905+
889906
/// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem
890907
/// paths required by proxy-mode sandboxes and GPU runtimes. Used for the
891908
/// local-file code path where no proto is available.
@@ -1050,6 +1067,89 @@ mod baseline_tests {
10501067
);
10511068
}
10521069

1070+
#[test]
1071+
fn proto_strip_provider_policy_entries_removes_only_reserved_entries() {
1072+
let mut policy = openshell_policy::restrictive_default_policy();
1073+
policy.network_policies.insert(
1074+
"_provider_work_github".to_string(),
1075+
openshell_core::proto::NetworkPolicyRule {
1076+
name: "_provider_work_github".to_string(),
1077+
..Default::default()
1078+
},
1079+
);
1080+
policy.network_policies.insert(
1081+
"sandbox_only".to_string(),
1082+
openshell_core::proto::NetworkPolicyRule {
1083+
name: "sandbox_only".to_string(),
1084+
..Default::default()
1085+
},
1086+
);
1087+
1088+
assert!(strip_proto_provider_policy_entries(&mut policy));
1089+
assert!(
1090+
!policy
1091+
.network_policies
1092+
.contains_key("_provider_work_github")
1093+
);
1094+
assert!(policy.network_policies.contains_key("sandbox_only"));
1095+
assert!(!strip_proto_provider_policy_entries(&mut policy));
1096+
}
1097+
1098+
#[test]
1099+
fn proto_sync_payload_not_created_for_provider_entries_without_enrichment() {
1100+
let mut runtime_policy = openshell_policy::restrictive_default_policy();
1101+
runtime_policy.network_policies.insert(
1102+
"_provider_work_github".to_string(),
1103+
openshell_core::proto::NetworkPolicyRule {
1104+
name: "_provider_work_github".to_string(),
1105+
..Default::default()
1106+
},
1107+
);
1108+
1109+
assert!(proto_sync_payload_for_enriched_policy(&runtime_policy, false).is_none());
1110+
assert!(
1111+
runtime_policy
1112+
.network_policies
1113+
.contains_key("_provider_work_github"),
1114+
"provider-derived rules alone must not trigger sync or mutate runtime policy"
1115+
);
1116+
}
1117+
1118+
#[test]
1119+
fn proto_sync_payload_for_enrichment_strips_provider_entries_without_mutating_runtime_policy() {
1120+
let mut runtime_policy = openshell_policy::restrictive_default_policy();
1121+
runtime_policy.network_policies.insert(
1122+
"_provider_work_github".to_string(),
1123+
openshell_core::proto::NetworkPolicyRule {
1124+
name: "_provider_work_github".to_string(),
1125+
..Default::default()
1126+
},
1127+
);
1128+
runtime_policy.network_policies.insert(
1129+
"sandbox_only".to_string(),
1130+
openshell_core::proto::NetworkPolicyRule {
1131+
name: "sandbox_only".to_string(),
1132+
..Default::default()
1133+
},
1134+
);
1135+
1136+
let sync_policy = proto_sync_payload_for_enriched_policy(&runtime_policy, true)
1137+
.expect("enrichment should create a sync payload");
1138+
1139+
assert!(
1140+
runtime_policy
1141+
.network_policies
1142+
.contains_key("_provider_work_github"),
1143+
"runtime policy must retain provider-derived rules for OPA input"
1144+
);
1145+
assert!(
1146+
!sync_policy
1147+
.network_policies
1148+
.contains_key("_provider_work_github")
1149+
);
1150+
assert!(sync_policy.network_policies.contains_key("sandbox_only"));
1151+
}
1152+
10531153
#[test]
10541154
fn proto_gpu_enrichment_promotes_proc_without_network_policy() {
10551155
let mut policy = openshell_policy::restrictive_default_policy();
@@ -1298,6 +1398,7 @@ async fn load_policy(
12981398
// Enrich before syncing so the gateway baseline includes
12991399
// baseline paths from the start.
13001400
enrich_proto_baseline_paths(&mut discovered);
1401+
strip_proto_provider_policy_entries(&mut discovered);
13011402
let sandbox = sandbox.as_deref().ok_or_else(|| {
13021403
miette::miette!(
13031404
"Cannot sync discovered policy: sandbox not available.\n\
@@ -1322,11 +1423,11 @@ async fn load_policy(
13221423
// sandboxes. If the policy was enriched, sync the updated version
13231424
// back to the gateway so users can see the effective policy.
13241425
let enriched = enrich_proto_baseline_paths(&mut proto_policy);
1325-
if enriched
1426+
let sync_policy = proto_sync_payload_for_enriched_policy(&proto_policy, enriched);
1427+
if let Some(sync_policy) = sync_policy
13261428
&& let Some(sandbox_name) = sandbox.as_deref()
13271429
&& let Err(e) =
1328-
openshell_core::grpc_client::sync_policy(endpoint, sandbox_name, &proto_policy)
1329-
.await
1430+
openshell_core::grpc_client::sync_policy(endpoint, sandbox_name, &sync_policy).await
13301431
{
13311432
warn!(
13321433
error = %e,

0 commit comments

Comments
 (0)