fix: reject a /v1/search 200 that is not a search response

search.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"strings"
 )
 
 // SearchRequest describes a search against the runtime's /v1/search endpoint.
@@ -144,6 +145,81 @@ func (c *SpiceClient) Search(ctx context.Context, req *SearchRequest) (*SearchRe
 	if err := decodeJSONExact(respBody, &searchResp); err != nil {
 		return nil, fmt.Errorf("error decoding response from POST %s: %w", url, err)
 	}
+	if err := checkSearchResponseFields(respBody, &searchResp); err != nil {
+		return nil, fmt.Errorf("error decoding response from POST %s: %w", url, err)
+	}
 
 	return &searchResp, nil
 }
+
+// checkSearchResponseFields fails when body lacks a field the runtime always
+// sends: without one it is not a search response, and decoding it as one would
+// yield zero values — an empty search in 0ms, indistinguishable from a real one.
+// Keys are looked up by exact name because encoding/json matches struct tags
+// case-insensitively, so {"RESULTS": []} would otherwise pass; a key that
+// differs from one of these only by case is rejected, since the struct decoder
+// would read it where this check does not. It also fills Score from "score",
+// the name older runtimes used.
+func checkSearchResponseFields(body []byte, resp *SearchResponse) error {
+	var top map[string]json.RawMessage
+	if err := json.Unmarshal(body, &top); err != nil {
+		return err
+	}
+	if k := casedVariant(top, "results", "duration_ms"); k != "" {
+		return fmt.Errorf("not a search response: unexpected key %q", k)
+	}
+	if !hasJSONField(top, "results") {
+		return fmt.Errorf(`not a search response: missing "results"`)
+	}
+	if !hasJSONField(top, "duration_ms") {
+		return fmt.Errorf(`not a search response: missing "duration_ms"`)
+	}
+	var results []map[string]json.RawMessage
+	if err := json.Unmarshal(top["results"], &results); err != nil {
+		return err
+	}
+	if len(results) != len(resp.Results) {
+		return fmt.Errorf("not a search response: %d results decoded, %d present", len(resp.Results), len(results))
+	}
+	for i, match := range results {
+		k := casedVariant(match, "dataset", "matches", "_score", "score")
+		switch {
+		case match == nil:
+			return fmt.Errorf("not a search response: result %d is null", i)
+		case k != "":
+			return fmt.Errorf("not a search response: result %d has unexpected key %q", i, k)
+		case !hasJSONField(match, "dataset"):
+			return fmt.Errorf(`not a search response: result %d is missing "dataset"`, i)
+		case !hasJSONField(match, "matches"):
+			return fmt.Errorf(`not a search response: result %d is missing "matches"`, i)
+		case hasJSONField(match, "_score"):
+		case hasJSONField(match, "score"):
+			if err := json.Unmarshal(match["score"], &resp.Results[i].Score); err != nil {
+				return err
+			}
+		default:
+			return fmt.Errorf(`not a search response: result %d is missing "_score"`, i)
+		}
+	}
+	return nil
+}
+
+// casedVariant returns a key of obj that matches one of names case-insensitively
+// but not exactly, or "" when there is none.
+func casedVariant(obj map[string]json.RawMessage, names ...string) string {
+	for k := range obj {
+		for _, name := range names {
+			if k != name && strings.EqualFold(k, name) {
+				return k
+			}
+		}
+	}
+	return ""
+}
+
+// hasJSONField reports whether obj carries key, by exact name, with a non-null
+// value.
+func hasJSONField(obj map[string]json.RawMessage, key string) bool {
+	v, ok := obj[key]
+	return ok && !bytes.Equal(bytes.TrimSpace(v), []byte("null"))
+}

search_test.go

@@ -342,3 +342,78 @@ func TestSearchErrorSurfacesJSONMessage(t *testing.T) {
 		})
 	}
 }
+
+// A 200 whose body is not a search response (schema drift, or a proxy
+// answering for the runtime) must not read as zero results in zero
+// milliseconds, which is indistinguishable from a real empty search.
+func TestSearchRejectsResponseMissingRequiredFields(t *testing.T) {
+	tests := []struct {
+		name string
+		body string
+		want string
+	}{
+		{"empty object", `{}`, `missing "results"`},
+		{"error envelope", `{"error": "upstream timeout"}`, `missing "results"`},
+		{"null results", `{"results": null, "duration_ms": 3}`, `missing "results"`},
+		{"no duration", `{"results": []}`, `missing "duration_ms"`},
+		{"match without dataset", `{"results": [{"matches": {}, "_score": 0.9}], "duration_ms": 3}`, `result 0 is missing "dataset"`},
+		{"match without score", `{"results": [{"matches": {}, "dataset": "d"}], "duration_ms": 3}`, `result 0 is missing "_score"`},
+		{"match without matches", `{"results": [{"dataset": "d", "_score": 0.9}], "duration_ms": 3}`, `result 0 is missing "matches"`},
+		{"null match", `{"results": [null], "duration_ms": 3}`, `result 0 is null`},
+		// encoding/json matches struct tags case-insensitively; the runtime's
+		// field names are exact, so differently-cased keys are not them.
+		{"uppercase fields", `{"RESULTS": [], "DURATION_MS": 3}`, `unexpected key`},
+		{"uppercase duration", `{"results": [], "Duration_Ms": 3}`, `unexpected key "Duration_Ms"`},
+		{"uppercase match fields", `{"results": [{"MATCHES": {}, "DATASET": "d", "_SCORE": 0.9}], "duration_ms": 3}`, `result 0 has unexpected key`},
+		{"uppercase score", `{"results": [{"matches": {}, "dataset": "d", "_SCORE": 0.9}], "duration_ms": 3}`, `result 0 has unexpected key "_SCORE"`},
+		// A differently-cased duplicate would be read by the struct decoder but
+		// not by the exact-name check, so the two would judge different data.
+		{"cased duplicate results, legacy score", `{"results": [{"dataset": "d", "matches": {}, "score": 0.5}], "RESULTS": [], "duration_ms": 1}`, `unexpected key "RESULTS"`},
+		{"cased duplicate results", `{"results": [{"dataset": "d", "matches": {}, "_score": 0.5}], "RESULTS": [], "duration_ms": 1}`, `unexpected key "RESULTS"`},
+		{"cased duplicate dataset", `{"results": [{"dataset": "d", "DATASET": "e", "matches": {}, "_score": 0.5}], "duration_ms": 1}`, `result 0 has unexpected key "DATASET"`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			spice := newSearchTestClient(t, func(w http.ResponseWriter, r *http.Request) {
+				_, _ = io.WriteString(w, tt.body)
+			})
+
+			resp, err := spice.Search(context.Background(), &SearchRequest{Text: "tokyo"})
+			if err == nil {
+				t.Fatalf("expected an error, got %+v", resp)
+			}
+			if !strings.Contains(err.Error(), tt.want) {
+				t.Errorf("error = %q, want it to contain %q", err.Error(), tt.want)
+			}
+		})
+	}
+}
+
+func TestSearchAcceptsEmptyResults(t *testing.T) {
+	spice := newSearchTestClient(t, func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.WriteString(w, `{"results": [], "duration_ms": 3}`)
+	})
+
+	resp, err := spice.Search(context.Background(), &SearchRequest{Text: "tokyo"})
+	if err != nil {
+		t.Fatalf("Search() error = %v", err)
+	}
+	if len(resp.Results) != 0 || resp.DurationMs != 3 {
+		t.Errorf("resp = %+v, want no results in 3ms", resp)
+	}
+}
+
+// Older runtimes serialized the score as "score" rather than "_score".
+func TestSearchAcceptsLegacyScoreName(t *testing.T) {
+	spice := newSearchTestClient(t, func(w http.ResponseWriter, r *http.Request) {
+		_, _ = io.WriteString(w, `{"results": [{"matches": {}, "dataset": "d", "score": 0.5}], "duration_ms": 1}`)
+	})
+
+	resp, err := spice.Search(context.Background(), &SearchRequest{Text: "tokyo"})
+	if err != nil {
+		t.Fatalf("Search() error = %v", err)
+	}
+	if got := resp.Results[0].Score; got != 0.5 {
+		t.Errorf("Score = %v, want 0.5", got)
+	}
+}