docs(seatbelt): correct rule-precedence and listen() rationale - #754
Merged
Soham Das (SohamDas2021) merged 3 commits intoAug 5, 2026
Merged
Conversation
Follow-up to microsoft#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 <caarlos0@users.noreply.github.com>
Carlos Alexandro Becker (caarlos0)
requested a review
from a team
as a code owner
August 5, 2026 19:13
Copilot started reviewing on behalf of
Carlos Alexandro Becker (caarlos0)
August 5, 2026 19:14
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Corrects Seatbelt documentation and code comments without changing generated profiles.
Changes:
- Clarifies filtered-rule precedence and
deniedPathsordering. - Corrects the rationale for read-only deny rules.
- Documents that
network-inboundgovernslisten().
Show a summary per file
| File | Description |
|---|---|
src/backends/seatbelt/common/src/profile_builder.rs |
Updates precedence and socket-operation comments. |
docs/macos-support/seatbelt-backend.md |
Corrects filesystem and network policy explanations. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
…gged Both review passes on microsoft#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 <caarlos0@users.noreply.github.com>
Copilot started reviewing on behalf of
Carlos Alexandro Becker (caarlos0)
August 5, 2026 19:37
View session
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 microsoft#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 <caarlos0@users.noreply.github.com>
Copilot started reviewing on behalf of
Carlos Alexandro Becker (caarlos0)
August 5, 2026 20:08
View session
Soham Das (SohamDas2021)
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.github/copilot-instructions.md.Summary
Follow-up to #749, which a post-merge Copilot review correctly flagged for describing Seatbelt's precedence rules inaccurately. Comments and docs only — no behavior change, and the generated profile is byte-for-byte identical.
Three claims were wrong. All three are corrected against measurements, not reasoning.
1. "An
allowcan never take authority back from an earlier rule"False, and it contradicts this backend's own design: the shallow-to-deep emission depends on a later filtered allow overriding an earlier filtered deny.
allowthendeny, same pathEPERMdenythenallow, same pathCONNECT_OKdeny, deeperallowCONNECT_OKThe real reason a read-only path still needs an explicit
(deny file-write* network-bind network-outbound …)is narrower: the read-only allow names onlyfile-read*, so it says nothing about write or socket operations and cannot displace a broader grant on its own.2. "
deniedPathsis 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, in either direction:
EPERMEPERMCONNECT_OK(control)The control confirms
(allow network-outbound)genuinely does grant AF_UNIXconnect(), so the first two rows are the deny winning, not a missing capability.This one matters beyond accuracy: documenting an ordering constraint that does not exist invites a future change to "fix" the emission order on a false premise.
3. "
network-inboundis required in addition tonetwork-bindbefore the kernel will acceptlisten()"Backwards.
(allow network-inbound (local ip))aloneLISTEN_OK(allow network-bind (local ip))alonebind()OK,listen()denied (EPERM)bind()denied (EPERM)network-inboundis what governslisten(), and it covers thebind()too. It isnetwork-bindthat is insufficient on its own. This also explains whyallowLocalNetworkworks underdefaultPolicy: "block", wherewrite_local_network_rulesemitsnetwork-inboundand nothing else.Follow-ups in this PR
Two things surfaced after the initial push, both extending claim 2 rather than adding new scope:
The strip site itself (
0d5248b). Both review passes on #749 — Copilot and a human — landed on the same line, the read-onlynetwork-outboundstrip, and asked whether the later unfiltered allow defeats it. Nothing at that site said otherwise; the comment there only claimed "anallownever denies", which is the phrasing that produced the wrong model. It now states the rule where the question actually gets asked, and names the test that pins it.Re-verified end-to-end with the real
mxc-exec-macagainst a listener created outside the sandbox, one field of config apart:Two tests still enforced the false constraint (
f6fd568, from review feedback). Asserting the deny is emitted after the unfiltered(allow network-outbound)encodes exactly the ordering this PR documents as nonexistent.denied_paths_deny_outbound_after_unfiltered_allow→denied_paths_deny_outbound_under_default_allow; keeps the real guarantee, drops the order assert.readonly_socket_strip_survives_a_default_allow_outboundhad the same guard, added in fix(seatbelt): allow AF_UNIX sockets and resolve symlinked root paths #749 for the same wrong reason. Dropped.The order assertions that remain are all against filtered allows, where last-match-wins genuinely applies and the deny must come second —
denied_paths_appear_after_allows_to_overrideanddenied_paths_deny_unix_socket_ops_after_allows. Those stay; that constraint is real.Validation
cargo test -p seatbelt_common→ 63 passed, 0 failedcargo clippy -p seatbelt_common --all-targets -- -D warnings→ cleancargo fmt --all -- --check→ cleanEvery table above was produced with
sandbox-execagainst a live listener on this host (macOS 26.5.2, arm64).Microsoft Reviewers: Open in CodeFlow