Environment variables: the environ verbs, and the sanitise kind rule - #10
Merged
Conversation
… a trap A reviewer asked whether `@stubs` being a switch with a hardcoded directory means "insert a value into a variable" is unavailable to user profiles. The short answer is "mostly, and inconsistently". Working out why produced this. Measured against main, not against a branch -- an earlier draft measured an unmerged branch and reported a defect that does not exist here. What survived that check is §2, and it is worth more than the bug report was: a branch that authored the five XDG variables did so unconditionally in Resolve, while the directories they name come from `@home`. Select without `@home` and the variables survive naming nothing. main authors no XDG variable, so this is not a defect here -- it is a trap for whoever implements them, and the obvious implementation walks straight into it. §3 is the reviewer's second note and it reorganises the problem: variables are typed even though they are all char*. Scalars join by equality-or-conflict and have no order to get wrong, which confines the entire ordering question to the list type. Semi-structured values (PS1-PS4, PROMPT_COMMAND, LS_COLORS) have no join at all, which is what `forbiddenEnv` has been expressing as a blocklist -- restating it as a type judgement says what else belongs there without waiting to be bitten. The class x type table has illegal cells, and the holes are the specification. §3(d) is a live hazard for the sanitise capability, measured: env -i PATH="/usr/bin:" sh -c 'victim' -> PWD-BINARY-RAN env -i PATH="/usr/bin::/bin" sh -c 'victim' -> PWD-BINARY-RAN env -i PATH="/usr/bin:/bin" sh -c 'victim' -> command not found An empty element in PATH is the current working directory, which in snug is the target -- the one place a hostile payload has full write access. Implement "drop the ungranted elements" as a string replace and a feature sold as tightening the environment creates a code-execution vector that did not exist. The rule: rebuild the list from survivors, never edit the string; unset rather than set empty. §9 makes it a named regression test with a positive control. Also surveyed: makeWrapper, systemd.exec, flatpak, Lmod, the NixOS module system, Nickel, CUE. Two conclusions independent of which candidate wins. Every neighbour except CUE ships subtraction, so borrowing a vocabulary by analogy imports the thing invariant 1 exists to prevent. And an expression language would move the "no negation key" guarantee from syntax, where it is checked by listing keys, to semantics, where it would have to be re-proved. Recorded but not fixed, live on main: `forbiddenEnv` sits inside the "is it set on the host" guard, so a profile carrying env = ["LD_PRELOAD"] is accepted on a host where the variable is unset and refused where it is set (§1(e)). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tests it
A reviewer asked for it to be explicit that `snug . -- podman` runs the command
using the sandbox's PATH rather than the host's. It does, and the property is a
precondition for the rest of the document: if the payload name were resolved
host-side, every ordering question here would be moot, `shadows` could not work,
and the podman stub would be decoration. bwrap --clearenv's, sets PATH with
--setenv, and execvp's inside the namespaces with its own modified environ, so
the lookup happens inside.
Measured four ways, all quoted in §1(f):
PATH=/…/hostonly:$PATH snug . -- hostmarker
bwrap: execvp hostmarker: No such file or directory
snug -p tbin . -- ls
SANDBOX-LS-RAN # fake ls in a profile's path dir beat /usr/bin/ls
The second is the decisive one: it is `shadows` (§7C) already working, unnamed
and undeclared. Also recorded is a case that looks like a leak and is not -- a
host binary reached by absolute path ran, because @parent-ro grants the target's
parent read-only and the binary lived under it; the same directory returns
"Read-only file system" to touch, and a binary outside every grant does not run.
Two gaps fall out, both now in §8 as fixable now.
Nothing tests any of it. A refactor to host-side exec.LookPath would leave every
existing test green while resolving the payload's name in the wrong namespace --
and it would make the negative case SUCCEED, which reads like a feature. §9 asks
for the three-part test with its own positive control.
And the error is bwrap's, not snug's: "bwrap: execvp podman: No such file or
directory" is what a user gets for the most ordinary mistake in the tool, asking
for a command no profile granted. It names neither the sandbox's own PATH nor
the profile that would grant the binary. "Errors name the fix" is a working
agreement rule and this is the highest-traffic error there is.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An independent reviewer re-ran every load-bearing measurement and read the code behind every claim about snug itself. The measurements about the outside world all reproduced. The failures were concentrated exactly where the author had reasoned instead of executed -- claims about snug's own code and tests -- which is the failure mode CLAUDE.md documents five times over. Verified each before accepting it; all six hold. §2 said "this is not a defect on main". Wrong, and it understated the problem. main authors no XDG variable, which is true and is not the point: HOME is assigned unconditionally at resolve.go:396 while the directory comes from @home's tmpfs, and SHELL and PATH name things @sys grants. So the pattern is already instantiated three times and already dangles: snug --dry-run --no-defaults -p @parent-ro . -- true HOME=/home/michal no @home: does not exist SHELL=/usr/bin/bash no @sys: does not exist That turns §9's checklist item from a forward-looking nicety into a test that fails on main today. §3(b) claimed forbiddenEnv "is a type judgement in disguise". It is not. The list on main holds no semi-structured name at all, while LD_LIBRARY_PATH is in it and is a list; PROMPT_COMMAND, LS_COLORS and GIT_CONFIG_PARAMETERS all pass through unrefused, measured. The two rules are orthogonal and both are needed -- forbiddenEnv is about what the value DOES, at any type; "semi-structured values have no join" is a separate and currently unenforced type fact. Merging them would have argued for adding PS1 and dropping LD_LIBRARY_PATH, both wrong. §9 reported a missing commutativity test for the environment. TestResolveIsCommutative already covers it -- canon() renders p.Env, and the fixture carries a `path` entry deliberately so PATH assembly is covered. The correction cuts both ways: §1(a)'s alphabetical sort is enforced, not accidental. §3(e) called the sanitised-scalar cell meaningless. It is the degenerate one-element case and the most useful application there is: inherit a host scalar iff it names something the policy grants, else unset. That is the points-inside check, and it is what repairs the three live instances above -- so excluding it put the one class with a measured problem outside the scope of the one new capability proposed. §7F shipped two ways for a profile to subtract. `passthrough` and `sanitise` over the same name lets profile B remove elements profile A's grant put in the sandbox, which is invariant 1 with no deny key in sight; it needs to be a resolve-time conflict or one key, not the ergonomics question §10 called it. And `authors` needs a reserved namespace as its FIRST guard: authors = { PATH } subtracts basePATH in one line, and SNUG_PROFILES / SNUG_TARGET would let a profile lie to the artifacts a human reads to decide whether to trust the sandbox. That is closed today only by accident of ordering (the prof.Env loop runs before snug's own assignments), and an authors key would have to make the accident deliberate. Also recorded: env = ["PATH"] is accepted and silently discarded today -- no error, no warning, nothing in --dry-run. Under "no silent downgrade, ever" that should be a named refusal, and it is fixable in the same patch as §1(e). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he file
A second agent measured the variables a development sandbox actually meets and
classified them by type, separator, and what an empty element means. The result
is §3a, and it changes two conclusions the document had already drawn.
§3(d) said "unset rather than empty". Half right, and the other half inverts:
env -i PATH="" ./execvp_probe -> PWD-BINARY-RAN # empty = the cwd
env -i ./execvp_probe -> rc=7 # unset = confstr(_CS_PATH)
env -i bash --noprofile --norc -c 'echo "${PATH-UNSET}"'
/usr/local/bin:/usr/bin:/bin:. # bash's compiled-in default
For execvp unset is the safe floor; for bash it is worse, because bash
substitutes DEFAULT_PATH_VALUE, which on this build ends in ".". PATH has no
safe absent state at all.
And the discriminator for `sanitise` is not the type, it is the empty-element
semantics -- safe where an empty element is ignored, hazardous where it is the
cwd, and illegal where it is an OPERATOR. MANPATH is the last: a leading empty
element means "prepend the system manpath", so removing an element can add
directories. "Rebuild, never edit the string" is necessary and not sufficient
there.
The finding that outranks the rest is not about the environment language at all.
CLAUDE.md's "generate, don't bind" rule pins a tool's config FILE and leaves its
environment, which is a higher-precedence source. Measured:
env -i GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null \
GIT_CONFIG_PARAMETERS="'user.name'='StillInjected'" git config --get user.name
StillInjected
... GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=user.name GIT_CONFIG_VALUE_0=AlsoInjected
AlsoInjected
Re-verified by hand before recording. A hostile process inside the sandbox can
set GIT_CONFIG_KEY_0=core.sshCommand and have the next git fetch -- including one
an unsuspecting user or agent runs -- execute its command, with
GIT_CONFIG_GLOBAL pointing at a clean generated file. Not a break in the sandbox
boundary; the payload already runs code. It is a break in identity pinning,
which is the guarantee GIT_CONFIG_GLOBAL exists to make, and it wants its own
investigation.
forbiddenEnv measures both too wide and too narrow: PYTHONSTARTUP is in it and
does not fire for a non-interactive interpreter, while PYTHONPATH is not in it
and fires on every python3 via sitecustomize.py (measured). glibc's own
secure-execution strip list in ld.so(8) is the closest thing to an authoritative
denylist and should seed ours. Four vectors are name PREFIXES that a
map[string]bool cannot express at all.
Also corrected: §1(b) overstated the stub's gating -- it needs a detected
HostShims entry as well as p.Podman, so on a host where podman is a real binary
/run/snug/bin never appears.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ge rule Replaces the CUE-syntax section, which was written in a notation nobody implementing this has to read. Same semantics, expressed as the TOML snug already parses. The owner's sketch left one thing rough and said so: "lists like PATH can be merged, while scalars like SHELL do not -- this is not very well expressed here." It does not need expressing. TOML already has both types, so the value's own type carries it: PATH = ["/usr/bin", "/usr/sbin"] array -> list -> merges (union, sorted) SHELL = "/usr/bin/bash" string -> scalar -> two different values is an error No lookup table, no `type =` key, nothing to keep in sync. You say which one you mean by how you write it. That also closes the empty-element hazard by construction, which is the finding this document spent the most measurement on. A profile never writes a separator, so it cannot write "/usr/bin:" and cannot produce "/usr/bin::/bin" by dropping an element from a string; snug joins with the right separator per variable and refuses an empty element. §3(d)'s PATH-means-cwd hazard and §3a.2's MANPATH-empty-element-is-an-operator problem both stop being things the implementation has to remember. A string where a list is expected is a load error rather than a coercion, or the separator returns through the front door. The prepend rule is the owner's: once across the whole selected set, a second is a failure. Cost stated plainly rather than buried -- @stubs-in-path is in defaults, so it holds the slot on every ordinary run and a user profile wanting it must displace it with --no-defaults or a defaults list. inherit-env moves passthrough from profiles to config.toml, so selecting a profile can no longer put a host credential inside; the human writes that line for their own machine. Flagged honestly: CLAUDE.md says config holds preferences and never grants, and this is that rule being amended rather than applied. Also corrected from the sketch: XDG_CONFIG_DIR is XDG_CONFIG_HOME. XDG_CONFIG_DIRS is a different variable -- plural, a list, defaulting to /etc/xdg -- which is exactly the confusion the type table exists to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The owner asked for consistent naming, with the verb telling how the variable
is handled. Taken literally that gives five verbs, each with exactly one
meaning: environ-set (scalars), environ-merge (lists, union), environ-prepend
(lists, at most one across the selected set), and in config.toml
environ-inherit and environ-sanitise.
This replaces the previous draft's idea of letting the TOML value type imply the
operation. The verb is better for a reason that is not aesthetic: it is
greppable and a type is not. `grep environ-prepend ~/.config/snug/profiles.d/`
finds every ordered claim on the host in one command, and a reader does not need
to know whether PATH is a list to understand the line.
The type does not go away, it becomes a named error. snug ships the type table
from §3a, so environ-merge on SHELL and environ-set on PATH are both refused
with the verb to use instead. An unknown name defaults to scalar, which is the
conservative reading -- a scalar merges with nothing, so it can only conflict,
never silently combine.
The rule that kills the empty-element hazard survives intact and is now stated
as a rule rather than a consequence: snug never splits a string on a separator,
ever. A string is one element. So a profile cannot write "/usr/bin:", cannot
produce "/usr/bin::/bin" by dropping an element, and cannot smuggle a ";" into
LD_LIBRARY_PATH.
Semi-structured variables get no verb at all. LS_COLORS, TERMCAP,
DBUS_SESSION_BUS_ADDRESS, GIT_CONFIG_PARAMETERS and IFS are ignored, and that is
a position rather than a gap -- there is nothing to refuse because the operation
does not exist, the same shape as "no X11 profile ships".
PS1 is the exception and §11.6 works it through. The thing that is easy to get
backwards: PS1 is not a security control. Anything inside can set it, so
refusing to let the USER configure it buys nothing against a hostile payload and
only hurts the honest case. But inheriting the host's PS1 is a real code channel
-- bash runs command substitution on prompt strings, promptvars on by default,
measured -- so environ-inherit must refuse the whole PS0-PS4 and PROMPT_COMMAND
family. The answer is neither a verb nor a flat no: a `prompt` preference with a
constrained template ({lock}, {profiles}, {target}, {cwd}) that snug renders,
which is "generate, don't bind" applied to a display string. With one caveat
worth writing down now rather than discovering later: a template is still a
display string, so \r or a cursor-movement escape can erase whatever precedes
it, and the marker should be placed last or control characters rejected.
Also recorded as open rather than smoothed over: the environ family are verbs
while the mount keys are nouns. The defence -- mount keys are all one operation
differing by access, the environment has five -- reads as justification after
the fact, and should either be argued properly or the mount keys should move
too. Not both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recommended, and not on taste -- the flat spelling does not parse for the case
that needs it most. Multi-line inline tables are invalid in TOML 1.0, so
`environ-set = { ...several variables... }` across lines is a syntax error.
Measured in a reference parser and in snug's own:
environ-set = { python3 tomllib: Invalid initial character for a key part
XDG_CONFIG_HOME = "...", go-toml/v2: toml: invalid character at start of key
XDG_CACHE_HOME = "...",
}
[profile.home.environ.set] both parsers: OK
XDG_CONFIG_HOME = "..."
XDG_CACHE_HOME = "..."
@home sets four XDG variables and @sys sets several, so the flat form forces
every profile onto one long line or onto invalid TOML. Every example in the
document had the illegal shape; all nine TOML blocks in §11 now parse, checked
by a script rather than by eye.
Nesting also closes the naming inconsistency the previous commit recorded as
open, rather than arguing around it. `environ` is a noun at the top level
alongside ro, rw, tmpfs and symlink; the verbs live one level down, describing
operations within one thing instead of competing with grants for the root
namespace. The flat spelling had no answer to that and the nested one does not
need to invent one.
Two smaller consequences. An unknown verb is refused for free -- environ becomes
a struct with known fields, so DisallowUnknownFields catches environ.deny
exactly as it catches an unknown root key, which is the "a negation key cannot
be smuggled in" guarantee applying one level down with no new code. And adding
`append` later costs a nested field rather than a sixth root key.
Greppability survives, which was the whole reason to prefer verbs over letting
the value type imply the operation: `grep -rn 'environ.prepend'` still finds
every ordered claim on the host, because the section header spells the full
path. That would not hold for a nested INLINE form, which is a further argument
for writing these as table headers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1302 lines to 470. The document had grown as an investigation and read like one: the format it was arguing towards arrived in §11, after ten sections of prior art, algebra and rejected candidates. Reordered so the format is the document and the reasoning supports it. §1 the format five verbs, nested, with worked profiles §2 how it behaves verb/type agreement, the errors, --dry-run output §3 the variable types what drives which verb applies §4 why the measurements that forced each rule §5 sidenote considered and rejected, kept short §6 open New in §2.4: what `--dry-run` prints. The format is only half the answer to "how will this look" -- a user meets the rendered environment far more often than the TOML, and it has to say two things a flat list cannot: which verb produced a value, and for sanitise, what was dropped. A filter that silently removes two of three elements is the exact failure shape the rest of the document is trying to avoid. The survey of makeWrapper, systemd, flatpak, Lmod, NixOS, Nickel, CUE and Kubernetes collapses from a full section to two paragraphs, keeping only the two conclusions that actually shaped the format: everyone offering `prepend` buys commutativity back with a number, and everyone except CUE ships subtraction, so borrowing a vocabulary by analogy imports the thing invariant 1 exists to prevent. Candidates A-F become six bullets. The expression-language argument is the one kept at length, because it is the reason the answer is "borrow CUE's semantics, do not take the dependency". Every measurement survives -- PATH empty element, MANPATH's operator, bash's DEFAULT_PATH_VALUE, the payload-name resolution, the three dangling variables on main, the git config env bypass. Checked mechanically after compression rather than by eye: code blocks byte-identical, headings byte-identical, and all nine TOML blocks still parse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e model An independent spec review attacked the format as something to implement from, and found the core sound but the document self-contradictory in three places and regressive in two. Verified each against the code before accepting it. The root cause is one omission: the model had no place for snug's OWN environment authorship. Everything else followed from that. @stubs-in-path was shown prepending /run/snug/bin. That is illegal under the document's own rule -- the directory is KindData created by Policy.Replace, so no profile can grant it -- and it inverted a decision the code states explicitly. resolve.go:395-409 places the stub AFTER every profile path entry, "because a profile entry is an explicit human grant and the stub is snug's own generated fallback". The format had it beating everything. The profile is a switch with no environ key at all; snug authors the directory, as it already does. The fix is the distinction the codebase already draws for mounts and CLAUDE.md already states -- a profile mounting over another profile's grant is masking and is refused, snug replacing a path with its own content is replacement and is allowed. Mount has an Authored field and Policy.Replace is its single writer. The environment needs the same and did not have it. §4.2's repair was worse than the defect. "Author only what the profile grants" would move PATH into @sys, so `--no-defaults -p @cwd-rw . -- bash` yields a sandbox with no PATH -- and §4.3, two sections later, measures bash substituting a compiled-in default ending in "." which is the target. Same for HOME, which is where the identity generator writes .gitconfig, .ssh/config and known_hosts. The repair is now to MARK unbacked authored values in --dry-run, which is invariant 5's shape, and to leave the granted-path rule on profiles, which have no floor to protect. The one-prepend guarantee was overclaimed. merge is "union then sorted", so /opt/bin sorts ahead of /usr/bin and beats @sys without anyone using prepend or consuming the slot -- merge makes the same claim and ASCII decides it. §2.3 now states the narrower true guarantee: the front is exclusive and checkable from declarations; base entries are structurally last; merge-vs-merge is sorted and sorting is not a decision. The bands are named. Moving inherit/sanitise to config.toml was a security regression, not the improvement it was sold as. @claude carries ANTHROPIC_API_KEY today and it enters only when @claude is selected; a host-wide config line puts it in every sandbox on the machine, inverting CLAUDE.md's bound on adapters. The trust argument does not survive either -- profiles.d and config.toml are the same tree at the same trust level, so moving it changes only how narrowly it can be scoped. Both verbs stay in profiles. Also recorded: `path` must be retired alongside `env`, since environ.merge on PATH does the same thing and @claude uses both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit landed the model fix but two of its consequences did not make it into the file. 4.2 said the repair for the three dangling variables on main was to author only what the profile grants. That is worse than the defect: 4.3, two sections later, measures bash substituting a compiled-in default ending in "." when PATH is unset, and "." is the target. HOME is worse still -- it is where the identity generator writes .gitconfig, .ssh/config and known_hosts, so a profile able to move it defeats identity pinning silently. The repair is to MARK unbacked authored values in --dry-run, which is invariant 5 shape, and to leave the granted-path rule on profiles. Section 6 still asked whether environ.inherit is a preference or a grant, and leaned toward amending CLAUDE.md. Settled the other way in 1.2: it is a grant, it stays in a profile, and config keeps only defaults and prompt. Also recorded there: path must be retired alongside env, since environ.merge on PATH does the same job and @claude uses both today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A second independent audit re-ran every measurement and read the primary sources behind the prior-art claims. The compression pass came out clean -- code blocks byte-identical, no negation or hedge altered. Everything below came from the restructure or from claims that were wrong when written. The one that matters most is mine. §1.1's heaviest argument for nesting was "multi-line inline tables do not parse, measured in a reference parser and in snug's own". Half false: python3 tomllib refuses them, but the scratch module I used to check go-toml pinned v2.2.3 while go.mod says v2.4.3 -- and v2.4.3 accepts them. Re-verified against the real version. The lesson is worth more than the argument was: check the version the project builds with, not the one a test module resolved to. What survives is smaller and still worth saying -- the flat form is spec-invalid but silently accepted here, so it works on this host and breaks on any TOML 1.0 parser. That is a portability trap and an argument for snug rejecting the form deliberately rather than inheriting whatever the dependency allows this month. §1.2's worked @sys set SHELL and the four base PATH entries, which §1.1 had just declared snug's and forbidden to profiles -- the document contradicting itself one section apart. @sys now sets no environment at all, and a new @rust shows the shape a profile actually wants: grant the directory, merge it. The canonical scalar-conflict error moved off SHELL for the same reason. §2.5's --dry-run block was never updated, though the commit that reworked the model claimed it had been. It showed @stubs-in-path prepending, (config) provenance for verbs that had moved back to profiles, and snug's stub in the front band when §2.3's own diagram puts it after every profile entry. Rewritten: (snug) provenance for snug's own authorship, bands top to bottom in resolution order so the rendering IS the diagram, and dropped sanitise elements named rather than counted. Four sidenote claims were wrong against primary sources. flatpak's empty-means-unset is pre-1.10 behaviour, not current. makeWrapper is not the only system putting the separator in the signature -- Environment Modules has -d and Lmod takes a delimiter. "Everyone offering prepend buys commutativity back with a number" is false for makeWrapper and Modules; the true rule is that a number appears where units are INDEPENDENTLY AUTHORED and no sequence exists, which is exactly snug's case and a better argument. And "everyone except CUE and Kubernetes ships subtraction" conflates removal with override: mkForce and Nickel's force win a priority comparison and never delete. The three-way split makes the conclusion sharper, since borrowing by analogy imports removal from one group and a priority field from the other, and invariant 1 forbids both. Also restored from the restructure: inherit is refused for every list variable (it was a table column that a rewrite dropped, and it is a rule); forbiddenEnv survives the new format unchanged and orthogonal to the type table, which had been re-learned once already; and sanitise with zero survivors unsets rather than setting empty, because an empty PATH is the current directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of the review gaps that needed no further thinking. 3.2 is new: the type table had no scalars in it at all, while 2.1 teaches the rule with a scalar example. So the one case a reader learns from was not in the table it points at, and SHELL, TERM and LANG could not be looked up anywhere. The marks are also defined now -- an earlier draft used checkmark/warning/cross throughout without ever saying what they meant, and warning in particular read as "probably fine" when it means "allowed, with this constraint". Adding the table restored the TZ finding the restructure had dropped, which belongs here anyway: TZ is a two-branch grammar, and when TZDIR is unreachable glibc silently re-reads the value as an inline POSIX rule rather than failing. TZDIR=/nonexistent TZ=Asia/Tokyo gives +0000 Asia -- every timestamp in the sandbox wrong, on no channel at all. It is the best non-XDG example of the document own rule, and it is why the cell is a warning and not a tick. 2.3 is new: nothing said a variable name has to look like one. TOML keys are arbitrary strings, so "A=B" = "c" parses, as does an empty key or one carrying a newline, and all of them reach --setenv. Profile names have checkName for exactly this reason. The = case deserves its own sentence: NAME=VALUE is the wire format of the environment, so a key containing = is a second assignment smuggled inside the first, and the only thing preventing that today is that no key accepts a variable name yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three agents took the grant-check semantics, the verb interactions, and the authorship split. Verified their load-bearing claims before accepting. Two of them disagreed on one point and the disagreement is resolved in the document rather than papered over. The reframing that dissolves most of the grant question: it is a COUPLING rule, not an existence check. internal/policy may not touch the filesystem, a tmpfs grant creates an empty directory, and a bind's contents are host state -- so the rule cannot prove a path exists. What it can do is require that the profile naming a path is the profile that put a node on the chain to it, so a reviewer reading one profile sees both acts. That has to go in the code comment, because otherwise someone will cite the check as a boundary. It is not one. The decisions: coverage rather than exact match, downward, no depth limit; symlinks resolved first and never a grant themselves; the include closure counts and the selected set does not; checked against profile TEXT so an absent optional grant cannot make legality host-dependent; the guest side of a host:guest spec; and a hard refusal for profiles while snug's own values are only marked -- because HOME, PATH and SHELL have no safe absent state and refusing would make the sandbox worse. Where refusing makes the sandbox worse, mark; where it costs only an author's line, refuse. sanitise drops rather than rewrites, because the host-to-guest map is not a function. ro is enough, and the honest scope is narrower than the name: it is a truthfulness filter, not a capability filter. Unset and empty collapse for lists but must NOT be written as a shared helper, because the flag scalars are "set to any value, including empty" and the collapse is exactly wrong for them. The disagreement was the sanitise band's order. Sorted, said one, because host order is an order snug did not author. Host order, said the other, because sanitise's contract is "copy the host value" and GOPATH element 0 is semantic. Host order wins: sorting is a second silent transformation, and the objection it answers -- that the value depends on a host string -- is true either way. Two profiles prepending the IDENTICAL value now agree rather than conflict. They do not disagree about who is first and the resolved policy is byte-identical, so refusing it refuses a non-conflict; and it collapses prepend into the rule set, identity, address, gateway and mtu already share. This softens the stated "once per set" rule and is flagged as such. Also corrected: the same-value exemption's stated justification was false -- expand folds includes into a set keyed by name, so a diamond never reaches an agreement check at all. And the conflict error must name every claimant, which today's scalar conflicts do not. @stubs-in-path is deleted rather than argued into defaults. Its abuse sentence cannot be written, which makes it @null under another name; the two existing gates already select the exact intersection, measured four ways; and putting it in defaults would create a silent-downgrade path, since defaults = [...] replaces the built-in list wholesale. What it was reaching for is telling the human, and --dry-run's COMMANDS block already does that. forbiddenEnv splits by verb. set carries a reviewable file value; inherit is a hole punched in --clearenv. So BASH_ENV and its family are allowed for set under the grant rule and refused for inherit -- which composes the two rules instead of maintaining them separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
They surfaced during the design reviews and were sitting only in commit messages, where nobody looking for open defects would find them. All three measured on main, all three fixable today whether or not the environ format ever lands. §6 says so explicitly, so they are not read as blocked on it. (a) A variable set to the empty string is silently dropped. resolve.go:283 reads `if v := env.Getenv(e); v != ""`, so set-but-empty is indistinguishable from unset: NO_COLOR= snug --dry-run -p @claude . -> no NO_COLOR at all NO_COLOR=1 snug --dry-run -p @claude . -> NO_COLOR=1 NO_COLOR's spec is "set to any value, including empty", so NO_COLOR= means disable colour and snug silently re-enables it. §3.2's flag row already has the semantics right; the code is wrong. The fix is Environ.LookupEnv plus a presence check, which is the same line that carries §4.4's host-conditional refusal -- so both go together. (b) One unparseable file in profiles.d disables the entire registry, builtins included. Load() returns on the first bad file rather than collecting, so a single unknown key makes @sys unreachable and takes down `snug profile list` -- the one command that would tell the user what still works. This is also what would make any future change to the variable type table frightening, since reclassifying one name becomes a total registry failure on every host that used the old verb. The fix shape is the interesting part: diagnostics report the broken file and continue with what loaded, anything that runs a sandbox stays fatal, and `unknown profile` must consult the skipped-file record or it becomes a silent downgrade. (c) PATH entries are not deduplicated and an ungranted directory is accepted in silence -- path = ["/nonexistent/bin", "/bin"] yields /bin twice and no message. Harmless today; worth fixing regardless, because a duplicated entry makes the rendered value depend on how many profiles happened to name a directory, which is a fold artifact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The file was named for how it started, not for what it is. It stopped being a brainstorm several reviews ago -- it now carries decided semantics for five verbs, a variable type table, the resolution order, the errors, and three live bugs recorded against main. A reader looking for the environment design would not have found it under BRAINSTORM. Status line follows: accepted, implementation in progress. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
§1.1 said flatly "no profile may write a name snug writes" and listed PATH. §1.2 worked @rust with environ.merge PATH, §2.4 gives PATH a merge band, and §2.8 renders one. Both cannot hold, and the implementer hit the contradiction on the first profile that used it. The reading that makes every one of those statements true: ownership refuses the verbs that REPLACE a value. For a scalar that is every verb a profile has. For a list, snug authorship is a band, so merge/prepend/sanitise contribute ahead of snug base and displace nothing, while set and inherit are already refused by the type rules. What stays unconditional is the base PATH, not the variable. Verified while writing this: PATH is the only list among the twenty owned names, so the exemption is exactly one variable wide. Also corrected: the list was given as nineteen keys and is twenty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lden
The spec is accepted; this is how it gets built. Twelve independently
committable steps, each leaving `make gate` green, with step 3 named as the
first that changes observable behaviour and therefore the first golden diff.
Four places the design contradicts itself are decided rather than handed to
the implementer:
1. a TOML string on a list variable is one element for `merge` as well as
`prepend`; the refusal is about the SEPARATOR, not the shape, because
PATH = ["/usr/bin:/usr/sbin"] is the identical smuggle and the
shape-based rule in §2.7 case 3 does not catch it;
2. `set` and `inherit` on one scalar are a symmetric conflict — "set beats
inherit" is a priority field wearing a verb's clothes;
3. TERM/TZ/LANG are snug-owned per §1.1, so §3.2's `set` cells for them are
dead and the "TZ requires /usr/share/zoneinfo" guard becomes a --dry-run
mark rather than a refusal;
4. PIP_*/npm_config_* are forbidden for `inherit` only, GIT_CONFIG_*/LD_*/
BASH_FUNC_* for both — §4.5's finding is about the host's environment
outranking a pinned file, which is an argument about inherit.
Three things measured while writing it, not recalled. go-toml v2.4.3 refuses
[profile.x.environ.deny] under strict mode with a caret at the table, so
§1.1(b) holds; it accepts "A=B" and "" as keys under environ.set, so §2.3's
grammar is load-bearing rather than theoretical; and it accepts `inherit =
false`, so the true-only rule has to be code. bwrap refuses `--setenv 'A=B' c`
but names neither variable nor profile, which is why the check belongs at
parse time. `--setenv NAME ''` delivers a present-empty value, so §4.6(a)
needs no new flag.
One finding that changes the shape of the work: the *.bwrap.txt goldens are
generated from testRegistry(), a fake, so nothing in base.toml has a golden at
all. Changing @home and @claude — the user-visible payload of this design —
would ship with no golden diff, which the working agreement says to treat as
untested. So step 1 is a real-registry ENVIRONMENT golden in cmd/snug, built
before any behaviour changes, and every later step is reviewed against it.
Also retracted: §1.1's suggestion that snug reject the multi-line inline
`environ = {...}` form. It is not implementable post-decode — the decoded
value is identical to the header form and go-toml hands us no syntax
provenance. Recorded as a portability note; no comment may claim it is a gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The .bwrap.txt goldens are generated from testRegistry(), a fake profile set, so nothing golden covers internal/profile/profiles/base.toml. A change to what @home or @claude puts in the environment would therefore ship with no diff for a human to read — which CLAUDE.md says to treat as probably untested. cmd/snug/testdata/env.*.txt is the ENVIRONMENT block of --dry-run, resolved against profile.Builtins() with a fake host, for three selections: the shipped defaults, defaults + @claude, and containers on a host whose podman is a distrobox shim. It captures TODAY's behaviour so the later steps of the environ work produce a reviewable diff rather than arriving fully formed. No production behaviour changes: profile.builtins is exported as Builtins (it is the only registry that is a pure function of the binary, which is what makes it goldenable), and dryRun's eight-line environment block moves into describeEnvironment so the test can render exactly it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Policy.Env becomes map[string]EnvVar. A map of strings could carry the values and nothing else — not which verb produced one, not which profile, not where in a list's ordering an entry sits, not what a filter dropped — and every one of those is something --dry-run has to print, because that screen is how a human decides whether to trust the sandbox at all. No new noun: policy.Environ already exists as the injected host lookup, and a policy.Env sitting two letters away from it is the one-word-two-things confusion the (builtin) -> (snug) rename was done to remove. So the writers are Policy methods mirroring Policy.Replace. AuthorEnv/AuthorEnvList are the only writers of a VerbSnug entry, the same way Replace is the only writer of Mount.Authored, and that is what makes the ownership set derivable rather than retyped. policy.SnugOwnedEnv ships as data because the refusal that will consult it runs at parse time, and parse time cannot run a resolve. Two tests keep it honest and neither alone is enough: a static pass over every AuthorEnv call in the tree asserting set equality (it hard-fails on a computed name, because an unchecked name is the hole this closes), and an executed one over a resolved policy. An earlier draft of the list was hand-counted and missed the six writers that run after Resolve, which are the dangerous half. canon() now renders the environment entry by entry rather than as a joined string, and testRegistry gains two profiles that actually use it — widening canon asserts nothing if no fixture exercises it, which is the same trap its own comment already records for the network scalars. No golden diff, and that is expected: this is a pure refactor and adds no capability. A diff here would have meant the refactor was not faithful. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he host Two bugs sharing one line, so they are fixed together. Environ gains LookupEnv, and the profile `env` fold reads presence rather than `v != ""`. A variable the host set to the empty string is SET: NO_COLOR's specification is "set to any value, including empty", so `NO_COLOR=` means disable colour and snug was silently re-enabling it. bwrap delivers it faithfully — `--setenv NO_COLOR ''` produces a present, empty variable inside — so no new flag and no --unsetenv is needed. envOr keeps the old reading deliberately, with a comment saying why: its one caller wants a fallback for USER, where empty is no more useful than unset. The two readings must not be unified — "right for one type, wrong for the other" is how this class of bug ships. The forbiddenEnv refusal moves OUT of the presence guard. It used to fire only where the host happened to have the variable set, so a profile carrying env = ["LD_PRELOAD"] passed review on one machine and failed on another. Whether a grant is legal is a property of the profile. Golden diff, exactly as planned: one new line `NO_COLOR=` in the @claude environment golden (the fixture host now has it present and empty), and one new unconditional case in refusals.txt. Nothing else moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…verb snug owns the variable types, because the alternative is inferring an operation from a value's shape — and the shape is identical for a search path, a URL, a template bash performs command substitution on, and a set of delimiter characters. internal/policy/envtypes.go is §3.2, §3.3 and §3.4 as data, with the empty-element kind alongside the separator: a type that carries one without the other is a filter written once and wrong for a third of its inputs. checkEnvName is checkName for variables, and for the same reason. TOML keys are arbitrary strings, so nothing in the syntax stops environ.set carrying "A=B", an empty key, or a name with a newline, and those go straight to --setenv NAME VALUE. '=' gets its own message: NAME=VALUE is the wire format of the environment, so a name containing one is a second assignment smuggled inside the first. bwrap does catch it, but its message names neither the variable nor the profile nor the file. forbiddenEnv splits by verb (CALL 4). A `set` carries a value from a reviewed file in a trusted layer; an `inherit` carries whatever the process that launched snug had. inherit is a hole punched in --clearenv and set is not, so BASH_ENV, NODE_OPTIONS, PIP_* and npm_config_* are legal as set and refused as inherit, while LD_*, GIT_CONFIG_* and BASH_FUNC_* are refused at both. The prefixes are the half a map[string]bool could never express, and four of §4.4's findings are exactly that shape. ONE PLACE THE DESIGN CONTRADICTS ITSELF, resolved and flagged for review. §1.1 says no profile may write a name snug writes, and PATH is in that list — but §1.2's worked @rust profile, §2.4's whole band diagram, §2.8's rendering and step 12's migration message all require `environ.merge PATH = [...]` to be legal. There is one reading under which every one of those holds: ownership refuses the verbs that REPLACE a value, and for a list snug's authorship is a BAND rather than the value. So set and inherit on PATH are refused by the type rules (naming merge/sanitise as the fix, which is §2.1's own wording), while merge and prepend contribute a band ahead of snug's base and displace nothing. Every scalar snug owns stays refused at every verb. §1.2 is precise about this scope where §1.1 is loose: what stays unconditional is "the base PATH", not the variable. checkEnvOwnership carries the argument and TestPATHIsSharedButNotReplaceable pins both halves. Applied immediately to the legacy `env = [...]` key, which is semantically inherit. Its forbidden-name refusal keeps its older wording, because that text is pinned in the refusals golden and the key is on its way out. Golden diff: refusals.txt grows by one case per rule and no existing line changes. The env.*.txt goldens do not move, which is the check that the forbid table is not too wide — @claude inherits ANTHROPIC_BASE_URL, EDITOR, VISUAL, PAGER and NO_COLOR, and none of them is forbidden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`environ` is one nested key rather than five root keys, and the load-bearing reason is that it makes `environ.deny` fatal for free: `environ` is a struct with known fields, so DisallowUnknownFields catches an unknown VERB exactly as it catches an unknown root key. "A negation key cannot be smuggled in" now applies one level down with no new code, and there is a test for each spelling someone might reach for. merge and prepend decode into `any` because that is what go-toml v2.4.3 actually accepts both spellings into — measured, not assumed. Both are legal: a bare string is exactly ONE element, because snug never splits a value on a separator (CALL 1). The converter says which profile, which file, which verb and which variable for anything else; go-toml's own error names none of them. inherit and sanitise take `NAME = true`, and `NAME = false` is refused by name. A stored false would be a negation key that parsed — there is no way to un-inherit, because nothing was inherited to begin with. `env = [...]` and `path = [...]` are REWRITTEN into inherit and merge-on-PATH rather than carried alongside, so policy.Profile loses both fields and the resolver reads only EnvGrants. The retirement warning is silent for a builtin, on purpose: it names the file to edit, and for a builtin that is a file inside the binary — a warning nobody can act on trains people to ignore warnings, and snug's own use of the keys is caught by its test suite the day they turn fatal. ValidateEnvGrants runs in parse, beside checkName, so `snug profile show` reports a verdict with no target and the answer never depends on the host reading the file. It runs in Resolve too, because a Profile built in code never went through a parser and a gate on one of two paths is not a gate. Two consequences worth reading rather than skimming: - A verb snug can parse but cannot yet apply is REFUSED, not accepted and ignored. set, prepend and sanitise resolve in the commit that follows this one; until then a profile using one gets an error naming the verb. Accepting a grant and silently doing nothing with it is the failure mode this project treats as its worst. - `snug profile show` now renders all five verbs. It had to change anyway, and the old line rendered `env` and never rendered `path` at all — so a profile putting a directory on the sandbox's PATH looked, on that screen, like a profile that granted nothing to the environment. Golden diff: one line. The forbidden-name refusal now speaks with checkEnvName's single voice instead of the legacy key's older wording, which is the point of the rewrite. No env.*.txt or .bwrap.txt file moves — the rewrite is value-identical, and the review artifact for this step is the parse test table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e filter The engine, in internal/policy/envresolve.go. CONFLICTS ARE ACCUMULATED, THEN CHECKED. Every profile's claims go into a set during the fold and nothing is decided until the fold completes, because §2.7 requires a refusal to name EVERY claimant and a decision taken mid-fold can only name the two it happened to be holding. Today's scalar conflicts do exactly that: with three profiles where two agree, the message names the alphabetically-last agreeing profile and never mentions the first. Checking afterwards also leaves no fold order to keep order-independent. CALL 2 lands here: `set` and `inherit` on one scalar are ONE slot. "set beats inherit" would be a priority field wearing a verb's clothes, so they join the rule `set` already follows — equal claims agree, unequal claims are a symmetric error naming both verbs. An inherit of a name the host does not have contributes nothing and therefore never conflicts. BANDS are structural: prepend, then merge sorted, then sanitise in host order, then snug's generated band, then the base. Nothing a profile writes chooses its band. Sorting the merge band is not a tie-break dressed up as a decision — `merge` is where an author declined to say who is first, so ordering it by anything the fold saw would invent an answer nobody gave. SANITISE, with each decision separately asserted: drop and never rewrite (the host->guest map is not a function); `ro` is enough, no stat — it is a truthfulness filter, not a capability filter, and a surviving element may name an empty bind; survivors keep HOST order, because sorting would be a second silent transformation and §3.3 documents a variable where position is semantic; unset and empty both mean absent FOR LISTS, written inline rather than in a shared helper because the collapse is exactly inverted for flag scalars; and nothing surviving means the variable is UNSET, never set-empty. That last one is the single place in this feature where getting it wrong ADDS a hole rather than failing to close one — an empty PATH element is the current directory, which inside snug is the target. DEDUP to the earliest band runs last, after snug's own bands, because the duplicate that actually occurs is a profile's entry against the base. This is §4.6(c): `path = ["/nonexistent/bin", "/bin"]` used to yield /bin twice. Monotonicity is stated precisely in the file's doc comment, both halves. The entry set of a list only grows; the ORDER does not, because a prepend demotes another profile's merged entry. TestEnvIsMonotoneAsASet and TestPrependReordersWithoutRemoving pin them separately, the second existing so nobody reads the first as proving more than it does — the same relationship TestDeeperGrantOverridesShallowerAccess has to TestResolveIsMonotone. testRegistry gains fixtures using all five verbs, and they are in TestResolveIsCommutative's list: a claim accumulator is the newest thing in the resolver that could make resolve([a,b]) != resolve([b,a]), and canon() only covers it if a fixture exercises it. Golden diff: refusals.txt gains the four conflict cases. env.*.txt and the .bwrap.txt files do not move — no builtin uses a verb yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
resolveVia answered two different questions with one body and got the second one by accident. It returned the FIRST match Go's map iteration produced, which is nondeterministic the moment one link prefixes another — /lib and /lib64 on a usr-merged host is the shipped case, not a hypothetical — so a verdict could differ between two runs of the same policy. A security tool that is wrong reproducibly is better than one that is wrong occasionally. It also skipped g == link, which is right for a MOUNTPOINT (a grant at the link path is the link itself, and bwrap's refusal is about creating a mountpoint at a symlink DESTINATION) and wrong for a PATH element that is literally /bin. The coupling rule in the next step needs the second reading: on a usr-merged host /bin is a symlink, and judging a value unrewritten would refuse a profile that granted /usr and named the path the sandbox actually sees. So: resolveViaDeepest, which skips the link itself and replaces the existing call site, and resolveLinkForEnv, which matches it and rewrites. Not one function with a bool — the two comments have to be able to say which question each answers. No behaviour change for any shipped profile; the determinism test runs 200 iterations because a single call would have passed about half the time, which is the worst kind of green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
§2.5, and the framing matters more than the code: this is a COUPLING rule, not an existence check. The profile that names a path must be the profile that put a node on the chain to it, so a reviewer reading one profile sees both acts on one screen. It cannot prove the path exists — internal/policy may not touch the filesystem, a tmpfs grant creates an empty directory, and a bind's contents are host state — and the value is inert either way, since the payload can set any variable it likes once it is running. It stops a profile LYING; it does not stop anything REACHING, and envcoupling.go says so in its first paragraph because otherwise someone will cite it as a boundary. Scope is values a profile WRITES (set/merge/prepend) at names the type table marks path-valued. inherit and sanitise are exempt — the value is the host's, and sanitise already has a filter for exactly that question. Two decisions worth reading. Coverage is downward with no depth limit, which makes @home a rubber stamp for all of $HOME and makes the check vacuous under /usr for anything including @sys — accepted, because that profile DID bring them, on an include line --dry-run renders, and a false positive costs a variable naming an empty directory rather than a hole. And the include closure counts while the SELECTED SET DOES NOT: if it counted, Resolve([a]) could refuse what Resolve([a,b]) admits, so one profile would decide another profile's legality. TestCouplingVerdictDoesNotDependOnTheSelectedSet exists because that is a two-word edit nobody would notice in review. Every fixture that named a path it did not grant is now a fixture that grants it — eight of them, which is the rule doing its job on the first day. The one message this borrows rather than writes is the relative-path refusal: checkAbsoluteElement already owned it, so a relative `set` on a path variable now reaches it too, which it never did before. Golden diff: refusals.txt gains four cases and nothing else moves. The .bwrap.txt and env.*.txt goldens are unchanged, which is the expected shape — every shipped profile already satisfies the rule, @claude through its include of @home. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The environment block was a flat NAME=value list, which cannot say three things a human needs in order to trust it: which verb produced a value, which profile supplied it, and — for sanitise — what was removed on the way. §2.8 is those three, and the PATH bands now read top to bottom in resolution order, so the rendering IS the §2.4 band diagram. If the two ever disagree the renderer is lying, and a flat list could not disagree only because it said nothing. Dropped host elements are NAMED, not counted. A filter that silently removes two of three elements is the exact shape of failure this whole model exists to avoid, and "1 of 3 kept" does not let anyone check it. A variable whose every element was dropped is UNSET, and the block says that too rather than leaving a hole where a name used to be. The other half is §4.2's repair, measured on main and never rendered: snug authors HOME, SHELL and the four base PATH entries in every sandbox, and with only @parent-ro selected not one of those paths is granted. It must keep authoring them — §4.3 shows PATH and HOME have no safe absent state, and an earlier draft that concluded the opposite would have converted a confusion bug into a reachable hole — so the repair is to MARK them. The mark is computed against the resolved mounts, unlike the coupling rule of the previous step, which is deliberately text-only: refusing must not depend on the host, marking must. One predicate, exported (policy.GrantsGuestPath), shared with the sanitise filter, because two implementations of "is this granted" eventually disagree and the one on screen is the one a human trusts. It alone would call /run/snug/bin "not granted" — true in the policy's vocabulary, misleading on the line whose whole point is that the podman stub is found there — so the mark names how many grants sit inside the path. Same predicate, no second rule. Golden diff: env.*.txt rewritten wholesale, which is the review artifact for this change, plus a new env.parent-ro-marks.txt holding the §4.2 case. That selection is REFUSED, and --dry-run renders a refused policy by contract, which is the only way to see the marks at all. VERIFY.md gains both by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s the two retired keys
Four XDG variables, set by the profile that creates the directories they name.
The coupling is the point: they sit three lines under the `tmpfs` list, so a
reviewer reading @home sees both acts at once, and §2.5's rule is what makes that
structural rather than a convention somebody remembered.
@home gains a fifth tmpfs, {home}/.local/share, and it is a real grant change
with its own abuse sentence. Without it XDG_DATA_HOME would name a directory the
sandbox does not have — which is the coupling rule doing its job on the very
first profile to use it, rather than a rule bent to fit.
They are `set` and never `inherit`. The host's XDG_CONFIG_HOME names a real
directory full of real credentials; admitting it past --clearenv points every
tool in the sandbox at a path the sandbox does not have, and on a host that moved
it somewhere @home does not cover, tools fall back to writing wherever it says.
@claude's `env = [...]` becomes environ.inherit and `path = [...]` becomes
environ.merge on PATH. The rewrite is value-identical — the goldens move by
exactly the four XDG lines and nothing else, which is what says so — and every
sentence of the ANTHROPIC_API_KEY reasoning survives the move, because that
comment is the reason the list is five names rather than a prefix.
The fake @home in testRegistry now matches the shipped one entry for entry. It
had two of the five tmpfs directories and none of the XDG block, so the .bwrap.txt
goldens — the artifact a human reads to approve a boundary change — were
describing a sandbox no user gets. Hence three new --tmpfs lines and four new
--setenv lines there, in a step whose production change is a profile.
Verified by execution, not inferred: `snug . -- printenv | grep XDG` shows all
four inside, and ~/.local/share is a fresh empty tmpfs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…says it renders all five
Most of this step landed with the parser: deleting Profile.Env and Profile.Path
broke `show("env", p.Env)` at compile time, so showEnviron was written then. What
was missing is the part that makes it readable and the part that keeps it honest.
The labels now carry the `environ.` prefix the TOML uses. Bare "set" and "merge"
sit directly under "ro" and "tmpfs" on this screen, where they read as two more
kinds of filesystem grant — and "environ.prepend" is also the string somebody
will grep for when they want to find every ordered claim on a host. The label
column widens from 10 to 16 to fit "environ.sanitise".
The test is the point of the commit. The line this all replaced rendered one of
the two keys that existed and never rendered `path` at all, so a profile putting
a directory on the sandbox's PATH looked, on this screen, like a profile that
granted nothing to the environment — and nothing failed, because nothing checked.
A display that omits a grant is worse than no display, because it is read as
complete. The negative control is in there too: a verb nobody used must not print
an empty heading, or the reader learns to skim the block.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured on main: Load() returned at the first bad file, so a single stale file in profiles.d made `snug --dry-run -p @sys .` fail — and made `snug profile list`, the one command that would have said what still works, fail with the same error. A file the user may not have edited disabled the builtins. This is a prerequisite rather than a cleanup, and the sequencing is the argument. The environ work adds a whole new class of parse error to files users already have, and retiring `env = [...]` turns a key that is in shipped documentation into a hard error. With this unfixed, that sequence means one stale user file takes down @sys and `snug profile list` at the same moment. The split is by CONSEQUENCE, not severity. Running a sandbox stays fatal on any bad file: the file that did not parse may be the one granting what was asked for, so continuing would assemble a sandbox out of whatever happened to load, which is invariant 5's exact shape. A diagnostic — profile list/show/tree/dot, config, doctor — reports the file loudly, continues with what did load, and still exits non-zero, because the output is worth printing and the exit code must not claim everything is fine. A REDEFINITION stays hard everywhere. Two files claiming one name is a question with no answer, and continuing would mean picking one silently; it has its own test so the two failures do not blur. And the caveat without which the whole thing is a silent downgrade: `unknown profile` consults the skipped-file record. A name defined in the file that failed to parse would otherwise come back as "unknown profile", which is a lie — snug does not know whether it exists, and the difference between "you typed it wrong" and "the file defining it is broken" is the whole of what the user needs. The registry test carries a positive control that is the point of it: @sys must still be there. Without that assertion it passes on a registry that loaded nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d them Both keys are now a hard error. They stay as FIELDS on rawProfile rather than being deleted, and that is the whole design of this commit: deleting them would let DisallowUnknownFields produce the generic "unknown key", which is right for a key that should never have existed — publish_auto was retired exactly that way — and wrong for a key whose MEANING MOVED. `env = [...]` is still a thing a profile wants to say; the author needs the new spelling, not the news that a word does not exist. So each error prints the replacement block with the profile's OWN variables in it, ready to paste, and says what changed beyond the spelling: `inherit` is now checked per name (a value that is code is refused, a list variable is refused outright in favour of environ.sanitise), and a `merge` on PATH now has to GRANT the directories it names. Both of those would otherwise surface later as a refusal from a rule the author has not met yet. The prefix changed deliberately. A silently CHANGED meaning is worse than a removed key: anyone whose muscle memory reaches for `env` gets an error naming `environ.inherit` rather than a subtly different grant that parses. Each test carries a positive control — the replacement spelling parses — because without it the refusal reads as a ban on the CAPABILITY rather than on the retired spelling, which is the control TestRetiredPublishAutoIsAHardError already carries. Sequenced immediately after §4.6(b) and not before it: turning a key that is in shipped documentation into a hard error, while one such file still took down @sys and `snug profile list` together, would have been an unacceptable migration. Verified by hand: a stale file now names itself, prints the fix, and `snug profile list` still lists every builtin. TODO.md gains the two things this work leaves open — the multi-line inline `environ = {…}` form, which parses here and cannot be detected post-decode (and which no comment may claim snug refuses), and §4.5's pre-existing finding that the environment outranks the pinned config file, recorded now so nobody rediscovers it as a regression of this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…load's own name Both are on the design's "Tests this needs" list and neither existed. TestSanitiseNeverLeavesAnEmptyPATHElement pins §4.3/§2.2 — the one place in this design where getting it wrong ADDS a hole rather than failing to close one. An empty element in PATH is the current directory, and inside snug the current directory is the target: the one writable thing a hostile payload controls. The host PATH in the fixture has four elements of which policy grants exactly one, so a `sanitise` written as a string replacement has three gaps to leave behind. The positive control is the last line of the payload: with an empty element deliberately present the planted binary IS found, so "the sandbox did not run ./snugvictim" cannot pass on a sandbox that never started or on a binary that was never executable. Proved it fails: adding one line to sanitiseHostList that records an empty entry for each dropped element — the naive string-replacement bug — turns PATH into ":<granted>:/usr/bin:..." and the test reports the leading empty element, the empty element by index, AND the behavioural half: VICTIM-RAN in the bare section, meaning a file in the project root ran off the sandbox's own PATH. TestThePayloadNameResolvesAgainstTheSandboxPATH pins §4.1's precondition, which was measured true and asserted nowhere: `snug . -- podman` resolves against the sandbox's PATH, not the host's. Three parts — a binary only on the host's PATH must not run, one in a directory a profile grants and merges must (its positive control, identical invocation shape), and a name present in both resolves to the profile's. Part three is the one that earns its place. Refactor the lookup to a host-side exec.LookPath and parts one and two stay green — one still fails, which reads like the feature working. Proved it fails: with exec.LookPath applied to p.Command[0] in internal/sandbox/exec.go, part three goes red with "bwrap: execvp <hostonly>/snugbothmarker: No such file or directory", the host's copy having won the lookup and then not existed inside. Every planted binary emits its own marker, and writeScript executes it on the host before the test relies on it, so a fixture that could not have run anywhere is a named precondition failure rather than a green tick. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
inheritEnv and mergeEnvList landed in step 2 with callers and lost the last one in step 6, when the `prof.Env` fold was replaced wholesale by the resolver — which writes through addEnvEntry directly. Verified: the only occurrences left in the tree were their own definitions and doc comments, and no interface anywhere declares either name, so there was no dynamic caller either. Unexported methods, so nothing forced the issue: the compiler does not mind and `go vet` does not look. The reason to remove them now rather than never is that they are WRITERS of environment entries with the verb baked in, sitting one call away from the resolver that is supposed to decide the verb. addEnvEntry says so where it stands, so the next reader knows this was a decision and not an omission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A redteam run reached git's transport through GIT_SSH while GIT_SSH_COMMAND --
its exact equivalent -- was refused two lines above it in the same table. Then it
hijacked a real `git fetch` in a sandbox whose ssh identity a DIFFERENT profile
had pinned:
snug -p work -p helper <tgt> -- git fetch origin
HIJACKED-GIT-TRANSPORT host=git@github.com … SSH_AUTH_SOCK=/run/snug/ssh-agent.sock
`helper` granted no filesystem path at all. That is one profile defeating a
guarantee another profile established, which is the composability case the table
exists to prevent.
Verified before fixing: GIT_SSH, GIT_ASKPASS and GIT_SEQUENCE_EDITOR all reached
the sandbox, and GIT_ASKPASS even rendered with "← not granted" beside it while
being passed anyway. GIT_SSH_COMMAND failed the same profile at parse time.
The rule was never "the newest spelling"; it is "the value is code". Added as
forbidBoth: GIT_SSH, GIT_PROXY_COMMAND, GIT_ASKPASS, SSH_ASKPASS,
GIT_SEQUENCE_EDITOR, and the same class in other runtimes -- JAVA_TOOL_OPTIONS,
_JAVA_OPTIONS, JDK_JAVA_OPTIONS, RUBYOPT. Deliberately NOT a blanket "forbid all
code": BASH_ENV, NODE_OPTIONS, PYTHONSTARTUP and PERL5OPT stay forbidInheritOnly,
because as `set` they carry a reviewable value from the trusted profile layer and
the coupling rule constrains where they point. These nine are peers of a name
already in forbidBoth.
The comment says why rather than what, and names the measurement, because
forgetting one entry here is indistinguishable from allowing it.
Regression test extends the forbidBoth table in envtypes_test.go. Confirmed the
refusal fires and that a legal `environ.set EDITOR` still resolves, so the test
cannot pass on a resolver that refuses everything.
Scope, honestly: against a payload already executing this adds nothing -- it can
set these itself. The weight is that it fires when a HUMAN or a different agent
later runs git inside, and that it lowers the bar for the planned §2.7 defence,
which will classify ro=["/"] as privileged but not environ.set GIT_SSH.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…CLAUDE.md Three things, all in the same direction: the documents narrated as live a state the implementation had already changed. §4 now says outright that its measurements are the evidence that argued for the design, not a description of this tree, and each closed subsection carries a `Closed by` line naming the commit. §4.4 (host-conditional refusal), §4.6(a) (set-empty dropped), §4.6(b) (one bad file takes down the registry) and §4.6(c) (PATH not deduplicated) are all closed here; nothing else in §4 is. §2.1 described the forbid-both set as "names snug owns, LD_* and BASH_FUNC_*", which under-describes what the code enforces by some distance: two more prefixes, the ten glibc strips, the git/ssh transport hooks, the interpreter option channels and the prompt hooks. It also never mentioned PIP_* and npm_config_* being inherit-only. The summary now matches the table, and says which one runs. CLAUDE.md told the next implementer to stage a substituted binary in "the writable tmpfs $HOME" and put that directory first on PATH. That is a shadow slot: a writable directory ahead of /usr/bin means the payload writes a file called git and the next git anyone runs inside is that file. The live case has always staged at /run/snug/bin, read-only after --remount-ro, so the code was right and the rule was wrong. Stated with the reason, because the same reason is about to decide what sanitise does with a host PATH element under a tmpfs.
`GrantsGuestPath` returned true for any covering mount regardless of its kind,
and `sanitiseHostList` kept every element it approved. `/tmp` is a tmpfs in every
policy and `@home` makes `{home}` and four subdirectories tmpfs too, so a host
PATH carrying `/tmp/x/bin` survived into the PATH snug writes — at a directory
that is empty and writable inside, in a band ahead of `/usr/bin`. The payload
creates the directory, drops a file called `git` in it, and the next `git` a
human or another agent runs inside the sandbox is that file. Reproduced end to
end, marker SHADOWED-GIT-RAN. Reachable only through a user-written
`environ.sanitise = ["PATH"]`, because no shipped profile sanitises anything.
The filter's contract is "copy the host value, keep only what policy grants", so
the question at each element is whether the sandbox has the HOST'S CONTENT at
that path. A tmpfs answers no: it grants an empty directory. So this is not a new
rule, it is the existing predicate giving a correct answer to the question it
already asks — and `ro` is still enough, no stat, no mode bits, internal/policy
still pure.
The wider candidate — drop anything whose covering mount is writable — was
rejected, and the reason bounds what this claims:
the payload can rewrite PATH inside the sandbox at will, so no filter can
close the shadowing attack. What the filter owes is that the environment SNUG
ITSELF hands over does not ship the shadow slot pre-installed. Dropping every
writable element chases an invariant the sandbox cannot hold; this makes
snug's own output truthful.
It would also drop the target's own `bin/`, which is a truthful element: a real
bind of real host content. A writable bind therefore still survives.
The walk is factored into `coveringMount` and `GrantsGuestPath` delegates to it,
its own semantics unchanged for every input it has ever been asked. `grantMark`
is deliberately untouched: --dry-run's mark asks whether a node exists, the
filter asks whether the host's content is there, and a tmpfs answers yes to the
first and no to the second. Unifying them would print "not granted" beside
@claude's `{home}/.local/bin`, which is false, on the screen that is the whole
mechanism by which a human trusts snug. One walk, two predicates, one exported.
The deepest covering mount decides, which stopped being incidental the moment the
kind mattered: a bind nested inside a tmpfs is kept and the tmpfs directory above
it is dropped. `keepHostElement` switches exhaustively on Kind and fails closed
on one it does not name.
`EnvDrop` gains a `Reason`, carried from the decision rather than recomputed at
render time, and --dry-run groups the dropped-element lines by it. "Nothing
grants that path" and "only an empty writable tmpfs is mounted there" are
materially different facts, and the second is the one that otherwise reads as a
bug ("/tmp/x/bin vanished but /tmp plainly exists inside").
No golden moved, exactly as the design predicted, because no shipped profile
sanitises anything — which per the working agreement means this change is
currently untested. The named tests and the golden that exercises the new band
are the next commit, and TODO.md's claim that
TestSanitiseMonotonicityRestsOnRejectMasking asserts the rejectMasking coupling
is not true until then.
… the band The change in effeceb moved no golden, because no shipped profile sanitises anything — which by this project's own working agreement means it was untested. This is the part that makes it reviewable. internal/policy/testdata/sanitise.bwrap.txt is new, and it exists so the security diff is one readable line: --setenv PATH /opt/tools/bin:/home/u/.local/bin/tool:/home/u/proj/sub/bin:/usr/bin:/bin:/usr/sbin:/sbin /tmp/x/bin, /tmp, /home/u/.local/bin and /srv/nothing were all in the probe host PATH and none of them is there. The fifth case in TestGoldenBwrapArgs needs a per-case host environment to supply that probe value, so the table gains an `env` field; the shared fixture deliberately still has no PATH, or every future case would inherit one. The eight tests, and what each is for: DropsAShadowSlotOnlyATmpfsCovers the named regression. Asserts the element is absent from the argv, not merely from Entries — those are two claims and only the second is the one that matters KeepsAnElementARealBindCovers the positive control, without which the first passes on a filter that drops everything. Its target-bind assertion is what pins this fix rather than the wider "drop anything writable" that was rejected UsesTheDeepestCoveringMount both directions in one test: the tmpfs directory drops, the bind nested inside it is kept. An implementation that kept an element because some mount exists at or below it passes half of this and fails half DropReasonDistinguishes... the two reasons are not conflated, which is what makes --dry-run's new line honest MonotonicityRestsOnRejectMasking TODO.md claimed this test asserted the coupling before it existed. It does now: a tmpfs beneath a bind is refused, and that refusal is the only reason adding a profile cannot turn a kept element into a dropped one EmitsTheHostElementVerbatim... guards drop-never-rewrite against a well-meaning "clean the path" fix GrantMarkStillUsesTheWiderPredicate guards against unifying the two predicates. The mark asks whether a node exists, the filter asks whether the host's content is there DropLinesNameTheirReason the rendering, on a hand-built policy with no host dependency The positive control was verified by deliberately breaking keepHostElement to return false unconditionally: KeepsAnElementARealBindCovers failed on its target-bind and nested-bind assertions, and so did the two older sanitise tests the brief required to stay green — so those are real controls too, not decoration. One correction to the design's illustrative text: the verbatim test cannot look for a cleaned /usr/bin in all of Entries, because snug's own base band contributes a real /usr/bin. It filters Entries to the sanitise band, which is what actually pins the clean-to-decide, emit-verbatim split. TODO.md also gets two stale references corrected. ANTHROPIC_API_KEY is no longer injected into the environment at all, so the secrets audit's row 3 says closed rather than describing a live leak; and forbiddenEnv has moved to internal/policy/envtypes.go and is split by verb. The credential-shaped-name refusal that entry proposes is still open, and its counter-example is gone — which argues for writing it now, while it costs nothing that ships.
…ry-run line Found against a8652ba, after the tmpfs fix had shipped and claimed the class was closed. It was not. FINDING 1 — /proc and /dev re-installed the shadow slot. keepHostElement kept KindProc and KindDev, justified as "kernel- and bwrap-populated, not empty". That is true of the DIRECTORY and false of what /proc's magic symlinks RESOLVE TO. The filter is lexical and deliberately does not follow symlinks, so coveringMount stops at /proc while the kernel walks /proc/self/root/tmp/x/bin to the writable tmpfs and /proc/self/cwd to the target — where the shadow binary also persists to the host after the sandbox exits. Both reproduced end to end, markers SHADOWED-GIT-RAN-VIA-PROC-ROOT and SHADOWED-GIT-VIA-PROC-CWD. The argument for the fix had already been written down as an argument against it: TODO.md declined to widen the rule to KindDev because "an element under /dev on a host PATH does not occur". If it never legitimately occurs, keeping it costs nothing to drop and serves only an attacker's spelling. Both kinds now drop, under their own DropPseudoOnly reason — the fact is not the tmpfs one wearing another name, because the directory really is populated; what makes the element untruthful is that its magic links leave it. KindSymlink still survives, and the line is principled rather than convenient: a KindSymlink is authored by a GRANT and points where that grant says, so following it would be a second resolution rule with its own failure modes. /proc's magic links are authored by the KERNEL, point at whatever the reading process has open, and are not a grant at all. The lesson generalises past this filter, and is the same shape as the dirfd bullet in CLAUDE.md: a lexical predicate answers about the path it was handed, the kernel answers about the path it resolves, and wherever the two can differ is where the attack is. Probed and NOT exploitable: `..`, trailing slashes, repeated slashes — coveringMount cleans before it walks. FINDING 2 — a newline in a host PATH element forged a line in --dry-run. The drop line rendered the host-supplied value verbatim, so an element carrying a newline split the line and the injected second line read as a legitimate ENVIRONMENT row, complete with a plausible provenance column. It escapes nothing, but --dry-run is the mechanism by which a human can trust snug at all, and a value that can author a row in it is a hole in the trust artifact. internal/policy already applied exactly this guard to variable NAMES in its error messages (quoteVisible); the values had no equivalent. visibleValue now covers kept entries as well as dropped ones, because a host element under a bind survives the filter and can carry a newline just as easily. A value with no control characters renders unchanged, so no golden moved. Both findings have permanent named tests, and both were verified to FAIL against the pre-fix code before being confirmed against the fix: the pure table asserts the reason rather than only the verdict, and its end-to-end companion asserts the element is absent from the --setenv PATH operand bwrap actually receives, not merely from p.Env. The control-character test carries a newline in a KEPT entry too, since visibleValue has two call sites and a fix at only one would look identical on the drop line alone. VERIFY.md gets 6f, the by-hand /proc/self/cwd reproduction, run and its real output pasted.
…s away .claude/design/ENV-IMPLEMENTATION-PLAN.md is deleted. It was scaffolding — a twelve-step sequence for work that is now done, and the kind of document that rots fastest, because every step it describes is better read as the commit that performed it. What was worth keeping is in ENVIRONMENT-VARIABLES.md, which describes the design rather than the route to it. Nothing referenced the plan. The README gains an "Environment variables" section. It had none, and the environment is now as much of the boundary as the mounts are: the five verbs, the band order that decides what beats what, the conflict rule, and the four rules that catch a profile author out — grant what you name, names whose value is code are refused, names snug writes cannot be replaced, and sanitise filters harder than "is it granted". Every claim in it was executed rather than recalled, and the first draft was wrong twice as a result. The inherit example used TERM, which snug owns and which is refused outright; it now uses COLORTERM. And a bullet listed PATH among the names a profile cannot write, which contradicts the merge example three lines above it — ownership refuses REPLACEMENT, and PATH is a list, so contributing to it is exactly what merge and prepend are for. The section also carries the grant line the example needs to parse, since the coupling rule refuses a PATH entry naming a path the profile does not grant. ENVIRONMENT-VARIABLES.md joins the design table at the bottom of the README.
`sanitise` is easy to read as hygiene — "clean the unwanted references out of PATH" — and in this codebase that reading is backwards. The floor is an empty environment, so nothing of the host's is inside to be cleaned; `sanitise` is the list counterpart of `inherit`, and it ADDS. A builtin using it would give every sandbox host-derived entries on behalf of a human who never asked. So the bound is that the host's environment enters only where a human on this host wrote a profile saying so. What snug ships is the knowledge rather than the selection: forbiddenEnv refuses the names whose value is code at every verb, and envTypes' `sanitisable` column says which lists may be filtered at all. "No builtin sanitises" was true and accidental. Now it is a decision with a failure message that says what to do instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is the screen someone reads to decide WHETHER to select a profile, which puts it upstream of every --dry-run, and it had no golden. The gap was not theoretical: relocating @claude's binary changed ro {home}/.local/bin/claude -> {home}/.local/bin/claude:/run/snug/bin/claude and produced no test diff at all. TestProfileShowRendersEveryEnvironVerb calls showEnviron directly with a hand-built EnvGrants, so it pins five label strings and never exercises the header, the grant keys, `optional` or the layout. Three profiles for three distinct shapes: @sys (ro/rw/tmpfs and symlink, no environ), @claude (the host:guest pair, includes, inherit, optional) and @podman-socket (the include closure, and the interim @net include whose eventual removal now has a second place to be conscious). It drives the real profileCmd rather than an extracted renderer, so the exit code and the profile.Load() path are covered too — a golden built from a refactored-out helper would pass while the command printed something else. Also: an unknown name must be a named error on stderr with a non-zero exit. "Printed nothing, exited 0" reads as "this profile grants nothing", which is the worst answer because it looks like information. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y is writable @claude bound its binary read-only at {home}/.local/bin/claude and merged that DIRECTORY onto PATH. The bind was sound; the directory is @home's writable tmpfs, so snug was handing over a shadow slot pre-installed — the payload writes a file called `git` there and the next `git` a human or another agent runs inside the sandbox is that file. It passed review, passed `make gate`, and sat in TODO.md as an accepted residual under a defence ("a profile's own declaration") that only applies to profiles a human wrote. `sanitise` structurally cannot reach it: that filter only ever judges the HOST's value for an imported variable, never a `merge` entry written in a file. The rule, now stated once and enforced: snug never mounts an executable anywhere the payload can write. PodmanStubDir becomes StagedBinDir, because it is no longer podman's — every executable snug puts in front of the payload goes in that one directory, whether snug generated it or a profile bound it. It sits on the root tmpfs, so --remount-ro / covers it (measured: touch and `echo >` both EROFS). No profile names a PATH directory at all any more; snug adds the staging directory itself, iff something is staged there, so a profile cannot pick a writable one by accident because it does not pick one. And the screen now says it. --dry-run was rendering two entries with the identical property in opposite ways, four lines apart: a profile's merge of a writable directory kept and unmarked, directly above a sanitise drop line explaining that a writable directory is a shadow slot. Both were correct and the asymmetry was invisible, which is exactly where {home}/.local/bin sat for a milestone. Marks are PATH-only — those entries are searched for COMMANDS, while a writable CARGO_HOME is what that variable is FOR, and marking it would teach the reader to skip the mark where it matters. It stays a mark, not a refusal. A human's own profile may do this deliberately; what snug may never do is ship one, and that is enforced separately and absolutely by a sweep over the builtins. Tests: TestNoBuiltinPutsAWritableDirectoryOnPATH with a positive control that reconstructs the old @claude shape and asserts the predicate fires on it; TestSnugStagesNoCommandInAWritableDirectory end to end (mkdir -p before touch, because a PATH element that does not exist yet on a writable tmpfs is still a slot and probing with touch alone fails with ENOENT, which reads as a refusal); TestWritableMarkIsPathOnlyAndDistinctFromNotGranted. Both new tests were confirmed to fail when the old merge line is restored. describeCommands now enumerates the staging directory instead of hard-coding the podman stub — it had gone stale the moment @claude's binary moved there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--setenv NAME VALUE` is three elements of a flag list snug NUL-joins into the
args memfd, and bwrap's --args splits it on NUL. VALUE is last in the triple, so
a NUL inside it re-syncs bwrap's parser onto the remainder. Measured:
EDITOR = "vim\u0000--ro-bind\u0000~/.ssh\u0000~/.ssh"
mounted the host's ssh keys into the sandbox, while --dry-run listed ~/.ssh
under NOT GRANTED and the FILESYSTEM block showed no such line — there was no
Mount, so Validate, rejectMasking and the provenance model never saw it. The
same shape with --tmpfs masked @sys's `ro /usr`: a profile expressing
subtraction, which invariant 1 calls structurally impossible.
A raw NUL never got this far; go-toml refuses control characters in a basic
string. The \u0000 escape is accepted and produces the same byte, which anyone
re-testing needs.
checkEnvName has refused NUL in a NAME since the beginning, with reasoning that
applies word for word to the value — a rule written once and applied to one of
its two halves. checkEnvValue now refuses every C0 control and DEL in a value a
profile wrote, for set/merge/prepend; inherit and sanitise carry the host's
value and are deliberately out of scope (§2.3, and the environment execve hands
over cannot contain a NUL anyway).
nulJoin is the second guard, at the code that owns the separator and at the last
point the whole argv exists as Go values: it refuses any element containing a
NUL whatever wrote it, and names the index.
The other control characters are refused for a different reason and the two are
worth keeping apart: NUL authors a MOUNT, newline and ESC author a LIE in the
artifact a human reads to decide whether to trust the sandbox.
Found by an independent adversarial review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two unrelated repairs to the same file ride along, because they touch the same
tables and splitting them would mean splitting a file rather than a change.
forbiddenEnv gains GIT_PAGER, GIT_TEMPLATE_DIR, GIT_DIR, GIT_ALLOW_PROTOCOL and
GIT_PROTOCOL_FROM_USER, each measured on git 2.55 before being added. Three
different shapes, which is why the rule is "the value is code" and not "the
value is a command": GIT_PAGER is a command; GIT_TEMPLATE_DIR and GIT_DIR are
directories whose HOOKS are code, installed into every repo a later clone
creates or run on the next commit; the last two carry no code at all and merely
switch off git's refusal of the ext:: transport, which runs an arbitrary
command.
What that does NOT do is close the class, and the comment now says so. git falls
back GIT_EDITOR -> core.editor -> VISUAL -> EDITOR and GIT_PAGER -> core.pager
-> PAGER, and EDITOR/VISUAL/PAGER are legal by §3.2's decision — @claude
inherits all three. The GIT_* entries close the INVISIBLE half of the class.
Carried in TODO.md, pinned by a test so withdrawing those three is a deliberate
§3.2 decision rather than a table edit. §3.2's claim that they were "refused
inside @git-ro-style identity" was a phantom gate — nothing reads
Policy.Identity — and is deleted rather than implemented.
IsEnvList is exported for the renderer, which needs to know whether a space in a
value separates two elements or belongs to one.
…cannot know A profile could mount a tmpfs — or a rw bind — AT the staging directory and stage one file inside it. HasStagedBin then saw the staged file, snug put the now-writable directory FIRST on PATH in its own `(snug)` provenance, and the payload wrote `git` into it. Measured: WROTE-OK, and the shadowed git ran. The rw-bind spelling was worse, persisting the shadowed command to the host directory. The profile named PATH nowhere, so this is not the accepted-residual class either: no human read a declaration. It is the case CLAUDE.md's staging rule says cannot happen — "a profile cannot pick a writable directory by accident, because it does not pick one at all" — defeated one indirection below the layer the rule was written about. podmanstub.go predicted the exact failure and nothing checked it. StagedBinDir joins /proc and /dev in snugsOwn. It is keyed on the exact guest path, so @claude's `{home}/.local/bin/claude:/run/snug/bin/claude` and the Authored podman stub are untouched — staging one executable INSIDE the directory is what the directory exists for. Two sentences in describeCommands were false on the same screen: - the closing paragraph printed "NOT writable from inside" unconditionally, four lines above `PATH /run/snug/bin (snug) staged bin ← writable from inside`. It is now gated on IsShadowSlot and says the opposite when the predicate fires. Validate refuses that arrangement, so the branch should be unreachable — which is why it is worth keeping: a refusal that is later relaxed must not silently restore a false sentence. - the per-command line never read m.Access, so a rw staged bind was announced as "read-only" while the payload rewrote it and the rewrite reached the host file. Two assertions that named this property could not fail. Their fixtures staged nothing, so IsShadowSlot answered false through its "nothing is there" branch and grantMark returned "not granted" — both measuring ABSENT and reading as UNWRITABLE, and both would have kept passing under the defect above. Each now has a control that puts a tmpfs there and requires the predicate to say yes. Found by an independent adversarial review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It had exactly three call sites, all in describeEnvironment. Unguarded: the
bwrap argv block (formatArgs, which had no test at all), the FILESYSTEM loop,
and `snug profile show`. Each of the three was reached, measured:
(a) FILESYSTEM. filepath.Clean leaves a newline alone, so a guest path could
carry one and render as TWO correctly-columned grant rows — the second
spelled to look like `ro /etc/shadow @sys` for a mount that did not exist.
The sandbox really had one directory whose name contained a newline.
(b) The argv block, from a HOST value with no profile file involved: @claude
ships `inherit EDITOR`, and EDITOR=$'vim\n --ro-bind ~/.ssh ~/.ssh'
printed that as a --ro-bind line in the argv of a policy with no such
mount. The ENVIRONMENT block on the same screen escaped the same string
correctly. This is the identical source d2888b5 closed one commit ago, at
the block four lines below the one it fixed.
(c) `snug profile show`, in a real 110-column terminal: a value ending in
ESC[1A CR overwrote the row above it, and `rw /home/michal` — the whole of
$HOME, writable — was simply not on the screen. `cat -v` showed it there
all along. That screen is upstream of every --dry-run: it is what someone
reads to decide WHETHER to select a profile.
The renderers now escape at every sink, and Validate refuses a control character
in a GUEST path outright, next to the clean-path check it sat beside. Guest
only: a host path is not snug's to refuse, since a file on this machine may
legally be named with a newline, and the renderer covers that half.
The new test asserts the SET of sinks rather than a site — it drives the whole
dry-run and the whole `profile show` and asks whether anything anywhere emitted
a raw ESC or CR. The existing per-site test calls describeEnvironment directly
and structurally cannot observe another sink; its own comment warns that a fix
at one site would look identical, which is exactly what happened.
Found by an independent adversarial review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing itself
Both need an unusual path to fire, both are a tightening rather than an
escalation, and both were silent — which is what puts them in the same commit.
checkPrependAgreement keyed on `strings.Join(values, " ")`, directly under a doc
comment stating that equality is over the whole ordered sequence. A space-join is
injective only if no element contains a space, and an absolute path may:
-p ptools two elements, /opt/a and /opt/b
-p qtools ONE element, "/opt/a b"
-p ptools -p qtools keys equal -> "they agree" -> one entry deleted,
exit 0, nothing on the screen
A profile removing what another profile put on PATH is the property envresolve.go
opens by promising. seqKey renders with %q, which quotes each element separately,
and reads well enough to go straight into the refusal message — so the two
prepend refusals in the golden now show `["/opt/a" "/opt/b"]` instead of a run of
paths that could be read either way.
The screen had the same ambiguity from the other side: consecutive entries from
one verb and one profile are collapsed onto a line and joined with a space, so
`/opt/a /opt/b` could be two elements or one. elementValue quotes an element
containing a space — for LIST variables only, via the newly exported
policy.IsEnvList, because PS1 is mostly spaces and quoting a scalar would be
noise. No golden moved.
expandVars restarted its scan over the whole result after every substitution, so
substituted text was itself expanded — and the substituted text is a path the
HUMAN chose, not profile text. Measured:
mkdir -p '/tmp/x/{home}' /tmp/x/home/michal
snug '/tmp/x/{home}'
TARGET /tmp/x/{home} read-only, via @parent-ro
rw /tmp/x/home/michal @cwd-rw <- a different directory, writable
The payload wrote there and it persisted to the host, while the directory the
user named was read-only: "the target is the only writable thing that persists"
was false for that input. A directory named `{target}` was worse — expansion fed
its own output back in and spun at 224% CPU until killed, quadratic in TIME with
RSS flat at 18 MB, so it presents as a hang rather than an OOM. One pass, with
the literal text and the substituted value both committed to a builder and never
looked at again, kills both. The self-reference case is tested with a deadline
rather than inline, so a regression fails in five seconds instead of hanging the
package.
Found by an independent adversarial review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unit suite covers both defects at the layer they were fixed. These run them in a real sandbox, because at every layer above the kernel both were invisible: the first produced a real mount with no Mount in the policy, and the second a real writable directory that --dry-run described as unwritable. Only a running sandbox can say which was true. Each has a control that must still work — the same profile without the NUL, and one file staged INSIDE the directory — so "snug refused" cannot be satisfied by a snug that refuses everything. Both were proved to fail: with the fixes backed out, the first sandbox started and listed the host's ssh keys, and the second wrote an executable into the directory snug puts first on PATH. Docs, all of them corrections rather than additions: CLAUDE.md gains the NUL-in-a-value fact — the flag list is a NUL-separated string, so a profile's bytes could author a mount — and the general shape it belongs to: a rule written once and applied to one of its two halves (checkEnvName refused NUL in a NAME from the beginning). The staging bullet is corrected where it said a profile cannot pick a writable directory by accident; it could, one indirection down. sandbox-policy gains that correction as part of invariant 6, and a new invariant 7: text a profile wrote is not text snug wrote, at ANY sink, and the two questions to ask of each are different — can it author a MOUNT, or can it author a LIE. redteam gains both attacks as checklist items, including the instruction to read `profile show` in a real terminal as well as through `cat -v`, because the two disagree and the terminal is the one a human uses. VERIFY.md gains §6h and §6i, with the escape spelling anyone re-testing needs. TODO.md carries the residual this work did NOT close: forbiddenEnv does not close git's exec class, because EDITOR/VISUAL/PAGER are legal by §3.2 and git falls back to them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TEST.md was a local scratch list of things worth poking at while deciding
whether this branch was right. Everything in it that is a check now lives here,
re-run verbatim against HEAD rather than copied — the prepend refusal in
particular has changed since it was written, and now renders each element with
%q, which is the whole point of that check.
6d gains the run where all three sanitise drop reasons appear at once. Each
one alone reads as "an entry went away"; together they say which of "not
in the sandbox", "in it and empty and writable" and "in it and its magic
symlinks leave it" happened, and the three must never collapse.
6j all five verbs in one profile, then the payload picking the prepended
binary over the merged one of the same name. The screen agreeing with
itself proves nothing about what execve resolves.
6k a host variable set to empty arrives set. NO_COLOR is specified as "any
value, including empty", so dropping it silently re-enabled colour — and
both the bug and the fix are invisible on every other screen.
6l the parse-time refusals, one config dir each, because a file that fails to
load makes every later sandbox in the sequence refuse and mask the next
case. Carries the bound as well: this list does not close git's exec class,
since PAGER/EDITOR/VISUAL stay legal by §3.2 and git falls back to them.
8b a disagreement names every claimant, and the two -p orders must print the
same bytes. §8 already checked that for the argv; this is the same property
for the environment, where it is enforced by sorting rather than by the
model having no order.
8c a file that does not parse degrades diagnostics and refuses sandboxes.
Including the wording check: -p on a profile from that file must say the
file failed to parse, never "unknown profile", which would send someone to
fix their command line while the grant sat unloaded.
Two sentences were simply false and are corrected while nearby: §7 still said
"M0 is offline by design ... networking arrives in M2", and the closing section
listed seccomp, networking and containers as not covered, four sections after
covering all three. What is genuinely not covered is GUI/audio/D-Bus, and that
is by decision rather than by schedule.
The scratch file's last section was not checks but the places its author thought
the reasoning was thinnest. Three of those are still live and are kept as such,
named as review notes rather than steps. The fourth — profile show has no golden
— is no longer true.
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
1 participant
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.
Supersedes #8, which carried the design document alone. Every commit on that
branch is contained here, and the one defect it recorded as open is fixed below.
What this is
environreplaces the two ad-hoc keys (env,path) with five verbs under onesection, and gives the environment the same treatment the mount model already
has: one resolved policy, structural bands, conflicts that name every claimant,
and a
--dry-runblock that is the review artifact.The design is
.claude/design/ENVIRONMENT-VARIABLES.md; the twelve-step plan itwas built from is
.claude/design/ENV-IMPLEMENTATION-PLAN.md.Three bugs the design measured on
mainare closed here, each named in thedocument with the commit that closed it: a variable set to the empty string was
silently dropped (which inverts the meaning of every flag variable), one
unparseable file in
profiles.dtook down the entire registry including thebuiltins, and
PATHentries were not deduplicated.The security-relevant part
Three findings, all confirmed by execution, all fixed with permanent named tests.
Transport hooks passed where their own equivalents were refused.
GIT_SSHpassed while
GIT_SSH_COMMAND— the same capability, newer spelling — wasrefused two entries above it in the same table. The red team hijacked a real
git fetchin a sandbox whose ssh identity a different profile had pinned,from a profile granting no filesystem path at all. Nine names added; the rule is
not "the newest spelling", it is the value is code.
sanitisecarried a shadow slot onto thePATHsnug itself writes. A hostPATHelement under/tmpsurvived the filter, because the filter asked "isthis granted" and
/tmpis granted — as an empty writable tmpfs. The payloadcreates the directory and drops a file named
git, and the nextgita human oranother agent runs inside is that file. An element whose deepest covering mount
is a tmpfs is now dropped.
The wider fix — drop anything writable — was considered and rejected, and the
reasoning bounds what this claims: the payload can rewrite
PATHat will, so nofilter closes shadowing. What the filter owes is that the environment snug
itself hands over does not ship the shadow slot pre-installed. A writable
bind still survives, target included.
/proc's magic symlinks reopened it. Found after the above had shipped andclaimed the class was closed.
KindProcandKindDevwere kept as "kernel- andbwrap-populated, not empty" — true of the directory, false of what the magic
links resolve to. The walk is lexical and stops at
/proc; the kernel walks/proc/self/cwdto the target, where the shadow binary also persists to thehost. Both kinds now drop. The generalisable form is in
TODO.md: a lexicalpredicate answers about the path it was handed, the kernel answers about the path
it resolves, and wherever those differ is the attack.
A fourth, low: a newline in a host
PATHelement forged a row in the--dry-runENVIRONMENT block. Escapes nothing, but that screen is the trust artifact.
Reviewing this
internal/policy/testdata/sanitise.bwrap.txtis the new golden and it exists sothe security diff is one readable line — the
--setenv PATHoperand, with theshadow slots absent. No existing golden moved, which is itself a claim the tests
check rather than assume.
Every regression test here was verified to fail against the pre-fix code
before being accepted.
VERIFY.mdgains the by-hand/proc/self/cwdreproduction.
make gategreen.make integrationgreen withSNUG_REQUIRE_SANDBOX=1 SNUG_TEST_NET=1.Known gaps, recorded not carried
TODO.mdgains the residual shadow slots this deliberately does not close(
@claude's merged{home}/.local/bin,@tmp-sharedmaking/tmpa realbind, a profile's own
KindSymlink), the note thatsanitise's monotonicity nowrests on
rejectMasking, and the flatenviron = { set = { ... } }form thatparses with this go-toml and would not with a stricter one.
🤖 Generated with Claude Code