From 2176383729a6512986572e9e0b1bb1a08acb591c Mon Sep 17 00:00:00 2001 From: hzhang Date: Tue, 26 May 2026 12:43:04 +0100 Subject: [PATCH] fix(cli): send api-keys via X-API-Key in client.New + help surface passmgr.GetToken returns an api-key in padded-cell mode (provisioned by scripts/provision-hf-accounts.sh via 'hf user reset-apikey'), but every call site funneled that through client.New which sent it as a 'Authorization: Bearer '. The HF backend's HTTPBearer middleware expects JWT shape there and rejects hex strings as 'Could not validate credentials'. The d2b83ad backend fix added a Bearer-fallback that tries the value as an api-key, which masked the issue against current prod; older backends or any future change in that fallback still 401. Two changes: - client.New auto-detects shape: 'eyJ'-prefix + two dots == JWT (Bearer), anything else == api-key (X-API-Key). Empty token sets neither header. - internal/help/surface.go's loadPermissionState (called by hf --help introspection) switches to client.NewWithAPIKey explicitly so the command-discovery path doesn't depend on the heuristic at all. When that path failed silently (Known:false), agents would see only the always-permitted commands ('user.*', 'agent.status', 'config', 'health', 'version') and conclude they had no project permission. Adds internal/client/client_test.go covering both header paths plus empty-token, isLikelyJWT cases, and NewWithAPIKey precedence. Verified end-to-end in sim against a rebuilt hf-backend matching prod (commit d2b83ad): cli with --token sends X-Api-Key header, backend returns 200 on /projects + /auth/me/permissions. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/client/client.go | 24 +++++++- internal/client/client_test.go | 104 +++++++++++++++++++++++++++++++++ internal/help/surface.go | 7 ++- 3 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 internal/client/client_test.go diff --git a/internal/client/client.go b/internal/client/client.go index 9fce9f5..194f6da 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -19,14 +19,34 @@ type Client struct { } // New creates a Client with the given base URL and optional auth token. +// +// The token is sent as Authorization: Bearer when it looks like a JWT +// (eyJ-prefixed, three dot-separated segments). Anything else is treated as +// an API key and sent via X-API-Key. This lets call sites that historically +// passed an api-key as a "token" (e.g. the value returned by passmgr.GetToken +// in padded-cell mode) authenticate correctly without per-callsite churn. func New(baseURL, token string) *Client { - return &Client{ + c := &Client{ BaseURL: strings.TrimRight(baseURL, "/"), - Token: token, HTTPClient: &http.Client{ Timeout: 30 * time.Second, }, } + if isLikelyJWT(token) { + c.Token = token + } else if token != "" { + c.APIKey = token + } + return c +} + +// isLikelyJWT returns true for tokens that look like a JSON Web Token: +// "eyJ"-prefixed (base64-encoded JSON header opening with `{"`) and exactly +// two dots separating header.payload.signature. API keys minted by the HF +// backend are hex (`/users/{id}/apikey` returns a 64-hex-char `key`); fabric +// keys use a `fak_` prefix. None of those match this shape. +func isLikelyJWT(token string) bool { + return strings.HasPrefix(token, "eyJ") && strings.Count(token, ".") == 2 } // NewWithAPIKey creates a Client that authenticates using X-API-Key. diff --git a/internal/client/client_test.go b/internal/client/client_test.go new file mode 100644 index 0000000..aa72cc1 --- /dev/null +++ b/internal/client/client_test.go @@ -0,0 +1,104 @@ +package client + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestIsLikelyJWT(t *testing.T) { + cases := []struct { + in string + want bool + why string + }{ + // Real JWT minted by an HS256 signer (header `{"alg":"HS256","typ":"JWT"}`). + {"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIn0.signature", true, "valid JWT shape"}, + // HF backend api-keys are 64-char hex from /users/{id}/apikey. + {"f654c3ff0bbc09e6a22294dfbbbff371a4550366849f59de68ddf064742831a0", false, "hex api-key"}, + // Fabric api-keys carry a fak_ prefix. + {"fak_30791357ca11ac2ff963999bf265f6a5f240593eb01c06fc", false, "fabric api-key"}, + // eyJ prefix without three segments isn't a JWT. + {"eyJabc", false, "prefix only"}, + {"eyJabc.def", false, "two segments"}, + // Empty / nonsense. + {"", false, "empty"}, + {"....", false, "dots only"}, + } + for _, c := range cases { + if got := isLikelyJWT(c.in); got != c.want { + t.Errorf("isLikelyJWT(%q) = %v, want %v (%s)", c.in, got, c.want, c.why) + } + } +} + +func TestNewAutoSelectsAuthHeader(t *testing.T) { + // Capture which auth header reaches the server for each token shape. + var lastReq *http.Request + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastReq = r + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "{}") + })) + defer srv.Close() + + // api-key path: should go via X-API-Key, NOT Authorization. + apiKey := "f654c3ff0bbc09e6a22294dfbbbff371a4550366849f59de68ddf064742831a0" + if _, err := New(srv.URL, apiKey).Get("/anything"); err != nil { + t.Fatalf("api-key call failed: %v", err) + } + if got := lastReq.Header.Get("X-API-Key"); got != apiKey { + t.Errorf("api-key not sent as X-API-Key (got %q)", got) + } + if got := lastReq.Header.Get("Authorization"); got != "" { + t.Errorf("api-key leaked into Authorization header (got %q)", got) + } + + // JWT path: should go via Authorization: Bearer, NOT X-API-Key. + jwt := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIn0.signature" + if _, err := New(srv.URL, jwt).Get("/anything"); err != nil { + t.Fatalf("jwt call failed: %v", err) + } + if got := lastReq.Header.Get("Authorization"); !strings.HasPrefix(got, "Bearer ") || !strings.HasSuffix(got, jwt) { + t.Errorf("jwt not sent as Bearer (got %q)", got) + } + if got := lastReq.Header.Get("X-API-Key"); got != "" { + t.Errorf("jwt leaked into X-API-Key header (got %q)", got) + } + + // Empty token: neither header set. + if _, err := New(srv.URL, "").Get("/anything"); err != nil { + t.Fatalf("empty-token call failed: %v", err) + } + if got := lastReq.Header.Get("Authorization"); got != "" { + t.Errorf("empty token set Authorization (got %q)", got) + } + if got := lastReq.Header.Get("X-API-Key"); got != "" { + t.Errorf("empty token set X-API-Key (got %q)", got) + } +} + +func TestNewWithAPIKeyAlwaysUsesAPIKeyHeader(t *testing.T) { + // Even if someone passes a JWT-shaped string via NewWithAPIKey, it must + // still go via X-API-Key — the explicit constructor wins. + var lastReq *http.Request + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastReq = r + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "{}") + })) + defer srv.Close() + + jwtShape := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxIn0.signature" + if _, err := NewWithAPIKey(srv.URL, jwtShape).Get("/anything"); err != nil { + t.Fatalf("call failed: %v", err) + } + if got := lastReq.Header.Get("X-API-Key"); got != jwtShape { + t.Errorf("NewWithAPIKey didn't use X-API-Key (got %q)", got) + } + if got := lastReq.Header.Get("Authorization"); got != "" { + t.Errorf("NewWithAPIKey set Authorization (got %q)", got) + } +} diff --git a/internal/help/surface.go b/internal/help/surface.go index faf5e39..a2ce8c2 100644 --- a/internal/help/surface.go +++ b/internal/help/surface.go @@ -224,7 +224,12 @@ func loadPermissionState(token string) permissionState { return permissionState{Known: false, Permissions: map[string]struct{}{}} } - c := client.New(cfg.BaseURL, token) + // passmgr.GetToken() returns an api-key in padded-cell mode (provisioned + // by scripts/provision-hf-accounts.sh via `hf user reset-apikey`), so go + // through the X-API-Key path explicitly. client.New also auto-detects this + // nowadays, but the explicit call keeps the introspection path independent + // of that heuristic. + c := client.NewWithAPIKey(cfg.BaseURL, token) data, err := c.Get("/auth/me/permissions") if err != nil { return permissionState{Known: false, Permissions: map[string]struct{}{}}