Skip to content

Commit 612c99a

Browse files
committed
fix: only send X-API-Key from RefreshDataset when a key is configured
RefreshDataset set the header unconditionally, so a client with no key configured sent an empty X-API-Key. Auth middleware reads that as a supplied-but-invalid credential rather than as an absent one, which is the same bug just fixed in Search: client.go, nsql.go, status.go and active_queries.go all guard the header, and these were the two callers that did not. TestRefreshDatasetApiKeyHeader asserts the header is absent, not empty, when no key is set. It needs no live runtime, so it joins the allowlist the Windows CI job runs natively.
1 parent 925af21 commit 612c99a

3 files changed

Lines changed: 53 additions & 2 deletions

File tree

.github/workflows/go.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ jobs:
4141
# runtime install + integration tests to WSL (see the WSL steps below).
4242
- name: Unit tests (Windows native)
4343
if: matrix.os == 'windows-latest'
44-
run: go test -v -run 'TestUserAgent|TestPrependedUserAgent|TestInferArrowType|TestAppendValueToBuilder|TestComprehensiveArrowTypes|TestParamType|TestTypedParamInference|TestExtendedArrowTypes|TestSearch|TestNsql|TestListActiveQueries|TestCancelActiveQuery|TestActiveQueryStartedAt' ./...
44+
run: go test -v -run 'TestUserAgent|TestPrependedUserAgent|TestInferArrowType|TestAppendValueToBuilder|TestComprehensiveArrowTypes|TestParamType|TestTypedParamInference|TestExtendedArrowTypes|TestSearch|TestNsql|TestListActiveQueries|TestCancelActiveQuery|TestActiveQueryStartedAt|TestRefreshDatasetApiKeyHeader' ./...
4545

4646
- name: Install Spice (https://install.spiceai.org) (Linux)
4747
if: matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest'

datasets.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,14 @@ func (c *SpiceClient) RefreshDataset(ctx context.Context, dataset string, opts *
4343

4444
req = req.WithContext(c.traceHttpRequest(ctx, "RefreshDataset", req))
4545

46-
req.Header.Set("X-API-Key", c.apiKey)
4746
req.Header.Set("Content-Type", "application/json")
4847
req.Header.Set("user-agent", c.userAgent)
48+
// Only send the key when there is one — an empty X-API-Key reads as a
49+
// supplied-but-invalid credential to auth middleware, which is different
50+
// from omitting the header. Matches IsSpiceReady in client.go.
51+
if c.apiKey != "" {
52+
req.Header.Set("X-API-Key", c.apiKey)
53+
}
4954

5055
resp, err := c.httpClient.Do(req)
5156
if err != nil {

datasets_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package gospice
22

33
import (
44
"context"
5+
"net/http"
6+
"net/http/httptest"
57
"testing"
68
"time"
79
)
@@ -44,3 +46,47 @@ func TestLocalRuntimeDatasetRefresh(t *testing.T) {
4446
}
4547
})
4648
}
49+
50+
func TestRefreshDatasetApiKeyHeader(t *testing.T) {
51+
// An empty X-API-Key is not the same as no X-API-Key: auth middleware can
52+
// read the former as a supplied-but-invalid credential.
53+
tests := []struct {
54+
name string
55+
apiKey string
56+
want string
57+
}{
58+
{name: "no key omits the header", apiKey: "", want: ""},
59+
{name: "key is sent", apiKey: "test-app|test-key", want: "test-app|test-key"},
60+
}
61+
62+
for _, tt := range tests {
63+
t.Run(tt.name, func(t *testing.T) {
64+
var got string
65+
var present bool
66+
67+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
68+
_, present = r.Header["X-Api-Key"]
69+
got = r.Header.Get("X-API-Key")
70+
w.WriteHeader(http.StatusCreated)
71+
}))
72+
t.Cleanup(server.Close)
73+
74+
spice := NewSpiceClient()
75+
if err := WithHttpAddress(server.URL)(spice); err != nil {
76+
t.Fatalf("error setting http address: %v", err)
77+
}
78+
spice.apiKey = tt.apiKey
79+
80+
if err := spice.RefreshDataset(context.Background(), "app_messages", nil); err != nil {
81+
t.Fatalf("unexpected error: %v", err)
82+
}
83+
84+
if tt.want == "" && present {
85+
t.Error("X-API-Key should be absent when no key is configured")
86+
}
87+
if got != tt.want {
88+
t.Errorf("X-API-Key = %q, want %q", got, tt.want)
89+
}
90+
})
91+
}
92+
}

0 commit comments

Comments
 (0)