Skip to content

Commit 7ceea96

Browse files
johntmyerspimlock
andauthored
fix(policy): ignore advisor provenance during contract inference (#3069)
* test(policy): reproduce advisor provenance contract conflict Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(policy): ignore advisor provenance during contract inference Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * test(policy): clarify advisor overlay provenance expectations Signed-off-by: John Myers <johntmyers@users.noreply.github.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Co-authored-by: Piotr Mlocek <pmlocek@nvidia.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
1 parent f54a7a6 commit 7ceea96

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

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

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ pub fn canonicalize_advisor_add_rule(
4545
})
4646
.cloned()
4747
.map(|mut endpoint| {
48-
// This marker is derived from provider/credential context by the
49-
// gateway and must never be persisted from an advisor proposal.
48+
// Provenance does not change the endpoint contract. The gateway
49+
// derives the credential marker, and the advisor marker records
50+
// where a persisted endpoint came from.
5051
endpoint.provider_credentialed = false;
52+
endpoint.advisor_proposed = false;
5153
// A denial observes one binary-to-port authorization. Preserve the
5254
// existing inspection contract, but never copy sibling ports from
5355
// a multi-port endpoint into the proposal.
@@ -84,6 +86,7 @@ pub fn canonicalize_advisor_add_rule(
8486
.any(|endpoint| {
8587
let mut normalized = endpoint.clone();
8688
normalized.provider_credentialed = false;
89+
normalized.advisor_proposed = false;
8790
normalize_endpoint(&mut normalized);
8891
normalized == contract
8992
})
@@ -92,6 +95,12 @@ pub fn canonicalize_advisor_add_rule(
9295
.collect::<Vec<_>>();
9396
sandbox_owners.sort();
9497

98+
let mut contract = contract;
99+
if sandbox_owners.is_empty() {
100+
// A provider-owned contract is mirrored into a new sandbox-owned
101+
// advisor overlay, so retain the incoming proposal provenance.
102+
contract.advisor_proposed = incoming_endpoint.advisor_proposed;
103+
}
95104
let target_name = sandbox_owners
96105
.first()
97106
.cloned()
@@ -2220,12 +2229,65 @@ mod tests {
22202229
assert_eq!(canonical.endpoints[0].protocol, "rest");
22212230
assert_eq!(canonical.endpoints[0].access, "read-only");
22222231
assert!(!canonical.endpoints[0].provider_credentialed);
2232+
assert!(canonical.endpoints[0].advisor_proposed);
22232233
assert_eq!(
22242234
effective.network_policies["_provider_example"].endpoints[0],
22252235
provider_endpoint
22262236
);
22272237
}
22282238

2239+
#[test]
2240+
fn canonicalize_advisor_ignores_endpoint_provenance_when_inferring_contract() {
2241+
let mut provider_endpoint = endpoint("api.example.com", 443);
2242+
provider_endpoint.protocol = "rest".to_string();
2243+
provider_endpoint.enforcement = "enforce".to_string();
2244+
provider_endpoint.access = "read-only".to_string();
2245+
provider_endpoint.provider_credentialed = true;
2246+
2247+
let mut advisor_endpoint = provider_endpoint.clone();
2248+
advisor_endpoint.provider_credentialed = false;
2249+
advisor_endpoint.advisor_proposed = true;
2250+
2251+
let mut base = SandboxPolicy::default();
2252+
base.network_policies.insert(
2253+
"existing_advisor".to_string(),
2254+
NetworkPolicyRule {
2255+
name: "existing-advisor".to_string(),
2256+
endpoints: vec![advisor_endpoint],
2257+
binaries: vec![advisor_binary("/usr/bin/curl")],
2258+
},
2259+
);
2260+
2261+
let mut effective = base.clone();
2262+
effective.network_policies.insert(
2263+
"_provider_example".to_string(),
2264+
NetworkPolicyRule {
2265+
name: "provider-example".to_string(),
2266+
endpoints: vec![provider_endpoint],
2267+
binaries: vec![binary("/usr/bin/gh")],
2268+
},
2269+
);
2270+
2271+
let incoming = NetworkPolicyRule {
2272+
name: "advisor_example".to_string(),
2273+
endpoints: vec![NetworkEndpoint {
2274+
host: "api.example.com".to_string(),
2275+
port: 443,
2276+
advisor_proposed: true,
2277+
..Default::default()
2278+
}],
2279+
binaries: vec![advisor_binary("/usr/bin/python")],
2280+
};
2281+
2282+
let (rule_name, canonical) =
2283+
canonicalize_advisor_add_rule(&base, &effective, "advisor_example", &incoming)
2284+
.expect("provenance alone must not create multiple endpoint contracts");
2285+
2286+
assert_eq!(rule_name, "existing_advisor");
2287+
assert_eq!(canonical.endpoints[0].protocol, "rest");
2288+
assert_eq!(canonical.endpoints[0].access, "read-only");
2289+
}
2290+
22292291
#[test]
22302292
fn canonicalize_advisor_narrows_multi_port_contract_and_keeps_overlay() {
22312293
let mut existing_endpoint = endpoint("index.crates.io", 443);

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12973,7 +12973,10 @@ mod tests {
1297312973
let canonical = chunk.proposed_rule.as_ref().unwrap();
1297412974
assert_eq!(canonical.endpoints[0].protocol, "rest");
1297512975
assert_eq!(canonical.endpoints[0].access, "read-only");
12976-
assert!(!canonical.endpoints[0].advisor_proposed);
12976+
assert!(
12977+
canonical.endpoints[0].advisor_proposed,
12978+
"a new advisor overlay must retain proposal provenance"
12979+
);
1297712980

1297812981
let revision = state
1297912982
.store
@@ -12994,6 +12997,10 @@ mod tests {
1299412997
assert_eq!(curl_rule.endpoints[0].ports, vec![443]);
1299512998
assert_eq!(curl_rule.endpoints[0].protocol, "rest");
1299612999
assert_eq!(curl_rule.endpoints[0].access, "read-only");
13000+
assert!(
13001+
curl_rule.endpoints[0].advisor_proposed,
13002+
"the persisted advisor overlay must retain proposal provenance"
13003+
);
1299713004
assert_eq!(curl_rule.binaries.len(), 1);
1299813005
assert_eq!(curl_rule.binaries[0].path, "/usr/bin/curl");
1299913006
}

0 commit comments

Comments
 (0)