From 6a29a4f16a879fe48697914394bdc2a03c97e2da Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:35:49 +1200
Subject: [PATCH 1/7] security: widen the agentID so a chosen-id collision
 isn't purchasable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 internal/link/cred.go      | 27 +++++++++++++++++++++------
 internal/link/cred_test.go | 29 ++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/internal/link/cred.go b/internal/link/cred.go
index 99c915d..ff0a44c 100644
--- a/internal/link/cred.go
+++ b/internal/link/cred.go
@@ -54,18 +54,33 @@ const crockfordLower = "0123456789abcdefghjkmnpqrstvwxyz"
 var idEncoding = base32.NewEncoding(crockfordLower).WithPadding(base32.NoPadding)
 
 // fingerprint renders the first 40 bits of sha256(seed) as an 8-char base32 tag.
-// 40 bits is ample to tell a home fleet's agents apart; the astronomically rare
-// clash is resolved with a -2 suffix by whoever mints against a namespace.
+// 40 bits is a *display* budget, sized to tell a home fleet's surfaces apart at a
+// glance — it is deliberately not used where a label carries authority. GatewayID
+// is its only caller: the gateway's trust rests on the full sha256 pin carried in
+// the pairing code (cert.go Fingerprint), so gw_ is a name, never a credential.
+// Contrast AgentID, which the gateway keys authorization on and which therefore
+// takes agentFingerprintBytes.
 func fingerprint(seed []byte) string {
 	sum := sha256.Sum256(seed)
 	return idEncoding.EncodeToString(sum[:5])
 }
 
-// AgentID derives an agent's stable, unforgeable public identity from its Ed25519
-// public key: agt_<base32 fingerprint>. Derived rather than stored, so the same
-// machine always re-derives the same ID and no two keys render alike.
+// agentFingerprintBytes is how much of sha256(pubkey) an agentID carries. It is a
+// security parameter, not a display choice: the gateway keys supersede, revocation,
+// scope, and gateway-authoritative config on this label, so anyone who can find a
+// second key hashing to the same one inherits the victim's authority. At the
+// original 5 bytes (40 bits) that search cost ~2^40 keygens — hours on one rented
+// machine, i.e. forgeable. 10 bytes puts it at 2^80, which no amount of money buys,
+// and still renders as a 16-char tag. AgentStore.Enroll enforces uniqueness besides,
+// so even a found collision fails closed instead of silently sharing an identity.
+const agentFingerprintBytes = 10
+
+// AgentID derives an agent's stable public identity from its Ed25519 public key:
+// agt_<base32 fingerprint>. Derived rather than stored, so the same machine always
+// re-derives the same ID and no two keys render alike.
 func AgentID(pub ed25519.PublicKey) string {
-	return "agt_" + fingerprint(pub)
+	sum := sha256.Sum256(pub)
+	return "agt_" + idEncoding.EncodeToString(sum[:agentFingerprintBytes])
 }
 
 // GatewayID derives the gateway's display identity from its (pinned) certificate
diff --git a/internal/link/cred_test.go b/internal/link/cred_test.go
index 9c94600..68d473c 100644
--- a/internal/link/cred_test.go
+++ b/internal/link/cred_test.go
@@ -59,17 +59,14 @@ func TestAgentIDDerivation(t *testing.T) {
 	}
 }
 
