redteam: #582's own INDEX.md corrections were wrong, twice - #583
Merged
Merged
Conversation
… to #582) Round on #582, run before it merged and landing after it. The rename held — resolved policy bit-identical across it for every builtin and every combination tried, generated ~/.gitconfig byte-identical with `credential.helper = !/bin/evil` planted in the host config and correctly not carried, no name collision, and the retired-name lookup held against nine attack shapes including include chains and an unreadable profile file. What it broke was the prose #582 added, which is now on main. F1. INDEX §9.5 said ONE file is generated under ~/.config. Two are, and the omitted one is the one that matters: ~/.config/gh/hosts.yml, 0600, holding a GitHub OAuth token minted on the host by `gh auth token` (internal/cli/identity.go, `dir := pol.Home + "/.config/gh"`), under a (snug anchor) tmpfs at .config/gh. `authored()` in dryrun.go exists BECAUSE a --dry-run sentence about that exact path was already false for the same reason — its comment says "Both were true of ~/.config/gh until identity staged a generated hosts.yml there". #582 reintroduced the same false claim one file over. Second time at the same path, which is the argument for mechanising it. F2. INDEX said the things inside @home's tmpfs are "all KindData that snug authors, not binds a profile expresses", and attributed that to validate.go. validate.go says the opposite and names both halves: @claude expresses `ro` binds at ~/.claude/skills and ~/.claude/plugins inside that tmpfs. Worse than a typo — RULE 3 already exempts every Authored mount (`if m.Authored { continue }`), so the sentence licenses deleting RULE 2's KindTmpfs row, and deleting it makes rejectMasking refuse @claude's skills bind on the first invocation. F3, pre-existing, in a paragraph #582 edited: file.go's comma row and INDEX both claim `engine.New` comma-joins profile names into the container store key, "two consumers". engineKey is targetkey.Hash(pol.Target) and nothing else (internal/engine/paths.go); issue #276 took the selection out, and `grep -rn Profiles internal/engine/` over non-test files returns nothing. SNUG_PROFILES is the only comma join of names in the tree. The grammar rule stands on that alone; the false second consumer is gone. F4, mine: VERIFY's `grep -o '@git\b'` cannot fail. `\b` is a word boundary and `-` is a non-word character, so it matches inside @git-ro and prints @git whether or not the rename happened. Replaced with `grep -c '^ @git '`. Measured on this host: 0 and 1. The block also now says plainly that both greps run against the real XDG_CONFIG_HOME while the two blocks below it are hermetic. Two permanent regression tests, both negative-controlled, because prose that went wrong twice at one path is prose nothing was holding: TestTheGeneratedConfigInventoryIsExactlyTwoFiles (internal/cli) walks p.Mounts for every Guest under pol.Home + "/.config" and pins the set — {.config/gh/hosts.yml} from a stubbed tokenMinter, joined by .config/git/allowed_signers once identity.git.signing_key is set. Its failure message says INDEX §9.5 carries the inventory in prose and has to be edited in the same change. Teeth verified by repointing the hosts.yml Guest to /tmp/hosts.yml: fails with the expected message. TestClaudeSkillsIsABindNestedInsideHomesTmpfsNotAuthoredContent (internal/policy, no privileges) resolves @sys @home @target-rw + a @claude mirror and asserts ~/.claude/skills and ~/.claude/plugins are KindBind with Authored == false, after a positive control that /home/u really is KindTmpfs. Teeth verified by making checkNesting's `case KindTmpfs` return an error: fails with "masking refused under KindTmpfs at /home/u". NOT fixed here, needs a maintainer ruling and carries its reproduction (F5, pre-existing, reproduces byte-identically on 83f0da2): OpenSSH resolves the per-user config from getpwuid()->pw_dir, not $HOME. @sys binds the host's /etc/passwd, so where $HOME differs from the passwd home, ssh never reads the config snug generated — losing IdentitiesOnly yes, the pinned IdentityFile and UserKnownHostsFile, so StrictHostKeyChecking accept-new reverts to ask. Silent; --dry-run shows the row as though the pin were live, which is invariant 5. The key pin still holds through the agent proxy, so it is a loss of the config-level pin and not of the key. It also explains the one integration FAIL, TestSSHConfigDirectoryCannotBeRenamedAway, which asserts the pin by reading `ssh -G` with no -F and so depends on $HOME equalling pw_dir; identical failure on the origin/main control, so not a regression from #582. make gate green. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Red-team round on #582, commissioned before it merged, landing after. The rename held; the prose #582 added did not, and it is now on main.
What held
Resolved policy bit-identical across the rename — every builtin selection and every combination tried,
--dry-run --json, mount set, access bits, environ, full bwrap argv. Generated~/.gitconfigbyte-identical inside a real sandbox withcredential.helper = !/bin/evilplanted in the host config and correctly not carried.No name collision:
gitis not insubcommands; a user profile namedgitand the builtin@gitare distinct map keys (-p gitselects the user's,-p @gitthe builtin); a user profile named@gitis refused at parse bycheckName; no code matches on a profile-name literal other than@claude. The engine store key istargetkey.Hash(target), so the rename orphans no persistent host state.Retired-name lookup held against nine shapes:
-p,defaults =,includechains,profile show/tree/dot, case variants, forging runes, a user-definedgit-ro, and agit-roin a file with a TOML syntax error (refused earlier and louder, so the notice never gets a chance to misdirect).F1 — §9.5 said one generated file under
~/.config. Two, and the omitted one holds a token~/.config/gh/hosts.yml, 0600, holding a GitHub OAuth token minted on the host bygh auth token(internal/cli/identity.go,dir := pol.Home + "/.config/gh"), under a(snug anchor)tmpfs at.config/gh.authored()indryrun.goexists BECAUSE a--dry-runsentence about that exact path was already false for the same reason — its own comment: "Both were true of~/.config/ghuntil identity staged a generatedhosts.ymlthere". #582 reintroduced it one file over. Second time at one path.The sentence two lines above it in §9.5 names
~/.config/gh/hosts.ymlas the canonical example of where applications keep tokens.F2 — §"deepest mount" and RULE 2 asserted a mechanism the code contradicts
#582 wrote that everything inside
@home's tmpfs is "allKindDatathat snug authors, not binds a profile expresses", and attributed it tovalidate.go.validate.gosays the opposite and names both halves:Worse than a typo. RULE 3 exempts every
Authoredmount unconditionally (if m.Authored { continue }), so if the tmpfs cases really were all authored, RULE 2'sKindTmpfsrow is dead code — and a reader trusting the INDEX sentence has an argument for deleting it. Deleting it makesrejectMaskingrefuse@claude'sro ~/.claude/skillsinsidetmpfs ~/.claudeon the first invocation.F3 — pre-existing, in a paragraph #582 edited
internal/profile/file.go's comma row and INDEX both claimedengine.Newcomma-joins profile names into the container store key, "two consumers, and only one of them ever had a rule written for it". It does not.engineKeyistargetkey.Hash(pol.Target)(internal/engine/paths.go); issue #276 took the profile selection out;grep -rn "Profiles" internal/engine/over non-test files returns nothing.SNUG_PROFILESis the only comma join of names in the tree. Grammar rule stands on that alone.F4 — a VERIFY assertion that cannot fail
grep -o '@git\b'.\bis a word boundary,-is a non-word character, so it matches inside@git-ro:Prints
@gitwhether or not the rename happened. Replaced withgrep -c '^ @git '; measured 0 and 1 on this host. The block now also states that both greps run against the realXDG_CONFIG_HOMEwhile the two below it are hermetic.Two permanent regression tests, both negative-controlled
Prose that went wrong twice at one path is prose nothing was holding.
TestTheGeneratedConfigInventoryIsExactlyTwoFiles(internal/cli) walksp.Mountsfor everyGuestunderpol.Home + "/.config"and pins the set:{.config/gh/hosts.yml}from a stubbedtokenMinter, joined by.config/git/allowed_signersonceidentity.git.signing_keyis set. Failure message says INDEX §9.5 carries the inventory in prose and must be edited in the same change. Teeth verified by repointing thehosts.ymlGuestto/tmp/hosts.yml— fails with the expected message.TestClaudeSkillsIsABindNestedInsideHomesTmpfsNotAuthoredContent(internal/policy, no privileges) resolves@sys @home @target-rw+ a@claudemirror and asserts~/.claude/skillsand~/.claude/pluginsareKindBindwithAuthored == false, after a positive control that/home/ureally isKindTmpfs. Teeth verified by makingcheckNesting'scase KindTmpfsreturn an error — fails withmasking refused under KindTmpfs at /home/u.NOT fixed here — needs a ruling, carries its reproduction
F5, pre-existing, reproduces byte-identically on
83f0da2. OpenSSH resolves the per-user config fromgetpwuid()->pw_dir, not$HOME.@sysbinds the host's/etc/passwd, so inside the sandboxpw_diris the host's real home path. snug writes the generated config atpol.Home + "/.ssh/config". Where the two differ,sshreads a path that does not exist inside and silently falls back to compiled-in defaults.Lost:
IdentitiesOnly yes, the pinnedIdentityFile,UserKnownHostsFile(soknown_hostsis the nonexistent default andStrictHostKeyChecking accept-newreverts toask).--dry-runshowsdata ~/.ssh/config identity:<profile>and the SSH block as though the pin were live. Invariant 5. The key pin still holds through the agent proxy, so this is a loss of the config-level pin, not of the key pin.It also explains the one integration FAIL,
TestSSHConfigDirectoryCannotBeRenamedAway(test/integration/targetrepoint_test.go), which asserts the pin by readingssh -Gwith no-Fand so depends on$HOMEequallingpw_dir. Identical failure on the origin/main control — not a regression.Refuse on the mismatch, warn, or document? The test fixture wants
-F "$HOME/.ssh/config"or a skip either way.Not settled on this host, plainly
The
@nethost-loopback negative with-p @gitselected: timed out at 60s on the pasta path, killed by pid rather than widening the budget. Covered by the committed integration suite; the rename cannot plausibly reach it./etc/snug/profiles.das a second layer is absent on this host, so everyprofiles.dcase exercised only the XDG layer.make gategreen.🤖 Generated with Claude Code