security: make the agentID trust model real (identity takeover, scope escape, revocation bypass, token leak)#12
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for _, e := range ring.EntriesSince(0) { | ||
| fmt.Fprintf(w, "%s %-5s %s %s\n", time.UnixMilli(e.TimeMs).Format(time.RFC3339), e.Level, e.Msg, e.Attrs) | ||
| line := fmt.Sprintf("%s %-5s %s %s\n", time.UnixMilli(e.TimeMs).Format(time.RFC3339), e.Level, e.Msg, e.Attrs) | ||
| io.WriteString(w, scrub.clean(line)) |
| // Scanner's 64 KiB token cap and silently truncate the rest of the file. | ||
| line, err := br.ReadString('\n') | ||
| if line != "" { | ||
| io.WriteString(w, s.clean(line)) |
agentID was sha256(pubkey) truncated to 40 bits. The gateway keys supersede, revocation, scope, and gateway-authoritative config on that label, so its width is the cost of forging a second key that answers to a victim's name — and 2^40 keygens is hours on one rented machine, roughly $50 of cloud CPU. "Unforgeable" was true of the public key, not of a 40-bit truncation of it. Take 10 bytes (80 bits) for AgentID, which is not purchasable at any price, and still renders as a 16-char tag. fingerprint() keeps its 40-bit display budget and its now-sole caller GatewayID, where the label carries no authority (the gateway is trusted via the full sha256 pin in the pairing code). Existing agt_ ids change, so enrolled agents must re-pair. That is deliberate: the old ids are the vulnerable artifact. TestAgentIDIsWideEnoughToBeUnforgeable asserts the 80-bit floor.
…identity
The legacy shared-token path authenticates a gateway-wide bearer token and then
believes whatever agentID the peer names. The gateway keys supersede, per-conn
delivery, scope, and gateway-authoritative config on that label, so a token
holder naming itself agt_<victim> would:
- evict the enrolled agent (actor.go admit supersedes on agentID equality),
- inherit its public ports and its tunnels, and
- escape its scope entirely — a shared-token identity carries no Scope, and an
empty Scope means unrestricted (agentstore.go).
Revoking the victim did not help: the impersonator never presents the revoked
key, so identityValidator's Revoked check is never reached. The shared token is
gateway-wide, so every legacy agent could do this to every enrolled one.
Only AgentID mints agt_ ids, and config.NewID gives legacy agents bare 32-hex, so
the prefix is a namespace no honest shared-token agent needs. Reject it there.
This is what makes CLAUDE.md's promise — enrolled agents are protected by their
key and by revocation — actually true; the residual risk of the legacy path
narrows to peers that are legacy on both sides.
TestSharedTokenCannotClaimDerivedAgentID covers takeover, scope escape, and the
revocation bypass, and asserts legacy agents still pair.
…d agent The store is keyed by pubkey, but every management op above it — Revoke, SetScope, Rename, DesiredConfig, AdoptConfig — resolves an agent by scanning for the agentID label and taking the first match, and the actor's live-session map is keyed by it. Two records answering to one name therefore make revocation and scope land on whichever record map iteration yields: an operator revokes an agent and the wrong one dies, nondeterministically. Enroll accepted that silently. Refuse the join instead, so the label stays a true primary key and a found collision fails closed rather than sharing an identity. Belt-and-braces behind the 80-bit widening: that makes the search infeasible, this makes winning it useless. Re-enrolling the same key stays a reconnect. TestEnrollRejectsAgentIDCollision.
RunStarted logged the pxf:// pairing code at Info. The code embeds the shared
gateway token verbatim (pxf://host:port/v1/pair/<token>#sha256:<fp>), and slog
fans out to three places that outlive the moment the code is useful:
- the rotating log file on disk (and its .1/.2/.3 rotations),
- the GUI ring, and
- every diagnostics bundle — app/tools.go ships the ring as logs-recent.txt
and copies each on-disk log file in verbatim.
Bundles are built to be handed to someone else. redactConfig masking
Gateway.Token in config.redacted.toml bought nothing while the same token rode
along in cleartext inside a logged pairing code two files over. Any bundle ever
shared handed over agent-level access to the gateway, and this fires in GUI mode
too (engine.go RunStarted), not just the CLI.
Print it to the console instead: transient, unshipped, and already where the
headless operator is looking. The GUI never needed the line — it mints codes over
IPC (app.go PairingCode). fmt.Println is the house convention for operator output
(main.go) and the role commands attach a console via wincon.AttachParent.
TestRunStartedKeepsThePairingCodeOutOfLogs.
The bundle redacted config.redacted.toml and stats.redacted.json, then shipped the GUI ring and every on-disk log file verbatim beside them. Masking Gateway.Token in one file while the same token sits in cleartext in a log line one file over is not redaction, and CLAUDE.md's "bundles redact every secret, host, IP, and identity" was not structurally true. TestWriteDiagnosticsNoLeaks did not catch it because it passed ring=nil into a dir with no log files: the two unredacted channels were never populated, so its secret sweep ran over an empty set and passed vacuously. That blind spot is the root — the logged pairing code (previous commit) was one symptom of it, and the next careless log call would have been the next. logScrubber replaces the *exact* values redactConfig masks, read out of the live config — never guessed patterns, so it can't pass off a guess as a guarantee. It is a net, not a licence: secrets still must not be logged, since this only knows what config holds. Line-oriented, so a rotated log stays bounded in memory and a secret can't slip through a chunk boundary. copyIntoZip goes: it had no callers left, and an unscrubbed path into the bundle is exactly what shouldn't exist. The test now fills the ring and the rotating file through the real logging fan-out, asserts the seeded log actually holds the token (no vacuous pass), and checks the logs survive scrubbing readable — masking by shipping nothing would pass the sweep and defeat the bundle.
SetAgentScope wrote the store and stopped there. A session captures identity.Scope at admission (buildAndAdmit) and validateSpec reads that copy for the session's whole life, so an agent the operator had just narrowed kept serving its old grant — holding the very ports being taken away — until it happened to reconnect. A misbehaving agent has no reason to reconnect, and the GUI showed the new scope the whole time, so the operator believed a containment that had not happened. Narrowing is the urgent case: it is how you contain an agent. Evict, exactly as RevokeAgent already does to make its own guarantee land: the agent reconnects, re-reads its record, and every bind is re-checked against the narrowed scope. Doing it on the actor keeps lifecycle actor-owned and sidesteps mutating sess.scope under readers on other goroutines. RenameAgent deliberately does not evict — a nickname changes nothing about what an agent may bind. e2e TestSetAgentScopeEvictsLiveSession; with the eviction removed it holds the port for the full 20s window and fails, which is the bug it pins.
The security bullets described a shared-token world and promised a bundle redaction that log shipping quietly undid. Bring them level with the code: - the residual risk of the legacy path is now confined to other shared-token agents, because the agt_ namespace is reserved; - agentID's width is a security parameter, and a scope change evicts; - bundles ship logs, so logs are scrubbed — and a leak test must seed every channel it ships or it passes vacuously. Note for whoever merges feat/pillar8-agent-mgmt: its CLAUDE.md (a745be5) already claims "enrolled agents are protected by their key and by revocation" and that agentID is "unforgeable". Both were aspiration when written — a shared-token peer could name itself agt_<victim>, and 40 bits was searchable. This branch is what makes them true, so it should land first.
xeri
force-pushed
the
security/agent-identity-hardening
branch
from
July 16, 2026 12:18
f9f81d7 to
4c95119
Compare
xeri
added a commit
that referenced
this pull request
Jul 16, 2026
… escape, revocation bypass, token leak) (#12)
xeri
added a commit
that referenced
this pull request
Jul 16, 2026
… escape, revocation bypass, token leak) (#12)
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
2 participants
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.
Five issues, each fixed at the root and each with a regression test verified to fail without the fix. Branched off
master, not stacked on the in-flight agent-management work, so it can land on its own.What was wrong
1. Shared-token identity takeover (
auth.go) — the legacy path authenticates a gateway-wide bearer token and then believes whateveragentIDthe peer names. The gateway keys supersede, per-conn delivery, scope, and gateway-config on that label, so a token holder naming itselfagt_<victim>would evict the enrolled agent, inherit its ports, and escape its scope entirely (a shared-token identity carries no Scope, and an empty Scope means unrestricted). Revoking the victim did not help — the impersonator never presents the revoked key, so theRevokedcheck is never reached. The shared token is gateway-wide, so every legacy agent could do this to every enrolled one.2. Forgeable agentID (
cred.go) —agentIDwassha256(pubkey)truncated to 40 bits. Finding a second key that answers to a victim's name was ~2^40 keygens: hours on one rented machine, ~$50 of cloud CPU. "Unforgeable" was true of the public key, not of a 40-bit truncation of it. Now 80 bits.3. Enrollment collision (
agentstore.go) — the store is keyed by pubkey, but every management op above it (Revoke, SetScope, Rename, DesiredConfig, AdoptConfig) resolves an agent by scanning for the label. Two records sharing one name made revocation land on whichever record map iteration yielded: you revoke an agent and the wrong one dies, nondeterministically.4. Pairing code in logs and diagnostics bundles (
gateway.go) — the code embeds the gateway token verbatim (pxf://host:port/v1/pair/<token>#sha256:<fp>) and was logged at Info. slog fans out to the rotating log file, the GUI ring, and — sinceapp/tools.goships both verbatim — every diagnostics bundle. Bundles are built to be handed to someone else. MaskingGateway.Tokeninconfig.redacted.tomlbought nothing while the same token sat in cleartext one file over. Fires in GUI mode too, not just the CLI.5. Scope change did nothing to a live session (
gateway.go) — a session capturesidentity.Scopeat admission andvalidateSpecreads that copy for its whole life.SetAgentScopewrote the store and stopped, so an agent you had just narrowed kept serving its old grant — holding the ports you were taking away — until it felt like reconnecting. A misbehaving agent has no reason to reconnect, and the GUI showed the new scope the whole time. Narrowing is the urgent case: it is how you contain an agent.The root behind #4
TestWriteDiagnosticsNoLeakspassedring=nilinto a dir with no log files, so the two unredacted channels were never populated and its secret sweep ran over an empty set — a vacuous pass. That blind spot is why the leak shipped. The test now seeds every channel through the real logging fan-out, asserts the seeded log actually holds the token, and checks the logs survive scrubbing readable.logScrubbermasks the exact valuesredactConfigmasks, read out of live config — never guessed patterns. It is a net, not a licence: secrets still must not be logged.Verification
go test ./...green (unit + e2e + goleak + doccheck). Each fix demonstrated failing without its change:shared-token peer claiming enrolled id "agt_...": err = <nil>, want ErrReservedAgentID-racedid not run here (no C compiler on this machine) — CI is the only place that gate runs.Merge order
Land this before the identity-finalize branch (#13). That branch's CLAUDE.md already claims agentID is "unforgeable" and that "enrolled agents are protected by their key and by revocation". Both were aspiration when written; this PR is what makes them true. Expect one CLAUDE.md conflict there — it is the two descriptions of the same model meeting, and reconciling is the point.
Breaking
Existing
agt_ids change (40→80 bits), so enrolled agents must re-pair. Deliberate: the old ids are the vulnerable artifact. Legacy shared-token agents (bare 32-hex ids fromconfig.NewID) are untouched and still pair — covered by a test.