fix(trust): reject unsigned runtime allowlists

README.md

@@ -6,8 +6,8 @@
 
 Trusted-agents plugin for the Pilot Protocol daemon. Ships an embedded
 allowlist of public node IDs that the daemon auto-accepts handshake
-requests from, plus a 1-hour refresher loop that pulls the canonical
-list from this repo on a schedule.
+requests from. Release builds may include an Ed25519 verifier that enables
+a 1-hour refresher loop; every remotely fetched list must be signed.
 
 ## Install
 
@@ -65,9 +65,10 @@ path requires upstream wiring — see the TODO on `Service.IsTrustedWithKey`.
 
 ## Updating the list
 
-Edit `trusted-agents.json` and open a PR. Once merged, daemons in the
-field pick up the new list on their next 1-hour refresh tick. Brand-new
-daemons get the embedded copy compiled into the binary.
+Edit `trusted-agents.json` and open a PR. Brand-new daemons get the reviewed
+copy compiled into the binary. Binaries built with `embeddedPubKeyHex` also
+pick up a validly signed remote list on their next 1-hour refresh tick.
+Binaries without that verifier never fetch or accept runtime trust changes.
 
 ## Build tags
 

data.go

@@ -6,23 +6,23 @@
 // and the CLI (cmd/pilotctl) can read it without violating the strict
 // downward layer rule.
 //
-// The list is plain JSON in this directory, embedded at build time and
-// refreshed hourly from raw.githubusercontent.com by
-// plugins/trustedagents.Run. Authenticity is handled in two tiers:
-// unsigned lists are accepted over TLS with a warning (backward-compatible);
-// signed lists require embeddedPubKey to be configured and undergo full
-// Ed25519 verification via VerifyAndStripSig before the payload is trusted.
+// The list is plain JSON in this directory and embedded at build time.
+// Runtime refresh is enabled only when the binary contains an Ed25519
+// verifier key. Every fetched list must carry a valid signature; otherwise
+// the reviewed build-embedded list remains active.
 //
