identity: nest the block by tool, and derive the field set from the struct (closes #454) - #575
Merged
Merged
Conversation
…truct (closes #454) [identity] is now a container of per-tool blocks with no keys of its own: [profile.work.identity.ssh] host = "ghe.corp" key = "{home}/.ssh/work.pub" agent = "proxy" [profile.work.identity.git] name = "Your Name" email = "you@work.example" signing_key = "{home}/.ssh/sign.pub" [profile.work.identity.gh] host = "ghe.corp" user = "work-account" Scoped by SUBJECT, not by consumer: signing_key sits under git because git signs with it, even though the ssh proxy holds the pin. Each block is independently optional and nothing is inherited between them. THE FIELD SET IS DERIVED FROM THE TYPE, which is the part that makes nesting safe rather than merely tidier. identityFields walks Identity's `snug:"key,path"` tags at package init and drives both sinks — CheckText's forging-rune refusal and Resolve's expand-and-symlink-check loop — so a leaf cannot exist without passing through both. That is #549's stated acceptance criterion, met here. It is also what the nesting would otherwise have BROKEN. TestCheckTextCoversEveryIdentityField walked reflect.TypeOf(Identity{}) and skipped any field whose kind was not String; nesting makes all seven fields struct-kinded, so that sweep would have passed vacuously while CheckText stopped checking every field — a forging rune back into ~/.gitconfig, ~/.ssh/config and hosts.yml. The sweep is now TestCheckTextRefusesAForgingRuneAtEveryIdentityLeaf, driven off identityFields, asserting the enumeration is exactly eight keys. NESTED BY VALUE, AND THE == ASSERTION DOES NOT GUARD IT. A pointer is comparable, so `var _ = Identity{} == Identity{}` passes while three things break: the pin comparison `*p.Identity != id` degrades to pointer identity; `id := *prof.Identity` shares the inner struct with the LOADED profile, so the normalisation write-back mutates the registry and makes resolve([a,b]) differ from resolve([b,a]) in rendered output; and FieldByIndex panics on a nil pointer in the path. mustIdentityFields refuses any kind but Struct and String at init, and TestIdentityHasNoReferenceKindedField names all three. gh_host WAS ONE FIELD FOUR CONSUMERS READ, AND IS NOW TWO. identity.ssh.host feeds the generated ~/.ssh/config's Host line, the known_hosts filter and git's insteadOf rewrite — the host you push to. identity.gh.host feeds the token. The unexported host() is gone; SSHHost() and GhHost() replace it and collapse six copies of the "github.com" literal into policy.DefaultIdentityHost. identityHost is identitySSHHost and reads ssh.host. ssh_mode is identity.ssh.agent and the value changed with it: "agent-proxy" is now "proxy". ParseSSHMode grows no arm for the old spelling — its doc says why the accepted set must read as the whole set — so the retired value is refused by name in internal/profile instead. Five refusals, each naming the fix: - the flat keys, ALL present keys in one error with a pasteable nested block built from the author's own values (retiredAnonKey's shape: migrating seven keys must not take seven runs) - agent = "agent-proxy", naming "proxy" - an [identity] block that sets nothing. Not inert: a non-nil Identity sets IdentityOwner, which relabels the generated ~/.gitconfig's provenance from git:<name> to identity:<name>, so --dry-run and `snug profile show` claimed a pin that did not exist - a host named in one block while the other is active and names none, both arms. Without it, [identity.gh] host = "ghe.corp" alone leaves ~/.ssh/config, known_hosts and insteadOf all on github.com: every push fails `Permission denied (publickey)` against generated files that misdescribe the cause. Not the cross-field constraint #454 declined — the hosts may DIFFER; this refuses a profile that is INCOMPLETE, the shape of the two refusals already here (agent needs a key, signing_key needs an agent) - identity.gh.host with no identity.gh.user. That staged the token of whatever account the host's gh was logged in to, as user x-access-token, and showIdentity rendered nothing because its gh row keys on the user. The gate in stageGhConfig is now `id.Gh.User == ""` — fail-closed, so deleting the refusal reverts to "no token" rather than "unpinned token" — the x-access-token fallback is deleted, and test/guard keeps it out of non-test Go. The first three are in toIdentity: properties of the profile TEXT, so the verdict is the same on every host and a refused key cannot reach a generator. The last two are in Resolve, because that is the door every Identity passes through including one built in Go, which is what both identity goldens are. Measured: `snug profile show` does NOT run Resolve (showCapabilities renders a Profile pre-resolution, which is why its golden shows unexpanded {home}/… paths), so those two show on --dry-run and on a real run but not on that screen — the same reach ParseSSHMode's unknown-value refusal has always had. Goldens. internal/cli/testdata/identity.bwrap.txt does NOT move: zero-line diff, and the test now says a diff there means staging changed. show-identity.capabilities.txt moves twice — (agent-proxy) to (proxy), and the gh row becomes a capRows block naming the token's blast radius, because admin:public_key lets anything inside add an SSH key to the account and that OUTLIVES the sandbox. refusals.txt moves: the remediation block is the nested spelling. A CONTENT GOLDEN IS NEW, and it is the only thing that can ever see the host split. --ro-bind-data carries generated bytes over an fd, so no argv golden contains them — measured: a --dry-run of a profile with ssh.host = ssh.example and gh.host = gh.example prints neither string anywhere. testdata/identity-generated.txt pins ~/.gitconfig, ~/.ssh/config and known_hosts, all three naming ssh.example. One commit because it cannot be split without a broken build at each step: the struct, the decode and the two derived sinks are one edit, and the retired-key refusals must exist before every fixture is re-spelled or the fixtures fail for the wrong reason and the refusals go untested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TestDoctorRunsCleanOnAHostThatCanRunSnug ran `snug doctor` with no environment isolation, so it read the real ~/.config/snug/profiles.d. doctor reports `❌ 1 profile file(s) did not load` for anything there that snug refuses — which is doctor working — and then this test failed for it. The verdict of a repository test therefore depended on state no reader of the repository can see, and nothing in the tree could fix it. Measured on the #454 rename: a personal accounts.toml still carrying the flat identity keys turned this green tick into --- FAIL: TestDoctorRunsCleanOnAHostThatCanRunSnug (0.13s) ❌ 1 profile file(s) did not load; snug will refuse to start a sandbox while `make gate` stayed clean. It was the ONLY failure in the 302s integration run. Any future schema change reproduces it for every developer until each one migrates their own file. XDG_CONFIG_HOME is now an empty t.TempDir(). Nothing is lost: this test exists for the stage probe and for host capability, and whether a profile file parses is covered by every test that WRITES one. TestDoctorReportsAProfileFileThatDidNotLoad is the counterpart, and it is the reason the isolation is safe rather than a coverage hole. Before it, doctor's profile-set row was graded by accident — green on a clean store, red on a stale one, asserted in neither case — so deleting the check outright would have made the suite greener rather than red. It asserts the exit code first, because a doctor that printed ❌ and exited 0 would pass every string check. Its fixture is a profile that parses as TOML and is refused by snug's own rules (an [identity] block setting nothing), not malformed TOML: both fire the row, only one shows it survives a refusal that comes from snug rather than from go-toml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on (redteam F1)
Resolve-time symlink refusals rendered an attacker-controlled path with %s. The
destination of a symlink planted inside the WRITABLE TARGET — which is the exact
scenario underTargetIsLiteral's own comment says it defends, "a previous run could
have planted the link" — therefore reached the terminal as control bytes.
Measured before the fix:
$ ln -s "$D/real$(printf '\033')[2K$(printf '\033')[1Asnug: policy verified, sandbox is safe" $TGT/vendor
$ snug --dry-run --no-defaults -p rt $TGT # ro = ["{target}/vendor"]
exit=77
snug: profile "rt": grant .../tgt/vendor resolves to .../real^[[2K^[[1Asnug: policy
verified, sandbox is safe: a symlink inside the sandbox's own writable area redirects it
^[[2K^[[1A is erase-line plus cursor-up: on a vt100 snug's warning is erased and
that sentence is printed over it. The word "safe", in snug's voice, is the
attacker's. forging.go's VisibleText doc already names this sink — "INCLUDING IN A
REFUSAL… A refusal is the sink a human reads most carefully" — and these three
sites were missed.
TWO ARMS, AND THE SECOND IS WHY ESCAPING THE OBVIOUS HALF WOULD NOT HAVE CLOSED IT.
underTargetIsLiteral prints the destination itself, so VisibleText on `requested`
and `real` covers that one. The other two sites wrapped an fs.PathError with %w,
and for a DANGLING symlink that error's Path is the destination — so the bytes
arrived through the error text while the interpolated path looked handled. visibleErr
renders err.Error() through VisibleText and drops %w deliberately: no caller needs
the chain (the ErrNotExist branch is taken above on the same error, and
ErrTargetUnusable is wrapped elsewhere), and a refusal is a screen first.
A/B on the identical payload, dangling destination whose first missing component
carries it: raw ESC present on the parent commit, \x1b with the fix.
Four golden rows in testdata/refusals.txt pin the escaped form, and
`grep -cP '\x1b'` over that file returns 0. Each has a paired unit test driven
through the injected Environ — fakeEnv gains symlinkErrs so a fixture can hand
Resolve an EvalSymlinks error carrying chosen bytes with no real filesystem — and
the two symlink arms have positive controls, because a function that escaped
everything into noise would pass the negatives.
WHY IT SHIPPED, stated where someone will look: TestNoSnugScreenEmitsARawControlCharacter
drives dryRun over a SUCCESSFULLY RESOLVED policy, so it structurally cannot reach a
Resolve-time refusal. visible_test.go now says so and points at these goldens rather
than implying a coverage it does not have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
snug is pre-alpha and the profile schema is not stable, so a developer whose ~/.config/snug/profiles.d predates a schema change is expected to rewrite their own file rather than have the suite work around it. That makes the isolation this commit added a workaround for a problem the project does not have: the maintainer's accounts.toml has been rewritten to the nested identity schema (#454), and TestDoctorRunsCleanOnAHostThatCanRunSnug passes against the real store again. Reverting restores doctor's profile-set row to the coverage it had before, and takes TestDoctorReportsAProfileFileThatDidNotLoad with it: that test existed only to fill the hole the isolation opened. This reverts commit bb0eee6. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nown key
snug is pre-alpha and commits to no configuration compatibility, so a key it
no longer has needs no migration arm. Deleted, with the struct fields that
existed only to keep them decoding:
retiredEnvKey env = [...] -> environ.inherit
retiredPathKey path = [...] -> environ.merge on PATH
retiredAnonKey address/gateway/address6/gateway6 (feature gone)
retiredFlatIdentity ssh_key ssh_mode signing_key git_name git_email
gh_user gh_host -> the per-tool identity blocks
retiredAgentProxyValue agent = "agent-proxy" -> agent = "proxy"
The fields are gone from rawProfile and rawIdentity, so DisallowUnknownFields
answers instead, and its answer is not the bare "unknown key" the essay above
these functions argued against: go-toml quotes the file, the line and underlines
the field. Measured, exit=77 either way:
.../p.toml: unknown key (snug decodes profiles strictly, so a key it does not
understand is an error rather than a silently ignored grant):
4| address = "10.13.13.2/24"
| ~~~~~~~ unknown field
A retired VALUE is the one that does not go through the decoder, and it needed
no arm to begin with. ParseSSHMode already refuses by naming the accepted set:
snug: profile "x": unknown identity.ssh.agent "agent-proxy" (want proxy or none)
That is invariant 5 — refuse, name what to write — and it keeps the accepted set
readable as the whole set, which is what an arm per dropped spelling costs.
emptyIdentityBlock stays: an empty [identity] PARSES, sets IdentityOwner and
relabels the generated ~/.gitconfig's provenance, so it is a shape that is
silently wrong rather than a spelling that is gone.
Prose that claimed the named errors exist, corrected where it lives:
ENVIRONMENT-VARIABLES.md's §6 header and its two open items, policy/profile.go's
Environ comment, VERIFY.md §4b-ter (rewritten around the decoder's message plus
the value case). VERIFY.md and INDEX.md also still spelled the ssh agent mode
"agent-proxy" in four places the #454 rename missed, including INDEX's copy of
the const block.
make gate green. SNUG_REQUIRE_SANDBOX=1 make integration green, 0 failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d in writing
Maintainer decision. The alternative was to drop refuseHalfNamedHost and render
host provenance on the capability screen instead — `host github.com — DEFAULTED,
not named by this profile` — letting the reader spot the mismatch. It loses on
two counts, and both are now in the refusal's doc comment where the decision is
enforced rather than one indirection away:
- a run without --dry-run prints no capability screen, so a screen protects
only somebody who looks. Measured: the refusal exits 77 on --dry-run and on
a real run, while `snug profile show` exits 0 either way because it renders
pre-resolution.
- a marker on every host row is noise on the one screen a human reads to
decide whether to trust the sandbox.
No behaviour change; comment only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t is testable The plan named this and it was left unsaid. #454 split one gh_host field into identity.ssh.host and identity.gh.host, and only the ssh half got a test that can see it: testdata/identity-generated.txt pins the generated ~/.ssh/config, known_hosts and git's insteadOf byte for byte. The gh half is not a file Resolve generates — it is `gh auth token --hostname`, the hosts.yml top key and GH_HOST, produced in internal/cli after resolution and behind an exec. So the half that MOVES A CREDENTIAL was assertable by nothing that runs without a forge account, and swapping the two accessors in stageGhConfig produced no golden diff anywhere. stageGhConfig takes a `mint tokenMinter` parameter; the one non-test caller passes ghToken. A parameter and not a package-level var a test swaps: a mutable global here is reachable from every other test in the package, and the value it carries decides which account's token is staged. internal/cli/ghhostsplit_test.go, four tests: - ssh.host and gh.host set to DIFFERENT values, pinning all four places the gh host reaches (the --hostname argument, the hosts.yml top key, GH_HOST, GH_CONFIG_DIR) plus the negative, that ssh.example appears in none. - neither host named: the gh side reaches DefaultIdentityHost through its own accessor. "Both say github.com" has to be two independent defaults, because no fallback runs between the blocks. - the gate fires BEFORE the host touch — the fake minter is never called with no identity.gh.user. Asserting only "no file was staged" would pass on a gate placed after the call, which has already asked for the token of whatever account the host is logged in to. - invariant 5 at this seam: a real run with no token refuses naming the user, the host and `gh auth login`; the dry run continues and stages no hosts.yml. A/B'd against the unfixed tree: `host := id.GhHost()` swapped to id.SSHHost() fails three assertions in the first test; restoring it passes. No golden moves either way, which is the gap. identitygeneratedgolden_test.go now states what it cannot see rather than leaving it to be discovered, and names where the gh half is covered. make gate green. SNUG_REQUIRE_SANDBOX=1 make integration green, both suites, 0 failures (324.905s and 67.301s). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aid otherwise Two redteam findings on this branch, both on the honesty of what the screens claim versus what a run does. Both fixed here, both with a named regression test A/B'd against the unfixed tree. F1, invariant 5. startIdentity opened with one early return on `id.SSH.Agent == SSHNone`, and stageGhConfig sat at the BOTTOM of the function that return guarded. So an [identity] naming only a gh account got no token, no GH_CONFIG_DIR, no GH_HOST and no line anywhere saying so, and exited 0 — the exact sentence stageGhConfig's own refusal exists to prevent, reached by the gate ABOVE it rather than by the gate it guards. `snug profile show` meanwhile printed THE SANDBOX HOLDS A FORGE TOKEN FOR THIS ACCOUNT in capitals. Measured before, gh a recording stub on PATH: snug -p y $PROJ -- sh -c 'env | grep -c "^GH_"' -> 0, rc=0, stub log empty #454 is what made the shape easy to write: once [identity] is a container of per-tool blocks, `[identity.gh] user = "you"` alone is the natural spelling for "I want gh in the sandbox, not ssh", and the maintainer decision on this ticket keeps a one-tool identity legal. Legal has to mean it WORKS. The ssh half moves to startSSHIdentity verbatim; startIdentity gates that on the agent mode and calls stageGhConfig unconditionally. Nothing else belongs behind the agent gate: the staged public key exists so the generated ~/.ssh/config's IdentityFile resolves and resolve.go generates that file for every mode except none, and identity.git.signing_key is refused in Resolve without an agent. After, same profile and stub: GH_CONFIG_DIR=/.../.config/gh GH_HOST=github.com github.com: oauth_token: MARKERTOKEN_gho_AAAA user: victim-a stub log: ARGV: auth token --hostname github.com --user victim-a No builtin defines a gh identity (base.toml's are comments and the template), so nothing previously inert starts staging a credential. F2. The ACCEPTED SET of agent modes was checked only in policy.Resolve, and `snug profile show` does not resolve — it renders a *policy.Profile straight from the registry. So a profile with `agent = "agent-proxy"` rendered its ssh key row, the mode verbatim as `(agent-proxy)` and the blast-radius paragraph, and exited 0 on the screen a human reads to decide whether to select it, while every run of it exited 77. toIdentity now calls policy.ParseSSHMode too. Not a second LIST of accepted spellings — a second CALL to the one function that owns the list, so the two doors cannot drift. profile show and --dry-run now both exit 77 naming the file, the profile and the accepted set. Regression tests: test/integration/identity_test.go TestAGhOnlyIdentityIsNotSilentlyInert internal/profile/identitynested_test.go TestUnknownAgentModeIsRefusedAtParseTime A/B: both FAIL against the unfixed code — the first on "--dry-run said nothing about the pinned gh account", the second on all four rejected spellings parsing. VERIFY.md §4b-ter's value case rewritten: the refusal now arrives as a bad-file report at parse time, and profile show is part of the check. What redteam could NOT break, recorded so the round is not re-spent: every deleted key refused at every nesting and spelling (root, inline table, dotted key, inline sub-table), no case-folding collision onto a surviving tag, no downgrade through the BadFile path (run/dry-run/json/list/show/tree/config all 77, doctor 69), the staged token absent from --dry-run, -v, --json, --explain, /run/user, $HOME and /tmp, and every forging class refused at identity.gh.* by CheckText including U+2028/U+2029/U+0085, which would otherwise have written a second host entry into hosts.yml. make gate green. SNUG_REQUIRE_SANDBOX=1 make integration green, both suites, 0 failures (305.728s and 67.439s). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #454.
[identity]becomes a container of per-tool blocks with no keys of its own:Scoped by SUBJECT, not consumer:
signing_keyundergitbecause git signs with it, even though the ssh proxy holds the pin. Each block independently optional, nothing inherited between them.The field set is derived from the type
identityFieldswalksIdentity'ssnug:"key,path"tags at package init and drives BOTH sinks —CheckText's forging-rune refusal andResolve's expand-and-symlink-check loop. The type is the table, so a leaf cannot exist without passing through both. That is #549's stated acceptance criterion, met here; #549 is re-scoped accordingly.It is also what nesting would otherwise have BROKEN.
TestCheckTextCoversEveryIdentityFieldwalkedreflect.TypeOf(Identity{})and skipped non-Stringkinds; nesting makes all seven fields struct-kinded, so that sweep would have passed vacuously whileCheckTextsilently stopped checking every field — a forging rune back into~/.gitconfig,~/.ssh/configandhosts.yml. NowTestCheckTextRefusesAForgingRuneAtEveryIdentityLeaf, driven offidentityFields, asserting the enumeration is exactly eight keys.Nested by value, and
==does not guard itA pointer is comparable, so
var _ = Identity{} == Identity{}passes while three things break:*p.Identity != iddegrades to pointer identity — byte-identical blocks refuse each other.id := *prof.Identityshares the inner struct with the LOADED profile, so the normalisation write-back mutates the registry:resolve([a,b])differs fromresolve([b,a])in rendered output. The decisive one — an invariant, not a message.FieldByIndexpanics on a nil pointer in the path.mustIdentityFieldsrefuses any kind but struct/string at init;TestIdentityHasNoReferenceKindedFieldnames all three.gh_host was one field four consumers read, now two
identity.ssh.hostfeeds the generated~/.ssh/configHostline, theknown_hostsfilter and git'sinsteadOf— the host you push to.identity.gh.hostfeeds the token. Unexportedhost()gone;SSHHost()/GhHost()replace it and collapse six copies of the"github.com"literal intopolicy.DefaultIdentityHost.identityHost→identitySSHHost, readingssh.host.ssh_mode→identity.ssh.agent, and the VALUE changed:"agent-proxy"→"proxy".ParseSSHModegrows no arm for the old spelling — its doc says why the accepted set must read as the whole set — soagent = "agent-proxy"is refused by naming the accepted set:unknown identity.ssh.agent "agent-proxy" (want proxy or none), exit 77.Three refusals, each naming the fix
[identity]block setting nothing. Not inert: a non-nilIdentitysetsIdentityOwner, relabelling the generated~/.gitconfig's provenancegit:<name>→identity:<name>, so--dry-runandsnug profile showclaimed a pin that did not exist[identity.gh] host = "ghe.corp"alone leaves ssh ongithub.com: every push failsPermission denied (publickey)against generated files that misdescribe the cause. Not the cross-field constraint [identity] Nest the identity block by tool: identity.ssh.key, identity.git.signing_key, identity.gh.user #454 declined — the hosts may DIFFER; this refuses a profile that is INCOMPLETE, the shape of the two refusals already thereidentity.gh.hostwith noidentity.gh.user. That staged the token of whatever account the host'sghwas logged into, asuser: x-access-token, andshowIdentityrendered nothing because its gh row keys on the user.stageGhConfig's gate is nowid.Gh.User == ""— fail-closed, so deleting the refusal reverts to "no token" not "unpinned token" — thex-access-tokenfallback is deleted, andtest/guardkeeps the literal out of non-test GoFirst three in
toIdentity(properties of the profile TEXT, so the verdict is host-independent and a refused key cannot reach a generator). Last two inResolve, the door everyIdentitypasses including one built in Go — which is what both identity goldens are.Measured:
snug profile showdoes NOT runResolve(showCapabilitiesrenders aProfilepre-resolution, which is why its golden shows unexpanded{home}/…). So those two show on--dry-runand on a real run, not on that screen — the same reachParseSSHMode's unknown-value refusal has always had. No second copy added.redteam: one confirmed finding, fixed here
A refusal forged snug's own voice. A symlink planted in the writable target by a previous run —
underTargetIsLiteral's own stated threat — with control bytes in its destination:^[[2K^[[1Aerases the warning and prints that over it. The word "safe", in snug's voice, is the attacker's.Two arms, and escaping the obvious half would not have closed it:
underTargetIsLiteralprints the destination itself, but two sites wrapped anfs.PathErrorwith%w, and for a dangling symlink that error'sPathIS the destination — the bytes arrived through the error text while the interpolated path looked handled.visibleErrrenderserr.Error()throughVisibleTextand drops%wdeliberately (no caller needs the chain; theErrNotExistbranch is taken above on the same error).A/B on the identical payload: raw ESC on the parent commit,
\x1bwith the fix. Four golden rows intestdata/refusals.txtpin the escaped form;grep -cP '\x1b'over that file returns 0. Each has a paired unit test through the injectedEnviron(fakeEnvgainssymlinkErrs), and the two symlink arms have positive controls, because a function escaping everything into noise would pass the negatives.Why it shipped, stated where someone will look:
TestNoSnugScreenEmitsARawControlCharacterdrivesdryRunover a SUCCESSFULLY RESOLVED policy, so it structurally cannot reach a Resolve-time refusal.visible_test.gonow says that rather than implying coverage it lacks.Held under attack:
CheckTextover all eight leaves against seven rune classes;toIdentityas the only constructor;showIdentity's rows (theshow()closure re-escapes the assembled line); the pin in both orders, throughinclude, with byte-identical blocks resolving together;stageGhConfig's gate.Second round, after the refusal deletions and the token seam. Two findings, both fixed here with named regression tests A/B'd against the unfixed tree.
F1,
sev:medium, invariant 5. A gh-only[identity]was silently inert.startIdentityopened with one early return onid.SSH.Agent == SSHNoneandstageGhConfigsat at the BOTTOM of the function that return guarded. So[identity.gh] user = "you"alone — the natural spelling once the block is nested by tool, and a shape the maintainer decision on this ticket keeps legal — got no token, noGH_CONFIG_DIR, noGH_HOST, no line saying so,rc=0. Measured with a recordingghstub:env | grep -c '^GH_'→0, stub log empty, so not even the host touch happened.snug profile showmeanwhile printed THE SANDBOX HOLDS A FORGE TOKEN FOR THIS ACCOUNT in capitals. The exact sentencestageGhConfig's own refusal exists to prevent, reached by the gate ABOVE it.The ssh half moves to
startSSHIdentityverbatim;startIdentitygates that on the agent mode and callsstageGhConfigunconditionally. Nothing else belongs behind the agent gate — the staged public key exists so the generated~/.ssh/config's IdentityFile resolves andresolve.gogenerates that file for every mode exceptnone;identity.git.signing_keyis refused inResolvewithout an agent. No builtin defines a gh identity, so nothing previously inert starts staging a credential.F2,
sev:low.profile showrendered a capability row for a profile no run can use. The accepted set of agent modes was checked only inResolve, andsnug profile showdoes not resolve — it renders a*policy.Profilestraight from the registry.agent = "agent-proxy"therefore rendered its ssh key row, the mode verbatim as(agent-proxy)and the blast-radius paragraph,rc=0, while every run exited 77.toIdentitynow callspolicy.ParseSSHModeas well: not a second LIST of accepted spellings, a second CALL to the one function that owns the list. Both screens now exit 77 naming the file, the profile and the accepted set.Regression tests:
TestAGhOnlyIdentityIsNotSilentlyInert(integration),TestUnknownAgentModeIsRefusedAtParseTime(internal/profile). Both fail against the unfixed code.What the round could NOT break, recorded so it is not re-spent: every deleted key refused at every nesting and spelling (root, inline table, dotted key, inline sub-table); no case-folding collision onto a surviving tag (go-toml matches
toml:tags case-insensitively but not Go field names —SigningKeyis an unknown field); no downgrade through theBadFilepath (run,--dry-run,--json,profile list/show/tree,configall 77,doctor69); the staged token absent from--dry-run,-v,--json,--explain,/run/user,$HOMEand/tmp— the argv carries--file 19, never the bytes; the half-named-host refusal fires BEFORE theghexec (stub log empty); and every forging class refused atidentity.gh.*byCheckText, including U+2028/U+2029/U+0085, which YAML treats as line breaks and which would otherwise have written a second host entry intohosts.yml.Every retired-key refusal is gone, including the ones that predate this change
snug commits to no configuration compatibility, so a key it no longer has gets no migration arm. Deleted with the struct fields that existed only to keep them decoding:
retiredEnvKey(env = [...]),retiredPathKey(path = [...]),retiredAnonKey(address/gateway/address6/gateway6),retiredFlatIdentity(the seven flat identity keys) andretiredAgentProxyValue.DisallowUnknownFieldsanswers instead, and its answer is not the bare "unknown key" the deleted essay argued against — go-toml quotes the file, the line and underlines the field:Exit 77 either way. Invariant 5 asks a refusal to say what to write; it never asked it to name the previous spelling.
emptyIdentityBlockstays, and the line is why: an empty[identity]PARSES, so it is a shape that is silently wrong rather than a spelling that is gone.Prose corrected where it lives:
ENVIRONMENT-VARIABLES.md§6 and its header,policy/profile.go'sEnvironcomment,VERIFY.md§4b-ter. Four places still spelling the agent modeagent-proxyafter the rename are fixed too, includingINDEX.md's copy of the const block.Goldens
internal/cli/testdata/identity.bwrap.txt— zero-line diff, and the test now says a diff there means staging changed.show-identity.capabilities.txt—(agent-proxy)→(proxy), and the gh row becomes acapRowsblock naming the token's blast radius:admin:public_keylets anything inside add an SSH key to the account, an effect that OUTLIVES the sandbox.refusals.txt— the remediation block in the nested spelling, plus the four forging rows.testdata/identity-generated.txtis new, and it is the only thing that can ever see the host split.--ro-bind-datacarries generated bytes over an fd, so no argv golden contains them — measured: a--dry-runwithssh.host = ssh.exampleandgh.host = gh.exampleprints neither string anywhere. It pins~/.gitconfig,~/.ssh/configandknown_hosts, all three namingssh.example.Verification
make gategreen.SNUG_REQUIRE_SANDBOX=1 make integrationexit 0, 0 failures, engine floor 46/46, 397 passes.stageGhConfiggained atokenMinterparameter so the gh half of the host split —gh auth token --hostname, thehosts.ymltop key,GH_HOST— is assertable without a forge account. It was covered by nothing pure before, and swapping the two accessors produced no golden diff anywhere. Four tests ininternal/cli/ghhostsplit_test.go;identitygeneratedgolden_test.gonow states what it cannot see.resolve.gochange.NOT verified:
VERIFY.md§13 is rewritten but not walked by hand — it needs two real accounts with two agent-held keys. Its TOML blocks are parsed bydocexamples_test.go(green), so the fixtures load; the procedure itself is unexecuted.Migration is a hard break by design. A profile with the flat keys is refused by the strict decoder, with the offending line quoted and underlined — never silently accepted.
🤖 Generated with Claude Code