fix(sandbox): trust exact declared private endpoints (#1560)

* fix(sandbox): trust exact declared private endpoints

* fix(sandbox): preserve advisor endpoint provenance

* fix(sandbox): repair advisor provenance lint failures

---------

Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com>

Author: mjamiv

crates/openshell-policy/src/lib.rs

@@ -328,6 +328,9 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy {
                             allow_encoded_slash: e.allow_encoded_slash,
                             websocket_credential_rewrite: e.websocket_credential_rewrite,
                             request_body_credential_rewrite: e.request_body_credential_rewrite,
+                            // Advisor provenance is internal runtime state, not
+                            // a user-authored policy schema field.
+                            advisor_proposed: false,
                             persisted_queries: e.persisted_queries,
                             graphql_persisted_queries: e
                                 .graphql_persisted_queries

crates/openshell-policy/src/merge.rs

@@ -537,6 +537,7 @@ fn merge_endpoint(
     existing.allow_encoded_slash |= incoming.allow_encoded_slash;
     existing.websocket_credential_rewrite |= incoming.websocket_credential_rewrite;
     existing.request_body_credential_rewrite |= incoming.request_body_credential_rewrite;
+    existing.advisor_proposed |= incoming.advisor_proposed;
     normalize_endpoint(existing);
     Ok(())
 }
@@ -723,6 +724,12 @@ fn expand_access_preset(protocol: &str, access: &str) -> Option<Vec<L7Rule>> {
 fn append_unique_binaries(existing: &mut Vec<NetworkBinary>, incoming: &[NetworkBinary]) {
     let mut seen: HashSet<String> = existing.iter().map(|binary| binary.path.clone()).collect();
     for binary in incoming {
+        if let Some(existing_binary) = existing.iter_mut().find(|item| item.path == binary.path) {
+            if !is_advisor_proposed_binary(binary) {
+                mark_user_declared_binary(existing_binary);
+            }
+            continue;
+        }
         if seen.insert(binary.path.clone()) {
             existing.push(binary.clone());
         }
@@ -778,8 +785,30 @@ fn dedup_strings(values: &mut Vec<String>) {
 }
 
 fn dedup_binaries(values: &mut Vec<NetworkBinary>) {
-    let mut seen = HashSet::new();
-    values.retain(|binary| seen.insert(binary.path.clone()));
+    let mut deduped: Vec<NetworkBinary> = Vec::with_capacity(values.len());
+    for binary in std::mem::take(values) {
+        if let Some(existing) = deduped.iter_mut().find(|item| item.path == binary.path) {
+            if !is_advisor_proposed_binary(&binary) {
+                mark_user_declared_binary(existing);
+            }
+        } else {
+            deduped.push(binary);
+        }
+    }
+    *values = deduped;
+}
+
+fn is_advisor_proposed_binary(binary: &NetworkBinary) -> bool {
+    #[allow(deprecated)]
+    let advisor_proposed = binary.harness;
+    advisor_proposed
+}
+
+fn mark_user_declared_binary(binary: &mut NetworkBinary) {
+    #[allow(deprecated)]
+    {
+        binary.harness = false;
+    }
 }
 
 fn dedup_l7_rules(values: &mut Vec<L7Rule>) {
@@ -878,6 +907,18 @@ mod tests {
         }
     }
 
+    fn advisor_binary(path: &str) -> NetworkBinary {
+        let mut binary = NetworkBinary {
+            path: path.to_string(),
+            ..Default::default()
+        };
+        #[allow(deprecated)]
+        {
+            binary.harness = true;
+        }
+        binary
+    }
+
     fn rest_rule(method: &str, path: &str) -> L7Rule {
         L7Rule {
             allow: Some(L7Allow {
@@ -949,6 +990,131 @@ mod tests {
         assert_eq!(rule.binaries.len(), 2);
     }
 
+    #[test]
+    fn add_rule_user_binary_clears_advisor_marker_for_same_path() {
+        let mut policy = restrictive_default_policy();
+        policy.network_policies.insert(
+            "existing".to_string(),
+            NetworkPolicyRule {
+                name: "existing".to_string(),
+                endpoints: vec![endpoint("api.github.com", 443)],
+                binaries: vec![advisor_binary("/usr/bin/curl")],
+            },
+        );
+
+        let incoming = NetworkPolicyRule {
+            name: "incoming".to_string(),
+            endpoints: vec![endpoint("api.github.com", 443)],
+            binaries: vec![NetworkBinary {
+                path: "/usr/bin/curl".to_string(),
+                ..Default::default()
+            }],
+        };
+
+        let result = merge_policy(
+            policy,
+            &[PolicyMergeOp::AddRule {
+                rule_name: "existing".to_string(),
+                rule: incoming,
+            }],
+        )
+        .expect("merge should succeed");
+
+        let rule = &result.policy.network_policies["existing"];
+        assert_eq!(rule.binaries.len(), 1);
+        #[allow(deprecated)]
+        {
+            assert!(!rule.binaries[0].harness);
+        }
+    }
+
+    #[test]
+    fn add_rule_duplicate_binaries_prefer_user_declared_marker() {
+        let incoming = NetworkPolicyRule {
+            name: "incoming".to_string(),
+            endpoints: vec![endpoint("api.github.com", 443)],
+            binaries: vec![
+                advisor_binary("/usr/bin/curl"),
+                NetworkBinary {
+                    path: "/usr/bin/curl".to_string(),
+                    ..Default::default()
+                },
+            ],
+        };
+
+        let result = merge_policy(
+            restrictive_default_policy(),
+            &[PolicyMergeOp::AddRule {
+                rule_name: "github".to_string(),
+                rule: incoming,
+            }],
+        )
+        .expect("merge should succeed");
+
+        let rule = &result.policy.network_policies["github"];
+        assert_eq!(rule.binaries.len(), 1);
+        #[allow(deprecated)]
+        {
+            assert!(!rule.binaries[0].harness);
+        }
+    }
+
+    #[test]
+    fn add_rule_preserves_advisor_endpoint_marker_when_binary_is_deduped() {
+        let mut policy = restrictive_default_policy();
+        policy.network_policies.insert(
+            "app-api".to_string(),
+            NetworkPolicyRule {
+                name: "app-api".to_string(),
+                endpoints: vec![endpoint("api.example.com", 443)],
+                binaries: vec![NetworkBinary {
+                    path: "/usr/bin/python".to_string(),
+                    ..Default::default()
+                }],
+            },
+        );
+
+        let incoming = NetworkPolicyRule {
+            name: "app-api".to_string(),
+            endpoints: vec![NetworkEndpoint {
+                host: "internal-admin.local".to_string(),
+                port: 443,
+                ports: vec![443],
+                advisor_proposed: true,
+                ..Default::default()
+            }],
+            binaries: vec![advisor_binary("/usr/bin/python")],
+        };
+
+        let result = merge_policy(
+            policy,
+            &[PolicyMergeOp::AddRule {
+                rule_name: "app-api".to_string(),
+                rule: incoming,
+            }],
+        )
+        .expect("merge should succeed");
+
+        let rule = &result.policy.network_policies["app-api"];
+        assert_eq!(rule.binaries.len(), 1, "binary should still dedupe");
+        #[allow(deprecated)]
+        {
+            assert!(
+                !rule.binaries[0].harness,
+                "existing user binary provenance should be retained"
+            );
+        }
+        let internal_endpoint = rule
+            .endpoints
+            .iter()
+            .find(|endpoint| endpoint.host == "internal-admin.local")
+            .expect("advisor endpoint should be appended");
+        assert!(
+            internal_endpoint.advisor_proposed,
+            "endpoint provenance must survive merge even when binary provenance is deduped"
+        );
+    }
+
     #[test]
     fn add_rule_merges_websocket_credential_rewrite_flag() {
         let mut policy = restrictive_default_policy();

crates/openshell-providers/src/profiles.rs

@@ -587,6 +587,7 @@ fn endpoint_to_proto(endpoint: &EndpointProfile) -> NetworkEndpoint {
         allow_encoded_slash: endpoint.allow_encoded_slash,
         websocket_credential_rewrite: endpoint.websocket_credential_rewrite,
         request_body_credential_rewrite: endpoint.request_body_credential_rewrite,
+        advisor_proposed: false,
         persisted_queries: endpoint.persisted_queries.clone(),
         graphql_persisted_queries: endpoint
             .graphql_persisted_queries

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

@@ -161,6 +161,32 @@ binary_allowed(policy, exec) if {
 	glob.match(b.path, ["/"], p)
 }
 
+user_declared_binary_allowed(policy, exec) if {
+	some b
+	b := policy.binaries[_]
+	not object.get(b, "advisor_proposed", false)
+	not contains(b.path, "*")
+	b.path == exec.path
+}
+
+user_declared_binary_allowed(policy, exec) if {
+	some b
+	b := policy.binaries[_]
+	not object.get(b, "advisor_proposed", false)
+	not contains(b.path, "*")
+	ancestor := exec.ancestors[_]
+	b.path == ancestor
+}
+
+user_declared_binary_allowed(policy, exec) if {
+	some b in policy.binaries
+	not object.get(b, "advisor_proposed", false)
+	contains(b.path, "*")
+	all_paths := array.concat([exec.path], exec.ancestors)
+	some p in all_paths
+	glob.match(b.path, ["/"], p)
+}
+
 # --- Network action (allow / deny) ---
 #
 # These rules are mutually exclusive by construction:
@@ -647,6 +673,22 @@ matched_endpoint_config := _matching_endpoint_configs[0] if {
 	count(_matching_endpoint_configs) > 0
 }
 
+_policy_has_exact_declared_endpoint(policy) if {
+	some ep
+	ep := policy.endpoints[_]
+	not object.get(ep, "advisor_proposed", false)
+	not contains(ep.host, "*")
+	lower(ep.host) == lower(input.network.host)
+	ep.ports[_] == input.network.port
+}
+
+exact_declared_endpoint_host if {
+	some pname
+	policy := data.network_policies[pname]
+	user_declared_binary_allowed(policy, input.exec)
+	_policy_has_exact_declared_endpoint(policy)
+}
+
 # Hosted endpoint: exact host match + port in ports list.
 endpoint_matches_request(ep, network) if {
 	not contains(ep.host, "*")

crates/openshell-sandbox/src/mechanistic_mapper.rs

@@ -129,24 +129,33 @@ pub fn generate_proposals(summaries: &[DenialSummary]) -> Vec<PolicyChunk> {
                 protocol: "rest".to_string(),
                 enforcement: "enforce".to_string(),
                 rules: l7_rules,
+                advisor_proposed: true,
                 ..Default::default()
             }
         } else {
             NetworkEndpoint {
                 host: host.clone(),
                 port: *port,
                 ports: vec![*port],
+                advisor_proposed: true,
                 ..Default::default()
             }
         };
 
         let binaries: Vec<NetworkBinary> = if binary.is_empty() {
             vec![]
         } else {
-            vec![NetworkBinary {
+            let mut proposal_binary = NetworkBinary {
                 path: binary.clone(),
                 ..Default::default()
-            }]
+            };
+            // The deprecated harness bit is ignored by policy YAML, but OPA
+            // maps it to advisor_proposed to preserve the SSRF two-step flow.
+            #[allow(deprecated)]
+            {
+                proposal_binary.harness = true;
+            }
+            vec![proposal_binary]
         };
 
         let proposed_rule = NetworkPolicyRule {
@@ -500,6 +509,10 @@ mod tests {
         assert_eq!(rule.endpoints[0].port, 443);
         assert_eq!(rule.binaries.len(), 1);
         assert_eq!(rule.binaries[0].path, "/usr/bin/curl");
+        #[allow(deprecated)]
+        {
+            assert!(rule.binaries[0].harness);
+        }
 
         // No L7 fields when no samples provided.
         assert!(rule.endpoints[0].protocol.is_empty());