From 6a8ce8b341d8bf7b7c8ba64566b71f8f452cac06 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 10:54:58 +0530 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20Fix?= =?UTF-8?q?=20signature=20verification=20bypass=20for=20Kafka,=20Mongo,=20?= =?UTF-8?q?and=20Redis=20proxy=20actions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 6 ++++++ pkg/ws/handler.go | 47 ++++++++++++++++++++++++++++-------------- pkg/ws/handler_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e9c373c..25bd432 100644 --- a/.jules/sentinel.md +++ b/.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 - Comprehensive Registration of Actions in `signedActions` Map +**Vulnerability:** Newly added proxy actions (e.g. Kafka, Mongo status/stats, Redis info/slowlog/client list) were missing from the central `signedActions` map in `pkg/ws/handler.go`, allowing unsigned messages for those actions to bypass signature verification even when message signing was enabled. +**Learning:** `signedActions` used an explicit opt-in map rather than a default-signed approach, so adding new proxy packages or actions without updating `signedActions` silently creates unsigned execution paths. +**Prevention:** Always register every action supported by any proxy module in `signedActions` (or add unit tests asserting all proxy actions are present in `signedActions`). + diff --git a/pkg/ws/handler.go b/pkg/ws/handler.go index e657058..7912286 100644 --- a/pkg/ws/handler.go +++ b/pkg/ws/handler.go @@ -21,7 +21,7 @@ import ( ) // signedActions are actions that require signature verification when signing is enabled. -// All actions that can modify state or execute commands should be listed here. +// All actions that can modify state, query data, 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, @@ -43,25 +43,40 @@ var signedActions = map[string]bool{ // 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, + // MongoDB — queries, aggregations, server/db status and collection info + "mongo_query": true, + "mongo_aggregate": true, + "mongo_server_status": true, + "mongo_repl_status": true, + "mongo_collection_stats": true, + "mongo_current_ops": true, + "mongo_db_stats": true, + "mongo_list_databases": true, + "mongo_list_collections": true, + + // Redis — commands, info, slowlog, client list, memory stats + "redis_command": true, + "redis_info": true, + "redis_info_section": true, + "redis_slowlog": true, + "redis_client_list": true, + "redis_memory_stats": true, + "redis_cluster_info": true, + "redis_keyspace_stats": true, + + // Kafka — lag, groups, topics, brokers, offsets + "kafka_consumer_lag": true, + "kafka_consumer_groups": true, + "kafka_consumer_group_describe": true, + "kafka_topics": true, + "kafka_topic_describe": true, + "kafka_brokers": true, + "kafka_topic_offsets": 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 — inventory, sweep, ldap "discovery_sweep": true, "discovery_ldap": true, "discovery_inventory": true, diff --git a/pkg/ws/handler_test.go b/pkg/ws/handler_test.go index 6c7cd43..db140d3 100644 --- a/pkg/ws/handler_test.go +++ b/pkg/ws/handler_test.go @@ -303,3 +303,49 @@ func TestHandler_BuildErrorResponse(t *testing.T) { t.Fatalf("expected req-123, got %s", r.RequestID) } } + +func TestHandler_SignedActionsEnforcement(t *testing.T) { + // Create verifier with a dummy public key to enable signature verification + 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(), + } + + actionsToTest := []string{ + "kafka_consumer_groups", + "kafka_topics", + "mongo_server_status", + "mongo_list_databases", + "redis_info", + "redis_client_list", + } + + for _, action := range actionsToTest { + msg := map[string]any{ + "action": action, + "request_id": "req-sig-test", + } + data, _ := json.Marshal(msg) + + resp, err := h.HandleMessage(context.Background(), data) + if err != nil { + t.Fatalf("HandleMessage for action %s failed: %v", action, err) + } + + var r proxy.ActionResponse + if err := json.Unmarshal(resp, &r); err != nil { + t.Fatalf("unmarshal response for action %s: %v", action, err) + } + + if r.StatusCode != 403 { + t.Errorf("action %s expected 403 for unsigned message, got %d", action, r.StatusCode) + } + } +} From 82faf9917d6bfbe372c18f35006c930d50ec6ff8 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 10:58:54 +0530 Subject: [PATCH 02/11] test(ws): dynamically iterate over signedActions in TestHandler_SignedActionsEnforcement --- pkg/ws/handler_test.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/pkg/ws/handler_test.go b/pkg/ws/handler_test.go index db140d3..3b39b3f 100644 --- a/pkg/ws/handler_test.go +++ b/pkg/ws/handler_test.go @@ -318,16 +318,7 @@ func TestHandler_SignedActionsEnforcement(t *testing.T) { logger: testLogger(), } - actionsToTest := []string{ - "kafka_consumer_groups", - "kafka_topics", - "mongo_server_status", - "mongo_list_databases", - "redis_info", - "redis_client_list", - } - - for _, action := range actionsToTest { + for action := range signedActions { msg := map[string]any{ "action": action, "request_id": "req-sig-test", From 775941811687d849275a575a362755f1b1437113 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 21:02:52 +0530 Subject: [PATCH 03/11] refactor(ws): enforce fail-closed signature verification for all incoming messages --- .jules/sentinel.md | 8 ++-- pkg/ws/discovery_wire_test.go | 39 ++++++++++++--- pkg/ws/handler.go | 90 ++++------------------------------- pkg/ws/handler_test.go | 17 ++++++- 4 files changed, 59 insertions(+), 95 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 25bd432..6a3c10f 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -3,8 +3,8 @@ **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 - Comprehensive Registration of Actions in `signedActions` Map -**Vulnerability:** Newly added proxy actions (e.g. Kafka, Mongo status/stats, Redis info/slowlog/client list) were missing from the central `signedActions` map in `pkg/ws/handler.go`, allowing unsigned messages for those actions to bypass signature verification even when message signing was enabled. -**Learning:** `signedActions` used an explicit opt-in map rather than a default-signed approach, so adding new proxy packages or actions without updating `signedActions` silently creates unsigned execution paths. -**Prevention:** Always register every action supported by any proxy module in `signedActions` (or add unit tests asserting all proxy actions are present in `signedActions`). +## 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. diff --git a/pkg/ws/discovery_wire_test.go b/pkg/ws/discovery_wire_test.go index 6cc3cef..0628c22 100644 --- a/pkg/ws/discovery_wire_test.go +++ b/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) } } } diff --git a/pkg/ws/handler.go b/pkg/ws/handler.go index 7912286..32732cc 100644 --- a/pkg/ws/handler.go +++ b/pkg/ws/handler.go @@ -20,68 +20,6 @@ import ( "nudgebee/forager/pkg/signing" ) -// signedActions are actions that require signature verification when signing is enabled. -// All actions that can modify state, query data, 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 — queries, aggregations, server/db status and collection info - "mongo_query": true, - "mongo_aggregate": true, - "mongo_server_status": true, - "mongo_repl_status": true, - "mongo_collection_stats": true, - "mongo_current_ops": true, - "mongo_db_stats": true, - "mongo_list_databases": true, - "mongo_list_collections": true, - - // Redis — commands, info, slowlog, client list, memory stats - "redis_command": true, - "redis_info": true, - "redis_info_section": true, - "redis_slowlog": true, - "redis_client_list": true, - "redis_memory_stats": true, - "redis_cluster_info": true, - "redis_keyspace_stats": true, - - // Kafka — lag, groups, topics, brokers, offsets - "kafka_consumer_lag": true, - "kafka_consumer_groups": true, - "kafka_consumer_group_describe": true, - "kafka_topics": true, - "kafka_topic_describe": true, - "kafka_brokers": true, - "kafka_topic_offsets": true, - - // Config test — creates temporary proxy to test connectivity - "test_datasource_config": true, - - // Discovery — inventory, sweep, ldap - "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 @@ -123,26 +61,14 @@ func (h *Handler) HandleMessage(ctx context.Context, msg []byte) ([]byte, error) effectiveAction = envelope.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 - } - } - } - // 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, - ) + // Verify signature for all incoming messages (fail-closed, secure by default) + 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 } } diff --git a/pkg/ws/handler_test.go b/pkg/ws/handler_test.go index 3b39b3f..fec9949 100644 --- a/pkg/ws/handler_test.go +++ b/pkg/ws/handler_test.go @@ -304,7 +304,7 @@ func TestHandler_BuildErrorResponse(t *testing.T) { } } -func TestHandler_SignedActionsEnforcement(t *testing.T) { +func TestHandler_SignatureEnforcement_FailClosed(t *testing.T) { // Create verifier with a dummy public key to enable signature verification dummyKey := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAG5e/k5wQ5l5X+5b5W5d5e5f5g5h5i5j5k5l5m5n5o5 test@test" verifier, err := signing.NewVerifier(dummyKey, testLogger()) @@ -318,7 +318,20 @@ func TestHandler_SignedActionsEnforcement(t *testing.T) { logger: testLogger(), } - for action := range signedActions { + // Verify signature enforcement for standard actions as well as unknown/unregistered future actions (fail-closed) + actionsToTest := []string{ + "db_query", + "ssh_command", + "http_request", + "mcp_request", + "kafka_consumer_groups", + "mongo_server_status", + "redis_info", + "unknown_action", + "future_proxy_action", + } + + for _, action := range actionsToTest { msg := map[string]any{ "action": action, "request_id": "req-sig-test", From fd251b9582497ed66cfb44f5669c314921f7d591 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 21:26:33 +0530 Subject: [PATCH 04/11] refactor(ws): remove redundant verifier.Enabled() check in HandleMessage --- pkg/ws/handler.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/ws/handler.go b/pkg/ws/handler.go index 32732cc..e1fb853 100644 --- a/pkg/ws/handler.go +++ b/pkg/ws/handler.go @@ -68,9 +68,7 @@ func (h *Handler) HandleMessage(ctx context.Context, msg []byte) ([]byte, error) "request_id", envelope.RequestID, "err", err, ) - if h.verifier.Enabled() { - return h.buildErrorResponse(envelope.RequestID, 403, "signature verification failed"), nil - } + return h.buildErrorResponse(envelope.RequestID, 403, "signature verification failed"), nil } switch envelope.Action { From 654455463c4f5991719db5581bea749e2f367e3d Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 21:51:28 +0530 Subject: [PATCH 05/11] fix(signing): include test_datasource_config and legacy HTTP fields in SigningFields --- pkg/signing/sign.go | 8 ++++++ pkg/signing/verify_test.go | 51 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/pkg/signing/sign.go b/pkg/signing/sign.go index d2fcf49..0638b31 100644 --- a/pkg/signing/sign.go +++ b/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"}, @@ -146,6 +149,11 @@ func (s *Signer) Sign(msg []byte) ([]byte, error) { fields := DefaultSigningFields 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 diff --git a/pkg/signing/verify_test.go b/pkg/signing/verify_test.go index 46eec59..5ad0232 100644 --- a/pkg/signing/verify_test.go +++ b/pkg/signing/verify_test.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "time" ) @@ -446,3 +447,53 @@ 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") + } +} From f693c853985d57647f66c5a6e01acb2ba5152cbc Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 22:51:37 +0530 Subject: [PATCH 06/11] test(signing): add TestSignAndVerify_LegacyAction_AntiTamper --- pkg/signing/verify_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/signing/verify_test.go b/pkg/signing/verify_test.go index 5ad0232..9731537 100644 --- a/pkg/signing/verify_test.go +++ b/pkg/signing/verify_test.go @@ -497,3 +497,28 @@ func TestSignAndVerify_LegacyHTTP_AntiTamper(t *testing.T) { 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") + } +} From 173fe17274f0dae65c2fdd180ac27c98d98f1495 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 22:57:18 +0530 Subject: [PATCH 07/11] fix(ws): add defensive nil check for verifier in HandleMessage --- pkg/ws/handler.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/ws/handler.go b/pkg/ws/handler.go index e1fb853..90f0767 100644 --- a/pkg/ws/handler.go +++ b/pkg/ws/handler.go @@ -62,6 +62,10 @@ func (h *Handler) HandleMessage(ctx context.Context, msg []byte) ([]byte, error) } // 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 + } if err := h.verifier.Verify(msg); err != nil { h.logger.Error("message signature verification failed", "action", effectiveAction, From d880523229f00272eb2a6bc260865533b8f1e01d Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 13 Aug 2026 23:27:59 +0530 Subject: [PATCH 08/11] fix(signing): sign body object for legacy action requests to prevent payload tampering --- pkg/signing/sign.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/signing/sign.go b/pkg/signing/sign.go index 0638b31..f6a309d 100644 --- a/pkg/signing/sign.go +++ b/pkg/signing/sign.go @@ -147,7 +147,10 @@ func (s *Signer) Sign(msg []byte) ([]byte, error) { } fields := DefaultSigningFields - if f, ok := SigningFields[action]; ok { + if _, ok := raw["action"]; !ok && action != "" { + // 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) From 177cb5861e098cf02496a985e70ebef09fcfd09d Mon Sep 17 00:00:00 2001 From: shiv Date: Fri, 14 Aug 2026 00:44:43 +0530 Subject: [PATCH 09/11] fix(signing): robustly detect legacy action requests using body.action_name --- pkg/signing/sign.go | 20 +++++++++++--------- pkg/signing/verify_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/pkg/signing/sign.go b/pkg/signing/sign.go index f6a309d..b38b3d5 100644 --- a/pkg/signing/sign.go +++ b/pkg/signing/sign.go @@ -134,20 +134,22 @@ 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 _, ok := raw["action"]; !ok && action != "" { + if isLegacyAction { // Legacy action request format: fields live inside top-level body object fields = []string{"body"} } else if f, ok := SigningFields[action]; ok { diff --git a/pkg/signing/verify_test.go b/pkg/signing/verify_test.go index 9731537..4edb70d 100644 --- a/pkg/signing/verify_test.go +++ b/pkg/signing/verify_test.go @@ -522,3 +522,29 @@ func TestSignAndVerify_LegacyAction_AntiTamper(t *testing.T) { 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") + } +} From b4f8ee1f43d1c7d88a1b2c28c3d74d480199c2be Mon Sep 17 00:00:00 2001 From: shiv Date: Fri, 14 Aug 2026 20:26:43 +0530 Subject: [PATCH 10/11] fix(ws): use json.RawMessage for envelope.Body to support string bodies in legacy HTTP requests --- pkg/ws/handler.go | 20 +++++++++++--------- pkg/ws/handler_test.go | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/ws/handler.go b/pkg/ws/handler.go index 90f0767..b01b036 100644 --- a/pkg/ws/handler.go +++ b/pkg/ws/handler.go @@ -44,21 +44,23 @@ 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 all incoming messages (fail-closed, secure by default) diff --git a/pkg/ws/handler_test.go b/pkg/ws/handler_test.go index fec9949..251dc72 100644 --- a/pkg/ws/handler_test.go +++ b/pkg/ws/handler_test.go @@ -128,6 +128,21 @@ func TestHandler_HandleMessage_HTTPRequest_NoProxy(t *testing.T) { } } +func TestHandler_HandleMessage_HTTPRequest_StringBody(t *testing.T) { + h := newTestHandler(t) + msg := `{"method": "POST", "url": "/api/v1/metrics", "request_id": "req-str-body", "header": {}, "body": "raw-string-body"}` + resp, err := h.HandleMessage(context.Background(), []byte(msg)) + if err != nil { + t.Fatalf("HandleMessage failed for HTTP request with string body: %v", err) + } + + var r proxy.ActionResponse + _ = json.Unmarshal(resp, &r) + if r.StatusCode != 404 { + t.Fatalf("expected 404 for no http-proxy, got %d", r.StatusCode) + } +} + func TestHandler_ConfigSync_HTTPProxy(t *testing.T) { h := newTestHandler(t) From a9d644d116d9baa9887bfed77577c3919bf6247f Mon Sep 17 00:00:00 2001 From: shiv Date: Fri, 14 Aug 2026 20:34:37 +0530 Subject: [PATCH 11/11] build(deps): bump go version in go.mod to 1.25.13 to resolve stdlib govulncheck findings --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 683fad8..d73015c 100644 --- a/go.mod +++ b/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