From 7e01c740f7248e2c13df1ac735705cd0a1d6d0f2 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Wed, 5 Aug 2026 16:12:53 -0300 Subject: [PATCH 1/3] docs(seatbelt): correct rule-precedence and listen() rationale Follow-up to #749. Comments and docs only; no behavior change, no generated profile changes. Three claims were wrong, all measured against sandbox-exec: 1. "An allow can never take authority back from an earlier rule." False - a later filtered allow does override an earlier filtered deny, which is exactly what the shallow-to-deep emission relies on. The actual reason a read-only path needs an explicit (deny file-write* network-bind network-outbound ...) is narrower: the read-only allow names only file-read*, so it says nothing about write or socket operations and cannot displace a broader grant. 2. "deniedPaths is emitted after the network rules so it also overrides the unfiltered (allow network-outbound)." The override is real, but it does not depend on that ordering - an unfiltered rule never overrides a path-filtered one. Stating a constraint that does not exist invites someone to "fix" the ordering on a false premise. 3. "network-inbound is required in addition to network-bind before the kernel will accept listen()." Backwards. network-inbound (local ip) alone -> LISTEN_OK network-bind (local ip) alone -> bind OK, LISTEN_DENIED (EPERM) neither -> BIND_DENIED (EPERM) network-inbound governs listen() and covers the bind(); it is network-bind that is insufficient on its own. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 184c815b-d8ac-4bad-8bac-c18ab420e1b7 Signed-off-by: Carlos Alexandro Becker --- docs/macos-support/seatbelt-backend.md | 15 ++++++------ .../seatbelt/common/src/profile_builder.rs | 24 ++++++++++--------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/docs/macos-support/seatbelt-backend.md b/docs/macos-support/seatbelt-backend.md index fa45fe5e9..16252553f 100644 --- a/docs/macos-support/seatbelt-backend.md +++ b/docs/macos-support/seatbelt-backend.md @@ -189,13 +189,14 @@ no filter will not override an earlier path-scoped deny. per path, using the same ordering the Linux backends apply (`wxc_common::filesystem_resolve`). Last-match-wins then makes the *deepest* intent win at every path, so a read-only entry nested inside a broader -read-write subtree stays read-only. Because an `allow` can never take authority -back from an earlier rule, each read-only path also emits an explicit -`(deny file-write* network-bind network-outbound …)`. +read-write subtree stays read-only. That last part needs an explicit +`(deny file-write* network-bind network-outbound …)` per read-only path, +because the read-only `allow` only names `file-read*` — it says nothing about +write or socket operations, so on its own it cannot displace a broader grant. -`deniedPaths` is not part of that plan — it is emitted after the network rules -so it also overrides the unfiltered `(allow network-outbound)`, which makes it -win outright regardless of depth. +`deniedPaths` is not part of that plan. It is emitted last so it outranks the +filtered allows above regardless of depth. Its position relative to the network +rules does not matter: as noted above, an unfiltered allow cannot override it. #### Path resolution @@ -282,7 +283,7 @@ independently of the profile. |---|---| | `defaultPolicy: "block"` | No `(allow network-outbound)` is emitted; the baseline `(deny default)` then blocks all IP sockets. | | `defaultPolicy: "allow"` (no host list) | `(allow network-outbound)` plus `(allow network-bind (local ip))` and `(allow system-socket)`. | -| `allowLocalNetwork: true` | `(allow network-inbound (local ip))` — required in addition to `network-bind` before the kernel will accept `listen()` on an IP socket. Independent of `defaultPolicy`, and unrelated to AF_UNIX sockets (see above). | +| `allowLocalNetwork: true` | `(allow network-inbound (local ip))` — on its own this is what lets a process `listen()` on a local address; it covers the `bind()` too. (`network-bind (local ip)` alone is *not* enough: `bind()` succeeds and `listen()` is denied.) Independent of `defaultPolicy`, and unrelated to AF_UNIX sockets (see above). | | `allowedHosts` | Accepted for SDK compatibility, but Seatbelt cannot filter DNS names; the profile degrades to allow-all outbound as best-effort. | | `blockedHosts` | Rejected during validation because Seatbelt cannot enforce hostname blocks. | | `proxy` (loopback: `localhost` / `builtinTestServer`) | Under `defaultPolicy: "block"`, allows only the resolved `localhost:`. Other loopback services and the wider network remain blocked. Under `allow`, the existing allow-all covers it. | diff --git a/src/backends/seatbelt/common/src/profile_builder.rs b/src/backends/seatbelt/common/src/profile_builder.rs index 9a791954a..baeaa6838 100644 --- a/src/backends/seatbelt/common/src/profile_builder.rs +++ b/src/backends/seatbelt/common/src/profile_builder.rs @@ -200,12 +200,12 @@ fn write_filesystem_allow(out: &mut String, paths: &ResolvedPaths) { // Emit shallow-to-deep, one rule per path, using the same ordering the // Linux backends apply (`wxc_common::filesystem_resolve`). Seatbelt is - // last-match-wins, so ordering by depth makes the *deepest* intent win at - // every path — a `readonlyPaths` entry nested inside a broader - // `readwritePaths` subtree stays read-only rather than inheriting the - // parent's write grant. `deniedPaths` is deliberately not part of this - // plan: it is emitted after the network rules so it also overrides the - // unfiltered `(allow network-outbound)`, which makes it win outright. + // last-match-wins between rules that carry a filter, so ordering by depth + // makes the *deepest* intent win at every path — a `readonlyPaths` entry + // nested inside a broader `readwritePaths` subtree stays read-only rather + // than inheriting the parent's write grant. `deniedPaths` is deliberately + // not part of this plan: it is emitted last so it outranks these filtered + // allows regardless of depth. out.push_str(";; --- policy.readonlyPaths / policy.readwritePaths (shallow-to-deep) ---\n"); for mount in resolve_path_plan(&paths.readwrite, &paths.readonly, &[]) { let subpath = [mount.path.clone()]; @@ -863,8 +863,9 @@ mod tests { #[test] fn allow_local_network_emits_inbound_rule() { - // server.listen() on macOS needs `network-inbound` in addition to - // `network-bind` — the kernel rejects listen() with EPERM otherwise. + // server.listen() on macOS is governed by `network-inbound`, not + // `network-bind` — with only `network-bind (local ip)` the bind() + // succeeds and the kernel then rejects listen() with EPERM. let mut r = req(); r.policy.default_network_policy = NetworkPolicy::Allow; r.policy.allow_local_network = true; @@ -1096,9 +1097,10 @@ mod tests { #[test] fn nested_readonly_keeps_write_away_from_broader_readwrite() { // `/tmp` resolves to `/private/tmp`, which is an ancestor of the - // read-only entry. An `allow` cannot take authority back, so the - // read-only path must emit an explicit removal *after* the broader - // read-write allow. + // read-only entry. The read-only `allow` names only `file-read*`, so + // it says nothing about write or socket ops and cannot displace the + // broader grant on its own — hence the explicit removal, emitted + // *after* the read-write allow. let mut r = req(); r.policy.readwrite_paths = vec!["/tmp".into()]; r.policy.readonly_paths = vec!["/private/tmp/secret".into()]; From 0d5248bc6df427e9a71fc67dedca9fc11922ff75 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Wed, 5 Aug 2026 16:37:01 -0300 Subject: [PATCH 2/3] docs(seatbelt): explain the read-only strip at the site reviewers flagged Both review passes on #749 landed on the same line - the read-only network-outbound strip - and asked whether the later unfiltered (allow network-outbound) defeats it. It does not, but nothing at that site said so; the comment there only claimed "an allow never denies", which is the phrasing that fed the wrong model in the first place. Spell it out where the question gets asked: the deny survives because last-match-wins applies between rules that carry a filter, and an unfiltered rule does not override a path-filtered one. Points at the test that pins it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 184c815b-d8ac-4bad-8bac-c18ab420e1b7 Signed-off-by: Carlos Alexandro Becker --- src/backends/seatbelt/common/src/profile_builder.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/backends/seatbelt/common/src/profile_builder.rs b/src/backends/seatbelt/common/src/profile_builder.rs index baeaa6838..91c88ff15 100644 --- a/src/backends/seatbelt/common/src/profile_builder.rs +++ b/src/backends/seatbelt/common/src/profile_builder.rs @@ -224,9 +224,16 @@ fn write_filesystem_allow(out: &mut String, paths: &ResolvedPaths) { } FsIntent::ReadOnly => { write_path_rule(out, "allow file-read*", &subpath); - // The read allow alone cannot take write or socket authority - // back from a shallower read-write rule — an `allow` never - // denies — so the removal has to be explicit. + // The read allow names only `file-read*`, so it says nothing + // about write or socket ops and cannot displace a shallower + // read-write grant — the removal has to be explicit. + // + // This deny survives the unfiltered `(allow network-outbound)` + // that `write_network_rules` emits below under + // `defaultPolicy: "allow"`: last-match-wins applies between + // rules that carry a filter, and an unfiltered rule does not + // override a path-filtered one. Pinned by + // `readonly_socket_strip_survives_a_default_allow_outbound`. write_path_rule( out, "deny file-write* network-bind network-outbound", From f6fd56845ccd93ae087dca4f8b23ecaf71f4583b Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Wed, 5 Aug 2026 17:07:10 -0300 Subject: [PATCH 3/3] test(seatbelt): stop pinning order against the unfiltered outbound allow Review feedback: two tests still enforced the ordering constraint this PR documents as nonexistent - a path-filtered deny is not overridden by an unfiltered allow in either direction, so asserting the deny is emitted after (allow network-outbound) encodes a constraint that does not exist. - denied_paths_deny_outbound_after_unfiltered_allow -> denied_paths_deny_outbound_under_default_allow: keeps the real guarantee (a denied subtree still denies network-outbound under defaultPolicy: "allow") and drops the order assertion. - readonly_socket_strip_survives_a_default_allow_outbound: drops the same guard, which was added in #749 for the same wrong reason. The order assertions that remain are against *filtered* allows, where last-match-wins genuinely applies and the deny must come second: denied_paths_appear_after_allows_to_override and denied_paths_deny_unix_socket_ops_after_allows. Those stay. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 184c815b-d8ac-4bad-8bac-c18ab420e1b7 Signed-off-by: Carlos Alexandro Becker --- .../seatbelt/common/src/profile_builder.rs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/backends/seatbelt/common/src/profile_builder.rs b/src/backends/seatbelt/common/src/profile_builder.rs index 91c88ff15..229a53e9d 100644 --- a/src/backends/seatbelt/common/src/profile_builder.rs +++ b/src/backends/seatbelt/common/src/profile_builder.rs @@ -1006,19 +1006,22 @@ mod tests { } #[test] - fn denied_paths_deny_outbound_after_unfiltered_allow() { - // `defaultPolicy: allow` emits a bare `(allow network-outbound)`; the - // deny must come after it or a denied subtree's UNIX sockets stay - // connectable. + fn denied_paths_deny_outbound_under_default_allow() { + // `defaultPolicy: allow` emits a bare `(allow network-outbound)`, which + // on its own grants AF_UNIX `connect()`. A denied subtree must still + // deny it. Position relative to that unfiltered allow is deliberately + // not asserted: an unfiltered rule cannot override a path-filtered one + // in either direction, so pinning the order would encode a constraint + // that does not exist. The orderings that *do* matter — deny after the + // filtered read-write allows — are covered by + // `denied_paths_appear_after_allows_to_override` and + // `denied_paths_deny_unix_socket_ops_after_allows`. let mut r = req(); r.policy.default_network_policy = NetworkPolicy::Allow; r.policy.denied_paths = vec!["/tmp/secret".into()]; let p = build_profile(&r).unwrap(); - let allow_idx = p - .find("(allow network-outbound)") - .expect("unfiltered outbound allow"); let deny_idx = p.find(DENY_RULE).expect("deny must cover network-outbound"); - assert!(deny_idx > allow_idx); + assert!(p[deny_idx..].contains("(subpath \"/private/tmp/secret\")")); } #[test] @@ -1143,12 +1146,15 @@ mod tests { #[test] fn readonly_socket_strip_survives_a_default_allow_outbound() { // `defaultPolicy: "allow"` emits an unfiltered `(allow - // network-outbound)` *after* the filesystem section. Seatbelt does not - // let an unfiltered rule override a path-filtered one, so the - // read-only strip still governs AF_UNIX `connect()` under that - // subtree. Verified against `sandbox-exec`: with both rules present in - // this order, connecting to a pre-existing socket there is EPERM, - // while the same profile without the strip connects fine. + // network-outbound)`. The read-only strip still governs AF_UNIX + // `connect()` under that subtree, because an unfiltered rule does not + // override a path-filtered one. Verified end-to-end against + // `mxc-exec-mac` with a listener created outside the sandbox: this + // policy denies `connect()` with EPERM, while the same policy with the + // path moved to `readwrite_paths` connects. + // + // Emission order relative to the unfiltered allow is deliberately not + // asserted — it has no bearing on the outcome. let mut r = req(); r.policy.default_network_policy = NetworkPolicy::Allow; r.policy.readonly_paths = vec!["/tmp/ro".into()]; @@ -1156,10 +1162,6 @@ mod tests { let strip_idx = p.find(RO_STRIP).expect("read-only strip"); assert!(p[strip_idx..].contains("(subpath \"/private/tmp/ro\")")); - assert!( - p.find("(allow network-outbound)\n").expect("default allow") > strip_idx, - "this test is only meaningful while the unfiltered allow comes last, profile:\n{p}" - ); } #[test]