From 15afd988916194243158df4084df7ef97a31da6e Mon Sep 17 00:00:00 2001 From: yetone Date: Thu, 8 Oct 2026 16:07:38 +0800 Subject: [PATCH] provider: marshalling Claude credentials no longer writes into the cached sign-in, so two requests reading it at once don't race (main's Race red since 01e66e0f) claudeCredential hands out its cached credentials by value, but their raw blob is one map, and marshal wrote the token fields into it. 01e66e0f calls ClaudeCodeMovedOff from limitHeaders beside a run's own read, so two goroutines marshalled the same map: CI run 37744825866 (52ad2d98) failed Race on ubuntu and macOS in six TestClaude* gateway tests with "DATA RACE" in claudeCredentials.marshal (account.go) and "http: panic ... invalid concurrent Body.Read call". marshal now writes into clones of the blob and of claudeAiOauth. TestClaudeCredentialsMarshalLeavesTheBlob marshals one credential from 8 goroutines and checks the blob is unchanged: on the old marshal it reports WARNING: DATA RACE under -race; passes with the fix (-race -count=5). Run at f00ed290 under a temp HOME with pinned caches: the six gateway tests with -race -count=3 FAIL on the old marshal and pass with the fix; go test -race ./internal/provider ok; go vet (darwin, GOOS=linux) and GOOS=windows go build ./... pass; gofmt clean. --- internal/provider/account.go | 5 ++++- internal/provider/account_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/provider/account.go b/internal/provider/account.go index 38cca4b2..3af850c2 100644 --- a/internal/provider/account.go +++ b/internal/provider/account.go @@ -360,12 +360,15 @@ func parseClaudeCredentials(b []byte) (claudeCredentials, bool) { return c, c.OAuth.AccessToken != "" } +// marshal writes into copies: c.raw is shared with every copy of c, the +// cached one included, which other goroutines marshal at the same time. func (c claudeCredentials) marshal() ([]byte, error) { - raw := c.raw + raw := maps.Clone(c.raw) if raw == nil { raw = map[string]any{} } oauth, _ := raw["claudeAiOauth"].(map[string]any) + oauth = maps.Clone(oauth) if oauth == nil { oauth = map[string]any{} } diff --git a/internal/provider/account_test.go b/internal/provider/account_test.go index 2a246dba..f79c384d 100644 --- a/internal/provider/account_test.go +++ b/internal/provider/account_test.go @@ -584,3 +584,29 @@ func TestKeychainText(t *testing.T) { t.Fatal("hex that isn't JSON taken") } } + +// The cached credentials are handed out by value, but their raw blob is one +// map: marshal is called on it from several goroutines at once (the +// gateway's limitHeaders beside a run's own read, CI's -race on 01e66e0f), +// so it must not write into it. +func TestClaudeCredentialsMarshalLeavesTheBlob(t *testing.T) { + c, ok := parseClaudeCredentials([]byte(`{"claudeAiOauth":{"accessToken":"sk-ant-oat01-a","refreshToken":"sk-ant-ort01-a","expiresAt":1},"mcpOAuth":{"x":{"accessToken":"m"}}}`)) + if !ok { + t.Fatal("not parsed") + } + c.OAuth.AccessToken, c.OAuth.SubscriptionType = "sk-ant-oat01-b", "max" + var wg sync.WaitGroup + for range 8 { + wg.Add(1) + go func() { + defer wg.Done() + if b, err := c.marshal(); err != nil || !strings.Contains(string(b), "sk-ant-oat01-b") || !strings.Contains(string(b), "mcpOAuth") { + t.Errorf("marshal %s %v", b, err) + } + }() + } + wg.Wait() + if o := c.raw["claudeAiOauth"].(map[string]any); o["accessToken"] != "sk-ant-oat01-a" || o["subscriptionType"] != nil { + t.Fatalf("the blob was written into: %v", o) + } +}