mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 19:11:23 +00:00
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 <steipete@gmail.com>
This commit is contained in:
@@ -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<string, { key?: string }>;
|
||||
};
|
||||
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<string, { key?: string }>;
|
||||
};
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user