fix(auth): distinguish revoked API keys from transient auth errors (#25754)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 8f9c07a200
Co-authored-by: rrenamed <87486610+rrenamed@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
This commit is contained in:
Aleksandrs Tihenko
2026-02-26 02:47:16 +02:00
committed by GitHub
parent f312222159
commit c0026274d9
15 changed files with 247 additions and 18 deletions

View File

@@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai
- Agents/Subagents delivery: refactor subagent completion announce dispatch into an explicit queue/direct/fallback state machine, recover outbound channel-plugin resolution in cold/stale plugin-registry states across announce/message/gateway send paths, finalize cleanup bookkeeping when announce flow rejects, and treat Telegram sends without `message_id` as delivery failures (instead of false-success `"unknown"` IDs). (#26867, #25961, #26803, #25069, #26741) Thanks @SmithLabsLLC and @docaohieu2808.
- Slack/Session threads: prevent oversized parent-session inheritance from silently bricking new thread sessions, surface embedded context-overflow empty-result failures to users, and add configurable `session.parentForkMaxTokens` (default `100000`, `0` disables). (#26912) Thanks @markshields-tl.
- Models/Auth probes: map permanent auth failover reasons (`auth_permanent`, for example revoked keys) into probe auth status instead of `unknown`, so `openclaw models status --probe` reports actionable auth failures. (#25754) thanks @rrenamed.
- Security/Signal: enforce DM/group authorization before reaction-only notification enqueue so unauthorized senders can no longer inject Signal reaction system events under `dmPolicy`/`groupPolicy`; reaction notifications now require channel access checks first. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Security/Discord + Slack reactions: enforce DM policy/allowlist authorization before reaction-event system enqueue in direct messages; Discord reaction handling now also honors DM/group-DM enablement and guild `groupPolicy` channel gating to keep reaction ingress aligned with normal message preflight. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Security/Telegram reactions: enforce `dmPolicy`/`allowFrom` and group allowlist authorization on `message_reaction` events before enqueueing reaction system events, preventing unauthorized reaction-triggered input in DMs and groups; ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.

View File

@@ -114,6 +114,22 @@ describe("markAuthProfileFailure", () => {
expect(reloaded.usageStats?.["anthropic:default"]?.cooldownUntil).toBe(firstCooldownUntil);
});
});
it("disables auth_permanent failures via disabledUntil (like billing)", async () => {
await withAuthProfileStore(async ({ agentDir, store }) => {
await markAuthProfileFailure({
store,
profileId: "anthropic:default",
reason: "auth_permanent",
agentDir,
});
const stats = store.usageStats?.["anthropic:default"];
expect(typeof stats?.disabledUntil).toBe("number");
expect(stats?.disabledReason).toBe("auth_permanent");
// Should NOT set cooldownUntil (that's for transient errors)
expect(stats?.cooldownUntil).toBeUndefined();
});
});
it("resets backoff counters outside the failure window", async () => {
const agentDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-auth-"));
try {

View File

@@ -34,6 +34,7 @@ export type AuthProfileCredential = ApiKeyCredential | TokenCredential | OAuthCr
export type AuthProfileFailureReason =
| "auth"
| "auth_permanent"
| "format"
| "rate_limit"
| "billing"

View File

@@ -141,6 +141,24 @@ describe("resolveProfilesUnavailableReason", () => {
).toBe("billing");
});
it("returns auth_permanent for active permanent auth disables", () => {
const now = Date.now();
const store = makeStore({
"anthropic:default": {
disabledUntil: now + 60_000,
disabledReason: "auth_permanent",
},
});
expect(
resolveProfilesUnavailableReason({
store,
profileIds: ["anthropic:default"],
now,
}),
).toBe("auth_permanent");
});
it("uses recorded non-rate-limit failure counts for active cooldown windows", () => {
const now = Date.now();
const store = makeStore({
@@ -490,7 +508,7 @@ describe("markAuthProfileFailure — active windows do not extend on retry", ()
async function markFailureAt(params: {
store: ReturnType<typeof makeStore>;
now: number;
reason: "rate_limit" | "billing";
reason: "rate_limit" | "billing" | "auth_permanent";
}): Promise<void> {
vi.useFakeTimers();
vi.setSystemTime(params.now);
@@ -528,6 +546,18 @@ describe("markAuthProfileFailure — active windows do not extend on retry", ()
}),
readUntil: (stats: WindowStats | undefined) => stats?.disabledUntil,
},
{
label: "disabledUntil(auth_permanent)",
reason: "auth_permanent" as const,
buildUsageStats: (now: number): WindowStats => ({
disabledUntil: now + 20 * 60 * 60 * 1000,
disabledReason: "auth_permanent",
errorCount: 5,
failureCounts: { auth_permanent: 5 },
lastFailureAt: now - 60_000,
}),
readUntil: (stats: WindowStats | undefined) => stats?.disabledUntil,
},
];
for (const testCase of activeWindowCases) {
@@ -573,6 +603,19 @@ describe("markAuthProfileFailure — active windows do not extend on retry", ()
expectedUntil: (now: number) => now + 20 * 60 * 60 * 1000,
readUntil: (stats: WindowStats | undefined) => stats?.disabledUntil,
},
{
label: "disabledUntil(auth_permanent)",
reason: "auth_permanent" as const,
buildUsageStats: (now: number): WindowStats => ({
disabledUntil: now - 60_000,
disabledReason: "auth_permanent",
errorCount: 5,
failureCounts: { auth_permanent: 2 },
lastFailureAt: now - 60_000,
}),
expectedUntil: (now: number) => now + 20 * 60 * 60 * 1000,
readUntil: (stats: WindowStats | undefined) => stats?.disabledUntil,
},
];
for (const testCase of expiredWindowCases) {

View File

@@ -4,6 +4,7 @@ import { saveAuthProfileStore, updateAuthProfileStoreWithLock } from "./store.js
import type { AuthProfileFailureReason, AuthProfileStore, ProfileUsageStats } from "./types.js";
const FAILURE_REASON_PRIORITY: AuthProfileFailureReason[] = [
"auth_permanent",
"auth",
"billing",
"format",
@@ -394,8 +395,8 @@ function computeNextProfileUsageStats(params: {
lastFailureAt: params.now,
};
if (params.reason === "billing") {
const billingCount = failureCounts.billing ?? 1;
if (params.reason === "billing" || params.reason === "auth_permanent") {
const billingCount = failureCounts[params.reason] ?? 1;
const backoffMs = calculateAuthProfileBillingDisableMsWithConfig({
errorCount: billingCount,
baseMs: params.cfgResolved.billingBackoffMs,
@@ -408,7 +409,7 @@ function computeNextProfileUsageStats(params: {
now: params.now,
recomputedUntil: params.now + backoffMs,
});
updatedStats.disabledReason = "billing";
updatedStats.disabledReason = params.reason;
} else {
const backoffMs = calculateAuthProfileCooldownMs(nextErrorCount);
// Keep active cooldown windows immutable so retries within the window
@@ -424,8 +425,9 @@ function computeNextProfileUsageStats(params: {
}
/**
* Mark a profile as failed for a specific reason. Billing failures are treated
* as "disabled" (longer backoff) vs the regular cooldown window.
* Mark a profile as failed for a specific reason. Billing and permanent-auth
* failures are treated as "disabled" (longer backoff) vs the regular cooldown
* window.
*/
export async function markAuthProfileFailure(params: {
store: AuthProfileStore;

View File

@@ -4,6 +4,7 @@ import {
describeFailoverError,
isTimeoutError,
resolveFailoverReasonFromError,
resolveFailoverStatus,
} from "./failover-error.js";
describe("failover-error", () => {
@@ -69,6 +70,36 @@ describe("failover-error", () => {
expect(err?.status).toBe(400);
});
it("401/403 with generic message still returns auth (backward compat)", () => {
expect(resolveFailoverReasonFromError({ status: 401, message: "Unauthorized" })).toBe("auth");
expect(resolveFailoverReasonFromError({ status: 403, message: "Forbidden" })).toBe("auth");
});
it("401 with permanent auth message returns auth_permanent", () => {
expect(resolveFailoverReasonFromError({ status: 401, message: "invalid_api_key" })).toBe(
"auth_permanent",
);
});
it("403 with revoked key message returns auth_permanent", () => {
expect(resolveFailoverReasonFromError({ status: 403, message: "api key revoked" })).toBe(
"auth_permanent",
);
});
it("resolveFailoverStatus maps auth_permanent to 403", () => {
expect(resolveFailoverStatus("auth_permanent")).toBe(403);
});
it("coerces permanent auth error with correct reason", () => {
const err = coerceToFailoverError(
{ status: 401, message: "invalid_api_key" },
{ provider: "anthropic", model: "claude-opus-4-6" },
);
expect(err?.reason).toBe("auth_permanent");
expect(err?.provider).toBe("anthropic");
});
it("describes non-Error values consistently", () => {
const described = describeFailoverError(123);
expect(described.message).toBe("123");

View File

@@ -1,4 +1,8 @@
import { classifyFailoverReason, type FailoverReason } from "./pi-embedded-helpers.js";
import {
classifyFailoverReason,
isAuthPermanentErrorMessage,
type FailoverReason,
} from "./pi-embedded-helpers.js";
const TIMEOUT_HINT_RE =
/timeout|timed out|deadline exceeded|context deadline exceeded|stop reason:\s*abort|reason:\s*abort|unhandled stop reason:\s*abort/i;
@@ -47,6 +51,8 @@ export function resolveFailoverStatus(reason: FailoverReason): number | undefine
return 429;
case "auth":
return 401;
case "auth_permanent":
return 403;
case "timeout":
return 408;
case "format":
@@ -158,6 +164,10 @@ export function resolveFailoverReasonFromError(err: unknown): FailoverReason | n
return "rate_limit";
}
if (status === 401 || status === 403) {
const msg = getErrorMessage(err);
if (msg && isAuthPermanentErrorMessage(msg)) {
return "auth_permanent";
}
return "auth";
}
if (status === 408) {

View File

@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
import {
classifyFailoverReason,
isAuthErrorMessage,
isAuthPermanentErrorMessage,
isBillingErrorMessage,
isCloudCodeAssistFormatError,
isCloudflareOrHtmlErrorPage,
@@ -16,6 +17,39 @@ import {
parseImageSizeError,
} from "./pi-embedded-helpers.js";
describe("isAuthPermanentErrorMessage", () => {
it("matches permanent auth failure patterns", () => {
const samples = [
"invalid_api_key",
"api key revoked",
"api key deactivated",
"key has been disabled",
"key has been revoked",
"account has been deactivated",
"could not authenticate api key",
"could not validate credentials",
"API_KEY_REVOKED",
"api_key_deleted",
];
for (const sample of samples) {
expect(isAuthPermanentErrorMessage(sample)).toBe(true);
}
});
it("does not match transient auth errors", () => {
const samples = [
"unauthorized",
"invalid token",
"authentication failed",
"forbidden",
"access denied",
"token has expired",
];
for (const sample of samples) {
expect(isAuthPermanentErrorMessage(sample)).toBe(false);
}
});
});
describe("isAuthErrorMessage", () => {
it("matches credential validation errors", () => {
const samples = [
@@ -480,6 +514,12 @@ describe("classifyFailoverReason", () => {
),
).toBe("rate_limit");
});
it("classifies permanent auth errors as auth_permanent", () => {
expect(classifyFailoverReason("invalid_api_key")).toBe("auth_permanent");
expect(classifyFailoverReason("Your api key has been revoked")).toBe("auth_permanent");
expect(classifyFailoverReason("key has been disabled")).toBe("auth_permanent");
expect(classifyFailoverReason("account has been deactivated")).toBe("auth_permanent");
});
it("classifies JSON api_error internal server failures as timeout", () => {
expect(
classifyFailoverReason(

View File

@@ -16,6 +16,7 @@ export {
getApiErrorPayloadFingerprint,
isAuthAssistantError,
isAuthErrorMessage,
isAuthPermanentErrorMessage,
isModelNotFoundErrorMessage,
isBillingAssistantError,
parseApiErrorInfo,

View File

@@ -649,6 +649,14 @@ const ERROR_PATTERNS = {
"plans & billing",
"insufficient balance",
],
authPermanent: [
/api[_ ]?key[_ ]?(?:revoked|invalid|deactivated|deleted)/i,
"invalid_api_key",
"key has been disabled",
"key has been revoked",
"account has been deactivated",
/could not (?:authenticate|validate).*(?:api[_ ]?key|credentials)/i,
],
auth: [
/invalid[_ ]?api[_ ]?key/,
"incorrect api key",
@@ -755,6 +763,10 @@ export function isBillingAssistantError(msg: AssistantMessage | undefined): bool
return isBillingErrorMessage(msg.errorMessage ?? "");
}
export function isAuthPermanentErrorMessage(raw: string): boolean {
return matchesErrorPatterns(raw, ERROR_PATTERNS.authPermanent);
}
export function isAuthErrorMessage(raw: string): boolean {
return matchesErrorPatterns(raw, ERROR_PATTERNS.auth);
}
@@ -899,6 +911,9 @@ export function classifyFailoverReason(raw: string): FailoverReason | null {
if (isTimeoutErrorMessage(raw)) {
return "timeout";
}
if (isAuthPermanentErrorMessage(raw)) {
return "auth_permanent";
}
if (isAuthErrorMessage(raw)) {
return "auth";
}

View File

@@ -2,6 +2,7 @@ export type EmbeddedContextFile = { path: string; content: string };
export type FailoverReason =
| "auth"
| "auth_permanent"
| "format"
| "rate_limit"
| "billing"

View File

@@ -0,0 +1,28 @@
import { describe, expect, it } from "vitest";
import { resolveUnusableProfileHint } from "./doctor-auth.js";
describe("resolveUnusableProfileHint", () => {
it("returns billing guidance for disabled billing profiles", () => {
expect(resolveUnusableProfileHint({ kind: "disabled", reason: "billing" })).toBe(
"Top up credits (provider billing) or switch provider.",
);
});
it("returns credential guidance for permanent auth disables", () => {
expect(resolveUnusableProfileHint({ kind: "disabled", reason: "auth_permanent" })).toBe(
"Refresh or replace credentials, then retry.",
);
});
it("falls back to cooldown guidance for non-billing disable reasons", () => {
expect(resolveUnusableProfileHint({ kind: "disabled", reason: "unknown" })).toBe(
"Wait for cooldown or switch provider.",
);
});
it("returns cooldown guidance for cooldown windows", () => {
expect(resolveUnusableProfileHint({ kind: "cooldown" })).toBe(
"Wait for cooldown or switch provider.",
);
});
});

View File

@@ -206,6 +206,21 @@ type AuthIssue = {
remainingMs?: number;
};
export function resolveUnusableProfileHint(params: {
kind: "cooldown" | "disabled";
reason?: string;
}): string {
if (params.kind === "disabled") {
if (params.reason === "billing") {
return "Top up credits (provider billing) or switch provider.";
}
if (params.reason === "auth_permanent" || params.reason === "auth") {
return "Refresh or replace credentials, then retry.";
}
}
return "Wait for cooldown or switch provider.";
}
function formatAuthIssueHint(issue: AuthIssue): string | null {
if (issue.provider === "anthropic" && issue.profileId === CLAUDE_CLI_PROFILE_ID) {
return `Deprecated profile. Use ${formatCliCommand("openclaw models auth setup-token")} or ${formatCliCommand(
@@ -245,13 +260,14 @@ export async function noteAuthProfileHealth(params: {
}
const stats = store.usageStats?.[profileId];
const remaining = formatRemainingShort(until - now);
const kind =
typeof stats?.disabledUntil === "number" && now < stats.disabledUntil
? `disabled${stats.disabledReason ? `:${stats.disabledReason}` : ""}`
: "cooldown";
const hint = kind.startsWith("disabled:billing")
? "Top up credits (provider billing) or switch provider."
: "Wait for cooldown or switch provider.";
const disabledActive = typeof stats?.disabledUntil === "number" && now < stats.disabledUntil;
const kind = disabledActive
? `disabled${stats.disabledReason ? `:${stats.disabledReason}` : ""}`
: "cooldown";
const hint = resolveUnusableProfileHint({
kind: disabledActive ? "disabled" : "cooldown",
reason: stats?.disabledReason,
});
out.push(`- ${profileId}: ${kind} (${remaining})${hint ? `${hint}` : ""}`);
}
return out;

View File

@@ -0,0 +1,22 @@
import { describe, expect, it } from "vitest";
import { mapFailoverReasonToProbeStatus } from "./list.probe.js";
describe("mapFailoverReasonToProbeStatus", () => {
it("maps auth_permanent to auth", () => {
expect(mapFailoverReasonToProbeStatus("auth_permanent")).toBe("auth");
});
it("keeps existing failover reason mappings", () => {
expect(mapFailoverReasonToProbeStatus("auth")).toBe("auth");
expect(mapFailoverReasonToProbeStatus("rate_limit")).toBe("rate_limit");
expect(mapFailoverReasonToProbeStatus("billing")).toBe("billing");
expect(mapFailoverReasonToProbeStatus("timeout")).toBe("timeout");
expect(mapFailoverReasonToProbeStatus("format")).toBe("format");
});
it("falls back to unknown for unrecognized values", () => {
expect(mapFailoverReasonToProbeStatus(undefined)).toBe("unknown");
expect(mapFailoverReasonToProbeStatus(null)).toBe("unknown");
expect(mapFailoverReasonToProbeStatus("model_not_found")).toBe("unknown");
});
});

View File

@@ -82,11 +82,13 @@ export type AuthProbeOptions = {
maxTokens: number;
};
const toStatus = (reason?: string | null): AuthProbeStatus => {
export function mapFailoverReasonToProbeStatus(reason?: string | null): AuthProbeStatus {
if (!reason) {
return "unknown";
}
if (reason === "auth") {
if (reason === "auth" || reason === "auth_permanent") {
// Keep probe output backward-compatible: permanent auth failures still
// surface in the auth bucket instead of showing as unknown.
return "auth";
}
if (reason === "rate_limit") {
@@ -102,7 +104,7 @@ const toStatus = (reason?: string | null): AuthProbeStatus => {
return "format";
}
return "unknown";
};
}
function buildCandidateMap(modelCandidates: string[]): Map<string, string[]> {
const map = new Map<string, string[]>();
@@ -346,7 +348,7 @@ async function probeTarget(params: {
label: target.label,
source: target.source,
mode: target.mode,
status: toStatus(described.reason),
status: mapFailoverReasonToProbeStatus(described.reason),
error: redactSecrets(described.message),
latencyMs: Date.now() - start,
};