Greptile Summary

This PR adds a clearStaleProfileLockouts() helper to modelsAuthLoginCommand that resets auth_permanent/billing disabled state for all profiles belonging to the target provider before the new OAuth flow runs. The intent is correct and the call-order test is solid, but two logic issues in the helper need attention before this can safely ship.

• No error handling in clearStaleProfileLockouts — Any I/O error, stale lock file, or corrupt store will surface as an unhandled rejection and abort modelsAuthLoginCommand entirely. Since this is housekeeping, it should be wrapped in a try/catch so that a clearing failure is logged as a warning and never blocks re-authentication.
• Wrong store loaded — agent-scoped profiles can be silently skipped — loadAuthProfileStore() always loads from the global default path (ignoring agentDir). If the configured default agent keeps its own auth-profiles.json, listProfilesForProvider will search the wrong file and find nothing. loadAuthProfileStoreForRuntime(agentDir) (already exported from auth-profiles.ts) merges both the agent-specific and main stores and is the correct call here.
• The gpt-5.3-codex → gpt-5.4 model name fixes in pre-existing tests are correct housekeeping.

Confidence Score: 2/5

• Not safe to merge as-is — the helper can block login on error and may silently skip agent-scoped profiles
• The fix addresses a real user-facing bug but introduces two regressions in the implementation: missing error handling that turns a background cleanup step into a hard failure gate, and a store-loading mismatch that means the fix may be a no-op for users with agent-specific auth stores. Both issues are in the same new function and straightforward to fix, but until they are resolved the PR could make the lockout situation worse (user can't log in at all if cleanup throws) or silently ineffective (wrong store searched).
• src/commands/models/auth.ts — specifically the new clearStaleProfileLockouts function

_{Last reviewed commit: cc5f6ff}

Re: Aisle (CWE-841 lockout bypass)

By design. A user explicitly running models auth login is manually re-authenticating - clearing stale lockout state is the whole point. The lockout exists to prevent automated retry storms, not to block manual re-auth. Even if the user aborts mid-flow, the lockout was already stale (the original server-side token revocation already happened), so clearing it reflects reality.

Re: Greptile (2/5, two findings)

Both findings addressed in 88b3fb1: wrapped in try/catch so corrupt store files can't block re-auth, and switched to loadAuthProfileStoreForRuntime(agentDir) for correct agent-scoped store resolution. Score was assigned before the fixes landed.

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

1. 🔵 Potential local DoS via unbounded per-profile disk I/O when clearing auth lockouts on login

Description

models auth login now performs best-effort housekeeping by clearing cooldown/disabled state for all profiles matching a provider before initiating login.

While this is reasonable for normal installations (few profiles), the implementation can become very slow when the auth store contains many profiles (e.g., a corrupted/hand-edited auth-profiles.json, or a shared environment where another local actor can add many entries):

• clearStaleProfileLockouts() enumerates all profile IDs for the provider and clears them sequentially.
• Each clearAuthProfileCooldown() call can acquire a file lock and perform disk read/write (updateAuthProfileStoreWithLock → saveAuthProfileStore → saveJsonFile).
• Result: O(N) lock acquisitions and potentially O(N) full-file writes on the login path, enabling resource exhaustion / severe slowdown of models auth login.

Vulnerable code (login path loop):

const profileIds = listProfilesForProvider(store, provider);
for (const profileId of profileIds) {
  await clearAuthProfileCooldown({ store, profileId, agentDir });
}

Supporting sink (per-profile locked update + save):

const updated = await updateAuthProfileStoreWithLock({ agentDir, updater: ... });
...
saveAuthProfileStore(store, agentDir);

Impact:

• A local attacker (or poisoned state in shared CI runners) can create thousands of profiles for a provider and cause models auth login to take minutes or time out due to repeated lock retries and file writes.
• The broad catch {} hides failures and may mask pathological slowness until the UX is already degraded.

Recommendation

Mitigate worst-case cost by clearing provider lockouts in one locked transaction (single read/lock + single write), and/or bounding work:

1. Add a bulk clear helper that runs under a single updateAuthProfileStoreWithLock and clears all matching entries in-memory, then saves once.
2. Apply a hard cap / warning (e.g., stop after 100 profiles) to avoid pathological runtime.
3. Avoid repeated lock acquisition inside a loop.

Example (single lock + single save):

import { updateAuthProfileStoreWithLock } from "../../agents/auth-profiles/store.js";
import { resetUsageStats } from "../../agents/auth-profiles/usage.js";

await updateAuthProfileStoreWithLock({
  agentDir,
  updater: (freshStore) => {
    const providerKey = normalizeProviderIdForAuth(provider);
    let changed = false;
    for (const [profileId, cred] of Object.entries(freshStore.profiles)) {
      if (normalizeProviderIdForAuth(cred.provider) !== providerKey) continue;
      if (!freshStore.usageStats?.[profileId]) continue;
      updateUsageStatsEntry(freshStore, profileId, (e) => resetUsageStats(e));
      changed = true;
    }
    return changed;
  },
});

This reduces the operation to O(N) in-memory updates with a single lock and write, preventing large N from causing excessive I/O and lock contention.

2. 🔵 Prototype pollution via proto/constructor keys in auth profile store JSON parsing

Description

The auth profile store loader copies user-controlled JSON object keys directly into plain JavaScript objects. Special keys like __proto__, constructor, or prototype can mutate the target object's prototype (prototype pollution) when assigned on a normal {} object.

This occurs while coercing auth-profiles.json (and legacy auth.json) into in-memory stores:

• coerceAuthStore() assigns normalized[key] = parsed.credential for every entry in record.profiles
• coerceLegacyStore() assigns entries[key] = parsed.credential for every entry in the legacy object
• coerceAuthStore() also builds order with acc[provider] = list

If an attacker can cause a crafted auth-profiles.json to be loaded (e.g., by writing it into the agent directory), these assignments can pollute prototypes and lead to unexpected behavior in subsequent authorization/selection logic that consumes store.profiles, store.order, etc.

Vulnerable code:

const normalized: Record<string, AuthProfileCredential> = {};
for (const [key, value] of Object.entries(profiles)) {
  ...
  normalized[key] = parsed.credential;
}

Recommendation

Harden JSON-to-object coercion against prototype pollution.

Options:

1. Use Object.create(null) for map-like objects so special keys are not magic:

const normalized: Record<string, AuthProfileCredential> = Object.create(null);
for (const [key, value] of Object.entries(profiles)) {
  if (key === "__proto__" || key === "constructor" || key === "prototype") continue;
  ...
  normalized[key] = parsed.credential;
}

2. Prefer Map<string, T> for untrusted-key dictionaries and only materialize to a safe object when needed.

Also apply the same hardening to other dictionaries derived from JSON (order, lastGood, usageStats, legacy store entries).

Analyzed PR: #43450 at commit b7c3bb8

_{Last updated on: 2026-03-14T07:18:27Z}

Re: Aisle CWE-307 (pre-auth lockout reset)

The lockout is already stale by design here. When a user explicitly runs `openclaw models auth login`, they're telling us their credentials need fixing. The lockout state they're bypassing was set because those same credentials failed - keeping it around while they're actively trying to fix it doesn't add protection, it blocks recovery.

If the user cancels the OAuth flow mid-way, the worst case is: lockout state is cleared but credentials are still broken, so the next API call fails and the lockout gets re-applied immediately. The circuit breaker re-engages on the next failure.

Moving the clear to post-auth would mean a user with a corrupted `disabledReason: "auth_permanent"` entry (the bug this PR fixes) can't even start the login flow on some provider plugin paths where the disabled check fires before the OAuth redirect.

The silent `catch {}` is intentional - lockout clearing is best-effort housekeeping. Blocking re-authentication on a lockout-clearing failure would be worse than the CWE-307 risk.

Rebased onto latest main. CHANGELOG entry verified. All code checks, extensions, and protocol pass. The node test shard and Windows shard failures are from the temp-path-guard.test.ts regression on main (da1ec45505) - same failures on every open PR in the repo, including PRs that maintainers are actively merging.