-// TestAgentIDAlphabet: the fingerprint is 8 chars of lowercase Crockford base32,
-// so it never contains the confusable i/l/o/u. (identity)
+// TestAgentIDAlphabet: the fingerprint is lowercase Crockford base32, so it never
+// contains the confusable i/l/o/u. (identity)
 func TestAgentIDAlphabet(t *testing.T) {
 	pub, _, err := ed25519.GenerateKey(rand.Reader)
 	if err != nil {
 		t.Fatal(err)
 	}
 	fp := strings.TrimPrefix(AgentID(pub), "agt_")
-	if len(fp) != 8 {
-		t.Fatalf("fingerprint should be 8 chars, got %d (%q)", len(fp), fp)
-	}
 	const allowed = "0123456789abcdefghjkmnpqrstvwxyz"
 	for _, r := range fp {
 		if !strings.ContainsRune(allowed, r) {
@@ -78,6 +75,28 @@ func TestAgentIDAlphabet(t *testing.T) {
 	}
 }
 
+// TestAgentIDIsWideEnoughToBeUnforgeable pins the agentID's width as the security
+// parameter it is. The gateway keys supersede, revocation, scope, and config on this
+// label, so its width *is* the cost of forging a second key that answers to a
+// victim's name. At the original 40 bits that search was ~2^40 keygens — hours of
+// rented CPU, which is not a threat model, it's a budget line. Anything below 80
+// bits here puts that attack back on the table. Asserts the floor, not the exact
+// encoding. (identity)
+func TestAgentIDIsWideEnoughToBeUnforgeable(t *testing.T) {
+	if bits := agentFingerprintBytes * 8; bits < 80 {
+		t.Fatalf("agentID carries %d bits; below 80 a chosen-id collision is purchasable", bits)
+	}
+	pub, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The rendered tag must actually carry those bits (base32 packs 5 bits/char).
+	fp := strings.TrimPrefix(AgentID(pub), "agt_")
+	if got := len(fp) * 5; got < agentFingerprintBytes*8 {
+		t.Fatalf("rendered agentID carries only %d bits, want >= %d", got, agentFingerprintBytes*8)
+	}
+}
+
 // TestGatewayIDDerivation: the gateway's display ID derives deterministically from
 // its cert DER with the gw_ prefix. (identity)
 func TestGatewayIDDerivation(t *testing.T) {

From 9f8308cd721ca0b99efedb4243752677b76e8ba1 Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:40:22 +1200
Subject: [PATCH 2/7] security: reserve the agt_ namespace so a shared token
 can't wear an identity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 internal/gateway/auth.go      | 20 ++++++++++++
 internal/gateway/auth_test.go | 60 +++++++++++++++++++++++++++++++++++
 internal/link/cred.go         | 18 ++++++++++-
 internal/link/cred_test.go    | 21 ++++++++++++
 4 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/internal/gateway/auth.go b/internal/gateway/auth.go
index 77fd14f..0f61f32 100644
--- a/internal/gateway/auth.go
+++ b/internal/gateway/auth.go
@@ -32,11 +32,28 @@ var (
 	ErrBadToken       = errors.New("bad token")
 	ErrMissingAgentID = errors.New("missing agentId")
 	ErrRevoked        = errors.New("agent identity revoked")
+	// ErrReservedAgentID rejects a shared-token peer that names itself in the
+	// key-derived agt_ namespace. Only a verified key mints those, so the claim is
+	// always a lie — and an accepted one would hand over the named agent's session,
+	// ports, and scope.
+	ErrReservedAgentID = errors.New("agentId is reserved for enrolled identities — this agent must enroll with a pairing code")
 )
 
 // sharedTokenValidator is the legacy authenticator: one token admits every agent,
 // told apart only by the self-asserted agentID. Retained as a migration fallback,
 // gated by Gateway.AcceptSharedToken.
+//
+// Because the agentID here is *asserted* rather than proved, it must stay out of the
+// key-derived namespace: the gateway keys supersede, per-conn delivery, scope, and
+// gateway-authoritative config on that label, so a token holder allowed to name
+// itself agt_<victim> would evict the real agent, inherit its ports, and — a
+// shared-token identity carrying no Scope, and an empty Scope meaning unrestricted —
+// escape the victim's grant entirely. Revoking the victim would not even help: the
+// impersonator never presents the revoked key, so identityValidator's Revoked check
+// is never reached. Rejecting the prefix is what keeps the documented promise that
+// enrolled agents are protected by their key and by revocation; the residual risk of
+// this path stays confined to peers that are legacy on both sides.
+// Regression: TestSharedTokenCannotClaimDerivedAgentID.
 type sharedTokenValidator struct {
 	token string
 }
@@ -48,6 +65,9 @@ func (v sharedTokenValidator) Validate(hello *control.Hello, _ string) (Identity
 	if hello.AgentID == "" {
 		return Identity{}, ErrMissingAgentID
 	}
+	if link.IsDerivedAgentID(hello.AgentID) {
+		return Identity{}, ErrReservedAgentID
+	}
 	return Identity{AgentID: hello.AgentID}, nil
 }
 
diff --git a/internal/gateway/auth_test.go b/internal/gateway/auth_test.go
index 1c5b8ec..add3f3d 100644
--- a/internal/gateway/auth_test.go
+++ b/internal/gateway/auth_test.go
@@ -27,6 +27,8 @@ func TestSharedTokenValidator(t *testing.T) {
 		{"wrong token", control.Hello{Token: "nope", AgentID: "agent-1"}, "", ErrBadToken},
 		{"empty token", control.Hello{Token: "", AgentID: "agent-1"}, "", ErrBadToken},
 		{"good token but empty agentID", control.Hello{Token: "s3cret", AgentID: ""}, "", ErrMissingAgentID},
+		// The agt_ namespace belongs to proved keys; a token holder may not wear it.
+		{"good token but derived agentID", control.Hello{Token: "s3cret", AgentID: "agt_10524h9zg1n66yk1"}, "", ErrReservedAgentID},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -41,6 +43,64 @@ func TestSharedTokenValidator(t *testing.T) {
 	}
 }
 
+// TestSharedTokenCannotClaimDerivedAgentID is the regression for the shared-token
+// identity takeover: the legacy path authenticates a gateway-wide bearer token and
+// then believes whatever agentID the peer names. Since the gateway keys supersede,
+// per-conn delivery, scope, and gateway-config on that label, a token holder allowed
+// to name itself agt_<victim> would evict the enrolled agent, inherit its ports,
+// and — a shared-token identity carrying no Scope, and an empty Scope meaning
+// unrestricted — escape the victim's grant. Revoking the victim would not help: the
+// impersonator never presents the revoked key, so identityValidator's Revoked check
+// is never reached. Reserving the agt_ namespace is what keeps the documented
+// promise that enrolled agents are protected by their key and by revocation. (D7)
+func TestSharedTokenCannotClaimDerivedAgentID(t *testing.T) {
+	const fp = "sha256:deadbeef"
+	vpub, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	store, err := LoadAgentStore(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	ticket, err := store.IssueEnrollment(false, time.Time{}, Scope{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, err := store.Enroll(vpub, link.AgentID(vpub), ticket, time.Now())
+	if err != nil {
+		t.Fatal(err)
+	}
+	victimID := rec.AgentID
+	store.SetScope(victimID, Scope{Ports: []int{25565}, TunnelIDs: []string{"tnl_victim"}})
+
+	v := compositeValidator{
+		identity: identityValidator{store: store, now: time.Now},
+		shared:   &sharedTokenValidator{token: "s3cret"},
+	}
+
+	// An attacker holding only the shared token, presenting no key at all.
+	if _, err := v.Validate(&control.Hello{Token: "s3cret", AgentID: victimID}, fp); !errors.Is(err, ErrReservedAgentID) {
+		t.Fatalf("shared-token peer claiming enrolled id %q: err = %v, want ErrReservedAgentID", victimID, err)
+	}
+
+	// Same after the victim is revoked, so revocation cannot be walked around by
+	// dropping the key and falling back to the token.
+	if !store.Revoke(victimID) {
+		t.Fatal("revoke failed")
+	}
+	if _, err := v.Validate(&control.Hello{Token: "s3cret", AgentID: victimID}, fp); !errors.Is(err, ErrReservedAgentID) {
+		t.Fatalf("revoked identity %q re-admitted via shared token: err = %v", victimID, err)
+	}
+
+	// A genuinely legacy agent (bare 32-hex id from config.NewID) still pairs, so
+	// the migration path this fallback exists for is untouched.
+	const legacyID = "9f86d081884c7d659a2feaa0c55ad015"
+	if id, err := v.Validate(&control.Hello{Token: "s3cret", AgentID: legacyID}, fp); err != nil || id.AgentID != legacyID {
+		t.Fatalf("legacy agent must still authenticate: id=%+v err=%v", id, err)
+	}
+}
+
 // enrollHello builds a per-identity hello: pubkey + proof-of-possession signature
 // bound to fp, optionally carrying an enrollment ticket.
 func enrollHello(t *testing.T, priv ed25519.PrivateKey, pub ed25519.PublicKey, fp, ticket string) control.Hello {
diff --git a/internal/link/cred.go b/internal/link/cred.go
index ff0a44c..e7d2ab2 100644
--- a/internal/link/cred.go
+++ b/internal/link/cred.go
@@ -65,6 +65,15 @@ func fingerprint(seed []byte) string {
 	return idEncoding.EncodeToString(sum[:5])
 }
 
+// AgentIDPrefix marks an agentID as *derived from an Ed25519 public key*. The
+// prefix is a reserved namespace, not decoration: a legacy shared-token agent
+// self-asserts its agentID (config.NewID gives it bare 32-hex), so anything wearing
+// this prefix must have been minted by AgentID from a key the gateway verified. The
+// shared-token path therefore refuses to assert it (gateway/auth.go
+// sharedTokenValidator) — otherwise a mere token holder could name itself an
+// enrolled agent and inherit its session, ports, and scope.
+const AgentIDPrefix = "agt_"
+
 // agentFingerprintBytes is how much of sha256(pubkey) an agentID carries. It is a
 // security parameter, not a display choice: the gateway keys supersede, revocation,
 // scope, and gateway-authoritative config on this label, so anyone who can find a
@@ -80,7 +89,14 @@ const agentFingerprintBytes = 10
 // re-derives the same ID and no two keys render alike.
 func AgentID(pub ed25519.PublicKey) string {
 	sum := sha256.Sum256(pub)
-	return "agt_" + idEncoding.EncodeToString(sum[:agentFingerprintBytes])
+	return AgentIDPrefix + idEncoding.EncodeToString(sum[:agentFingerprintBytes])
+}
+
+// IsDerivedAgentID reports whether s claims the key-derived agent namespace. Only
+// AgentID mints these; a self-asserted one is a peer claiming an identity it did not
+// prove, which is why the shared-token validator rejects it outright.
+func IsDerivedAgentID(s string) bool {
+	return strings.HasPrefix(s, AgentIDPrefix)
 }
 
 // GatewayID derives the gateway's display identity from its (pinned) certificate
diff --git a/internal/link/cred_test.go b/internal/link/cred_test.go
index 68d473c..2a7efc5 100644
--- a/internal/link/cred_test.go
+++ b/internal/link/cred_test.go
@@ -97,6 +97,27 @@ func TestAgentIDIsWideEnoughToBeUnforgeable(t *testing.T) {
 	}
 }
 
+// TestIsDerivedAgentID: the agt_ namespace is reserved for key-derived ids, which is
+// what lets the gateway refuse a shared-token peer that self-asserts one
+// (gateway/auth.go sharedTokenValidator). Legacy ids from config.NewID are bare hex
+// and must not be mistaken for derived ones, or the migration path breaks. (identity)
+func TestIsDerivedAgentID(t *testing.T) {
+	pub, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if id := AgentID(pub); !IsDerivedAgentID(id) {
+		t.Fatalf("a minted agentID must be recognized as derived: %q", id)
+	}
+	// A legacy config.NewID() id: 32 hex chars, no prefix.
+	if IsDerivedAgentID("9f86d081884c7d659a2feaa0c55ad015") {
+		t.Fatal("a legacy 32-hex agentID must not read as key-derived")
+	}
+	if IsDerivedAgentID("") {
+		t.Fatal("an empty agentID must not read as key-derived")
+	}
+}
+
 // TestGatewayIDDerivation: the gateway's display ID derives deterministically from
 // its cert DER with the gw_ prefix. (identity)
 func TestGatewayIDDerivation(t *testing.T) {

From 04cb4d38b46126db12c70e9579476e20a6643605 Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:41:14 +1200
Subject: [PATCH 3/7] security: refuse an enrollment whose agentID collides
 with an enrolled agent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 internal/gateway/agentstore.go      | 16 +++++++++++
 internal/gateway/agentstore_test.go | 43 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/internal/gateway/agentstore.go b/internal/gateway/agentstore.go
index 62c2fd4..e47b66d 100644
--- a/internal/gateway/agentstore.go
+++ b/internal/gateway/agentstore.go
@@ -32,6 +32,11 @@ var (
 	ErrTicketUnknown  = errors.New("pairing code not recognized (it may have been rotated)")
 	ErrTicketConsumed = errors.New("this pairing code was already used — ask for a fresh one")
 	ErrTicketExpired  = errors.New("this pairing code has expired — ask for a fresh one")
+	// ErrAgentIDCollision fires when a joining key derives an agentID another key
+	// already holds. Every management op below resolves an agent by that label, so
+	// two keys sharing one would make revocation, scope, and config land on an
+	// arbitrary record. Refusing the join keeps the label a true primary key.
+	ErrAgentIDCollision = errors.New("this agent's derived id collides with an enrolled agent — regenerate its identity key and re-pair")
 )
 
 // Scope restricts which public ports and tunnel IDs an agent may bind. An empty
@@ -181,6 +186,17 @@ func (s *AgentStore) Enroll(pubKey []byte, agentID, ticket string, now time.Time
 	}
 
 	key := hex.EncodeToString(pubKey)
+	// agentID derives from pubKey, so a clash here means a *different* key hashed to
+	// the same label — the astronomical accident, or someone who went looking for one.
+	// Fail the join rather than let two keys answer to one name: every management op
+	// below (and the actor's live-session map) resolves an agent by that label and
+	// would otherwise land on whichever record map iteration happened to yield.
+	// Re-enrolling the *same* key is a reconnect, not a clash.
+	for k, r := range s.agents {
+		if r.AgentID == agentID && k != key {
+			return AgentRecord{}, ErrAgentIDCollision
+		}
+	}
 	rec := AgentRecord{
 		AgentID:  agentID,
 		PubKey:   append([]byte(nil), pubKey...),
diff --git a/internal/gateway/agentstore_test.go b/internal/gateway/agentstore_test.go
index 6be4b53..2b75cb1 100644
--- a/internal/gateway/agentstore_test.go
+++ b/internal/gateway/agentstore_test.go
@@ -11,6 +11,49 @@ import (
 
 func pub(b byte) []byte { return bytes.Repeat([]byte{b}, 32) }
 
+// TestEnrollRejectsAgentIDCollision: two different keys may never share one agentID.
+// The store is keyed by pubkey, but every management op below it — Revoke, SetScope,
+// Rename, DesiredConfig, AdoptConfig — resolves an agent by scanning for the *label*
+// and taking the first match, and the actor's live-session map is keyed by it too. So
+// two records answering to one name make revocation and scope land on whichever
+// record Go's map iteration happens to yield: an operator revokes an agent and the
+// wrong one dies, nondeterministically. agentID derives from the key, so a clash is
+// either an astronomical accident or someone who went looking for one; either way the
+// join fails closed and the label stays a true primary key. (identity)
+func TestEnrollRejectsAgentIDCollision(t *testing.T) {
+	s, err := LoadAgentStore(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	now := time.Now()
+	mkTicket := func() string {
+		tk, err := s.IssueEnrollment(false, time.Time{}, Scope{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		return tk
+	}
+
+	if _, err := s.Enroll(pub(1), "agt_collide", mkTicket(), now); err != nil {
+		t.Fatalf("first enroll: %v", err)
+	}
+	// A different key deriving the same label must be refused.
+	if _, err := s.Enroll(pub(2), "agt_collide", mkTicket(), now); !errors.Is(err, ErrAgentIDCollision) {
+		t.Fatalf("colliding key: err = %v, want ErrAgentIDCollision", err)
+	}
+	// The incumbent is untouched: a collision attempt must not evict or rewrite it.
+	if rec, ok := s.Lookup(pub(1)); !ok || rec.AgentID != "agt_collide" {
+		t.Fatalf("incumbent record damaged by the collision attempt: %+v ok=%v", rec, ok)
+	}
+	if _, ok := s.Lookup(pub(2)); ok {
+		t.Fatal("the colliding key must not have been enrolled")
+	}
+	// Re-enrolling the same key under its own label is a reconnect, not a clash.
+	if _, err := s.Enroll(pub(1), "agt_collide", mkTicket(), now); err != nil {
+		t.Fatalf("re-enrolling the same key must succeed: %v", err)
+	}
+}
+
 // TestAgentStoreEnrollLookupRevoke: a single-use ticket enrolls one agent, is then
 // spent, and revoke flips the record. (identity)
 func TestAgentStoreEnrollLookupRevoke(t *testing.T) {

From 8dbaaac42c4e39da2d2c19b2c61c3f3e59092387 Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:44:53 +1200
Subject: [PATCH 4/7] security: keep the pairing code out of logs and
 diagnostics bundles
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 internal/gateway/gateway.go      | 12 ++++++--
 internal/gateway/gateway_test.go | 49 ++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 internal/gateway/gateway_test.go

diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go
index 40c7caf..d0a31f3 100644
--- a/internal/gateway/gateway.go
+++ b/internal/gateway/gateway.go
@@ -142,8 +142,16 @@ func RunStarted(ctx context.Context, g *Gateway, cfg *config.Config, logger *slo
 		Token:       cfg.Gateway.Token,
 		Fingerprint: g.Fingerprint(),
 	}
-	logger.Info("gateway ready — pair agents with the code below (replace YOUR-PUBLIC-ADDRESS with this machine's public hostname or IP)")
-	logger.Info("pairing code", "code", code.String())
+	// The pairing code embeds the shared gateway token, so it goes to the console and
+	// nowhere else. slog would fan it out to three places that outlive the moment: the
+	// rotating log file, the GUI ring, and — because app/tools.go ships both verbatim
+	// — every diagnostics bundle. Bundles are built to be handed to someone else, and
+	// redactConfig masking Gateway.Token there is worth nothing while the same token
+	// rides along in cleartext inside a logged pairing code. stdout is transient and
+	// unshipped; the GUI never needed this line (it mints codes over IPC — app.go
+	// PairingCode), and the CLI operator reads it off the console they're standing at.
+	logger.Info("gateway ready — pairing code printed on the console")
+	fmt.Printf("\npair agents with this code (replace YOUR-PUBLIC-ADDRESS with this machine's public hostname or IP):\n\n    %s\n\n", code.String())
 	<-ctx.Done()
 	g.Shutdown()
 	return nil
diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go
new file mode 100644
index 0000000..b7a7c1d
--- /dev/null
+++ b/internal/gateway/gateway_test.go
@@ -0,0 +1,49 @@
+package gateway
+
+import (
+	"bytes"
+	"context"
+	"log/slog"
+	"strings"
+	"testing"
+
+	"proxyforward/internal/config"
+)
+
+// TestRunStartedKeepsThePairingCodeOutOfLogs pins the pairing code to the console.
+//
+// The code embeds the shared gateway token, and slog fans out to three places that
+// outlive the moment it is useful: the rotating log file, the GUI ring, and — since
+// app/tools.go ships both verbatim — every diagnostics bundle. Bundles exist to be
+// handed to someone else, so a logged pairing code hands over agent-level access to
+// the gateway; redactConfig masking Gateway.Token there buys nothing while the same
+// token rides along in cleartext inside a logged code. (security)
+func TestRunStartedKeepsThePairingCodeOutOfLogs(t *testing.T) {
+	var logs bytes.Buffer
+	logger := slog.New(slog.NewTextHandler(&logs, nil))
+
+	cfg := config.Default()
+	cfg.Gateway.BindAddr = "127.0.0.1" // loopback: no firewall prompt on a test bind
+	cfg.Gateway.ControlPort = 0        // ephemeral
+	cfg.Gateway.QUICEnabled = false
+	cfg.Gateway.Token = "GWTOKENSECRET"
+
+	// An already-cancelled ctx: RunStarted still starts, mints the code and emits it,
+	// then returns at its own <-ctx.Done(). Deterministic, and no sleep.
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	if err := RunStarted(ctx, New(cfg, t.TempDir(), logger), cfg, logger); err != nil {
+		t.Fatal(err)
+	}
+
+	got := logs.String()
+	if got == "" {
+		t.Fatal("RunStarted logged nothing at all — this test would pass vacuously")
+	}
+	if strings.Contains(got, cfg.Gateway.Token) {
+		t.Errorf("the gateway token reached the logs, and therefore diagnostics bundles:\n%s", got)
+	}
+	if strings.Contains(got, "pxf://") {
+		t.Errorf("the pairing code reached the logs:\n%s", got)
+	}
+}

From 1ea8fb9698aff8244030ce3c89713948d0f98254 Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:48:25 +1200
Subject: [PATCH 5/7] security: scrub known secrets from the logs a diagnostics
 bundle ships
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 app/tools.go      | 98 +++++++++++++++++++++++++++++++++++++++++------
 app/tools_test.go | 59 ++++++++++++++++++++++++++--
 2 files changed, 141 insertions(+), 16 deletions(-)

diff --git a/app/tools.go b/app/tools.go
index 1553672..0d57311 100644
--- a/app/tools.go
+++ b/app/tools.go
@@ -2,6 +2,7 @@ package app
 
 import (
 	"archive/zip"
+	"bufio"
 	"crypto/sha256"
 	"encoding/json"
 	"fmt"
@@ -51,11 +52,13 @@ func testReachability(host string, port int) (string, error) {
 	return fmt.Sprintf("Reachable: %s answered in %s — players can connect.", addr, time.Since(start).Round(time.Millisecond)), nil
 }
 
-// writeDiagnostics builds the support bundle: version, a fully redacted config,
-// a health summary, the recent in-memory log lines, the persisted stats (peer
-// IPs pseudonymized), and every on-disk log file (rotated + crash + wails).
-// Everything that could identify a host, network, or client is masked so the
-// bundle is safe to share.
+// writeDiagnostics builds the support bundle: version, a fully redacted config, a
+// health summary, the recent in-memory log lines, the persisted stats (peer IPs
+// pseudonymized), and every on-disk log file (rotated + crash + wails). Everything
+// that could identify a host, network, or client is masked so the bundle is safe to
+// share — logs included, via logScrubber: they are shipped, so redacting only the
+// config would leave the same secrets in cleartext one file over.
+// Leak-tested by TestWriteDiagnosticsNoLeaks, which seeds every channel here.
 func writeDiagnostics(path string, cfg *config.Config, configDir, health string, ring *logging.Ring) error {
 	f, err := os.Create(path)
 	if err != nil {
@@ -93,11 +96,17 @@ func writeDiagnostics(path string, cfg *config.Config, configDir, health string,
 		}
 	}
 
+	// Logs ship with the config's own secrets masked. Without this the redaction
+	// above is theatre: config.redacted.toml hides Gateway.Token while a log line
+	// three files over spells it out.
+	scrub := newLogScrubber(cfg)
+
 	// logs-recent.txt — the GUI ring (what the user was just looking at).
 	if ring != nil {
 		if w, err := zw.Create("logs-recent.txt"); err == nil {
 			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))
 			}
 		}
 	}
@@ -114,21 +123,86 @@ func writeDiagnostics(path string, cfg *config.Config, configDir, health string,
 		filepath.Join(logDir, "wails.log"),
 	}
 	for _, p := range logFiles {
-		copyIntoZip(zw, filepath.Base(p), p)
+		copyScrubbedIntoZip(zw, filepath.Base(p), p, scrub)
 	}
 	return nil
 }
 
-// copyIntoZip streams src into the archive under nameInZip; missing files are
-// skipped without error (a diagnostics bundle is best-effort).
-func copyIntoZip(zw *zip.Writer, nameInZip, src string) {
+// logScrubber replaces the exact secret values this bundle already knows — the ones
+// redactConfig masks in config.redacted.toml — wherever they appear in shipped log
+// text. It is the reason the bundle can claim to be shareable at all: redacting the
+// config is worth nothing if the same token is sitting in a log line two files over,
+// which is exactly how the pairing code used to escape (gateway.go RunStarted).
+//
+// Exact-value replacement, never pattern-guessing: every entry is a literal read out
+// of the live config, so this cannot pass off a guess as a guarantee. It is a net,
+// not a licence — secrets still must not be logged in the first place, because this
+// only knows the values config holds. Values shorter than minScrubLen are skipped:
+// they carry little entropy and would smear over unrelated log text.
+type logScrubber struct{ pairs []string } // old1, new1, old2, new2 … for strings.NewReplacer
+
+// minScrubLen is the shortest value worth exact-matching in log text. Below this a
+// "secret" is more likely to collide with ordinary words than to be the secret.
+const minScrubLen = 4
+
+func newLogScrubber(cfg *config.Config) *logScrubber {
+	const secret = "[redacted]"
+	const host = "[redacted-host]"
+	s := &logScrubber{}
+	add := func(val, with string) {
+		if len(val) >= minScrubLen {
+			s.pairs = append(s.pairs, val, with)
+		}
+	}
+	// Mirrors redactConfig field for field: whatever is a secret in the config is a
+	// secret in the logs. If a field is added there, add it here.
+	add(cfg.Gateway.Token, secret)
+	add(cfg.Agent.Token, secret)
+	add(cfg.Agent.AgentID, secret)
+	add(cfg.Agent.CertFingerprint, secret)
+	add(cfg.Gateway.PublicHost, host)
+	add(cfg.Gateway.BindAddr, host)
+	add(cfg.Agent.GatewayHost, host)
+	add(cfg.Metrics.PrometheusAddr, host)
+	for _, t := range cfg.Agent.Tunnels {
+		add(t.LocalAddr, host)
+	}
+	return s
+}
+
+// clean returns line with every known secret replaced.
+func (s *logScrubber) clean(line string) string {
+	if len(s.pairs) == 0 {
+		return line
+	}
+	return strings.NewReplacer(s.pairs...).Replace(line)
+}
+
+// copyScrubbedIntoZip streams src into the archive under nameInZip with every known
+// secret masked. Line-oriented, so memory stays bounded on a rotated log and a secret
+// (which never spans a newline) can't slip through a chunk boundary. Missing files
+// are skipped without error — a diagnostics bundle is best-effort.
+func copyScrubbedIntoZip(zw *zip.Writer, nameInZip, src string, s *logScrubber) {
 	lf, err := os.Open(src)
 	if err != nil {
 		return
 	}
 	defer lf.Close()
-	if w, err := zw.Create(nameInZip); err == nil {
-		io.Copy(w, lf)
+	w, err := zw.Create(nameInZip)
+	if err != nil {
+		return
+	}
+	br := bufio.NewReader(lf)
+	for {
+		// ReadString, not bufio.Scanner: a single pathological log line must not hit
+		// 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))
+		}
+		if err != nil {
+			return
+		}
 	}
 }
 
diff --git a/app/tools_test.go b/app/tools_test.go
index f15d3e2..37d0401 100644
--- a/app/tools_test.go
+++ b/app/tools_test.go
@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"proxyforward/internal/config"
+	"proxyforward/internal/logging"
 )
 
 func sampleConfig() *config.Config {
@@ -71,15 +72,51 @@ func TestRedactStatsJSONHashesPeerIPs(t *testing.T) {
 	}
 }
 
+// TestWriteDiagnosticsNoLeaks drives every channel writeDiagnostics ships, because a
+// bundle is only as shareable as its leakiest file. It previously passed ring=nil
+// into an empty dir, so the two *unredacted* channels — the GUI ring and the on-disk
+// log files — were never populated and the secret sweep below ran over an empty set.
+// That blind spot is how a logged pairing code (which embeds Gateway.Token verbatim)
+// shipped in cleartext while config.redacted.toml sat right beside it masking the
+// same token. The logger here is the real production fan-out, so the ring and the
+// rotating file are filled the way they are in a live process.
 func TestWriteDiagnosticsNoLeaks(t *testing.T) {
 	dir := t.TempDir()
 	// Seed a stats.json with a client IP that must not leak.
 	os.WriteFile(filepath.Join(dir, "stats.json"),
 		[]byte(`{"v":3,"peers":[{"ip":"203.0.113.77","totalConns":2}]}`), 0o600)
 
-	out := filepath.Join(dir, "diag.zip")
 	cfg := sampleConfig()
-	if err := writeDiagnostics(out, cfg, dir, "health: good\n", nil); err != nil {
+
+	// Fill the ring and the on-disk log through the real logger, with lines that
+	// carry the secrets the way a careless call site would.
+	ring := logging.NewRing(64)
+	logger, closeLog, err := logging.New(logging.Options{
+		FilePath: logging.DefaultFilePath(dir),
+		Ring:     ring,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	logger.Info("pairing code", "code",
+		"pxf://"+cfg.Gateway.PublicHost+":8474/v1/pair/"+cfg.Gateway.Token+"#"+cfg.Agent.CertFingerprint)
+	logger.Info("agent connected", "agentId", cfg.Agent.AgentID, "gateway", cfg.Agent.GatewayHost)
+	logger.Warn("dial failed", "local", cfg.Agent.Tunnels[0].LocalAddr, "bind", cfg.Gateway.BindAddr)
+	if err := closeLog(); err != nil {
+		t.Fatal(err)
+	}
+	// Guard against a vacuous pass: the on-disk log must really hold the secret, or
+	// this test proves nothing about the path that ships it.
+	onDisk, err := os.ReadFile(logging.DefaultFilePath(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(onDisk), cfg.Gateway.Token) {
+		t.Fatal("setup bug: the seeded log does not contain the token, so this test would pass vacuously")
+	}
+
+	out := filepath.Join(dir, "diag.zip")
+	if err := writeDiagnostics(out, cfg, dir, "health: good\n", ring); err != nil {
 		t.Fatal(err)
 	}
 
@@ -100,13 +137,27 @@ func TestWriteDiagnosticsNoLeaks(t *testing.T) {
 		names[f.Name] = string(b)
 	}
 
-	for _, want := range []string{"version.txt", "health.txt", "config.redacted.toml", "stats.redacted.json"} {
+	// The log channels must actually be in the bundle — if they silently stopped
+	// shipping, the sweep below would pass for the wrong reason.
+	for _, want := range []string{"version.txt", "health.txt", "config.redacted.toml", "stats.redacted.json", "logs-recent.txt", "proxyforward.log"} {
 		if _, ok := names[want]; !ok {
 			t.Errorf("bundle missing %s (have %v)", want, keys(names))
 		}
 	}
+	// Both log files must still be diagnostically useful after scrubbing — masking by
+	// shipping nothing would pass the sweep and defeat the point of the bundle.
+	for _, name := range []string{"logs-recent.txt", "proxyforward.log"} {
+		if !strings.Contains(names[name], "pairing code") || !strings.Contains(names[name], "dial failed") {
+			t.Errorf("%s lost its log lines to scrubbing:\n%s", name, names[name])
+		}
+	}
+
 	all := strings.Join(values(names), "\n")
-	for _, secret := range []string{"AGENTTOKENSECRET", "GWTOKENSECRET", "gw.secret.example.com", "203.0.113.77", "deadbeefsecret"} {
+	for _, secret := range []string{
+		"AGENTTOKENSECRET", "GWTOKENSECRET", "AGENTIDSECRET", "deadbeefsecret",
+		"gw.secret.example.com", "public.secret.example.com", "192.168.50.1",
+		"10.9.8.7", "127.0.0.99", "203.0.113.77",
+	} {
 		if strings.Contains(all, secret) {
 			t.Errorf("diagnostics bundle leaked %q", secret)
 		}

From 73d705db06364bbc4786589e1c6695e36b5f1dad Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:50:49 +1200
Subject: [PATCH 6/7] security: evict on scope change so a narrowed grant takes
 effect now
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 internal/e2e/e2e_test.go    | 40 +++++++++++++++++++++++++++++++++++++
 internal/gateway/gateway.go | 32 ++++++++++++++++++++++++++---
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/internal/e2e/e2e_test.go b/internal/e2e/e2e_test.go
index a1fb751..171f0b5 100644
--- a/internal/e2e/e2e_test.go
+++ b/internal/e2e/e2e_test.go
@@ -1324,6 +1324,46 @@ func TestEnrollAndRevoke(t *testing.T) {
 	h.agentStop = nil // we already drained agentDone; keep cleanup from blocking
 }
 
+// TestSetAgentScopeEvictsLiveSession: narrowing an agent's scope must bind now, not
+// eventually. A session captures identity.Scope at admission and validateSpec reads
+// that copy for the session's whole life, so writing the store alone left an agent
+// the operator had just narrowed serving its old grant — holding the very ports the
+// operator was taking away — until it happened to reconnect, which a misbehaving
+// agent has no reason to do. Narrowing is the urgent case: it is how you contain an
+// agent. RevokeAgent already evicts to make its guarantee land; this is the same
+// guarantee for scope. (identity)
+func TestSetAgentScopeEvictsLiveSession(t *testing.T) {
+	echoAddr, closeEcho := echoServer(t)
+	defer closeEcho()
+	h := newHarnessWith(t, echoAddr, harnessOpts{enroll: true})
+	addr := h.waitPublicPort()
+
+	// The agent is enrolled and serving on its granted (unrestricted) scope.
+	agents := h.gw.ListAgents()
+	if len(agents) != 1 {
+		t.Fatalf("want 1 enrolled agent, got %d: %+v", len(agents), agents)
+	}
+	rec := agents[0]
+	roundTrip(t, addr, []byte("in scope"))
+
+	// Narrow the grant so the tunnel it is currently serving is no longer allowed.
+	if !h.gw.SetAgentScope(rec.AgentID, gateway.Scope{TunnelIDs: []string{"tnl_something-else"}}) {
+		t.Fatal("SetAgentScope reported the agent was not found")
+	}
+
+	// The listener must go away on its own. The agent stays up and reconnects, but
+	// every bind is now re-checked against the narrowed scope, so it never comes
+	// back — without the eviction the old session keeps serving here indefinitely.
+	deadline := time.Now().Add(20 * time.Second)
+	for time.Now().Before(deadline) {
+		if _, ok := h.gw.TunnelPort(h.tunnelID); !ok {
+			return
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	t.Fatal("narrowed scope never took effect: the tunnel is still bound, so the agent kept the port the operator took away")
+}
+
 // TestBurstThroughputAndCrossStreamLatency pushes 64 MiB through one
 // connection while a second connection does small echo round-trips; the
 // burst must move fast and must not starve the small stream. This is the
diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go
index d0a31f3..641fcc6 100644
--- a/internal/gateway/gateway.go
+++ b/internal/gateway/gateway.go
@@ -302,14 +302,40 @@ func (g *Gateway) RevokeAgent(agentID string) bool {
 	return ok
 }
 
-// RenameAgent sets an agent's display nickname; SetAgentScope replaces its bind
-// scope. Both report whether the agent was found.
+// RenameAgent sets an agent's display nickname. A nickname is display sugar with no
+// bearing on what the agent may bind, so a live session needs no disturbing. Reports
+// whether the agent was found.
 func (g *Gateway) RenameAgent(agentID, nickname string) bool {
 	return g.agents != nil && g.agents.Rename(agentID, nickname)
 }
 
+// SetAgentScope replaces an agent's bind scope and evicts any live session, so the
+// new grant takes effect now rather than whenever the agent next reconnects.
+//
+// A session captures identity.Scope at admission (buildAndAdmit) and validateSpec
+// reads that copy for the session's whole life, so writing the store alone would
+// leave an agent an operator just narrowed running on its old grant indefinitely —
+// keeping the ports it already holds — while the GUI showed the new scope. Narrowing
+// is the urgent case precisely because it is how you contain a misbehaving agent.
+// Evicting is how RevokeAgent already makes this guarantee: the agent reconnects,
+// re-reads its record, and every bind is re-checked against the narrowed scope. It
+// also sidesteps mutating sess.scope under readers on other goroutines. Reports
+// whether the agent was found.
+// Regression: e2e TestSetAgentScopeEvictsLiveSession.
 func (g *Gateway) SetAgentScope(agentID string, scope Scope) bool {
-	return g.agents != nil && g.agents.SetScope(agentID, scope)
+	if g.agents == nil {
+		return false
+	}
+	ok := g.agents.SetScope(agentID, scope)
+	if a := g.act(); a != nil && ok {
+		// One actor hop: eviction is lifecycle, and lifecycle is actor-owned.
+		a.do(func() {
+			if s := a.agents[agentID]; s != nil {
+				a.evict(s, "agent scope changed")
+			}
+		})
+	}
+	return ok
 }
 
 // AgentView is one agent's management row for the GUI roster: its persisted

From 4c95119bb22105d5189db6365685974c4f3cc7bd Mon Sep 17 00:00:00 2001
From: xeri <109935338+xeri@users.noreply.github.com>
Date: Thu, 16 Jul 2026 23:53:37 +1200
Subject: [PATCH 7/7] docs: state the agentID trust model the code now actually
 keeps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 CLAUDE.md | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index d06b113..d09eb86 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -77,23 +77,40 @@ Each entry: the rule, why, and the symbol that embodies it today. Numbers live i
   connGate`; defaults in `config.go`).
 - **Per-agent identity is the trust model.** Each agent proves possession of a long-term
   Ed25519 key the gateway allowlists; its derived `agentID` (`agt_…`) is unforgeable,
-  individually **scoped** (ports/tunnels) and **revocable** — revocation evicts the live
-  session and makes the next connect fatal `ErrCodeRevoked`. Agents join via a single-use
-  enrollment ticket in the pairing code (`gateway/auth.go identityValidator`, `agentstore.go`,
-  `link/cred.go`; mechanics in `docs/agent/architecture.md`). A matching agentID
-  **supersedes** (reconnect), a distinct one is admitted **alongside**; supersede is
-  anti-flap dampened so an ID collision degrades to a slow contest, not a loop
-  (`actor.go admit`, `noteSupersede`).
+  individually **scoped** (ports/tunnels) and **revocable** — revocation *and* a scope
+  change both evict the live session, and the next connect is fatal `ErrCodeRevoked`. A
+  session captures its scope at admission, so without that eviction narrowing contains
+  nothing (`gateway.go SetAgentScope`). Agents join via a single-use enrollment ticket in
+  the pairing code (`gateway/auth.go identityValidator`, `agentstore.go`, `link/cred.go`;
+  mechanics in `docs/agent/architecture.md`). A matching agentID **supersedes**
+  (reconnect), a distinct one is admitted **alongside**; supersede is anti-flap dampened
+  so an ID collision degrades to a slow contest, not a loop (`actor.go admit`,
+  `noteSupersede`).
+- "Unforgeable" above is a claim about **width**, and therefore a security parameter, not
+  a display choice: `agentID` is the label the gateway keys supersede, scope, revocation,
+  and config on, so it must stay wide enough that no second key answering to a victim's
+  name can be searched for (`cred.go agentFingerprintBytes`, floored by
+  `TestAgentIDIsWideEnoughToBeUnforgeable`); `AgentStore.Enroll` refuses a colliding join
+  besides, so a found collision fails closed.
 - The legacy **shared token** survives only as a migration fallback, accepted while
   `Gateway.AcceptSharedToken` is on (default for now). A shared-token agent self-asserts its
   `agentID`, so the old residual risk — supersede or port-squat, recoverable only by rotating
   the token — persists **for that path only**; enrolled agents are protected by their key and
-  by revocation, and disabling `AcceptSharedToken` once every agent is enrolled closes it.
+  by revocation *because* the key-derived `agt_` namespace is **reserved**, so a self-asserted
+  ID can never name an enrolled one (`link/cred.go IsDerivedAgentID`, `gateway/auth.go
+  sharedTokenValidator`). Disabling `AcceptSharedToken` once every agent is enrolled closes it.
 - The IPC pipe ACL admits Administrators, SYSTEM, and the interactive user only
   (`ipc/server_windows.go pipeSecurity`).
 - Diagnostics bundles redact every secret, host, IP, and identity; peer IPs become
   stable sha256 pseudonyms (`app/tools.go`, leak-tested in `app/tools_test.go`).
-  Anything new that exports data must pass the same no-leak test style.
+  Anything new that exports data must pass the same no-leak test style — seeding
+  *every* channel it ships, since a leak test that leaves one empty passes vacuously.
+  Bundles **ship logs**, so redacting only the config is theatre: log text is scrubbed
+  of the config's own secrets (`tools.go logScrubber`).
+- Never log a secret; the scrubber is a net, not a licence (it only knows what config
+  holds). The pairing code embeds the gateway token, so it goes to the console, never
+  through `slog` — slog fans out to the rotating file, the GUI ring, and every bundle
+  (`gateway.go RunStarted`).
 - Fatal auth errors (`bad_token`, `agent_conflict`, `version`, `revoked`) stop the agent
   instead of retry-hammering the gateway (fatal classification in `agent.go isFatal`), and
   surface in the UI via `EngineFatal` on the tick.