fix(onboard): reject invalid setup options before writes (#111680)

* fix(onboard): validate setup options before mutation

* fix(onboard): keep flow helper type local
This commit is contained in:
Peter Steinberger
2026-07-19 23:05:23 -07:00
committed by GitHub
parent 8c9ecd9242
commit 250e636ffd
9 changed files with 171 additions and 35 deletions
+15 -11
View File
@@ -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();
+3 -3
View File
@@ -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<string, unknown>),
gatewayPort: gatewayPort ?? undefined,
gatewayPort,
gatewayBind: opts.gatewayBind as GatewayBind | undefined,
gatewayAuth: opts.gatewayAuth as GatewayAuthChoice | undefined,
gatewayToken: opts.gatewayToken as string | undefined,
+37 -1
View File
@@ -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"]);
+30 -3
View File
@@ -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<string, (typeof command.options)[number]>();
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<boolean> {
const { readConfigFileSnapshot } = await import("../../config/config.js");
const snapshot = await readConfigFileSnapshot();
@@ -89,13 +108,19 @@ async function runOnboardingEntry(
runtime: RuntimeEnv,
): Promise<void> {
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 <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)",