node: report the verdict of the key that signed the token, not the last key tried

With two trusted control-plane keys, the node tried each in turn and
returned the last error. A token that verified under the first key and was
denied by policy then came back as "biscuit: invalid signature" from the
second key, so the caller never saw the denial. Which error won depended
on key order, which made the policy e2e Negative and Attenuation paths
flaky.

Only a signature mismatch now moves on to the next key; once a key
verifies the token, its verdict is the answer. A trusted key of the wrong
size is skipped with a warning instead of ending the search.

Author: saurav-shakya

internal/node/middleware.go

@@ -16,6 +16,7 @@ package node
 
 import (
 	"crypto/ed25519"
+	"errors"
 	"fmt"
 	"runtime/debug"
 	"sync/atomic"
@@ -223,28 +224,30 @@ func (n *AgentMeshNode) verifyBiscuitTokenWithRules(biscuitBytes []byte, reqCtx
 	keys := n.trustedKeys
 	n.keysMu.RUnlock()
 
-	var authorized bool
-	var taskRules []*api.TaskAuthorizationRule
+	// Only a signature mismatch moves on to the next trusted key. Once a key
+	// verifies the token, that key's verdict is the answer: carrying on would
+	// replace a policy denial with the next key's "invalid signature".
 	var lastErr error
 	for _, pubKey := range keys {
+		if len(pubKey.Key) != ed25519.PublicKeySize {
+			logger.Warnf("[Auth] Skipping trusted key of invalid size %d", len(pubKey.Key))
+			continue
+		}
 		logger.Infof("[Auth] Trying key: %x", pubKey.Key)
-		if rules, err := n.authorizeWithRules(biscuitBytes, reqCtx, pubKey.Key); err == nil {
-			authorized = true
-			taskRules = rules
-			break
-		} else {
-			lastErr = err
+		rules, err := n.authorizeWithRules(biscuitBytes, reqCtx, pubKey.Key)
+		if err == nil {
+			return rules, nil
 		}
-	}
-
-	if !authorized {
-		if lastErr != nil {
-			return nil, lastErr
+		if !errors.Is(err, biscuit.ErrInvalidSignature) {
+			return nil, err
 		}
-		return nil, fmt.Errorf("authorization failed")
+		lastErr = err
 	}
 
-	return taskRules, nil
+	if lastErr != nil {
+		return nil, lastErr
+	}
+	return nil, fmt.Errorf("authorization failed")
 }
 
 func (n *AgentMeshNode) Authorize(rawToken []byte, req RequestContext, pubKey ed25519.PublicKey) error {

internal/node/middleware_test.go

@@ -21,6 +21,7 @@ import (
 	cryptorand "crypto/rand"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"io"
@@ -253,6 +254,83 @@ func TestAuthorizeRejectsExpiredBiscuit(t *testing.T) {
 	}
 }
 
+// TestVerifyBiscuitTokenReportsVerdictOfSigningKey reproduces the flaky
+// policy e2e: with two trusted control-plane keys, a token whose signature
+// verifies under one key and is denied by policy was reported as
+// "invalid signature" when the other key happened to be tried last, hiding
+// the denial from the caller.
+func TestVerifyBiscuitTokenReportsVerdictOfSigningKey(t *testing.T) {
+	store, err := NewStore(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = store.Close() })
+
+	signingPub, signingPriv, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	otherPub, _, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	caller := peer.ID("caller-peer")
+	builder := biscuit.NewBuilder(signingPriv)
+	_ = builder.AddAuthorityFact(api.MarkerFact(api.FactTargetUnrestricted))
+	_ = builder.AddAuthorityFact(biscuit.Fact{Predicate: biscuit.Predicate{
+		Name: "node",
+		IDs:  []biscuit.Term{biscuit.String(caller.String())},
+	}})
+	_ = builder.AddAuthorityFact(biscuit.Fact{Predicate: biscuit.Predicate{
+		Name: api.FactClientPeerID,
+		IDs:  []biscuit.Term{biscuit.String(caller.String())},
+	}})
+	_ = builder.AddAuthorityFact(biscuit.Fact{Predicate: biscuit.Predicate{
+		Name: api.FactGrantedServiceExact,
+		IDs:  []biscuit.Term{biscuit.String("mcp"), biscuit.String("calculator")},
+	}})
+	b, err := builder.Build()
+	if err != nil {
+		t.Fatal(err)
+	}
+	tokenBytes, err := b.Serialize()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// The token grants mcp://calculator only, so this request is denied by
+	// policy after its signature verified under signingPub.
+	req := RequestContext{
+		PeerID:   caller,
+		Protocol: "/mesh/mcp/1.0.0",
+		Target:   "mcp://unauthorized-service",
+	}
+
+	for _, tt := range []struct {
+		name string
+		keys []TrustedKey
+	}{
+		{"signing key first", []TrustedKey{{Key: signingPub}, {Key: otherPub}}},
+		{"signing key last", []TrustedKey{{Key: otherPub}, {Key: signingPub}}},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			node := &AgentMeshNode{
+				Store:          store,
+				trustedKeys:    tt.keys,
+				BiscuitTimeout: 500 * time.Millisecond,
+			}
+			err := node.VerifyBiscuitToken(tokenBytes, req)
+			if err == nil {
+				t.Fatal("VerifyBiscuitToken allowed a target the token does not grant")
+			}
+			if errors.Is(err, biscuit.ErrInvalidSignature) {
+				t.Fatalf("VerifyBiscuitToken = %v, want the policy denial from the key that signed the token", err)
+			}
+		})
+	}
+}
+
 // TestAuthorizeRejectsAppendedPeerBinding covers the dataplane half of the
 // binding bypass. Appending a block needs no root key, so a peer holding any
 // valid token could append node(<its own peer id>) and connect under its own