-
Notifications
You must be signed in to change notification settings - Fork 38
feat: parse profiles in the core, and have the Python SDK stop parsing #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -179,6 +179,12 @@ pub fn http_acl_check( | |
| /// allowed to reach the original destination on the intercepted ports. Concrete | ||
| /// HTTP rule hosts tighten the IP allowlist to those hosts; wildcard hosts or | ||
| /// explicit HTTP ports with no rules allow any IP on the HTTP ports. | ||
| /// | ||
| /// Derived entries a caller already carries are not added twice. A policy can | ||
| /// be taken apart and rebuilt (`sandlock run --profile-file` rebuilds a builder | ||
| /// from the parsed profile, then applies flag overrides on top), and the | ||
| /// rebuilt net allowlist arrives here already holding the entries this | ||
| /// function added on the first build. | ||
| pub(crate) fn extend_net_allow_for_http( | ||
| net_allow: &mut Vec<NetAllow>, | ||
| http_allow: &[HttpRule], | ||
|
|
@@ -189,6 +195,12 @@ pub(crate) fn extend_net_allow_for_http( | |
| return; | ||
| } | ||
|
|
||
| fn push_unique(net_allow: &mut Vec<NetAllow>, rule: NetAllow) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CONFIRMED] Exact-equality dedup only handles the unchanged-inputs rebuild; when derivation inputs differ between passes, pass-1 residue survives as grants nothing requested, wider than a single-pass build. Profile with bare |
||
| if !net_allow.contains(&rule) { | ||
| net_allow.push(rule); | ||
| } | ||
| } | ||
|
|
||
| let mut wildcard_seen = false; | ||
| let mut concrete_hosts: Vec<String> = Vec::new(); | ||
| for rule in http_allow.iter().chain(http_deny.iter()) { | ||
|
|
@@ -203,7 +215,7 @@ pub(crate) fn extend_net_allow_for_http( | |
| } | ||
|
|
||
| if wildcard_seen || (http_allow.is_empty() && http_deny.is_empty()) { | ||
| net_allow.push(NetAllow { | ||
| push_unique(net_allow, NetAllow { | ||
| protocol: Protocol::Tcp, | ||
| target: NetTarget::AnyIp, | ||
| ports: http_ports.to_vec(), | ||
|
|
@@ -212,7 +224,7 @@ pub(crate) fn extend_net_allow_for_http( | |
| } | ||
|
|
||
| for host in concrete_hosts { | ||
| net_allow.push(NetAllow { | ||
| push_unique(net_allow, NetAllow { | ||
| protocol: Protocol::Tcp, | ||
| target: NetTarget::Host(host), | ||
| ports: http_ports.to_vec(), | ||
|
|
@@ -481,6 +493,29 @@ mod tests { | |
| assert_eq!(net_allow[1].ports, vec![80, 443]); | ||
| } | ||
|
|
||
| #[test] | ||
| fn extend_net_allow_for_http_is_idempotent() { | ||
| // A policy that is taken apart and rebuilt feeds the already derived | ||
| // entries back in as plain net-allow specs (that is what | ||
| // `sandlock run --profile-file` does before applying flag overrides), | ||
| // so a second pass must not grow the allowlist. | ||
| let allow = vec![HttpRule::parse("GET api.example.com/v1/*").unwrap()]; | ||
| let mut net_allow = Vec::new(); | ||
|
|
||
| extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]); | ||
| let first = net_allow.clone(); | ||
| extend_net_allow_for_http(&mut net_allow, &allow, &[], &[80]); | ||
|
|
||
| assert_eq!(net_allow, first); | ||
|
|
||
| // Same for the any-IP entry, which comes from a different branch. | ||
| let mut wide = Vec::new(); | ||
| extend_net_allow_for_http(&mut wide, &[], &[], &[8080]); | ||
| let first_wide = wide.clone(); | ||
| extend_net_allow_for_http(&mut wide, &[], &[], &[8080]); | ||
| assert_eq!(wide, first_wide); | ||
| } | ||
|
|
||
| #[test] | ||
| fn extend_net_allow_for_http_adds_any_ip_for_wildcard_or_bare_port() { | ||
| let mut net_allow = Vec::new(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CONFIRMED] A deny-only profile with any
[http]rules still fails the CLI rebuild with a spurious "mutually exclusive" error; this comment claims the round-trip is safe, but the failure fires before this function runs.Profile with
[network] deny = ["1.2.3.4"]plus[http]ports/rules:build()runs the allow/deny exclusivity check (builder.rs:976) beforeextend_net_allow_for_http(builder.rs:992) derives at least one NetAllow entry, so the first build passes and the resulting Sandbox has both lists non-empty. The CLI rebuild (main.rs:502-506) feeds the derived entry back as a user spec; on the secondbuild()the exclusivity check sees allow+deny both non-empty and errors with zero override flags.push_uniquecannot help because the error fires before extend runs, yet this doc comment and the new idempotency test assert the taken-apart-and-rebuilt flow is handled.