From d4ebd4cb9de043a01e888f97a77dd1f53c908fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Fri, 9 Oct 2026 13:56:22 +0300 Subject: [PATCH 01/10] feat(fill): read text from stdin with --text-stdin fill can take its text from stdin instead of argv so a secret never appears in the CLI's process arguments. The read is bounded to 64 KiB and strips exactly one trailing LF or CRLF; a text argument, a terminal, empty input and invalid UTF-8 are refused with typed reasons that never echo the input. The value goes through the existing sensitivity contract: it is registered as a diagnostics secret in the CLI and daemon scopes, and the daemon keeps it out of recorded actions unless --record-as parameterizes it. While recording is armed the flag needs --record-as or --no-record. Batch steps, MCP tools and env/config defaults cannot set it. Closes #3260 --- .../src/flag-definitions-action.ts | 10 +++ packages/contracts/src/cli-flags.ts | 2 + packages/contracts/src/client-gesture.ts | 2 + src/cli/resolve-cli-options.test.ts | 34 ++++++++++ src/commands/batch/cli.test.ts | 12 ++++ src/commands/batch/projection.ts | 14 +++++ src/commands/cli-runner.test.ts | 59 ++++++++++++++++++ src/commands/cli-runner.ts | 7 +++ .../interaction/fill-text-stdin.test.ts | 57 +++++++++++++++++ src/commands/interaction/fill-text-stdin.ts | 62 +++++++++++++++++++ src/commands/interaction/index.ts | 7 ++- src/commands/interaction/interactions.test.ts | 33 ++++++++++ src/commands/interaction/interactions.ts | 12 ++++ src/commands/interaction/metadata.ts | 9 +++ src/commands/schema/cli-help.ts | 3 + src/commands/schema/option-schema.ts | 8 ++- .../request-router-record-flags.test.ts | 15 +++++ .../__tests__/session-action-recorder.test.ts | 30 +++++++++ .../interaction-recorded-input.test.ts | 35 +++++++++++ .../internal/interaction-recorded-input.ts | 23 ++++++- src/daemon/request-router.ts | 16 +++-- src/daemon/session-action-recorder.ts | 2 + .../command-tools-operator-inputs.test.ts | 25 ++++++++ website/docs/docs/commands.md | 2 + 24 files changed, 470 insertions(+), 9 deletions(-) create mode 100644 src/commands/cli-runner.test.ts create mode 100644 src/commands/interaction/fill-text-stdin.test.ts create mode 100644 src/commands/interaction/fill-text-stdin.ts create mode 100644 src/daemon/interaction/internal/__tests__/interaction-recorded-input.test.ts 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/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..88e9e0af9e 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -436,3 +436,15 @@ 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/); +}); diff --git a/src/commands/batch/projection.ts b/src/commands/batch/projection.ts index 353dae5493..2a0559937e 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 === undefined) 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..6ea353a6e5 --- /dev/null +++ b/src/commands/interaction/fill-text-stdin.test.ts @@ -0,0 +1,57 @@ +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])], 'ç'], +])('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.each([ + ['a terminal', () => terminal(), 'fill_text_stdin_tty'], + ['empty input', () => pipe(), 'fill_text_stdin_empty'], + ['input that is only a newline', () => pipe('\n'), 'fill_text_stdin_empty'], + [ + 'input over the byte limit', + () => pipe('s3cret'.repeat(FILL_TEXT_STDIN_MAX_BYTES)), + 'fill_text_stdin_too_large', + ], + ['invalid UTF-8', () => pipe(Buffer.from([0x73, 0x33, 0xff])), 'fill_text_stdin_invalid_utf8'], +])('refuses %s with a typed reason and no echoed input', async (_name, stdin, reason) => { + const error = await readError(stdin()); + + expect(error.code).toBe('INVALID_ARGS'); + expect(error.details?.reason).toBe(reason); + expect(JSON.stringify({ message: error.message, details: error.details })).not.toMatch(/s3cret/); +}); diff --git a/src/commands/interaction/fill-text-stdin.ts b/src/commands/interaction/fill-text-stdin.ts new file mode 100644 index 0000000000..412f930c1a --- /dev/null +++ b/src/commands/interaction/fill-text-stdin.ts @@ -0,0 +1,62 @@ +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 + * `echo "$X"` and `printf '%s' "$X"` send the same text; all other whitespace 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; + for await (const chunk of stdin) { + const bytes = typeof chunk === 'string' ? Buffer.from(chunk, 'utf8') : chunk; + byteLength += bytes.byteLength; + 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(bytes); + } + 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; +} + +function decodeUtf8(bytes: Uint8Array): string { + try { + return new TextDecoder('utf-8', { fatal: true }).decode(bytes); + } catch { + throw new AppError('INVALID_ARGS', 'fill --text-stdin input is not valid UTF-8.', { + reason: 'fill_text_stdin_invalid_utf8', + }); + } +} + +function stripOneTrailingNewline(text: string): string { + if (text.endsWith('\r\n')) return text.slice(0, -2); + if (text.endsWith('\n')) return text.slice(0, -1); + return text; +} diff --git a/src/commands/interaction/index.ts b/src/commands/interaction/index.ts index 0d8d8cb13f..2af0067cd7 100644 --- a/src/commands/interaction/index.ts +++ b/src/commands/interaction/index.ts @@ -425,7 +425,8 @@ const interactionCliSchemas = { allowedFlags: ['delayMs'], }, fill: { - usageOverride: 'fill | fill <@ref|selector> ', + usageOverride: + 'fill | fill <@ref|selector> | fill |<@ref|selector> --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..ebf8468a67 100644 --- a/src/commands/interaction/interactions.test.ts +++ b/src/commands/interaction/interactions.test.ts @@ -73,6 +73,39 @@ 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 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..fa2e2fe294 100644 --- a/src/commands/interaction/interactions.ts +++ b/src/commands/interaction/interactions.ts @@ -85,6 +85,17 @@ export const interactionCliReaders = { }), fill: (positionals, flags) => { const decoded = readFillTargetFromPositionals(positionals); + if (flags.textStdin && decoded.text !== undefined) { + // The message names neither source: the positional may already be the secret. + 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 { ...commonInputFromFlags(flags), ...selectorSnapshotInputFromFlags(flags), @@ -92,6 +103,7 @@ export const interactionCliReaders = { text: decoded.text, delayMs: flags.delayMs, recordAs: flags.recordAs, + textStdin: flags.textStdin, verify: flags.verify, }; }, diff --git a/src/commands/interaction/metadata.ts b/src/commands/interaction/metadata.ts index 54c607af99..7e313a1a3b 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 b05f1cbbbb..db7c3a640a 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, or an extra text argument. While recording is armed it needs --record-as or --no-record; otherwise the step is left out of recorded actions. 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-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..91564efea7 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(JSON.stringify(session)).not.toContain(secret); +}); + +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-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..a4e7ada768 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -465,12 +465,17 @@ function mutuallyExclusiveRecordFlagsResponse(): DaemonResponse { ); } +const FILL_ONLY_FLAGS = [ + ['recordAs', '--record-as'], + ['textStdin', '--text-stdin'], +] 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 undefined; + 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.`); } /** @@ -493,7 +498,8 @@ function customActionFlagsResponse(req: DaemonRequest): DaemonResponse | undefin } function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { - if (req.command !== 'fill' || typeof req.flags?.recordAs !== 'string') return; + if (req.command !== 'fill') return; + if (typeof req.flags?.recordAs !== 'string' && req.flags?.textStdin !== true) return; registerDiagnosticSensitiveValue( inferFillText({ ts: 0, diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 547dc8ef83..8cff03fd27 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -59,6 +59,8 @@ export function recordActionEntry( entry: RecordActionEntry, ): SessionAction | undefined { if (entry.flags?.noRecord) return undefined; + // Stdin text stays out of session actions unless `--record-as` parameterizes it. + if (entry.flags?.textStdin === true && 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 146bf6ba67..68561626c3 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. It reads at most 64 KiB, removes exactly one trailing newline (`\n` or `\r\n`, so `echo` and `printf %s` send the same value), and refuses a terminal, empty input, or an additional text argument. The value is registered as sensitive for diagnostics. While script recording is armed, combine it with `--record-as ` to publish `${VAR}`, or `--no-record` to leave the step out; otherwise the step is not recorded. Batch steps do not accept it. 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. If `type` reports `TEXT_INPUT_NOT_FOCUSED`, focus a visible text input and retry; when accessibility does not expose the input, use a coordinate focus command before typing. From 1839fb34fe5c1be3467ba7e170bcfa2f1fcc6bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Fri, 9 Oct 2026 13:56:22 +0300 Subject: [PATCH 02/10] chore(gates): classify textStdin as a client-only flag --- scripts/integration-progress-model.ts | 1 + 1 file changed, 1 insertion(+) 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', From 9d2bc9074f36d0d26f2f0b99f96e407dfd62b96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Fri, 9 Oct 2026 14:21:31 +0300 Subject: [PATCH 03/10] fix(fill): harden --text-stdin input handling and docs - Mask target parse failures under --text-stdin: a stray positional may be the secret, and the generic target error echoed it. - Keep a leading byte order mark and refuse lone surrogates in string chunks instead of replacing them. - Only refuse batch steps that set textStdin to true. - Clarify help and docs: passing neither --record-as nor --no-record while recording is an error, invalid UTF-8 is refused, and printf %s is the verbatim way to pipe a value. - Tighten tests to assert each case's own input is never echoed. --- src/commands/batch/cli.test.ts | 11 +++++ src/commands/batch/projection.ts | 2 +- .../interaction/fill-text-stdin.test.ts | 30 ++++++++++--- src/commands/interaction/fill-text-stdin.ts | 29 ++++++++---- src/commands/interaction/index.ts | 2 +- src/commands/interaction/interactions.test.ts | 15 +++++++ src/commands/interaction/interactions.ts | 45 ++++++++++++++----- src/commands/schema/cli-help.ts | 2 +- .../__tests__/session-action-recorder.test.ts | 2 +- website/docs/docs/commands.md | 2 +- 10 files changed, 109 insertions(+), 31 deletions(-) diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index 88e9e0af9e..548526a486 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -448,3 +448,14 @@ test('batch rejects a fill step that asks for stdin text before daemon dispatch' 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.doesNotMatch(result.stderr, /textStdin is not supported/); +}); diff --git a/src/commands/batch/projection.ts b/src/commands/batch/projection.ts index 2a0559937e..5343eb52f1 100644 --- a/src/commands/batch/projection.ts +++ b/src/commands/batch/projection.ts @@ -82,7 +82,7 @@ function assertNoStdinFillStep( input: CommandInput, stepNumber: number, ): void { - if (command !== PUBLIC_COMMANDS.fill || input.textStdin === undefined) return; + 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.`, diff --git a/src/commands/interaction/fill-text-stdin.test.ts b/src/commands/interaction/fill-text-stdin.test.ts index 6ea353a6e5..83957d871b 100644 --- a/src/commands/interaction/fill-text-stdin.test.ts +++ b/src/commands/interaction/fill-text-stdin.test.ts @@ -38,20 +38,38 @@ test('accepts input exactly at the byte limit', async () => { 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'], - ['empty input', () => pipe(), 'fill_text_stdin_empty'], - ['input that is only a newline', () => pipe('\n'), 'fill_text_stdin_empty'], + ['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-', ], - ['invalid UTF-8', () => pipe(Buffer.from([0x73, 0x33, 0xff])), 'fill_text_stdin_invalid_utf8'], -])('refuses %s with a typed reason and no echoed input', async (_name, stdin, reason) => { + [ + 'a string chunk with a lone surrogate', + () => pipe('lone-surrogate-\uD800'), + 'fill_text_stdin_invalid_utf8', + 'lone-surrogate-', + ], +])('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); - expect(JSON.stringify({ message: error.message, details: error.details })).not.toMatch(/s3cret/); + 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 index 412f930c1a..8ca411d338 100644 --- a/src/commands/interaction/fill-text-stdin.ts +++ b/src/commands/interaction/fill-text-stdin.ts @@ -6,9 +6,10 @@ 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 - * `echo "$X"` and `printf '%s' "$X"` send the same text; all other whitespace is kept. - * The value may be a secret: no error raised here includes any of the bytes read. + * 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) { @@ -24,7 +25,7 @@ export async function readFillTextFromStdin(stdin: StdinSource): Promise const chunks: Uint8Array[] = []; let byteLength = 0; for await (const chunk of stdin) { - const bytes = typeof chunk === 'string' ? Buffer.from(chunk, 'utf8') : chunk; + const bytes = typeof chunk === 'string' ? encodeWellFormed(chunk) : chunk; byteLength += bytes.byteLength; if (byteLength > FILL_TEXT_STDIN_MAX_BYTES) { throw new AppError( @@ -45,16 +46,28 @@ export async function readFillTextFromStdin(stdin: StdinSource): Promise return text; } +const LONE_SURROGATE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? | fill <@ref|selector> | fill |<@ref|selector> --text-stdin', + 'fill | fill <@ref|selector> | fill --text-stdin', usageFlags: [], positionalArgs: ['targetOrX', 'yOrText', 'text?'], allowsExtraPositionals: true, diff --git a/src/commands/interaction/interactions.test.ts b/src/commands/interaction/interactions.test.ts index ebf8468a67..0f64f3dd85 100644 --- a/src/commands/interaction/interactions.test.ts +++ b/src/commands/interaction/interactions.test.ts @@ -83,6 +83,21 @@ test('fill --text-stdin reads only the target and leaves the text to stdin', () } }); +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'], diff --git a/src/commands/interaction/interactions.ts b/src/commands/interaction/interactions.ts index fa2e2fe294..79154b99d1 100644 --- a/src/commands/interaction/interactions.ts +++ b/src/commands/interaction/interactions.ts @@ -84,18 +84,9 @@ export const interactionCliReaders = { delayMs: flags.delayMs, }), fill: (positionals, flags) => { - const decoded = readFillTargetFromPositionals(positionals); - if (flags.textStdin && decoded.text !== undefined) { - // The message names neither source: the positional may already be the secret. - 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.', - }, - ); - } + const decoded = flags.textStdin + ? readStdinFillTarget(positionals) + : readFillTargetFromPositionals(positionals); return { ...commonInputFromFlags(flags), ...selectorSnapshotInputFromFlags(flags), @@ -166,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/schema/cli-help.ts b/src/commands/schema/cli-help.ts index db7c3a640a..a8a9b91f47 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -180,7 +180,7 @@ Reusable open-to-destination scripts: 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, or an extra text argument. While recording is armed it needs --record-as or --no-record; otherwise the step is left out of recorded actions. It is not available inside batch steps. + --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/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 91564efea7..e90d60e9f8 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -787,7 +787,7 @@ test('a stdin fill without --record-as never enters recording state', () => { expect(action).toBeUndefined(); expect(session.actions).toHaveLength(0); - expect(JSON.stringify(session)).not.toContain(secret); + expect(session.recordedFillLiterals).toBeUndefined(); }); test('a stdin fill with --record-as is recorded as its ${VAR} placeholder', () => { diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 68561626c3..d7aefeb655 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -465,7 +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. It reads at most 64 KiB, removes exactly one trailing newline (`\n` or `\r\n`, so `echo` and `printf %s` send the same value), and refuses a terminal, empty input, or an additional text argument. The value is registered as sensitive for diagnostics. While script recording is armed, combine it with `--record-as ` to publish `${VAR}`, or `--no-record` to leave the step out; otherwise the step is not recorded. Batch steps do not accept it. +`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. 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. 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. If `type` reports `TEXT_INPUT_NOT_FOCUSED`, focus a visible text input and retry; when accessibility does not expose the input, use a coordinate focus command before typing. From 0964704e4fb84068369a5bd175d5ef82e5c8c9aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Fri, 9 Oct 2026 14:43:45 +0300 Subject: [PATCH 04/10] fix(fill): accept a surrogate pair split across stdin string chunks - A string stream may split a surrogate pair across chunks, and the per-chunk lone-surrogate check refused valid input. A trailing high surrogate is now held back until the next string chunk; a byte chunk or the end of input still refuses it as invalid UTF-8. - Size a string chunk with Buffer.byteLength before encoding it, so an oversized chunk is refused without being copied first. --- .../interaction/fill-text-stdin.test.ts | 13 ++++++ src/commands/interaction/fill-text-stdin.ts | 44 ++++++++++++++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/commands/interaction/fill-text-stdin.test.ts b/src/commands/interaction/fill-text-stdin.test.ts index 83957d871b..c2612581e4 100644 --- a/src/commands/interaction/fill-text-stdin.test.ts +++ b/src/commands/interaction/fill-text-stdin.test.ts @@ -29,6 +29,7 @@ test.each([ ['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); }); @@ -64,6 +65,18 @@ test.each([ '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()); diff --git a/src/commands/interaction/fill-text-stdin.ts b/src/commands/interaction/fill-text-stdin.ts index 8ca411d338..288ba3e417 100644 --- a/src/commands/interaction/fill-text-stdin.ts +++ b/src/commands/interaction/fill-text-stdin.ts @@ -3,7 +3,7 @@ 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 }; +type StdinSource = AsyncIterable & { isTTY?: boolean }; /** * Reads the `fill --text-stdin` value. Exactly one trailing `\n` or `\r\n` is removed so a value @@ -24,9 +24,11 @@ export async function readFillTextFromStdin(stdin: StdinSource): Promise } const chunks: Uint8Array[] = []; let byteLength = 0; + let heldHighSurrogate = ''; for await (const chunk of stdin) { - const bytes = typeof chunk === 'string' ? encodeWellFormed(chunk) : chunk; - byteLength += bytes.byteLength; + const next = joinHeldHighSurrogate(heldHighSurrogate, chunk); + heldHighSurrogate = next.held; + byteLength += utf8ByteLength(next.piece); if (byteLength > FILL_TEXT_STDIN_MAX_BYTES) { throw new AppError( 'INVALID_ARGS', @@ -34,8 +36,9 @@ export async function readFillTextFromStdin(stdin: StdinSource): Promise { reason: 'fill_text_stdin_too_large', maxBytes: FILL_TEXT_STDIN_MAX_BYTES }, ); } - chunks.push(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.', { @@ -47,11 +50,38 @@ export async function readFillTextFromStdin(stdin: StdinSource): Promise } const LONE_SURROGATE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? Date: Sat, 10 Oct 2026 01:01:39 +0300 Subject: [PATCH 05/10] fix(fill): never return --text-stdin text on any response surface A fill whose text is sensitive (--record-as set or --text-stdin) now decides it through one predicate, isSensitiveFillText, for the response payloads, the recorder skip and diagnostics registration. Without --record-as, the response and result text read [REDACTED] and value-carrying selector candidates are dropped, so an unrecorded or --no-record stdin fill no longer echoes the value in --json output or the client result. An error leaving the router now passes through the request's registered sensitive values, so a backend message that echoes the value after admission is redacted as well. --- packages/ad-script/src/index.ts | 1 + .../ad-script/src/internal/recorded-input.ts | 12 ++ packages/host-kit/src/diagnostics.ts | 1 + packages/host-kit/src/internal/diagnostics.ts | 10 ++ .../request-router-fill-text-stdin.test.ts | 122 ++++++++++++++++++ .../internal/interaction-common.ts | 16 ++- src/daemon/request-router.ts | 8 +- src/daemon/session-action-recorder.ts | 5 +- 8 files changed, 166 insertions(+), 9 deletions(-) create mode 100644 src/daemon/__tests__/request-router-fill-text-stdin.test.ts diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index ac520cc657..f4fb54af38 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -66,6 +66,7 @@ export { export { inferFillText, + isSensitiveFillText, readRecordedInputVariableName, recordedInputPlaceholder, validateRecordedInputVariableName, diff --git a/packages/ad-script/src/internal/recorded-input.ts b/packages/ad-script/src/internal/recorded-input.ts index e74e5e56c2..fcfa32010a 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,17 @@ 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; +} + export function recordedInputPlaceholder(variableName: string): string { return `\${${variableName}}`; } diff --git a/packages/host-kit/src/diagnostics.ts b/packages/host-kit/src/diagnostics.ts index def5222700..4e6e730a56 100644 --- a/packages/host-kit/src/diagnostics.ts +++ b/packages/host-kit/src/diagnostics.ts @@ -5,6 +5,7 @@ export { emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, + redactRegisteredSensitiveValues, registerDiagnosticSensitiveValue, type ResourceDiagnostic, updateDiagnosticsScope, diff --git a/packages/host-kit/src/internal/diagnostics.ts b/packages/host-kit/src/internal/diagnostics.ts index 94df8f4da5..0b99a528c8 100644 --- a/packages/host-kit/src/internal/diagnostics.ts +++ b/packages/host-kit/src/internal/diagnostics.ts @@ -107,6 +107,16 @@ export function registerDiagnosticSensitiveValue(value: string): void { scope.sortedSensitiveValues = undefined; } +/** + * Replace this request's registered sensitive values in `input`, as its diagnostics already do. For + * an error leaving the daemon: a backend message can echo a value the caller kept out of argv. + */ +export function redactRegisteredSensitiveValues(input: T): T { + const scope = diagnosticsStorage.getStore(); + if (!scope || scope.sensitiveValues.size === 0) return input; + return replaceSensitiveValues(input, sortedScopeSensitiveValues(scope)) as T; +} + /** * Sum the number of diagnostic events emitted in the current scope whose phase * is one of `phases`. Backed by the flush-surviving `phaseCounts` tally, so it 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..84223fc82d --- /dev/null +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -0,0 +1,122 @@ +import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts'; +/** + * #3260: `fill --text-stdin` keeps the value out of argv, so it must not come back on the + * response either. 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 path from 'node:path'; +import type { WebProvider } from '@agent-device/platform-web'; +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 { + createPlatformRuntimeGateway, + createRequestPlatformProviders, +} from '../../platform-runtime.ts'; + +const SECRET = 'stdin-s3cret-value'; + +function fillWithStdinText( + session: SessionState, + flags: Record = {}, + 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, + command: 'fill', + positionals: ['10', '20', SECRET], + flags: { textStdin: true, ...flags }, + 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); + 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('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'); +}); diff --git a/src/daemon/interaction/internal/interaction-common.ts b/src/daemon/interaction/internal/interaction-common.ts index 73f0694c72..79e7e48afe 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, + recordedInputPlaceholder, +} from '@agent-device/ad-script'; import { parameterizeRecordedFillPayload } from '@agent-device/selectors/parameterized-recorded-fill'; import type { InteractionFinalizationOperations } from './types.ts'; @@ -90,6 +94,9 @@ function computeMultiTargetEvidence(recordedTargets: { return source && destination ? { source, destination } : undefined; } +/** What sensitive fill text reads as when no `--record-as` placeholder names it. */ +const REDACTED_FILL_TEXT = '[REDACTED]'; + function parameterizeFillPayloads(params: { command: string; positionals: string[]; @@ -97,7 +104,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 +114,10 @@ function parameterizeFillPayloads(params: { flags: params.flags, result: params.result, }); - const placeholder = recordedInputPlaceholder(params.flags.recordAs); + const placeholder = + typeof params.flags?.recordAs === 'string' + ? recordedInputPlaceholder(params.flags.recordAs) + : REDACTED_FILL_TEXT; return [ parameterizeRecordedFillPayload(params.result, literal, placeholder), parameterizeRecordedFillPayload(params.responseData, literal, placeholder), diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index a4e7ada768..5ff6426492 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -37,6 +37,7 @@ import { emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, + redactRegisteredSensitiveValues, registerDiagnosticSensitiveValue, withDiagnosticsScope, } from '@agent-device/host-kit/diagnostics'; @@ -55,7 +56,7 @@ 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 } from '@agent-device/ad-script'; import { createPlatformRequestScope } from './platform-request-scope.ts'; import { createOwnerScopedDeviceClaimReconciler } from './device/device-claim-owner-recovery.ts'; import { isConfinedToAppLease, scopeRequestSession } from './request-admission.ts'; @@ -207,7 +208,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { repairExpiredIfTombstoned(req, response.error, sessionStore), sessionStore, ); - return { ok: false, error: enrichDaemonError(error) }; + return { ok: false, error: redactRegisteredSensitiveValues(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. @@ -498,8 +499,7 @@ function customActionFlagsResponse(req: DaemonRequest): DaemonResponse | undefin } function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { - if (req.command !== 'fill') return; - if (typeof req.flags?.recordAs !== 'string' && req.flags?.textStdin !== true) return; + if (req.command !== 'fill' || !isSensitiveFillText(req.flags)) return; registerDiagnosticSensitiveValue( inferFillText({ ts: 0, diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 8cff03fd27..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,8 +60,8 @@ export function recordActionEntry( entry: RecordActionEntry, ): SessionAction | undefined { if (entry.flags?.noRecord) return undefined; - // Stdin text stays out of session actions unless `--record-as` parameterizes it. - if (entry.flags?.textStdin === true && entry.flags.recordAs === undefined) 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); From 68acc36d7aa93107baddc037510354fce43c3959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Sat, 10 Oct 2026 01:33:15 +0300 Subject: [PATCH 06/10] fix(fill): refuse a fill-only flag of the wrong type at the request seam - recordAs and textStdin mark fill text as sensitive, but the request boundary only checks that flags is an object. A direct daemon request with textStdin: "true" was read as unmarked, so the value came back in data.text; recordAs: 42 failed later as an UNKNOWN TypeError. Both are now refused as INVALID_ARGS before any device work. - Assert the --no-record response text is exactly [REDACTED]. --- .../request-router-fill-text-stdin.test.ts | 22 +++++++++++++++++++ src/daemon/request-router.ts | 19 +++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts index 84223fc82d..cc7442e5e5 100644 --- a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -91,6 +91,7 @@ test('an armed --text-stdin --no-record fill returns no part of the value and re 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([]); }); @@ -107,6 +108,27 @@ test('an armed --text-stdin fill without --record-as or --no-record is refused b 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 }), diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 5ff6426492..1a95447083 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -467,18 +467,31 @@ function mutuallyExclusiveRecordFlagsResponse(): DaemonResponse { } const FILL_ONLY_FLAGS = [ - ['recordAs', '--record-as'], - ['textStdin', '--text-stdin'], + ['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.command === '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]}.`); +} + /** * `--actions` reads custom actions through the private-AX snapshot path, which * a raw capture deliberately does not take (ADR 0004: raw preserves the tree From 2230e33b1c22c77bb13197689f08a33a4b6045db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Sat, 10 Oct 2026 14:29:05 +0300 Subject: [PATCH 07/10] fix(fill): redact only a sensitive fill's own text from its error - The error exit replaced every value registered for the request across the whole error. Maestro inputText and --record-as literals are registered too, so a short value such as "1" rewrote logPath, diagnosticId and the error of a non-fill replay. Only a fill marked sensitive is redacted now, only its own text, and only in message, hint, cause and details, with the same placeholder as the success path (${VAR} with --record-as, [REDACTED] otherwise). - Refuse textStdin on a daemon batch step: a failed step returns its positionals, and the CLI already refuses it there. - Remove redactRegisteredSensitiveValues from host-kit. --- packages/ad-script/src/index.ts | 1 + .../ad-script/src/internal/recorded-input.ts | 9 ++ packages/command-registry/src/batch.ts | 17 ++++ packages/host-kit/src/diagnostics.ts | 1 - packages/host-kit/src/internal/diagnostics.ts | 10 --- .../request-router-error-redaction.test.ts | 88 +++++++++++++++++++ .../request-router-fill-text-stdin.test.ts | 72 ++++++++++++++- .../internal/interaction-common.ts | 10 +-- src/daemon/request-router.ts | 61 ++++++++++--- website/docs/docs/commands.md | 2 +- 10 files changed, 236 insertions(+), 35 deletions(-) create mode 100644 src/daemon/__tests__/request-router-error-redaction.test.ts diff --git a/packages/ad-script/src/index.ts b/packages/ad-script/src/index.ts index f4fb54af38..9ce3160af6 100644 --- a/packages/ad-script/src/index.ts +++ b/packages/ad-script/src/index.ts @@ -69,6 +69,7 @@ export { 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 fcfa32010a..742934f7d4 100644 --- a/packages/ad-script/src/internal/recorded-input.ts +++ b/packages/ad-script/src/internal/recorded-input.ts @@ -32,6 +32,15 @@ export function isSensitiveFillText | 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..d87c5c8b0c 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,22 @@ 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` on a step before reading stdin; this refuses it for every other daemon client. + */ +function assertNoTextStdinStep( + flags: Record | undefined, + stepNumber: number, +): void { + if (flags?.textStdin === undefined) 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/host-kit/src/diagnostics.ts b/packages/host-kit/src/diagnostics.ts index 4e6e730a56..def5222700 100644 --- a/packages/host-kit/src/diagnostics.ts +++ b/packages/host-kit/src/diagnostics.ts @@ -5,7 +5,6 @@ export { emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, - redactRegisteredSensitiveValues, registerDiagnosticSensitiveValue, type ResourceDiagnostic, updateDiagnosticsScope, diff --git a/packages/host-kit/src/internal/diagnostics.ts b/packages/host-kit/src/internal/diagnostics.ts index 0b99a528c8..94df8f4da5 100644 --- a/packages/host-kit/src/internal/diagnostics.ts +++ b/packages/host-kit/src/internal/diagnostics.ts @@ -107,16 +107,6 @@ export function registerDiagnosticSensitiveValue(value: string): void { scope.sortedSensitiveValues = undefined; } -/** - * Replace this request's registered sensitive values in `input`, as its diagnostics already do. For - * an error leaving the daemon: a backend message can echo a value the caller kept out of argv. - */ -export function redactRegisteredSensitiveValues(input: T): T { - const scope = diagnosticsStorage.getStore(); - if (!scope || scope.sensitiveValues.size === 0) return input; - return replaceSensitiveValues(input, sortedScopeSensitiveValues(scope)) as T; -} - /** * Sum the number of diagnostic events emitted in the current scope whose phase * is one of `phases`. Backed by the flush-surviving `phaseCounts` tally, so it 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 index cc7442e5e5..7631b4ddff 100644 --- a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -1,12 +1,14 @@ -import { createTestDeviceInventoryGateways } from '../../__tests__/test-utils/device-inventory-gateways.ts'; /** * #3260: `fill --text-stdin` keeps the value out of argv, so it must not come back on the * response either. 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'; @@ -25,6 +27,19 @@ 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); @@ -65,9 +80,7 @@ function fillWithStdinText( const response = handler({ token: 'test-token', session: session.name, - command: 'fill', - positionals: ['10', '20', SECRET], - flags: { textStdin: true, ...flags }, + ...request, meta: { requestId: 'req-fill-text-stdin' }, }); return { response, typed, sessionStore }; @@ -142,3 +155,54 @@ test('a backend error that echoes the --text-stdin value does not return it', as 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('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); +}); diff --git a/src/daemon/interaction/internal/interaction-common.ts b/src/daemon/interaction/internal/interaction-common.ts index 79e7e48afe..520b4e428a 100644 --- a/src/daemon/interaction/internal/interaction-common.ts +++ b/src/daemon/interaction/internal/interaction-common.ts @@ -10,7 +10,7 @@ import type { MultiTargetAnnotationV1 } from '@agent-device/contracts/replay'; import { inferFillText, isSensitiveFillText, - recordedInputPlaceholder, + sensitiveFillPlaceholder, } from '@agent-device/ad-script'; import { parameterizeRecordedFillPayload } from '@agent-device/selectors/parameterized-recorded-fill'; import type { InteractionFinalizationOperations } from './types.ts'; @@ -94,9 +94,6 @@ function computeMultiTargetEvidence(recordedTargets: { return source && destination ? { source, destination } : undefined; } -/** What sensitive fill text reads as when no `--record-as` placeholder names it. */ -const REDACTED_FILL_TEXT = '[REDACTED]'; - function parameterizeFillPayloads(params: { command: string; positionals: string[]; @@ -114,10 +111,7 @@ function parameterizeFillPayloads(params: { flags: params.flags, result: params.result, }); - const placeholder = - typeof params.flags?.recordAs === 'string' - ? recordedInputPlaceholder(params.flags.recordAs) - : REDACTED_FILL_TEXT; + const placeholder = sensitiveFillPlaceholder(params.flags); return [ parameterizeRecordedFillPayload(params.result, literal, placeholder), parameterizeRecordedFillPayload(params.responseData, literal, placeholder), diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 1a95447083..5195497e1f 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -37,7 +37,6 @@ import { emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, - redactRegisteredSensitiveValues, registerDiagnosticSensitiveValue, withDiagnosticsScope, } from '@agent-device/host-kit/diagnostics'; @@ -56,7 +55,11 @@ 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, isSensitiveFillText } from '@agent-device/ad-script'; +import { + inferFillText, + isSensitiveFillText, + sensitiveFillPlaceholder, +} from '@agent-device/ad-script'; import { createPlatformRequestScope } from './platform-request-scope.ts'; import { createOwnerScopedDeviceClaimReconciler } from './device/device-claim-owner-recovery.ts'; import { isConfinedToAppLease, scopeRequestSession } from './request-admission.ts'; @@ -208,7 +211,10 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { repairExpiredIfTombstoned(req, response.error, sessionStore), sessionStore, ); - return { ok: false, error: redactRegisteredSensitiveValues(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. @@ -511,15 +517,48 @@ 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' || !isSensitiveFillText(req.flags)) 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 replace = (value: string) => value.replaceAll(text, sensitiveFillPlaceholder(req.flags)); + 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]) => [key, mapErrorStrings(entry, replace)]), ); } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 274a988b31..87f05d1716 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -465,7 +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. 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. +`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, error messages included, shows it as `${VAR}` with `--record-as` and as `[REDACTED]` otherwise. 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. From 14eafc4bb22c64043b59382cc4191b938b08f293 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Sat, 10 Oct 2026 16:16:29 +0300 Subject: [PATCH 08/10] fix(fill): redact a replayed fill's error and accept textStdin false in batch - A same-session replay runs its fill through the replay-scoped invoker, which registers the sensitive value but returned the fill's error as is. It now redacts that error like handleRequest does, so the replay message carries the fill's placeholder. - The daemon batch validator refuses only textStdin: true, matching the CLI guard; false is the ordinary argv path. - The error path uses the success path's parameterizeSensitiveString for values and keys, so a whitespace-only value collapses the string. - Docs: limit the redaction guarantee to value-bearing fields. --- packages/command-registry/src/batch.ts | 5 +- .../src/parameterized-recorded-fill.ts | 6 +- src/commands/batch/cli.test.ts | 1 + .../request-router-fill-text-stdin.test.ts | 61 ++++++++++++++++++- src/daemon/request-router.ts | 18 +++++- website/docs/docs/commands.md | 2 +- 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/packages/command-registry/src/batch.ts b/packages/command-registry/src/batch.ts index d87c5c8b0c..f0fad3e615 100644 --- a/packages/command-registry/src/batch.ts +++ b/packages/command-registry/src/batch.ts @@ -239,13 +239,14 @@ export function validateAndNormalizeBatchSteps( /** * A batch step has no stdin of its own, and a failed step reports its positionals. The CLI refuses - * `textStdin` on a step before reading stdin; this refuses it for every other daemon client. + * `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 === undefined) return; + 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.`, diff --git a/packages/selectors/src/parameterized-recorded-fill.ts b/packages/selectors/src/parameterized-recorded-fill.ts index 93f320ce53..9f33ca8147 100644 --- a/packages/selectors/src/parameterized-recorded-fill.ts +++ b/packages/selectors/src/parameterized-recorded-fill.ts @@ -362,7 +362,11 @@ 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. diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index 548526a486..04f152d5d0 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -457,5 +457,6 @@ test('batch passes an explicit fill textStdin false through to dispatch', async ]); 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/daemon/__tests__/request-router-fill-text-stdin.test.ts b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts index 7631b4ddff..0465eb40f5 100644 --- a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -1,7 +1,8 @@ /** * #3260: `fill --text-stdin` keeps the value out of argv, so it must not come back on the - * response either. Sent through the real request handler to a web session, the narrowest - * runtime that still runs the full fill admission, dispatch and response projection. + * 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'; @@ -16,6 +17,7 @@ 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, @@ -206,3 +208,58 @@ test('a batch step carrying --text-stdin is refused before typing and returns no 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/request-router.ts b/src/daemon/request-router.ts index 5195497e1f..9e04ca2f25 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -61,6 +61,7 @@ import { 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'; @@ -426,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), + ); }; } @@ -544,7 +549,8 @@ const ERROR_TEXT_FIELDS: ReadonlySet = new Set(['message', 'hint', 'caus function redactSensitiveFillTextInError(req: DaemonRequest, error: DaemonError): DaemonError { const text = sensitiveFillText(req); if (!text) return error; - const replace = (value: string) => value.replaceAll(text, sensitiveFillPlaceholder(req.flags)); + const placeholder = sensitiveFillPlaceholder(req.flags); + const replace = (value: string) => parameterizeSensitiveString(value, text, placeholder); return Object.fromEntries( Object.entries(error).map(([field, value]) => [ field, @@ -558,10 +564,16 @@ function mapErrorStrings(value: unknown, replace: (value: string) => string): un 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]) => [key, mapErrorStrings(entry, replace)]), + 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/website/docs/docs/commands.md b/website/docs/docs/commands.md index 3c8f39daf0..65cb4ad818 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -465,7 +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, error messages included, shows it as `${VAR}` with `--record-as` and as `[REDACTED]` otherwise. 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. +`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. From 6cdfbc7413592cad1cf7431b88f85b70e3e5f312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Sat, 10 Oct 2026 19:12:02 +0300 Subject: [PATCH 09/10] fix(fill): keep a stdin value that contains its placeholder out of responses parameterizeSensitiveString split its input on the placeholder before looking for the value, so a value that contains the placeholder (`abc[REDACTED]xyz`, or `${VAR}` inside a `--record-as VAR` value) was never found and came back whole in a backend error. The helper now collapses the whole string to the placeholder whenever the value still occurs outside a placeholder. A value found only inside placeholders (`$` in `${DOLLAR}`) stays as before. --- .../src/parameterized-recorded-fill.ts | 47 +++++++++++++++++-- .../request-router-fill-text-stdin.test.ts | 38 +++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/packages/selectors/src/parameterized-recorded-fill.ts b/packages/selectors/src/parameterized-recorded-fill.ts index 9f33ca8147..44ee6eaff3 100644 --- a/packages/selectors/src/parameterized-recorded-fill.ts +++ b/packages/selectors/src/parameterized-recorded-fill.ts @@ -373,15 +373,54 @@ export function parameterizeSensitiveString( 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/src/daemon/__tests__/request-router-fill-text-stdin.test.ts b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts index 0465eb40f5..ae83c55064 100644 --- a/src/daemon/__tests__/request-router-fill-text-stdin.test.ts +++ b/src/daemon/__tests__/request-router-fill-text-stdin.test.ts @@ -171,6 +171,44 @@ test('a backend error that echoes a --record-as stdin value shows its placeholde 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( From b508271c349c80b8392f175e6f19db79bd66367d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Metehan=20Bi=C3=A7er?= Date: Sat, 10 Oct 2026 20:54:27 +0300 Subject: [PATCH 10/10] test(selectors): pin the placeholder collapse in parameterizeSensitiveString Covers a value that contains the placeholder, a value that crosses a placeholder already in the string, a value found only inside placeholders, and a plain replace that is stable on a second pass. --- .../src/parameterized-recorded-fill.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 packages/selectors/src/parameterized-recorded-fill.test.ts 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); +});