mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:59:36 +00:00
fix: harden ACP secret handling and exec preflight boundaries
This commit is contained in:
@@ -1,4 +1,7 @@
|
||||
import { Command } from "commander";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const runAcpClientInteractive = vi.fn(async (_opts: unknown) => {});
|
||||
@@ -42,4 +45,64 @@ describe("acp cli option collisions", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("loads gateway token/password from files", async () => {
|
||||
const { registerAcpCli } = await import("./acp-cli.js");
|
||||
const program = new Command();
|
||||
registerAcpCli(program);
|
||||
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-cli-"));
|
||||
const tokenFile = path.join(dir, "token.txt");
|
||||
const passwordFile = path.join(dir, "password.txt");
|
||||
await fs.writeFile(tokenFile, "tok_file\n", "utf8");
|
||||
await fs.writeFile(passwordFile, "pw_file\n", "utf8");
|
||||
|
||||
await program.parseAsync(["acp", "--token-file", tokenFile, "--password-file", passwordFile], {
|
||||
from: "user",
|
||||
});
|
||||
|
||||
expect(serveAcpGateway).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
gatewayToken: "tok_file",
|
||||
gatewayPassword: "pw_file",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects mixed secret flags and file flags", async () => {
|
||||
const { registerAcpCli } = await import("./acp-cli.js");
|
||||
const program = new Command();
|
||||
registerAcpCli(program);
|
||||
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-acp-cli-"));
|
||||
const tokenFile = path.join(dir, "token.txt");
|
||||
await fs.writeFile(tokenFile, "tok_file\n", "utf8");
|
||||
|
||||
await program.parseAsync(["acp", "--token", "tok_inline", "--token-file", tokenFile], {
|
||||
from: "user",
|
||||
});
|
||||
|
||||
expect(serveAcpGateway).not.toHaveBeenCalled();
|
||||
expect(defaultRuntime.error).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/Use either --token or --token-file/),
|
||||
);
|
||||
expect(defaultRuntime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("warns when inline secret flags are used", async () => {
|
||||
const { registerAcpCli } = await import("./acp-cli.js");
|
||||
const program = new Command();
|
||||
registerAcpCli(program);
|
||||
|
||||
await program.parseAsync(["acp", "--token", "tok_inline", "--password", "pw_inline"], {
|
||||
from: "user",
|
||||
});
|
||||
|
||||
expect(defaultRuntime.error).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/--token can be exposed via process listings/),
|
||||
);
|
||||
expect(defaultRuntime.error).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/--password can be exposed via process listings/),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,18 +1,45 @@
|
||||
import type { Command } from "commander";
|
||||
import { runAcpClientInteractive } from "../acp/client.js";
|
||||
import { readSecretFromFile } from "../acp/secret-file.js";
|
||||
import { serveAcpGateway } from "../acp/server.js";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { formatDocsLink } from "../terminal/links.js";
|
||||
import { theme } from "../terminal/theme.js";
|
||||
import { inheritOptionFromParent } from "./command-options.js";
|
||||
|
||||
function resolveSecretOption(params: {
|
||||
direct?: string;
|
||||
file?: string;
|
||||
directFlag: string;
|
||||
fileFlag: string;
|
||||
label: string;
|
||||
}) {
|
||||
const direct = params.direct?.trim();
|
||||
const file = params.file?.trim();
|
||||
if (direct && file) {
|
||||
throw new Error(`Use either ${params.directFlag} or ${params.fileFlag} for ${params.label}.`);
|
||||
}
|
||||
if (file) {
|
||||
return readSecretFromFile(file, params.label);
|
||||
}
|
||||
return direct || undefined;
|
||||
}
|
||||
|
||||
function warnSecretCliFlag(flag: "--token" | "--password") {
|
||||
defaultRuntime.error(
|
||||
`Warning: ${flag} can be exposed via process listings. Prefer ${flag}-file or environment variables.`,
|
||||
);
|
||||
}
|
||||
|
||||
export function registerAcpCli(program: Command) {
|
||||
const acp = program.command("acp").description("Run an ACP bridge backed by the Gateway");
|
||||
|
||||
acp
|
||||
.option("--url <url>", "Gateway WebSocket URL (defaults to gateway.remote.url when configured)")
|
||||
.option("--token <token>", "Gateway token (if required)")
|
||||
.option("--token-file <path>", "Read gateway token from file")
|
||||
.option("--password <password>", "Gateway password (if required)")
|
||||
.option("--password-file <path>", "Read gateway password from file")
|
||||
.option("--session <key>", "Default session key (e.g. agent:main:main)")
|
||||
.option("--session-label <label>", "Default session label to resolve")
|
||||
.option("--require-existing", "Fail if the session key/label does not exist", false)
|
||||
@@ -25,10 +52,30 @@ export function registerAcpCli(program: Command) {
|
||||
)
|
||||
.action(async (opts) => {
|
||||
try {
|
||||
const gatewayToken = resolveSecretOption({
|
||||
direct: opts.token as string | undefined,
|
||||
file: opts.tokenFile as string | undefined,
|
||||
directFlag: "--token",
|
||||
fileFlag: "--token-file",
|
||||
label: "Gateway token",
|
||||
});
|
||||
const gatewayPassword = resolveSecretOption({
|
||||
direct: opts.password as string | undefined,
|
||||
file: opts.passwordFile as string | undefined,
|
||||
directFlag: "--password",
|
||||
fileFlag: "--password-file",
|
||||
label: "Gateway password",
|
||||
});
|
||||
if (opts.token) {
|
||||
warnSecretCliFlag("--token");
|
||||
}
|
||||
if (opts.password) {
|
||||
warnSecretCliFlag("--password");
|
||||
}
|
||||
await serveAcpGateway({
|
||||
gatewayUrl: opts.url as string | undefined,
|
||||
gatewayToken: opts.token as string | undefined,
|
||||
gatewayPassword: opts.password as string | undefined,
|
||||
gatewayToken,
|
||||
gatewayPassword,
|
||||
defaultSessionKey: opts.session as string | undefined,
|
||||
defaultSessionLabel: opts.sessionLabel as string | undefined,
|
||||
requireExistingSession: Boolean(opts.requireExisting),
|
||||
|
||||
Reference in New Issue
Block a user