ro never restrained a FIFO either, and the rule was written for one of its two nouns (refs #287, #289, #290) - #295
Merged
Conversation
… of its two nouns (refs #287, refs #289, refs #290) rejectSocketSource refused fs.ModeSocket and nothing else. A FIFO is the same noun — spoken through, not interpreted — and read-only restrains it no better. Measured with no user profile at all: payload wrote out of the sandbox to a host process through a FIFO sitting in @parent-ro's grant, which ships in defaults, while a plain-file write in the same directory correctly got EROFS. prw-r--r--. 1 michal michal 0 ../sibfifo WROTE-OK touch: cannot touch '../regularfile': Read-only file system HOST received: EXFIL-THROUGH-READONLY-PARENT One kernel fact under both nouns: may_open() clears MAY_WRITE for S_IFSOCK, S_IFIFO, S_IFBLK and S_IFCHR before MNT_READONLY is ever consulted. Read-only guards the filesystem, not the process on the other end of the node. rejectSocketSource -> rejectEndpointSource, predicate ModeSocket|ModeNamedPipe. Everything else about the loop unchanged: KindBind only, Authored skipped, stat through the injected Environ, absent source still skipped. DEVICES DELIBERATELY NOT IN THE PREDICATE, and the reason is measured rather than assumed. bwrap sets nosuid,nodev on every bind it creates, so a bound host device node cannot be opened at all, in either direction. One sandbox, three --ro-binds, bwrap 0.11.2: fifo:WROTE-OK /bin/sh: line 1: /tmp/devnull: Permission denied devnull:FAIL /bin/sh: line 1: /tmp/regular: Read-only file system regular:FAIL mountinfo showed ro,nosuid,nodev on both. There is no MS_NOFIFO and no MS_NOSOCK — the kernel gives snug the device case free through a flag that has to be there anyway, and gives it nothing for the other two. That asymmetry is the whole reason the predicate is socket-plus-FIFO. It depends on snug never emitting --dev-bind/--dev-bind-try, so that is now asserted rather than assumed (TestBwrapArgvNeverAllowsDeviceAccess, with a positive control on the comparison itself since no Kind switch path can emit the flag today). G6, grafts (#290). rejectEndpointSource loops p.Mounts; checkGraft had no source-kind test at all, so a graft whose source is a socket was accepted. #290's stated fix direction — exempt snug's own Authored proxy sockets — is unsound: Policy.Graft sets g.Authored = true BEFORE calling checkGraft, so the exemption is unconditionally true and the rule could never fire. Shipped instead as a positive predicate: a graft's SOURCE must be a DIRECTORY. Nothing to exempt, nothing for a future inode kind to be forgotten from, and it covers device too — which matters here and not at mount level, because the graft path is open_tree(OPEN_TREE_CLONE)/move_mount, which carries the source mount's own flags and adds only MOUNT_ATTR_RDONLY. bwrap's nodev does not travel with the clone into the engine's view. G3 already requires the DESTINATION to be a directory; G6 is that rule's other half. Absent source stays G4's business, so --dry-run does not start failing on host state. All five shipped graft sources are directories. #289, the phantom profile. The refusal told the user to select '@ssh-agent'. snug ships no such profile — `snug: unknown profile "@ssh-agent"` — and checkName refuses a leading @ in every file it parses, so a user cannot define it either. An error naming a fix nobody can apply is worse than one naming none. Message now names the real mechanism, an identity block with ssh_key and ssh_mode = "agent-proxy", verified against internal/cli/identity.go. @podman-socket in the same message stays: it does ship (base.toml:686). Five sites in validate.go changed. TestRefusalMessagesNameOnlyProfilesSnugShips (internal/profile, where Builtins() lives) pins it — bounded to two refusals, no allowlist; an unrestricted sweep is not viable because resolve.go names @null deliberately to say it does not exist. WHAT THIS DOES NOT CLOSE, stated here because softening it is the failure mode. #287's headline measurement STAYS OPEN. The check sees the node a grant NAMES, and only if it exists at resolve time. @parent-ro plus a pre-existing host FIFO still exfiltrates with default profiles. What lands is the ratchet #219 got for sockets: the spelled-out case is refused, the accidental one is not. Two bounds keep it medium rather than critical, both measured: the payload cannot create the endpoint inside a read-only grant (mkfifo -> Read-only file system), so a host process must already have made it and be holding it open; and no mount flag exists that would let snug close this the way nodev closes devices. A resolve-time scan of granted directories was considered and rejected — policy acceptance would depend on host state changing under it, unbounded work, and it races anything creating a node after resolve. So the residual is asserted POSITIVELY, integration, default selection: TestAFifoInAGrantedDirectoryStillReachesTheHost. It fails the day someone believes #287 is fully closed. Related: snug already refused to READ a FIFO as data (internal/cli/claude.go, readHostFileBounded, "will not read a FIFO, a device or a directory there") and was still willing to MOUNT one. Sixth recorded instance of a rule written once and applied to one of its two halves. No golden argv diff, and one would mean the change is wrong: no flag added, no accepted policy's mounts altered — only policies that were refused nowhere are now refused. The review artifact here is the refusal text plus VERIFY.md §4b. testdata/refusals.txt regenerated for the new message. Tests: eight, per definition-of-done step 5. TestBindOfAnEndpointSourceIsRefused socket+fifo refused, file+dir accepted TestEndpointRefusalNamesTheRealRemediation #289, asserts @ssh-agent ABSENT TestRefusalMessagesNameOnlyProfilesSnugShips @-token sweep over two refusals TestASymlinkToAnEndpointIsRefused stat follows the final symlink, as --ro-bind does TestEndpointRefusalIsNotAPathList FIFO spellings added; drift back to a catalogue fails it TestBwrapArgvNeverAllowsDeviceAccess makes the device argument checked, not claimed TestGraftSourceMustBeADirectory + TestG6HasNoAuthoredExemption + TestG6DoesNotApplyToFreshMountGrafts TestAFifoInAGrantedDirectoryStillReachesTheHost the residual, asserted positively Negative controls run by hand: reverting the predicate to ModeSocket alone fails the fifo and symlink cases; disabling G6 fails five graft cases. make gate green. make integration green with SNUG_REQUIRE_SANDBOX=1, 168 passed, 0 failed, 2 environment-only skips (no un-shimmed podman here, both explained in output). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Three redteam findings, one function.
rejectSocketSourcerefusedfs.ModeSocketand nothing else.#287 (sev:medium) —
rois not read-only for a FIFOA FIFO is the same noun as a socket: spoken through, not interpreted. Measured with no user profile at all, payload wrote OUT of the sandbox to a host process through a FIFO in
@parent-ro's grant — a profile that ships indefaults— while a plain-file write in the same directory correctly got EROFS.One kernel fact under both nouns:
may_open()clearsMAY_WRITEforS_IFSOCK,S_IFIFO,S_IFBLK,S_IFCHRbeforeMNT_READONLYis ever consulted. Read-only guards the filesystem, not the process on the other end of the node.rejectSocketSource→rejectEndpointSource, predicateModeSocket|ModeNamedPipe. Loop otherwise unchanged:KindBindonly,Authoredskipped, stat through the injectedEnviron, absent source skipped.Devices deliberately NOT in the predicate
Measured, not assumed. bwrap sets
nosuid,nodevon every bind, so a bound host device node cannot be opened at all. One sandbox, three--ro-binds, bwrap 0.11.2:mountinfoshowedro,nosuid,nodevon both. There is noMS_NOFIFOand noMS_NOSOCK— kernel gives the device case free through a flag that has to be there anyway, gives nothing for the other two. That asymmetry is the whole reason the predicate is socket-plus-FIFO. It depends on snug never emitting--dev-bind/--dev-bind-try, now asserted rather than claimed.#290 (sev:low, latent) — grafts
checkGrafthad no source-kind test, so a graft whose source is a socket was accepted.The issue's stated fix direction is unsound.
Policy.Graftsetsg.Authored = trueBEFORE callingcheckGraft, so "exempt snug's own Authored proxy sockets" is unconditionally true and the rule could never fire — this repo's documented-but-not-implemented shape.Shipped as G6: a graft's SOURCE must be a DIRECTORY. Positive predicate: nothing to exempt, nothing for a future inode kind to be forgotten from, covers device too. Device breadth matters here and not at mount level because the graft path is
open_tree(OPEN_TREE_CLONE)/move_mount, which carries the source mount's own flags and adds onlyMOUNT_ATTR_RDONLY— bwrap'snodevdoes not travel with the clone into the engine's view. G3 already requires the DESTINATION to be a directory; G6 is that rule's other half. Absent source stays G4's business so--dry-rundoes not start failing on host state. All five shipped graft sources are directories.If snug ever needs the engine to see a socket, it grafts the DIRECTORY containing it and the engine binds the socket itself — what
EngineSockGuestalready does.#289 (sev:low) — the refusal named a profile snug does not ship
Message said "select '@ssh-agent'".
snug: unknown profile "@ssh-agent", andcheckNamerefuses a leading@in every parsed file, so a user cannot define it either. An error naming a fix nobody can apply is worse than one naming none. Now names the real mechanism — an[identity]block withssh_keyandssh_mode = "agent-proxy", key names verified againstinternal/cli/identity.go.@podman-socketin the same message stays: it ships (base.toml:686). Five sites invalidate.go.WHAT THIS DOES NOT CLOSE
#287's headline measurement stays open. The check sees the node a grant NAMES, and only if it exists at resolve time.
@parent-roplus a pre-existing host FIFO still exfiltrates on default profiles. What lands is the ratchet #219 got for sockets: spelled-out case refused, accidental case not.Two bounds keep it medium rather than critical, both measured: the payload cannot CREATE the endpoint inside a read-only grant (
mkfifo: cannot create fifo ...: Read-only file system), so a host process must already have made it and be holding it open; and no mount flag exists that would let snug close this the waynodevcloses devices. Resolve-time scanning of granted directories was considered and rejected — policy acceptance would depend on host state changing under it, unbounded work, races anything creating a node after resolve.So the residual is asserted POSITIVELY:
TestAFifoInAGrantedDirectoryStillReachesTheHost, integration, default selection. Confirmed still open on this build. It fails the day someone believes #287 is fully closed.Do not close #287 on this PR. Either leave it open with the spelled half marked done, or file a follow-up for the directory case. Maintainer's call.
Maintainer edit not made here
CLAUDE.md's "A SOCKET is the third noun" bullet ends a grant of a directory is a grant of every socket anyone puts in it later. It needs and every FIFO. Left alone deliberately — that bullet was rewritten on main in #282 and belongs to whoever owns it.Related
snug already refused to READ a FIFO as data (
internal/cli/claude.go,readHostFileBounded, "will not read a FIFO, a device or a directory there") and was still willing to MOUNT one. Sixth recorded instance of a rule written once and applied to one of its two halves.Review artifact
No golden argv diff, and one would mean the change is wrong — no flag added, no accepted policy's mounts altered; only policies refused nowhere are now refused. The artifact a human reads here is the refusal text plus
VERIFY.md§4b.testdata/refusals.txtregenerated for the new message.Tests (definition-of-done step 5)
TestBindOfAnEndpointSourceIsRefusedTestEndpointRefusalNamesTheRealRemediation@ssh-agentABSENTTestRefusalMessagesNameOnlyProfilesSnugShips@-token sweep over two refusals, no allowlistTestASymlinkToAnEndpointIsRefused--ro-binddoesTestEndpointRefusalIsNotAPathListTestBwrapArgvNeverAllowsDeviceAccessTestGraftSourceMustBeADirectory,TestG6HasNoAuthoredExemption,TestG6DoesNotApplyToFreshMountGraftsTestAFifoInAGrantedDirectoryStillReachesTheHostAn unrestricted
@-token sweep is not viable and the comment records why:resolve.gonames@nulldeliberately to say it does not exist, and prose carries@claude-shaped/@net-shaped.Negative controls run by hand: reverting the predicate to
ModeSocketalone fails the fifo and symlink cases; disabling G6 fails five graft cases.make gategreen.make integrationgreen withSNUG_REQUIRE_SANDBOX=1— 168 passed, 0 failed, 2 environment-only skips (no un-shimmed podman here, both explained in output rather than silent).refs #287, refs #289, refs #290
🤖 Generated with Claude Code