diff --git a/src/cli/acp-cli.option-collisions.test.ts b/src/cli/acp-cli.option-collisions.test.ts index 131db6a67cb..068f415de79 100644 --- a/src/cli/acp-cli.option-collisions.test.ts +++ b/src/cli/acp-cli.option-collisions.test.ts @@ -1,9 +1,7 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; import { Command } from "commander"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { runRegisteredCli } from "../test-utils/command-runner.js"; +import { withTempSecretFiles } from "../test-utils/secret-file-fixture.js"; const runAcpClientInteractive = vi.fn(async (_opts: unknown) => {}); const serveAcpGateway = vi.fn(async (_opts: unknown) => {}); @@ -30,27 +28,6 @@ vi.mock("../runtime.js", () => ({ describe("acp cli option collisions", () => { let registerAcpCli: typeof import("./acp-cli.js").registerAcpCli; - async function withSecretFiles( - secrets: { token?: string; password?: string }, - run: (files: { tokenFile?: string; passwordFile?: string }) => Promise, - ): Promise { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-cli-")); - try { - const files: { tokenFile?: string; passwordFile?: string } = {}; - if (secrets.token !== undefined) { - files.tokenFile = path.join(dir, "token.txt"); - await fs.writeFile(files.tokenFile, secrets.token, "utf8"); - } - if (secrets.password !== undefined) { - files.passwordFile = path.join(dir, "password.txt"); - await fs.writeFile(files.passwordFile, secrets.password, "utf8"); - } - return await run(files); - } finally { - await fs.rm(dir, { recursive: true, force: true }); - } - } - function createAcpProgram() { const program = new Command(); registerAcpCli(program); @@ -93,15 +70,19 @@ describe("acp cli option collisions", () => { }); it("loads gateway token/password from files", async () => { - await withSecretFiles({ token: "tok_file\n", [passwordKey()]: "pw_file\n" }, async (files) => { - // pragma: allowlist secret - await parseAcp([ - "--token-file", - files.tokenFile ?? "", - "--password-file", - files.passwordFile ?? "", - ]); - }); + await withTempSecretFiles( + "openclaw-acp-cli-", + { token: "tok_file\n", [passwordKey()]: "pw_file\n" }, + async (files) => { + // pragma: allowlist secret + await parseAcp([ + "--token-file", + files.tokenFile ?? "", + "--password-file", + files.passwordFile ?? "", + ]); + }, + ); expect(serveAcpGateway).toHaveBeenCalledWith( expect.objectContaining({ @@ -111,21 +92,30 @@ describe("acp cli option collisions", () => { ); }); - it("rejects mixed secret flags and file flags", async () => { - await withSecretFiles({ token: "tok_file\n" }, async (files) => { - await parseAcp(["--token", "tok_inline", "--token-file", files.tokenFile ?? ""]); + it.each([ + { + name: "rejects mixed secret flags and file flags", + files: { token: "tok_file\n" }, + args: (tokenFile: string) => ["--token", "tok_inline", "--token-file", tokenFile], + expected: /Use either --token or --token-file/, + }, + { + name: "rejects mixed password flags and file flags", + files: { password: "pw_file\n" }, // pragma: allowlist secret + args: (_tokenFile: string, passwordFile: string) => [ + "--password", + "pw_inline", + "--password-file", + passwordFile, + ], + expected: /Use either --password or --password-file/, + }, + ])("$name", async ({ files, args, expected }) => { + await withTempSecretFiles("openclaw-acp-cli-", files, async ({ tokenFile, passwordFile }) => { + await parseAcp(args(tokenFile ?? "", passwordFile ?? "")); }); - expectCliError(/Use either --token or --token-file/); - }); - - it("rejects mixed password flags and file flags", async () => { - const passwordFileValue = "pw_file\n"; // pragma: allowlist secret - await withSecretFiles({ password: passwordFileValue }, async (files) => { - await parseAcp(["--password", "pw_inline", "--password-file", files.passwordFile ?? ""]); - }); - - expectCliError(/Use either --password or --password-file/); + expectCliError(expected); }); it("warns when inline secret flags are used", async () => { @@ -140,7 +130,7 @@ describe("acp cli option collisions", () => { }); it("trims token file path before reading", async () => { - await withSecretFiles({ token: "tok_file\n" }, async (files) => { + await withTempSecretFiles("openclaw-acp-cli-", { token: "tok_file\n" }, async (files) => { await parseAcp(["--token-file", ` ${files.tokenFile ?? ""} `]); }); diff --git a/src/cli/daemon-cli/register-service-commands.test.ts b/src/cli/daemon-cli/register-service-commands.test.ts index cec45d62769..e249b00c835 100644 --- a/src/cli/daemon-cli/register-service-commands.test.ts +++ b/src/cli/daemon-cli/register-service-commands.test.ts @@ -39,34 +39,37 @@ describe("addGatewayServiceCommands", () => { runDaemonUninstall.mockClear(); }); - it("forwards install option collisions from parent gateway command", async () => { + it.each([ + { + name: "forwards install option collisions from parent gateway command", + argv: ["install", "--force", "--port", "19000", "--token", "tok_test"], + assert: () => { + expect(runDaemonInstall).toHaveBeenCalledWith( + expect.objectContaining({ + force: true, + port: "19000", + token: "tok_test", + }), + ); + }, + }, + { + name: "forwards status auth collisions from parent gateway command", + argv: ["status", "--token", "tok_status", "--password", "pw_status"], + assert: () => { + expect(runDaemonStatus).toHaveBeenCalledWith( + expect.objectContaining({ + rpc: expect.objectContaining({ + token: "tok_status", + password: "pw_status", // pragma: allowlist secret + }), + }), + ); + }, + }, + ])("$name", async ({ argv, assert }) => { const gateway = createGatewayParentLikeCommand(); - await gateway.parseAsync(["install", "--force", "--port", "19000", "--token", "tok_test"], { - from: "user", - }); - - expect(runDaemonInstall).toHaveBeenCalledWith( - expect.objectContaining({ - force: true, - port: "19000", - token: "tok_test", - }), - ); - }); - - it("forwards status auth collisions from parent gateway command", async () => { - const gateway = createGatewayParentLikeCommand(); - await gateway.parseAsync(["status", "--token", "tok_status", "--password", "pw_status"], { - from: "user", - }); - - expect(runDaemonStatus).toHaveBeenCalledWith( - expect.objectContaining({ - rpc: expect.objectContaining({ - token: "tok_status", - password: "pw_status", // pragma: allowlist secret - }), - }), - ); + await gateway.parseAsync(argv, { from: "user" }); + assert(); }); }); diff --git a/src/cli/gateway-cli/register.option-collisions.test.ts b/src/cli/gateway-cli/register.option-collisions.test.ts index 1ef5ba2c238..665886c76eb 100644 --- a/src/cli/gateway-cli/register.option-collisions.test.ts +++ b/src/cli/gateway-cli/register.option-collisions.test.ts @@ -128,30 +128,34 @@ describe("gateway register option collisions", () => { gatewayStatusCommand.mockClear(); }); - it("forwards --token to gateway call when parent and child option names collide", async () => { - await sharedProgram.parseAsync(["gateway", "call", "health", "--token", "tok_call", "--json"], { - from: "user", - }); - - expect(callGatewayCli).toHaveBeenCalledWith( - "health", - expect.objectContaining({ - token: "tok_call", - }), - {}, - ); - }); - - it("forwards --token to gateway probe when parent and child option names collide", async () => { - await sharedProgram.parseAsync(["gateway", "probe", "--token", "tok_probe", "--json"], { - from: "user", - }); - - expect(gatewayStatusCommand).toHaveBeenCalledWith( - expect.objectContaining({ - token: "tok_probe", - }), - defaultRuntime, - ); + it.each([ + { + name: "forwards --token to gateway call when parent and child option names collide", + argv: ["gateway", "call", "health", "--token", "tok_call", "--json"], + assert: () => { + expect(callGatewayCli).toHaveBeenCalledWith( + "health", + expect.objectContaining({ + token: "tok_call", + }), + {}, + ); + }, + }, + { + name: "forwards --token to gateway probe when parent and child option names collide", + argv: ["gateway", "probe", "--token", "tok_probe", "--json"], + assert: () => { + expect(gatewayStatusCommand).toHaveBeenCalledWith( + expect.objectContaining({ + token: "tok_probe", + }), + defaultRuntime, + ); + }, + }, + ])("$name", async ({ argv, assert }) => { + await sharedProgram.parseAsync(argv, { from: "user" }); + assert(); }); }); diff --git a/src/cli/gateway-cli/run.option-collisions.test.ts b/src/cli/gateway-cli/run.option-collisions.test.ts index 3a1f8bf57c7..a896a7a3f76 100644 --- a/src/cli/gateway-cli/run.option-collisions.test.ts +++ b/src/cli/gateway-cli/run.option-collisions.test.ts @@ -1,8 +1,6 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; import { Command } from "commander"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { withTempSecretFiles } from "../../test-utils/secret-file-fixture.js"; import { createCliRuntimeCapture } from "../test-runtime-capture.js"; const startGatewayServer = vi.fn(async (_port: number, _opts?: unknown) => ({ @@ -195,16 +193,10 @@ describe("gateway run option collisions", () => { ); }); - it("accepts --auth none override", async () => { - await runGatewayCli(["gateway", "run", "--auth", "none", "--allow-unconfigured"]); + it.each(["none", "trusted-proxy"] as const)("accepts --auth %s override", async (mode) => { + await runGatewayCli(["gateway", "run", "--auth", mode, "--allow-unconfigured"]); - expectAuthOverrideMode("none"); - }); - - it("accepts --auth trusted-proxy override", async () => { - await runGatewayCli(["gateway", "run", "--auth", "trusted-proxy", "--allow-unconfigured"]); - - expectAuthOverrideMode("trusted-proxy"); + expectAuthOverrideMode(mode); }); it("prints all supported modes on invalid --auth value", async () => { @@ -244,36 +236,34 @@ describe("gateway run option collisions", () => { }); it("reads gateway password from --password-file", async () => { - const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gateway-run-")); - try { - const passwordFile = path.join(tempDir, "gateway-password.txt"); - await fs.writeFile(passwordFile, "pw_from_file\n", "utf8"); + await withTempSecretFiles( + "openclaw-gateway-run-", + { password: "pw_from_file\n" }, + async ({ passwordFile }) => { + await runGatewayCli([ + "gateway", + "run", + "--auth", + "password", + "--password-file", + passwordFile ?? "", + "--allow-unconfigured", + ]); + }, + ); - await runGatewayCli([ - "gateway", - "run", - "--auth", - "password", - "--password-file", - passwordFile, - "--allow-unconfigured", - ]); - - expect(startGatewayServer).toHaveBeenCalledWith( - 18789, - expect.objectContaining({ - auth: expect.objectContaining({ - mode: "password", - password: "pw_from_file", // pragma: allowlist secret - }), + expect(startGatewayServer).toHaveBeenCalledWith( + 18789, + expect.objectContaining({ + auth: expect.objectContaining({ + mode: "password", + password: "pw_from_file", // pragma: allowlist secret }), - ); - expect(runtimeErrors).not.toContain( - "Warning: --password can be exposed via process listings. Prefer --password-file or OPENCLAW_GATEWAY_PASSWORD.", - ); - } finally { - await fs.rm(tempDir, { recursive: true, force: true }); - } + }), + ); + expect(runtimeErrors).not.toContain( + "Warning: --password can be exposed via process listings. Prefer --password-file or OPENCLAW_GATEWAY_PASSWORD.", + ); }); it("warns when gateway password is passed inline", async () => { @@ -293,26 +283,24 @@ describe("gateway run option collisions", () => { }); it("rejects using both --password and --password-file", async () => { - const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gateway-run-")); - try { - const passwordFile = path.join(tempDir, "gateway-password.txt"); - await fs.writeFile(passwordFile, "pw_from_file\n", "utf8"); + await withTempSecretFiles( + "openclaw-gateway-run-", + { password: "pw_from_file\n" }, + async ({ passwordFile }) => { + await expect( + runGatewayCli([ + "gateway", + "run", + "--password", + "pw_inline", + "--password-file", + passwordFile ?? "", + "--allow-unconfigured", + ]), + ).rejects.toThrow("__exit__:1"); + }, + ); - await expect( - runGatewayCli([ - "gateway", - "run", - "--password", - "pw_inline", - "--password-file", - passwordFile, - "--allow-unconfigured", - ]), - ).rejects.toThrow("__exit__:1"); - - expect(runtimeErrors).toContain("Use either --password or --password-file."); - } finally { - await fs.rm(tempDir, { recursive: true, force: true }); - } + expect(runtimeErrors).toContain("Use either --password or --password-file."); }); }); diff --git a/src/cli/update-cli.option-collisions.test.ts b/src/cli/update-cli.option-collisions.test.ts index c0dd2d88404..6db4cfdd260 100644 --- a/src/cli/update-cli.option-collisions.test.ts +++ b/src/cli/update-cli.option-collisions.test.ts @@ -44,30 +44,36 @@ describe("update cli option collisions", () => { defaultRuntime.exit.mockClear(); }); - it("forwards parent-captured --json/--timeout to `update status`", async () => { - await runRegisteredCli({ - register: registerUpdateCli as (program: Command) => void, + it.each([ + { + name: "forwards parent-captured --json/--timeout to `update status`", argv: ["update", "status", "--json", "--timeout", "9"], - }); - - expect(updateStatusCommand).toHaveBeenCalledWith( - expect.objectContaining({ - json: true, - timeout: "9", - }), - ); - }); - - it("forwards parent-captured --timeout to `update wizard`", async () => { + assert: () => { + expect(updateStatusCommand).toHaveBeenCalledWith( + expect.objectContaining({ + json: true, + timeout: "9", + }), + ); + }, + }, + { + name: "forwards parent-captured --timeout to `update wizard`", + argv: ["update", "wizard", "--timeout", "13"], + assert: () => { + expect(updateWizardCommand).toHaveBeenCalledWith( + expect.objectContaining({ + timeout: "13", + }), + ); + }, + }, + ])("$name", async ({ argv, assert }) => { await runRegisteredCli({ register: registerUpdateCli as (program: Command) => void, - argv: ["update", "wizard", "--timeout", "13"], + argv, }); - expect(updateWizardCommand).toHaveBeenCalledWith( - expect.objectContaining({ - timeout: "13", - }), - ); + assert(); }); }); diff --git a/src/test-utils/secret-file-fixture.ts b/src/test-utils/secret-file-fixture.ts new file mode 100644 index 00000000000..8e780929f94 --- /dev/null +++ b/src/test-utils/secret-file-fixture.ts @@ -0,0 +1,30 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +export type SecretFiles = { + passwordFile?: string; + tokenFile?: string; +}; + +export async function withTempSecretFiles( + prefix: string, + secrets: { password?: string; token?: string }, + run: (files: SecretFiles) => Promise, +): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + try { + const files: SecretFiles = {}; + if (secrets.token !== undefined) { + files.tokenFile = path.join(dir, "token.txt"); + await fs.writeFile(files.tokenFile, secrets.token, "utf8"); + } + if (secrets.password !== undefined) { + files.passwordFile = path.join(dir, "password.txt"); + await fs.writeFile(files.passwordFile, secrets.password, "utf8"); + } + return await run(files); + } finally { + await fs.rm(dir, { recursive: true, force: true }); + } +}