johannSoCodeyPublic

claudecode: harden refresh against the ways write-back can go wrong

Review of the previous commit found two ways it could hurt the user.

A failed write-back after rotation was swallowed in silence — the exact
harm the conditional write exists to prevent. The session runs on the
new token, the dead one stays on disk, and nothing anywhere says so.
Source now carries an OnWriteBackFailure hook, wired to the event log
always and to stderr outside the TUI.

A 200 without expires_in minted a token that was already expired, so
every subsequent request refreshed again — a hammering loop of our own
making against an endpoint that only ever answers 429. Now rejected as
malformed.

Four more, all reachable: Save is a compare-and-swap, so running
`claude` mid-session no longer gets its fresh login overwritten by our
older chain; the temp file is fsynced before the rename and the
directory after, so power loss cannot publish an empty credential file;
a rotated token's unknown refresh lifetime reads as unknown rather than
expired, instead of refusing a request the server would have honoured;
and error bodies are bounded and truncated, because a token endpoint is
a plausible place for a submitted credential to be echoed back.

Save still never deletes a key. A rotated token's expiry is unknowable,
and deleting the stale one would be more honest — but Claude Code reads
this file and we cannot see what it does with a missing key, while a
too-early expiry only makes it renew sooner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Browse Files

Jjonicommitted 4 weeks ago1 parent d10d851commit0e8321b

Showing 6 changed files with 449 additions and 46 deletions.

+1−1cmd/harness/auth_test.goView File

@@ -37,7 +37,7 @@ func TestClaudeCodeFillsInAMissingCredential(t *testing.T) {
37	37	if src == nil {
38	38	t.Fatal("expected a token source")
39	39	}
40		- tok, err := src(context.Background())
	40	+ tok, err := src.Token(context.Background())
41	41	if err != nil {
42	42	t.Fatalf("token: %v", err)
43	43	}

+28−3cmd/harness/main.goView File

@@ -83,13 +83,17 @@ func run() error {
83	83	// request that omits it is rejected (as a 429, not a 401 — it reads like a
84	84	// quota problem and is not one).
85	85	var systemPrefix string
	86	+ // ccSource is kept so its write-back failure hook can be wired once the
	87	+ // places a warning can go — the notice channel, the event log — exist.
	88	+ var ccSource *claudecode.Source
86	89	if home != "" {
87	90	src, note, err := resolveClaudeCode(client, home, *noClaudeCodeAuth)
88	91	if err != nil {
89	92	return err
90	93	}
91	94	if src != nil {
92		- client = llm.New(append(opts, llm.WithTokenSource(src))...)
	95	+ ccSource = src
	96	+ client = llm.New(append(opts, llm.WithTokenSource(src.Token))...)
93	97	// Not expected to ever trigger: resolveClaudeCode already checked
94	98	// the endpoint before we agreed to borrow this credential, but
95	99	// that check ran against the client we had before this rebuild.
@@ -138,6 +142,23 @@ func run() error {
138	142	if authNote != "" {
139	143	notice("%s", authNote)
140	144	}
	145	+ if ccSource != nil {
	146	+ // A rotation we could not store means the credential on disk is dead
	147	+ // and its replacement dies with this process — the user's next `claude`
	148	+ // will ask them to log in again, for a reason nothing else would
	149	+ // explain. It goes to the event log unconditionally, because that
	150	+ // survives the session; stderr only outside the TUI, which owns the
	151	+ // screen once it starts.
	152	+ ccSource.OnWriteBackFailure = func(err error) {
	153	+ log.Log(metrics.Event{Type: "credential_writeback_failed", Extra: map[string]any{
	154	+ "error": err.Error(), "file": "~/" + claudecode.Filename,
	155	+ }})
	156	+ if !useTUI {
	157	+ fmt.Fprintf(os.Stderr, "harness: your Claude Code login was renewed but ~/%s could not be updated: %v\n", claudecode.Filename, err)
	158	+ fmt.Fprintln(os.Stderr, "harness: run `claude` when convenient — its stored login is now out of date.")
	159	+ }
	160	+ }
	161	+ }
141	162
142	163	var found []skills.Skill
143	164	if !*noSkills {
@@ -344,7 +365,7 @@ To use a non-Anthropic endpoint, set ANTHROPIC_BASE_URL or -base-url as well.`)
344	365	// because a credential we found on disk is not one the user chose to send —
345	366	// forwarding it to a -base-url someone else operates would leak it to a third
346	367	// party because a file happened to exist.
347		- func resolveClaudeCode(client *llm.Client, home string, disabled bool) (func(context.Context) (string, error), string, error) {
	368	+ func resolveClaudeCode(client llm.Client, home string, disabled bool) (claudecode.Source, string, error) {
348	369	if disabled \|\| client.HasCredentials() \|\| !client.IsDefaultEndpoint() {
349	370	return nil, "", nil
350	371	}
@@ -359,7 +380,11 @@ func resolveClaudeCode(client *llm.Client, home string, disabled bool) (func(con
359	380	// home is handed on so a rotated refresh token can be written back. That
360	381	// is the only circumstance in which this process writes to Claude Code's
361	382	// file, and not writing then is what would break its login.
362		- return claudecode.NewSource(creds, client.HTTP, home).Token, note, nil
	383	+ //
	384	+ // The refresh client is deliberately not the harness's: that one allows
	385	+ // ten minutes for a model to answer, which is the wrong budget for a token
	386	+ // exchange running under a lock. NewSource supplies a short-timeout client.
	387	+ return claudecode.NewSource(creds, nil, home), note, nil
363	388	}
364	389
365	390	// interactive reports whether both ends of the terminal are a real one. A

+28−3docs/superpowers/specs/2026-09-17-claude-code-oauth-design.mdView File

@@ -190,9 +190,34 @@ in the same directory, `0600`, then rename, so a crash midway leaves the
190	190	original rather than half a credential. Verified by round-tripping a copy of a
191	191	real credentials file and diffing every key.
192	192
193		- Write-back is best effort. A store we cannot write is a worse next session, not
194		- a failed current one — we are holding a token that works, and refusing to use it
195		- would be strictly worse than a stale file.
	193	+ Write-back is best effort but never silent. A store we cannot write is a worse
	194	+ next session, not a failed current one — we are holding a token that works, and
	195	+ refusing to use it would be strictly worse than a stale file. But rotated-and-
	196	+ unstorable is exactly the state this design exists to prevent: the token on disk
	197	+ is dead and its replacement dies with the process. So `Source` carries an
	198	+ `OnWriteBackFailure` hook, wired to the event log unconditionally and to stderr
	199	+ outside the TUI, and swallowing that error is a bug rather than a policy.
	200	+
	201	+ Four more things the write has to survive, none of them hypothetical:
	202	+
	203	+ - The owner logging in underneath us. If the user runs `claude` mid-session,
	204	+ their fresh credential is strictly better than ours. `Save` takes the refresh
	205	+ token it expects to find and declines with `ErrStoreChanged` if the file no
	206	+ longer holds it — a compare-and-swap, not a blind overwrite.
	207	+ - Power loss. The temp file is fsynced before the rename and the directory
	208	+ after, or the rename can be durable while the contents are not, and the
	209	+ machine comes back to an empty credential file.
	210	+ - A response with no `expires_in`. That would mint a token born expired, so
	211	+ the next request refreshes again, and the one after that — a hammering loop of
	212	+ our own making against an endpoint that already only answers 429. It is
	213	+ rejected as malformed instead.
	214	+ - A rotated token's unknown lifetime. The exchange never says how long the
	215	+ new refresh token lives. In memory that is the zero time, and `RefreshExpired`
	216	+ reads zero as unknown rather than expired — otherwise we would refuse to
	217	+ send a request the server would have honoured. On disk the stale key is left
	218	+ alone rather than deleted: Claude Code reads this file, we cannot see what it
	219	+ does with a missing key, and a too-early expiry merely makes it renew sooner.
	220	+ Between annoying a tool we do not own and possibly breaking it, annoying wins.
196	221
197	222	Because the endpoint has never once answered, none of this is verified against
198	223	the live service. It is verified against `httptest` and against the real file's

+17−3internal/claudecode/claudecode.goView File

@@ -73,14 +73,19 @@ func Load(home string) (*Credentials, error) {
73	73	if o.ExpiresAt == 0 {
74	74	return nil, fmt.Errorf("claudecode: %s has no expiresAt", path)
75	75	}
76		- return &Credentials{
	76	+ c := &Credentials{
77	77	AccessToken: o.AccessToken,
78	78	RefreshToken: o.RefreshToken,
79	79	ExpiresAt: time.UnixMilli(o.ExpiresAt),
80		- RefreshExpiresAt: time.UnixMilli(o.RefreshTokenExpiresAt),
81	80	Scopes: o.Scopes,
82	81	SubscriptionType: o.SubscriptionType,
83		- }, nil
	82	+ }
	83	+ // Absent means unknown, and unknown must stay the zero Time: UnixMilli(0)
	84	+ // is 1970, which reads as long expired and would refuse to refresh at all.
	85	+ if o.RefreshTokenExpiresAt != 0 {
	86	+ c.RefreshExpiresAt = time.UnixMilli(o.RefreshTokenExpiresAt)
	87	+ }
	88	+ return c, nil
84	89	}
85	90
86	91	// Expired reports whether the access token is past its expiry, or close enough
@@ -91,6 +96,15 @@ func (c *Credentials) Expired(now time.Time) bool {
91	96
92	97	// RefreshExpired reports whether the refresh token is past its expiry. That is
93	98	// terminal: no amount of refreshing fixes it, only logging in again.
	99	+ //
	100	+ // A zero expiry means unknown, not expired — the store may not carry the key,
	101	+ // and a rotated token's lifetime is never returned by the exchange. Treating
	102	+ // unknown as dead would refuse to send a request the server would have
	103	+ // honoured, and turn any schema drift into a false "your login has expired".
	104	+ // When we do not know, let the server be the authority: it refuses with a 400.
94	105	func (c *Credentials) RefreshExpired(now time.Time) bool {
	106	+ if c.RefreshExpiresAt.IsZero() {
	107	+ return false
	108	+ }
95	109	return !now.Before(c.RefreshExpiresAt)
96	110	}

+133−34internal/claudecode/refresh.goView File

@@ -24,6 +24,17 @@ const tokenURLEnv = "CLAUDECODE_TOKEN_URL"
24	24
25	25	const defaultTokenURL = "https://console.anthropic.com/v1/oauth/token"
26	26
	27	+ // refreshTimeout bounds a token exchange. The harness's own HTTP client allows
	28	+ // ten minutes, which is right for a model response and badly wrong here: the
	29	+ // refresh runs under a lock every other request is queued behind.
	30	+ const refreshTimeout = 30 * time.Second
	31	+
	32	+ // maxErrorBody is how much of a failed response is quoted back. Unbounded, a
	33	+ // wrong endpoint returning a large page becomes the error message; and the
	34	+ // body of a token-endpoint error is a place a submitted credential could
	35	+ // plausibly be echoed, so it is truncated as well as capped.
	36	+ const maxErrorBody = 512
	37	+
27	38	// tokenURL is the endpoint a refresh is sent to.
28	39	func tokenURL() string {
29	40	if u := os.Getenv(tokenURLEnv); u != "" {
@@ -33,20 +44,24 @@ func tokenURL() string {
33	44	}
34	45
35	46	// ErrExpired means the credential has lapsed and cannot be renewed here —
36		- // either the refresh token is gone or itself expired. Only logging in again
37		- // fixes it, and the tool that owns the credential is the one to do it.
	47	+ // the refresh token is missing, itself expired, or was refused. Only logging
	48	+ // in again fixes it, and the tool that owns the credential is the one to do it.
38	49	var ErrExpired = errors.New("claudecode: your Claude Code login has expired, run `claude` to refresh it")
39	50
40	51	// Refresh exchanges the refresh token for a new access token. It returns new
41	52	// credentials and writes nothing — persisting is a separate decision, made by
42	53	// Source, because whether it is safe depends on whether the server rotated.
43	54	//
44		- // Every failure says to run `claude`, including a 429. On these OAuth surfaces
45		- // a 429 is not reliable evidence of a rate limit: the Messages API answers a
	55	+ // A refusal wraps ErrExpired, so callers can classify it with errors.Is rather
	56	+ // than by reading status codes. That includes a 429: on these OAuth surfaces a
	57	+ // 429 is not reliable evidence of a rate limit — the Messages API answers a
46	58	// request shape it declines the same way, and every probe of this endpoint has
47		- // returned one without ever completing an exchange. Advising someone to wait
48		- // would be advice that may never come good.
	59	+ // returned one without ever completing an exchange — so advising someone to
	60	+ // wait would be advice that may never come good.
49	61	func Refresh(ctx context.Context, h http.Client, c Credentials) (*Credentials, error) {
	62	+ ctx, cancel := context.WithTimeout(ctx, refreshTimeout)
	63	+ defer cancel()
	64	+
50	65	body, err := json.Marshal(map[string]string{
51	66	"grant_type": "refresh_token",
52	67	"refresh_token": c.RefreshToken,
@@ -66,13 +81,12 @@ func Refresh(ctx context.Context, h http.Client, c Credentials) (*Credentials,
66	81	return nil, fmt.Errorf("claudecode: refreshing: %w", err)
67	82	}
68	83	defer resp.Body.Close()
69		- raw, err := io.ReadAll(resp.Body)
	84	+ raw, err := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody+1))
70	85	if err != nil {
71	86	return nil, fmt.Errorf("claudecode: reading refresh response: %w", err)
72	87	}
73	88	if resp.StatusCode != http.StatusOK {
74		- return nil, fmt.Errorf("claudecode: refresh refused (%d), run `claude` to log in again: %s",
75		- resp.StatusCode, bytes.TrimSpace(raw))
	89	+ return nil, fmt.Errorf("%w (refresh refused: %d %s)", ErrExpired, resp.StatusCode, clip(raw))
76	90	}
77	91
78	92	var out struct {
@@ -81,35 +95,65 @@ func Refresh(ctx context.Context, h http.Client, c Credentials) (*Credentials,
81	95	ExpiresIn int64 `json:"expires_in"`
82	96	}
83	97	if err := json.Unmarshal(raw, &out); err != nil {
84		- return nil, fmt.Errorf("claudecode: decoding refresh response: %w", err)
	98	+ // A 200 carrying HTML — a captive portal, a proxy interstitial — lands
	99	+ // here, so say what arrived rather than only that it would not parse.
	100	+ return nil, fmt.Errorf("claudecode: refresh returned something that is not a token response (%s): %w", clip(raw), err)
85	101	}
86	102	if out.AccessToken == "" {
87	103	return nil, errors.New("claudecode: refresh returned no access token")
88	104	}
	105	+ // Without a lifetime the new token would be born expired — Expired() would
	106	+ // report true immediately and every single request would refresh again,
	107	+ // hammering the endpoint on a loop of our own making.
	108	+ if out.ExpiresIn <= 0 {
	109	+ return nil, fmt.Errorf("claudecode: refresh returned no usable expires_in (%d)", out.ExpiresIn)
	110	+ }
89	111
90	112	next := *c
91	113	next.AccessToken = out.AccessToken
	114	+ next.ExpiresAt = time.Now().Add(time.Duration(out.ExpiresIn) * time.Second)
92	115	// An omitted refresh_token means the old one is still good. Blanking it
93	116	// would throw away the only thing that can refresh again.
94		- if out.RefreshToken != "" {
	117	+ if out.RefreshToken != "" && out.RefreshToken != c.RefreshToken {
95	118	next.RefreshToken = out.RefreshToken
	119	+ // The response says nothing about how long the NEW refresh token
	120	+ // lives, and carrying the old one's expiry forward would eventually
	121	+ // make a perfectly good token look dead. Unknown is the truth.
	122	+ next.RefreshExpiresAt = time.Time{}
96	123	}
97		- next.ExpiresAt = time.Now().Add(time.Duration(out.ExpiresIn) * time.Second)
98	124	return &next, nil
99	125	}
100	126
101		- // Save writes credentials back to the store.
	127	+ // clip bounds untrusted response text before it reaches an error message.
	128	+ func clip(b []byte) string {
	129	+ b = bytes.TrimSpace(b)
	130	+ if len(b) > maxErrorBody {
	131	+ return string(b[:maxErrorBody]) + "…"
	132	+ }
	133	+ return string(b)
	134	+ }
	135	+
	136	+ // ErrStoreChanged means the credential store no longer holds the refresh token
	137	+ // we refreshed from: something else — Claude Code itself, most likely — logged
	138	+ // in while we were running. Its credential is newer than ours, so we leave it
	139	+ // alone rather than overwrite a fresh login with our older chain.
	140	+ var ErrStoreChanged = errors.New("claudecode: credential store changed underneath us")
	141	+
	142	+ // Save writes credentials back to the store, provided the refresh token on
	143	+ // disk is still expectRefresh. Passing "" skips that check.
102	144	//
103	145	// This rewrites a file Claude Code owns, so it is a read-modify-write: the
104		- // document is parsed, only the three fields we came to change are replaced,
105		- // and everything else — keys this package does not model, keys it has never
106		- // heard of — goes back exactly as it was found. Numbers are decoded as literals
	146	+ // document is parsed, only the fields we came to change are replaced, and
	147	+ // everything else — keys this package does not model, keys it has never heard
	148	+ // of — goes back exactly as it was found. Numbers are decoded as literals
107	149	// rather than float64s, or the millisecond timestamps would come back as
108	150	// 1.78966826946e+12 and Claude Code would be reading a file we corrupted.
109	151	//
110		- // The write is atomic: a temp file in the same directory, then a rename, so a
111		- // crash midway leaves the original rather than a truncated credential.
112		- func Save(home string, c *Credentials) error {
	152	+ // The write is a temp file in the same directory, fsynced, then renamed, and
	153	+ // the directory is fsynced after. That survives both a process crash and a
	154	+ // machine losing power: the rename is atomic, and the sync before it is what
	155	+ // stops the rename from publishing a file whose contents have not landed yet.
	156	+ func Save(home string, c *Credentials, expectRefresh string) error {
113	157	path := filepath.Join(home, Filename)
114	158	raw, err := os.ReadFile(path)
115	159	if err != nil {
@@ -133,9 +177,29 @@ func Save(home string, c *Credentials) error {
133	177	return fmt.Errorf("claudecode: parsing claudeAiOauth in %s: %w", path, err)
134	178	}
135	179
	180	+ // Compare-and-swap against the owner of this file. Re-reading is not
	181	+ // enough on its own: between our Load at startup and this write, the user
	182	+ // may have run `claude` and logged in afresh, and that credential is
	183	+ // strictly better than ours.
	184	+ if expectRefresh != "" {
	185	+ if cur, _ := oauth["refreshToken"].(string); cur != expectRefresh {
	186	+ return ErrStoreChanged
	187	+ }
	188	+ }
	189	+
136	190	oauth["accessToken"] = c.AccessToken
137	191	oauth["refreshToken"] = c.RefreshToken
138	192	oauth["expiresAt"] = json.Number(strconv.FormatInt(c.ExpiresAt.UnixMilli(), 10))
	193	+ // A rotated refresh token has an unknown lifetime: the exchange does not
	194	+ // say. In memory that is the zero time, but the stale key is left in place
	195	+ // here rather than deleted. Deleting would be more honest and is the wrong
	196	+ // trade — Claude Code reads this file and we cannot see what it does with
	197	+ // a missing key, whereas a too-early expiry only makes it renew or ask for
	198	+ // a login sooner than it needed to. Between annoying a tool we do not own
	199	+ // and possibly breaking it, annoying wins.
	200	+ if !c.RefreshExpiresAt.IsZero() {
	201	+ oauth["refreshTokenExpiresAt"] = json.Number(strconv.FormatInt(c.RefreshExpiresAt.UnixMilli(), 10))
	202	+ }
139	203
140	204	if blob, err = json.Marshal(oauth); err != nil {
141	205	return fmt.Errorf("claudecode: encoding claudeAiOauth: %w", err)
@@ -145,26 +209,49 @@ func Save(home string, c *Credentials) error {
145	209	if err != nil {
146	210	return fmt.Errorf("claudecode: encoding %s: %w", path, err)
147	211	}
	212	+ return writeAtomic(path, out)
	213	+ }
148	214
149		- tmp, err := os.CreateTemp(filepath.Dir(path), ".credentials-*.tmp")
	215	+ // writeAtomic replaces path with data, or leaves it untouched.
	216	+ func writeAtomic(path string, data []byte) error {
	217	+ dir := filepath.Dir(path)
	218	+ tmp, err := os.CreateTemp(dir, ".credentials-*.tmp")
150	219	if err != nil {
151	220	return fmt.Errorf("claudecode: writing %s: %w", path, err)
152	221	}
153		- defer os.Remove(tmp.Name())
	222	+ name := tmp.Name()
	223	+ // Removing by the captured name, and only while the temp file still
	224	+ // exists under it: after a successful rename this path is gone, and
	225	+ // os.Remove on a missing file is a no-op rather than a hazard.
	226	+ defer os.Remove(name)
	227	+
154	228	if err := tmp.Chmod(0o600); err != nil {
155	229	tmp.Close()
156		- return fmt.Errorf("claudecode: securing %s: %w", tmp.Name(), err)
	230	+ return fmt.Errorf("claudecode: securing %s: %w", name, err)
	231	+ }
	232	+ if _, err := tmp.Write(data); err != nil {
	233	+ tmp.Close()
	234	+ return fmt.Errorf("claudecode: writing %s: %w", name, err)
157	235	}
158		- if _, err := tmp.Write(out); err != nil {
	236	+ // Without this the rename can be durable while the contents are not, and
	237	+ // a machine that loses power here comes back to an empty credential file.
	238	+ if err := tmp.Sync(); err != nil {
159	239	tmp.Close()
160		- return fmt.Errorf("claudecode: writing %s: %w", tmp.Name(), err)
	240	+ return fmt.Errorf("claudecode: syncing %s: %w", name, err)
161	241	}
162	242	if err := tmp.Close(); err != nil {
163		- return fmt.Errorf("claudecode: closing %s: %w", tmp.Name(), err)
	243	+ return fmt.Errorf("claudecode: closing %s: %w", name, err)
164	244	}
165		- if err := os.Rename(tmp.Name(), path); err != nil {
	245	+ if err := os.Rename(name, path); err != nil {
166	246	return fmt.Errorf("claudecode: replacing %s: %w", path, err)
167	247	}
	248	+ // Fsync the directory so the rename itself survives a power loss. Failing
	249	+ // here means the new file is written and probably linked; reporting it as
	250	+ // a save failure would be more alarming than useful.
	251	+ if d, err := os.Open(dir); err == nil {
	252	+ d.Sync()
	253	+ d.Close()
	254	+ }
168	255	return nil
169	256	}
170	257
@@ -177,6 +264,13 @@ type Source struct {
177	264	// home is where a rotated credential is written back. Empty disables
178	265	// write-back entirely.
179	266	home string
	267	+
	268	+ // OnWriteBackFailure is called when the server rotated the refresh token
	269	+ // but the new one could not be stored. That combination is the one this
	270	+ // whole design exists to avoid — the token on disk is dead and its
	271	+ // replacement is about to be lost when this process exits — so it must not
	272	+ // pass silently. Leave it nil only if something else is watching.
	273	+ OnWriteBackFailure func(error)
180	274	}
181	275
182	276	// NewSource wraps credentials in a token source. h is the client refreshes are
@@ -184,7 +278,7 @@ type Source struct {
184	278	// refresh without ever writing.
185	279	func NewSource(c Credentials, h http.Client, home string) *Source {
186	280	if h == nil {
187		- h = http.DefaultClient
	281	+ h = &http.Client{Timeout: refreshTimeout}
188	282	}
189	283	return &Source{creds: c, http: h, home: home}
190	284	}
@@ -200,7 +294,9 @@ func NewSource(c Credentials, h http.Client, home string) *Source {
200	294	// than routine.
201	295	//
202	296	// The lock means concurrent requests — streaming and buffered can be in flight
203		- // together — produce one refresh rather than a thundering herd.
	297	+ // together — produce one refresh rather than a thundering herd. Refresh bounds
	298	+ // its own time, so the wait behind that lock is seconds, not the harness
	299	+ // client's ten-minute model-response budget.
204	300	func (s *Source) Token(ctx context.Context) (string, error) {
205	301	s.mu.Lock()
206	302	defer s.mu.Unlock()
@@ -213,19 +309,22 @@ func (s *Source) Token(ctx context.Context) (string, error) {
213	309	return "", ErrExpired
214	310	}
215	311
	312	+ from := s.creds.RefreshToken
216	313	next, err := Refresh(ctx, s.http, s.creds)
217	314	if err != nil {
218	315	return "", err
219	316	}
220		- rotated := next.RefreshToken != s.creds.RefreshToken
	317	+ rotated := next.RefreshToken != from
221	318	s.creds = next
222	319
223	320	if rotated && s.home != "" {
224		- // Best effort. A store we cannot write is a worse next session, not a
225		- // failed this one — we are holding a token that works, and refusing to
226		- // use it would be a strictly worse outcome than the stale file.
227		- if err := Save(s.home, s.creds); err != nil {
228		- return s.creds.AccessToken, nil
	321	+ // Best effort on the request, but never silent: we hold a token that
	322	+ // works, and failing the request would be strictly worse than a stale
	323	+ // file. What would be worse still is saying nothing, because the
	324	+ // user's Claude Code login is now broken in a way only this message
	325	+ // explains.
	326	+ if err := Save(s.home, s.creds, from); err != nil && s.OnWriteBackFailure != nil {
	327	+ s.OnWriteBackFailure(err)
229	328	}
230	329	}
231	330	return s.creds.AccessToken, nil

+242−2internal/claudecode/refresh_test.goView File

+  	}
   	c.AccessToken, c.RefreshToken = "new-access", "new-refresh"
   	c.ExpiresAt = time.UnixMilli(1999999999000)
+- 	if err := Save(home, c); err != nil {
++ 	if err := Save(home, c, ""); err != nil {
   		t.Fatalf("Save: %v", err)
+  	}
   	if err != nil {
   		t.Fatal(err)
+  	}
+- 	if err := Save(home, c); err != nil {
++ 	// Loosen it first, or this test cannot tell "Save sets 0600" from
++ 	// "Save did nothing at all".
++ 	if err := os.Chmod(filepath.Join(home, Filename), 0o644); err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	if err := Save(home, c, ""); err != nil {
   		t.Fatalf("Save: %v", err)
+  	}
   	info, err := os.Stat(filepath.Join(home, Filename))
   		t.Errorf("Token = %q, want fresh", tok)
+  	}
+  }
++ // A 200 with no usable lifetime would mint a token that is already expired,
++ // so the very next request refreshes again — an endless loop of our own making
++ // against an endpoint we already know answers 429.
++ func TestRefreshWithoutExpiresInIsAnError(t *testing.T) {
++ 	var hits int
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		hits++
++ 		io.WriteString(w, `{"access_token":"new-access","refresh_token":"r1"}`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	if _, err := Refresh(context.Background(), srv.Client(), &Credentials{RefreshToken: "r1"}); err == nil {
++ 		t.Fatal("a response with no expires_in must be an error, not a token born expired")
++ 	}
++ 	// And the Source must not spin on it.
++ 	s := NewSource(liveCreds(), srv.Client(), "")
++ 	for i := 0; i < 3; i++ {
++ 		if _, err := s.Token(context.Background()); err == nil {
++ 			t.Fatal("Token should surface the malformed refresh")
++ 		}
++ 	}
++ 	if hits > 4 {
++ 		t.Errorf("sent %d requests — a malformed response should not become a retry loop", hits)
++ 	}
++ }
++ // A captive portal or proxy interstitial arrives as a 200 full of HTML.
++ func TestRefreshNonJSONResponseSaysWhatArrived(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		io.WriteString(w, `<html><body>Attention Required! Cloudflare</body></html>`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	_, err := Refresh(context.Background(), srv.Client(), &Credentials{RefreshToken: "r"})
++ 	if err == nil {
++ 		t.Fatal("HTML is not a token response")
++ 	}
++ 	if !strings.Contains(err.Error(), "Cloudflare") {
++ 		t.Errorf("the error should quote what actually arrived, got %q", err)
++ 	}
++ }
++ // A refusal is a terminal expiry, so callers can classify it without reading
++ // status codes back out of a string.
++ func TestRefreshRefusalIsErrExpired(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		w.WriteHeader(http.StatusBadRequest)
++ 		io.WriteString(w, `{"error":"invalid_grant"}`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	_, err := Refresh(context.Background(), srv.Client(), &Credentials{RefreshToken: "dead"})
++ 	if !errors.Is(err, ErrExpired) {
++ 		t.Fatalf("err = %v, want it to wrap ErrExpired", err)
++ 	}
++ }
++ // An enormous error body must not become the error message, and must not carry
++ // a credential back to the screen wholesale.
++ func TestRefreshErrorBodyIsBounded(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		w.WriteHeader(http.StatusBadRequest)
++ 		io.WriteString(w, strings.Repeat("A", 100_000))
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	_, err := Refresh(context.Background(), srv.Client(), &Credentials{RefreshToken: "r"})
++ 	if err == nil {
++ 		t.Fatal("expected an error")
++ 	}
++ 	if len(err.Error()) > 2*maxErrorBody {
++ 		t.Errorf("error is %d bytes; the response body was not bounded", len(err.Error()))
++ 	}
++ }
++ // Rotation gives us a token whose lifetime the response never states. Carrying
++ // the old one's expiry forward would eventually refuse to refresh a perfectly
++ // good token.
++ func TestRotationClearsTheRefreshExpiry(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		io.WriteString(w, `{"access_token":"a","refresh_token":"r2","expires_in":28800}`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	got, err := Refresh(context.Background(), srv.Client(), liveCreds())
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	if !got.RefreshExpiresAt.IsZero() {
++ 		t.Errorf("RefreshExpiresAt = %s, want zero (unknown) after rotation", got.RefreshExpiresAt)
++ 	}
++ 	// Unknown must not read as expired, or the next lapse refuses to refresh.
++ 	if got.RefreshExpired(time.Now()) {
++ 		t.Error("an unknown refresh expiry must not count as expired")
++ 	}
++ }
++ // If the user runs `claude` and logs in afresh while we are running, their new
++ // credential is strictly better than ours. Overwriting it would undo a login
++ // they just performed.
++ func TestSaveDeclinesWhenTheStoreMovedOn(t *testing.T) {
++ 	home := storeWith(t, "somebody-elses-newer-token")
++ 	c, err := Load(home)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	before := mustRaw(t, home)
++ 	c.AccessToken = "ours"
++ 	if err := Save(home, c, "the-one-we-refreshed-from"); !errors.Is(err, ErrStoreChanged) {
++ 		t.Fatalf("err = %v, want ErrStoreChanged", err)
++ 	}
++ 	if got := mustRaw(t, home); string(got) != string(before) {
++ 		t.Error("Save wrote despite the store having changed underneath it")
++ 	}
++ }
++ // A write that cannot happen must leave the original readable, not truncated.
++ func TestSaveFailureLeavesTheOriginalIntact(t *testing.T) {
++ 	home := storeWith(t, "r1")
++ 	c, err := Load(home)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	before := mustRaw(t, home)
++ 	// A read-only directory: the temp file cannot be created.
++ 	dir := filepath.Join(home, ".claude")
++ 	if err := os.Chmod(dir, 0o500); err != nil {
++ 		t.Skip("cannot make the directory read-only here")
++ 	}
++ 	t.Cleanup(func() { os.Chmod(dir, 0o700) })
++ 	c.AccessToken = "new"
++ 	if err := Save(home, c, ""); err == nil {
++ 		t.Skip("the directory was writable after all (running as root?)")
++ 	}
++ 	if got := mustRaw(t, home); string(got) != string(before) {
++ 		t.Errorf("a failed Save damaged the original:\n%s", got)
++ 	}
++ }
++ // The scenario the whole design exists to prevent: the server rotated, we
++ // cannot store the replacement, and the token on disk is now dead. The request
++ // still succeeds — but it must not do so silently.
++ func TestRotatedButUnstorableIsReported(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		io.WriteString(w, `{"access_token":"fresh","refresh_token":"r2-rotated","expires_in":28800}`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	c := liveCreds()
++ 	s := NewSource(c, srv.Client(), t.TempDir()) // a home with no store in it
++ 	var reported error
++ 	s.OnWriteBackFailure = func(err error) { reported = err }
++ 	tok, err := s.Token(context.Background())
++ 	if err != nil {
++ 		t.Fatalf("the request must still succeed: %v", err)
++ 	}
++ 	if tok != "fresh" {
++ 		t.Errorf("Token = %q, want fresh", tok)
++ 	}
++ 	if reported == nil {
++ 		t.Fatal("a rotation that could not be stored must be reported, never swallowed")
++ 	}
++ }
++ // No rotation, no write, and therefore nothing to report.
++ func TestNoRotationReportsNothing(t *testing.T) {
++ 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
++ 		io.WriteString(w, `{"access_token":"fresh","refresh_token":"r1","expires_in":28800}`)
++ 	}))
++ 	defer srv.Close()
++ 	t.Setenv(tokenURLEnv, srv.URL)
++ 	s := NewSource(liveCreds(), srv.Client(), t.TempDir())
++ 	var called bool
++ 	s.OnWriteBackFailure = func(error) { called = true }
++ 	if _, err := s.Token(context.Background()); err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	if called {
++ 		t.Error("nothing was rotated, so nothing should have been written or reported")
++ 	}
++ }
++ // A store with no refreshTokenExpiresAt must not read as permanently expired:
++ // the package doc admits the schema can drift, and drift should not silently
++ // turn refresh into a false "your login has expired".
++ func TestMissingRefreshExpiryIsUnknownNotExpired(t *testing.T) {
++ 	home := t.TempDir()
++ 	if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o700); err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	body := `{"claudeAiOauth":{"accessToken":"a","refreshToken":"r","expiresAt":1789668269460}}`
++ 	if err := os.WriteFile(filepath.Join(home, Filename), []byte(body), 0o600); err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	c, err := Load(home)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	if c.RefreshExpired(time.Now()) {
++ 		t.Error("an absent refreshTokenExpiresAt must mean unknown, not expired")
++ 	}
++ }
++ // Write-back must not remove a key from a file Claude Code reads, even one we
++ // can no longer describe accurately after a rotation.
++ func TestSaveNeverDeletesTheRefreshExpiryKey(t *testing.T) {
++ 	home := storeWith(t, "r1")
++ 	c, err := Load(home)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	c.RefreshToken = "r2-rotated"
++ 	c.RefreshExpiresAt = time.Time{} // unknown, as after a rotation
++ 	if err := Save(home, c, "r1"); err != nil {
++ 		t.Fatalf("Save: %v", err)
++ 	}
++ 	oauth := readStore(t, home)["claudeAiOauth"].(map[string]any)
++ 	if _, ok := oauth["refreshTokenExpiresAt"]; !ok {
++ 		t.Error("refreshTokenExpiresAt was deleted; a key we did not add is not ours to remove")
++ 	}
++ 	if oauth["refreshToken"] != "r2-rotated" {
++ 		t.Errorf("refreshToken = %v, want the rotated one", oauth["refreshToken"])
++ 	}
++ }