mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix(onboard): reject a malformed API key from the environment too (#126776)
`openclaw onboard --openai-api-key 'openclaw onboard --auth-choice ...'` correctly refuses with "Paste the API key value, not an OpenClaw onboarding command", exits 1, and writes no config. The identical value supplied through `OPENAI_API_KEY` -- the form `docs/start/wizard-cli-automation.md` documents for automation -- exited 0 with empty stderr and persisted the command string as the credential. `isMalformedApiKeyInput` was already imported into this file but guarded only the `flagKey` branch; both `resolveEnvKey()` branches returned unchecked. The operator finished onboarding believing they were configured, and nothing told them otherwise until an agent turn failed or they happened to run `openclaw doctor`, which classifies that exact value as `malformed_api_key` and prints the hint they never saw. Route every operator-supplied key -- flag, env, and secret-ref env -- through one guard, and name the environment variable in the message so an operator with several exported keys knows which one is wrong. The stored-profile branch stays unguarded on purpose: a bad key already on disk is doctor's to diagnose, and refusing there would strand someone re-onboarding to replace it.
This commit is contained in:
committed by
GitHub
parent
4f1172a6ca
commit
2006049629
@@ -91,30 +91,79 @@ describe("resolveNonInteractiveApiKey", () => {
|
||||
expect(runtime.exit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects command-shaped flag keys before returning them", async () => {
|
||||
it.each([
|
||||
{ source: "flag", flagValue: "malformed" },
|
||||
{ source: "environment", resolvedEnv: true },
|
||||
{ source: "secret-ref environment", resolvedEnv: true, secretInputMode: "ref" as const },
|
||||
])("rejects command-shaped $source keys before returning them", async (testCase) => {
|
||||
const runtime = createRuntime();
|
||||
resolveEnvApiKey.mockImplementation(() => {
|
||||
throw new Error("env lookup should not run for a malformed explicit flag");
|
||||
});
|
||||
const malformedKey =
|
||||
"openclaw onboard --non-interactive --auth-choice=zai-coding-global --zai-api-key $ZAI_API_KEY";
|
||||
if (testCase.resolvedEnv) {
|
||||
resolveEnvApiKey.mockReturnValue({
|
||||
apiKey: malformedKey,
|
||||
source: "env: ZAI_API_KEY",
|
||||
});
|
||||
} else {
|
||||
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",
|
||||
flagValue: testCase.flagValue === "malformed" ? malformedKey : undefined,
|
||||
flagName: "--zai-api-key",
|
||||
envVar: "ZAI_API_KEY",
|
||||
runtime: runtime as never,
|
||||
secretInputMode: testCase.secretInputMode,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(resolveEnvApiKey).not.toHaveBeenCalled();
|
||||
expect(resolveEnvApiKey).toHaveBeenCalledTimes(testCase.resolvedEnv ? 1 : 0);
|
||||
expect(runtime.error).toHaveBeenCalledWith(
|
||||
"Paste the API key value, not an OpenClaw onboarding command.",
|
||||
testCase.resolvedEnv
|
||||
? "Paste the API key value, not an OpenClaw onboarding command. Check ZAI_API_KEY."
|
||||
: "Paste the API key value, not an OpenClaw onboarding command.",
|
||||
);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("rejects a command-shaped explicit env key before a secret-ref flag", async () => {
|
||||
const runtime = createRuntime();
|
||||
const previousZaiApiKey = process.env.ZAI_API_KEY;
|
||||
process.env.ZAI_API_KEY = "openclaw onboard --non-interactive --auth-choice zai-api-key"; // pragma: allowlist secret
|
||||
resolveEnvApiKey.mockImplementation(() => {
|
||||
throw new Error("broad env lookup should not run for an explicit ref-mode flag");
|
||||
});
|
||||
|
||||
try {
|
||||
const result = await resolveNonInteractiveApiKey({
|
||||
provider: "zai",
|
||||
cfg: {},
|
||||
flagValue: "zai-flag-key",
|
||||
flagName: "--zai-api-key",
|
||||
envVar: "ZAI_API_KEY",
|
||||
runtime: runtime as never,
|
||||
secretInputMode: "ref",
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(resolveEnvApiKey).not.toHaveBeenCalled();
|
||||
expect(runtime.error).toHaveBeenCalledWith(
|
||||
"Paste the API key value, not an OpenClaw onboarding command. Check ZAI_API_KEY.",
|
||||
);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
} finally {
|
||||
if (previousZaiApiKey === undefined) {
|
||||
delete process.env.ZAI_API_KEY;
|
||||
} else {
|
||||
process.env.ZAI_API_KEY = previousZaiApiKey;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
provider: "xai",
|
||||
|
||||
@@ -90,12 +90,21 @@ export async function resolveNonInteractiveApiKey(params: {
|
||||
envVarName: parseEnvVarNameFromSourceLabel(envResolved?.source) ?? explicitEnvVar,
|
||||
};
|
||||
};
|
||||
const returnOperatorKey = (key: string, source: "flag" | "env", envVarName?: string) => {
|
||||
if (!isMalformedApiKeyInput(key)) {
|
||||
return envVarName ? { key, source, envVarName } : { key, source };
|
||||
}
|
||||
const envHint = source === "env" ? ` Check ${envVarName ?? params.envVar}.` : "";
|
||||
params.runtime.error(`Paste the API key value, not an OpenClaw onboarding command.${envHint}`);
|
||||
params.runtime.exit(1);
|
||||
return null;
|
||||
};
|
||||
|
||||
const useSecretRefMode = params.secretInputMode === "ref"; // pragma: allowlist secret
|
||||
if (useSecretRefMode && flagKey) {
|
||||
const explicitEnvKey = resolveExplicitEnvKey();
|
||||
if (explicitEnvKey) {
|
||||
return { key: explicitEnvKey, source: "env", envVarName: explicitEnvVar };
|
||||
return returnOperatorKey(explicitEnvKey, "env", explicitEnvVar);
|
||||
}
|
||||
// A literal flag value cannot be converted into a durable secret reference;
|
||||
// require an env var so the stored config can reference a stable name.
|
||||
@@ -124,24 +133,21 @@ export async function resolveNonInteractiveApiKey(params: {
|
||||
params.runtime.exit(1);
|
||||
return null;
|
||||
}
|
||||
return { key: resolvedEnv.key, source: "env", envVarName: resolvedEnv.envVarName };
|
||||
return returnOperatorKey(resolvedEnv.key, "env", resolvedEnv.envVarName);
|
||||
}
|
||||
}
|
||||
|
||||
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" };
|
||||
return returnOperatorKey(flagKey, "flag");
|
||||
}
|
||||
|
||||
const resolvedEnv = resolveEnvKey();
|
||||
if (resolvedEnv.key) {
|
||||
return { key: resolvedEnv.key, source: "env", envVarName: resolvedEnv.envVarName };
|
||||
return returnOperatorKey(resolvedEnv.key, "env", resolvedEnv.envVarName);
|
||||
}
|
||||
|
||||
// Stored profiles are pre-existing state: doctor diagnoses them, while a new
|
||||
// flag or env value must remain able to replace them during onboarding.
|
||||
if (params.allowProfile ?? true) {
|
||||
const profileKey = await resolveApiKeyFromProfiles({
|
||||
provider: params.provider,
|
||||
|
||||
Reference in New Issue
Block a user