🛡️ Sentinel: Fix signature verification bypass for Kafka, Mongo, and Redis proxy actions

.jules/sentinel.md

@@ -2,3 +2,9 @@
 **Vulnerability:** TOCTOU race condition in `pkg/signing/verify.go` allowed concurrent duplicate requests with identical nonces to bypass replay protection because `isReplayedNonce` checked nonces before `recordNonce` was called at the end of message verification.
 **Learning:** Checking nonces separately from recording them leaves a race condition window under concurrent load, and checking nonces before signature verification allows unauthenticated requests to pollute or query nonce tracking.
 **Prevention:** Perform cryptographic signature verification first, followed by an atomic check-and-record operation for nonces under mutex lock.
+
+## 2026-08-13 - Fail-Closed Signature Verification for Relay Messages
+**Vulnerability:** Newly added proxy actions (e.g. Kafka actions, Mongo status/stats, Redis info/slowlog/client list) were missing from an explicit opt-in `signedActions` map in `pkg/ws/handler.go`, allowing unsigned messages for those actions to silently bypass signature verification even when message signing was enabled.
+**Learning:** Opt-in authorization/verification maps are fail-open security antipatterns. When new proxy modules or sub-actions are added, forgetting to register them in an explicit map leaves unauthenticated execution vectors.
+**Prevention:** Enforce signature verification uniformly for all incoming control plane messages in `HandleMessage` (fail-closed, secure by default) when signature verification is enabled, eliminating manual opt-in map maintenance.
+

go.mod

@@ -1,6 +1,6 @@
 module nudgebee/forager
 
