mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-13 02:06:10 +00:00
fix(cli): protect protocol stdout during startup (#89997)
* fix(cli): suppress mcp serve startup stdout * fix(cli): suppress mcp serve startup stdout * fix(cli): protect protocol stdout during startup * fix(cli): match ACP protocol options at startup * fix(cli): derive protocol ownership from command action --------- Co-authored-by: JARVIS <kenners22@users.noreply.github.com> Co-authored-by: openclaw-clownfish[bot] <280122609+openclaw-clownfish[bot]@users.noreply.github.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org> Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
This commit is contained in:
@@ -36,6 +36,7 @@ export type CliCommandPathPolicy = {
|
||||
routeConfigGuard: CliRouteConfigGuardPolicy;
|
||||
loadPlugins: CliCommandPluginLoadPolicy;
|
||||
pluginRegistry: CliPluginRegistryPolicy;
|
||||
ownsProtocolStdout: boolean;
|
||||
hideBanner: boolean;
|
||||
ensureCliPath: boolean;
|
||||
networkProxy: CliNetworkProxyPolicyResolver;
|
||||
@@ -258,6 +259,11 @@ export const cliCommandCatalog: readonly CliCommandCatalogEntry[] = [
|
||||
policy: { loadPlugins: "never", ensureCliPath: false, networkProxy: "bypass" },
|
||||
},
|
||||
{ commandPath: ["acp"], policy: { networkProxy: "bypass" } },
|
||||
{
|
||||
commandPath: ["acp"],
|
||||
exact: true,
|
||||
policy: { ownsProtocolStdout: true },
|
||||
},
|
||||
{ commandPath: ["approvals"], policy: { networkProxy: "bypass" } },
|
||||
{ commandPath: ["backup"], policy: { bypassConfigGuard: true, networkProxy: "bypass" } },
|
||||
{ commandPath: ["chat"], policy: { networkProxy: "bypass" } },
|
||||
@@ -282,6 +288,11 @@ export const cliCommandCatalog: readonly CliCommandCatalogEntry[] = [
|
||||
{ commandPath: ["hooks"], policy: { networkProxy: "bypass" } },
|
||||
{ commandPath: ["logs"], policy: { networkProxy: "bypass" } },
|
||||
{ commandPath: ["mcp"], policy: { networkProxy: "bypass" } },
|
||||
{
|
||||
commandPath: ["mcp", "serve"],
|
||||
exact: true,
|
||||
policy: { ownsProtocolStdout: true },
|
||||
},
|
||||
{
|
||||
commandPath: ["node"],
|
||||
policy: { networkProxy: "bypass" },
|
||||
|
||||
@@ -111,6 +111,25 @@ describe("command-execution-startup", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("uses the resolved action command path for protocol startup policy", () => {
|
||||
expect(
|
||||
mod.resolveCliExecutionStartupContext({
|
||||
argv: ["node", "openclaw", "acp", "--token", "-secret"],
|
||||
protocolCommandPath: ["acp"],
|
||||
jsonOutputMode: false,
|
||||
env: {},
|
||||
}).startupPolicy.suppressDoctorStdout,
|
||||
).toBe(true);
|
||||
expect(
|
||||
mod.resolveCliExecutionStartupContext({
|
||||
argv: ["node", "openclaw", "acp", "--verbose", "client"],
|
||||
protocolCommandPath: ["acp", "client"],
|
||||
jsonOutputMode: false,
|
||||
env: {},
|
||||
}).startupPolicy.suppressDoctorStdout,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("routes logs to stderr and emits banner only when allowed", async () => {
|
||||
await mod.applyCliExecutionStartupPresentation({
|
||||
startupPolicy: {
|
||||
|
||||
@@ -16,6 +16,7 @@ const hasVersionFlag = (argv: readonly string[]) =>
|
||||
|
||||
export function resolveCliExecutionStartupContext(params: {
|
||||
argv: string[];
|
||||
protocolCommandPath?: string[];
|
||||
jsonOutputMode: boolean;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
routeMode?: boolean;
|
||||
@@ -29,6 +30,7 @@ export function resolveCliExecutionStartupContext(params: {
|
||||
startupPolicy: resolveCliStartupPolicy({
|
||||
argv: params.argv,
|
||||
commandPath,
|
||||
protocolCommandPath: params.protocolCommandPath,
|
||||
jsonOutputMode: params.jsonOutputMode,
|
||||
env: params.env,
|
||||
routeMode: params.routeMode,
|
||||
@@ -43,7 +45,7 @@ export async function applyCliExecutionStartupPresentation(params: {
|
||||
showBanner?: boolean;
|
||||
version?: string;
|
||||
}) {
|
||||
// JSON-mode commands must keep stdout machine-readable; route diagnostics away first.
|
||||
// Machine-readable commands must route diagnostics away before startup can print.
|
||||
if (params.startupPolicy.suppressDoctorStdout && params.routeLogsToStderrOnSuppress !== false) {
|
||||
routeLogsToStderr();
|
||||
}
|
||||
|
||||
@@ -9,5 +9,4 @@ describe("command-path-matches", () => {
|
||||
expect(matchesCommandPath(["status", "watch"], ["status"], { exact: true })).toBe(false);
|
||||
expect(matchesCommandPath(["config", "get"], ["config", "get"], { exact: true })).toBe(true);
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -13,6 +13,7 @@ const DEFAULT_EXPECTED_POLICY: CliCommandPathPolicy = {
|
||||
routeConfigGuard: "never",
|
||||
loadPlugins: "never",
|
||||
pluginRegistry: { scope: "all" },
|
||||
ownsProtocolStdout: false,
|
||||
hideBanner: false,
|
||||
ensureCliPath: true,
|
||||
networkProxy: "default",
|
||||
|
||||
@@ -14,6 +14,7 @@ const DEFAULT_CLI_COMMAND_PATH_POLICY: CliCommandPathPolicy = {
|
||||
routeConfigGuard: "never",
|
||||
loadPlugins: "never",
|
||||
pluginRegistry: { scope: "all" },
|
||||
ownsProtocolStdout: false,
|
||||
hideBanner: false,
|
||||
ensureCliPath: true,
|
||||
networkProxy: "default",
|
||||
|
||||
@@ -235,4 +235,18 @@ describe("command-startup-policy", () => {
|
||||
pluginRegistry: { scope: "channels" },
|
||||
});
|
||||
});
|
||||
|
||||
it("suppresses startup stdout for the mcp serve protocol", () => {
|
||||
expect(resolvePolicy({ commandPath: ["mcp", "serve"] }).suppressDoctorStdout).toBe(true);
|
||||
});
|
||||
|
||||
it("suppresses startup stdout for the bare acp protocol", () => {
|
||||
expect(resolvePolicy({ commandPath: ["acp"] }).suppressDoctorStdout).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps startup stdout for non-protocol commands", () => {
|
||||
expect(resolvePolicy({ commandPath: ["mcp", "list"] }).suppressDoctorStdout).toBe(false);
|
||||
expect(resolvePolicy({ commandPath: ["acp", "client"] }).suppressDoctorStdout).toBe(false);
|
||||
expect(resolvePolicy({ commandPath: ["status"] }).suppressDoctorStdout).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -28,12 +28,19 @@ function shouldLoadPlugins(params: {
|
||||
export function resolveCliStartupPolicy(params: {
|
||||
argv?: string[];
|
||||
commandPath: string[];
|
||||
protocolCommandPath?: string[];
|
||||
jsonOutputMode: boolean;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
routeMode?: boolean;
|
||||
}) {
|
||||
const suppressDoctorStdout = params.jsonOutputMode;
|
||||
const commandPolicy = resolveCliCommandPathPolicy(params.commandPath);
|
||||
// Commander resolves required option values before selecting the action command, so this path
|
||||
// remains authoritative when a protocol option value itself begins with "-".
|
||||
const ownsProtocolStdout = params.protocolCommandPath
|
||||
? resolveCliCommandPathPolicy(params.protocolCommandPath).ownsProtocolStdout
|
||||
: commandPolicy.ownsProtocolStdout;
|
||||
// Protocol commands own stdout from process startup, before their action installs later routing.
|
||||
const suppressDoctorStdout = params.jsonOutputMode || ownsProtocolStdout;
|
||||
const env = params.env ?? process.env;
|
||||
return {
|
||||
suppressDoctorStdout,
|
||||
|
||||
@@ -146,6 +146,19 @@ describe("registerPreActionHooks", () => {
|
||||
.command("status")
|
||||
.option("--json")
|
||||
.action(() => {});
|
||||
const acp = programLocal
|
||||
.command("acp")
|
||||
.option("--token <token>")
|
||||
.option("--verbose")
|
||||
.action(() => {});
|
||||
acp
|
||||
.command("client")
|
||||
.option("--cwd <dir>")
|
||||
.action(() => {});
|
||||
programLocal
|
||||
.command("mcp")
|
||||
.command("serve")
|
||||
.action(() => {});
|
||||
const gateway = programLocal
|
||||
.command("gateway")
|
||||
.option("--force")
|
||||
@@ -607,6 +620,45 @@ describe("registerPreActionHooks", () => {
|
||||
expect(routeLogsToStderrMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses the Commander action path for protocol stdout ownership", async () => {
|
||||
await runPreAction({
|
||||
parseArgv: ["acp"],
|
||||
processArgv: ["node", "openclaw", "acp", "--token", "-secret"],
|
||||
});
|
||||
|
||||
expect(routeLogsToStderrMock).toHaveBeenCalledOnce();
|
||||
expect(ensureConfigReadyMock).toHaveBeenCalledWith({
|
||||
runtime: runtimeMock,
|
||||
commandPath: ["acp"],
|
||||
suppressDoctorStdout: true,
|
||||
});
|
||||
|
||||
vi.clearAllMocks();
|
||||
await runPreAction({
|
||||
parseArgv: ["acp", "client"],
|
||||
processArgv: ["node", "openclaw", "acp", "--verbose", "client"],
|
||||
});
|
||||
|
||||
expect(routeLogsToStderrMock).not.toHaveBeenCalled();
|
||||
expect(ensureConfigReadyMock).toHaveBeenCalledWith({
|
||||
runtime: runtimeMock,
|
||||
commandPath: ["acp", "client"],
|
||||
});
|
||||
|
||||
vi.clearAllMocks();
|
||||
await runPreAction({
|
||||
parseArgv: ["mcp", "serve"],
|
||||
processArgv: ["node", "openclaw", "mcp", "serve"],
|
||||
});
|
||||
|
||||
expect(routeLogsToStderrMock).toHaveBeenCalledOnce();
|
||||
expect(ensureConfigReadyMock).toHaveBeenCalledWith({
|
||||
runtime: runtimeMock,
|
||||
commandPath: ["mcp", "serve"],
|
||||
suppressDoctorStdout: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not preload plugins for agents list JSON output", async () => {
|
||||
await runPreAction({
|
||||
parseArgv: ["agents", "list"],
|
||||
|
||||
@@ -53,6 +53,16 @@ function getRootCommand(command: Command): Command {
|
||||
return current;
|
||||
}
|
||||
|
||||
function getActionCommandPath(actionCommand: Command): string[] {
|
||||
const commandPath: string[] = [];
|
||||
let current: Command | null = actionCommand;
|
||||
while (current.parent) {
|
||||
commandPath.unshift(current.name());
|
||||
current = current.parent;
|
||||
}
|
||||
return commandPath;
|
||||
}
|
||||
|
||||
function getCliLogLevel(actionCommand: Command): LogLevel | undefined {
|
||||
const root = getRootCommand(actionCommand);
|
||||
if (typeof root.getOptionValueSource !== "function") {
|
||||
@@ -119,6 +129,7 @@ export function registerPreActionHooks(program: Command, programVersion: string)
|
||||
const jsonOutputMode = isCommandJsonOutputMode(actionCommand, argv);
|
||||
const { commandPath, startupPolicy } = resolveCliExecutionStartupContext({
|
||||
argv,
|
||||
protocolCommandPath: getActionCommandPath(actionCommand),
|
||||
jsonOutputMode,
|
||||
env: process.env,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user