mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-01 05:03:34 +00:00
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
This commit is contained in:
@@ -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 = {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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<void>;
|
||||
}): Promise<string | undefined> {
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user