Skip to content

Concurrent token refresh corrupts credentials.json (SaveCredentials is not atomic) #10

Description

@raykuo998

What happens

auth.SaveCredentials persists credentials with a plain os.WriteFile (open O_TRUNC, then write). There is no cross-process lock and the write is not atomic. When several ghealth processes run concurrently and the access token has expired, each refreshes independently and writes the file at the same time. The writes interleave, trailing bytes from the longer writer survive, and the file becomes invalid JSON.

Every later invocation then fails with:

authentication failed: not authenticated: stored credentials:
no stored credentials: invalid character '}' after top-level value

The CLI never recovers on its own — a valid refresh token is still sitting in the file, but it reads as "not authenticated" until the user hand-edits the file or re-runs ghealth auth login.

Repro

Any script fanning out several calls across an expiry boundary. Mine was a scheduled digest issuing 6 concurrent calls (sleep / resting-hr / exercise / steps / calories / heart-rate).

The file-level mechanism reproduces deterministically with two file descriptors — which is exactly what two processes each doing os.WriteFile hold:

const FLAGS = O_WRONLY | O_CREAT | O_TRUNC
const fdA = openSync(FILE, FLAGS, 0o600)   // A opens, truncates
const fdB = openSync(FILE, FLAGS, 0o600)   // B opens, truncates
writeSync(fdA, long);  closeSync(fdA)      // 625 bytes
writeSync(fdB, short); closeSync(fdB)      // 624 bytes, overwrites the front only

// -> file = 625 bytes = B's 624 valid bytes + A's leftover '}'

Token payloads differ in length between refreshes, so "one writer is longer than the other" is routine rather than rare.

Expected

Credential persistence survives concurrent invocations. The usual fix is an atomic write: marshal to a temp file in the same directory, fsync, then os.Rename over the target. A file lock around read-refresh-write would additionally keep two processes from both burning a rotating refresh token.

Code

  • pkg/auth/auth.go:339SaveCredentials, ends in os.WriteFile(path, data, 0600)
  • pkg/auth/auth.go:442KeyringTokenSource.Token saves after every refresh; the k.mu mutex above it is in-process only

Minor, same area

The message says "no stored credentials" when the file is present but unparseable. Separating "missing" from "corrupt" would have cut my diagnosis time a lot — a scheduled job of mine failed hourly for five days before I looked at the actual bytes.

Version: built from 9cf0274, go1.26.4, windows/arm64.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions