fix: reject implicit wallet fallback and foreign account polling

README.md

@@ -330,6 +330,20 @@ either rail.
 - **`GetSpending()` is a floor, not a total** — see [Cost Tracking](#cost-tracking).
 - **Nothing else.** Every method, option and response type is identical.
 
+### Switching between the rails
+
+An explicit wallet credential chooses the wallet rail even when
+`BLOCKRUN_API_KEY` is set, so `NewLLMClient("0x…")` always pays on-chain. A
+**blank** `BLOCKRUN_API_KEY` counts as unset — that is what `docker -e
+BLOCKRUN_API_KEY`, an unpopulated `${{ secrets.X }}`, and a bare
+`BLOCKRUN_API_KEY=` line all produce, and none of them mean you wanted the
+account rail. A **non-blank** value that is not a key is an error rather than a
+silent fall back to a wallet: someone typed a credential and got it wrong, and
+spending USDC instead of credit is the wrong way to tell them.
+
+Credentials are read once, at construction. Build a new client to change rails;
+an existing one keeps the account it started with.
+
 ### Environment
 
 ```bash

apikey.go

@@ -64,7 +64,12 @@ const (
 // IsAPIKey reports whether a credential string is a BlockRun API key rather
 // than a wallet private key.
 func IsAPIKey(credential string) bool {
-	return strings.HasPrefix(strings.TrimSpace(credential), APIKeyPrefix)
+	// The prefix alone is not a key. A truncated secret ("brk_") would
+	// otherwise select the account rail and fail at request time with a 401,
+	// which is the opposite of this rail's promise that a bad credential fails
+	// where you set it rather than where you use it.
+	return len(strings.TrimSpace(credential)) > len(APIKeyPrefix) &&
+		strings.HasPrefix(strings.TrimSpace(credential), APIKeyPrefix)
 }
 
 // resolveAPIKey decides whether a constructor call is an API-key call.
@@ -75,20 +80,32 @@ func IsAPIKey(credential string) bool {
 // set it keeps the wallet behaviour they already had, and one who has set it
 // meant to, even if an old BLOCKRUN_WALLET_KEY is still sitting in their
 // profile. PaymentMode() exists so that decision is never invisible.
-func resolveAPIKey(credential string) string {
+func resolveAPIKey(credential string) (string, error) {
 	if IsAPIKey(credential) {
-		return strings.TrimSpace(credential)
+		return strings.TrimSpace(credential), nil
 	}
-	// An explicit non-key credential (a wallet key) is a deliberate choice of
-	// the x402 rail and must not be overridden by the environment.
+	// Explicit wallet selection takes precedence, even over an invalid env key.
 	if strings.TrimSpace(credential) != "" {
-		return ""
+		return "", nil
 	}
+	// Blank is unset, not invalid. `BLOCKRUN_API_KEY=` in a .env file, a bare
+	// `docker -e BLOCKRUN_API_KEY`, and an unpopulated `${{ secrets.X }}` all
+	// arrive as the empty string, and every one of them means "I am not on the
+	// account rail" — erroring there breaks wallet users who never opted in,
+	// on upgrade, in CI. Keying on os.LookupEnv instead of the value made all
+	// three a hard failure.
+	//
+	// A non-blank value that is not a key is a different thing: someone typed a
+	// credential and got it wrong, and silently spending USDC instead of credit
+	// is the wrong way to tell them.
 	env := strings.TrimSpace(os.Getenv(EnvAPIKey))
-	if IsAPIKey(env) {
-		return env
+	if env == "" {
+		return "", nil
 	}
-	return ""
+	if !IsAPIKey(env) {
+		return "", &ValidationError{Field: EnvAPIKey, Message: "Invalid configured API key: expected a key starting with \"brk_\". Correct it, clear it, or explicitly pass a wallet key."}
+	}
+	return env, nil
 }
 
 // newAPIKeyBaseClient builds a baseClient on the account rail. It cannot fail:

apikey_test.go

@@ -3,6 +3,7 @@ package blockrun
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -47,35 +48,35 @@ func TestIsAPIKey(t *testing.T) {
 func TestResolveAPIKeyPrecedence(t *testing.T) {
 	t.Run("explicit key wins over everything", func(t *testing.T) {
 		t.Setenv(EnvAPIKey, "brk_live_fromenv")
-		if got := resolveAPIKey(testAPIKey); got != testAPIKey {
+		if got, err := resolveAPIKey(testAPIKey); err != nil || got != testAPIKey {
 			t.Errorf("got %q, want the explicit key", got)
 		}
 	})
 
 	t.Run("explicit wallet key opts out of the env key", func(t *testing.T) {
 		t.Setenv(EnvAPIKey, testAPIKey)
-		if got := resolveAPIKey(testPrivateKey); got != "" {
+		if got, err := resolveAPIKey(testPrivateKey); err != nil || got != "" {
 			t.Errorf("got %q, want no API key: an explicit wallet key chooses the x402 rail", got)
 		}
 	})
 
 	t.Run("env key beats the wallet env vars", func(t *testing.T) {
 		t.Setenv(EnvAPIKey, testAPIKey)
 		t.Setenv("BLOCKRUN_WALLET_KEY", testPrivateKey)
-		if got := resolveAPIKey(""); got != testAPIKey {
+		if got, err := resolveAPIKey(""); err != nil || got != testAPIKey {
 			t.Errorf("got %q, want the env API key", got)
 		}
 	})
 
 	t.Run("no key anywhere", func(t *testing.T) {
-		if got := resolveAPIKey(""); got != "" {
+		if got, err := resolveAPIKey(""); err != nil || got != "" {
 			t.Errorf("got %q, want empty", got)
 		}
 	})
 
 	t.Run("a non-brk env value is not a key", func(t *testing.T) {
 		t.Setenv(EnvAPIKey, "not-a-key")
-		if got := resolveAPIKey(""); got != "" {
+		if got, err := resolveAPIKey(""); err == nil || got != "" {
 			t.Errorf("got %q, want empty", got)
 		}
 	})
@@ -269,24 +270,24 @@ func TestResolvePollURLStripsAPIPrefixOnAccountRail(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewLLMClient: %v", err)
 	}
-	got := apiKeyClient.resolvePollURL("/api/v1/images/generations/job_1")
+	got, err := apiKeyClient.resolvePollURL("/api/v1/images/generations/job_1")
 	want := DefaultAPIKeyURL + "/v1/images/generations/job_1"
-	if got != want {
+	if err != nil || got != want {
 		t.Errorf("account rail: got %q, want %q", got, want)
 	}
 
 	walletClient, err := NewLLMClient(testPrivateKey)
 	if err != nil {
 		t.Fatalf("NewLLMClient: %v", err)
 	}
-	got = walletClient.resolvePollURL("/api/v1/images/generations/job_1")
+	got, err = walletClient.resolvePollURL("/api/v1/images/generations/job_1")
 	want = "https://blockrun.ai/api/v1/images/generations/job_1"
-	if got != want {
+	if err != nil || got != want {
 		t.Errorf("wallet rail: got %q, want %q", got, want)
 	}
 
-	if got := apiKeyClient.resolvePollURL("https://elsewhere.example/x"); got != "https://elsewhere.example/x" {
-		t.Errorf("absolute poll_url was rewritten: %q", got)
+	if _, err := apiKeyClient.resolvePollURL("https://elsewhere.example/x"); err == nil {
+		t.Error("foreign polling origin was accepted")
 	}
 }
 
@@ -456,3 +457,224 @@ func TestAPIKeyPaidGETCarriesTheKey(t *testing.T) {
 		t.Errorf("TotalUSD = %v, want 0.001", spend.TotalUSD)
 	}
 }
+
+// Narrowed from the original: "" and "   " used to be asserted as errors here
+// too, which is the regression TestBlankEnvKeyIsUnsetNotInvalid covers. A blank
+// value is how an unpopulated CI secret and a bare `docker -e` arrive, so it
+// means unset. A non-blank non-key stays an error, which is the half that
+// protects the money.
+func TestInvalidAPIKeyEnvDoesNotSelectWallet(t *testing.T) {
+	for _, value := range []string{"not-a-key", "sk-wrong-vendor"} {
+		t.Run("invalid="+value, func(t *testing.T) {
+			t.Setenv(EnvAPIKey, value)
+			t.Setenv("BLOCKRUN_WALLET_KEY", testPrivateKey)
+			if _, err := NewLLMClient(""); err == nil {
+				t.Fatal("expected invalid configured API key error")
+			}
+			client, err := NewLLMClient(testPrivateKey)
+			if err != nil || client.PaymentMode() != PaymentModeWallet {
+				t.Fatal("explicit wallet must still work")
+			}
+		})
+	}
+}
+
+func TestAPIKeyRotationDoesNotChangeExistingClient(t *testing.T) {
+	t.Setenv(EnvAPIKey, testAPIKey)
+	first, err := NewLLMClient("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(EnvAPIKey, "brk_test_second")
+	second, err := NewLLMClient("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if first.apiKey != testAPIKey || second.apiKey != "brk_test_second" {
+		t.Fatal("client credentials crossed")
+	}
+}
+
+func TestForeignPollURLCannotReceiveAccountCredential(t *testing.T) {
+	// Use the public image API and a second server that must receive no key.
+	seen := 0
+	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { seen++; w.WriteHeader(500) }))
+	defer foreign.Close()
+	gateway := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(202)
+		json.NewEncoder(w).Encode(map[string]string{"id": "test", "poll_url": foreign.URL + "/job"})
+	}))
+	defer gateway.Close()
+	client, err := NewImageClient(testAPIKey, WithImageAPIURL(gateway.URL))
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = client.Generate(context.Background(), "test", &ImageGenerateOptions{Model: "openai/gpt-image-1"})
+	if err == nil || !strings.Contains(err.Error(), "origin") {
+		t.Fatalf("expected origin error, got %v", err)
+	}
+	if seen != 0 {
+		t.Fatal("account credential reached foreign poll server")
+	}
+}
+
+func TestAccountAndWalletCanShareHTTPClientWithoutSharingAuth(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/account/v1/chat/completions":
+			if r.Header.Get("Authorization") != "Bearer "+testAPIKey || r.Header.Get("Payment-Signature") != "" {
+				t.Error("account authentication changed or included a wallet proof")
+			}
+		case "/wallet/v1/chat/completions":
+			if r.Header.Get("Authorization") != "" {
+				t.Error("account key crossed into wallet request")
+			}
+		default:
+			t.Errorf("unexpected endpoint: %s", r.URL.Path)
+		}
+		w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
+	}))
+	defer server.Close()
+	shared := server.Client()
+	originalTransport := shared.Transport
+	t.Setenv("BLOCKRUN_API_KEY", testAPIKey)
+	account, err := NewLLMClient("", WithAPIURL(server.URL+"/account"), WithHTTPClient(shared))
+	if err != nil {
+		t.Fatal(err)
+	}
+	wallet, err := NewLLMClient(testPrivateKey, WithAPIURL(server.URL+"/wallet"), WithHTTPClient(shared))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("BLOCKRUN_API_KEY", "brk_live_changed_account")
+	t.Setenv("BLOCKRUN_API_KEY_URL", "https://changed.example")
+	for _, client := range []*LLMClient{account, wallet, account} {
+		if reply, err := client.Chat(context.Background(), "openai/gpt-5.2", "hi"); err != nil || reply != "ok" {
+			t.Fatalf("call failed: %q %v", reply, err)
+		}
+	}
+	if shared.Transport != originalTransport || wallet.GetWalletAddress() != testWalletAddress {
+		t.Fatal("account setup mutated caller transport or wallet identity")
+	}
+}
+
+// A blank BLOCKRUN_API_KEY is unset, not invalid.
+//
+// The first version of this guard keyed on os.LookupEnv, so a variable that was
+// SET but empty counted as configured and hard-failed. `docker -e
+// BLOCKRUN_API_KEY`, an unpopulated `${{ secrets.X }}`, and a bare
+// `BLOCKRUN_API_KEY=` line all produce exactly that, which made every one of
+// them break a wallet user's CI on upgrade.
+func TestBlankEnvKeyIsUnsetNotInvalid(t *testing.T) {
+	for _, blank := range []string{"", "   ", "\t\n"} {
+		t.Run(fmt.Sprintf("value=%q", blank), func(t *testing.T) {
+			t.Setenv(EnvAPIKey, blank)
+			t.Setenv("BLOCKRUN_WALLET_KEY", testPrivateKey)
+
+			client, err := NewLLMClient("")
+			if err != nil {
+				t.Fatalf("blank %s refused a wallet user: %v", EnvAPIKey, err)
+			}
+			if client.PaymentMode() != PaymentModeWallet {
+				t.Errorf("PaymentMode = %q, want %q", client.PaymentMode(), PaymentModeWallet)
+			}
+		})
+	}
+}
+
+// A non-blank value that is not a key is the opposite case: someone typed a
+// credential and got it wrong, and silently spending USDC instead of credit is
+// the wrong way to tell them.
+func TestMalformedEnvKeyRefusesRatherThanFallingBack(t *testing.T) {
+	for _, bad := range []string{"oops-typo", "sk-not-ours", APIKeyPrefix} {
+		t.Run(bad, func(t *testing.T) {
+			t.Setenv(EnvAPIKey, bad)
+			t.Setenv("BLOCKRUN_WALLET_KEY", testPrivateKey)
+
+			if _, err := NewLLMClient(""); err == nil {
+				t.Errorf("%q silently selected a wallet", bad)
+			}
+		})
+	}
+}
+
+// The prefix alone is a truncated secret, not a key.
+func TestBarePrefixIsNotAKey(t *testing.T) {
+	if IsAPIKey(APIKeyPrefix) {
+		t.Errorf("IsAPIKey(%q) = true: a truncated secret would select the account rail and 401 later", APIKeyPrefix)
+	}
+	if !IsAPIKey(APIKeyPrefix + "x") {
+		t.Errorf("IsAPIKey(%q) = false", APIKeyPrefix+"x")
+	}
+}
+
+// The origin guard covers BOTH rails.
+//
+// The API key is the obvious credential, but PAYMENT-SIGNATURE is one too:
+// image.go and video.go set it on every poll, so a gateway answering with an
+// off-origin poll_url collects a signed payment authorization. Guarding one
+// rail and not the other was the asymmetry this closes.
+func TestPollURLRefusesForeignOriginOnBothRails(t *testing.T) {
+	apiKeyClient, err := NewLLMClient(testAPIKey)
+	if err != nil {
+		t.Fatalf("NewLLMClient: %v", err)
+	}
+	walletClient, err := NewLLMClient(testPrivateKey)
+	if err != nil {
+		t.Fatalf("NewLLMClient: %v", err)
+	}
+
+	hostile := []string{
+		"https://attacker.example/job",
+		"http://attacker.example/job",
+		"//attacker.example/job",               // protocol-relative
+		"https://user:pw@api.blockrun.ai/v1/x", // credentials smuggled in
+	}
+	for _, rail := range []struct {
+		name   string
+		client *LLMClient
+	}{{"account", apiKeyClient}, {"wallet", walletClient}} {
+		for _, u := range hostile {
+			if _, err := rail.client.resolvePollURL(u); err == nil {
+				t.Errorf("%s rail accepted %q — a credential would be sent there", rail.name, u)
+			}
+		}
+	}
+}
+
+// Same origin is still served, including the shapes that are one origin written
+// two ways.
+func TestPollURLAcceptsSameOrigin(t *testing.T) {
+	c, err := NewLLMClient(testAPIKey)
+	if err != nil {
+		t.Fatalf("NewLLMClient: %v", err)
+	}
+	for _, u := range []string{
+		DefaultAPIKeyURL + "/v1/images/generations/job_1",
+		"https://api.blockrun.ai:443/v1/images/generations/job_1", // explicit default port
+		"HTTPS://API.BLOCKRUN.AI/v1/images/generations/job_1",     // case variance
+	} {
+		if _, err := c.resolvePollURL(u); err != nil {
+			t.Errorf("same-origin %q refused: %v", u, err)
+		}
+	}
+}
+
+// An absolute same-origin poll_url still has to lose the gateway's /api mount,
+// or it reaches the account rail as /api/v1 and answers wrong_host — the exact
+// failure resolvePollURL exists to prevent.
+func TestAbsoluteSameOriginStillStripsTheAPIMount(t *testing.T) {
+	c, err := NewLLMClient(testAPIKey)
+	if err != nil {
+		t.Fatalf("NewLLMClient: %v", err)
+	}
+	got, err := c.resolvePollURL(DefaultAPIKeyURL + "/api/v1/videos/generations/job_1?d=8")
+	if err != nil {
+		t.Fatalf("resolvePollURL: %v", err)
+	}
+	want := DefaultAPIKeyURL + "/v1/videos/generations/job_1?d=8"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}