From fd4da9e7169045644d0e2c006d388df9dc79e91a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 05:10:49 -0700 Subject: [PATCH] fix(serve): validate direct binary options --- src/cli/command-suggestion.ts | 27 +----- src/cli/handlers/core.ts | 36 +++----- src/cli/serve-electron-flag-parity.test.ts | 18 ++-- src/main/index.ts | 42 +--------- src/main/startup/serve-mode-argv.ts | 3 +- src/main/startup/serve-options.test.ts | 95 ++++++++++++++++++++++ src/main/startup/serve-options.ts | 78 ++++++++++++++++++ src/shared/edit-distance.ts | 23 ++++++ src/shared/serve-option-validation.test.ts | 59 ++++++++++++++ src/shared/serve-option-validation.ts | 66 +++++++++++++++ 10 files changed, 348 insertions(+), 99 deletions(-) create mode 100644 src/main/startup/serve-options.test.ts create mode 100644 src/main/startup/serve-options.ts create mode 100644 src/shared/edit-distance.ts create mode 100644 src/shared/serve-option-validation.test.ts create mode 100644 src/shared/serve-option-validation.ts diff --git a/src/cli/command-suggestion.ts b/src/cli/command-suggestion.ts index db481de6ea8..6bc6d6eee0b 100644 --- a/src/cli/command-suggestion.ts +++ b/src/cli/command-suggestion.ts @@ -1,4 +1,7 @@ import { specPaths, type CommandSpec } from './command-spec' +import { levenshtein } from '../shared/edit-distance' + +export { levenshtein } from '../shared/edit-distance' // Why: rank the live registry so typo recovery cannot drift from accepted paths. @@ -46,30 +49,6 @@ export type CommandErrorData = { nextSteps: string[] } -export function levenshtein(a: string, b: string): number { - const m = a.length - const n = b.length - if (m === 0) { - return n - } - if (n === 0) { - return m - } - let prev = Array.from({ length: n + 1 }, (_, index) => index) - let curr = Array.from({ length: n + 1 }, () => 0) - for (let i = 1; i <= m; i += 1) { - curr[0] = i - for (let j = 1; j <= n; j += 1) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1 - curr[j] = Math.min(prev[j] + 1, curr[j - 1] + 1, prev[j - 1] + cost) - } - const swap = prev - prev = curr - curr = swap - } - return prev[n] -} - // Why: one bounded near-match ranking keeps command and flag recovery consistent. function rankByDistance(scored: { label: string; distance: number }[]): string[] { return scored diff --git a/src/cli/handlers/core.ts b/src/cli/handlers/core.ts index 145540bb627..d44873f47fd 100644 --- a/src/cli/handlers/core.ts +++ b/src/cli/handlers/core.ts @@ -3,6 +3,7 @@ import type { CommandHandler } from '../dispatch' import { formatCliStatus, formatStatus, printResult } from '../format' import { RuntimeClientError, serveOrcaApp } from '../runtime-client' import { stripElectronRunAsNode } from '../runtime/launch' +import { getServeOptionValidationError } from '../../shared/serve-option-validation' function envRecord(): Record { // Why: the `orca` launcher runs Orca's Electron binary as Node, so this CLI @@ -92,31 +93,16 @@ export const CORE_HANDLERS: Record = { printResult(result, json, formatCliStatus) }, serve: async ({ flags, json }) => { - if (flags.get('no-pairing') === true && flags.get('mobile-pairing') === true) { - throw new RuntimeClientError( - 'invalid_argument', - 'Use either --mobile-pairing or --no-pairing, not both.' - ) - } - if (flags.get('recipe-json') === true && flags.get('no-pairing') === true) { - throw new RuntimeClientError( - 'invalid_argument', - 'Recipe JSON output requires runtime pairing; remove --no-pairing.' - ) - } - if (flags.get('recipe-json') === true && flags.get('mobile-pairing') === true) { - throw new RuntimeClientError( - 'invalid_argument', - 'Recipe JSON output requires runtime pairing; remove --mobile-pairing.' - ) - } - const projectRoot = - typeof flags.get('project-root') === 'string' ? (flags.get('project-root') as string) : null - if (flags.get('recipe-json') === true && !projectRoot) { - throw new RuntimeClientError( - 'invalid_argument', - 'Recipe JSON output requires --project-root.' - ) + const projectRootValue = flags.get('project-root') + const projectRoot = typeof projectRootValue === 'string' ? projectRootValue : null + const validationError = getServeOptionValidationError({ + noPairing: flags.get('no-pairing') === true, + mobilePairing: flags.get('mobile-pairing') === true, + recipeJson: flags.get('recipe-json') === true, + projectRoot + }) + if (validationError) { + throw new RuntimeClientError('invalid_argument', validationError) } const port = getOptionalServePort(flags) const exitCode = await serveOrcaApp({ diff --git a/src/cli/serve-electron-flag-parity.test.ts b/src/cli/serve-electron-flag-parity.test.ts index 1abcf84ef64..a4964e1a824 100644 --- a/src/cli/serve-electron-flag-parity.test.ts +++ b/src/cli/serve-electron-flag-parity.test.ts @@ -35,7 +35,7 @@ describe('serve flag parity between the CLI spec and the Electron argv rewrite', expect(normalizeServeModeArgv(argv)).toEqual(expected) if (takesValue) { - // The equals form is the other shape `orca serve` accepts, and getServeOptions only reads the next token. + // The equals form is the other shape `orca serve` accepts; normalize it to the internal shape. expect(normalizeServeModeArgv(['/AppRun', 'serve', `--${flag}=value`])).toEqual(expected) } else { // A boolean with an attached value is not a truthy assertion: the CLI reads these as @@ -53,17 +53,19 @@ describe('serve flag parity between the CLI spec and the Electron argv rewrite', }) it('emits the same --serve-* names the CLI spawns with and the main process reads', () => { - // Why source text: serveOrcaApp spawns a real process and getServeOptions is not exported, so - // both ends of the contract are only readable statically. Without this leg the rewrite could - // emit a name nothing reads and every behavioural assertion above would still pass. + // Why source text: serveOrcaApp spawns a real process; keeping both names visible here makes + // the rewrite/parser contract fail loudly if either side drifts. const launchSource = readFileSync(join(process.cwd(), 'src/cli/runtime/launch.ts'), 'utf8') - const mainSource = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') - const start = mainSource.indexOf('function getServeOptions(') + const serveOptionsSource = readFileSync( + join(process.cwd(), 'src/main/startup/serve-options.ts'), + 'utf8' + ) + const start = serveOptionsSource.indexOf('export function getServeOptions(') // Why bound the anchor: an unresolved indexOf slices to EOF and passes vacuously. expect(start).toBeGreaterThanOrEqual(0) - const end = mainSource.indexOf('\n}', start) + const end = serveOptionsSource.indexOf('\n}', start) expect(end).toBeGreaterThan(start) - const getServeOptionsBody = mainSource.slice(start, end) + const getServeOptionsBody = serveOptionsSource.slice(start, end) for (const flag of translatedFlags) { expect(launchSource).toContain(`'--serve-${flag}'`) diff --git a/src/main/index.ts b/src/main/index.ts index 0f46961129a..6c582640f68 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -169,6 +169,7 @@ import { } from './startup/main-process-error-guards' import { enableRendererHeapHeadroom } from './startup/renderer-heap-headroom' import { argvRequestsServeMode, normalizeServeModeArgv } from './startup/serve-mode-argv' +import { getServeOptions, type ServeOptions } from './startup/serve-options' import { ensureVirtualDisplayForHeadlessServe, hasUsableLinuxDisplay, @@ -2080,45 +2081,6 @@ const syntheticTitleSpinnerByPaneKey = new Map< >() let syntheticTitleSpinnerTimer: ReturnType | null = null -type ServeOptions = { - json: boolean - wsPort?: number - pairingAddress: string | null - noPairing: boolean - mobilePairing: boolean - recipeJson: boolean - projectRoot: string | null -} - -function getServeOptions(argv = process.argv): ServeOptions { - const valueAfter = (flag: string): string | null => { - const index = argv.indexOf(flag) - if (index === -1) { - return null - } - const value = argv[index + 1] - return value && !value.startsWith('--') ? value : null - } - const rawPort = valueAfter('--serve-port') - let wsPort: number | undefined - if (rawPort) { - const parsedPort = Number(rawPort) - if (!Number.isInteger(parsedPort) || parsedPort < 0 || parsedPort > 65535) { - throw new Error(`Invalid --serve-port value: ${rawPort}`) - } - wsPort = parsedPort - } - return { - json: argv.includes('--serve-json'), - ...(wsPort !== undefined ? { wsPort } : {}), - pairingAddress: valueAfter('--serve-pairing-address'), - noPairing: argv.includes('--serve-no-pairing'), - mobilePairing: argv.includes('--serve-mobile-pairing'), - recipeJson: argv.includes('--serve-recipe-json'), - projectRoot: valueAfter('--serve-project-root') - } -} - function getBundledWebClientRoot(): string | undefined { const appPath = app.getAppPath() const roots = [ @@ -3376,7 +3338,7 @@ void app.whenReady().then(async () => { const devWsPort = is.dev && !isE2E ? 6769 : undefined let serveOptions: ServeOptions | null = null try { - serveOptions = isServeMode ? getServeOptions() : null + serveOptions = isServeMode ? getServeOptions(process.argv) : null } catch (error) { console.error(error instanceof Error ? error.message : String(error)) app.exit(1) diff --git a/src/main/startup/serve-mode-argv.ts b/src/main/startup/serve-mode-argv.ts index 55f94d895e6..bb3b1b51751 100644 --- a/src/main/startup/serve-mode-argv.ts +++ b/src/main/startup/serve-mode-argv.ts @@ -136,8 +136,7 @@ export function normalizeServeModeArgv(argv: readonly string[]): string[] { next.push(...argv.slice(i)) break } - // Why: the CLI accepts `--port=6768` as well as `--port 6768`, but - // getServeOptions only reads the next token, so `=` must be split apart. + // Why: keep the internal argv shape canonical even though getServeOptions accepts both forms. const eq = token.indexOf('=') const name = eq === -1 ? token : token.slice(0, eq) // Why only the bare form: the CLI reads its serve booleans as `flags.get(name) === true` diff --git a/src/main/startup/serve-options.test.ts b/src/main/startup/serve-options.test.ts new file mode 100644 index 00000000000..037f96569cd --- /dev/null +++ b/src/main/startup/serve-options.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest' +import { getServeOptions } from './serve-options' +import { normalizeServeModeArgv } from './serve-mode-argv' + +describe('getServeOptions', () => { + it('parses a valid launch', () => { + expect( + getServeOptions(['/AppRun', '--serve', '--serve-port', '6768', '--serve-no-pairing']) + ).toEqual({ + json: false, + wsPort: 6768, + pairingAddress: null, + noPairing: true, + mobilePairing: false, + recipeJson: false, + projectRoot: null + }) + }) + + it('accepts equals-form values in the normalized shape', () => { + expect( + getServeOptions([ + '/AppRun', + '--serve', + '--serve-port=6768', + '--serve-pairing-address=127.0.0.1', + '--serve-project-root=/tmp/repo' + ]) + ).toMatchObject({ + wsPort: 6768, + pairingAddress: '127.0.0.1', + projectRoot: '/tmp/repo' + }) + }) + + it('shares cross-flag validation with the CLI-form launch', () => { + const argv = normalizeServeModeArgv([ + '/opt/orca/orca-ide', + 'serve', + '--no-pairing', + '--mobile-pairing' + ]) + expect(() => getServeOptions(argv)).toThrow(/either --mobile-pairing or --no-pairing/i) + }) + + it('rejects recipe JSON without runtime pairing and a project root', () => { + expect(() => + getServeOptions([ + '/AppRun', + '--serve', + '--serve-recipe-json', + '--serve-no-pairing', + '--serve-project-root', + '/tmp/repo' + ]) + ).toThrow(/requires runtime pairing.*--no-pairing/i) + expect(() => getServeOptions(['/AppRun', '--serve', '--serve-recipe-json'])).toThrow( + /requires --project-root/i + ) + }) + + it('rejects a security-shaped typo while allowing Chromium switches', () => { + const normalized = normalizeServeModeArgv(['/AppRun', 'serve', '--no-pairng']) + expect(() => getServeOptions(normalized)).toThrow(/Unknown flag --no-pairng.*--no-pairing/i) + expect( + getServeOptions(['/AppRun', '--serve', '--disable-gpu', '--disable-features=Vulkan']) + .noPairing + ).toBe(false) + }) + + it('ignores serve-looking arguments after the terminator', () => { + expect( + getServeOptions(['/AppRun', '--serve', '--', '--serve-port', '1', '--serve-no-pairing']) + ).toEqual({ + json: false, + pairingAddress: null, + noPairing: false, + mobilePairing: false, + recipeJson: false, + projectRoot: null + }) + }) + + it('requires a port value', () => { + expect(() => getServeOptions(['/AppRun', '--serve', '--serve-port'])).toThrow( + 'Missing value for --serve-port.' + ) + }) + + it.each(['', '--serve-json', '--'])('rejects an unusable port value %j', (value) => { + expect(() => getServeOptions(['/AppRun', '--serve', '--serve-port', value])).toThrow( + 'Missing value for --serve-port.' + ) + }) +}) diff --git a/src/main/startup/serve-options.ts b/src/main/startup/serve-options.ts new file mode 100644 index 00000000000..c25ada0319d --- /dev/null +++ b/src/main/startup/serve-options.ts @@ -0,0 +1,78 @@ +import { + getServeFlagTypoError, + getServeOptionValidationError +} from '../../shared/serve-option-validation' + +export type ServeOptions = { + json: boolean + wsPort?: number + pairingAddress: string | null + noPairing: boolean + mobilePairing: boolean + recipeJson: boolean + projectRoot: string | null +} + +function optionsBeforeTerminator(argv: readonly string[]): readonly string[] { + const terminatorIndex = argv.indexOf('--') + return terminatorIndex === -1 ? argv : argv.slice(0, terminatorIndex) +} + +function valueAfter(argv: readonly string[], flag: string, required: boolean): string | null { + const assignmentPrefix = `${flag}=` + for (let index = 0; index < argv.length; index += 1) { + const token = argv[index]! + if (token.startsWith(assignmentPrefix)) { + const value = token.slice(assignmentPrefix.length) + if (value || !required) { + return value || null + } + throw new Error(`Missing value for ${flag}.`) + } + if (token !== flag) { + continue + } + const value = argv[index + 1] + if (value && !value.startsWith('--')) { + return value + } + if (required) { + throw new Error(`Missing value for ${flag}.`) + } + return null + } + return null +} + +export function getServeOptions(argv: readonly string[]): ServeOptions { + const optionsArgv = optionsBeforeTerminator(argv) + const typoError = getServeFlagTypoError(optionsArgv) + if (typoError) { + throw new Error(typoError) + } + + const rawPort = valueAfter(optionsArgv, '--serve-port', true) + let wsPort: number | undefined + if (rawPort) { + const parsedPort = Number(rawPort) + if (!Number.isInteger(parsedPort) || parsedPort < 0 || parsedPort > 65535) { + throw new Error(`Invalid --serve-port value: ${rawPort}`) + } + wsPort = parsedPort + } + + const options: ServeOptions = { + json: optionsArgv.includes('--serve-json'), + ...(wsPort !== undefined ? { wsPort } : {}), + pairingAddress: valueAfter(optionsArgv, '--serve-pairing-address', false), + noPairing: optionsArgv.includes('--serve-no-pairing'), + mobilePairing: optionsArgv.includes('--serve-mobile-pairing'), + recipeJson: optionsArgv.includes('--serve-recipe-json'), + projectRoot: valueAfter(optionsArgv, '--serve-project-root', false) + } + const validationError = getServeOptionValidationError(options) + if (validationError) { + throw new Error(validationError) + } + return options +} diff --git a/src/shared/edit-distance.ts b/src/shared/edit-distance.ts new file mode 100644 index 00000000000..7a496712b89 --- /dev/null +++ b/src/shared/edit-distance.ts @@ -0,0 +1,23 @@ +export function levenshtein(a: string, b: string): number { + const m = a.length + const n = b.length + if (m === 0) { + return n + } + if (n === 0) { + return m + } + let previous = Array.from({ length: n + 1 }, (_, index) => index) + let current = Array.from({ length: n + 1 }, () => 0) + for (let i = 1; i <= m; i += 1) { + current[0] = i + for (let j = 1; j <= n; j += 1) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1 + current[j] = Math.min(previous[j] + 1, current[j - 1] + 1, previous[j - 1] + cost) + } + const swap = previous + previous = current + current = swap + } + return previous[n] +} diff --git a/src/shared/serve-option-validation.test.ts b/src/shared/serve-option-validation.test.ts new file mode 100644 index 00000000000..730007dfc8b --- /dev/null +++ b/src/shared/serve-option-validation.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest' +import { getServeFlagTypoError, getServeOptionValidationError } from './serve-option-validation' + +const validOptions = { + noPairing: false, + mobilePairing: false, + recipeJson: false, + projectRoot: null +} + +describe('getServeOptionValidationError', () => { + it('accepts compatible options', () => { + expect(getServeOptionValidationError(validOptions)).toBeNull() + }) + + it.each([ + [{ noPairing: true, mobilePairing: true }, /either --mobile-pairing or --no-pairing/i], + [ + { recipeJson: true, noPairing: true, projectRoot: '/tmp/repo' }, + /requires runtime pairing.*--no-pairing/i + ], + [ + { recipeJson: true, mobilePairing: true, projectRoot: '/tmp/repo' }, + /requires runtime pairing.*--mobile-pairing/i + ], + [{ recipeJson: true }, /requires --project-root/i] + ])('rejects incompatible options', (override, expected) => { + expect( + getServeOptionValidationError({ ...validOptions, ...override } as typeof validOptions) + ).toMatch(expected) + }) +}) + +describe('getServeFlagTypoError', () => { + it('accepts exact serve flags and arbitrary Chromium switches', () => { + expect( + getServeFlagTypoError([ + '/opt/orca/orca-ide', + '--serve', + '--serve-no-pairing', + '--disable-gpu', + '--disable-features=Vulkan' + ]) + ).toBeNull() + }) + + it.each(['--no-pairng', '--no-paring', '--mobile-pairng'])( + 'suggests the intended pairing flag for %s', + (flag) => { + expect(getServeFlagTypoError(['/opt/orca/orca-ide', '--serve', flag])).toMatch( + /Unknown flag .*Did you mean --(?:no-pairing|mobile-pairing)\?/i + ) + } + ) + + it('does not reinterpret tokens after --', () => { + expect(getServeFlagTypoError(['/opt/orca/orca-ide', '--serve', '--', '--no-pairng'])).toBeNull() + }) +}) diff --git a/src/shared/serve-option-validation.ts b/src/shared/serve-option-validation.ts new file mode 100644 index 00000000000..c7113b61839 --- /dev/null +++ b/src/shared/serve-option-validation.ts @@ -0,0 +1,66 @@ +import { levenshtein } from './edit-distance' + +export type ServeOptionValidationInput = { + noPairing: boolean + mobilePairing: boolean + recipeJson: boolean + projectRoot: string | null | undefined +} + +export function getServeOptionValidationError(options: ServeOptionValidationInput): string | null { + if (options.noPairing && options.mobilePairing) { + return 'Use either --mobile-pairing or --no-pairing, not both.' + } + if (options.recipeJson && options.noPairing) { + return 'Recipe JSON output requires runtime pairing; remove --no-pairing.' + } + if (options.recipeJson && options.mobilePairing) { + return 'Recipe JSON output requires runtime pairing; remove --mobile-pairing.' + } + if (options.recipeJson && !options.projectRoot) { + return 'Recipe JSON output requires --project-root.' + } + return null +} + +const SERVE_SECURITY_FLAG_NAMES = [ + '--no-pairing', + '--serve-no-pairing', + '--mobile-pairing', + '--serve-mobile-pairing', + '--recipe-json', + '--serve-recipe-json', + '--pairing-address', + '--serve-pairing-address' +] as const + +function flagName(token: string): string { + const equalsIndex = token.indexOf('=') + return equalsIndex === -1 ? token : token.slice(0, equalsIndex) +} + +/** Reject only near-miss pairing flags; Electron/Chromium switches stay open-ended. */ +export function getServeFlagTypoError(argv: readonly string[]): string | null { + for (const token of argv) { + if (token === '--') { + break + } + if (!token.startsWith('--')) { + continue + } + const name = flagName(token) + let suggestion: string | null = null + let bestDistance = 3 + for (const candidate of SERVE_SECURITY_FLAG_NAMES) { + const distance = levenshtein(name, candidate) + if (distance > 0 && distance < bestDistance) { + suggestion = candidate + bestDistance = distance + } + } + if (suggestion) { + return `Unknown flag ${name}. Did you mean ${suggestion}?` + } + } + return null +}