-go 1.25.12
+go 1.25.13
 
 require (
 	cloud.google.com/go/auth v0.22.0

pkg/signing/sign.go

@@ -91,6 +91,9 @@ var SigningFields = map[string][]string{
 	// Config sync: what datasources are being configured
 	"datasource_config_sync": {"action", "account_id", "datasources"},
 
+	// Config test: temporary datasource configuration and credentials
+	"test_datasource_config": {"action", "datasource"},
+
 	// Action requests (new format): what action on which datasource with what params
 	"db_query":    {"action", "datasource_id", "params"},
 	"db_execute":  {"action", "datasource_id", "params"},
@@ -131,21 +134,31 @@ func (s *Signer) Sign(msg []byte) ([]byte, error) {
 	if actionRaw, ok := raw["action"]; ok {
 		_ = json.Unmarshal(actionRaw, &action)
 	}
-	// For legacy format, try body.action_name
-	if action == "" {
-		if bodyRaw, ok := raw["body"]; ok {
-			var body map[string]json.RawMessage
-			if json.Unmarshal(bodyRaw, &body) == nil {
-				if actionNameRaw, ok := body["action_name"]; ok {
-					_ = json.Unmarshal(actionNameRaw, &action)
-				}
+	// For legacy format, check body.action_name
+	isLegacyAction := false
+	if bodyRaw, ok := raw["body"]; ok {
+		var body struct {
+			ActionName string `json:"action_name"`
+		}
+		if json.Unmarshal(bodyRaw, &body) == nil && body.ActionName != "" {
+			isLegacyAction = true
+			if action == "" {
+				action = body.ActionName
 			}
 		}
 	}
 
 	fields := DefaultSigningFields
-	if f, ok := SigningFields[action]; ok {
+	if isLegacyAction {
+		// Legacy action request format: fields live inside top-level body object
+		fields = []string{"body"}
+	} else if f, ok := SigningFields[action]; ok {
 		fields = f
+	} else if action == "" {
+		// Legacy HTTP proxy request format (no action field, has url)
+		if _, ok := raw["url"]; ok {
+			fields = []string{"method", "url", "header", "body"}
+		}
 	}
 
 	// Extract the fields to sign

pkg/signing/verify_test.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -446,3 +447,104 @@ func TestVerify_ConcurrentReplay(t *testing.T) {
 		t.Fatalf("expected %d replay rejections, got %d", goroutines-1, replayCount)
 	}
 }
+
+func TestSignAndVerify_TestDatasourceConfig_AntiTamper(t *testing.T) {
+	pub, priv := generateTestKeypair()
+	v, err := NewVerifier(base64.StdEncoding.EncodeToString(pub), testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+
+	signer := testSigner(t, priv)
+	msg := []byte(`{"action":"test_datasource_config","datasource":{"type":"postgresql","config":{"host":"localhost"}}}`)
+	signed, err := signer.Sign(msg)
+	if err != nil {
+		t.Fatalf("Sign: %v", err)
+	}
+
+	if err := v.Verify(signed); err != nil {
+		t.Fatalf("Verify valid test_datasource_config failed: %v", err)
+	}
+
+	// Tamper with datasource config payload
+	tampered := strings.Replace(string(signed), "localhost", "evil.com", 1)
+	if err := v.Verify([]byte(tampered)); err == nil {
+		t.Fatal("expected verification error for tampered test_datasource_config host")
+	}
+}
+
+func TestSignAndVerify_LegacyHTTP_AntiTamper(t *testing.T) {
+	pub, priv := generateTestKeypair()
+	v, err := NewVerifier(base64.StdEncoding.EncodeToString(pub), testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+
+	signer := testSigner(t, priv)
+	msg := []byte(`{"method":"GET","url":"/api/v1/metrics","header":{},"body":""}`)
+	signed, err := signer.Sign(msg)
+	if err != nil {
+		t.Fatalf("Sign: %v", err)
+	}
+
+	if err := v.Verify(signed); err != nil {
+		t.Fatalf("Verify valid legacy HTTP request failed: %v", err)
+	}
+
+	// Tamper with URL
+	tampered := strings.Replace(string(signed), "/api/v1/metrics", "/api/v1/admin/delete", 1)
+	if err := v.Verify([]byte(tampered)); err == nil {
+		t.Fatal("expected verification error for tampered legacy HTTP request URL")
+	}
+}
+
+func TestSignAndVerify_LegacyAction_AntiTamper(t *testing.T) {
+	pub, priv := generateTestKeypair()
+	v, err := NewVerifier(base64.StdEncoding.EncodeToString(pub), testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+
+	signer := testSigner(t, priv)
+	msg := []byte(`{"body":{"action_name":"db_query","action_params":{"datasource_id":"ds-1"}}}`)
+	signed, err := signer.Sign(msg)
+	if err != nil {
+		t.Fatalf("Sign: %v", err)
+	}
+
+	if err := v.Verify(signed); err != nil {
+		t.Fatalf("Verify valid legacy action request failed: %v", err)
+	}
+
+	// Tamper with action_params
+	tampered := strings.Replace(string(signed), "ds-1", "ds-2", 1)
+	if err := v.Verify([]byte(tampered)); err == nil {
+		t.Fatal("expected verification error for tampered legacy action request params")
+	}
+}
+
+func TestSignAndVerify_LegacyAction_EmptyActionField_AntiTamper(t *testing.T) {
+	pub, priv := generateTestKeypair()
+	v, err := NewVerifier(base64.StdEncoding.EncodeToString(pub), testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+
+	signer := testSigner(t, priv)
+	// Message with empty action field but legacy body format
+	msg := []byte(`{"action":"","body":{"action_name":"db_query","action_params":{"datasource_id":"ds-1"}}}`)
+	signed, err := signer.Sign(msg)
+	if err != nil {
+		t.Fatalf("Sign: %v", err)
+	}
+
+	if err := v.Verify(signed); err != nil {
+		t.Fatalf("Verify valid legacy action request with empty action failed: %v", err)
+	}
+
+	// Tamper with action_params
+	tampered := strings.Replace(string(signed), "ds-1", "ds-2", 1)
+	if err := v.Verify([]byte(tampered)); err == nil {
+		t.Fatal("expected verification error for tampered legacy action request params when action field is empty")
+	}
+}

pkg/ws/discovery_wire_test.go

@@ -166,17 +166,35 @@ func (r *recordingProxy) Close() error                      { return nil }
 // This pins them, so adding a fourth without registering it fails here rather
 // than in production.
 func TestEveryDiscoveryActionRequiresSignature(t *testing.T) {
-	// Mirrors the actions discovery.Proxy.HandleRequest dispatches on.
+	dummyKey := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAG5e/k5wQ5l5X+5b5W5d5e5f5g5h5i5j5k5l5m5n5o5 test@test"
+	verifier, err := signing.NewVerifier(dummyKey, testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+	h := &Handler{registry: proxy.NewRegistry(), verifier: verifier, logger: testLogger()}
+
 	for _, action := range []string{"discovery_sweep", "discovery_ldap", "discovery_inventory"} {
-		if !signedActions[action] {
-			t.Errorf("%s is not in signedActions — it would bypass signature verification", action)
+		msg, _ := json.Marshal(map[string]any{"action": action, "request_id": "req-1"})
+		resp, err := h.HandleMessage(context.Background(), msg)
+		if err != nil {
+			t.Fatalf("HandleMessage failed: %v", err)
+		}
+		var r proxy.ActionResponse
+		_ = json.Unmarshal(resp, &r)
+		if r.StatusCode != 403 {
+			t.Errorf("%s expected 403 for unsigned message, got %d", action, r.StatusCode)
 		}
 	}
 }
 
-// The allowlist's default is the hazard, so state the invariant that matters:
-// anything that reaches a host or a network must be signed.
 func TestActionsThatTouchRemoteSystemsAreSigned(t *testing.T) {
+	dummyKey := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAG5e/k5wQ5l5X+5b5W5d5e5f5g5h5i5j5k5l5m5n5o5 test@test"
+	verifier, err := signing.NewVerifier(dummyKey, testLogger())
+	if err != nil {
+		t.Fatalf("NewVerifier: %v", err)
+	}
+	h := &Handler{registry: proxy.NewRegistry(), verifier: verifier, logger: testLogger()}
+
 	mustBeSigned := []string{
 		"ssh_command", "ssh_upload", "ssh_download", "ssh_list_dir",
 		"db_query", "db_execute",
@@ -185,8 +203,15 @@ func TestActionsThatTouchRemoteSystemsAreSigned(t *testing.T) {
 		"datasource_config_sync", "test_datasource_config",
 	}
 	for _, action := range mustBeSigned {
-		if !signedActions[action] {
-			t.Errorf("%s executes against a remote system but is not signed", action)
+		msg, _ := json.Marshal(map[string]any{"action": action, "request_id": "req-1"})
+		resp, err := h.HandleMessage(context.Background(), msg)
+		if err != nil {
+			t.Fatalf("HandleMessage failed: %v", err)
+		}
+		var r proxy.ActionResponse
+		_ = json.Unmarshal(resp, &r)
+		if r.StatusCode != 403 {
+			t.Errorf("%s expected 403 for unsigned message, got %d", action, r.StatusCode)
 		}
 	}
 }

pkg/ws/handler.go

@@ -20,53 +20,6 @@ import (
 	"nudgebee/forager/pkg/signing"
 )
 
-// signedActions are actions that require signature verification when signing is enabled.
-// All actions that can modify state or execute commands should be listed here.
-var signedActions = map[string]bool{
-	// Config sync — can push arbitrary datasources including RCE via MCP stdio
-	"datasource_config_sync": true,
-
-	// Database — arbitrary SQL execution
-	"db_query":    true,
-	"db_execute":  true,
-	"db_metadata": true,
-
-	// SSH — arbitrary command execution, file read/write
-	"ssh_command":  true,
-	"ssh_upload":   true,
-	"ssh_download": true,
-	"ssh_list_dir": true,
-
-	// HTTP — SSRF, credential theft via redirect
-	"http_request": true,
-
-	// MCP — arbitrary JSON-RPC to local processes
-	"mcp_request": true,
-
-	// MongoDB — arbitrary queries/aggregations
-	"mongo_query":     true,
-	"mongo_aggregate": true,
-
-	// Redis — arbitrary command execution
-	"redis_command": true,
-
-	// Config test — creates temporary proxy to test connectivity
-	"test_datasource_config": true,
-
-	// Discovery — inventory executes commands on remote hosts over SSH, the
-	// same capability as ssh_command above. The commands themselves come
-	// from a signature-verified content pack, but the targets do not: an
-	// unsigned action lets a caller choose which hosts we connect to.
-	//
-	// Sweep sends probes across a network segment. Its scope is bounded by
-	// allowed_cidrs, but triggering one is not harmless — an unexpected scan
-	// reads as an attack originating from our agent, and some IDS and
-	// fail2ban configurations act on it.
-	"discovery_sweep":     true,
-	"discovery_ldap":      true,
-	"discovery_inventory": true,
-}
-
 // Handler dispatches incoming relay messages to the appropriate proxy module.
 type Handler struct {
 	registry   *proxy.Registry
@@ -91,45 +44,37 @@ func NewHandler(registry *proxy.Registry, credStore *secrets.CloudPushStore, sec
 // All requests use a unified format: {request_id, datasource_id, action, params, ...}
 func (h *Handler) HandleMessage(ctx context.Context, msg []byte) ([]byte, error) {
 	var envelope struct {
-		Action       string `json:"action"`
-		RequestID    string `json:"request_id"`
-		DatasourceID string `json:"datasource_id"`
-		Body         struct {
-			ActionName string `json:"action_name"`
-		} `json:"body"`
+		Action       string          `json:"action"`
+		RequestID    string          `json:"request_id"`
+		DatasourceID string          `json:"datasource_id"`
+		Body         json.RawMessage `json:"body"`
 	}
 	if err := json.Unmarshal(msg, &envelope); err != nil {
 		return nil, fmt.Errorf("unmarshal envelope: %w", err)
 	}
 
-	// Resolve the effective action — legacy messages use body.action_name
+	// Resolve the effective action — legacy action messages use body.action_name
 	effectiveAction := envelope.Action
-	if effectiveAction == "" {
-		effectiveAction = envelope.Body.ActionName
+	if effectiveAction == "" && len(envelope.Body) > 0 {
+		var body struct {
+			ActionName string `json:"action_name"`
+		}
+		_ = json.Unmarshal(envelope.Body, &body)
+		effectiveAction = body.ActionName
 	}
 
-	// Verify signature for actions that require it
-	if signedActions[effectiveAction] {
-		if err := h.verifier.Verify(msg); err != nil {
-			h.logger.Error("message signature verification failed",
-				"action", effectiveAction,
-				"request_id", envelope.RequestID,
-				"err", err,
-			)
-			if h.verifier.Enabled() {
-				return h.buildErrorResponse(envelope.RequestID, 403, "signature verification failed"), nil
-			}
-		}
+	// Verify signature for all incoming messages (fail-closed, secure by default)
+	if h.verifier == nil {
+		h.logger.Error("verifier is not initialized", "request_id", envelope.RequestID)
+		return h.buildErrorResponse(envelope.RequestID, 500, "internal server error: verifier not initialized"), nil
 	}
-	// Legacy HTTP proxy requests (no action field) also require verification when signing is enabled
-	if effectiveAction == "" && h.verifier.Enabled() {
-		if err := h.verifier.Verify(msg); err != nil {
-			h.logger.Error("unsigned legacy HTTP request rejected",
-				"request_id", envelope.RequestID,
-				"err", err,
-			)
-			return h.buildErrorResponse(envelope.RequestID, 403, "signature verification failed"), nil
-		}
+	if err := h.verifier.Verify(msg); err != nil {
+		h.logger.Error("message signature verification failed",
+			"action", effectiveAction,
+			"request_id", envelope.RequestID,
+			"err", err,
+		)
+		return h.buildErrorResponse(envelope.RequestID, 403, "signature verification failed"), nil
 	}
 
 	switch envelope.Action {