diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index ac520cc657..9ce3160af6 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -66,8 +66,10 @@ export { export { inferFillText, + isSensitiveFillText, readRecordedInputVariableName, recordedInputPlaceholder, + sensitiveFillPlaceholder, validateRecordedInputVariableName, } from './internal/recorded-input.ts'; diff --git a/packages/ad-script/src/internal/recorded-input.ts b/packages/ad-script/src/internal/recorded-input.ts index e74e5e56c2..742934f7d4 100644 --- a/packages/ad-script/src/internal/recorded-input.ts +++ b/packages/ad-script/src/internal/recorded-input.ts @@ -1,3 +1,4 @@ +import type { CommandFlags } from '@agent-device/contracts/command'; import type { SessionAction } from '@agent-device/contracts/session'; import { AppError } from '@agent-device/kernel/errors'; import { REPLAY_VAR_KEY_RE } from './script.ts'; @@ -20,6 +21,26 @@ export function validateRecordedInputVariableName(raw: string): string { return raw; } +/** + * A fill's text is sensitive when the caller marked it: `--record-as` names it for the script, or + * `--text-stdin` kept it out of argv. Sensitive text never appears as a literal on any surface: + * response data, result, recorded action or diagnostics. + */ +export function isSensitiveFillText>( + flags: TFlags | undefined, +): flags is TFlags { + return typeof flags?.recordAs === 'string' || flags?.textStdin === true; +} + +/** What sensitive fill text reads as on any response: its `--record-as` placeholder, else `[REDACTED]`. */ +export function sensitiveFillPlaceholder( + flags: Pick | undefined, +): string { + return typeof flags?.recordAs === 'string' + ? recordedInputPlaceholder(flags.recordAs) + : '[REDACTED]'; +} + export function recordedInputPlaceholder(variableName: string): string { return `\${${variableName}}`; } diff --git a/packages/command-registry/src/batch.ts b/packages/command-registry/src/batch.ts index a21ebed8c8..f0fad3e615 100644 --- a/packages/command-registry/src/batch.ts +++ b/packages/command-registry/src/batch.ts @@ -222,6 +222,7 @@ export function validateAndNormalizeBatchSteps( if (step.flags !== undefined && !isRecord(step.flags)) { throw new AppError('INVALID_ARGS', `Batch step ${index + 1} flags must be an object.`); } + assertNoTextStdinStep(step.flags as Record | undefined, index + 1); if (step.input !== undefined && !isRecord(step.input)) { throw new AppError('INVALID_ARGS', `Batch step ${index + 1} input must be an object.`); } @@ -236,6 +237,23 @@ export function validateAndNormalizeBatchSteps( return normalized; } +/** + * A batch step has no stdin of its own, and a failed step reports its positionals. The CLI refuses + * `textStdin: true` on a step before reading stdin; this refuses it for every other daemon client. + * `false` is the ordinary argv path, and a value of another type is refused per step by the router. + */ +function assertNoTextStdinStep( + flags: Record | undefined, + stepNumber: number, +): void { + if (flags?.textStdin !== true) return; + throw new AppError( + 'INVALID_ARGS', + `Batch step ${stepNumber}: fill textStdin is not supported because batch steps have no stdin of their own.`, + { reason: 'fill_text_stdin_in_batch' }, + ); +} + function buildBatchStepFlags( parentFlags: BatchFlags | Record | undefined, stepFlags: DaemonBatchStep['flags'] | Record | undefined, diff --git a/packages/command-registry/src/flag-definitions-action.ts b/packages/command-registry/src/flag-definitions-action.ts index 733a42454b..ceb6766d2b 100644 --- a/packages/command-registry/src/flag-definitions-action.ts +++ b/packages/command-registry/src/flag-definitions-action.ts @@ -108,6 +108,16 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ projectConfig: true, recorded: false, }, + { + key: 'textStdin', + names: ['--text-stdin'], + type: 'boolean', + usageLabel: '--text-stdin', + usageDescription: + 'Fill: read the text from stdin (max 64 KiB, one trailing newline removed) instead of argv', + projectConfig: false, + recorded: false, + }, { key: 'durationMs', names: ['--duration-ms'], diff --git a/packages/contracts/src/cli-flags.ts b/packages/contracts/src/cli-flags.ts index 01e8162e90..b5d5402c30 100644 --- a/packages/contracts/src/cli-flags.ts +++ b/packages/contracts/src/cli-flags.ts @@ -105,6 +105,8 @@ export type CliFlags = CloudProviderProfileFields & delayMs?: number; /** Fill: publish the live text as a late-bound ${VAR} in a recorded .ad script. */ recordAs?: string; + /** Fill: the text came from the CLI's stdin rather than argv; keep it out of logs and scripts. */ + textStdin?: boolean; durationMs?: number; keyframes?: string; holdMs?: number; diff --git a/packages/contracts/src/client-gesture.ts b/packages/contracts/src/client-gesture.ts index efa8556e64..75f2a7b769 100644 --- a/packages/contracts/src/client-gesture.ts +++ b/packages/contracts/src/client-gesture.ts @@ -134,6 +134,8 @@ export type FillOptions = DeviceCommandBaseOptions & delayMs?: number; /** Publish this fill value as `${VAR}` when script recording is armed. */ recordAs?: string; + /** The text came from the CLI's stdin; keep it out of diagnostics and recorded actions. */ + textStdin?: boolean; verify?: boolean; }; diff --git a/packages/selectors/src/parameterized-recorded-fill.test.ts b/packages/selectors/src/parameterized-recorded-fill.test.ts new file mode 100644 index 0000000000..3f10c31724 --- /dev/null +++ b/packages/selectors/src/parameterized-recorded-fill.test.ts @@ -0,0 +1,51 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { parameterizeSensitiveString } from './parameterized-recorded-fill.ts'; + +test('a value that contains the placeholder collapses the whole string', () => { + assert.equal( + parameterizeSensitiveString( + 'could not type "abc[REDACTED]xyz"', + 'abc[REDACTED]xyz', + '[REDACTED]', + ), + '[REDACTED]', + ); + assert.equal( + parameterizeSensitiveString( + 'could not type "abc${PASSWORD}xyz"', + 'abc${PASSWORD}xyz', + '${PASSWORD}', + ), + '${PASSWORD}', + ); +}); + +test('a value that crosses a placeholder already in the string collapses the whole string', () => { + // `D]tail` starts inside the earlier `[REDACTED]` and ends after it, so no segment holds it. + assert.equal( + parameterizeSensitiveString('typed [REDACTED]tail', 'D]tail', '[REDACTED]'), + '[REDACTED]', + ); +}); + +test('a value found only inside placeholders leaves the string as it is', () => { + assert.equal( + parameterizeSensitiveString('value ${DOLLAR} kept', '$', '${DOLLAR}'), + 'value ${DOLLAR} kept', + ); + assert.equal( + parameterizeSensitiveString('typed ${PASSWORD}', 'PASSWORD', '${PASSWORD}'), + 'typed ${PASSWORD}', + ); +}); + +test('an ordinary value is replaced in place and a second pass changes nothing', () => { + const once = parameterizeSensitiveString( + 'could not type "s3cret" twice: s3cret', + 's3cret', + '[REDACTED]', + ); + assert.equal(once, 'could not type "[REDACTED]" twice: [REDACTED]'); + assert.equal(parameterizeSensitiveString(once, 's3cret', '[REDACTED]'), once); +}); diff --git a/packages/selectors/src/parameterized-recorded-fill.ts b/packages/selectors/src/parameterized-recorded-fill.ts index 93f320ce53..44ee6eaff3 100644 --- a/packages/selectors/src/parameterized-recorded-fill.ts +++ b/packages/selectors/src/parameterized-recorded-fill.ts @@ -362,22 +362,65 @@ function filterSensitiveSelectorCandidates( return value.filter((candidate) => !carries(candidate)); } -function parameterizeSensitiveString(value: string, literal: string, placeholder: string): string { +export function parameterizeSensitiveString( + value: string, + literal: string, + placeholder: string, +): string { // The empty literal (`fill "" --record-as VAR`, #2063) matches inside every string: // it reveals nothing if echoed, so it redacts nothing. Only the fill's own semantic `text` // field is force-parameterized, by `parameterizeRecordedFillPayload`, not here. if (!literal) return value; const unparameterizedSegments = placeholder ? value.split(placeholder) : [value]; - if (unparameterizedSegments.every((segment) => !segment.includes(literal))) return value; + if (unparameterizedSegments.every((segment) => !segment.includes(literal))) { + return collapseIfLiteralRemains(value, literal, placeholder); + } // Replacing whitespace inline would make every ordinary separator look like // authored secret data. Collapse the whole untrusted string/key instead. if (!literal.trim()) return placeholder; - return unparameterizedSegments - .map((segment) => segment.replaceAll(literal, placeholder)) - .join(placeholder); + return collapseIfLiteralRemains( + unparameterizedSegments + .map((segment) => segment.replaceAll(literal, placeholder)) + .join(placeholder), + literal, + placeholder, + ); +} + +/** + * The output never carries the literal outside a placeholder. A literal that contains the + * placeholder (`abc[REDACTED]xyz`, or `${VAR}` inside a `--record-as VAR` value) or crosses one + * spans the segments split on it, so the segment-wise replace above leaves it whole; the whole + * string collapses to the placeholder instead. A literal found only inside placeholders (`$` in + * `${DOLLAR}`) reveals nothing and stays. + */ +function collapseIfLiteralRemains(value: string, literal: string, placeholder: string): string { + const placeholderStarts: number[] = []; + if (placeholder) { + for ( + let at = value.indexOf(placeholder); + at !== -1; + at = value.indexOf(placeholder, at + placeholder.length) + ) { + placeholderStarts.push(at); + } + } + // Placeholders do not overlap, so the first one ending at or after an occurrence of the + // literal is the only one that can cover it, and that index only moves forward. + let next = 0; + for (let at = value.indexOf(literal); at !== -1; at = value.indexOf(literal, at + 1)) { + while ( + next < placeholderStarts.length && + placeholderStarts[next]! + placeholder.length < at + literal.length + ) { + next += 1; + } + if (next === placeholderStarts.length || placeholderStarts[next]! > at) return placeholder; + } + return value; } /** diff --git a/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index bc7575bdb3..a40956f3b6 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -328,6 +328,7 @@ function summarizeProviderScenarioFlagExclusions() { 'shardSplit', 'searchPath', 'stepsFile', + 'textStdin', 'proxyHost', 'proxyPort', 'stale', diff --git a/src/cli/resolve-cli-options.test.ts b/src/cli/resolve-cli-options.test.ts index bd16fe7657..c5ac3d474e 100644 --- a/src/cli/resolve-cli-options.test.ts +++ b/src/cli/resolve-cli-options.test.ts @@ -1,5 +1,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; +import fs from 'node:fs'; import { mkdtempForTestSync } from '../__tests__/test-utils/tmp-dir.ts'; import { resolveCliOptions } from './resolve-cli-options.ts'; @@ -75,3 +76,36 @@ test('the same env default keeps filling doctor, which reads a configured app wi assert.equal(parsed.flags.targetApp, 'com.example.configured'); }); + +// #3260: --text-stdin makes fill block on stdin, so only a typed flag may turn it on. +test('fill --text-stdin has no environment default', () => { + const parsed = resolveCliOptions(['fill', '@e3', 'typed text'], { + cwd: process.cwd(), + env: isolatedEnv({ AGENT_DEVICE_TEXT_STDIN: '1' }), + }); + + assert.equal(parsed.flags.textStdin, undefined); +}); + +test('a config file cannot set fill --text-stdin', () => { + const configPath = `${mkdtempForTestSync('agent-device-cli-config-')}/config.json`; + fs.writeFileSync(configPath, JSON.stringify({ textStdin: true })); + + assert.throws( + () => + resolveCliOptions(['fill', '@e3', 'typed text', '--config', configPath], { + cwd: process.cwd(), + env: isolatedEnv({}), + }), + /Config key "textStdin" is not allowed/, + ); +}); + +test('a typed --text-stdin reaches fill', () => { + const parsed = resolveCliOptions(['fill', '@e3', '--text-stdin'], { + cwd: process.cwd(), + env: isolatedEnv({}), + }); + + assert.equal(parsed.flags.textStdin, true); +}); diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index b412376bac..04f152d5d0 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -436,3 +436,27 @@ test('batch human output renders per-step results', async () => { assert.match(result.stdout, /1\. OK Opened: Settings \(7ms\)/); assert.match(result.stdout, /2\. OK Typed 5 chars \(8ms\)/); }); + +test('batch rejects a fill step that asks for stdin text before daemon dispatch', async () => { + const result = await runCliCapture([ + 'batch', + '--steps', + '[{"command":"fill","input":{"target":{"kind":"ref","ref":"@e3"},"text":"x","textStdin":true}}]', + ]); + + assert.equal(result.code, 1); + assert.equal(result.calls.length, 0); + assert.match(result.stderr, /Batch step 1: fill textStdin is not supported/); +}); + +test('batch passes an explicit fill textStdin false through to dispatch', async () => { + const result = await runCliCapture([ + 'batch', + '--steps', + '[{"command":"fill","input":{"target":{"kind":"ref","ref":"@e3"},"text":"x","textStdin":false}}]', + ]); + + assert.equal(result.calls.length, 1); + assert.equal((result.calls[0]?.flags?.batchSteps ?? [])[0]?.flags?.textStdin, false); + assert.doesNotMatch(result.stderr, /textStdin is not supported/); +}); diff --git a/src/commands/batch/projection.ts b/src/commands/batch/projection.ts index 353dae5493..5343eb52f1 100644 --- a/src/commands/batch/projection.ts +++ b/src/commands/batch/projection.ts @@ -65,6 +65,7 @@ async function readBatchDaemonStep( const record = readBatchStepRecord(step, stepNumber); const command = readBatchStepCommand(record, stepNumber); const input = readBatchStepInputObject(record, stepNumber) as CommandInput; + assertNoStdinFillStep(command, input, stepNumber); const runtime = parseBatchStepRuntime(record.runtime, stepNumber); const prepared = await prepareDaemonCommandRequest(command, input, stepNumber); return { @@ -76,6 +77,19 @@ async function readBatchDaemonStep( }; } +function assertNoStdinFillStep( + command: BatchCommandName, + input: CommandInput, + stepNumber: number, +): void { + if (command !== PUBLIC_COMMANDS.fill || input.textStdin !== true) return; + throw new AppError( + 'INVALID_ARGS', + `Batch step ${stepNumber}: fill textStdin is not supported because batch steps have no stdin of their own. Run that fill on its own with --text-stdin.`, + { reason: 'fill_text_stdin_in_batch' }, + ); +} + function readBatchStepCommand( record: Record, stepNumber: number, diff --git a/src/commands/cli-runner.test.ts b/src/commands/cli-runner.test.ts new file mode 100644 index 0000000000..fd40a39fc3 --- /dev/null +++ b/src/commands/cli-runner.test.ts @@ -0,0 +1,59 @@ +import { Readable } from 'node:stream'; +import { afterEach, expect, test, vi } from 'vitest'; +import { createAgentDeviceClient } from '../agent-device-client.ts'; +import { parseArgs } from '../cli/parser/args.ts'; +import type { DaemonRequest } from '@agent-device/kernel/contracts'; +import { runCliCommandWithOutput } from './cli-runner.ts'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Drives the real argv chain to the transport, with `text` piped to the process's stdin. */ +async function runFillWithStdin(argv: string[], text: string): Promise { + vi.spyOn(process, 'stdin', 'get').mockReturnValue(Readable.from([text]) as typeof process.stdin); + const seen: DaemonRequest[] = []; + const client = createAgentDeviceClient( + {}, + { + transport: async (req) => { + seen.push(req as DaemonRequest); + return { ok: true, data: {} } as never; + }, + }, + ); + const parsed = parseArgs(argv, { strictFlags: true }); + try { + await runCliCommandWithOutput({ + client, + command: 'fill', + positionals: parsed.positionals, + flags: parsed.flags, + }); + } catch { + // The stub's empty payload can fail result normalization after the request was captured. + } + return seen; +} + +test('fill --text-stdin sends the piped text, not argv, and marks it for the daemon', async () => { + const [request] = await runFillWithStdin( + ['fill', 'id="password"', '--text-stdin'], + 'piped-secret\n', + ); + + expect(request?.command).toBe('fill'); + expect(request?.positionals).toEqual(['id="password"', 'piped-secret']); + expect(request?.flags?.textStdin).toBe(true); +}); + +test('fill --text-stdin forwards --record-as with the piped text', async () => { + const [request] = await runFillWithStdin( + ['fill', '@e3', '--text-stdin', '--record-as', 'PASSWORD'], + 'piped-secret', + ); + + expect(request?.positionals).toEqual(['@e3', 'piped-secret']); + expect(request?.flags?.recordAs).toBe('PASSWORD'); + expect(request?.flags?.textStdin).toBe(true); +}); diff --git a/src/commands/cli-runner.ts b/src/commands/cli-runner.ts index 6d2718bd9e..3ad96a2877 100644 --- a/src/commands/cli-runner.ts +++ b/src/commands/cli-runner.ts @@ -1,4 +1,6 @@ +import { registerDiagnosticSensitiveValue } from '@agent-device/host-kit/diagnostics'; import type { AgentDeviceClient, CommandRequestResult } from '../agent-device-client.ts'; +import { readFillTextFromStdin } from './interaction/fill-text-stdin.ts'; import { formatCliOutput } from './cli-output.ts'; import { readInputFromCli } from './cli-grammar.ts'; import { runCommand, type CommandName } from './command-surface.ts'; @@ -23,6 +25,11 @@ export async function runCliCommandWithOutput(options: CliRunOptions): Promise<{ cliOutput?: CliOutput; }> { const input = readInputFromCli(options.command, options.positionals, options.flags); + if (options.flags.textStdin) { + const text = await readFillTextFromStdin(process.stdin); + registerDiagnosticSensitiveValue(text); + input.text = text; + } const result = (await runCommand(options.client, options.command, input)) as CommandRequestResult; return { result, diff --git a/src/commands/interaction/fill-text-stdin.test.ts b/src/commands/interaction/fill-text-stdin.test.ts new file mode 100644 index 0000000000..c2612581e4 --- /dev/null +++ b/src/commands/interaction/fill-text-stdin.test.ts @@ -0,0 +1,88 @@ +import { Readable } from 'node:stream'; +import { expect, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { FILL_TEXT_STDIN_MAX_BYTES, readFillTextFromStdin } from './fill-text-stdin.ts'; + +function pipe(...chunks: Array): Readable { + return Readable.from(chunks); +} + +function terminal(): Readable & { isTTY: boolean } { + return Object.assign(Readable.from([]), { isTTY: true }); +} + +async function readError(stdin: Parameters[0]): Promise { + try { + await readFillTextFromStdin(stdin); + } catch (error) { + if (error instanceof AppError) return error; + throw error; + } + throw new Error('expected readFillTextFromStdin to reject'); +} + +test.each([ + ['no trailing newline', ['hunter2'], 'hunter2'], + ['one trailing LF', ['hunter2\n'], 'hunter2'], + ['one trailing CRLF', ['hunter2\r\n'], 'hunter2'], + ['only the last of two newlines', ['hunter2\n\n'], 'hunter2\n'], + ['surrounding spaces and inner newlines', [' two\nlines \n'], ' two\nlines '], + ['a value split across chunks', ['hun', Buffer.from('ter'), '2\n'], 'hunter2'], + ['a multi-byte character split across chunks', [Buffer.from([0xc3]), Buffer.from([0xa7])], 'ç'], + ['a surrogate pair split across string chunks', ['a\uD83D', '\uDE00b'], 'a\u{1F600}b'], +])('reads %s', async (_name, chunks, expected) => { + expect(await readFillTextFromStdin(pipe(...chunks))).toBe(expected); +}); + +test('accepts input exactly at the byte limit', async () => { + const text = 'a'.repeat(FILL_TEXT_STDIN_MAX_BYTES); + expect(await readFillTextFromStdin(pipe(text))).toBe(text); +}); + +test('keeps a leading byte order mark', async () => { + expect(await readFillTextFromStdin(pipe(Buffer.from([0xef, 0xbb, 0xbf, 0x61])))).toBe('\uFEFFa'); +}); + +test.each([ + ['a terminal', () => terminal(), 'fill_text_stdin_tty', undefined], + ['empty input', () => pipe(), 'fill_text_stdin_empty', undefined], + ['input that is only a newline', () => pipe('\n'), 'fill_text_stdin_empty', undefined], + [ + 'input over the byte limit', + () => pipe('s3cret'.repeat(FILL_TEXT_STDIN_MAX_BYTES)), + 'fill_text_stdin_too_large', + 's3cret', + ], + [ + 'invalid UTF-8 bytes', + () => pipe(Buffer.from('bad-bytes-'), Buffer.from([0xff])), + 'fill_text_stdin_invalid_utf8', + 'bad-bytes-', + ], + [ + 'a string chunk with a lone surrogate', + () => pipe('lone-surrogate-\uD800'), + 'fill_text_stdin_invalid_utf8', + 'lone-surrogate-', + ], + [ + 'a high surrogate followed by a byte chunk', + () => pipe('held-surrogate-\uD83D', Buffer.from('x')), + 'fill_text_stdin_invalid_utf8', + 'held-surrogate-', + ], + [ + 'an oversized string chunk before checking its surrogates', + () => pipe(`${'s3cret'.repeat(FILL_TEXT_STDIN_MAX_BYTES)}\uDC00`), + 'fill_text_stdin_too_large', + 's3cret', + ], +])('refuses %s with a typed reason and no echoed input', async (_name, stdin, reason, input) => { + const error = await readError(stdin()); + + expect(error.code).toBe('INVALID_ARGS'); + expect(error.details?.reason).toBe(reason); + if (input !== undefined) { + expect(JSON.stringify({ message: error.message, details: error.details })).not.toContain(input); + } +}); diff --git a/src/commands/interaction/fill-text-stdin.ts b/src/commands/interaction/fill-text-stdin.ts new file mode 100644 index 0000000000..288ba3e417 --- /dev/null +++ b/src/commands/interaction/fill-text-stdin.ts @@ -0,0 +1,105 @@ +import { AppError } from '@agent-device/kernel/errors'; + +/** Most UTF-8 input `fill --text-stdin` accepts, trailing newline included. */ +export const FILL_TEXT_STDIN_MAX_BYTES = 64 * 1024; + +type StdinSource = AsyncIterable & { isTTY?: boolean }; + +/** + * Reads the `fill --text-stdin` value. Exactly one trailing `\n` or `\r\n` is removed so a value + * piped with or without a final newline sends the same text; all other whitespace, including a + * leading byte order mark, is kept. The value may be a secret: no error raised here includes any + * of the bytes read. + */ +export async function readFillTextFromStdin(stdin: StdinSource): Promise { + if (stdin.isTTY) { + throw new AppError( + 'INVALID_ARGS', + 'fill --text-stdin reads piped input, but stdin is a terminal.', + { + reason: 'fill_text_stdin_tty', + hint: 'Pipe the text in, for example: printf %s "$PASSWORD" | agent-device fill @e3 --text-stdin', + }, + ); + } + const chunks: Uint8Array[] = []; + let byteLength = 0; + let heldHighSurrogate = ''; + for await (const chunk of stdin) { + const next = joinHeldHighSurrogate(heldHighSurrogate, chunk); + heldHighSurrogate = next.held; + byteLength += utf8ByteLength(next.piece); + if (byteLength > FILL_TEXT_STDIN_MAX_BYTES) { + throw new AppError( + 'INVALID_ARGS', + `fill --text-stdin accepts at most ${FILL_TEXT_STDIN_MAX_BYTES} bytes of input.`, + { reason: 'fill_text_stdin_too_large', maxBytes: FILL_TEXT_STDIN_MAX_BYTES }, + ); + } + chunks.push(encodeWellFormed(next.piece)); + } + if (heldHighSurrogate) throw invalidUtf8Error(); + const text = stripOneTrailingNewline(decodeUtf8(Buffer.concat(chunks))); + if (text === '') { + throw new AppError('INVALID_ARGS', 'fill --text-stdin received no text.', { + reason: 'fill_text_stdin_empty', + hint: 'Check that the piped value is set. To clear a field, pass an empty text argument instead: fill @e3 ""', + }); + } + return text; +} + +const LONE_SURROGATE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? | fill <@ref|selector> ', + usageOverride: + 'fill | fill <@ref|selector> | fill --text-stdin', usageFlags: [], positionalArgs: ['targetOrX', 'yOrText', 'text?'], allowsExtraPositionals: true, @@ -433,6 +434,7 @@ const interactionCliSchemas = { ...SELECTOR_SNAPSHOT_FLAGS, 'delayMs', 'recordAs', + 'textStdin', ...postActionObservationCliFlags('fill'), ], }, @@ -479,7 +481,7 @@ const fillCommandFacet = defineCommandFacet({ text: { summary: 'Replace text in a UI input', cliDetail: - 'Every positional after an @ref is the replacement text, so fill @e57 good morning enters "good morning"; quote the text when the shell must preserve exact whitespace. Clear a field with an empty text argument: fill @e57 "" (the argument must be present — fill @e57 alone is a missing argument, not a clear). When visible label text also matches a non-input element, constrain the target with editable=true, for example fill \'label="Email" editable=true\' "qa@example.com".', + 'Every positional after an @ref is the replacement text, so fill @e57 good morning enters "good morning"; quote the text when the shell must preserve exact whitespace. Clear a field with an empty text argument: fill @e57 "" (the argument must be present — fill @e57 alone is a missing argument, not a clear). When visible label text also matches a non-input element, constrain the target with editable=true, for example fill \'label="Email" editable=true\' "qa@example.com". To keep a secret out of argv, omit the text and pipe it in: printf %s "$PASSWORD" | fill @e57 --text-stdin (max 64 KiB, one trailing newline removed).', }, metadata: metadata('fill'), run: (client, input) => client.interactions.fill(toFillOptions(input)), @@ -689,6 +691,7 @@ function toFillOptions(input: FillInput): FillOptions { text: input.text, delayMs: input.delayMs, recordAs: input.recordAs, + textStdin: input.textStdin, verify: input.verify, ...toSettleOptions(input), }; diff --git a/src/commands/interaction/interactions.test.ts b/src/commands/interaction/interactions.test.ts index d8a3a49afc..0f64f3dd85 100644 --- a/src/commands/interaction/interactions.test.ts +++ b/src/commands/interaction/interactions.test.ts @@ -73,6 +73,54 @@ test('fill projects an empty text as its own positional', () => { expect(request.positionals).toEqual(['@e57', '']); }); +test('fill --text-stdin reads only the target and leaves the text to stdin', () => { + const flags = { ...BASE_FLAGS, textStdin: true }; + + for (const positionals of [['@e57'], ['id="password"'], ['10', '20']]) { + const input = interactionCliReaders.fill(positionals, flags); + expect(input.text, positionals.join(' ')).toBeUndefined(); + expect(input.textStdin, positionals.join(' ')).toBe(true); + } +}); + +test('fill --text-stdin refuses an unreadable target without echoing it', () => { + for (const positionals of [['stray-secret'], ['e3stray'], ['stray secret words']]) { + let caught: unknown; + try { + interactionCliReaders.fill(positionals, { ...BASE_FLAGS, textStdin: true }); + } catch (error) { + caught = error; + } + expect(caught, positionals.join(' ')).toBeInstanceOf(AppError); + const error = caught as AppError; + expect(error.details?.reason).toBe('fill_text_stdin_target_invalid'); + expect(JSON.stringify({ message: error.message, details: error.details })).not.toMatch(/stray/); + } +}); + +test('fill --text-stdin refuses a text argument without echoing it', () => { + for (const positionals of [ + ['@e57', 'argv-secret'], + ['@e57', ''], + ['id="password"', 'argv-secret'], + ['10', '20', 'argv-secret'], + ]) { + let caught: unknown; + try { + interactionCliReaders.fill(positionals, { ...BASE_FLAGS, textStdin: true }); + } catch (error) { + caught = error; + } + expect(caught, positionals.join(' ')).toBeInstanceOf(AppError); + const error = caught as AppError; + expect(error.code).toBe('INVALID_ARGS'); + expect(error.details?.reason).toBe('fill_text_source_conflict'); + expect(JSON.stringify({ message: error.message, details: error.details })).not.toMatch( + /argv-secret/, + ); + } +}); + // `find fill ""` is the same clear request through the find grammar (#2063): the empty // value reaches the typed options, while a missing value is refused at the reader so the typed // `value: string` contract stays intact. diff --git a/src/commands/interaction/interactions.ts b/src/commands/interaction/interactions.ts index 8f15ff32f4..79154b99d1 100644 --- a/src/commands/interaction/interactions.ts +++ b/src/commands/interaction/interactions.ts @@ -84,7 +84,9 @@ export const interactionCliReaders = { delayMs: flags.delayMs, }), fill: (positionals, flags) => { - const decoded = readFillTargetFromPositionals(positionals); + const decoded = flags.textStdin + ? readStdinFillTarget(positionals) + : readFillTargetFromPositionals(positionals); return { ...commonInputFromFlags(flags), ...selectorSnapshotInputFromFlags(flags), @@ -92,6 +94,7 @@ export const interactionCliReaders = { text: decoded.text, delayMs: flags.delayMs, recordAs: flags.recordAs, + textStdin: flags.textStdin, verify: flags.verify, }; }, @@ -154,6 +157,36 @@ export const interactionDaemonWriters = { ]), } satisfies Record; +/** + * Reads only the target for `fill --text-stdin`. A stray positional may be the secret itself + * (`fill --text-stdin "$PASSWORD"`), so neither a parse failure nor the conflict echoes any of them. + */ +function readStdinFillTarget( + positionals: string[], +): ReturnType { + let decoded: ReturnType; + try { + decoded = readFillTargetFromPositionals(positionals); + } catch (error) { + if (!(error instanceof AppError)) throw error; + throw new AppError('INVALID_ARGS', 'fill --text-stdin could not read the target.', { + reason: 'fill_text_stdin_target_invalid', + hint: 'Pass only the target: an @ref, a quoted selector, or x y coordinates. The text itself goes to stdin.', + }); + } + if (decoded.text !== undefined) { + throw new AppError( + 'INVALID_ARGS', + 'fill --text-stdin reads the text from stdin, so it cannot also take a text argument.', + { + reason: 'fill_text_source_conflict', + hint: 'Pass only the target, for example: printf %s "$PASSWORD" | agent-device fill @e3 --text-stdin. Quote a multi-word selector so no part of it is read as text.', + }, + ); + } + return decoded; +} + function readLongPressTargetFromPositionals(positionals: string[]): LongPressOptions { const targetPositionals = readLongPressTargetPositionals(positionals); return { diff --git a/src/commands/interaction/metadata.ts b/src/commands/interaction/metadata.ts index 31e785902c..0279783498 100644 --- a/src/commands/interaction/metadata.ts +++ b/src/commands/interaction/metadata.ts @@ -132,6 +132,15 @@ const fillFields = { recordAs: stringField( 'When script recording is armed, send text to the live app but publish it as ${VAR}. Use an uppercase replay variable name such as PASSWORD.', ), + textStdin: operatorField( + booleanField( + "Operator-only: text was read from the CLI's stdin, so it is kept out of diagnostics and recorded actions.", + ), + { + operatorPath: + 'Pass --text-stdin on the CLI and pipe the text in; it is not exposed to model-facing tools.', + }, + ), ...selectorSnapshotFields(), ...postActionObservationFields('fill'), }; diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 469375ec12..494e96b9a7 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -178,6 +178,9 @@ Reusable open-to-destination scripts: export AD_VAR_PASSWORD='' agent-device fill 'id="password"' "$AD_VAR_PASSWORD" --record-as PASSWORD The live app receives the value; recording state and the published script contain only \${PASSWORD}. Reuse the same name for repeated values; --record-as is fill-only, requires an armed recording, and is mutually exclusive with --no-record. Replay with AD_VAR_PASSWORD still set, or pass --env PASSWORD=. Do not record passwords/tokens without --record-as; their literal text is written to the .ad target. + To keep the value out of the CLI's argv as well, pipe it in with --text-stdin instead of a text argument: + printf %s "$AD_VAR_PASSWORD" | agent-device fill 'id="password"' --text-stdin --record-as PASSWORD + --text-stdin reads at most 64 KiB, removes exactly one trailing newline, and refuses a terminal, empty input, invalid UTF-8, or an extra text argument. While recording is armed, --record-as publishes \${VAR} and --no-record leaves the step out; passing neither is an error. Without an armed recording the step is not recorded. It is not available inside batch steps. Replay divergence and repair: A failing replay/test step returns REPLAY_DIVERGENCE with a bounded report (screen digest, ranked selector suggestions, resume). Fix app state, then resume with replay --from --plan-digest (both from the report's resume field) to continue without re-running earlier steps; resume never re-executes skipped steps, so app state there is the caller's responsibility. --from is replay-only; test rejects it. The digest binds the script, includes, effective --platform/--target, and per-action runtime/identity; native .ad interpolation is late-bound so changing only its values keeps the digest, while Maestro environment substitution can change it. diff --git a/src/commands/schema/option-schema.ts b/src/commands/schema/option-schema.ts index ba018c3d0e..d53b41377b 100644 --- a/src/commands/schema/option-schema.ts +++ b/src/commands/schema/option-schema.ts @@ -25,9 +25,15 @@ const CONFIG_EXCLUDED_FLAG_KEYS = new Set([ 'version', 'batchSteps', 'githubActionsArtifact', + // A default must never switch a fill to waiting on stdin. + 'textStdin', ]); -const ENV_EXCLUDED_FLAG_KEYS = new Set(['appsFilter', 'iosSimulatorDeviceSet']); +const ENV_EXCLUDED_FLAG_KEYS = new Set([ + 'appsFilter', + 'iosSimulatorDeviceSet', + 'textStdin', +]); const optionSpecs = buildOptionSpecs(); const optionSpecByKey = new Map(optionSpecs.map((spec) => [spec.key, spec])); diff --git a/src/daemon/__tests__/request-router-error-redaction.test.ts b/src/daemon/__tests__/request-router-error-redaction.test.ts new file mode 100644 index 0000000000..a4238d4cfe --- /dev/null +++ b/src/daemon/__tests__/request-router-error-redaction.test.ts @@ -0,0 +1,88 @@ +/** + * The router redacts an error only for a fill the caller marked sensitive. Another command can + * register sensitive values too (a Maestro `inputText` does), and its error, including a + * `logPath` that happens to contain such a value, comes back unchanged. + */ +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test, vi } from 'vitest'; + +vi.mock('@agent-device/platform-apple/runner/operations', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, stopIosRunnerSession: vi.fn(async () => {}) }; +}); + +vi.mock('../device/device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +import { AppError } from '@agent-device/kernel/errors'; +import type { DeviceRuntimeGateway, RuntimeFacts } from '@agent-device/contracts/platform-runtime'; +import type { PlatformRuntimeOperations } from '@agent-device/contracts/platform-runtime-operations'; +import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts'; +import { makeIosAppSession } from '../../__tests__/test-utils/session-factories.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { + createRequestHandler, + lifecycleDeviceRuntimeGateway, +} from './test-device-runtime-gateway.ts'; + +const SESSION = 'maestro-input'; + +const typeText = vi.fn(async (_input: { text: string }) => { + throw new AppError('COMMAND_FAILED', 'keyboard is not ready'); +}); + +function withTypeText( + facts: RuntimeFacts, +): RuntimeFacts { + return { ...facts, operations: { ...facts.operations, typeText: { available: true } } }; +} + +const typingDeviceRuntimeGateway: DeviceRuntimeGateway = { + inspectFacts: async (device) => + withTypeText(await lifecycleDeviceRuntimeGateway.inspectFacts(device)), + bind: async (request) => { + const binding = await lifecycleDeviceRuntimeGateway.bind(request); + return { + ...binding, + facts: withTypeText(binding.facts), + operations: { ...binding.operations, typeText }, + }; + }, + shutdown: async () => {}, +}; + +test('a Maestro replay error comes back unchanged after its inputText registered a short value', async () => { + const sessionStore = makeSessionStore('agent-device-router-error-redaction-'); + sessionStore.publish(SESSION, makeIosAppSession(SESSION)); + const root = mkdtempForTestSync('agent-device-router-error-redaction-'); + const flowPath = path.join(root, 'flow.yaml'); + fs.writeFileSync(flowPath, 'appId: com.example.app\n---\n- inputText: "1"\n'); + const handler = createRequestHandler({ + logPath: path.join(root, 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryGateways: createTestDeviceInventoryGateways(), + trackDownloadableArtifact: () => 'artifact-id', + deviceRuntimeGateway: typingDeviceRuntimeGateway, + }); + + const result = await handler({ + token: 'test-token', + session: SESSION, + command: 'replay', + positionals: [flowPath], + flags: { replayBackend: 'maestro', platform: 'ios' }, + meta: { requestId: 'req-maestro-input-1' }, + }); + + expect(typeText).toHaveBeenCalledWith(expect.objectContaining({ text: '1' })); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(JSON.stringify(result.error)).not.toContain('[REDACTED]'); + expect(result.error.logPath).toContain('req-maestro-input-1'); + expect(fs.existsSync(result.error.logPath!)).toBe(true); +}); diff --git a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts new file mode 100644 index 0000000000..ae83c55064 --- /dev/null +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -0,0 +1,303 @@ +/** + * #3260: `fill --text-stdin` keeps the value out of argv, so it must not come back on the + * response either, and neither may a replayed `--record-as` value. Sent through the real request + * handler to a web session, the narrowest runtime that still runs the full fill admission, + * dispatch and response projection. + */ +import { expect, test } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts'; +import type { WebProvider } from '@agent-device/platform-web'; +import type { DaemonRequest } from '../daemon-request.ts'; +import type { SessionState } from '../session-state.ts'; +import { createRequestHandler } from './test-device-runtime-gateway.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { makeAuthoringSession, makeSession } from '../../__tests__/test-utils/session-factories.ts'; +import { WEB_DESKTOP_DEVICE } from '../../__tests__/test-utils/device-fixtures.ts'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { replayScriptSourceBundleFor } from '../../__tests__/test-utils/replay-script-source.ts'; +import { + createPlatformRuntimeGateway, + createRequestPlatformProviders, +} from '../../platform-runtime.ts'; + +const SECRET = 'stdin-s3cret-value'; + +function fillWithStdinText( + session: SessionState, + flags: Record = {}, + fillFails?: (text: string) => Error, + text = SECRET, +) { + return sendToWebSession( + session, + { command: 'fill', positionals: ['10', '20', text], flags: { textStdin: true, ...flags } }, + fillFails, + ); +} + +function sendToWebSession( + session: SessionState, + request: Pick, + fillFails?: (text: string) => Error, +) { + const sessionStore = makeSessionStore('agent-device-router-fill-text-stdin-'); + sessionStore.publish(session.name, session); + const typed: string[] = []; + const webProvider: WebProvider = { + open: async () => {}, + close: async () => {}, + snapshot: async () => ({ nodes: [] }), + screenshot: async () => {}, + setViewport: async () => {}, + click: async () => {}, + fill: async (_x, _y, text) => { + typed.push(text); + if (fillFails) throw fillFails(text); + }, + typeText: async () => {}, + scroll: async () => {}, + }; + const dir = mkdtempForTestSync('agent-device-router-fill-text-stdin-runtime-'); + const handler = createRequestHandler({ + logPath: path.join(dir, 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + deviceInventoryGateways: createTestDeviceInventoryGateways(), + requestPlatformProviders: createRequestPlatformProviders({ + providers: { webProvider: () => webProvider }, + }), + deviceRuntimeGateway: createPlatformRuntimeGateway({ + sessionsDir: dir, + resolveSessionArtifacts: (sessionId) => ({ + outputPath: path.join(dir, sessionId, 'app.log'), + pidPath: path.join(dir, sessionId, 'app-log.pid'), + }), + }), + trackDownloadableArtifact: () => 'artifact-id', + }); + const response = handler({ + token: 'test-token', + session: session.name, + ...request, + meta: { requestId: 'req-fill-text-stdin' }, + }); + return { response, typed, sessionStore }; +} + +test('an unrecorded --text-stdin fill types the value but returns no part of it', async () => { + const { response, typed } = fillWithStdinText(makeSession('web', { device: WEB_DESKTOP_DEVICE })); + + const result = await response; + expect(result.ok).toBe(true); + expect(typed).toEqual([SECRET]); + expect(JSON.stringify(result)).not.toContain(SECRET); + if (result.ok) expect(result.data?.text).toBe('[REDACTED]'); +}); + +test('an armed --text-stdin --no-record fill returns no part of the value and records nothing', async () => { + const session = makeAuthoringSession('web', { device: WEB_DESKTOP_DEVICE }); + const { response, typed, sessionStore } = fillWithStdinText(session, { noRecord: true }); + + const result = await response; + expect(result.ok).toBe(true); + expect(typed).toEqual([SECRET]); + expect(JSON.stringify(result)).not.toContain(SECRET); + if (result.ok) expect(result.data?.text).toBe('[REDACTED]'); + expect(sessionStore.get('web')?.actions).toEqual([]); +}); + +test('an armed --text-stdin fill without --record-as or --no-record is refused before typing', async () => { + const session = makeAuthoringSession('web', { device: WEB_DESKTOP_DEVICE }); + const { response, typed } = fillWithStdinText(session); + + const result = await response; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.code).toBe('INVALID_ARGS'); + expect(result.error.details?.reason).toBe('fill_text_stdin_unparameterized_recording'); + expect(typed).toEqual([]); + expect(JSON.stringify(result)).not.toContain(SECRET); +}); + +test.each([ + ['recordAs', { textStdin: undefined, recordAs: 42 }, '--record-as'], + ['textStdin', { textStdin: 'true' }, '--text-stdin'], +])( + 'a fill whose %s marker has the wrong type is refused before typing', + async (_name, flags, flagName) => { + const { response, typed } = fillWithStdinText( + makeSession('web', { device: WEB_DESKTOP_DEVICE }), + flags, + ); + + const result = await response; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.code).toBe('INVALID_ARGS'); + expect(result.error.message).toContain(flagName); + expect(typed).toEqual([]); + expect(JSON.stringify(result)).not.toContain(SECRET); + }, +); + +test('a backend error that echoes the --text-stdin value does not return it', async () => { + const { response, typed } = fillWithStdinText( + makeSession('web', { device: WEB_DESKTOP_DEVICE }), + {}, + (text) => new Error(`could not type "${text}" into the field`), + ); + + const result = await response; + expect(result.ok).toBe(false); + expect(typed).toEqual([SECRET]); + expect(JSON.stringify(result)).not.toContain(SECRET); + if (!result.ok) expect(result.error.message).toBe('could not type "[REDACTED]" into the field'); +}); + +test('a backend error that echoes a --record-as stdin value shows its placeholder', async () => { + const { response, typed } = fillWithStdinText( + makeAuthoringSession('web', { device: WEB_DESKTOP_DEVICE }), + { recordAs: 'PASSWORD' }, + (text) => new Error(`could not type "${text}" into the field`), + ); + + const result = await response; + expect(result.ok).toBe(false); + expect(typed).toEqual([SECRET]); + if (!result.ok) expect(result.error.message).toBe('could not type "${PASSWORD}" into the field'); +}); + +test.each([ + ['an unrecorded', 'abc[REDACTED]xyz', {}, '[REDACTED]', makeSession], + [ + 'a --record-as', + 'abc${PASSWORD}xyz', + { recordAs: 'PASSWORD' }, + '${PASSWORD}', + makeAuthoringSession, + ], +])( + '%s --text-stdin value that contains its own placeholder is returned by neither a success nor an error', + async (_name, secret, flags, placeholder, sessionFor) => { + const filled = fillWithStdinText( + sessionFor('web', { device: WEB_DESKTOP_DEVICE }), + flags, + undefined, + secret, + ); + const failed = fillWithStdinText( + sessionFor('web', { device: WEB_DESKTOP_DEVICE }), + flags, + (text) => new Error(`could not type "${text}" into the field`), + secret, + ); + + const succeeded = await filled.response; + const errored = await failed.response; + expect(filled.typed).toEqual([secret]); + expect(failed.typed).toEqual([secret]); + expect(succeeded.ok).toBe(true); + expect(JSON.stringify(succeeded)).not.toContain(secret); + expect(JSON.stringify(filled.sessionStore.get('web')?.actions ?? [])).not.toContain(secret); + expect(errored.ok).toBe(false); + expect(JSON.stringify(errored)).not.toContain(secret); + if (!errored.ok) expect(errored.error.message).toBe(placeholder); + }, +); + +test('a short --text-stdin value is redacted from the error text but not from its log path', async () => { + // `stdin` also appears in this request's id, which names its diagnostics file. + const { response } = fillWithStdinText( + makeSession('web', { device: WEB_DESKTOP_DEVICE }), + {}, + (text) => new Error(`could not type "${text}" into the field`), + 'stdin', + ); + + const result = await response; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.message).toBe('could not type "[REDACTED]" into the field'); + expect(result.error.logPath).toContain('req-fill-text-stdin'); + expect(fs.existsSync(result.error.logPath!)).toBe(true); + expect(result.error.diagnosticsRecord?.requestId).toBe('req-fill-text-stdin'); +}); + +test('a batch step carrying --text-stdin is refused before typing and returns no part of the value', async () => { + const { response, typed } = sendToWebSession(makeSession('web', { device: WEB_DESKTOP_DEVICE }), { + command: 'batch', + positionals: [], + flags: { + batchSteps: [ + { command: 'fill', positionals: ['10', '20', SECRET], flags: { textStdin: true } }, + ], + }, + }); + + const result = await response; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.code).toBe('INVALID_ARGS'); + expect(result.error.details?.reason).toBe('fill_text_stdin_in_batch'); + expect(typed).toEqual([]); + expect(JSON.stringify(result)).not.toContain(SECRET); +}); + +test('a replayed --record-as fill whose backend error echoes the value does not return it', async () => { + const flowPath = path.join( + mkdtempForTestSync('agent-device-router-replay-record-as-'), + 'flow.ad', + ); + fs.writeFileSync(flowPath, 'fill 10 20 "${PASSWORD}"\n'); + const { response, typed } = sendToWebSession( + makeSession('web', { device: WEB_DESKTOP_DEVICE }), + { + command: 'replay', + positionals: [flowPath], + flags: { + replayEnv: [`PASSWORD=${SECRET}`], + replayScriptSource: replayScriptSourceBundleFor(flowPath), + }, + }, + (text) => new Error(`could not type "${text}" into the field`), + ); + + const result = await response; + expect(result.ok).toBe(false); + expect(typed).toEqual([SECRET]); + expect(JSON.stringify(result)).not.toContain(SECRET); + if (!result.ok) expect(result.error.message).toContain('could not type "${PASSWORD}"'); +}); + +test('a batch fill step with textStdin false is dispatched like any fill', async () => { + const { response, typed } = sendToWebSession(makeSession('web', { device: WEB_DESKTOP_DEVICE }), { + command: 'batch', + positionals: [], + flags: { + batchSteps: [ + { command: 'fill', positionals: ['10', '20', 'plain'], flags: { textStdin: false } }, + ], + }, + }); + + const result = await response; + expect(result.ok).toBe(true); + expect(typed).toEqual(['plain']); +}); + +test('a whitespace-only --text-stdin value collapses the error text it appears in', async () => { + const { response } = fillWithStdinText( + makeSession('web', { device: WEB_DESKTOP_DEVICE }), + {}, + (text) => new Error(`could not type "${text}" into the field`), + ' ', + ); + + const result = await response; + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error.message).toBe('[REDACTED]'); +}); diff --git a/src/daemon/__tests__/request-router-record-flags.test.ts b/src/daemon/__tests__/request-router-record-flags.test.ts index d21de850a0..4c9c46ccae 100644 --- a/src/daemon/__tests__/request-router-record-flags.test.ts +++ b/src/daemon/__tests__/request-router-record-flags.test.ts @@ -56,3 +56,18 @@ test('--record alone is accepted (rejected only by the pairing, not the flag its if (response.ok) return; expect(response.error.message).not.toMatch(/mutually exclusive/); }); + +test('--text-stdin on a command other than fill is rejected before any command runs', async () => { + const handler = createHandler(); + const response = await handler({ + token: 'test-token', + session: 'default', + command: 'type', + positionals: ['hello'], + flags: { textStdin: true }, + }); + expect(response.ok).toBe(false); + if (response.ok) return; + expect(response.error.code).toBe('INVALID_ARGS'); + expect(response.error.message).toBe('--text-stdin is supported only by fill.'); +}); diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 2c703d20be..e90d60e9f8 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -774,3 +774,33 @@ test('#1398: ordinary recordings with no --record-as fill are completely unaffec verification: 'verified', }); }); + +test('a stdin fill without --record-as never enters recording state', () => { + const session = makeIosSession('default'); + const secret = 'stdin-secret-3260'; + const action = recordActionEntry(session, { + command: 'fill', + positionals: ['id="password"', secret], + flags: { textStdin: true }, + result: { text: secret }, + }); + + expect(action).toBeUndefined(); + expect(session.actions).toHaveLength(0); + expect(session.recordedFillLiterals).toBeUndefined(); +}); + +test('a stdin fill with --record-as is recorded as its ${VAR} placeholder', () => { + const session = makeIosSession('default'); + const secret = 'stdin-secret-3260'; + recordActionEntry(session, { + command: 'fill', + positionals: ['id="password"', secret], + flags: { textStdin: true, recordAs: 'PASSWORD' }, + result: { text: secret }, + }); + + expect(session.actions).toHaveLength(1); + expect(session.actions[0]?.positionals).toEqual(['id="password"', '${PASSWORD}']); + expect(JSON.stringify(session.actions)).not.toContain(secret); +}); diff --git a/src/daemon/interaction/internal/__tests__/interaction-recorded-input.test.ts b/src/daemon/interaction/internal/__tests__/interaction-recorded-input.test.ts new file mode 100644 index 0000000000..36978b9a1f --- /dev/null +++ b/src/daemon/interaction/internal/__tests__/interaction-recorded-input.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from 'vitest'; +import type { CommandFlags } from '@agent-device/contracts/command'; +import { AppError } from '@agent-device/kernel/errors'; +import { assertRecordedFillParameterization } from '../interaction-recorded-input.ts'; + +function check(flags: CommandFlags, isSessionRecording: boolean): void { + assertRecordedFillParameterization({ flags, replayPlanStep: false, isSessionRecording }); +} + +test('a stdin fill is refused while recording is armed unless the caller says how to script it', () => { + let caught: unknown; + try { + check({ textStdin: true }, true); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(AppError); + expect((caught as AppError).code).toBe('INVALID_ARGS'); + expect((caught as AppError).details?.reason).toBe('fill_text_stdin_unparameterized_recording'); +}); + +test.each([ + ['--record-as while recording', { textStdin: true, recordAs: 'PASSWORD' }, true], + ['--no-record while recording', { textStdin: true, noRecord: true }, true], + ['no armed recording', { textStdin: true }, false], +])('a stdin fill is admitted with %s', (_name, flags, isSessionRecording) => { + expect(() => check(flags, isSessionRecording)).not.toThrow(); +}); + +test('--record-as keeps requiring an armed recording when the text came from stdin', () => { + expect(() => check({ textStdin: true, recordAs: 'PASSWORD' }, false)).toThrow( + /requires an armed script recording/, + ); +}); diff --git a/src/daemon/interaction/internal/interaction-common.ts b/src/daemon/interaction/internal/interaction-common.ts index 73f0694c72..520b4e428a 100644 --- a/src/daemon/interaction/internal/interaction-common.ts +++ b/src/daemon/interaction/internal/interaction-common.ts @@ -7,7 +7,11 @@ import { type RecordedTargetCapture, } from '@agent-device/selectors/target-evidence'; import type { MultiTargetAnnotationV1 } from '@agent-device/contracts/replay'; -import { inferFillText, recordedInputPlaceholder } from '@agent-device/ad-script'; +import { + inferFillText, + isSensitiveFillText, + sensitiveFillPlaceholder, +} from '@agent-device/ad-script'; import { parameterizeRecordedFillPayload } from '@agent-device/selectors/parameterized-recorded-fill'; import type { InteractionFinalizationOperations } from './types.ts'; @@ -97,7 +101,7 @@ function parameterizeFillPayloads(params: { result: Record; responseData: Record; }): readonly [result: Record, responseData: Record] { - if (params.command !== 'fill' || typeof params.flags?.recordAs !== 'string') { + if (params.command !== 'fill' || !isSensitiveFillText(params.flags)) { return [params.result, params.responseData]; } const literal = inferFillText({ @@ -107,7 +111,7 @@ function parameterizeFillPayloads(params: { flags: params.flags, result: params.result, }); - const placeholder = recordedInputPlaceholder(params.flags.recordAs); + const placeholder = sensitiveFillPlaceholder(params.flags); return [ parameterizeRecordedFillPayload(params.result, literal, placeholder), parameterizeRecordedFillPayload(params.responseData, literal, placeholder), diff --git a/src/daemon/interaction/internal/interaction-recorded-input.ts b/src/daemon/interaction/internal/interaction-recorded-input.ts index 2e7f868fa6..a5caf3e1b1 100644 --- a/src/daemon/interaction/internal/interaction-recorded-input.ts +++ b/src/daemon/interaction/internal/interaction-recorded-input.ts @@ -8,7 +8,10 @@ export function assertRecordedFillParameterization(params: { isSessionRecording: boolean; }): void { const recordAs = params.flags?.recordAs; - if (recordAs === undefined) return; + if (recordAs === undefined) { + assertStdinFillNotRecordedLiterally(params); + return; + } validateRecordedInputVariableName(recordAs); if (params.flags?.noRecord) { throw new AppError( @@ -23,3 +26,21 @@ export function assertRecordedFillParameterization(params: { ); } } + +/** + * Stdin text reaches the daemon as a literal, so an armed recording would publish it verbatim. + * The caller decides how the step is scripted: `--record-as` publishes `${VAR}`, `--no-record` + * leaves it out. + */ +function assertStdinFillNotRecordedLiterally(params: { + flags: CommandFlags | undefined; + isSessionRecording: boolean; +}): void { + if (params.flags?.textStdin !== true || params.flags.noRecord) return; + if (!params.isSessionRecording) return; + throw new AppError( + 'INVALID_ARGS', + 'fill --text-stdin needs --record-as or --no-record while script recording is armed, so the stdin text is not published in the script.', + { reason: 'fill_text_stdin_unparameterized_recording' }, + ); +} diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index fece5d1bb7..9e04ca2f25 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -55,8 +55,13 @@ import { import { unsupportedSaveScriptFlagResponse } from './request-save-script-policy.ts'; import { canRunReplayScopedAction } from './daemon-command-registry.ts'; import { isWebSession } from './web-session-names.ts'; -import { inferFillText } from '@agent-device/ad-script'; +import { + inferFillText, + isSensitiveFillText, + sensitiveFillPlaceholder, +} from '@agent-device/ad-script'; import { createPlatformRequestScope } from './platform-request-scope.ts'; +import { parameterizeSensitiveString } from '@agent-device/selectors/parameterized-recorded-fill'; import { createOwnerScopedDeviceClaimReconciler } from './device/device-claim-owner-recovery.ts'; import { isConfinedToAppLease, scopeRequestSession } from './request-admission.ts'; import { redactMacOsAppLeaseResponse } from './macos-app-lease.ts'; @@ -207,7 +212,10 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { repairExpiredIfTombstoned(req, response.error, sessionStore), sessionStore, ); - return { ok: false, error: enrichDaemonError(error) }; + return { + ok: false, + error: redactSensitiveFillTextInError(req, enrichDaemonError(error)), + }; } // Phase 4 (agent-cost) grafts on the success path. Runs inside the // diagnostics scope so cost can read this request's runner-round-trip tally. @@ -419,7 +427,11 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { } catch (error) { response = finalizeThrownRequestError(error); } - return await finalizeRequestBindingCleanup(childScope, response); + // This entry point registers a sensitive fill too, so it redacts that fill's error as well. + return redactSensitiveFillResponse( + req, + await finalizeRequestBindingCleanup(childScope, response), + ); }; } @@ -465,12 +477,30 @@ function mutuallyExclusiveRecordFlagsResponse(): DaemonResponse { ); } +const FILL_ONLY_FLAGS = [ + ['recordAs', '--record-as', 'string'], + ['textStdin', '--text-stdin', 'boolean'], +] as const; + function recordingFlagsResponse(req: DaemonRequest): DaemonResponse | undefined { if (req.flags?.record && req.flags?.noRecord) return mutuallyExclusiveRecordFlagsResponse(); - if (req.flags?.recordAs !== undefined && req.command !== 'fill') { - return errorResponse('INVALID_ARGS', '--record-as is supported only by fill.'); - } - return undefined; + if (req.command === 'fill') return malformedFillOnlyFlagResponse(req); + const fillOnly = FILL_ONLY_FLAGS.find(([key]) => req.flags?.[key] !== undefined); + if (!fillOnly) return undefined; + return errorResponse('INVALID_ARGS', `${fillOnly[1]} is supported only by fill.`); +} + +/** + * Both fill-only flags mark the text as sensitive, and the request boundary only checks that + * `flags` is an object. A marker of the wrong type is refused here instead of read as unmarked. + */ +function malformedFillOnlyFlagResponse(req: DaemonRequest): DaemonResponse | undefined { + const malformed = FILL_ONLY_FLAGS.find(([key, , type]) => { + const value = req.flags?.[key]; + return value !== undefined && typeof value !== type; + }); + if (!malformed) return undefined; + return errorResponse('INVALID_ARGS', `${malformed[1]} must be a ${malformed[2]}.`); } /** @@ -492,18 +522,58 @@ function customActionFlagsResponse(req: DaemonRequest): DaemonResponse | undefin return undefined; } +/** The text of a fill the caller marked sensitive, or `undefined` for any other request. */ +function sensitiveFillText(req: DaemonRequest): string | undefined { + if (req.command !== 'fill' || !isSensitiveFillText(req.flags)) return undefined; + return inferFillText({ + ts: 0, + command: 'fill', + positionals: req.positionals ?? [], + flags: req.flags, + }); +} + function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { - if (req.command !== 'fill' || typeof req.flags?.recordAs !== 'string') return; - registerDiagnosticSensitiveValue( - inferFillText({ - ts: 0, - command: 'fill', - positionals: req.positionals ?? [], - flags: req.flags, - }), + const text = sensitiveFillText(req); + if (text !== undefined) registerDiagnosticSensitiveValue(text); +} + +/** Error fields that carry text a backend may have built from the value it was typing. */ +const ERROR_TEXT_FIELDS: ReadonlySet = new Set(['message', 'hint', 'cause', 'details']); + +/** + * A backend error can quote the text a sensitive fill failed to type. Only that fill's own text is + * redacted, and only in the error's text fields: `logPath`, `diagnosticId` and the other + * daemon-owned fields come back as they are, and so does every other request's error. + */ +function redactSensitiveFillTextInError(req: DaemonRequest, error: DaemonError): DaemonError { + const text = sensitiveFillText(req); + if (!text) return error; + const placeholder = sensitiveFillPlaceholder(req.flags); + const replace = (value: string) => parameterizeSensitiveString(value, text, placeholder); + return Object.fromEntries( + Object.entries(error).map(([field, value]) => [ + field, + ERROR_TEXT_FIELDS.has(field) ? mapErrorStrings(value, replace) : value, + ]), + ) as DaemonError; +} + +function mapErrorStrings(value: unknown, replace: (value: string) => string): unknown { + if (typeof value === 'string') return replace(value); + if (Array.isArray(value)) return value.map((entry) => mapErrorStrings(entry, replace)); + if (!value || typeof value !== 'object') return value; + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [replace(key), mapErrorStrings(entry, replace)]), ); } +function redactSensitiveFillResponse(req: DaemonRequest, response: DaemonResponse): DaemonResponse { + return response.ok + ? response + : { ...response, error: redactSensitiveFillTextInError(req, response.error) }; +} + async function dispatchGenericForLockedScope(params: { lockedScope: LockedRequestScope; logPath: string; diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 547dc8ef83..3617e1c251 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -9,6 +9,7 @@ import { repairSessionBoundary } from './session-replay-transaction.ts'; import type { MultiTargetAnnotationV1, TargetAnnotationV1 } from '@agent-device/contracts/replay'; import { inferFillText, + isSensitiveFillText, recordedInputPlaceholder, validateRecordedInputVariableName, } from '@agent-device/ad-script'; @@ -59,6 +60,8 @@ export function recordActionEntry( entry: RecordActionEntry, ): SessionAction | undefined { if (entry.flags?.noRecord) return undefined; + // Sensitive fill text stays out of session actions unless `--record-as` parameterizes it. + if (isSensitiveFillText(entry.flags) && entry.flags.recordAs === undefined) return undefined; if (isExcludedRepairSegmentObservation(session, entry)) return undefined; if (entry.flags) applyRecordedSaveScriptFlags(session, entry.flags); const fillLiteral = readRecordedFillLiteral(entry); diff --git a/src/mcp/__tests__/command-tools-operator-inputs.test.ts b/src/mcp/__tests__/command-tools-operator-inputs.test.ts index 58d1eae43f..a87c337a26 100644 --- a/src/mcp/__tests__/command-tools-operator-inputs.test.ts +++ b/src/mcp/__tests__/command-tools-operator-inputs.test.ts @@ -239,3 +239,28 @@ test('MCP still resolves operator env values outside the model-writable surface' assert.equal(calls[0]?.input.daemonAuthToken, 'operator-env-token'); assert.equal(createdConfigs[0]?.stateDir, '/operator/state-dir'); }); + +test('MCP neither advertises nor admits the CLI-only fill textStdin marker', async () => { + for (const tool of listCommandTools()) { + const properties = tool.inputSchema.properties ?? {}; + assert.equal('textStdin' in properties, false, `${tool.name} advertises it`); + } + const calls: unknown[] = []; + const executor = createCommandToolExecutor({ + createClient: () => ({}) as AgentDeviceClient, + runCommand: async (_client, name, input) => { + calls.push({ name, input }); + return {}; + }, + }); + + const result = await executor.execute('fill', { + target: { kind: 'ref', ref: '@e3' }, + text: 'secret', + textStdin: true, + }); + + assert.equal(result.isError, true); + assert.match(result.content[0]?.text ?? '', /textStdin is not accepted as a tool argument/); + assert.deepEqual(calls, [], 'a refused operator input must never reach the command route'); +}); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index d3b0bc9313..65cb4ad818 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -441,6 +441,7 @@ agent-device click @e1 --button secondary # macOS secondary click / context me agent-device focus @e2 agent-device fill @e2 "text" # Clear then type agent-device fill @e2 "search" --delay-ms 80 +printf %s "$PASSWORD" | agent-device fill @e2 --text-stdin # Text from stdin, kept out of argv agent-device type "text" # Type into focused field without clearing agent-device type "query" --delay-ms 80 agent-device press 300 500 @@ -464,6 +465,7 @@ agent-device gesture transform 200 420 80 -40 2 35 700 # combined pan, zoom, and ``` `fill` clears then types. `type` does not clear. +`fill --text-stdin` reads the text from stdin instead of a text argument, so a secret never appears in the CLI's process arguments. Pipe the value with `printf %s`, which sends it verbatim. It reads at most 64 KiB, removes exactly one trailing newline (`\n` or `\r\n`), and refuses a terminal, empty input, invalid UTF-8, or an additional text argument. The value is registered as sensitive for diagnostics, and command output shows it as `${VAR}` with `--record-as` and as `[REDACTED]` otherwise, in success data and in an error's message, hint, cause and details; daemon-owned fields such as `logPath` and `diagnosticId` are left unchanged. While script recording is armed, combine it with `--record-as ` to publish `${VAR}`, or `--no-record` to leave the step out; passing neither is an error. Without an armed recording the step is not recorded. Batch steps do not accept it. A successful `click` or `press` means the action was sent to the target where the command found it, whether that is a tap, a mouse click, or an accessibility action. It does not confirm that the action reached that element or changed the screen. When your next step depends on it, add `--verify` to learn whether the screen changed (`changedFromBefore`), add `--settle` to get the diff once the UI goes quiet, or follow with `wait ` for the screen you expect. When an interaction fails, see [Retry after a failed command](#retry-after-a-failed-command) before you retry. `type` accepts text only. Do not pass `@ref` to `type`; use `fill @ref "text"` to target a field directly, or `press @ref` then `type "text"` to append in the focused field.