From 0a289974a5b95d6c41163386d061247357343716 Mon Sep 17 00:00:00 2001
From: matthew-pilot <matthew@vulturelabs.io>
Date: Sat, 30 May 2026 09:10:03 +0000
Subject: [PATCH] fix(registry/client): surface malformed and non-string error
 responses (PILOT-132)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The registry client's sendOnEntry, sendLocked, and sendJSONLocked
treated any valid-JSON response without an "error" key as success,
silently accepting garbage (e.g. {"unexpected":"key"}). Additionally,
only string-typed error values were detected — numeric or object
error values (e.g. {"error":403}) were silently swallowed.

Two changes in each of the three send functions:

1. Broaden error detection to handle any error value type
   (resp["error"].(string) → resp["error"] with %v formatting).

2. Add a "type" envelope field check: if a non-empty response lacks
   both "error" and "type", it is rejected as malformed. Empty
   responses remain valid to preserve backward compatibility with
   any edge case.

The rendezvous server always includes a "type" field in its
responses, so this check is a safe defense-in-depth measure.

Closes PILOT-132
---
 registry/client/binary_client.go         |  8 +++-
 registry/client/client.go                | 16 ++++++--
 registry/client/zz_client_branch_test.go | 51 ++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/registry/client/binary_client.go b/registry/client/binary_client.go
index 3268679..70e5bb6 100644
--- a/registry/client/binary_client.go
+++ b/registry/client/binary_client.go
@@ -271,8 +271,12 @@ func (c *BinaryClient) sendJSONLocked(msg map[string]interface{}) (map[string]in
 	if err := json.Unmarshal(payload, &resp); err != nil {
 		return nil, fmt.Errorf("json decode response: %w", err)
 	}
-	if errMsg, ok := resp["error"].(string); ok {
-		return resp, fmt.Errorf("registry: %s", errMsg)
+	if errVal, ok := resp["error"]; ok {
+		return resp, fmt.Errorf("registry: %v", errVal)
+	}
+	// PILOT-132: reject valid JSON that lacks the expected "type" envelope key.
+	if _, hasType := resp["type"]; !hasType && len(resp) > 0 {
+		return resp, fmt.Errorf("registry: malformed response (missing %q field)", "type")
 	}
 	return resp, nil
 }
diff --git a/registry/client/client.go b/registry/client/client.go
index 404b190..c24b163 100644
--- a/registry/client/client.go
+++ b/registry/client/client.go
@@ -442,8 +442,12 @@ func (c *Client) sendOnEntry(entry *pooledConn, msg map[string]interface{}) (map
 	if err != nil {
 		return nil, fmt.Errorf("recv: %w", err)
 	}
-	if errMsg, ok := resp["error"].(string); ok {
-		return resp, fmt.Errorf("registry: %s", errMsg)
+	if errVal, ok := resp["error"]; ok {
+		return resp, fmt.Errorf("registry: %v", errVal)
+	}
+	// PILOT-132: reject valid JSON that lacks the expected "type" envelope key.
+	if _, hasType := resp["type"]; !hasType && len(resp) > 0 {
+		return resp, fmt.Errorf("registry: malformed response (missing %q field)", "type")
 	}
 	return resp, nil
 }
@@ -514,8 +518,12 @@ func (c *Client) sendLocked(msg map[string]interface{}) (map[string]interface{},
 	if err != nil {
 		return nil, fmt.Errorf("recv: %w", err)
 	}
-	if errMsg, ok := resp["error"].(string); ok {
-		return resp, fmt.Errorf("registry: %s", errMsg)
+	if errVal, ok := resp["error"]; ok {
+		return resp, fmt.Errorf("registry: %v", errVal)
+	}
+	// PILOT-132: reject valid JSON that lacks the expected "type" envelope key.
+	if _, hasType := resp["type"]; !hasType && len(resp) > 0 {
+		return resp, fmt.Errorf("registry: malformed response (missing %q field)", "type")
 	}
 	return resp, nil
 }
diff --git a/registry/client/zz_client_branch_test.go b/registry/client/zz_client_branch_test.go
index 9d23714..b0dacde 100644
--- a/registry/client/zz_client_branch_test.go
+++ b/registry/client/zz_client_branch_test.go
@@ -416,6 +416,57 @@ func TestSendOnEntryReturnsServerErrorResponse(t *testing.T) {
 	}
 }
 
+// TestSendOnEntryReturnsErrorOnNonStringError verifies that a server error
+// value of a non-string type (e.g. int) is still treated as an error (PILOT-132).
+// Previously only resp["error"].(string) would trigger, silently swallowing
+// numeric or object error values.
+func TestSendOnEntryReturnsErrorOnNonStringError(t *testing.T) {
+	t.Parallel()
+	srv := newFakeJSONServer(t, func(_ map[string]interface{}) map[string]interface{} {
+		return map[string]interface{}{"error": float64(403)}
+	})
+	defer srv.close()
+
+	c, err := DialPool(srv.addr(), 2)
+	if err != nil {
+		t.Fatalf("DialPool: %v", err)
+	}
+	defer c.Close()
+
+	_, err = c.Send(map[string]interface{}{"type": "x"})
+	if err == nil {
+		t.Fatalf("expected error for non-string error value")
+	}
+	if !strings.Contains(err.Error(), "403") {
+		t.Fatalf("error should contain the numeric error value 403, got: %v", err)
+	}
+}
+
+// TestSendReturnsErrorOnMalformedResponse verifies that a valid-JSON response
+// that lacks both an "error" key and a "type" key is treated as a protocol
+// violation, not silently accepted (PILOT-132).
+func TestSendReturnsErrorOnMalformedResponse(t *testing.T) {
+	t.Parallel()
+	srv := newFakeJSONServer(t, func(_ map[string]interface{}) map[string]interface{} {
+		return map[string]interface{}{"unexpected": "key"}
+	})
+	defer srv.close()
+
+	c, err := Dial(srv.addr())
+	if err != nil {
+		t.Fatalf("dial: %v", err)
+	}
+	defer c.Close()
+
+	_, err = c.Send(map[string]interface{}{"type": "ping"})
+	if err == nil {
+		t.Fatalf("expected error for malformed response (missing 'type' key)")
+	}
+	if !strings.Contains(err.Error(), "malformed") {
+		t.Fatalf("error should describe malformed response: %v", err)
+	}
+}
+
 // --- DialTLS happy path ---------------------------------------------------
 
 func TestDialTLSHappyPathConnects(t *testing.T) {