-// Adding an agent: edit trusted-agents.json, commit. Daemons in the
-// field pick it up within ~1h. Brand-new daemons get the embedded copy
-// from the binary, so the feature works on first boot even airgapped.
+// Adding an agent: edit trusted-agents.json, review, and commit. Brand-new
+// daemons get that embedded copy from the binary, so the feature works on
+// first boot even airgapped. Runtime updates require a signed list and a
+// verifier key compiled into the binary.
 package trustedagents
 
 import (
 	"crypto/ed25519"
 	"crypto/subtle"
 	_ "embed"
 	"encoding/base64"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"log/slog"
@@ -94,6 +94,7 @@ func decodePin(b64 string) (ed25519.PublicKey, error) {
 }
 
 func init() {
+	configureEmbeddedPubKey()
 	if err := Load(embeddedJSON); err != nil {
 		// CI guards this via TestEmbeddedListLoads; if it ever fires in
 		// production, an empty list (zero auto-accepts) is the safe default.
@@ -202,11 +203,9 @@ func All() []Agent {
 	return out
 }
 
-// embeddedPubKey is the ed25519 public key used to verify the signature
-// on the runtime-fetched trusted-agents JSON. When all 32 bytes are zero
-// the key has not been configured yet and signature verification is
-// skipped (backward-compatible). Set this to the real public key once the
-// signing infrastructure is in place.
+// embeddedPubKey is the Ed25519 public key used to verify the signature on
+// runtime-fetched trusted-agents JSON. When all 32 bytes are zero, runtime
+// refresh is disabled and the reviewed build-embedded list remains active.
 //
 // To inject: go build -ldflags "-X github.com/pilot-protocol/trustedagents.embeddedPubKeyHex=<64-hex-chars>"
 // Generate keypair: scripts/gen-signing-key.sh
@@ -215,11 +214,27 @@ var embeddedPubKey = ed25519.PublicKey{
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 }
 
+// embeddedPubKeyHex is intentionally a string so release builds can inject a
+// verifier with -ldflags -X. An invalid value leaves embeddedPubKey at zero,
+// which safely disables runtime refresh.
+var embeddedPubKeyHex string
+
+func configureEmbeddedPubKey() {
+	if embeddedPubKeyHex == "" {
+		return
+	}
+	raw, err := hex.DecodeString(embeddedPubKeyHex)
+	if err != nil || len(raw) != ed25519.PublicKeySize {
+		slog.Error("trustedagents: invalid embedded verifier key; runtime refresh disabled",
+			"bytes", len(raw), "err", err)
+		return
+	}
+	copy(embeddedPubKey, raw)
+}
+
 // VerifyAndStripSig checks the ed25519 signature embedded in the fetched
-// JSON. If no "signature" field is present the raw body is returned as-is
-// (backward-compatible with unsigned lists). If the field is present the
-// signature is verified against embeddedPubKey; on mismatch an error is
-// returned so the caller falls back to the embedded list.
+// JSON. Both a configured verifier and a non-empty signature are required.
+// Any failure leaves the caller's reviewed build-embedded list untouched.
 func VerifyAndStripSig(raw []byte) ([]byte, error) {
 	// Decode the entire doc to extract the signature field.
 	var envelope struct {
@@ -230,12 +245,10 @@ func VerifyAndStripSig(raw []byte) ([]byte, error) {
 		return nil, fmt.Errorf("verify: parse: %w", err)
 	}
 
-	// No signature → accept unsigned (backward compat).
+	// Runtime trust updates must be authenticated. HTTPS still protects the
+	// transport, but it is not authorization to change an auto-trust list.
 	if envelope.Signature == nil || *envelope.Signature == "" {
-		slog.Warn("trustedagents: fetched list has no signature — " +
-			"accepting anyway (TLS-only trust). This will become a hard " +
-			"error once signing is deployed.")
-		return raw, nil
+		return nil, fmt.Errorf("verify: signature is required")
 	}
 
 	// Public key not configured → reject: a signature exists but we

runtime.go

@@ -38,11 +38,17 @@ var initialJitterMax = 30 * time.Second
 // real network.
 var httpClientForRun = func() *http.Client { return &http.Client{Timeout: 30 * time.Second} }
 
-// Run polls the canonical URL on a timer, replacing the active list
-// whenever a new one is fetched. Blocks until ctx is cancelled. The
-// first fetch is delayed 0–30s so a fleet rebooting at the same time
-// doesn't thunder the URL.
+// Run polls the canonical URL on a timer, replacing the active list only
+// after signature verification. A binary without a verifier key keeps its
+// reviewed build-embedded list and performs no network refresh. Blocks until
+// ctx is cancelled. The first fetch is delayed 0–30s so a fleet rebooting at
+// the same time doesn't thunder the URL.
 func Run(ctx context.Context) {
+	if isZeroKey(embeddedPubKey) {
+		slog.Info("trustedagents: runtime refresh disabled; using build-embedded list")
+		<-ctx.Done()
+		return
+	}
 	client := httpClientForRun()
 	timer := time.NewTimer(jitter(initialJitterMax))
 	defer timer.Stop()
@@ -78,9 +84,8 @@ func fetchOnce(ctx context.Context, client *http.Client) error {
 	if err != nil {
 		return err
 	}
-	// Verify ed25519 signature (if present) before trusting the list.
-	// On absent/mismatched signature, return error → Run falls back to
-	// the embedded list.
+	// Require and verify the Ed25519 signature before trusting the list.
+	// On any verification failure, Run keeps the embedded list.
 	verified, err := VerifyAndStripSig(body)
 	if err != nil {
 		return fmt.Errorf("verify: %w", err)

zz_fetch_test.go

@@ -9,10 +9,8 @@
 // raw.githubusercontent.com to a local httptest server. The transport
 // is the only seam available without refactoring source files.
 //
-// Iter-1 audit (HIGH) flagged: no signature verification on
-// runtime-fetched allowlist. TestFetchOnce_AcceptsAnyJSON_NoSignatureCheck
-// pins that behaviour so any future signature work breaks the test and
-// forces a deliberate update.
+// Runtime-fetched allowlists are fail closed: a verifier key must be
+// configured and every response must carry a valid signature.
 
 package trustedagents
 
@@ -61,22 +59,56 @@ func newRewriteClient(srv *httptest.Server) (*http.Client, *rewriteTransport) {
 	return &http.Client{Transport: rt, Timeout: 5 * time.Second}, rt
 }
 
+func setVerifierForTest(t *testing.T, key ed25519.PublicKey) {
+	t.Helper()
+	previous := append(ed25519.PublicKey(nil), embeddedPubKey...)
+	t.Cleanup(func() { copy(embeddedPubKey, previous) })
+	clear(embeddedPubKey)
+	copy(embeddedPubKey, key)
+}
+
+func signAgentListForTest(t *testing.T, priv ed25519.PrivateKey, agents json.RawMessage) []byte {
+	t.Helper()
+	type envelope struct {
+		Agents    json.RawMessage `json:"agents"`
+		Signature *string         `json:"signature,omitempty"`
+	}
+	env := envelope{Agents: agents}
+	payload, err := json.Marshal(env)
+	if err != nil {
+		t.Fatalf("marshal payload: %v", err)
+	}
+	sig := base64.StdEncoding.EncodeToString(ed25519.Sign(priv, payload))
+	env.Signature = &sig
+	signed, err := json.Marshal(env)
+	if err != nil {
+		t.Fatalf("marshal signed payload: %v", err)
+	}
+	return signed
+}
+
 // TestFetchOnce_Success drives the full happy path: 200 + valid JSON
 // body. After the call Load() must have populated the global list.
 func TestFetchOnce_Success(t *testing.T) {
 	// Mutates package state via Load — no t.Parallel.
 	restore := SetForTest(nil)
 	t.Cleanup(restore)
+	pub, priv, err := ed25519.GenerateKey(cryptorand.Reader)
+	if err != nil {
+		t.Fatalf("keygen: %v", err)
+	}
+	setVerifierForTest(t, pub)
+	signed := signAgentListForTest(t, priv, json.RawMessage(`[
+		{"hostname":"injected","address":"0:0:1","node_id":4242}
+	]`))
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		// Confirm fetchOnce attached the documented UA.
 		if got := req.Header.Get("User-Agent"); got != "pilot-daemon/trustedagents" {
 			t.Errorf("User-Agent = %q, want pilot-daemon/trustedagents", got)
 		}
 		w.WriteHeader(200)
-		_, _ = io.WriteString(w, `{"agents":[
-			{"hostname":"injected","address":"0:0:1","node_id":4242}
-		]}`)
+		_, _ = w.Write(signed)
 	}))
 	defer srv.Close()
 
@@ -92,13 +124,16 @@ func TestFetchOnce_Success(t *testing.T) {
 	}
 }
 
-// TestFetchOnce_AcceptsUnsignedJSON_BackwardCompat verifies that an
-// unsigned trusted-agents list (no "signature" field) is still accepted
-// when the embedded public key is the zero placeholder. This preserves
-// backward compatibility until the operator deploys signing.
-func TestFetchOnce_AcceptsUnsignedJSON_BackwardCompat(t *testing.T) {
+// TestFetchOnce_RejectsUnsignedJSON verifies an unsigned runtime response
+// cannot replace the active reviewed list.
+func TestFetchOnce_RejectsUnsignedJSON(t *testing.T) {
 	restore := SetForTest(nil)
 	t.Cleanup(restore)
+	pub, _, err := ed25519.GenerateKey(cryptorand.Reader)
+	if err != nil {
+		t.Fatalf("keygen: %v", err)
+	}
+	setVerifierForTest(t, pub)
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(200)
@@ -109,24 +144,21 @@ func TestFetchOnce_AcceptsUnsignedJSON_BackwardCompat(t *testing.T) {
 	defer srv.Close()
 
 	client, _ := newRewriteClient(srv)
-	if err := fetchOnce(context.Background(), client); err != nil {
-		t.Fatalf("fetchOnce with unsigned JSON: %v", err)
+	err = fetchOnce(context.Background(), client)
+	if err == nil || !strings.Contains(err.Error(), "signature is required") {
+		t.Fatalf("fetchOnce unsigned error = %v, want signature-required error", err)
 	}
-	if _, ok := IsTrusted(1); !ok {
-		t.Fatal("unsigned JSON should still be accepted (backward compat)")
+	if _, ok := IsTrusted(1); ok {
+		t.Fatal("unsigned JSON changed the active trust list")
 	}
 }
 
-// TestVerifyAndStripSig_UnsignedIsOK confirms VerifyAndStripSig returns the
-// raw body unchanged when no signature field is present.
-func TestVerifyAndStripSig_UnsignedIsOK(t *testing.T) {
+// TestVerifyAndStripSig_UnsignedIsRejected confirms a signature is mandatory.
+func TestVerifyAndStripSig_UnsignedIsRejected(t *testing.T) {
 	raw := []byte(`{"agents":[{"hostname":"x","node_id":7}]}`)
-	out, err := VerifyAndStripSig(raw)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	if string(out) != string(raw) {
-		t.Errorf("output differs from input on unsigned payload")
+	_, err := VerifyAndStripSig(raw)
+	if err == nil || !strings.Contains(err.Error(), "signature is required") {
+		t.Fatalf("error = %v, want signature-required error", err)
 	}
 }
 
@@ -155,6 +187,7 @@ func TestVerifyAndStripSig_BadSignatureIsRejected(t *testing.T) {
 // TestVerifyAndStripSig_SignaturePresentButKeyNotConfigured confirms that
 // a payload WITH a signature is rejected when embeddedPubKey is zero.
 func TestVerifyAndStripSig_SignaturePresentButKeyNotConfigured(t *testing.T) {
+	setVerifierForTest(t, make(ed25519.PublicKey, ed25519.PublicKeySize))
 	raw := []byte(`{"agents":[{"hostname":"x","node_id":3}],"signature":"AAAA"}`)
 	_, err := VerifyAndStripSig(raw)
 	if err == nil {
@@ -207,6 +240,35 @@ func TestVerifyAndStripSig_ValidSignatureIsAccepted(t *testing.T) {
 	}
 }
 
+func TestConfigureEmbeddedPubKey_FromLinkerString(t *testing.T) {
+	setVerifierForTest(t, make(ed25519.PublicKey, ed25519.PublicKeySize))
+	previousHex := embeddedPubKeyHex
+	embeddedPubKeyHex = strings.Repeat("ab", ed25519.PublicKeySize)
+	t.Cleanup(func() { embeddedPubKeyHex = previousHex })
+
+	configureEmbeddedPubKey()
+	if isZeroKey(embeddedPubKey) {
+		t.Fatal("configured verifier remained zero")
+	}
+	for i, value := range embeddedPubKey {
+		if value != 0xab {
+			t.Fatalf("configured verifier byte %d = %x, want ab", i, value)
+		}
+	}
+}
+
+func TestConfigureEmbeddedPubKey_InvalidValueFailsClosed(t *testing.T) {
+	setVerifierForTest(t, make(ed25519.PublicKey, ed25519.PublicKeySize))
+	previousHex := embeddedPubKeyHex
+	embeddedPubKeyHex = "not-a-valid-ed25519-key"
+	t.Cleanup(func() { embeddedPubKeyHex = previousHex })
+
+	configureEmbeddedPubKey()
+	if !isZeroKey(embeddedPubKey) {
+		t.Fatal("invalid verifier configured a non-zero key")
+	}
+}
+
 // TestFetchOnce_BadStatus drives the resp.StatusCode != 200 branch.
 func TestFetchOnce_BadStatusDriven(t *testing.T) {
 	// Mutates package state via fetchMu — no t.Parallel.
@@ -414,6 +476,7 @@ func TestRun_TimerFires(t *testing.T) {
 		return &http.Client{Transport: &errTransport{err: errors.New("injected: no network")}}
 	}
 	t.Cleanup(func() { httpClientForRun = prevClient })
+	setVerifierForTest(t, ed25519.PublicKey(strings.Repeat("x", ed25519.PublicKeySize)))
 
 	restore := SetForTest(nil)
 	t.Cleanup(restore)
@@ -434,6 +497,36 @@ func TestRun_TimerFires(t *testing.T) {
 	}
 }
 
+// TestRun_WithoutVerifierNeverFetches proves an unconfigured release stays on
+// its reviewed embedded list and does not make a runtime trust request.
+func TestRun_WithoutVerifierNeverFetches(t *testing.T) {
+	setVerifierForTest(t, make(ed25519.PublicKey, ed25519.PublicKeySize))
+	var clients atomic.Int32
+	previous := httpClientForRun
+	httpClientForRun = func() *http.Client {
+		clients.Add(1)
+		return &http.Client{Transport: &errTransport{err: errors.New("must not fetch")}}
+	}
+	t.Cleanup(func() { httpClientForRun = previous })
+
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+	go func() {
+		Run(ctx)
+		close(done)
+	}()
+	time.Sleep(25 * time.Millisecond)
+	cancel()
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("Run did not return after cancellation")
+	}
+	if got := clients.Load(); got != 0 {
+		t.Fatalf("http clients created = %d, want 0", got)
+	}
+}
+
 // TestRun_FetchPath drives Run through the timer.C -> fetchOnce ->
 // timer.Reset arm. We can't shrink fetchInterval (const), so we can't
 // drive a second iteration in test time. But the first iteration —