From ec44e262bef864b5c2e099465692b9c4e7368a5a Mon Sep 17 00:00:00 2001 From: Marcus Castro Date: Fri, 13 Feb 2026 00:25:05 -0300 Subject: [PATCH] fix(security): prevent String(undefined) coercion in credential inputs (#12287) * fix(security): prevent String(undefined) coercion in credential inputs When a prompter returns undefined (due to cancel, timeout, or bug), String(undefined).trim() produces the literal string "undefined" instead of "". This truthy string prevents secure fallbacks from triggering, allowing predictable credential values (e.g., gateway password = "undefined"). Fix all 8 occurrences by using String(value ?? "").trim(), which correctly yields "" for null/undefined inputs and triggers downstream validation or fallback logic. Fixes #8054 * fix(security): also fix String(undefined) in api-provider credential inputs Address codex review feedback: 4 additional occurrences of the unsafe String(variable).trim() pattern in auth-choice.apply.api-providers.ts (Cloudflare Account ID, Gateway ID, synthetic API key inputs + validators). * fix(test): strengthen password coercion test per review feedback * fix(security): harden credential prompt coercion --------- Co-authored-by: Peter Steinberger --- src/commands/agents.commands.add.ts | 4 +- src/commands/auth-choice.apply.anthropic.ts | 4 +- .../auth-choice.apply.api-providers.ts | 40 +++---- src/commands/auth-choice.test.ts | 102 ++++++++++++++++++ src/commands/configure.gateway.test.ts | 27 +++++ src/commands/configure.gateway.ts | 2 +- src/commands/models/auth.ts | 6 +- src/wizard/onboarding.gateway-config.test.ts | 49 +++++++++ src/wizard/onboarding.gateway-config.ts | 2 +- 9 files changed, 207 insertions(+), 29 deletions(-) diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index f090d77dc..ef6b42077 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -194,7 +194,7 @@ export async function agentsAddCommand( }, })); - const agentName = String(name).trim(); + const agentName = String(name ?? "").trim(); const agentId = normalizeAgentId(agentName); if (agentName !== agentId) { await prompter.note(`Normalized id to "${agentId}".`, "Agent id"); @@ -220,7 +220,7 @@ export async function agentsAddCommand( initialValue: workspaceDefault, validate: (value) => (value?.trim() ? undefined : "Required"), }); - const workspaceDir = resolveUserPath(String(workspaceInput).trim() || workspaceDefault); + const workspaceDir = resolveUserPath(String(workspaceInput ?? "").trim() || workspaceDefault); const agentDir = resolveAgentDir(cfg, agentId); let nextConfig = applyAgentConfig(cfg, { diff --git a/src/commands/auth-choice.apply.anthropic.ts b/src/commands/auth-choice.apply.anthropic.ts index 545efc201..6c37a0424 100644 --- a/src/commands/auth-choice.apply.anthropic.ts +++ b/src/commands/auth-choice.apply.anthropic.ts @@ -28,7 +28,7 @@ export async function applyAuthChoiceAnthropic( message: "Paste Anthropic setup-token", validate: (value) => validateAnthropicSetupToken(String(value ?? "")), }); - const token = String(tokenRaw).trim(); + const token = String(tokenRaw ?? "").trim(); const profileNameRaw = await params.prompter.text({ message: "Token name (blank = default)", @@ -87,7 +87,7 @@ export async function applyAuthChoiceAnthropic( message: "Enter Anthropic API key", validate: validateApiKeyInput, }); - await setAnthropicApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setAnthropicApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "anthropic:default", diff --git a/src/commands/auth-choice.apply.api-providers.ts b/src/commands/auth-choice.apply.api-providers.ts index 73cf6d887..b606e68a3 100644 --- a/src/commands/auth-choice.apply.api-providers.ts +++ b/src/commands/auth-choice.apply.api-providers.ts @@ -177,7 +177,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter OpenRouter API key", validate: validateApiKeyInput, }); - await setOpenrouterApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setOpenrouterApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); hasCredential = true; } @@ -242,7 +242,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter LiteLLM API key", validate: validateApiKeyInput, }); - await setLitellmApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setLitellmApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); hasCredential = true; } } @@ -296,7 +296,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Vercel AI Gateway API key", validate: validateApiKeyInput, }); - await setVercelAiGatewayApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setVercelAiGatewayApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "vercel-ai-gateway:default", @@ -329,16 +329,16 @@ export async function applyAuthChoiceApiProviders( if (!accountId) { const value = await params.prompter.text({ message: "Enter Cloudflare Account ID", - validate: (val) => (String(val).trim() ? undefined : "Account ID is required"), + validate: (val) => (String(val ?? "").trim() ? undefined : "Account ID is required"), }); - accountId = String(value).trim(); + accountId = String(value ?? "").trim(); } if (!gatewayId) { const value = await params.prompter.text({ message: "Enter Cloudflare AI Gateway ID", - validate: (val) => (String(val).trim() ? undefined : "Gateway ID is required"), + validate: (val) => (String(val ?? "").trim() ? undefined : "Gateway ID is required"), }); - gatewayId = String(value).trim(); + gatewayId = String(value ?? "").trim(); } }; @@ -381,7 +381,7 @@ export async function applyAuthChoiceApiProviders( await setCloudflareAiGatewayConfig( accountId, gatewayId, - normalizeApiKeyInput(String(key)), + normalizeApiKeyInput(String(key ?? "")), params.agentDir, ); hasCredential = true; @@ -443,7 +443,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Moonshot API key", validate: validateApiKeyInput, }); - await setMoonshotApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setMoonshotApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "moonshot:default", @@ -490,7 +490,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Moonshot API key (.cn)", validate: validateApiKeyInput, }); - await setMoonshotApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setMoonshotApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "moonshot:default", @@ -550,7 +550,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Kimi Coding API key", validate: validateApiKeyInput, }); - await setKimiCodingApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setKimiCodingApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "kimi-coding:default", @@ -598,7 +598,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Gemini API key", validate: validateApiKeyInput, }); - await setGeminiApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setGeminiApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "google:default", @@ -666,7 +666,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Z.AI API key", validate: validateApiKeyInput, }); - apiKey = normalizeApiKeyInput(String(key)); + apiKey = normalizeApiKeyInput(String(key ?? "")); await setZaiApiKey(apiKey, params.agentDir); } @@ -763,7 +763,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Xiaomi API key", validate: validateApiKeyInput, }); - await setXiaomiApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setXiaomiApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "xiaomi:default", @@ -789,13 +789,13 @@ export async function applyAuthChoiceApiProviders( if (authChoice === "synthetic-api-key") { if (params.opts?.token && params.opts?.tokenProvider === "synthetic") { - await setSyntheticApiKey(String(params.opts.token).trim(), params.agentDir); + await setSyntheticApiKey(String(params.opts.token ?? "").trim(), params.agentDir); } else { const key = await params.prompter.text({ message: "Enter Synthetic API key", validate: (value) => (value?.trim() ? undefined : "Required"), }); - await setSyntheticApiKey(String(key).trim(), params.agentDir); + await setSyntheticApiKey(String(key ?? "").trim(), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "synthetic:default", @@ -854,7 +854,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Venice AI API key", validate: validateApiKeyInput, }); - await setVeniceApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setVeniceApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "venice:default", @@ -911,7 +911,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter OpenCode Zen API key", validate: validateApiKeyInput, }); - await setOpencodeZenApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setOpencodeZenApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "opencode:default", @@ -969,7 +969,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter Together AI API key", validate: validateApiKeyInput, }); - await setTogetherApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + await setTogetherApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "together:default", @@ -1025,7 +1025,7 @@ export async function applyAuthChoiceApiProviders( message: "Enter QIANFAN API key", validate: validateApiKeyInput, }); - setQianfanApiKey(normalizeApiKeyInput(String(key)), params.agentDir); + setQianfanApiKey(normalizeApiKeyInput(String(key ?? "")), params.agentDir); } nextConfig = applyAuthProfileConfig(nextConfig, { profileId: "qianfan:default", diff --git a/src/commands/auth-choice.test.ts b/src/commands/auth-choice.test.ts index 1854e5e3a..3b313cb35 100644 --- a/src/commands/auth-choice.test.ts +++ b/src/commands/auth-choice.test.ts @@ -32,6 +32,7 @@ describe("applyAuthChoice", () => { const previousStateDir = process.env.OPENCLAW_STATE_DIR; const previousAgentDir = process.env.OPENCLAW_AGENT_DIR; const previousPiAgentDir = process.env.PI_CODING_AGENT_DIR; + const previousAnthropicKey = process.env.ANTHROPIC_API_KEY; const previousOpenrouterKey = process.env.OPENROUTER_API_KEY; const previousLitellmKey = process.env.LITELLM_API_KEY; const previousAiGatewayKey = process.env.AI_GATEWAY_API_KEY; @@ -62,6 +63,11 @@ describe("applyAuthChoice", () => { } else { process.env.PI_CODING_AGENT_DIR = previousPiAgentDir; } + if (previousAnthropicKey === undefined) { + delete process.env.ANTHROPIC_API_KEY; + } else { + process.env.ANTHROPIC_API_KEY = previousAnthropicKey; + } if (previousOpenrouterKey === undefined) { delete process.env.OPENROUTER_API_KEY; } else { @@ -443,6 +449,102 @@ describe("applyAuthChoice", () => { expect(result.agentModelOverride).toBe("opencode/claude-opus-4-6"); }); + it("does not persist literal 'undefined' when Anthropic API key prompt returns undefined", async () => { + tempStateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-auth-")); + process.env.OPENCLAW_STATE_DIR = tempStateDir; + process.env.OPENCLAW_AGENT_DIR = path.join(tempStateDir, "agent"); + process.env.PI_CODING_AGENT_DIR = process.env.OPENCLAW_AGENT_DIR; + delete process.env.ANTHROPIC_API_KEY; + + const text = vi.fn(async () => undefined as unknown as string); + const prompter: WizardPrompter = { + intro: vi.fn(noopAsync), + outro: vi.fn(noopAsync), + note: vi.fn(noopAsync), + select: vi.fn(async () => "" as never), + multiselect: vi.fn(async () => []), + text, + confirm: vi.fn(async () => false), + progress: vi.fn(() => ({ update: noop, stop: noop })), + }; + const runtime: RuntimeEnv = { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn((code: number) => { + throw new Error(`exit:${code}`); + }), + }; + + const result = await applyAuthChoice({ + authChoice: "apiKey", + config: {}, + prompter, + runtime, + setDefaultModel: false, + }); + + expect(result.config.auth?.profiles?.["anthropic:default"]).toMatchObject({ + provider: "anthropic", + mode: "api_key", + }); + + const authProfilePath = authProfilePathFor(requireAgentDir()); + const raw = await fs.readFile(authProfilePath, "utf8"); + const parsed = JSON.parse(raw) as { + profiles?: Record; + }; + expect(parsed.profiles?.["anthropic:default"]?.key).toBe(""); + expect(parsed.profiles?.["anthropic:default"]?.key).not.toBe("undefined"); + }); + + it("does not persist literal 'undefined' when OpenRouter API key prompt returns undefined", async () => { + tempStateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-auth-")); + process.env.OPENCLAW_STATE_DIR = tempStateDir; + process.env.OPENCLAW_AGENT_DIR = path.join(tempStateDir, "agent"); + process.env.PI_CODING_AGENT_DIR = process.env.OPENCLAW_AGENT_DIR; + delete process.env.OPENROUTER_API_KEY; + + const text = vi.fn(async () => undefined as unknown as string); + const prompter: WizardPrompter = { + intro: vi.fn(noopAsync), + outro: vi.fn(noopAsync), + note: vi.fn(noopAsync), + select: vi.fn(async () => "" as never), + multiselect: vi.fn(async () => []), + text, + confirm: vi.fn(async () => false), + progress: vi.fn(() => ({ update: noop, stop: noop })), + }; + const runtime: RuntimeEnv = { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn((code: number) => { + throw new Error(`exit:${code}`); + }), + }; + + const result = await applyAuthChoice({ + authChoice: "openrouter-api-key", + config: {}, + prompter, + runtime, + setDefaultModel: false, + }); + + expect(result.config.auth?.profiles?.["openrouter:default"]).toMatchObject({ + provider: "openrouter", + mode: "api_key", + }); + + const authProfilePath = authProfilePathFor(requireAgentDir()); + const raw = await fs.readFile(authProfilePath, "utf8"); + const parsed = JSON.parse(raw) as { + profiles?: Record; + }; + expect(parsed.profiles?.["openrouter:default"]?.key).toBe(""); + expect(parsed.profiles?.["openrouter:default"]?.key).not.toBe("undefined"); + }); + it("uses existing OPENROUTER_API_KEY when selecting openrouter-api-key", async () => { tempStateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-auth-")); process.env.OPENCLAW_STATE_DIR = tempStateDir; diff --git a/src/commands/configure.gateway.test.ts b/src/commands/configure.gateway.test.ts index b22f4668b..94388a509 100644 --- a/src/commands/configure.gateway.test.ts +++ b/src/commands/configure.gateway.test.ts @@ -70,4 +70,31 @@ describe("promptGatewayConfig", () => { const result = await promptGatewayConfig({}, runtime); expect(result.token).toBe("generated-token"); }); + it("does not set password to literal 'undefined' when prompt returns undefined", async () => { + vi.clearAllMocks(); + mocks.resolveGatewayPort.mockReturnValue(18789); + // Flow: loopback bind → password auth → tailscale off + const selectQueue = ["loopback", "password", "off"]; + mocks.select.mockImplementation(async () => selectQueue.shift()); + // Port prompt → OK, then password prompt → returns undefined (simulating prompter edge case) + const textQueue = ["18789", undefined]; + mocks.text.mockImplementation(async () => textQueue.shift()); + mocks.randomToken.mockReturnValue("unused"); + mocks.buildGatewayAuthConfig.mockImplementation(({ mode, token, password }) => ({ + mode, + token, + password, + })); + + const runtime: RuntimeEnv = { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + }; + + await promptGatewayConfig({}, runtime); + const call = mocks.buildGatewayAuthConfig.mock.calls[0]?.[0]; + expect(call?.password).not.toBe("undefined"); + expect(call?.password).toBe(""); + }); }); diff --git a/src/commands/configure.gateway.ts b/src/commands/configure.gateway.ts index 6e92a94a0..1432b81d7 100644 --- a/src/commands/configure.gateway.ts +++ b/src/commands/configure.gateway.ts @@ -193,7 +193,7 @@ export async function promptGatewayConfig( }), runtime, ); - gatewayPassword = String(password).trim(); + gatewayPassword = String(password ?? "").trim(); } const authConfig = buildGatewayAuthConfig({ diff --git a/src/commands/models/auth.ts b/src/commands/models/auth.ts index 8f685d898..7615c54ad 100644 --- a/src/commands/models/auth.ts +++ b/src/commands/models/auth.ts @@ -92,7 +92,7 @@ export async function modelsAuthSetupTokenCommand( message: "Paste Anthropic setup-token", validate: (value) => validateAnthropicSetupToken(String(value ?? "")), }); - const token = String(tokenInput).trim(); + const token = String(tokenInput ?? "").trim(); const profileId = resolveDefaultTokenProfileId(provider); upsertAuthProfile({ @@ -135,11 +135,11 @@ export async function modelsAuthPasteTokenCommand( message: `Paste token for ${provider}`, validate: (value) => (value?.trim() ? undefined : "Required"), }); - const token = String(tokenInput).trim(); + const token = String(tokenInput ?? "").trim(); const expires = opts.expiresIn?.trim() && opts.expiresIn.trim().length > 0 - ? Date.now() + parseDurationMs(String(opts.expiresIn).trim(), { defaultUnit: "d" }) + ? Date.now() + parseDurationMs(String(opts.expiresIn ?? "").trim(), { defaultUnit: "d" }) : undefined; upsertAuthProfile({ diff --git a/src/wizard/onboarding.gateway-config.test.ts b/src/wizard/onboarding.gateway-config.test.ts index 7c861175a..c15f73e6e 100644 --- a/src/wizard/onboarding.gateway-config.test.ts +++ b/src/wizard/onboarding.gateway-config.test.ts @@ -73,4 +73,53 @@ describe("configureGatewayForOnboarding", () => { "reminders.add", ]); }); + it("does not set password to literal 'undefined' when prompt returns undefined", async () => { + mocks.randomToken.mockReturnValue("unused"); + + // Flow: loopback bind → password auth → tailscale off + const selectQueue = ["loopback", "password", "off"]; + // Port prompt → OK, then password prompt → returns undefined + const textQueue = ["18789", undefined]; + const prompter: WizardPrompter = { + intro: vi.fn(async () => {}), + outro: vi.fn(async () => {}), + note: vi.fn(async () => {}), + select: vi.fn(async () => selectQueue.shift() as string), + multiselect: vi.fn(async () => []), + text: vi.fn(async () => textQueue.shift() as string), + confirm: vi.fn(async () => false), + progress: vi.fn(() => ({ update: vi.fn(), stop: vi.fn() })), + }; + + const runtime: RuntimeEnv = { + log: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + }; + + const result = await configureGatewayForOnboarding({ + flow: "advanced", + baseConfig: {}, + nextConfig: {}, + localPort: 18789, + quickstartGateway: { + hasExisting: false, + port: 18789, + bind: "loopback", + authMode: "password", + tailscaleMode: "off", + token: undefined, + password: undefined, + customBindHost: undefined, + tailscaleResetOnExit: false, + }, + prompter, + runtime, + }); + + const authConfig = result.nextConfig.gateway?.auth as { mode?: string; password?: string }; + expect(authConfig?.mode).toBe("password"); + expect(authConfig?.password).toBe(""); + expect(authConfig?.password).not.toBe("undefined"); + }); }); diff --git a/src/wizard/onboarding.gateway-config.ts b/src/wizard/onboarding.gateway-config.ts index aef746a72..fa3b8be2e 100644 --- a/src/wizard/onboarding.gateway-config.ts +++ b/src/wizard/onboarding.gateway-config.ts @@ -217,7 +217,7 @@ export async function configureGatewayForOnboarding( auth: { ...nextConfig.gateway?.auth, mode: "password", - password: String(password).trim(), + password: String(password ?? "").trim(), }, }, };