fix: honor allowRead over wildcard denyRead in permissive mode on macOS by reneleonhardt · Pull Request #218 · fencesandbox/fence

reneleonhardt · 2026-08-07T13:56:22Z

Problem

allowRead is a no-op in permissive mode: generateReadRules emits a blanket (allow file-read*) and never consults allowPaths; denyRead emits after it and wins on seatbelt; config.Merge has no deny-exclusion. With extends: "code" a user can't re-allow a template-denied path — e.g. ~/.gnupg/pubring.kbx under ~/.gnupg/**, blocking gpg signing (public keyring read):

$ git commit --gpg-sign -m "<message>"  # or -S
gpg: keyblock resource '~/.gnupg/pubring.kbx': Permission denied

Change

Permissive mode + denyRead present: emit (allow file-read-data) + (allow file-read-metadata) (subpath, or regex via GlobToRegex) for each user allowRead path, AFTER the deny loop. Both ops needed — deny file-read* covers data + metadata; a specific allow is not overridden by a wildcard deny (repo comment), so the explicit grant wins while the rest of the denied subtree stays blocked.

Skipped when no denyRead → deny-free profiles byte-identical. defaultDenyRead/strictDenyRead untouched. Docs: denyRead marked macOS-only (Linux read control is bind-mount visibility, no deny layer); allowRead platform semantics noted.

Addresses #122 ("No way to remove inherited deny entries when extending a template"): allowRead-beats-deny makes deny-exclusion syntax unnecessary instead of adding it. Per-path allowRead keeps the override explicit and minimal — an exclusion syntax would let a config weaken a template's deny globally.

Tests

Unit (always runs):
go test ./internal/sandbox/ -run 'TestGenerateReadRules_Permissive|TestMacOS_DefaultDenyRead|TestMacOS_DenyReadRegexFormat|TestMacOS_DefaultDenyReadPrecedenceRegression' -count=1
— allow-after-deny ordering, both ops, glob pair, dedup, no-deny no-op (regression guard); existing read-rule tests green.
Integration (darwin-tagged, real sandbox-exec, hermetic temp dir; skip inside nested sandbox):
go test ./internal/sandbox/ -run 'TestMacOS_SeatbeltAllowReadOverridesDenyRead' -count=1 -v
— allowRead path readable under deny dir/**, sibling blocked.
Runtime A/B (manual, same config): patched binary READ_OK, unpatched blocked.
go build ./..., go vet ./..., full go test ./... green.

Developed with carefully directed, manually reviewed AI assistance.

cubic-dev-ai

All reported issues were addressed across 4 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Review (PR fencesandbox#218, cubic): my earlier docs claimed denyRead is macOS-only and ignored on Linux — wrong. Verified in source: - Linux enforces denyRead via read masking (empty bind mount, linux_mount_planner.go) and denyRead wins over allowRead at the mount level (hasExplicitLinuxReadGrant returns false for denied paths). - macOS: seatbelt deny rule; allowRead re-allows a specific path inside a denyRead subtree (specific allow beats wildcard deny) — that override is macOS-specific. docs/configuration.md: denyRead row + platform note corrected. docs/library.md: reads semantics synced (Linux deny wins; macOS allowRead override).

reneleonhardt · 2026-08-07T15:27:41Z

Separate commits only for reviewability, use Squash and merge for clean history.

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

…nyRead-wins Second review (PR fencesandbox#218, cubic): the library.md override sentence sat in the preflight section, but CheckReadPath/CheckWritePath are platform-agnostic and keep denyRead always winning. Fixed: - docs/library.md: reads split into preflight+Linux wrap-mode (denyRead always wins) vs macOS wrap-mode seatbelt only (allowRead re-allows a denied subtree); preflight explicitly does NOT reflect the override. - internal/sandbox/path_check.go: CheckReadPath doc comment synced - it intentionally diverges from macOS wrap-mode (denyRead always wins there).

cubic-dev-ai

All reported issues were addressed across 6 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

Third review (PR fencesandbox#218, cubic, P3): the override sentence was unqualified — it only holds in permissive mode. With defaultDenyRead/strictDenyRead the deny is emitted as specific file-read-data/file-read-metadata ops, not wildcard file-read*, so the specific allow does not beat them and denyRead wins (the implementation already gates the override on !defaultDenyRead). Synced everywhere the override is described: - ARCHITECTURE.md (the bot's target, per its suggested wording) - docs/configuration.md platform note - docs/library.md macOS reads bullet (permissive-only; defaultDenyRead wins) - internal/sandbox/path_check.go + pkg/fence CheckReadPath comments - internal/config/config.go DenyRead description + docs/schema (mirror) - PR body docs line

Review (PR fencesandbox#218, cubic): my earlier docs claimed denyRead is macOS-only and ignored on Linux — wrong. Verified in source: - Linux enforces denyRead via read masking (empty bind mount, linux_mount_planner.go) and denyRead wins over allowRead at the mount level (hasExplicitLinuxReadGrant returns false for denied paths). - macOS: seatbelt deny rule; allowRead re-allows a specific path inside a denyRead subtree (specific allow beats wildcard deny) — that override is macOS-specific. docs/configuration.md: denyRead row + platform note corrected. docs/library.md: reads semantics synced (Linux deny wins; macOS allowRead override).

…nyRead-wins Second review (PR fencesandbox#218, cubic): the library.md override sentence sat in the preflight section, but CheckReadPath/CheckWritePath are platform-agnostic and keep denyRead always winning. Fixed: - docs/library.md: reads split into preflight+Linux wrap-mode (denyRead always wins) vs macOS wrap-mode seatbelt only (allowRead re-allows a denied subtree); preflight explicitly does NOT reflect the override. - internal/sandbox/path_check.go: CheckReadPath doc comment synced - it intentionally diverges from macOS wrap-mode (denyRead always wins there).

Third review (PR fencesandbox#218, cubic, P3): the override sentence was unqualified — it only holds in permissive mode. With defaultDenyRead/strictDenyRead the deny is emitted as specific file-read-data/file-read-metadata ops, not wildcard file-read*, so the specific allow does not beat them and denyRead wins (the implementation already gates the override on !defaultDenyRead). Synced everywhere the override is described: - ARCHITECTURE.md (the bot's target, per its suggested wording) - docs/configuration.md platform note - docs/library.md macOS reads bullet (permissive-only; defaultDenyRead wins) - internal/sandbox/path_check.go + pkg/fence CheckReadPath comments - internal/config/config.go DenyRead description + docs/schema (mirror) - PR body docs line

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…fencesandbox#218) Bot P1 (cubic, confidence 9): the permissive allowRead re-allow loop emitted only the user's path spelling, while the deny loop emits both /tmp,/var,/etc and /private/* spellings via seatbeltPathSpellings. A glob allowRead override under /var or /etc therefore never matched the kernel-resolved /private/* path and the deny won. Apply seatbeltPathSpellings to the re-allow loop too, so both aliases receive file-read-data and file-read-metadata grants (matches the read/write/move rule loops). Also fix the assertRegexRule test helper: it quoted the regex with %q (double-escaping metachars like \.), which never matched profiles for globs with dots; mirror buildFileSystemRegexRule's quote-only escaping instead. Tests: literal re-allow under /tmp + glob re-allow under /var and /etc, both spellings asserted. Dogfood: allowRead /var/folders/.../secret/*.txt beats denyRead /var/folders/.../** in a real sandbox (TOP-SECRET); sibling blocked.

reneleonhardt · 2026-08-08T15:23:14Z

All four bot review findings are fixed.

Beyond that I changed, in order:

macOS path canonicalization: read/write/move rules now emit both the /tmp//var//etc and the /private/* spellings, so glob denies and allows match the kernel-resolved paths (the real TMPDIR is /var/folders/…).
Test-helper regex-escaping fix; applied to the related PR too.
Glob-override tests for /var and /etc added and kept in sync.
Verified with a real-sandbox run (override wins, sibling blocked) and Linux-container runs.

…fencesandbox#218) Bot P1 (cubic, confidence 9): the permissive allowRead re-allow loop emitted only the user's path spelling, while the deny loop emits both /tmp,/var,/etc and /private/* spellings via seatbeltPathSpellings. A glob allowRead override under /var or /etc therefore never matched the kernel-resolved /private/* path and the deny won. Apply seatbeltPathSpellings to the re-allow loop too, so both aliases receive file-read-data and file-read-metadata grants (matches the read/write/move rule loops). Also fix the assertRegexRule test helper: it quoted the regex with %q (double-escaping metachars like \.), which never matched profiles for globs with dots; mirror buildFileSystemRegexRule's quote-only escaping instead. Tests: literal re-allow under /tmp + glob re-allow under /var and /etc, both spellings asserted. Dogfood: allowRead /var/folders/.../secret/*.txt beats denyRead /var/folders/.../** in a real sandbox (TOP-SECRET); sibling blocked.

jy-tan · 2026-08-08T19:52:06Z

@reneleonhardt Thanks for looking into this, I'm not sure if I'm a fan of changing the allowRead semantics and having platform-specific quirks here. This is worth a more long-term/general fix along the lines of the discussion in #122, and I'll relook into that in the coming week.

In the meantime, to unblock your use case, consider using a modified local copy of the code template. Also happy to look into more fine-grained file denials in ~/.gnupg/... (that satisfies most reasonable use cases) for the code template if you want to open a PR for that.

allowRead is a no-op unless defaultDenyRead is set: in permissive mode generateReadRules emits one blanket (allow file-read*) and never consults allowPaths, and denyRead rules are emitted after it and win on seatbelt. So with extends:"code" a user cannot re-allow a template-denied path (e.g. ~/.gnupg/pubring.kbx under the ~/.gnupg/** deny, needed for gpg signing) — and config.Merge has no deny-exclusion syntax (see fencesandbox#122). In permissive mode, when denyRead is present, emit a specific (allow file-read-data) + (allow file-read-metadata) subpath/regex allow for each user allowRead path AFTER the deny loop. Per the existing comment, a specific operation allow is not overridden by a wildcard deny (file-read*), so the explicit user grant wins while everything else in the denied subtree stays blocked. Skipped when there is no denyRead: the blanket already permits all reads, so deny-free profiles stay byte-identical. Addresses fencesandbox#122 (no way to remove inherited deny entries): allowRead-beats- deny makes deny-exclusion syntax unnecessary instead of adding it. Docs: denyRead marked macOS-only (Linux read control is bind-mount visibility, no deny layer); allowRead platform semantics noted. Adds unit tests (override ordering, glob, dedup, no-deny no-op), a hermetic seatbelt integration test (temp-dir allowRead-beats-deny), and a runtime A/B (patched READ_OK, unpatched blocked).

Review (PR fencesandbox#218, cubic): my earlier docs claimed denyRead is macOS-only and ignored on Linux — wrong. Verified in source: - Linux enforces denyRead via read masking (empty bind mount, linux_mount_planner.go) and denyRead wins over allowRead at the mount level (hasExplicitLinuxReadGrant returns false for denied paths). - macOS: seatbelt deny rule; allowRead re-allows a specific path inside a denyRead subtree (specific allow beats wildcard deny) — that override is macOS-specific. docs/configuration.md: denyRead row + platform note corrected. docs/library.md: reads semantics synced (Linux deny wins; macOS allowRead override).

…nyRead-wins Second review (PR fencesandbox#218, cubic): the library.md override sentence sat in the preflight section, but CheckReadPath/CheckWritePath are platform-agnostic and keep denyRead always winning. Fixed: - docs/library.md: reads split into preflight+Linux wrap-mode (denyRead always wins) vs macOS wrap-mode seatbelt only (allowRead re-allows a denied subtree); preflight explicitly does NOT reflect the override. - internal/sandbox/path_check.go: CheckReadPath doc comment synced - it intentionally diverges from macOS wrap-mode (denyRead always wins there).

…ema, pkg) Proofread across all touched docs found three remaining contradictions (round-3 sweep): - ARCHITECTURE.md: 'denyRead masks even if broader allow rules exist' was false for macOS wrap-mode (allowRead override). Split by platform; note preflight stays denyRead-wins. (ARCHITECTURE.md should have been updated by the original PR per repo rule - missed, fixed now.) - internal/config/config.go DenyRead description: claimed denyRead always beats allowRead; macOS wrap-mode may re-allow a specific path. - pkg/fence CheckReadPath comment: implied parity with profile generators; preflight diverges (denyRead always wins, override not reflected). docs/schema/fence.schema.json: description synced to match the tag (generator could not run here - mixed-toolchain build cache; CI regen will produce identical text).

Third review (PR fencesandbox#218, cubic, P3): the override sentence was unqualified — it only holds in permissive mode. With defaultDenyRead/strictDenyRead the deny is emitted as specific file-read-data/file-read-metadata ops, not wildcard file-read*, so the specific allow does not beat them and denyRead wins (the implementation already gates the override on !defaultDenyRead). Synced everywhere the override is described: - ARCHITECTURE.md (the bot's target, per its suggested wording) - docs/configuration.md platform note - docs/library.md macOS reads bullet (permissive-only; defaultDenyRead wins) - internal/sandbox/path_check.go + pkg/fence CheckReadPath comments - internal/config/config.go DenyRead description + docs/schema (mirror) - PR body docs line

Deep-codemap proofread pass found the last stale claim: CheckWritePath's header said hook-mode and wrap-mode filesystem semantics are identical (avoiding a permanent asymmetry). The read-side override broke that: macOS wrap-mode permissive re-allows allowRead over a denyRead subtree while hook-mode CheckReadPath keeps denyRead winning - a deliberate exception. Scoped the parity claim to writes and pointed at CheckReadPath.

…fencesandbox#218) Bot P1 (cubic, confidence 9): the permissive allowRead re-allow loop emitted only the user's path spelling, while the deny loop emits both /tmp,/var,/etc and /private/* spellings via seatbeltPathSpellings. A glob allowRead override under /var or /etc therefore never matched the kernel-resolved /private/* path and the deny won. Apply seatbeltPathSpellings to the re-allow loop too, so both aliases receive file-read-data and file-read-metadata grants (matches the read/write/move rule loops). Also fix the assertRegexRule test helper: it quoted the regex with %q (double-escaping metachars like \.), which never matched profiles for globs with dots; mirror buildFileSystemRegexRule's quote-only escaping instead. Tests: literal re-allow under /tmp + glob re-allow under /var and /etc, both spellings asserted. Dogfood: allowRead /var/folders/.../secret/*.txt beats denyRead /var/folders/.../** in a real sandbox (TOP-SECRET); sibling blocked.

reneleonhardt requested a review from jy-tan as a code owner August 7, 2026 13:56

cubic-dev-ai Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread docs/configuration.md Outdated

cubic-dev-ai Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread docs/library.md Outdated

cubic-dev-ai Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread ARCHITECTURE.md Outdated

reneleonhardt force-pushed the fix/macos-allowread-override-deny branch from 0520cb2 to daa0f49 Compare August 8, 2026 11:20

cubic-dev-ai Bot reviewed Aug 8, 2026

View reviewed changes

Comment thread internal/sandbox/macos.go

reneleonhardt mentioned this pull request Aug 8, 2026

fix: permit AF_UNIX bind/connect under fence's own TMPDIR on macOS #217

Merged

reneleonhardt force-pushed the fix/macos-allowread-override-deny branch from 419937b to 3bcc431 Compare August 8, 2026 19:09

reneleonhardt added 7 commits August 8, 2026 23:49

reneleonhardt force-pushed the fix/macos-allowread-override-deny branch from 3bcc431 to 8711dde Compare August 8, 2026 21:54

Navigation Menu

Uh oh!

fix: honor allowRead over wildcard denyRead in permissive mode on macOS - #218

fix: honor allowRead over wildcard denyRead in permissive mode on macOS#218
reneleonhardt wants to merge 7 commits into
fencesandbox:mainfrom
reneleonhardt:fix/macos-allowread-override-deny

reneleonhardt commented Aug 7, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

reneleonhardt commented Aug 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

reneleonhardt commented Aug 8, 2026

Uh oh!

jy-tan commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

reneleonhardt commented Aug 7, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Tests

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reneleonhardt commented Aug 7, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reneleonhardt commented Aug 8, 2026

Uh oh!

jy-tan commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

reneleonhardt commented Aug 7, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading