AntFleet

Disagreement · b7190c33-anthropic-7

upsertAuthProfile performs unlocked read-modify-write on the auth store

mismatch
repo 5149da9d·PR #2·reviewed 2 days ago

Primary finding

upsertAuthProfile performs unlocked read-modify-write on the auth store

mediumconcurrencyhigh
  • src/agents/auth-profiles/profiles.ts:50-67
`upsertAuthProfile` (sync) reads the store from disk and writes it back without any file lock, while `upsertAuthProfileWithLock` exists as the locked variant. If both variants are reachable from runtime code paths (and given oauth.ts and other files use the locked variant heavily), the sync `upsertAuthProfile` is a footgun: concurrent invocations or concurrent locked writers can lose data. At minimum it should be documented as test-only or removed.

Recommendation

Either deprecate `upsertAuthProfile` and route all callers through `upsertAuthProfileWithLock`, or document that the sync version is unsafe outside of single-process startup code.

Counterpart finding

upsertAuthProfileWithLock stores raw secrets without normalization (inconsistent with upsertAuthProfile)

mediumapi-contracthigh
  • src/agents/auth-profiles/profiles.ts:53-63
  • src/agents/auth-profiles/profiles.ts:74-80
The non-locking upsert path normalizes API keys/tokens, trimming or cleaning inputs. The locking variant writes the credential object as-is, which can persist secrets with trailing whitespace/newlines and lead to auth failures or inconsistent behavior depending on which API is used.

Recommendation

Apply the same normalization in upsertAuthProfileWithLock as in upsertAuthProfile (e.g., reuse the normalization branch or call normalizeSecretInput on key/token fields before updating the store). Consider delegating both code paths to a common helper to avoid drift.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →
← back to all disagreementsview public receiptssee unanimous findings + anatomies →
Tweet ↗