From 250e636ffd0132fe8cee6054fd5a90a5cd748d9e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 19 Jul 2026 23:05:23 -0700 Subject: [PATCH] fix(onboard): reject invalid setup options before writes (#111680) * fix(onboard): validate setup options before mutation * fix(onboard): keep flow helper type local --- docs/cli/setup.md | 8 +++-- src/cli/program/register.onboard.test.ts | 26 ++++++++------ src/cli/program/register.onboard.ts | 6 ++-- src/cli/program/register.setup.test.ts | 38 ++++++++++++++++++++- src/cli/program/register.setup.ts | 33 ++++++++++++++++-- src/commands/onboard-skills.ts | 6 +--- src/commands/onboard-types.ts | 15 +++++++-- src/commands/onboard.test.ts | 43 ++++++++++++++++++++++++ src/commands/onboard.ts | 31 ++++++++++++----- 9 files changed, 171 insertions(+), 35 deletions(-) diff --git a/docs/cli/setup.md b/docs/cli/setup.md index b7de0ad2fd20..9f3c988bf070 100644 --- a/docs/cli/setup.md +++ b/docs/cli/setup.md @@ -47,7 +47,8 @@ auth (`--auth-choice`, `--token`, provider key flags), Gateway Tailscale (`--tailscale`), reset (`--reset`, `--reset-scope`), flow (`--flow quickstart|advanced|manual|import`), and skip flags (`--skip-channels`, `--skip-skills`, `--skip-bootstrap`, `--skip-search`, -`--skip-health`, `--skip-ui`, `--skip-hooks`). See [Onboard](/cli/onboard) and +`--skip-health`, `--skip-ui`, `--skip-hooks`). Pass `--tui` to use the same +terminal hatch as `openclaw onboard --tui`. See [Onboard](/cli/onboard) and [CLI automation](/start/wizard-cli-automation) for the full flag reference and non-interactive examples. `openclaw onboard --modern` remains a compatibility entry for the same inference-gated OpenClaw assistant. @@ -65,6 +66,7 @@ entry for the same inference-gated OpenClaw assistant. | `--workspace ` | Workspace proposal in guided mode; persisted directly by baseline, classic, and noninteractive setup. | | `--baseline` | Create baseline config/workspace/session folders without onboarding. | | `--wizard` | Force interactive onboarding. | +| `--tui` | Use the terminal hatch instead of the browser handoff. | | `--non-interactive` | Run onboarding without prompts. | | `--accept-risk` | Acknowledge full-system agent access risk; required with `--non-interactive`. | | `--mode ` | Onboarding mode: `local` or `remote`. | @@ -90,7 +92,9 @@ storage mode. `openclaw setup --baseline` preserves the older baseline-only behavior: it creates the config, workspace, and session directories, then exits without -running onboarding. +running onboarding. It accepts `--workspace` and harmless output controls, but +rejects explicit onboarding, Gateway, auth, reset, or daemon options instead of +silently ignoring them. ## Examples diff --git a/src/cli/program/register.onboard.test.ts b/src/cli/program/register.onboard.test.ts index 8a8861b1c915..54a3eb3734ba 100644 --- a/src/cli/program/register.onboard.test.ts +++ b/src/cli/program/register.onboard.test.ts @@ -147,20 +147,24 @@ describe("registerOnboardCommand", () => { expect(setupWizardOptions(2).installDaemon).toBe(false); }); - it("parses numeric gateway port and drops invalid values", async () => { + it("parses a valid numeric gateway port", async () => { await runCli(["onboard", "--gateway-port", "18789"]); - expect(setupWizardOptions(0).gatewayPort).toBe(18789); - - await runCli(["onboard", "--gateway-port", "nope"]); - expect(setupWizardOptions(1).gatewayPort).toBeUndefined(); - - await runCli(["onboard", "--gateway-port", "18789x"]); - expect(setupWizardOptions(2).gatewayPort).toBeUndefined(); - - await runCli(["onboard", "--gateway-port", "99999"]); - expect(setupWizardOptions(3).gatewayPort).toBeUndefined(); + expect(setupWizardOptions().gatewayPort).toBe(18789); }); + it.each(["not-a-port", "70000"])( + "rejects invalid --gateway-port %s before onboarding dispatch", + async (gatewayPort) => { + await runCli(["onboard", "--gateway-port", gatewayPort]); + + expect(runtime.error).toHaveBeenCalledWith( + "Error: --gateway-port must be an integer between 1 and 65535.", + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(setupWizardCommandMock).not.toHaveBeenCalled(); + }, + ); + it("forwards --reset-scope to setup wizard options", async () => { await runCli(["onboard", "--reset", "--reset-scope", "full"]); const options = setupWizardOptions(); diff --git a/src/cli/program/register.onboard.ts b/src/cli/program/register.onboard.ts index b6a1c3b3537b..5305a9729f1d 100644 --- a/src/cli/program/register.onboard.ts +++ b/src/cli/program/register.onboard.ts @@ -18,7 +18,7 @@ import type { import { resolveProviderOnboardAuthFlags } from "../../plugins/provider-auth-choices.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { formatCliCommand } from "../command-format.js"; -import { parsePort } from "../shared/parse-port.js"; +import { parseGatewayPortOption } from "../gateway-port-option.js"; export function resolveInstallDaemonFlag(command: Command): boolean | undefined { // Commander doesn't support option conflicts natively; keep original behavior. @@ -333,7 +333,7 @@ export function registerOnboardCommand(program: Command): void { } const installDaemon = resolveInstallDaemonFlag(commandRuntime); const tailscaleResetOnExit = resolveTailscaleResetOnExitFlag(commandRuntime); - const gatewayPort = parsePort(opts.gatewayPort); + const gatewayPort = parseGatewayPortOption(opts.gatewayPort, "--gateway-port"); const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand( { @@ -345,7 +345,7 @@ export function registerOnboardCommand(program: Command): void { flow: opts.flow as "quickstart" | "advanced" | "manual" | "import" | undefined, mode: opts.mode as "local" | "remote" | undefined, ...pickOnboardAuthOptionValues(opts as Record), - gatewayPort: gatewayPort ?? undefined, + gatewayPort, gatewayBind: opts.gatewayBind as GatewayBind | undefined, gatewayAuth: opts.gatewayAuth as GatewayAuthChoice | undefined, gatewayToken: opts.gatewayToken as string | undefined, diff --git a/src/cli/program/register.setup.test.ts b/src/cli/program/register.setup.test.ts index 44bb8893ad0b..3122715bca40 100644 --- a/src/cli/program/register.setup.test.ts +++ b/src/cli/program/register.setup.test.ts @@ -172,13 +172,28 @@ describe("registerSetupCommand", () => { }); it("runs baseline setup command when --baseline is set", async () => { - await runCli(["setup", "--baseline", "--workspace", "/tmp/ws"]); + await runCli(["setup", "--baseline", "--workspace", "/tmp/ws", "--json"]); expect(setupCommandMock).toHaveBeenCalledWith(lastSetupOptions(), runtime); expect(lastSetupOptions()?.workspace).toBe("/tmp/ws"); expect(setupWizardCommandMock).not.toHaveBeenCalled(); }); + it.each([ + ["onboarding mode", ["--mode", "remote"]], + ["remote Gateway", ["--remote-url", "wss://example.invalid"]], + ["reset", ["--reset"]], + ["daemon", ["--daemon-runtime", "node"]], + ["auth", ["--auth-choice", "skip"]], + ])("rejects explicit %s options with --baseline", async (_label, args) => { + await runCli(["setup", "--baseline", ...args]); + + expect(runtime.error).toHaveBeenCalledWith(expect.stringContaining(args[0]!)); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(setupCommandMock).not.toHaveBeenCalled(); + expect(setupWizardCommandMock).not.toHaveBeenCalled(); + }); + it("runs setup wizard command when --wizard is set", async () => { const remoteToken = ["fixture", "value"].join("-"); await runCli([ @@ -199,6 +214,27 @@ describe("registerSetupCommand", () => { expect(setupCommandMock).not.toHaveBeenCalled(); }); + it("forwards --tui through the canonical onboarding path", async () => { + await runCli(["setup", "--tui"]); + + expect(lastWizardOptions()?.tui).toBe(true); + expect(setupCommandMock).not.toHaveBeenCalled(); + }); + + it.each(["not-a-port", "70000"])( + "rejects invalid --gateway-port %s before onboarding dispatch", + async (gatewayPort) => { + await runCli(["setup", "--gateway-port", gatewayPort]); + + expect(runtime.error).toHaveBeenCalledWith( + "Error: --gateway-port must be an integer between 1 and 65535.", + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(setupWizardCommandMock).not.toHaveBeenCalled(); + expect(setupCommandMock).not.toHaveBeenCalled(); + }, + ); + it("runs setup wizard command when wizard-only flags are passed explicitly", async () => { await runCli(["setup", "--mode", "remote", "--non-interactive", "--accept-risk"]); diff --git a/src/cli/program/register.setup.ts b/src/cli/program/register.setup.ts index fe2f324501fb..8dc0b7c24ce4 100644 --- a/src/cli/program/register.setup.ts +++ b/src/cli/program/register.setup.ts @@ -14,7 +14,7 @@ import type { RuntimeEnv } from "../../runtime.js"; import { runCommandWithRuntime } from "../cli-utils.js"; import { hasExplicitOptions } from "../command-options.js"; import { isUnconfiguredConfigSource } from "../fresh-install-config.js"; -import { parsePort } from "../shared/parse-port.js"; +import { parseGatewayPortOption } from "../gateway-port-option.js"; import { pickOnboardAuthOptionValues, registerOnboardAuthOptions, @@ -23,6 +23,7 @@ import { } from "./register.onboard.js"; const SYSTEM_AGENT_OPTION_NAMES = new Set(["message", "yes", "json"]); +const BASELINE_OPTION_NAMES = new Set(["baseline", "workspace", "json"]); const optionalString = (value: unknown): string | undefined => typeof value === "string" ? value : undefined; @@ -55,6 +56,24 @@ function hasExplicitOnboardingOption(command: Command): boolean { }); } +function listUnsupportedBaselineOptions(command: Command): string[] { + const optionsByName = new Map(); + for (const option of command.options) { + const name = option.attributeName(); + if (BASELINE_OPTION_NAMES.has(name) || command.getOptionValueSource(name) !== "cli") { + continue; + } + const existing = optionsByName.get(name); + const valueIsNegated = command.getOptionValue(name) === false; + if (!existing || option.negate === valueIsNegated) { + optionsByName.set(name, option); + } + } + return [...optionsByName.values()] + .map((option) => option.long ?? option.short ?? option.flags) + .toSorted(); +} + async function isConfiguredInstance(): Promise { const { readConfigFileSnapshot } = await import("../../config/config.js"); const snapshot = await readConfigFileSnapshot(); @@ -89,13 +108,19 @@ async function runOnboardingEntry( runtime: RuntimeEnv, ): Promise { if (options.baseline) { + const unsupportedOptions = listUnsupportedBaselineOptions(commandRuntime); + if (unsupportedOptions.length > 0) { + runtime.error(`--baseline cannot be combined with: ${unsupportedOptions.join(", ")}.`); + runtime.exit(1); + return; + } const { setupCommand } = await import("../../commands/setup.js"); await setupCommand({ workspace: optionalString(options.workspace) }, runtime); return; } const installDaemon = resolveInstallDaemonFlag(commandRuntime); const tailscaleResetOnExit = resolveTailscaleResetOnExitFlag(commandRuntime); - const gatewayPort = parsePort(options.gatewayPort); + const gatewayPort = parseGatewayPortOption(options.gatewayPort, "--gateway-port"); const { setupWizardCommand } = await import("../../commands/onboard.js"); await setupWizardCommand( { @@ -103,12 +128,13 @@ async function runOnboardingEntry( nonInteractive: Boolean(options.nonInteractive), acceptRisk: Boolean(options.acceptRisk), classic: Boolean(options.classic), + tui: Boolean(options.tui), flow: options.flow as "quickstart" | "advanced" | "manual" | "import" | undefined, mode: options.mode as "local" | "remote" | undefined, ...pickOnboardAuthOptionValues(options), reset: Boolean(options.reset), resetScope: options.resetScope as ResetScope | undefined, - gatewayPort: gatewayPort ?? undefined, + gatewayPort, gatewayBind: options.gatewayBind as GatewayBind | undefined, gatewayAuth: options.gatewayAuth as GatewayAuthChoice | undefined, gatewayToken: optionalString(options.gatewayToken), @@ -179,6 +205,7 @@ export function registerSetupCommand(program: Command): void { .option("--reset-scope ", "Reset scope: config|config+creds+sessions|full") .option("--non-interactive", "Run onboarding without prompts", false) .option("--classic", "Use the classic multi-step setup wizard", false) + .option("--tui", "Use the terminal hatch instead of the browser handoff", false) .option( "--accept-risk", "Acknowledge that agents are powerful and full system access is risky (required for --non-interactive)", diff --git a/src/commands/onboard-skills.ts b/src/commands/onboard-skills.ts index 76722dc44e3b..867bbdf071d2 100644 --- a/src/commands/onboard-skills.ts +++ b/src/commands/onboard-skills.ts @@ -21,7 +21,7 @@ import { import { t } from "../wizard/i18n/index.js"; import type { WizardPrompter } from "../wizard/prompts.js"; import { detectBinary } from "./onboard-helpers.js"; -import type { NodeManagerChoice } from "./onboard-types.js"; +import { isNodeManagerChoice, type NodeManagerChoice } from "./onboard-types.js"; const HOMEBREW_PROMPT_PLATFORMS = new Set(["darwin", "linux"]); const SKIPPED_INSTALL_NAME_LIMIT = 8; @@ -123,10 +123,6 @@ function isTrustedAutoInstallableSkill(skill: { bundled: boolean; source: string return skill.bundled && skill.source === "openclaw-bundled"; } -function isNodeManagerChoice(value: unknown): value is NodeManagerChoice { - return value === "npm" || value === "pnpm" || value === "bun"; -} - function resolveDefaultNodeManager( config: OpenClawConfig, requested: NodeManagerChoice | undefined, diff --git a/src/commands/onboard-types.ts b/src/commands/onboard-types.ts index 010c240df04f..b85351eab87c 100644 --- a/src/commands/onboard-types.ts +++ b/src/commands/onboard-types.ts @@ -25,10 +25,21 @@ export type GatewayAuthChoice = "token" | "password"; export type ResetScope = "config" | "config+creds+sessions" | "full"; export type GatewayBind = "loopback" | "lan" | "auto" | "custom" | "tailnet"; export type TailscaleMode = "off" | "serve" | "funnel"; -export type NodeManagerChoice = "npm" | "pnpm" | "bun"; +const NODE_MANAGER_CHOICES = ["npm", "pnpm", "bun"] as const; +export type NodeManagerChoice = (typeof NODE_MANAGER_CHOICES)[number]; +const ONBOARD_FLOWS = ["quickstart", "advanced", "manual", "import"] as const; +type OnboardFlow = (typeof ONBOARD_FLOWS)[number]; export type ChannelChoice = ChannelId; export type { SecretInputMode } from "../plugins/provider-auth-types.js"; +export function isNodeManagerChoice(value: unknown): value is NodeManagerChoice { + return NODE_MANAGER_CHOICES.some((choice) => choice === value); +} + +export function isOnboardFlow(value: unknown): value is OnboardFlow { + return ONBOARD_FLOWS.some((flow) => flow === value); +} + type OnboardDynamicProviderOptions = { /** * Provider-specific non-interactive auth flags are plugin-owned and keyed by @@ -41,7 +52,7 @@ type OnboardDynamicProviderOptions = { export type OnboardOptions = OnboardDynamicProviderOptions & { mode?: OnboardMode; /** "manual" is an alias for "advanced". */ - flow?: "quickstart" | "advanced" | "manual" | "import"; + flow?: OnboardFlow; /** Force the classic multi-step interactive wizard instead of guided setup. */ classic?: boolean; /** Force the terminal hatch instead of the guided browser handoff. */ diff --git a/src/commands/onboard.test.ts b/src/commands/onboard.test.ts index 052179442925..9d8fe9afcb0d 100644 --- a/src/commands/onboard.test.ts +++ b/src/commands/onboard.test.ts @@ -395,6 +395,49 @@ describe("setupWizardCommand", () => { expect(mocks.runNonInteractiveSetup).not.toHaveBeenCalled(); }); + it.each([ + { + label: "unsupported flow", + options: { flow: "bogus" as never }, + expectedError: "Invalid --flow", + }, + { + label: "malformed remote URL", + options: { mode: "remote" as const, remoteUrl: "garbage" }, + expectedError: "URL must start with ws:// or wss://", + }, + { + label: "non-WebSocket remote URL", + options: { mode: "remote" as const, remoteUrl: "https://example.invalid" }, + expectedError: "URL must start with ws:// or wss://", + }, + { + label: "unsupported daemon runtime while daemon install is skipped", + options: { daemonRuntime: "bogus" as never, installDaemon: false }, + expectedError: "Invalid --daemon-runtime", + }, + { + label: "unsupported node manager", + options: { nodeManager: "bogus" as never }, + expectedError: "Invalid --node-manager", + }, + ])( + "rejects $label before non-interactive setup without reset", + async ({ options, expectedError }) => { + const runtime = makeRuntime(); + + await setupWizardCommand({ nonInteractive: true, acceptRisk: true, ...options }, runtime); + + expect(runtime.error).toHaveBeenCalledWith(expect.stringContaining(expectedError)); + expect(runtime.exit).toHaveBeenCalledWith(1); + expect(mocks.readConfigFileSnapshot).not.toHaveBeenCalled(); + expect(mocks.handleReset).not.toHaveBeenCalled(); + expect(mocks.runNonInteractiveSetup).not.toHaveBeenCalled(); + expect(mocks.runInteractiveSetup).not.toHaveBeenCalled(); + expect(mocks.runGuidedOnboarding).not.toHaveBeenCalled(); + }, + ); + it("validates dependent gateway options before reset", async () => { const runtime = makeRuntime(); diff --git a/src/commands/onboard.ts b/src/commands/onboard.ts index 154c9cec4ad6..b2423870931f 100644 --- a/src/commands/onboard.ts +++ b/src/commands/onboard.ts @@ -28,6 +28,7 @@ import { resolveDeprecatedAuthChoiceReplacement, } from "./auth-choice-legacy.js"; import { formatAuthChoiceChoicesForCli } from "./auth-choice-options.js"; +import { isGatewayDaemonRuntime } from "./daemon-runtime.js"; import { applyCustomApiConfig, CustomApiError, @@ -42,7 +43,12 @@ import { resolveNonInteractiveApiKey as resolveNonInteractiveCredential } from " import { inferAuthChoiceFromFlags } from "./onboard-non-interactive/local/auth-choice-inference.js"; import { applyNonInteractiveGatewayConfig } from "./onboard-non-interactive/local/gateway-config.js"; import { validateGatewayWebSocketUrl } from "./onboard-remote.js"; -import type { OnboardOptions, ResetScope } from "./onboard-types.js"; +import { + isNodeManagerChoice, + isOnboardFlow, + type OnboardOptions, + type ResetScope, +} from "./onboard-types.js"; const VALID_RESET_SCOPES = new Set(["config", "config+creds+sessions", "full"]); const BUILT_IN_AUTH_CHOICES = ["setup-token", "token", "apiKey", "custom-api-key", "skip"]; @@ -53,7 +59,7 @@ function rejectOption(runtime: RuntimeEnv, message: string): false { return false; } -function validateResetPreflightOptions(opts: OnboardOptions, runtime: RuntimeEnv): boolean { +function validatePreflightOptions(opts: OnboardOptions, runtime: RuntimeEnv): boolean { if (opts.mode !== undefined && opts.mode !== "local" && opts.mode !== "remote") { return rejectOption( runtime, @@ -61,12 +67,9 @@ function validateResetPreflightOptions(opts: OnboardOptions, runtime: RuntimeEnv ); } const choiceValidations: Array = [ - ["--flow", opts.flow, ["quickstart", "advanced", "import"]], ["--gateway-bind", opts.gatewayBind, ["loopback", "tailnet", "lan", "auto", "custom"]], ["--gateway-auth", opts.gatewayAuth, ["token", "password"]], ["--tailscale", opts.tailscale, ["off", "serve", "funnel"]], - ["--node-manager", opts.nodeManager, ["npm", "pnpm", "bun"]], - ["--daemon-runtime", opts.daemonRuntime, ["node"]], [ "--custom-compatibility", opts.customCompatibility, @@ -81,6 +84,18 @@ function validateResetPreflightOptions(opts: OnboardOptions, runtime: RuntimeEnv ); } } + if (opts.flow !== undefined && !isOnboardFlow(opts.flow)) { + return rejectOption( + runtime, + 'Invalid --flow. Use "quickstart", "advanced", "manual", or "import".', + ); + } + if (opts.daemonRuntime !== undefined && !isGatewayDaemonRuntime(opts.daemonRuntime)) { + return rejectOption(runtime, 'Invalid --daemon-runtime. Use "node".'); + } + if (opts.nodeManager !== undefined && !isNodeManagerChoice(opts.nodeManager)) { + return rejectOption(runtime, 'Invalid --node-manager. Use "npm", "pnpm", or "bun".'); + } if ( opts.gatewayPort !== undefined && (!Number.isFinite(opts.gatewayPort) || opts.gatewayPort <= 0 || opts.gatewayPort > 65_535) @@ -426,6 +441,9 @@ export async function setupWizardCommand( normalizedAuthChoice === opts.authChoice && flow === opts.flow ? opts : { ...opts, authChoice: normalizedAuthChoice, flow }; + if (!validatePreflightOptions(normalizedOpts, runtime)) { + return; + } if (normalizedOpts.classic && normalizedOpts.nonInteractive) { runtime.error( "--classic cannot be combined with --non-interactive. Remove --non-interactive to open the classic wizard, or remove --classic for automated setup.", @@ -485,9 +503,6 @@ export async function setupWizardCommand( : runGuidedOnboarding; if (normalizedOpts.reset) { - if (!validateResetPreflightOptions(normalizedOpts, runtime)) { - return; - } const snapshot = await readConfigFileSnapshot(); const baseConfig = snapshot.sourceConfig ?? (snapshot.valid ? snapshot.config : {}); const resetScope: ResetScope = normalizedOpts.resetScope ?? "config+creds+sessions";