From a2bcfef3270a7b4e2a4600c900daeecbf49b6ccb Mon Sep 17 00:00:00 2001 From: Prateek Rungta Date: Sun, 15 Nov 2026 04:23:31 -0500 Subject: [PATCH] feat(cli): let orca fill read its value from stdin (#29850) Add `orca fill --value-stdin` so passwords and other secrets stay out of argv and shell history. Fill drops one trailing newline from stdin (secret readers end with one) and rejects empty stdin so a failed reader cannot silently clear the field; `--value ''` still clears. Share one flag-or-stdin reader with `orca computer type`/`set-value` and the `--body-file -` reader. Computer commands keep their existing semantics: no newline trimming, set-value still accepts empty stdin, and `--text-stdin=true` style keeps reading stdin. Fixes #29849 Co-authored-by: Neil --- skill-guides/orca-cli/references/browser.md | 2 + src/cli/arguments/command-body-input.ts | 10 +- src/cli/arguments/flag-or-stdin-payload.ts | 44 +++++++ src/cli/browser/browser-fill-stdin.test.ts | 113 ++++++++++++++++++ src/cli/browser/browser-interact.ts | 3 +- src/cli/computer/computer-action-flags.ts | 42 +------ src/cli/discovery/flag-help-text.ts | 4 +- src/cli/discovery/root-help-text-secondary.ts | 1 + src/cli/skills/bundled-skill-guides.ts | 2 + src/cli/specs/browser-basic.ts | 6 +- 10 files changed, 176 insertions(+), 51 deletions(-) create mode 100644 src/cli/arguments/flag-or-stdin-payload.ts create mode 100644 src/cli/browser/browser-fill-stdin.test.ts diff --git a/skill-guides/orca-cli/references/browser.md b/skill-guides/orca-cli/references/browser.md index 89eddbb91f3..a80cca96b2d 100644 --- a/skill-guides/orca-cli/references/browser.md +++ b/skill-guides/orca-cli/references/browser.md @@ -32,6 +32,7 @@ ORCA full-screenshot --page ORCA pdf --page --json ORCA click --page --element --json ORCA fill --page --element --value --json + | ORCA fill --page --element --value-stdin --json ORCA type --page --input --json ORCA select --page --element --value --json ORCA check --page --element --json @@ -83,6 +84,7 @@ Browser rules: - Anything not listed above goes through `ORCA exec --page --command ""`, which takes agent-browser's own command syntax (for example `"get text @e3"` or `"is visible @e3"`) and prints JSON. It has no `help` command. - `upload` takes the file input itself or the button that opens a file picker, in any frame, including cross-origin iframes; Orca catches the picker and sets the files on its input. Use `ORCA upload`, not `ORCA exec --command "upload ..."`. On server-hosted pages pass the file input's ref. - `fill` sets a field's whole value in one step, at any size. `type` presses real keys into the focused element, so `minlength`/`maxlength`, key handlers, Enter and Tab act as they would for a person; `ORCA focus` the element first. Use `type` when the page reacts to keystrokes. +- Fill passwords and other secrets with `--value-stdin`, not `--value`: argv is visible to `ps` and shell history. One trailing newline is dropped. - On server-hosted pages `fill` and `type` still insert text the engine's own way. If either does nothing on a custom input there, try `ORCA focus --page --element @e1 --json` then `ORCA inserttext --page --text "text" --json`. - A client-hosted page renders in the paired desktop's browser engine, so every command against it needs that desktop online and returns `browser_host_unavailable` while it is closed, asleep, or disconnected. Server-hosted pages run with no desktop attached; prefer them for long or unattended automation. diff --git a/src/cli/arguments/command-body-input.ts b/src/cli/arguments/command-body-input.ts index b41544c52f8..ebbbedbe577 100644 --- a/src/cli/arguments/command-body-input.ts +++ b/src/cli/arguments/command-body-input.ts @@ -1,5 +1,6 @@ import { readFile } from 'node:fs/promises' import { isAbsolute, join } from 'node:path' +import { readStdin } from './flag-or-stdin-payload' import { getRequiredStringFlag, getRequiredStringFlagAllowingEmpty } from './flags' import { RuntimeClientError } from '../runtime/connection/types' @@ -66,12 +67,5 @@ async function readBodyFile(path: string, cwd: string): Promise { if (path !== '-') { return readFile(isAbsolute(path) ? path : join(cwd, path), 'utf8') } - if (process.stdin.isTTY) { - throw new RuntimeClientError('invalid_argument', 'stdin body requested but stdin is a TTY') - } - const chunks: Buffer[] = [] - for await (const chunk of process.stdin) { - chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk))) - } - return Buffer.concat(chunks).toString('utf8') + return readStdin('body') } diff --git a/src/cli/arguments/flag-or-stdin-payload.ts b/src/cli/arguments/flag-or-stdin-payload.ts new file mode 100644 index 00000000000..87655e18d80 --- /dev/null +++ b/src/cli/arguments/flag-or-stdin-payload.ts @@ -0,0 +1,44 @@ +import { getRequiredStringFlag, getRequiredStringFlagAllowingEmpty } from './flags' +import { RuntimeClientError } from '../runtime/connection/types' + +/** Reads `--`, or stdin for `---stdin` so secrets stay out of argv and shell history. */ +export async function getFlagOrStdinPayload( + flags: Map, + name: 'text' | 'value', + { + allowEmptyStdin = false, + trimTrailingNewline = false + }: { allowEmptyStdin?: boolean; trimTrailingNewline?: boolean } = {} +): Promise { + const stdinFlag = `${name}-stdin` + if (flags.has(stdinFlag)) { + if (flags.has(name)) { + throw new RuntimeClientError( + 'invalid_argument', + `Use either --${name} or --${stdinFlag}, not both` + ) + } + const raw = await readStdin('payload') + // Secret readers (`pass`, `op read`, `echo`) end with a newline that breaks number/date inputs. + const payload = trimTrailingNewline ? raw.replace(/\r?\n$/, '') : raw + // An empty pipe usually means the secret reader failed; don't silently clear the field. + if (payload.length === 0 && !allowEmptyStdin) { + throw new RuntimeClientError('invalid_argument', `Missing ${name} from stdin`) + } + return payload + } + return name === 'value' + ? getRequiredStringFlagAllowingEmpty(flags, name) + : getRequiredStringFlag(flags, name) +} + +export async function readStdin(label: string): Promise { + if (process.stdin.isTTY) { + throw new RuntimeClientError('invalid_argument', `stdin ${label} requested but stdin is a TTY`) + } + const chunks: Buffer[] = [] + for await (const chunk of process.stdin) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk))) + } + return Buffer.concat(chunks).toString('utf8') +} diff --git a/src/cli/browser/browser-fill-stdin.test.ts b/src/cli/browser/browser-fill-stdin.test.ts new file mode 100644 index 00000000000..f85616523ce --- /dev/null +++ b/src/cli/browser/browser-fill-stdin.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { callMock, runtimeClientConstructorMock, serveOrcaAppMock, getDefaultUserDataPathMock } = + vi.hoisted(() => ({ + callMock: vi.fn(), + runtimeClientConstructorMock: vi.fn(), + serveOrcaAppMock: vi.fn(), + getDefaultUserDataPathMock: vi.fn(() => '/tmp/orca-user-data') + })) + +vi.mock('../runtime/client', async () => { + const { createRuntimeClientModuleMock } = await import('../index-test-harness.js') + return createRuntimeClientModuleMock({ + callMock, + runtimeClientConstructorMock, + serveOrcaAppMock, + getDefaultUserDataPathMock + }) +}) + +vi.mock('../runtime/launch', async (importOriginal) => ({ + ...(await importOriginal>()), + serveOrcaApp: serveOrcaAppMock +})) + +vi.mock('../runtime/profile/metadata', async (importOriginal) => ({ + ...(await importOriginal>()), + getDefaultUserDataPath: getDefaultUserDataPathMock +})) + +import { main } from '../index' +import { okFixture } from '../runtime/fixtures/runtime-rpc-fixtures' + +function mockStdin(chunks: string[]): { restore: () => void } { + const stdin = process.stdin + const previous = [ + ['isTTY', Object.getOwnPropertyDescriptor(stdin, 'isTTY')], + [Symbol.asyncIterator, Object.getOwnPropertyDescriptor(stdin, Symbol.asyncIterator)] + ] as const + Object.defineProperty(stdin, 'isTTY', { configurable: true, value: false }) + Object.defineProperty(stdin, Symbol.asyncIterator, { + configurable: true, + value: async function* () { + for (const chunk of chunks) { + yield Buffer.from(chunk) + } + } + }) + return { + restore: () => { + for (const [key, descriptor] of previous) { + if (descriptor) { + Object.defineProperty(stdin, key, descriptor) + } else { + Reflect.deleteProperty(stdin, key) + } + } + } + } +} + +describe('orca fill --value-stdin', () => { + beforeEach(() => { + callMock.mockReset() + callMock.mockImplementation(async () => okFixture('req_fill', { filled: '@e3' })) + process.exitCode = undefined + }) + + afterEach(() => { + vi.restoreAllMocks() + process.exitCode = undefined + }) + + it('sends the stdin payload minus one trailing newline as the fill value', async () => { + vi.spyOn(console, 'log').mockImplementation(() => {}) + const stdin = mockStdin(['hunter', '2\n\r\n']) + try { + await main( + ['fill', '--element', '@e3', '--value-stdin', '--page', 'page-1', '--json'], + '/tmp/not-an-orca-worktree' + ) + } finally { + stdin.restore() + } + + expect(callMock).toHaveBeenCalledWith( + 'browser.fill', + expect.objectContaining({ element: '@e3', value: 'hunter2\n', page: 'page-1' }) + ) + expect(process.exitCode).toBeUndefined() + }) + + it.each([ + [['--value', 'secret', '--value-stdin'], 'Use either --value or --value-stdin, not both'], + [['--value-stdin'], 'Missing value from stdin'] + ])('rejects %j before calling the runtime', async (valueFlags, message) => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const stdin = mockStdin(['\n']) + try { + await main( + ['fill', '--element', '@e3', ...valueFlags, '--page', 'page-1'], + '/tmp/not-an-orca-worktree' + ) + } finally { + stdin.restore() + } + + expect(callMock).not.toHaveBeenCalledWith('browser.fill', expect.anything()) + expect(errorSpy.mock.calls.flat().join('\n')).toContain(message) + expect(errorSpy.mock.calls.flat().join('\n')).not.toContain('secret') + expect(process.exitCode).toBe(1) + }) +}) diff --git a/src/cli/browser/browser-interact.ts b/src/cli/browser/browser-interact.ts index ddbbdd53154..bb419da808d 100644 --- a/src/cli/browser/browser-interact.ts +++ b/src/cli/browser/browser-interact.ts @@ -13,6 +13,7 @@ import type { BrowserUploadResult } from '../../shared/browser/runtime/runtime-browser-contracts' import type { CommandHandler } from '../dispatch/dispatch' +import { getFlagOrStdinPayload } from '../arguments/flag-or-stdin-payload' import { resolveRepoPathArgument } from '../projects/repo-path-arguments' import { printResult } from '../output/command-result-output' import { @@ -52,7 +53,7 @@ export const BROWSER_INTERACT_HANDLERS: Record = { }, fill: async ({ flags, client, cwd, json }) => { const element = getRequiredStringFlag(flags, 'element') - const value = getRequiredStringFlagAllowingEmpty(flags, 'value') + const value = await getFlagOrStdinPayload(flags, 'value', { trimTrailingNewline: true }) const target = await getBrowserCommandTarget(flags, cwd, client) const result = await client.call('browser.fill', { element, diff --git a/src/cli/computer/computer-action-flags.ts b/src/cli/computer/computer-action-flags.ts index 936164ddb51..61ed5ecba26 100644 --- a/src/cli/computer/computer-action-flags.ts +++ b/src/cli/computer/computer-action-flags.ts @@ -8,9 +8,9 @@ import { getOptionalNumberFlag, getOptionalPositiveIntegerFlag, getOptionalStringFlag, - getRequiredStringFlag, - getRequiredStringFlagAllowingEmpty + getRequiredStringFlag } from '../arguments/flags' +import { getFlagOrStdinPayload } from '../arguments/flag-or-stdin-payload' import { RuntimeClientError } from '../runtime/connection/types' import { validateDragTarget, @@ -140,7 +140,7 @@ export function getComputerDragActionFlags(flags: Map) export async function getComputerTextActionFlags(flags: Map): Promise<{ text: string }> { - return { text: await getTextPayload(flags, 'text') } + return { text: await getFlagOrStdinPayload(flags, 'text') } } export async function getComputerTypeTextActionFlags( @@ -190,44 +190,10 @@ export async function getComputerSetValueActionFlags( }> { return { elementIndex: getRequiredNonNegativeIntegerFlag(flags, 'element-index'), - value: await getTextPayload(flags, 'value') + value: await getFlagOrStdinPayload(flags, 'value', { allowEmptyStdin: true }) } } -async function getTextPayload( - flags: Map, - name: 'text' | 'value' -): Promise { - const stdinFlag = `${name}-stdin` - if (flags.has(stdinFlag)) { - if (flags.has(name)) { - throw new RuntimeClientError( - 'invalid_argument', - `Use either --${name} or --${stdinFlag}, not both` - ) - } - const payload = await readStdin() - if (name === 'text' && payload.length === 0) { - throw new RuntimeClientError('invalid_argument', 'Missing text from stdin') - } - return payload - } - return name === 'value' - ? getRequiredStringFlagAllowingEmpty(flags, name) - : getRequiredStringFlag(flags, name) -} - -async function readStdin(): Promise { - if (process.stdin.isTTY) { - throw new RuntimeClientError('invalid_argument', 'stdin payload requested but stdin is a TTY') - } - const chunks: Buffer[] = [] - for await (const chunk of process.stdin) { - chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk))) - } - return Buffer.concat(chunks).toString('utf8') -} - function getOptionalPositiveNumberFlag( flags: Map, name: string diff --git a/src/cli/discovery/flag-help-text.ts b/src/cli/discovery/flag-help-text.ts index 6142fef2f20..442abdce867 100644 --- a/src/cli/discovery/flag-help-text.ts +++ b/src/cli/discovery/flag-help-text.ts @@ -80,7 +80,7 @@ export const FLAG_HELP_TEXT: Record = { shell: '--shell Windows shell the terminal itself runs as', terminal: '--terminal Runtime-issued terminal handle', text: '--text Text payload to send or type', - 'text-stdin': '--text-stdin Read text payload from stdin', + 'text-stdin': '--text-stdin Read text payload from stdin', 'task-id': '--task-id Task id to include in orchestration payload JSON', 'task-title': '--task-title Concise title for an orchestration task', 'dispatch-id': '--dispatch-id Dispatch id to include in orchestration payload JSON', @@ -124,7 +124,7 @@ export const FLAG_HELP_TEXT: Record = { '--extra-agent-args= Extra agent arguments for fresh runs; empty clears', 'workspace-mode': '--workspace-mode existing or new-per-run', 'missed-run-grace-minutes': '--missed-run-grace-minutes Missed-run grace window', - 'value-stdin': '--value-stdin Read set-value payload from stdin', + 'value-stdin': '--value-stdin Read value payload from stdin', 'window-id': '--window-id Target a window id from list-windows', 'window-index': '--window-index Target a window index from list-windows', element: '--element Element ref from snapshot (e.g. e3)', diff --git a/src/cli/discovery/root-help-text-secondary.ts b/src/cli/discovery/root-help-text-secondary.ts index 8010dbc2bad..b3e30303edb 100644 --- a/src/cli/discovery/root-help-text-secondary.ts +++ b/src/cli/discovery/root-help-text-secondary.ts @@ -144,6 +144,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' --element Element ref from snapshot (e.g. @e3)', ' --url URL to navigate to', ' --value Value to fill or select', + ' --value-stdin Read the fill value from stdin (keeps secrets out of argv)', ' --input Text to type at current focus (no element needed)', ' --expression JavaScript expression to evaluate', ' --key Key to press (Enter, Tab, Escape, Control+a, etc.)', diff --git a/src/cli/skills/bundled-skill-guides.ts b/src/cli/skills/bundled-skill-guides.ts index d1daac9f3b3..bdb826279c8 100644 --- a/src/cli/skills/bundled-skill-guides.ts +++ b/src/cli/skills/bundled-skill-guides.ts @@ -705,6 +705,7 @@ const ORCA_CLI_BROWSER_REFERENCE_MARKDOWN = [ "ORCA pdf --page --json", "ORCA click --page --element --json", "ORCA fill --page --element --value --json", + " | ORCA fill --page --element --value-stdin --json", "ORCA type --page --input --json", "ORCA select --page --element --value --json", "ORCA check --page --element --json", @@ -756,6 +757,7 @@ const ORCA_CLI_BROWSER_REFERENCE_MARKDOWN = [ "- Anything not listed above goes through `ORCA exec --page --command \"\"`, which takes agent-browser's own command syntax (for example `\"get text @e3\"` or `\"is visible @e3\"`) and prints JSON. It has no `help` command.", "- `upload` takes the file input itself or the button that opens a file picker, in any frame, including cross-origin iframes; Orca catches the picker and sets the files on its input. Use `ORCA upload`, not `ORCA exec --command \"upload ...\"`. On server-hosted pages pass the file input's ref.", "- `fill` sets a field's whole value in one step, at any size. `type` presses real keys into the focused element, so `minlength`/`maxlength`, key handlers, Enter and Tab act as they would for a person; `ORCA focus` the element first. Use `type` when the page reacts to keystrokes.", + "- Fill passwords and other secrets with `--value-stdin`, not `--value`: argv is visible to `ps` and shell history. One trailing newline is dropped.", "- On server-hosted pages `fill` and `type` still insert text the engine's own way. If either does nothing on a custom input there, try `ORCA focus --page --element @e1 --json` then `ORCA inserttext --page --text \"text\" --json`.", "- A client-hosted page renders in the paired desktop's browser engine, so every command against it needs that desktop online and returns `browser_host_unavailable` while it is closed, asleep, or disconnected. Server-hosted pages run with no desktop attached; prefer them for long or unattended automation.", "", diff --git a/src/cli/specs/browser-basic.ts b/src/cli/specs/browser-basic.ts index 314c2ba418d..250e94e15d2 100644 --- a/src/cli/specs/browser-basic.ts +++ b/src/cli/specs/browser-basic.ts @@ -63,8 +63,10 @@ export const BROWSER_BASIC_COMMAND_SPECS: CommandSpec[] = withBrowserPageTarget( { path: ['fill'], summary: 'Clear and fill a browser input by ref', - usage: 'orca fill --element --value [--worktree ] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'element', 'value', 'worktree'] + usage: + 'orca fill --element (--value | --value-stdin) [--worktree ] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'element', 'value', 'value-stdin', 'worktree'], + notes: ['--value-stdin drops one trailing newline and rejects empty input.'] }, { path: ['type'],