diff --git a/packages/cli/src/commands/play.ts b/packages/cli/src/commands/play.ts index b0398c189..d237c555f 100644 --- a/packages/cli/src/commands/play.ts +++ b/packages/cli/src/commands/play.ts @@ -8,12 +8,20 @@ export const examples: Example[] = [ ["Use a custom port", "hyperframes play --port 8080"], ["Start without opening the browser", "hyperframes play --no-open"], ["Open with a specific browser", "hyperframes play --browser-path /usr/bin/chromium"], + [ + "Open with CDP enabled (requires browser path + isolated profile)", + "hyperframes play --browser-path /usr/bin/chromium --user-data-dir /tmp/hf-profile --remote-debugging-port 9222", + ], ]; import { resolve, dirname } from "node:path"; import * as clack from "@clack/prompts"; import { c } from "../ui/colors.js"; import { resolveProject } from "../utils/project.js"; -import { openBrowser, parseRemoteDebuggingPort } from "../utils/openBrowser.js"; +import { + openBrowser, + parseRemoteDebuggingPort, + validateRemoteDebuggingPortDeps, +} from "../utils/openBrowser.js"; export default defineCommand({ meta: { name: "play", description: "Play a composition in a lightweight browser player" }, @@ -48,18 +56,28 @@ export default defineCommand({ process.exitCode = 1; return; } - // Validation: --remote-debugging-port requires --browser-path and --user-data-dir - if (args["remote-debugging-port"]) { - if (!args["browser-path"]) { - clack.log.error("--remote-debugging-port requires --browser-path"); - process.exitCode = 1; - return; - } - if (!args["user-data-dir"]) { - clack.log.error("--remote-debugging-port requires --user-data-dir"); - process.exitCode = 1; - return; - } + // Validation: --remote-debugging-port deps + const depsError = validateRemoteDebuggingPortDeps({ + browserPath: args["browser-path"] as string | undefined, + userDataDir: args["user-data-dir"] as string | undefined, + remoteDebuggingPort: args["remote-debugging-port"] as string | undefined, + }); + if (depsError) { + clack.log.error(depsError); + process.exitCode = 1; + return; + } + // Parse --remote-debugging-port before any server setup so an invalid value + // exits cleanly instead of leaving an orphan listening socket behind. + let remoteDebuggingPort: number | undefined; + try { + remoteDebuggingPort = parseRemoteDebuggingPort( + args["remote-debugging-port"] as string | undefined, + ); + } catch (err) { + clack.log.error((err as Error).message); + process.exitCode = 1; + return; } // Resolve runtime path — same logic as studioServer.ts @@ -185,16 +203,6 @@ export default defineCommand({ console.log(); console.log(` ${c.dim("Press Ctrl+C to stop")}`); console.log(); - let remoteDebuggingPort: number | undefined; - if (args["remote-debugging-port"]) { - try { - remoteDebuggingPort = parseRemoteDebuggingPort(args["remote-debugging-port"]); - } catch (err) { - clack.log.error((err as Error).message); - process.exitCode = 1; - return; - } - } if (args.open) { void openBrowser(url, { diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 711e7acf2..9ce9afc8b 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -9,6 +9,10 @@ export const examples: Example[] = [ ["Force a new server even if one is already running", "hyperframes preview --force-new"], ["Start without opening the browser", "hyperframes preview --no-open"], ["Open with a specific browser", "hyperframes preview --browser-path /usr/bin/chromium"], + [ + "Open with CDP enabled (requires browser path + isolated profile)", + "hyperframes preview --browser-path /usr/bin/chromium --user-data-dir /tmp/hf-profile --remote-debugging-port 9222", + ], ["List all active preview servers", "hyperframes preview --list"], ["Kill all active preview servers", "hyperframes preview --kill-all"], ]; @@ -19,7 +23,11 @@ import { createRequire } from "node:module"; import * as clack from "@clack/prompts"; import { c } from "../ui/colors.js"; import { isDevMode } from "../utils/env.js"; -import { openBrowser, parseRemoteDebuggingPort } from "../utils/openBrowser.js"; +import { + openBrowser, + parseRemoteDebuggingPort, + validateRemoteDebuggingPortDeps, +} from "../utils/openBrowser.js"; import { lintProject } from "../utils/lintProject.js"; import { formatLintFindings } from "../utils/lintFormat.js"; import { @@ -135,32 +143,30 @@ export default defineCommand({ process.exitCode = 1; return; } - // Validation: --remote-debugging-port requires --browser-path and --user-data-dir - if (args["remote-debugging-port"]) { - if (!args["browser-path"]) { - clack.log.error("--remote-debugging-port requires --browser-path"); - process.exitCode = 1; - return; - } - if (!args["user-data-dir"]) { - clack.log.error("--remote-debugging-port requires --user-data-dir"); - process.exitCode = 1; - return; - } + // Validation: --remote-debugging-port deps + const depsError = validateRemoteDebuggingPortDeps({ + browserPath: args["browser-path"] as string | undefined, + userDataDir: args["user-data-dir"] as string | undefined, + remoteDebuggingPort: args["remote-debugging-port"] as string | undefined, + }); + if (depsError) { + clack.log.error(depsError); + process.exitCode = 1; + return; } const noOpen = !args.open; const browserPath = args["browser-path"] as string | undefined; const userDataDir = args["user-data-dir"] as string | undefined; let remoteDebuggingPort: number | undefined; - if (args["remote-debugging-port"]) { - try { - remoteDebuggingPort = parseRemoteDebuggingPort(args["remote-debugging-port"]); - } catch (err) { - clack.log.error((err as Error).message); - process.exitCode = 1; - return; - } + try { + remoteDebuggingPort = parseRemoteDebuggingPort( + args["remote-debugging-port"] as string | undefined, + ); + } catch (err) { + clack.log.error((err as Error).message); + process.exitCode = 1; + return; } if (isDevMode()) { diff --git a/packages/cli/src/utils/openBrowser.test.ts b/packages/cli/src/utils/openBrowser.test.ts index e034c2171..3de1f307b 100644 --- a/packages/cli/src/utils/openBrowser.test.ts +++ b/packages/cli/src/utils/openBrowser.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect } from "vitest"; -import { buildBrowserArgs, parseRemoteDebuggingPort } from "./openBrowser.js"; +import { + buildBrowserArgs, + parseRemoteDebuggingPort, + validateRemoteDebuggingPortDeps, +} from "./openBrowser.js"; describe("buildBrowserArgs", () => { it("returns only the URL when no options are given", () => { @@ -38,18 +42,16 @@ describe("buildBrowserArgs", () => { ).toEqual(["--user-data-dir=C:\\Documents and Settings\\profile", "http://localhost:3002"]); }); - it("prepends --remote-debugging-port before the URL", () => { + it("omits --remote-debugging-port when userDataDir is missing (defense in depth)", () => { + // The CLI validation layer rejects this combination upstream, but + // buildBrowserArgs must not leak a CDP endpoint into the user's main + // profile even if a caller bypasses that check. expect( buildBrowserArgs("http://localhost:3002", { browserPath: "/usr/bin/chromium", - userDataDir: "/tmp/hf-profile", remoteDebuggingPort: 9222, }), - ).toEqual([ - "--user-data-dir=/tmp/hf-profile", - "--remote-debugging-port=9222", - "http://localhost:3002", - ]); + ).toEqual(["http://localhost:3002"]); }); it("includes all flags together", () => { @@ -114,3 +116,45 @@ describe("parseRemoteDebuggingPort", () => { expect(() => parseRemoteDebuggingPort("22.5")).toThrow(); }); }); + +describe("validateRemoteDebuggingPortDeps", () => { + it("returns null when --remote-debugging-port is not set", () => { + expect(validateRemoteDebuggingPortDeps({})).toBeNull(); + }); + + it("returns null when all required flags are present", () => { + expect( + validateRemoteDebuggingPortDeps({ + browserPath: "/usr/bin/chromium", + userDataDir: "/tmp/hf-profile", + remoteDebuggingPort: "9222", + }), + ).toBeNull(); + }); + + it("requires --browser-path when --remote-debugging-port is set", () => { + expect( + validateRemoteDebuggingPortDeps({ + userDataDir: "/tmp/hf-profile", + remoteDebuggingPort: "9222", + }), + ).toBe("--remote-debugging-port requires --browser-path"); + }); + + it("requires --user-data-dir when --remote-debugging-port is set", () => { + expect( + validateRemoteDebuggingPortDeps({ + browserPath: "/usr/bin/chromium", + remoteDebuggingPort: "9222", + }), + ).toBe("--remote-debugging-port requires --user-data-dir"); + }); + + it("reports --browser-path first when both deps are missing", () => { + expect( + validateRemoteDebuggingPortDeps({ + remoteDebuggingPort: "9222", + }), + ).toBe("--remote-debugging-port requires --browser-path"); + }); +}); diff --git a/packages/cli/src/utils/openBrowser.ts b/packages/cli/src/utils/openBrowser.ts index d09d2bd42..1bb3ad9b3 100644 --- a/packages/cli/src/utils/openBrowser.ts +++ b/packages/cli/src/utils/openBrowser.ts @@ -6,29 +6,35 @@ export interface OpenBrowserOptions { remoteDebuggingPort?: number; } -/** - * Validate and parse a --remote-debugging-port value. - * Returns the port number or undefined if not provided. - * Throws if the value is not a valid integer in 1..65535. - */ -export function parseRemoteDebuggingPort(value: unknown): number | undefined { - if (value === undefined || value === null || value === "") return undefined; - - const text = String(value); - - if (!/^\d+$/.test(text)) { +export function parseRemoteDebuggingPort(value: string | undefined): number | undefined { + if (value === undefined || value === "") return undefined; + if (!/^\d+$/.test(value)) { throw new Error("--remote-debugging-port must be an integer between 1 and 65535"); } - - const port = Number(text); - - if (!Number.isInteger(port) || port < 1 || port > 65535) { + const port = Number(value); + if (port < 1 || port > 65535) { throw new Error("--remote-debugging-port must be an integer between 1 and 65535"); } - return port; } +export interface RemoteDebuggingPortDeps { + browserPath?: string; + userDataDir?: string; + remoteDebuggingPort?: string; +} + +/** + * Returns an error message if --remote-debugging-port is set without its required + * dependencies (--browser-path and --user-data-dir), or null if everything is OK. + */ +export function validateRemoteDebuggingPortDeps(deps: RemoteDebuggingPortDeps): string | null { + if (!deps.remoteDebuggingPort) return null; + if (!deps.browserPath) return "--remote-debugging-port requires --browser-path"; + if (!deps.userDataDir) return "--remote-debugging-port requires --user-data-dir"; + return null; +} + /** * Build the argument list for spawning a browser process. * @@ -39,7 +45,11 @@ export function buildBrowserArgs(url: string, options: OpenBrowserOptions): stri if (options.userDataDir) { args.push(`--user-data-dir=${options.userDataDir}`); } - if (options.remoteDebuggingPort !== undefined) { + // Defense-in-depth: only emit --remote-debugging-port when paired with an + // isolated --user-data-dir. Without an isolated profile the CDP endpoint + // would expose the user's main browser session, which is the whole reason + // the CLI validation layer requires both flags together. + if (options.remoteDebuggingPort !== undefined && options.userDataDir) { args.push(`--remote-debugging-port=${options.remoteDebuggingPort}`); } args.push(url);