From fb7e10e86866dd2e76a02efbf22da8ce24ecda4b Mon Sep 17 00:00:00 2001 From: xingzhou Date: Mon, 29 Jun 2026 02:54:24 +0800 Subject: [PATCH] fix(auth): recover from malformed API-key profiles (#97520) * fix: reject malformed API-key auth profiles * fix(auth): detect onboard command API-key variants * fix(auth): reject malformed API-key flags --- src/agents/auth-health.test.ts | 23 +++++++ src/agents/auth-health.ts | 23 ++++++- .../auth-profiles/credential-state.test.ts | 17 ++++++ src/agents/auth-profiles/credential-state.ts | 14 ++++- src/agents/auth-profiles/oauth.ts | 8 ++- src/agents/model-auth.profiles.test.ts | 27 +++++++++ .../doctor-auth.profile-health.test.ts | 60 +++++++++++++++---- src/commands/doctor-auth.ts | 32 ++++++---- .../onboard-non-interactive/api-keys.test.ts | 24 ++++++++ .../onboard-non-interactive/api-keys.ts | 6 ++ src/plugins/provider-auth-input.test.ts | 31 ++++++++++ src/plugins/provider-auth-input.ts | 19 +++++- 12 files changed, 256 insertions(+), 28 deletions(-) diff --git a/src/agents/auth-health.test.ts b/src/agents/auth-health.test.ts index e396bde2c15..d3dd5810382 100644 --- a/src/agents/auth-health.test.ts +++ b/src/agents/auth-health.test.ts @@ -311,6 +311,29 @@ describe("buildAuthHealthSummary", () => { expect(statuses["google:no-refresh"]).toBe("expired"); }); + it("reports command-shaped API-key profiles as missing malformed auth", () => { + vi.spyOn(Date, "now").mockReturnValue(now); + const store = { + version: 1, + profiles: { + "zai:default": { + type: "api_key" as const, + provider: "zai", + key: "openclaw onboard --auth-choice zai-coding-global", + }, + }, + }; + + const summary = buildAuthHealthSummary({ + store, + warnAfterMs: DEFAULT_OAUTH_WARN_MS, + }); + + expect(profileStatuses(summary)["zai:default"]).toBe("missing"); + expect(profileReasonCodes(summary)["zai:default"]).toBe("malformed_api_key"); + expect(summary.providers.find((entry) => entry.provider === "zai")?.status).toBe("missing"); + }); + it("uses runtime provider credentials for profile health", () => { vi.spyOn(Date, "now").mockReturnValue(now); const store = { diff --git a/src/agents/auth-health.ts b/src/agents/auth-health.ts index 7e006e48b35..80381b966e0 100644 --- a/src/agents/auth-health.ts +++ b/src/agents/auth-health.ts @@ -142,6 +142,21 @@ function buildProfileHealth(params: { const provider = normalizeProviderId(healthCredential.provider); if (healthCredential.type === "api_key") { + const eligibility = evaluateStoredCredentialEligibility({ + credential: healthCredential, + now, + }); + if (!eligibility.eligible) { + return { + profileId, + provider, + type: "api_key", + status: "missing", + reasonCode: eligibility.reasonCode, + source, + label, + }; + } return { profileId, provider, @@ -377,7 +392,11 @@ export function buildAuthHealthSummary(params: { let earliestExpiry: number | undefined; for (const profile of effectiveProfiles) { if (profile.type === "api_key") { - hasApiKeyProfile = true; + if (profile.status === "static") { + hasApiKeyProfile = true; + } else if (profile.status === "missing") { + hasMissing = true; + } continue; } if (profile.type !== "oauth" && profile.type !== "token") { @@ -400,7 +419,7 @@ export function buildAuthHealthSummary(params: { } if (!hasExpirableProfile) { - provider.status = hasApiKeyProfile ? "static" : "missing"; + provider.status = hasMissing ? "missing" : hasApiKeyProfile ? "static" : "missing"; continue; } diff --git a/src/agents/auth-profiles/credential-state.test.ts b/src/agents/auth-profiles/credential-state.test.ts index 7e1c93e95e9..dd5dc5658a3 100644 --- a/src/agents/auth-profiles/credential-state.test.ts +++ b/src/agents/auth-profiles/credential-state.test.ts @@ -84,6 +84,23 @@ describe("evaluateStoredCredentialEligibility", () => { expect(result).toEqual({ eligible: true, reasonCode: "ok" }); }); + it.each([ + "openclaw onboard --auth-choice zai-coding-global", + "openclaw onboard --auth-choice=zai-coding-global", + "openclaw onboard --non-interactive --auth-choice zai-coding-global --zai-api-key $ZAI_API_KEY", + "openclaw onboard --non-interactive --auth-choice=zai-coding-global --zai-api-key $ZAI_API_KEY", + ])("marks pasted OpenClaw onboarding command %p as a malformed api key", (key) => { + const result = evaluateStoredCredentialEligibility({ + credential: { + type: "api_key", + provider: "zai", + key, + }, + now, + }); + expect(result).toEqual({ eligible: false, reasonCode: "malformed_api_key" }); + }); + it("marks tokenRef with missing expires as eligible", () => { const result = evaluateStoredCredentialEligibility({ credential: { diff --git a/src/agents/auth-profiles/credential-state.ts b/src/agents/auth-profiles/credential-state.ts index d0f440aa0b0..a0afa11c054 100644 --- a/src/agents/auth-profiles/credential-state.ts +++ b/src/agents/auth-profiles/credential-state.ts @@ -13,7 +13,8 @@ export type AuthCredentialReasonCode = | "missing_credential" | "invalid_expires" | "expired" - | "unresolved_ref"; + | "unresolved_ref" + | "malformed_api_key"; /** Default OAuth access-token refresh margin before expiry. */ export const DEFAULT_OAUTH_REFRESH_MARGIN_MS = 5 * 60 * 1000; @@ -82,6 +83,14 @@ function hasConfiguredSecretString(value: unknown): boolean { return normalizeSecretInputString(value) !== undefined; } +export function isMalformedApiKeyInput(value: unknown): boolean { + const normalized = normalizeSecretInputString(value); + return ( + normalized !== undefined && + /^openclaw\s+onboard(?:\s+.*)?\s+--auth-choice(?:\s|=|$)/i.test(normalized) + ); +} + /** Classifies whether a stored credential is eligible for auth selection. */ export function evaluateStoredCredentialEligibility(params: { credential: AuthProfileCredential; @@ -93,6 +102,9 @@ export function evaluateStoredCredentialEligibility(params: { if (credential.type === "api_key") { const hasKey = hasConfiguredSecretString(credential.key); const hasKeyRef = hasConfiguredSecretRef(credential.keyRef); + if (isMalformedApiKeyInput(credential.key)) { + return { eligible: false, reasonCode: "malformed_api_key" }; + } if (!hasKey && !hasKeyRef) { return { eligible: false, reasonCode: "missing_credential" }; } diff --git a/src/agents/auth-profiles/oauth.ts b/src/agents/auth-profiles/oauth.ts index 6c306d088b2..4f3d149c943 100644 --- a/src/agents/auth-profiles/oauth.ts +++ b/src/agents/auth-profiles/oauth.ts @@ -23,7 +23,10 @@ import { normalizeOptionalSecretInput } from "../../utils/normalize-secret-input import { refreshChutesTokens } from "../chutes-oauth.js"; import { resolveProviderIdForAuth } from "../provider-auth-aliases.js"; import { log } from "./constants.js"; -import { resolveTokenExpiryState } from "./credential-state.js"; +import { + evaluateStoredCredentialEligibility, + resolveTokenExpiryState, +} from "./credential-state.js"; import { formatAuthDoctorHint } from "./doctor.js"; import { readExternalCliBootstrapCredential, @@ -376,6 +379,9 @@ export async function resolveApiKeyForProfile( }); if (cred.type === "api_key") { + if (!evaluateStoredCredentialEligibility({ credential: cred }).eligible) { + return null; + } const key = await resolveProfileSecretString({ profileId, provider: cred.provider, diff --git a/src/agents/model-auth.profiles.test.ts b/src/agents/model-auth.profiles.test.ts index f8e4e7f49e3..4795be0d69a 100644 --- a/src/agents/model-auth.profiles.test.ts +++ b/src/agents/model-auth.profiles.test.ts @@ -733,6 +733,33 @@ describe("getApiKeyForModel", () => { ); }); + it("skips malformed stored ZAI command profiles and uses current env auth", async () => { + await withEnvAsync( + { + ZAI_API_KEY: "zai-current-key", // pragma: allowlist secret + Z_AI_API_KEY: undefined, + }, + async () => { + const resolved = await resolveApiKeyForProvider({ + provider: "zai", + store: { + version: 1, + profiles: { + "zai:default": { + type: "api_key", + provider: "zai", + key: "openclaw onboard --auth-choice zai-coding-global", + }, + }, + }, + }); + expect(resolved.apiKey).toBe("zai-current-key"); + expect(resolved.source).toContain("ZAI_API_KEY"); + expect(resolved.profileId).toBeUndefined(); + }, + ); + }); + it("keeps stored provider auth ahead of env by default", async () => { await withEnvAsync({ OPENAI_API_KEY: "env-openai-key" }, async () => { const resolved = await resolveApiKeyForProvider({ diff --git a/src/commands/doctor-auth.profile-health.test.ts b/src/commands/doctor-auth.profile-health.test.ts index fffb3d91040..9af5467d554 100644 --- a/src/commands/doctor-auth.profile-health.test.ts +++ b/src/commands/doctor-auth.profile-health.test.ts @@ -6,18 +6,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { DoctorPrompter } from "./doctor-prompter.js"; +type MockAuthProfileStore = { + version: number; + profiles: Record< + string, + | { type: "oauth"; provider: string; access: string; refresh: string; expires: number } + | { type: "api_key"; provider: string; key: string } + >; +}; + const authProfileMocks = vi.hoisted(() => ({ ensureAuthProfileStore: vi.fn< - ( - agentDir?: string, - options?: { allowKeychainPrompt?: boolean }, - ) => { - version: number; - profiles: Record< - string, - { type: "oauth"; provider: string; access: string; refresh: string; expires: number } - >; - } + (agentDir?: string, options?: { allowKeychainPrompt?: boolean }) => MockAuthProfileStore >(() => { throw new Error("unexpected auth profile load"); }), @@ -204,6 +204,46 @@ describe("noteAuthProfileHealth", () => { ); }); + it("prints malformed API-key profile diagnostics", async () => { + const agentDir = path.join(tempDir, "main-agent"); + writeAuthStore(agentDir); + authProfileMocks.hasAnyAuthProfileStoreSource.mockReturnValue(true); + authProfileMocks.ensureAuthProfileStore.mockImplementation( + (receivedAgentDir): MockAuthProfileStore => { + if (receivedAgentDir === agentDir) { + return { + version: 1, + profiles: { + "zai:default": { + type: "api_key", + provider: "zai", + key: "openclaw onboard --auth-choice zai-coding-global", + }, + }, + }; + } + return { version: 1, profiles: {} }; + }, + ); + + await noteAuthProfileHealth({ + cfg: { + agents: { + list: [{ id: "main", default: true, agentDir }], + }, + } as OpenClawConfig, + prompter: { + confirmAutoFix: vi.fn(async () => false), + } as unknown as DoctorPrompter, + allowKeychainPrompt: false, + }); + + expect(noteMock).toHaveBeenCalledWith( + expect.stringContaining("zai:default: missing [malformed_api_key]"), + "Model auth", + ); + }); + it("passes the target agent dir when refreshing OAuth profiles", async () => { const now = 1_700_000_000_000; vi.spyOn(Date, "now").mockReturnValue(now); diff --git a/src/commands/doctor-auth.ts b/src/commands/doctor-auth.ts index b0e9d55cd61..3aaf2fab311 100644 --- a/src/commands/doctor-auth.ts +++ b/src/commands/doctor-auth.ts @@ -234,6 +234,9 @@ async function resolveAuthIssueHint( if (issue.reasonCode === "invalid_expires") { return "Invalid token expires metadata. Set a future Unix ms timestamp or remove expires."; } + if (issue.reasonCode === "malformed_api_key") { + return "Paste the API key value, not an OpenClaw onboarding command."; + } const providerHint = await formatAuthDoctorHint({ cfg, store, @@ -307,29 +310,34 @@ async function noteAuthProfileHealthForTarget(params: { }); const findIssues = () => - summary.profiles.filter( - (profile) => + summary.profiles.filter((profile) => { + if (profile.type === "api_key") { + return profile.status === "missing"; + } + return ( (profile.type === "oauth" || profile.type === "token") && (profile.status === "expired" || profile.status === "expiring" || - profile.status === "missing"), - ); + profile.status === "missing") + ); + }); let issues = findIssues(); if (issues.length === 0) { return; } - const shouldRefresh = await params.prompter.confirmAutoFix({ - message: "Refresh expiring OAuth tokens now? (static tokens need re-auth)", - initialValue: true, - }); + const refreshTargets = issues.filter( + (issue) => issue.type === "oauth" && ["expired", "expiring", "missing"].includes(issue.status), + ); + const shouldRefresh = + refreshTargets.length > 0 && + (await params.prompter.confirmAutoFix({ + message: "Refresh expiring OAuth tokens now? (static tokens need re-auth)", + initialValue: true, + })); if (shouldRefresh) { - const refreshTargets = issues.filter( - (issue) => - issue.type === "oauth" && ["expired", "expiring", "missing"].includes(issue.status), - ); const errors: string[] = []; for (const profile of refreshTargets) { try { diff --git a/src/commands/onboard-non-interactive/api-keys.test.ts b/src/commands/onboard-non-interactive/api-keys.test.ts index d2e4a6291fd..c108b4def92 100644 --- a/src/commands/onboard-non-interactive/api-keys.test.ts +++ b/src/commands/onboard-non-interactive/api-keys.test.ts @@ -61,6 +61,30 @@ describe("resolveNonInteractiveApiKey", () => { expect(runtime.exit).not.toHaveBeenCalled(); }); + it("rejects command-shaped flag keys before returning them", async () => { + const runtime = createRuntime(); + resolveEnvApiKey.mockImplementation(() => { + throw new Error("env lookup should not run for a malformed explicit flag"); + }); + + const result = await resolveNonInteractiveApiKey({ + provider: "zai", + cfg: {}, + flagValue: + "openclaw onboard --non-interactive --auth-choice=zai-coding-global --zai-api-key $ZAI_API_KEY", + flagName: "--zai-api-key", + envVar: "ZAI_API_KEY", + runtime: runtime as never, + }); + + expect(result).toBeNull(); + expect(resolveEnvApiKey).not.toHaveBeenCalled(); + expect(runtime.error).toHaveBeenCalledWith( + "Paste the API key value, not an OpenClaw onboarding command.", + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + }); + it.each([ { provider: "xai", diff --git a/src/commands/onboard-non-interactive/api-keys.ts b/src/commands/onboard-non-interactive/api-keys.ts index cf5ea54224b..6917922ead9 100644 --- a/src/commands/onboard-non-interactive/api-keys.ts +++ b/src/commands/onboard-non-interactive/api-keys.ts @@ -9,6 +9,7 @@ import { resolveApiKeyForProfile, resolveAuthProfileOrder, } from "../../agents/auth-profiles.js"; +import { isMalformedApiKeyInput } from "../../agents/auth-profiles/credential-state.js"; import { resolveEnvApiKey } from "../../agents/model-auth.js"; import { formatCliCommand } from "../../cli/command-format.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; @@ -124,6 +125,11 @@ export async function resolveNonInteractiveApiKey(params: { } if (flagKey) { + if (isMalformedApiKeyInput(flagKey)) { + params.runtime.error("Paste the API key value, not an OpenClaw onboarding command."); + params.runtime.exit(1); + return null; + } return { key: flagKey, source: "flag" }; } diff --git a/src/plugins/provider-auth-input.test.ts b/src/plugins/provider-auth-input.test.ts index 01a175d7276..e5a5fd945bd 100644 --- a/src/plugins/provider-auth-input.test.ts +++ b/src/plugins/provider-auth-input.test.ts @@ -7,6 +7,7 @@ import { maybeApplyApiKeyFromOption, normalizeApiKeyInput, normalizeTokenProviderInput, + validateApiKeyInput, } from "./provider-auth-input.js"; const acceptAnyApiKeyInput = () => undefined; @@ -240,6 +241,19 @@ describe("normalizeApiKeyInput", () => { }); }); +describe("validateApiKeyInput", () => { + it.each([ + "openclaw onboard --auth-choice zai-coding-global", + "openclaw onboard --auth-choice=zai-coding-global", + "openclaw onboard --non-interactive --auth-choice zai-coding-global --zai-api-key $ZAI_API_KEY", + "openclaw onboard --non-interactive --auth-choice=zai-coding-global --zai-api-key $ZAI_API_KEY", + ])("rejects pasted OpenClaw onboarding command %p", (value) => { + expect(validateApiKeyInput(value)).toBe( + "Paste the API key value, not an OpenClaw onboarding command.", + ); + }); +}); + describe("maybeApplyApiKeyFromOption", () => { it.each(["demo-provider", " DeMo-PrOvIdEr "])( "stores normalized token when provider %p matches", @@ -265,6 +279,23 @@ describe("maybeApplyApiKeyFromOption", () => { expect(result).toBeUndefined(); expect(setCredential).not.toHaveBeenCalled(); }); + + it("rejects malformed command-shaped option keys before storing them", async () => { + const setCredential = vi.fn(async () => undefined); + + await expect( + maybeApplyApiKeyFromOption({ + token: + "openclaw onboard --non-interactive --auth-choice=zai-coding-global --zai-api-key $ZAI_API_KEY", + tokenProvider: "zai", + expectedProviders: ["zai"], + normalize: normalizeApiKeyInput, + validate: validateApiKeyInput, + setCredential, + }), + ).rejects.toThrow("Paste the API key value, not an OpenClaw onboarding command."); + expect(setCredential).not.toHaveBeenCalled(); + }); }); describe("ensureApiKeyFromEnvOrPrompt", () => { diff --git a/src/plugins/provider-auth-input.ts b/src/plugins/provider-auth-input.ts index 5e2175a066f..706d8d59d51 100644 --- a/src/plugins/provider-auth-input.ts +++ b/src/plugins/provider-auth-input.ts @@ -3,6 +3,7 @@ import { normalizeOptionalLowercaseString, normalizeStringifiedOptionalString, } from "@openclaw/normalization-core/string-coerce"; +import { isMalformedApiKeyInput } from "../agents/auth-profiles/credential-state.js"; import { resolveEnvApiKey } from "../agents/model-auth-env.js"; import type { OpenClawConfig } from "../config/types.js"; import type { SecretInput } from "../config/types.secrets.js"; @@ -55,8 +56,16 @@ export function normalizeApiKeyInput(raw: string): string { } /** Validates required API-key input for setup prompts. */ -export const validateApiKeyInput = (value: string) => - normalizeApiKeyInput(value).length > 0 ? undefined : "Required"; +export const validateApiKeyInput = (value: string) => { + const normalized = normalizeApiKeyInput(value); + if (!normalized) { + return "Required"; + } + if (isMalformedApiKeyInput(normalized)) { + return "Paste the API key value, not an OpenClaw onboarding command."; + } + return undefined; +}; /** Formats a redacted API-key preview for setup confirmation prompts. */ export function formatApiKeyPreview( @@ -105,6 +114,7 @@ export async function maybeApplyApiKeyFromOption(params: { secretInputMode?: SecretInputMode; expectedProviders: string[]; normalize: (value: string) => string; + validate?: (value: string) => string | undefined; setCredential: (apiKey: SecretInput, mode?: SecretInputMode) => Promise; }): Promise { const tokenProvider = normalizeTokenProviderInput(params.tokenProvider); @@ -115,6 +125,10 @@ export async function maybeApplyApiKeyFromOption(params: { return undefined; } const apiKey = params.normalize(params.token); + const validationError = params.validate?.(apiKey); + if (validationError) { + throw new Error(validationError); + } await params.setCredential(apiKey, params.secretInputMode); return apiKey; } @@ -143,6 +157,7 @@ export async function ensureApiKeyFromOptionEnvOrPrompt(params: { secretInputMode: params.secretInputMode, expectedProviders: params.expectedProviders, normalize: params.normalize, + validate: params.validate, setCredential: params.setCredential, }); if (optionApiKey) {