profile: drop @tmp-shared and the {host_tmpdir} variable (closes #399) - #572
Merged
Merged
Conversation
Maintainer ruling: drop it, YAGNI, it can be added back later. #399 asked whether the allocated directory should move from /tmp to /run/user/<uid>. Nothing needed the variable at all: only [profile.tmp-shared] named {host_tmpdir}, nothing includes that profile, engineforwardedpath.go cited it only as an example of a divergent host:guest bind (@claude's "{home}/.local/bin/claude:/snug/bin/claude" is the same example and stays), and policy.Context.HostTmpDir was a resolver input the CLI filled for that one profile. THE CAPABILITY DID NOT GO ANYWHERE; THE ALLOCATION DID. /tmp is still the one path yieldTo yields, so a profile that NAMES a host directory still takes it over and --dry-run still renders the path and the "this is the HOST's /tmp" note. What is gone is snug creating a per-project 0700 directory under os.TempDir(), checking it for the classic /tmp races and binding it — machinery whose whole job was to pick a path a profile can name itself. internal/cli/tmpdir.go goes with it. THE RETIRED VARIABLE REFUSES, and the message is the point rather than the refusal: snug: profile "old": {host_tmpdir} in "{host_tmpdir}:/tmp" was removed with the @tmp-shared profile and has no replacement: name the host directory yourself, rw = ["/path/on/host:/tmp"] exit 77. A profile still carrying it is not a typo — it was a real variable for the life of @tmp-shared — and "unknown variable" would send its author to check a spelling that is correct. Same shape as retiredAnonKey (internal/profile/file.go). TestTheRetiredHostTmpdirVariableNamesItsOwnRemoval (internal/policy) is the named regression and asserts the general arm beside it, so a special case that swallowed every unknown name would not pass. WHAT THE RED-TEAM FINDING LEAVES BEHIND. runsweep.go's filter (issue #425 F1) matches a name SHAPE rather than the "snug-<uid>-" prefix because a target-derived name collides with it; the concrete collision was this profile's payload-writable directory, where a sandbox writing /tmp/lock got another project's shared /tmp deleted. Filter and test stay — the claim is that the NEXT mechanism of that shape is safe without anyone remembering the file. Test renamed to TestSweepNeverTouchesATargetDerivedDirectory. A REDTEAM ROUND FOUND ONE SINK THE REFUSAL DID NOT REACH, AND IT IS FIXED HERE. `symlink = [{ at = "...", target = "..." }]` expanded `at` and passed `target` through untouched — one key of a two-key table behaving like the other, silently — so {host_tmpdir} rendered verbatim into a link inode there while refusing in ro, rw, tmpfs, optional, symlink.at, every environ verb, identity.ssh_key and through an include. So did {home}: a profile writing target = "{home}/x" got the six literal characters "{home}" in the sandbox and nothing said so. Both keys expand now. The target is NOT Cleaned, unlike `at` — a link target is resolved by the kernel relative to the link and the shipped ones are relative on purpose ("usr/bin"). TestASymlinkTargetExpandsVariablesLikeEveryOtherField asserts both halves; negative control run against the old line, which fails it on the {home} arm. No golden moves: no shipped profile writes a variable in a symlink target. The rest of the round is clean — no findings on the empty-host-path angle (splitSpec refuses ":/tmp" as "both sides must be absolute paths" before any mount is built), on the sweeps (a snug-<uid>-sha256_<64hex> directory is still refused by isEngineRunDirName's decimal-components rule, so the F1 shape is not newly deletable), or on the replacement grant, which behaves as an ordinary bind: EvalSymlinks canonicalises the source and --dry-run renders what gets bound. What a user no longer gets, stated plainly: prepareHostTmpDir's symlink-at-the-allocated-name refusal, its owner==uid check and its exactly-0700 requirement. Those were owed because SNUG chose the path; a path the human names gets the ordinary bind-grant contract every other ro/rw grant has always had. Tests. hosttmpvisibility_test.go (issue #87's local half) had a @tmp-shared arm; it now authors a profile binding a directory of its own at /tmp, and each arm carries a host-side control — the marker the payload writes to guest /tmp MUST appear in the named directory for the shared arm and MUST NOT for the private-tmpfs arm. Negative control run: with the -p flag removed the arm fails with "this arm never exercised a bind at /tmp and its decoy check proves nothing". Two builtin sweeps (envuncheckedsink, ownedenv) lose their @tmp-shared skip, because an exception list is where a sweep rots. jsonrefusal_test.go's "grant missing" class reached its refusal through the unallocated directory and now names a path that does not exist. VERIFY.md §9a-quater names every sink the refusal reaches and carries the symlink-target case with its real output. Docs: README's profile table, INDEX §7.3 rewritten plus the path-variable list, tree listing, builtin-name count and host-integration intro, THREAT-MODEL's #87 local half (argues from what a /tmp grant binds, not from the allocator), TARGET-LOCK, ENVIRONMENT-VARIABLES' sanitise example, PARAMETERISED-PROFILES, and VERIFY.md §9a-quater with both halves and real output. TestINDEXAdvertisesExactlyThePathVariablesResolveBuilds caught the INDEX list by itself — issue #224's guard working. make gate green. SNUG_REQUIRE_SANDBOX=1 SNUG_SANDBOX_TIMEOUT=25m make integration: both suites green, 210 PASS + 3 PASS, 2 SKIP, engine tests 46 ran against a floor of 46, podman 6.0.2. 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.
Closes #399.
What
@tmp-sharedand the{host_tmpdir}variable behind it are removed.internal/cli/tmpdir.gogoes with them.Maintainer ruling: drop it, YAGNI, can be added back later.
Why the whole surface and not the directory's location
#399 asked whether the allocation should move from
/tmpto/run/user/<uid>. Nothing needs the variable at all:[profile.tmp-shared]named{host_tmpdir}. No other shipped profile, nothingincludes it.engineforwardedpath.gocited it only as an example of a divergenthost:guestbind;@claude's{home}/.local/bin/claude:/snug/bin/claudeis the same example and stays.Environ.HostTmpDirwas a resolver input the CLI filled.The capability did not go anywhere —
/tmpis the one path snug's own mount yields, so a profile that NAMES a host directory still takes it over, and--dry-runrenders the path plus the "this is the HOST's /tmp" note. What is gone is snug allocating a per-project0700directory underos.TempDir(), checking it for the classic/tmpraces, and binding it: machinery whose whole job was to pick a path a profile can name itself.Also gone with it: 213MB/1209-directory
/tmpaccumulation #399 measured came from that allocator plus SIGKILL orphans.The retired variable REFUSES
A profile still carrying
{host_tmpdir}is not a typo — it was a real variable for the life of@tmp-shared— and plain "unknown variable" sends its author to check a spelling that is correct. Precedent:retiredAnonKeyininternal/profile/file.go.TestTheRetiredHostTmpdirVariableNamesItsOwnRemoval(internal/policy) is the named regression, and asserts the general arm beside it: a special case swallowing every unknown name would pass an assertion about{host_tmpdir}alone.What the red-team finding leaves behind
internal/engine/runsweep.go's name filter (issue #425 F1) matches a name SHAPE rather than thesnug-<uid>-prefix, because a target-derived name collides with it. The concrete collision was@tmp-shared's directory — payload-writable, so a sandbox writing/tmp/lockgot another project's shared/tmpdeleted, measured end to end. Filter and its test STAY: the claim is that the NEXT mechanism of that shape is safe without anyone remembering the file. Test renamedTestSweepNeverTouchesATargetDerivedDirectory, comments rewritten to say the profile is gone and the shape rule is not.The redteam round found one sink the refusal did not reach
symlink = [{ at = "...", target = "..." }]expandedatand passedtargetthrough untouched — one key of a two-key table behaving unlike the other, silently. So{host_tmpdir}rendered verbatim into a link inode there while refusing inro,rw,tmpfs,optional,symlink.at, everyenvironverb,identity.ssh_keyand through aninclude. So did{home}:target = "{home}/x"put the six literal characters{home}in the sandbox and nothing said so.Both keys expand now. Target is NOT
Cleaned, unlikeat— a link target is resolved by the kernel relative to the link and the shipped ones are relative on purpose (usr/bin).TestASymlinkTargetExpandsVariablesLikeEveryOtherFieldasserts both halves; negative control run against the old line, which fails it on the{home}arm. No golden moves: no shipped profile writes a variable in a symlink target.The rest of the round is clean
splitSpecrefuses":/tmp"with "both sides must be absolute paths" before any mount is built, and{host_tmpdir}refuses insideexpandVars, so no--bind "" /tmpargv and no nil-deref.snug-<uid>-sha256_<64hex>is still refused byisEngineRunDirName's decimal-components rule, so F1's shape is not newly deletable. Measured with five planted decoys and a live@podman-socketrun.EvalSymlinkscanonicalises the source,--dry-runrenders what actually gets bound, FIFO source refused (ro is not read-only for a FIFO: a payload writes out through the default @parent-ro grant #287), nonexistent source refused,rejectHostHomeBindstill fires.What a user no longer gets, stated plainly:
prepareHostTmpDir's symlink-at-the-allocated-name refusal, itsowner == uidcheck, and its exactly-0700 requirement. Those were owed because SNUG chose the path; a path the human names gets the ordinary bind-grant contract every otherro/rwgrant has always had.Tests
test/integration/hosttmpvisibility_test.go(issue #87's local half) had a@tmp-sharedarm. It now authors a profile binding a directory of its own at/tmp, and each arm carries a host-side control: the marker the payload writes to guest/tmpMUST appear in the named directory for the shared arm and MUST NOT for the private-tmpfs arm. Negative control run: with the profile flag removed the arm fails with "this arm never exercised a bind at /tmp and its decoy check proves nothing".Three fixtures that skipped
@tmp-sharedwhile sweeping every builtin (envuncheckedsink,ownedenv) lose the skip — an exception list is where a sweep rots.jsonrefusal_test.go's "grant missing" class reached its refusal through@tmp-shared's unallocated directory and now uses a profile naming a path that does not exist.Docs
README profile table,
INDEX.md(§7.3 rewritten, path-variable list, tree listing, builtin-name count, host-integration-surfaces intro, the sessions-share-this-surface paragraph),THREAT-MODEL.md(#87's local half now argues from what a/tmpgrant binds rather than from the allocator),TARGET-LOCK.md,ENVIRONMENT-VARIABLES.md(sanitise example),PARAMETERISED-PROFILES.md,VERIFY.md§9a-quater (new, both halves with real output).TestINDEXAdvertisesExactlyThePathVariablesResolveBuildscaught the INDEX path-variable list on its own — issue #224's guard doing its job.Verification
make gategreen.SNUG_REQUIRE_SANDBOX=1 SNUG_SANDBOX_TIMEOUT=25m make integrationon this host, both suites green: 210 PASS + 3 PASS, 2 SKIP,engine tests: 46 ran, floor 46 — podman version 6.0.2, exit 0. (25m ceiling because the default 8m does not fit this host; predates this branch.)Redteam round run against this branch before push, per the definition of done. One finding, fixed here; the round's own summary is quoted above.
🤖 Generated with Claude Code