Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions architecture/security-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,21 +281,23 @@ After any successful policy write, pending chunks already covered by the new
live effective policy are rejected as redundant. This keeps the review inbox
aligned with what the sandbox currently enforces.

Endpoint and binary advisor markers are provenance, not authorization or
connection metadata. Provider- or user-authored declarations carry explicit
provenance; `policy.local` declarations carry advisor provenance. A difference
in endpoint provenance alone is compatible during effective-policy ambiguity
validation. When identical endpoint or binary identities merge, an explicit
declaration dominates an advisor declaration. Proposal coverage likewise
ignores provenance so an approved overlay converges when an existing explicit
declaration already supplies the same identity.
Endpoint advisor markers are provenance, not authorization or connection
metadata. Provider- or user-authored endpoints carry explicit provenance;
`policy.local` endpoints carry advisor provenance. A difference in endpoint
provenance alone is compatible during effective-policy ambiguity validation.
When identical endpoints merge, an explicit declaration dominates an advisor
declaration. Proposal coverage likewise ignores provenance so an approved
overlay converges when an existing explicit declaration already supplies the
same identity.

This compatibility does not weaken SSRF classification. Exact-host trust
requires one matching rule to contain both an exact explicit endpoint and an
explicit binary identity. An advisor-only endpoint or binary cannot assemble
that trust from unrelated rules. A provider rule may independently establish
trust for its own explicit endpoint and binary pair, but an advisor overlay
does not broaden that pair to a different binary.
requires one matching rule to contain both an exact explicit endpoint and a
matching binary identity. An advisor endpoint cannot assemble that trust from
an unrelated explicit endpoint. When the advisor observes a new binary for an
existing explicit endpoint contract, canonicalization keeps the observation in
a separate rule whose endpoint retains advisor provenance. A provider or user
rule may independently establish trust for its own explicit endpoint and binary
pair, but an advisor overlay does not broaden that pair to a different binary.

### Security-notes gate

Expand Down
5 changes: 1 addition & 4 deletions crates/openshell-cli/src/policy_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ pub fn build_policy_update_plan(
endpoints: vec![endpoint.clone()],
binaries: deduped_binaries
.iter()
.map(|path| NetworkBinary {
path: path.clone(),
..Default::default()
})
.map(|path| NetworkBinary { path: path.clone() })
.collect(),
};
merge_operations.push(PolicyMergeOperation {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2551,7 +2551,6 @@ binaries: [/usr/bin/yaml-client]
}

#[tokio::test]
#[allow(deprecated)]
async fn provider_profile_import_preserves_advanced_network_policy_fields() {
let ts = run_server().await;
let dir = tempfile::tempdir().unwrap();
Expand Down Expand Up @@ -2580,7 +2579,6 @@ endpoints:
path: /v1
binaries:
- path: /usr/bin/advanced
harness: true
",
)
.unwrap();
Expand Down Expand Up @@ -2609,7 +2607,7 @@ binaries:
assert_eq!(endpoint.allowed_ips, vec!["10.0.0.0/24"]);
assert!(endpoint.allow_encoded_slash);
assert_eq!(endpoint.path, "/v1");
assert!(profile.binaries[0].harness);
assert_eq!(profile.binaries[0].path, "/usr/bin/advanced");
}

#[tokio::test]
Expand Down
90 changes: 90 additions & 0 deletions crates/openshell-core/src/proto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,93 @@ pub use middleware::v1::*;
pub use openshell::*;
pub use sandbox::v1::*;
pub use test::ObjectForTest;

#[cfg(test)]
mod tests {
use std::collections::HashMap;

use prost::Message;

use super::SandboxPolicy;

#[derive(Clone, PartialEq, Message)]
struct LegacyNetworkBinary {
#[prost(string, tag = "1")]
path: String,
#[prost(bool, tag = "2")]
harness: bool,
}

#[derive(Clone, PartialEq, Message)]
struct LegacyNetworkPolicyRule {
#[prost(message, repeated, tag = "3")]
binaries: Vec<LegacyNetworkBinary>,
}

#[derive(Clone, PartialEq, Message)]
struct LegacySandboxPolicy {
#[prost(map = "string, message", tag = "5")]
network_policies: HashMap<String, LegacyNetworkPolicyRule>,
}

#[test]
fn sandbox_policy_ignores_removed_network_binary_harness_wire_field() {
let legacy = LegacySandboxPolicy {
network_policies: HashMap::from([(
"legacy".to_string(),
LegacyNetworkPolicyRule {
binaries: vec![LegacyNetworkBinary {
path: "/usr/bin/curl".to_string(),
harness: true,
}],
},
)]),
};

let decoded = SandboxPolicy::decode(legacy.encode_to_vec().as_slice())
.expect("legacy policy should decode");
assert_eq!(
decoded.network_policies["legacy"].binaries[0].path,
"/usr/bin/curl"
);

let round_tripped =
LegacySandboxPolicy::decode(decoded.encode_to_vec().as_slice()).unwrap();
assert!(!round_tripped.network_policies["legacy"].binaries[0].harness);
}

#[test]
fn network_binary_reserves_removed_harness_name_and_tag() {
let descriptor = prost_types::FileDescriptorSet::decode(crate::FILE_DESCRIPTOR_SET)
.expect("descriptor set should decode");
let network_binary = descriptor
.file
.iter()
.find(|file| file.package.as_deref() == Some("openshell.sandbox.v1"))
.and_then(|file| {
file.message_type
.iter()
.find(|message| message.name.as_deref() == Some("NetworkBinary"))
})
.expect("NetworkBinary descriptor should exist");

assert!(
network_binary
.field
.iter()
.all(|field| field.name.as_deref() != Some("harness"))
);
assert!(
network_binary
.reserved_range
.iter()
.any(|range| range.start == Some(2) && range.end == Some(3))
);
assert!(
network_binary
.reserved_name
.iter()
.any(|name| name == "harness")
);
}
}
1 change: 0 additions & 1 deletion crates/openshell-driver-mxc/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ mod tests {
}],
binaries: vec![NetworkBinary {
path: "/usr/bin/curl".into(),
..Default::default()
}],
},
);
Expand Down
3 changes: 0 additions & 3 deletions crates/openshell-driver-mxc/tests/policy_mapper_matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,11 +766,9 @@ fn b_binaries_error_per_binary() {
binaries: vec![
NetworkBinary {
path: "/usr/bin/curl".into(),
..Default::default()
},
NetworkBinary {
path: "/usr/bin/wget".into(),
..Default::default()
},
],
},
Expand Down Expand Up @@ -1248,7 +1246,6 @@ fn handled_fields_inventory() {
endpoints: vec![full_ep, single_port_ep],
binaries: vec![NetworkBinary {
path: "/usr/bin/curl".into(),
..Default::default()
}],
},
);
Expand Down
2 changes: 0 additions & 2 deletions crates/openshell-policy/src/ambiguity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ mod tests {
endpoints: vec![left],
binaries: vec![NetworkBinary {
path: "/usr/bin/curl".to_string(),
..Default::default()
}],
},
);
Expand All @@ -737,7 +736,6 @@ mod tests {
endpoints: vec![right],
binaries: vec![NetworkBinary {
path: "/usr/bin/bash".to_string(),
..Default::default()
}],
},
);
Expand Down
32 changes: 23 additions & 9 deletions crates/openshell-policy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,6 @@ struct L7DenyRuleDef {
#[serde(deny_unknown_fields)]
struct NetworkBinaryDef {
path: String,
/// Deprecated: ignored. Kept for backward compat with existing YAML files.
#[serde(default, skip_serializing)]
#[allow(dead_code)]
harness: bool,
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -897,10 +893,7 @@ fn to_proto(raw: PolicyFile) -> Result<SandboxPolicy> {
binaries: rule
.binaries
.into_iter()
.map(|b| NetworkBinary {
path: b.path,
..Default::default()
})
.map(|b| NetworkBinary { path: b.path })
.collect(),
};
(key, proto_rule)
Expand Down Expand Up @@ -1053,7 +1046,6 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
.iter()
.map(|b| NetworkBinaryDef {
path: b.path.clone(),
harness: false,
})
.collect(),
};
Expand Down Expand Up @@ -4875,6 +4867,28 @@ network_policies:
assert!(parse_sandbox_policy(yaml).is_err());
}

#[test]
fn parse_rejects_removed_network_binary_harness_field() {
let yaml = r"
version: 1
network_policies:
legacy:
endpoints:
- host: example.com
port: 443
binaries:
- path: /usr/bin/curl
harness: true
";

let error = parse_sandbox_policy(yaml).expect_err("removed harness field must be rejected");
let error_debug = format!("{error:?}");
assert!(
error_debug.contains("unknown field `harness`"),
"unexpected error: {error_debug}"
);
}

#[test]
fn rejects_port_above_65535() {
let yaml = r"
Expand Down
Loading
Loading