From 8f244e8f35cca1b661e3757c06ef9e57302455c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 21:58:01 +0200 Subject: [PATCH 1/7] feat: parameterize recorded inputs --- CONTEXT.md | 5 + .../0016-active-session-script-publication.md | 23 +++-- .../adr/0017-parameterized-recorded-inputs.md | 85 +++++++++++++++++ docs/adr/README.md | 1 + scripts/integration-progress-model.ts | 1 + .../__tests__/args-parse-interaction.test.ts | 10 +- .../__tests__/cli-help-command-usage.test.ts | 1 + .../parser/__tests__/cli-help-topics.test.ts | 3 + src/cli/parser/cli-help.ts | 6 +- src/client/client-types.ts | 4 + .../cli-grammar/flag-definitions-action.ts | 7 ++ src/commands/command-flags.ts | 1 + src/commands/interaction/index.ts | 8 +- src/commands/interaction/interactions.test.ts | 11 +++ src/commands/interaction/interactions.ts | 1 + src/commands/interaction/metadata.ts | 3 + src/contracts/cli-flags.ts | 2 + .../__tests__/session-action-recorder.test.ts | 47 ++++++++++ src/daemon/__tests__/session-store.test.ts | 25 +++++ src/daemon/handlers/interaction-common.ts | 43 ++++++++- .../handlers/interaction-recorded-input.ts | 26 ++++++ src/daemon/handlers/interaction-touch.ts | 6 ++ .../handlers/session-replay-action-runtime.ts | 11 ++- src/daemon/request-router.ts | 28 +++++- src/daemon/session-action-recorder.ts | 63 +++++++++++-- src/mcp/__tests__/router.test.ts | 1 + src/replay/recorded-input.ts | 56 +++++++++++ src/utils/__tests__/diagnostics.test.ts | 22 +++++ src/utils/diagnostics.ts | 39 +++++++- .../active-session-script-publication.test.ts | 92 +++++++++++++++++++ 30 files changed, 597 insertions(+), 34 deletions(-) create mode 100644 docs/adr/0017-parameterized-recorded-inputs.md create mode 100644 src/daemon/handlers/interaction-recorded-input.ts create mode 100644 src/replay/recorded-input.ts diff --git a/CONTEXT.md b/CONTEXT.md index 6956b48169..0750d79420 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -30,6 +30,11 @@ - Session: daemon-owned state for a selected target and opened app or surface. - Script recording: opt-in session mode armed before actions so a persisted `.ad` can carry portable action inputs and recording-time target identity evidence. It is distinct from screen/video recording. +- Recorded input parameterization: explicit fill authoring contract that sends literal text only to the + live interaction while the recorder stores `${VAR}` before any durable recording/event/publication + boundary. The caller owns the uppercase variable name; no selector or field-name heuristic infers + sensitivity. Replay resolves the placeholder immediately before dispatch and preserves the authored + placeholder if that run is recorded again. - Open-to-destination script: self-contained `.ad` script with exactly one initial `open`, a destination guard after its last app-state mutation, no `close`, and an app session left active for subsequent work. Avoid: replay (artifact noun), fragment (reserved for lifecycle-free composition), partial script. diff --git a/docs/adr/0016-active-session-script-publication.md b/docs/adr/0016-active-session-script-publication.md index 0ce9a38b93..a123e1caa8 100644 --- a/docs/adr/0016-active-session-script-publication.md +++ b/docs/adr/0016-active-session-script-publication.md @@ -132,15 +132,14 @@ script's terminal `close` — or a repair-armed run's deferred equivalent — ha ### Sensitive inputs -Executable `.ad` artifacts serialize action inputs literally. A recorded `fill` therefore writes its -text to the published file; diagnostic and event-log redaction cannot protect an input that the replay -engine must later execute. V1 does not claim secret-bearing login flows are safe to publish. +Executable `.ad` artifacts serialize ordinary action inputs literally; diagnostic and event-log +redaction cannot protect an input that replay must later execute. ADR 0017 resolves #1348 for explicit +sensitive fills: `fill ... --record-as PASSWORD` sends the live text to the app while recording only +`${PASSWORD}`. Replay receives the value through `AD_VAR_PASSWORD` or `--env PASSWORD=`. -Native `.ad` replay already supports late-bound `${VAR}` values, but ordinary recording cannot yet -execute with a real value while publishing only its placeholder. That safe-authoring capability is -tracked in [#1348](https://github.com/callstack/agent-device/issues/1348). Until it ships, authors must not -record a journey that enters a secret: use pre-authenticated test state or deliberately non-secret fixture -credentials that are safe to persist. CLI help must state this warning next to the authoring workflow. +This is opt-in and fill-only. Unparameterized fill/type inputs remain literal artifact content, so +authors must use ADR 0017 for each sensitive fill and avoid secret-bearing `type` steps. CLI help states +both the safe workflow and the remaining literal-input warning next to publication guidance. ### Artifact contract @@ -188,8 +187,8 @@ executing that script, not the artifact being saved. bootstrap; authoring resumes only in a fresh session. - Intermediate lifecycle-free fragments, entry guards, include semantics, composed digests, and shared fragment pinning remain entirely under #1336. -- Secret-bearing authoring remains unsafe until #1348; the initial workflow is limited to journeys that - do not enter secrets. Arbitrary history ranges remain out of scope. +- Secret-bearing fill authoring uses ADR 0017's explicit `--record-as ` contract. Unparameterized + fill/type inputs remain literal script content. Arbitrary history ranges remain out of scope. - On consumption (issue #1384), a `replay` whose script has no terminal `close` leaves a live daemon and app session behind — including the ordinary one-shot `replay` invocation's own owned/ephemeral daemon, which the client keeps alive and reports a `--state-dir` address hint for instead of tearing down. An @@ -245,7 +244,7 @@ executing that script, not the artifact being saved. request-sensitive read-only/mutating subcommands, and destination-guard ordering consumes only that trait. - Repair-armed sessions refuse this action without changing repair state. -- CLI help warns that literal `fill` inputs are persisted and tells authors not to record secret-bearing - journeys until #1348's parameterized-input mechanism is available. +- CLI help documents ADR 0017's `--record-as` workflow and warns that unparameterized fill/type inputs + remain literal script content. - Provider-backed integration scenarios cover the public daemon route, and live iOS and Android runs prove the saved artifact and post-save session behavior on real backends. diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md new file mode 100644 index 0000000000..9ece01064f --- /dev/null +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -0,0 +1,85 @@ +# ADR 0017: Parameterized Recorded Inputs + +## Status + +Accepted + +## Context + +Ordinary `.ad` recording persists executable inputs. A literal `fill` value therefore becomes part of +the published script, including any password or token supplied during authoring. Diagnostic and event +redaction cannot make that executable artifact safe because replay still needs the value. + +Native replay already resolves `${VAR}` from `--env`/`-e` and `AD_VAR_*` immediately before dispatch. +The missing boundary is authoring: the live app needs the literal, while recording state and every +publication write must receive only the placeholder. Issue +[#1348](https://github.com/callstack/agent-device/issues/1348) defines this requirement; ADR 0016's +active-session publication deliberately excluded secret-bearing journeys until it was solved. + +## Decision + +Add a fill-only `recordAs` input, spelled `--record-as ` on the CLI and exposed as `recordAs` by +the Node and MCP fill contracts: + +```sh +export AD_VAR_PASSWORD='' +agent-device open com.example.app --save-script=login.ad +agent-device fill 'id="password"' "$AD_VAR_PASSWORD" --record-as PASSWORD +agent-device wait 'role="heading" label="Home"' +agent-device session save-script +``` + +The live fill dispatch receives the literal. Its public response, recording state, session event, and +published script use `${PASSWORD}`. Replay later resolves the placeholder from `AD_VAR_PASSWORD` or +`--env PASSWORD=` before the fill is dispatched. + +Variable names use replay's uppercase `[A-Z_][A-Z0-9_]*` grammar. `AD_*` remains reserved for runtime +built-ins. `--record-as` is rejected unless script recording is armed, and it cannot be combined with +`--no-record`. Ordinary fills without the option keep literal recording behavior. Sensitivity is never +inferred from a selector, label, field name, or value shape. + +### Mapping contract + +The caller names the mapping explicitly. Reusing `PASSWORD` deterministically emits `${PASSWORD}`; +using `API_TOKEN` emits `${API_TOKEN}`. There is no secret-to-name table and no literal comparison or +deduplication state. This both avoids retaining a second copy of the secret and makes distinct-value +naming an authoring decision visible in the script. + +### Data-flow boundary + +The literal exists only on the live request path long enough to execute the interaction: + +1. The request scope registers it as an explicit diagnostics secret before platform work. This is in + addition to key-name redaction, because an opaque value need not look secret. +2. The platform interaction receives the literal. +3. Before response/event/recording retention, value-bearing interaction-result fields replace every + occurrence of the literal with `${VAR}`. Stable selector/ref provenance is preserved byte-for-byte; + post-fill selector-chain candidates derived from the new value are dropped rather than serialized. +4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the + placeholder into `SessionAction`, and scrubs value-bearing result strings again. ADR 0012 + `target-v1` evidence keeps its structure, but any matching string is parameterized as well so a + repeated fill cannot leak through a pre-action accessibility label. The `recordAs` control flag + itself is not serialized into the script. +5. The existing ADR 0012/0016 writer receives an already-parameterized action. Its selector provenance, + portability checks, same-directory temp write, and atomic publication algorithm are unchanged. + +Because the target and temp file are written from the same prepared script string, neither can contain +the literal once this pre-publication boundary has run. + +### Replay and repair + +Missing variables retain replay's existing fail-loud behavior: `${VAR}` resolution throws before the +corresponding action is invoked. When replay dispatches an authored fill whose complete text token is a +`${VAR}` placeholder, its daemon-only provenance channel derives `recordAs=VAR` for that live request. +If the replay is itself being recorded or repaired, the recorder therefore retains the original +placeholder rather than serializing the expanded value. Embedded interpolation remains ordinary script +input; safe authoring emits one complete placeholder token. + +## Consequences + +- Secret-bearing login/bootstrap scripts can use ADR 0016 active publication safely. +- The caller must opt in for every sensitive fill; unparameterized fill/type values remain literal + script content and help warns accordingly. +- `type` and mutating `find ... fill|type` parameterization are not added by this decision. They require + their own surface and provenance design if needed. +- No new publication format, variable store, secret registry, or sensitivity heuristic is introduced. diff --git a/docs/adr/README.md b/docs/adr/README.md index b956a8f957..2b7a1ce1d7 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -18,6 +18,7 @@ | [0014 Session Ref-Frame Lifetime](0014-session-ref-frame-lifetime.md) | ref authorization epochs, complete/partial issuance, pre-side-effect expiration, replay/batch compatibility, and cross-platform stale-mutation policy | | [0015 Direct Maestro Compatibility Engine](0015-direct-maestro-engine.md) | Maestro YAML parsing/execution, compatibility observation policy, conformance, performance gates, gesture integration | | [0016 Active-Session Script Publication](0016-active-session-script-publication.md) | publishing an armed open-to-destination `.ad` script without closing its live session | +| [0017 Parameterized Recorded Inputs](0017-parameterized-recorded-inputs.md) | safely authoring sensitive fill inputs as `${VAR}` placeholders across recording, replay, and repair | ADRs record *why*; the registries and gates they describe are the living source of truth — when prose and a registry disagree, the registry wins and the ADR needs a follow-up. diff --git a/scripts/integration-progress-model.ts b/scripts/integration-progress-model.ts index ceb16e1f2a..81963894be 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -155,6 +155,7 @@ function summarizeProviderScenarioFlagCoverage(files) { ['recordingScope', 'recording app vs whole-screen scope', ['scope']], ['intervalMs', 'repeated press interval'], ['delayMs', 'typing/fill delay'], + ['recordAs', 'parameterized fill publication for recorded scripts'], ['durationMs', 'scroll, gesture, and TV remote duration'], ['holdMs', 'press hold duration'], ['jitterPx', 'press jitter'], diff --git a/src/cli/parser/__tests__/args-parse-interaction.test.ts b/src/cli/parser/__tests__/args-parse-interaction.test.ts index d1af133cd3..486bf8c69a 100644 --- a/src/cli/parser/__tests__/args-parse-interaction.test.ts +++ b/src/cli/parser/__tests__/args-parse-interaction.test.ts @@ -175,7 +175,7 @@ test('parseArgs recognizes gesture subcommand positionals', () => { assert.deepEqual(transform.positionals, ['transform', '200', '420', '80', '-40', '2', '35']); }); -test('parseArgs recognizes type and fill delay flags', () => { +test('parseArgs recognizes type delay and fill recording parameter flags', () => { const typeParsed = parseArgs(['type', 'hello', '--delay-ms', '75'], { strictFlags: true, }); @@ -183,12 +183,14 @@ test('parseArgs recognizes type and fill delay flags', () => { assert.deepEqual(typeParsed.positionals, ['hello']); assert.equal(typeParsed.flags.delayMs, 75); - const fillParsed = parseArgs(['fill', '@e5', 'search', '--delay-ms', '40'], { - strictFlags: true, - }); + const fillParsed = parseArgs( + ['fill', '@e5', 'search', '--delay-ms', '40', '--record-as', 'SEARCH_TERM'], + { strictFlags: true }, + ); assert.equal(fillParsed.command, 'fill'); assert.deepEqual(fillParsed.positionals, ['@e5', 'search']); assert.equal(fillParsed.flags.delayMs, 40); + assert.equal(fillParsed.flags.recordAs, 'SEARCH_TERM'); }); test('parseArgs recognizes record --fps flag', () => { diff --git a/src/cli/parser/__tests__/cli-help-command-usage.test.ts b/src/cli/parser/__tests__/cli-help-command-usage.test.ts index 208cede567..1d0c972261 100644 --- a/src/cli/parser/__tests__/cli-help-command-usage.test.ts +++ b/src/cli/parser/__tests__/cli-help-command-usage.test.ts @@ -236,6 +236,7 @@ test('command usage describes delayed typing flags', async () => { } assert.match(typeHelp, /--delay-ms /); assert.match(fillHelp, /--delay-ms /); + assert.match(fillHelp, /--record-as /); }); test('snapshot command usage documents diff alias', async () => { diff --git a/src/cli/parser/__tests__/cli-help-topics.test.ts b/src/cli/parser/__tests__/cli-help-topics.test.ts index b1334335b5..b32ee7bff7 100644 --- a/src/cli/parser/__tests__/cli-help-topics.test.ts +++ b/src/cli/parser/__tests__/cli-help-topics.test.ts @@ -224,6 +224,9 @@ test('usageForCommand resolves workflow help topic', async () => { assert.match(help, /snapshot -s @e7/); assert.match(help, /Use plain fill\/type first for ordinary login and form fields/); assert.match(help, /--delay-ms intentionally paces character entry/); + assert.match(help, /agent-device fill 'id="password"' "\$AD_VAR_PASSWORD" --record-as PASSWORD/); + assert.match(help, /published script contain only \$\{PASSWORD\}/); + assert.match(help, /Do not record passwords, tokens, or other secrets without --record-as/); assert.match(help, /Read-only visible\/state question: use snapshot\/get\/is\/find/); assert.match(help, /Use snapshot -i only when refs are needed/); assert.match(help, /install-from-source --github-actions-artifact org\/repo:app-debug/); diff --git a/src/cli/parser/cli-help.ts b/src/cli/parser/cli-help.ts index cb9c62b4e6..ef404be130 100644 --- a/src/cli/parser/cli-help.ts +++ b/src/cli/parser/cli-help.ts @@ -235,7 +235,11 @@ Reusable open-to-destination scripts: agent-device wait 'role="heading" label="Screen X"' agent-device session save-script session save-script [path] [--force] publishes the sole recorded open through the destination guard, omits close, and leaves the session active. The guard is a selector wait on a labeled or id-bearing landmark: its recorded identity is captured while armed, and replay verifies that identity after the wait's selector resolves, so a reshuffled screen with the same label elsewhere fails closed instead of false-passing. A duration wait, wait stable, wait @ref, or a selector wait on an unlabeled element is not a destination guard. A second successful open aborts publication; start a fresh session to author again. - Recorded fill/type inputs are written literally to the .ad file. Do not record passwords, tokens, or other secrets; use pre-authenticated test state or non-secret fixture credentials until parameterized input authoring is available. + Unparameterized fill/type inputs are literal .ad script content. For a sensitive fill, arm recording first, keep the live value in an environment variable, and name its replay placeholder explicitly: + export AD_VAR_PASSWORD='' + agent-device fill 'id="password"' "$AD_VAR_PASSWORD" --record-as PASSWORD + The live app receives the value, while recording state and the published script contain only \${PASSWORD}. Reuse the same name for repeated values and choose distinct names for distinct inputs. --record-as accepts uppercase replay variable names, is fill-only, requires an armed recording, and cannot be combined with --no-record. + Publish with session save-script, then replay with AD_VAR_PASSWORD still set or pass --env PASSWORD=. A missing value fails before that fill runs. Do not record passwords, tokens, or other secrets without --record-as; their literal text will be written to the .ad target. Snapshots and refs: snapshot reads visible state. snapshot -i gets current interactive refs only; it is the fast path when the next step is an interaction. diff --git a/src/client/client-types.ts b/src/client/client-types.ts index a126185eee..34e5208f27 100644 --- a/src/client/client-types.ts +++ b/src/client/client-types.ts @@ -783,6 +783,8 @@ export type FillOptions = DeviceCommandBaseOptions & SettleCommandOptions & { text: string; delayMs?: number; + /** Publish this fill value as `${VAR}` when script recording is armed. */ + recordAs?: string; verify?: boolean; }; @@ -1102,6 +1104,8 @@ export type InternalRequestOptions = AgentDeviceClientConfig & deviceHub?: boolean; testIme?: boolean; noRecord?: boolean; + /** Fill-only script parameter name used to publish `${VAR}` instead of literal text. */ + recordAs?: string; /** #1271 stage 2: force-record this action; mutually exclusive with `noRecord`. */ record?: boolean; backMode?: BackMode; diff --git a/src/commands/cli-grammar/flag-definitions-action.ts b/src/commands/cli-grammar/flag-definitions-action.ts index b255c3d7c0..45bb82d94d 100644 --- a/src/commands/cli-grammar/flag-definitions-action.ts +++ b/src/commands/cli-grammar/flag-definitions-action.ts @@ -72,6 +72,13 @@ export const ACTION_FLAG_DEFINITIONS: readonly FlagDefinition[] = [ usageLabel: '--delay-ms ', usageDescription: 'Delay between typed characters', }, + { + key: 'recordAs', + names: ['--record-as'], + type: 'string', + usageLabel: '--record-as ', + usageDescription: 'Fill: send the live text but publish ${VAR} in an armed .ad recording', + }, { key: 'durationMs', names: ['--duration-ms'], diff --git a/src/commands/command-flags.ts b/src/commands/command-flags.ts index 548e3df9f3..05db325a57 100644 --- a/src/commands/command-flags.ts +++ b/src/commands/command-flags.ts @@ -52,6 +52,7 @@ function buildFlags(options: InternalRequestOptions): CommandFlags { deviceHub: options.deviceHub, testIme: options.testIme, noRecord: options.noRecord, + recordAs: options.recordAs, record: options.record, backMode: options.backMode, metroHost: options.metroHost, diff --git a/src/commands/interaction/index.ts b/src/commands/interaction/index.ts index 631148f0dd..ae3cd69df3 100644 --- a/src/commands/interaction/index.ts +++ b/src/commands/interaction/index.ts @@ -133,7 +133,12 @@ const interactionCliSchemas = { usageOverride: 'fill | fill <@ref|selector> ', positionalArgs: ['targetOrX', 'yOrText', 'text?'], allowsExtraPositionals: true, - allowedFlags: [...SELECTOR_SNAPSHOT_FLAGS, 'delayMs', ...postActionObservationCliFlags('fill')], + allowedFlags: [ + ...SELECTOR_SNAPSHOT_FLAGS, + 'delayMs', + 'recordAs', + ...postActionObservationCliFlags('fill'), + ], }, scroll: { usageOverride: 'scroll [amount] [--pixels ] [--duration-ms ]', @@ -389,6 +394,7 @@ function toFillOptions(input: FillInput): FillOptions { ...toSelectorSnapshotOptions(input), text: input.text, delayMs: input.delayMs, + recordAs: input.recordAs, verify: input.verify, ...toSettleOptions(input), }; diff --git a/src/commands/interaction/interactions.test.ts b/src/commands/interaction/interactions.test.ts index 04ee93c91b..11a498b010 100644 --- a/src/commands/interaction/interactions.test.ts +++ b/src/commands/interaction/interactions.test.ts @@ -38,3 +38,14 @@ test('scroll reader rejects a @ref in the direction slot with a grammar hint (#1 expect(appError.details?.hint).toMatch(/no @ref or selector/i); } }); + +test('fill projects recordAs through the typed daemon flags', () => { + const request = interactionDaemonWriters.fill({ + selector: 'id="password"', + text: 'live-secret', + recordAs: 'PASSWORD', + }); + + expect(request.positionals).toEqual(['id="password"', 'live-secret']); + expect(request.options.recordAs).toBe('PASSWORD'); +}); diff --git a/src/commands/interaction/interactions.ts b/src/commands/interaction/interactions.ts index 6a9af3c956..584c40912d 100644 --- a/src/commands/interaction/interactions.ts +++ b/src/commands/interaction/interactions.ts @@ -88,6 +88,7 @@ export const interactionCliReaders = { target: targetInputFromClientTarget(decoded.target), text: decoded.text, delayMs: flags.delayMs, + recordAs: flags.recordAs, verify: flags.verify, }; }, diff --git a/src/commands/interaction/metadata.ts b/src/commands/interaction/metadata.ts index b5daebd559..437c35007f 100644 --- a/src/commands/interaction/metadata.ts +++ b/src/commands/interaction/metadata.ts @@ -125,6 +125,9 @@ const fillFields = { target: requiredField(interactionTargetField()), text: requiredField(stringField('Text to enter into the target.')), delayMs: integerField('Delay between typed characters.', { min: 0 }), + 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.', + ), ...selectorSnapshotFields(), ...postActionObservationFields('fill'), }; diff --git a/src/contracts/cli-flags.ts b/src/contracts/cli-flags.ts index 02e798d5c2..8350d32188 100644 --- a/src/contracts/cli-flags.ts +++ b/src/contracts/cli-flags.ts @@ -90,6 +90,8 @@ export type CliFlags = CloudProviderProfileFields & recordingScope?: RecordingScope; intervalMs?: number; delayMs?: number; + /** Fill: publish the live text as a late-bound ${VAR} in a recorded .ad script. */ + recordAs?: string; durationMs?: number; holdMs?: number; jitterPx?: number; diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 85b8d53b8a..92849d8ad9 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -86,3 +86,50 @@ test('--no-record still takes precedence over an observation-only action, repair expect(action).toBeUndefined(); expect(session.actions).toHaveLength(0); }); + +test('parameterized fills keep literals out of recording state with deterministic placeholders', () => { + const session = makeIosSession('default'); + const password = 'literal-password-1348'; + const token = 'literal-token-1348'; + const passwordEntry = { + command: 'fill', + positionals: ['id="password"', password], + flags: { recordAs: 'PASSWORD' }, + result: { + text: password, + message: `Filled ${password}`, + selectorChain: ['id="password"', `value="${password}" editable=true`], + settle: { diff: `value=${password}` }, + }, + targetEvidence: { + role: 'textinput', + label: `Current value ${password}`, + ancestry: [{ role: 'form', label: `Credential ${password}` }], + sibling: 0, + viewportOrder: 0, + verification: 'verified' as const, + }, + }; + + recordActionEntry(session, passwordEntry); + recordActionEntry(session, passwordEntry); + recordActionEntry(session, { + command: 'fill', + positionals: ['id="token"', token], + flags: { recordAs: 'API_TOKEN' }, + result: { text: token, message: `Filled ${token}` }, + }); + + expect(session.actions.map((action) => action.positionals.at(-1))).toEqual([ + '${PASSWORD}', + '${PASSWORD}', + '${API_TOKEN}', + ]); + const serialized = JSON.stringify(session.actions); + expect(serialized).not.toContain(password); + expect(serialized).not.toContain(token); + expect(serialized).toContain('value=${PASSWORD}'); + expect(session.actions[0]?.targetEvidence?.label).toBe('Current value ${PASSWORD}'); + expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credential ${PASSWORD}'); + expect(session.actions[0]?.result?.selectorChain).toEqual(['id="password"']); +}); diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 65bd8682bb..71eb3dd451 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/src/daemon/__tests__/session-store.test.ts @@ -147,6 +147,31 @@ test('saveScript flag enables .ad session log writing', () => { assert.equal(listSessionScriptFiles(root).length, 1); }); +test('parameterized fill publication writes only the placeholder to target and temp content', () => { + const fixture = makeFixture('agent-device-session-log-parameterized-fill-'); + const secret = 'publication-only-live-value-1348'; + recordOpen(fixture.store, fixture.session); + fixture.store.recordAction(fixture.session, { + command: 'fill', + positionals: ['id="password"', secret], + flags: { platform: 'ios', recordAs: 'PASSWORD' }, + result: { + text: secret, + message: `Filled ${secret}`, + selectorChain: ['id="password"'], + }, + }); + recordClose(fixture.store, fixture.session); + + const script = writeScript(fixture); + assert.equal(script.includes(secret), false); + assert.match(script, /fill "id=\\"password\\"" "\$\{PASSWORD\}"/); + assert.equal( + fs.readdirSync(fixture.root).some((entry) => entry.endsWith('.tmp')), + false, + ); +}); + test('recordAction writes a paged session event log', async () => { const { store, session } = makeFixture('agent-device-session-events-'); recordOpen(store, session, { platform: 'ios' }); diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index 96968d836b..1a422b9aca 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -14,6 +14,11 @@ import { } from '../interaction-outcome-policy.ts'; import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; import { computeTargetEvidence, type RecordedTargetCapture } from '../session-target-evidence.ts'; +import { inferFillText } from '../action-utils.ts'; +import { + recordedInputPlaceholder, + replaceRecordedInputLiteral, +} from '../../replay/recorded-input.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -61,13 +66,20 @@ export function finalizeTouchInteraction(params: { androidFreshnessBaseline, } = params; const actionFlags = stripInternalInteractionFlags(flags); + const [parameterizedResult, parameterizedResponseData] = parameterizeFillPayloads({ + command, + positionals, + flags: actionFlags, + result, + responseData, + }); const targetEvidence = session.recordSession && recordedTarget ? computeTargetEvidence(recordedTarget) : undefined; sessionStore.recordAction(session, { command, positionals, flags: actionFlags ?? {}, - result, + result: parameterizedResult, ...(targetEvidence ? { targetEvidence } : {}), }); markPendingInteractionOutcome({ @@ -89,10 +101,35 @@ export function finalizeTouchInteraction(params: { session, actionCommand, positionals, - result, + parameterizedResult, (actionFlags ?? {}) as Record, actionStartedAt, actionFinishedAt, ); - return { ok: true, data: responseData }; + return { ok: true, data: parameterizedResponseData }; +} + +function parameterizeFillPayloads(params: { + command: string; + positionals: string[]; + flags: CommandFlags | undefined; + result: Record; + responseData: Record; +}): readonly [result: Record, responseData: Record] { + if (params.command !== 'fill' || typeof params.flags?.recordAs !== 'string') { + return [params.result, params.responseData]; + } + const literal = inferFillText({ + ts: 0, + command: 'fill', + positionals: params.positionals, + flags: params.flags, + result: params.result, + }); + if (!literal) return [params.result, params.responseData]; + const placeholder = recordedInputPlaceholder(params.flags.recordAs); + return [ + replaceRecordedInputLiteral(params.result, literal, placeholder), + replaceRecordedInputLiteral(params.responseData, literal, placeholder), + ]; } diff --git a/src/daemon/handlers/interaction-recorded-input.ts b/src/daemon/handlers/interaction-recorded-input.ts new file mode 100644 index 0000000000..b7e0c76e8b --- /dev/null +++ b/src/daemon/handlers/interaction-recorded-input.ts @@ -0,0 +1,26 @@ +import type { CommandFlags } from '../../core/dispatch.ts'; +import { AppError } from '../../kernel/errors.ts'; +import { validateRecordedInputVariableName } from '../../replay/recorded-input.ts'; +import type { SessionState } from '../types.ts'; + +export function assertRecordedFillParameterization(params: { + session: SessionState; + flags: CommandFlags | undefined; + replayPlanStep: boolean; +}): void { + const recordAs = params.flags?.recordAs; + if (recordAs === undefined) return; + validateRecordedInputVariableName(recordAs); + if (params.flags?.noRecord) { + throw new AppError( + 'INVALID_ARGS', + 'fill --record-as cannot be combined with --no-record because no script step would be published.', + ); + } + if (!params.session.recordSession && !params.replayPlanStep) { + throw new AppError( + 'INVALID_ARGS', + 'fill --record-as requires an armed script recording. Start a fresh session with open --save-script, then retry the fill.', + ); + } +} diff --git a/src/daemon/handlers/interaction-touch.ts b/src/daemon/handlers/interaction-touch.ts index e1bde84e48..bbda1e4a6d 100644 --- a/src/daemon/handlers/interaction-touch.ts +++ b/src/daemon/handlers/interaction-touch.ts @@ -66,6 +66,7 @@ import { } from '../android-system-dialog.ts'; import { refMutationAdmissionResponse } from './interaction-ref-policy.ts'; import { expireRefFrame } from '../ref-frame.ts'; +import { assertRecordedFillParameterization } from './interaction-recorded-input.ts'; export async function handleTouchInteractionCommands( params: InteractionHandlerParams & { @@ -576,6 +577,11 @@ async function dispatchFillViaRuntime( if (unsupported) return unsupported; } if (!session) return noActiveSessionError(); + assertRecordedFillParameterization({ + session, + flags: req.flags, + replayPlanStep: req.internal?.replayPlanStep === true, + }); const invalidSettleFlags = settleFlagGuardResponse('fill', req.flags); if (invalidSettleFlags) return invalidSettleFlags; diff --git a/src/daemon/handlers/session-replay-action-runtime.ts b/src/daemon/handlers/session-replay-action-runtime.ts index afb1734e67..badb9dca2c 100644 --- a/src/daemon/handlers/session-replay-action-runtime.ts +++ b/src/daemon/handlers/session-replay-action-runtime.ts @@ -9,6 +9,8 @@ import { } from '../../contracts/gesture-normalization.ts'; import { buildDisplayPositionals } from '../session-event-action.ts'; import { appendReplayTraceEvent } from './session-replay-trace.ts'; +import { inferFillText } from '../action-utils.ts'; +import { readRecordedInputVariableName } from '../../replay/recorded-input.ts'; type ReplayBaseRequest = Omit; @@ -50,6 +52,7 @@ export async function invokeReplayAction(params: { req, sessionName, resolved, + sourceAction: action, scope, line, step, @@ -107,13 +110,19 @@ async function invokeResolvedReplayAction(params: { req: DaemonRequest; sessionName: string; resolved: SessionAction; + sourceAction: SessionAction; scope: ReplayVarScope; line: number; step: number; invoke: DaemonInvokeFn; }): Promise { - const { req, sessionName, resolved, invoke } = params; + const { req, sessionName, resolved, sourceAction, invoke } = params; const flags = buildReplayActionFlags(req.flags, resolved.flags); + const recordedInputVariable = + sourceAction.command === 'fill' + ? readRecordedInputVariableName(inferFillText(sourceAction)) + : undefined; + if (recordedInputVariable) flags.recordAs = recordedInputVariable; const baseReq: ReplayBaseRequest = { token: req.token, session: sessionName, diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 978059f726..061b738804 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -27,6 +27,7 @@ import { emitDiagnostic, flushDiagnosticsToSessionFile, getDiagnosticsMeta, + registerDiagnosticSensitiveValue, withDiagnosticsScope, } from '../utils/diagnostics.ts'; import type { LeaseRegistry } from './lease-registry.ts'; @@ -45,6 +46,7 @@ import { buildRequestFinishedEvent, shouldRecordEventForRequest } from './sessio import { canRunReplayScopedAction } from './daemon-command-registry.ts'; import { createAgentBrowserWebProvider } from '../platforms/web/agent-browser-provider.ts'; import { openWebSessionNames } from './web-session-names.ts'; +import { inferFillText } from './action-utils.ts'; // --------------------------------------------------------------------------- // Request handler API @@ -135,9 +137,9 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { if (!timingSafeStringEqual(req.token, token)) { return unauthorizedResponse(); } - if (req.flags?.record && req.flags?.noRecord) { - return mutuallyExclusiveRecordFlagsResponse(); - } + registerParameterizedFillDiagnosticValue(req); + const invalidRecordingFlags = recordingFlagsResponse(req); + if (invalidRecordingFlags) return invalidRecordingFlags; let scope: RequestExecutionScope | undefined; try { @@ -301,6 +303,26 @@ function mutuallyExclusiveRecordFlagsResponse(): DaemonResponse { ); } +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; +} + +function registerParameterizedFillDiagnosticValue(req: DaemonRequest): void { + if (req.command !== 'fill' || typeof req.flags?.recordAs !== 'string') return; + registerDiagnosticSensitiveValue( + inferFillText({ + ts: 0, + command: 'fill', + positionals: req.positionals ?? [], + flags: req.flags, + }), + ); +} + async function dispatchGenericForLockedScope(params: { lockedScope: LockedRequestScope; logPath: string; diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 45cf7f8c61..cb752eae2f 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -4,6 +4,12 @@ import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { DaemonRequest, SessionAction, SessionRuntimeHints, SessionState } from './types.ts'; import { expandSessionPath } from './session-paths.ts'; import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; +import { inferFillText } from './action-utils.ts'; +import { + recordedInputPlaceholder, + replaceRecordedInputLiteral, + validateRecordedInputVariableName, +} from '../replay/recorded-input.ts'; export type RecordActionEntry = { command: string; @@ -84,14 +90,15 @@ export function recordActionEntry( session.saveScriptForce = true; } } + const recordedEntry = parameterizeRecordedFill(entry); const action: SessionAction = { ts: Date.now(), - command: entry.command, - positionals: entry.positionals, - runtime: entry.runtime, - flags: sanitizeFlags(entry.flags), - result: entry.result, - ...(entry.targetEvidence ? { targetEvidence: entry.targetEvidence } : {}), + command: recordedEntry.command, + positionals: recordedEntry.positionals, + runtime: recordedEntry.runtime, + flags: sanitizeFlags(recordedEntry.flags), + result: recordedEntry.result, + ...(recordedEntry.targetEvidence ? { targetEvidence: recordedEntry.targetEvidence } : {}), }; session.actions.push(action); emitDiagnostic({ @@ -105,6 +112,50 @@ export function recordActionEntry( return action; } +/** + * #1348: the recorder is the first durable boundary. The live request keeps + * the literal fill value through device execution, but the SessionAction gets + * only `${VAR}`. Value-bearing result strings are scrubbed at the same boundary + * so backend echoes or settled diffs cannot smuggle the literal into recording + * state, the event display, or the eventual publication temp/target file; + * ADR 0012 identity evidence keeps its structure but is scrubbed too, covering + * repeated fills where the pre-action accessibility label already echoes the + * same sensitive value. + */ +function parameterizeRecordedFill(entry: RecordActionEntry): RecordActionEntry { + if (entry.command !== 'fill' || typeof entry.flags.recordAs !== 'string') return entry; + const variableName = validateRecordedInputVariableName(entry.flags.recordAs); + const placeholder = recordedInputPlaceholder(variableName); + const literal = inferFillText({ + ts: 0, + command: entry.command, + positionals: entry.positionals, + flags: entry.flags, + }); + if (!literal) return entry; + return { + ...entry, + positionals: replaceFillText(entry.positionals, placeholder), + result: replaceRecordedInputLiteral(entry.result, literal, placeholder), + targetEvidence: replaceRecordedInputLiteral(entry.targetEvidence, literal, placeholder), + }; +} + +function replaceFillText(positionals: string[], placeholder: string): string[] { + const first = positionals[0]; + if (first?.startsWith('@')) { + return positionals.length >= 3 ? [first, positionals[1]!, placeholder] : [first, placeholder]; + } + if ( + positionals.length >= 3 && + Number.isFinite(Number(positionals[0])) && + Number.isFinite(Number(positionals[1])) + ) { + return [positionals[0]!, positionals[1]!, placeholder]; + } + return first === undefined ? [placeholder] : [first, placeholder]; +} + /** * #1271 stage 2 (ADR 0012 amendment): observation-only commands — the ONLY * commands the repair-segment exclusion can drop. `wait` is deliberately diff --git a/src/mcp/__tests__/router.test.ts b/src/mcp/__tests__/router.test.ts index 8478fbf033..6e939fcfd6 100644 --- a/src/mcp/__tests__/router.test.ts +++ b/src/mcp/__tests__/router.test.ts @@ -28,6 +28,7 @@ test('MCP exposes every automatable CLI command as a structured direct tool', as .properties; assert.ok(!('positionals' in fillProperties)); assert.ok('target' in fillProperties); + assert.ok('recordAs' in fillProperties); const batchTool = (response.result as { tools: Array> }).tools.find( (tool) => tool.name === 'batch', diff --git a/src/replay/recorded-input.ts b/src/replay/recorded-input.ts new file mode 100644 index 0000000000..10ca40a38a --- /dev/null +++ b/src/replay/recorded-input.ts @@ -0,0 +1,56 @@ +import { AppError } from '../kernel/errors.ts'; +import { REPLAY_VAR_KEY_RE } from './vars.ts'; + +const RECORDED_INPUT_PLACEHOLDER_RE = /^\$\{([A-Z_][A-Z0-9_]*)\}$/; + +export function validateRecordedInputVariableName(raw: string): string { + if (raw !== raw.trim() || !REPLAY_VAR_KEY_RE.test(raw)) { + throw new AppError( + 'INVALID_ARGS', + `Invalid --record-as variable "${raw}": use uppercase letters, digits, and underscores (for example PASSWORD).`, + ); + } + if (raw.startsWith('AD_')) { + throw new AppError( + 'INVALID_ARGS', + `Invalid --record-as variable "${raw}": the AD_* namespace is reserved for built-in replay variables.`, + ); + } + return raw; +} + +export function recordedInputPlaceholder(variableName: string): string { + return `\${${variableName}}`; +} + +/** Exact placeholders produced by safe authoring; embedded interpolation stays ordinary script input. */ +export function readRecordedInputVariableName(value: string): string | undefined { + const match = RECORDED_INPUT_PLACEHOLDER_RE.exec(value); + if (!match?.[1] || match[1].startsWith('AD_')) return undefined; + return match[1]; +} + +export function replaceRecordedInputLiteral(value: T, literal: string, placeholder: string): T { + if (typeof value === 'string') return value.replaceAll(literal, placeholder) as T; + if (Array.isArray(value)) { + return value.map((entry) => replaceRecordedInputLiteral(entry, literal, placeholder)) as T; + } + if (!value || typeof value !== 'object') return value; + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => { + if (key === 'ref') return [key, entry]; + if (key === 'selector' && typeof entry === 'string') { + return [key, entry.includes(literal) ? undefined : entry]; + } + if (key === 'selectorChain' && Array.isArray(entry)) { + return [ + key, + entry.filter( + (candidate) => typeof candidate !== 'string' || !candidate.includes(literal), + ), + ]; + } + return [key, replaceRecordedInputLiteral(entry, literal, placeholder)]; + }), + ) as T; +} diff --git a/src/utils/__tests__/diagnostics.test.ts b/src/utils/__tests__/diagnostics.test.ts index 0a5e27e1cd..256fa8a606 100644 --- a/src/utils/__tests__/diagnostics.test.ts +++ b/src/utils/__tests__/diagnostics.test.ts @@ -6,6 +6,7 @@ import path from 'node:path'; import { emitDiagnostic, flushDiagnosticsToSessionFile, + registerDiagnosticSensitiveValue, withDiagnosticsScope, } from '../diagnostics.ts'; @@ -67,3 +68,24 @@ test('diagnostics redacts sensitive fields', async () => { process.env.HOME = previousHome; } }); + +test('diagnostics scrubs caller-declared recorded input literals regardless of field name', async () => { + const outputPath = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-diag-recorded-input-')), + 'request.ndjson', + ); + const secret = 'opaque-value-without-sensitive-keywords'; + + await withDiagnosticsScope({ command: 'fill', logPath: outputPath }, async () => { + registerDiagnosticSensitiveValue(secret); + emitDiagnostic({ + phase: 'platform_failure', + data: { text: secret, message: `Backend echoed ${secret}` }, + }); + flushDiagnosticsToSessionFile({ force: true }); + }); + + const diagnostics = fs.readFileSync(outputPath, 'utf8'); + assert.equal(diagnostics.includes(secret), false); + assert.match(diagnostics, /Backend echoed \[REDACTED\]/); +}); diff --git a/src/utils/diagnostics.ts b/src/utils/diagnostics.ts index 7428b3ca7f..131790355f 100644 --- a/src/utils/diagnostics.ts +++ b/src/utils/diagnostics.ts @@ -37,6 +37,7 @@ type DiagnosticsScope = DiagnosticsScopeOptions & { // can count phase occurrences for the whole request even in debug mode where // events are streamed out and reset mid-flight. phaseCounts: Map; + sensitiveValues: Set; }; const diagnosticsStorage = new AsyncLocalStorage(); @@ -59,6 +60,7 @@ export async function withDiagnosticsScope( events: [], liveWrittenEventCount: 0, phaseCounts: new Map(), + sensitiveValues: new Set(), }; return await diagnosticsStorage.run(scope, fn); } @@ -89,6 +91,12 @@ export function getDiagnosticsMeta(): { }; } +/** Register a caller-declared literal that must not reach this request's diagnostics. */ +export function registerDiagnosticSensitiveValue(value: string): void { + if (!value) return; + diagnosticsStorage.getStore()?.sensitiveValues.add(value); +} + /** * 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 @@ -121,7 +129,7 @@ export function emitDiagnostic(event: { requestId: scope.requestId, command: scope.command, durationMs: event.durationMs, - data: event.data ? redactDiagnosticData(event.data) : undefined, + data: event.data ? redactScopeData(scope, event.data) : undefined, }; scope.events.push(payload); scope.phaseCounts.set(event.phase, (scope.phaseCounts.get(event.phase) ?? 0) + 1); @@ -182,7 +190,7 @@ export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}) if (scope.logPath) { const pendingEvents = scope.events.slice(scope.liveWrittenEventCount); if (pendingEvents.length > 0) { - const lines = pendingEvents.map((entry) => JSON.stringify(redactDiagnosticData(entry))); + const lines = pendingEvents.map((entry) => JSON.stringify(redactScopeData(scope, entry))); appendDiagnosticLine(scope.logPath, `${lines.join('\n')}\n`); } const logPath = scope.logPath; @@ -197,7 +205,7 @@ export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}) fs.mkdirSync(baseDir, { recursive: true }); const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); const filePath = path.join(baseDir, `${timestamp}-${scope.diagnosticId}.ndjson`); - const lines = scope.events.map((entry) => JSON.stringify(redactDiagnosticData(entry))); + const lines = scope.events.map((entry) => JSON.stringify(redactScopeData(scope, entry))); fs.writeFileSync(filePath, `${lines.join('\n')}\n`); scope.events = []; return filePath; @@ -206,6 +214,31 @@ export function flushDiagnosticsToSessionFile(options: { force?: boolean } = {}) } } +function redactScopeData(scope: DiagnosticsScope, input: T): T { + const redacted = redactDiagnosticData(input); + const values = [...scope.sensitiveValues].sort((left, right) => right.length - left.length); + return replaceSensitiveValues(redacted, values) as T; +} + +function replaceSensitiveValues(value: unknown, sensitiveValues: readonly string[]): unknown { + if (typeof value === 'string') { + return sensitiveValues.reduce( + (output, sensitive) => output.replaceAll(sensitive, '[REDACTED]'), + value, + ); + } + if (Array.isArray(value)) { + return value.map((entry) => replaceSensitiveValues(entry, sensitiveValues)); + } + if (!value || typeof value !== 'object') return value; + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [ + key, + replaceSensitiveValues(entry, sensitiveValues), + ]), + ); +} + function sanitizePathPart(value: string): string { return value.replace(/[^a-zA-Z0-9._-]/g, '_'); } diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 6466ccd04c..4f601513fa 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -157,3 +157,95 @@ test('a second successful open aborts publication and terminal save flags fail b } }); }, 20_000); + +test('parameterized fill publishes only ${VAR} and replay resolves it immediately before fill', async () => { + await withProviderScenarioResource( + async () => await createAndroidSettingsWorld({ nativeTextInjection: true }), + async (world) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-parameterized-script-')); + const scriptPath = path.join(root, 'parameterized-search.ad'); + const secret = 'OpaqueProviderValue1348'; + const client = world.daemon.client(); + try { + await client.apps.open({ app: 'settings', saveScript: scriptPath, ...world.selection }); + const snapshot = await client.capture.snapshot({ + interactiveOnly: true, + ...world.selection, + }); + const search = snapshot.nodes.find((node) => node.label === 'Search'); + assert.ok(search?.ref); + + const invalidName = await world.daemon.callCommand('fill', [`@${search.ref}`, secret], { + ...world.selection, + recordAs: 'password', + }); + assertRpcError(invalidName, 'INVALID_ARGS', /Invalid --record-as variable/); + assert.equal(world.textInjectionCalls.length, 0); + + const contradictory = await world.daemon.callCommand('fill', [`@${search.ref}`, secret], { + ...world.selection, + recordAs: 'SEARCH_TERM', + noRecord: true, + }); + assertRpcError(contradictory, 'INVALID_ARGS', /cannot be combined with --no-record/); + assert.equal(world.textInjectionCalls.length, 0); + + const fill = await client.interactions.fill({ + ref: `@${search.ref}`, + text: secret, + recordAs: 'SEARCH_TERM', + ...world.selection, + }); + assert.equal(fill.text, '${SEARCH_TERM}'); + await client.command.wait({ + selector: 'id=com.android.settings:id/search', + ...world.selection, + }); + + const recordedState = JSON.stringify(world.daemon.session()?.actions); + assert.equal(recordedState.includes(secret), false); + assert.match(recordedState, /\$\{SEARCH_TERM\}/); + await client.sessions.saveScript({ path: scriptPath }); + const script = fs.readFileSync(scriptPath, 'utf8'); + assert.equal(script.includes(secret), false); + assert.match(script, /\$\{SEARCH_TERM\}/); + assert.equal(world.textInjectionCalls.at(-1)?.text, secret); + await client.sessions.close(); + + const callsBeforeMissingValue = world.textInjectionCalls.length; + const missingValue = await world.daemon.callCommand( + 'replay', + [scriptPath], + world.selection, + ); + assertRpcError(missingValue, 'INVALID_ARGS', /Unresolved variable \$\{SEARCH_TERM\}/); + assert.equal(world.textInjectionCalls.length, callsBeforeMissingValue); + await client.sessions.close(); + + const replay = await world.daemon.callCommand('replay', [scriptPath], { + ...world.selection, + replayEnv: [`SEARCH_TERM=${secret}`], + }); + assertRpcOk(replay); + assert.equal(world.textInjectionCalls.at(-1)?.text, secret); + const replayState = JSON.stringify(world.daemon.session()?.actions); + assert.equal(replayState.includes(secret), false); + assert.match(replayState, /\$\{SEARCH_TERM\}/); + await client.sessions.close(); + + await client.apps.open({ app: 'settings', ...world.selection }); + const callsBeforeUnarmed = world.textInjectionCalls.length; + const unarmed = await world.daemon.callCommand( + 'fill', + ['id=com.android.settings:id/search', secret], + { ...world.selection, recordAs: 'SEARCH_TERM' }, + ); + assertRpcError(unarmed, 'INVALID_ARGS', /requires an armed script recording/); + assert.equal(world.textInjectionCalls.length, callsBeforeUnarmed); + await client.sessions.close(); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, + ); +}, 20_000); From e234f538d1e8afccba05e0322eb61176b9f7aa59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 22 Jul 2026 23:22:58 +0200 Subject: [PATCH 2/7] fix: harden parameterized replay recording --- .../adr/0017-parameterized-recorded-inputs.md | 22 +++--- .../__tests__/session-action-recorder.test.ts | 70 ++++++++++++++++-- src/daemon/action-utils.ts | 12 ++- .../session-replay-action-runtime.test.ts | 48 ++++++++++++ src/daemon/handlers/interaction-common.ts | 11 +-- src/daemon/parameterized-recorded-fill.ts | 74 +++++++++++++++++++ src/daemon/request-router.ts | 1 + src/daemon/session-action-recorder.ts | 25 ++++--- src/replay/recorded-input.ts | 25 ------- .../active-session-script-publication.test.ts | 46 ++++++++++-- .../provider-scenarios/android-world.ts | 2 + 11 files changed, 261 insertions(+), 75 deletions(-) create mode 100644 src/daemon/handlers/__tests__/session-replay-action-runtime.test.ts create mode 100644 src/daemon/parameterized-recorded-fill.ts diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md index 9ece01064f..a80928a6c3 100644 --- a/docs/adr/0017-parameterized-recorded-inputs.md +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -52,14 +52,15 @@ The literal exists only on the live request path long enough to execute the inte 1. The request scope registers it as an explicit diagnostics secret before platform work. This is in addition to key-name redaction, because an opaque value need not look secret. 2. The platform interaction receives the literal. -3. Before response/event/recording retention, value-bearing interaction-result fields replace every - occurrence of the literal with `${VAR}`. Stable selector/ref provenance is preserved byte-for-byte; - post-fill selector-chain candidates derived from the new value are dropped rather than serialized. +3. Before response/event/recording retention, the fill result's semantic `text` field and an exact + post-fill `refLabel` become `${VAR}`. Stable selector/ref provenance and unrelated strings are + preserved byte-for-byte; post-fill selector-chain candidates are dropped only when a parsed + `text`, `label`, or `value` term exactly equals the supplied literal. 4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the - placeholder into `SessionAction`, and scrubs value-bearing result strings again. ADR 0012 - `target-v1` evidence keeps its structure, but any matching string is parameterized as well so a - repeated fill cannot leak through a pre-action accessibility label. The `recordAs` control flag - itself is not serialized into the script. + placeholder into `SessionAction`, and parameterizes the semantic result field again. ADR 0012 + `target-v1` evidence keeps its structure; exact value-bearing accessibility labels become the + placeholder without rewriting identity fragments that merely contain the same characters. The + `recordAs` control flag itself is not serialized into the script. 5. The existing ADR 0012/0016 writer receives an already-parameterized action. Its selector provenance, portability checks, same-directory temp write, and atomic publication algorithm are unchanged. @@ -71,9 +72,10 @@ the literal once this pre-publication boundary has run. Missing variables retain replay's existing fail-loud behavior: `${VAR}` resolution throws before the corresponding action is invoked. When replay dispatches an authored fill whose complete text token is a `${VAR}` placeholder, its daemon-only provenance channel derives `recordAs=VAR` for that live request. -If the replay is itself being recorded or repaired, the recorder therefore retains the original -placeholder rather than serializing the expanded value. Embedded interpolation remains ordinary script -input; safe authoring emits one complete placeholder token. +This derivation uses the unresolved source action, independently of whether the resolved value is empty +or whitespace. If the replay is itself being recorded or repaired, the recorder therefore retains the +original placeholder rather than serializing the expanded value. Embedded interpolation remains +ordinary script input; safe authoring emits one complete placeholder token. ## Consequences diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 92849d8ad9..f354efe6dc 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -97,14 +97,14 @@ test('parameterized fills keep literals out of recording state with deterministi flags: { recordAs: 'PASSWORD' }, result: { text: password, - message: `Filled ${password}`, + message: 'Filled 21 chars', + selector: 'id="password"', selectorChain: ['id="password"', `value="${password}" editable=true`], - settle: { diff: `value=${password}` }, }, targetEvidence: { role: 'textinput', - label: `Current value ${password}`, - ancestry: [{ role: 'form', label: `Credential ${password}` }], + label: password, + ancestry: [{ role: 'form', label: 'Credentials' }], sibling: 0, viewportOrder: 0, verification: 'verified' as const, @@ -117,7 +117,7 @@ test('parameterized fills keep literals out of recording state with deterministi command: 'fill', positionals: ['id="token"', token], flags: { recordAs: 'API_TOKEN' }, - result: { text: token, message: `Filled ${token}` }, + result: { text: token, message: 'Filled 18 chars' }, }); expect(session.actions.map((action) => action.positionals.at(-1))).toEqual([ @@ -128,8 +128,62 @@ test('parameterized fills keep literals out of recording state with deterministi const serialized = JSON.stringify(session.actions); expect(serialized).not.toContain(password); expect(serialized).not.toContain(token); - expect(serialized).toContain('value=${PASSWORD}'); - expect(session.actions[0]?.targetEvidence?.label).toBe('Current value ${PASSWORD}'); - expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credential ${PASSWORD}'); + expect(session.actions[0]?.result?.text).toBe('${PASSWORD}'); + expect(session.actions[0]?.result?.selector).toBe('id="password"'); + expect(session.actions[0]?.targetEvidence?.label).toBe('${PASSWORD}'); + expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credentials'); expect(session.actions[0]?.result?.selectorChain).toEqual(['id="password"']); }); + +test('a one-character parameterized fill preserves unrelated response and selector text', () => { + const session = makeIosSession('default'); + recordActionEntry(session, { + command: 'fill', + positionals: ['id="password"', 'a'], + flags: { recordAs: 'LETTER' }, + result: { + text: 'a', + message: 'Already safe', + selector: 'id="password"', + selectorChain: ['id="password"', 'label="Account"', 'value="a" editable=true'], + }, + targetEvidence: { + role: 'textinput', + label: 'Password', + ancestry: [{ role: 'form', label: 'Credentials area' }], + sibling: 0, + viewportOrder: 0, + verification: 'verified', + }, + }); + + expect(session.actions[0]).toMatchObject({ + positionals: ['id="password"', '${LETTER}'], + result: { + text: '${LETTER}', + message: 'Already safe', + selector: 'id="password"', + selectorChain: ['id="password"', 'label="Account"'], + }, + targetEvidence: { + label: 'Password', + ancestry: [{ role: 'form', label: 'Credentials area' }], + }, + }); +}); + +test.each(['', ' '])( + 'parameterized fills preserve the placeholder when the resolved value is %j', + (value) => { + const session = makeIosSession('default'); + recordActionEntry(session, { + command: 'fill', + positionals: ['id="password"', value], + flags: { recordAs: 'PASSWORD' }, + result: { text: value }, + }); + + expect(session.actions[0]?.positionals).toEqual(['id="password"', '${PASSWORD}']); + expect(session.actions[0]?.result?.text).toBe('${PASSWORD}'); + }, +); diff --git a/src/daemon/action-utils.ts b/src/daemon/action-utils.ts index f34e8f17b8..0a50c2489c 100644 --- a/src/daemon/action-utils.ts +++ b/src/daemon/action-utils.ts @@ -2,22 +2,20 @@ import type { SessionAction } from './types.ts'; export function inferFillText(action: SessionAction): string { const resultText = action.result?.text; - if (typeof resultText === 'string' && resultText.trim().length > 0) { - return resultText; - } + if (typeof resultText === 'string') return resultText; const positionals = action.positionals ?? []; if (positionals.length === 0) return ''; const first = positionals[0]; if (first?.startsWith('@')) { - if (positionals.length >= 3) return positionals.slice(2).join(' ').trim(); - return positionals.slice(1).join(' ').trim(); + if (positionals.length >= 3) return positionals.slice(2).join(' '); + return positionals.slice(1).join(' '); } if ( positionals.length >= 3 && !Number.isNaN(Number(positionals[0])) && !Number.isNaN(Number(positionals[1])) ) { - return positionals.slice(2).join(' ').trim(); + return positionals.slice(2).join(' '); } - return positionals.slice(1).join(' ').trim(); + return positionals.slice(1).join(' '); } diff --git a/src/daemon/handlers/__tests__/session-replay-action-runtime.test.ts b/src/daemon/handlers/__tests__/session-replay-action-runtime.test.ts new file mode 100644 index 0000000000..953fbb3d29 --- /dev/null +++ b/src/daemon/handlers/__tests__/session-replay-action-runtime.test.ts @@ -0,0 +1,48 @@ +import { expect, test } from 'vitest'; +import { makeIosSession } from '../../../__tests__/test-utils/index.ts'; +import { recordActionEntry } from '../../session-action-recorder.ts'; +import type { DaemonRequest, SessionAction } from '../../types.ts'; +import { invokeReplayAction } from '../session-replay-action-runtime.ts'; + +const REPLAY_REQUEST: DaemonRequest = { + token: 'token', + session: 'default', + command: 'replay', + positionals: ['login.ad'], + flags: {}, +}; + +test.each(['', ' '])( + 'replay keeps source placeholder provenance when PASSWORD resolves to %j', + async (value) => { + const session = makeIosSession('default'); + const sourceAction: SessionAction = { + ts: 0, + command: 'fill', + positionals: ['id="password"', '${PASSWORD}'], + flags: {}, + }; + const response = await invokeReplayAction({ + req: REPLAY_REQUEST, + sessionName: 'default', + action: sourceAction, + scope: { values: { PASSWORD: value } }, + filePath: 'login.ad', + line: 1, + step: 1, + invoke: async (request) => { + recordActionEntry(session, { + command: request.command, + positionals: request.positionals ?? [], + flags: request.flags ?? {}, + result: { text: request.positionals?.at(-1) }, + }); + return { ok: true, data: {} }; + }, + }); + + expect(response.ok).toBe(true); + expect(session.actions[0]?.positionals).toEqual(['id="password"', '${PASSWORD}']); + expect(session.actions[0]?.result?.text).toBe('${PASSWORD}'); + }, +); diff --git a/src/daemon/handlers/interaction-common.ts b/src/daemon/handlers/interaction-common.ts index 1a422b9aca..a90bf823de 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -15,10 +15,8 @@ import { import { markPostGestureStabilization } from '../post-gesture-stabilization.ts'; import { computeTargetEvidence, type RecordedTargetCapture } from '../session-target-evidence.ts'; import { inferFillText } from '../action-utils.ts'; -import { - recordedInputPlaceholder, - replaceRecordedInputLiteral, -} from '../../replay/recorded-input.ts'; +import { recordedInputPlaceholder } from '../../replay/recorded-input.ts'; +import { parameterizeRecordedFillPayload } from '../parameterized-recorded-fill.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -126,10 +124,9 @@ function parameterizeFillPayloads(params: { flags: params.flags, result: params.result, }); - if (!literal) return [params.result, params.responseData]; const placeholder = recordedInputPlaceholder(params.flags.recordAs); return [ - replaceRecordedInputLiteral(params.result, literal, placeholder), - replaceRecordedInputLiteral(params.responseData, literal, placeholder), + parameterizeRecordedFillPayload(params.result, literal, placeholder), + parameterizeRecordedFillPayload(params.responseData, literal, placeholder), ]; } diff --git a/src/daemon/parameterized-recorded-fill.ts b/src/daemon/parameterized-recorded-fill.ts new file mode 100644 index 0000000000..13a68d18bc --- /dev/null +++ b/src/daemon/parameterized-recorded-fill.ts @@ -0,0 +1,74 @@ +import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; +import { tryParseSelectorChain } from '../selectors/parse.ts'; + +const VALUE_BEARING_SELECTOR_KEYS = new Set(['text', 'label', 'value']); + +/** + * Parameterize only fields whose contract says they carry the fill value. + * Stable identity/provenance strings stay byte-for-byte unchanged; a derived + * selector candidate is dropped only when a parsed text/label/value term is + * exactly the supplied literal. + */ +export function parameterizeRecordedFillPayload< + TPayload extends Record | undefined, +>(payload: TPayload, literal: string, placeholder: string): TPayload { + if (!payload) return payload; + const selectorChain = readStringArray(payload.selectorChain); + return { + ...payload, + ...(typeof payload.text === 'string' ? { text: placeholder } : {}), + ...(payload.refLabel === literal ? { refLabel: placeholder } : {}), + ...(selectorChain + ? { + selectorChain: selectorChain.filter( + (candidate) => !selectorCandidateCarriesFillValue(candidate, literal), + ), + } + : {}), + } as TPayload; +} + +/** Parameterize exact accessibility labels without rewriting identity fragments. */ +export function parameterizeRecordedFillTargetEvidence( + evidence: TargetAnnotationV1 | undefined, + literal: string, + placeholder: string, +): TargetAnnotationV1 | undefined { + if (!evidence) return evidence; + return { + ...evidence, + ...(evidence.label === literal ? { label: placeholder } : {}), + ancestry: evidence.ancestry.map((entry) => ({ + ...entry, + ...(entry.label === literal ? { label: placeholder } : {}), + })), + ...(evidence.scrollRegion + ? { + scrollRegion: { + ...evidence.scrollRegion, + ...(evidence.scrollRegion.label === literal ? { label: placeholder } : {}), + }, + } + : {}), + }; +} + +function selectorCandidateCarriesFillValue(candidate: string, literal: string): boolean { + const parsed = tryParseSelectorChain(candidate); + return ( + parsed?.selectors.some((selector) => + selector.terms.some( + (term) => + VALUE_BEARING_SELECTOR_KEYS.has(term.key) && + typeof term.value === 'string' && + term.value === literal, + ), + ) ?? false + ); +} + +function readStringArray(value: unknown): string[] | undefined { + return Array.isArray(value) && value.every((entry) => typeof entry === 'string') + ? value + : undefined; +} diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 061b738804..8bae3e1c5c 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -250,6 +250,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { if (!timingSafeStringEqual(req.token, token)) { return unauthorizedResponse(); } + registerParameterizedFillDiagnosticValue(req); let childScope: RequestExecutionScope | undefined; try { diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index cb752eae2f..d337403c95 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -7,9 +7,12 @@ import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import { inferFillText } from './action-utils.ts'; import { recordedInputPlaceholder, - replaceRecordedInputLiteral, validateRecordedInputVariableName, } from '../replay/recorded-input.ts'; +import { + parameterizeRecordedFillPayload, + parameterizeRecordedFillTargetEvidence, +} from './parameterized-recorded-fill.ts'; export type RecordActionEntry = { command: string; @@ -115,12 +118,11 @@ export function recordActionEntry( /** * #1348: the recorder is the first durable boundary. The live request keeps * the literal fill value through device execution, but the SessionAction gets - * only `${VAR}`. Value-bearing result strings are scrubbed at the same boundary - * so backend echoes or settled diffs cannot smuggle the literal into recording - * state, the event display, or the eventual publication temp/target file; - * ADR 0012 identity evidence keeps its structure but is scrubbed too, covering - * repeated fills where the pre-action accessibility label already echoes the - * same sensitive value. + * only `${VAR}`. The result's semantic fill-value field is parameterized at + * the same boundary, while selector/ref provenance is preserved byte-for-byte. + * Exact value-bearing selector candidates are omitted, and exact accessibility + * value labels in ADR 0012 evidence are parameterized without rewriting + * unrelated identity fragments. */ function parameterizeRecordedFill(entry: RecordActionEntry): RecordActionEntry { if (entry.command !== 'fill' || typeof entry.flags.recordAs !== 'string') return entry; @@ -132,12 +134,15 @@ function parameterizeRecordedFill(entry: RecordActionEntry): RecordActionEntry { positionals: entry.positionals, flags: entry.flags, }); - if (!literal) return entry; return { ...entry, positionals: replaceFillText(entry.positionals, placeholder), - result: replaceRecordedInputLiteral(entry.result, literal, placeholder), - targetEvidence: replaceRecordedInputLiteral(entry.targetEvidence, literal, placeholder), + result: parameterizeRecordedFillPayload(entry.result, literal, placeholder), + targetEvidence: parameterizeRecordedFillTargetEvidence( + entry.targetEvidence, + literal, + placeholder, + ), }; } diff --git a/src/replay/recorded-input.ts b/src/replay/recorded-input.ts index 10ca40a38a..febd717407 100644 --- a/src/replay/recorded-input.ts +++ b/src/replay/recorded-input.ts @@ -29,28 +29,3 @@ export function readRecordedInputVariableName(value: string): string | undefined if (!match?.[1] || match[1].startsWith('AD_')) return undefined; return match[1]; } - -export function replaceRecordedInputLiteral(value: T, literal: string, placeholder: string): T { - if (typeof value === 'string') return value.replaceAll(literal, placeholder) as T; - if (Array.isArray(value)) { - return value.map((entry) => replaceRecordedInputLiteral(entry, literal, placeholder)) as T; - } - if (!value || typeof value !== 'object') return value; - return Object.fromEntries( - Object.entries(value).map(([key, entry]) => { - if (key === 'ref') return [key, entry]; - if (key === 'selector' && typeof entry === 'string') { - return [key, entry.includes(literal) ? undefined : entry]; - } - if (key === 'selectorChain' && Array.isArray(entry)) { - return [ - key, - entry.filter( - (candidate) => typeof candidate !== 'string' || !candidate.includes(literal), - ), - ]; - } - return [key, replaceRecordedInputLiteral(entry, literal, placeholder)]; - }), - ) as T; -} diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 4f601513fa..3cb25b67ff 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { emitDiagnostic } from '../../../src/utils/diagnostics.ts'; import { test } from 'vitest'; import { assertRpcError, assertRpcOk } from './assertions.ts'; import { androidSettingsXml, createAndroidSettingsWorld } from './android-world.ts'; @@ -159,15 +160,28 @@ test('a second successful open aborts publication and terminal save flags fail b }, 20_000); test('parameterized fill publishes only ${VAR} and replay resolves it immediately before fill', async () => { + const secret = 'OpaqueProviderValue1348'; await withProviderScenarioResource( - async () => await createAndroidSettingsWorld({ nativeTextInjection: true }), + async () => + await createAndroidSettingsWorld({ + nativeTextInjection: true, + onTextInjection: (request) => { + emitDiagnostic({ + phase: 'provider_text_echo_regression', + data: { text: request.text }, + }); + }, + }), async (world) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-parameterized-script-')); const scriptPath = path.join(root, 'parameterized-search.ad'); - const secret = 'OpaqueProviderValue1348'; const client = world.daemon.client(); try { - await client.apps.open({ app: 'settings', saveScript: scriptPath, ...world.selection }); + const opened = await client.apps.open({ + app: 'settings', + saveScript: scriptPath, + ...world.selection, + }); const snapshot = await client.capture.snapshot({ interactiveOnly: true, ...world.selection, @@ -222,15 +236,31 @@ test('parameterized fill publishes only ${VAR} and replay resolves it immediatel assert.equal(world.textInjectionCalls.length, callsBeforeMissingValue); await client.sessions.close(); - const replay = await world.daemon.callCommand('replay', [scriptPath], { - ...world.selection, - replayEnv: [`SEARCH_TERM=${secret}`], - }); + const replay = await world.daemon.callCommand( + 'replay', + [scriptPath], + { + ...world.selection, + replayEnv: [`SEARCH_TERM=${secret}`], + verbose: true, + }, + { + meta: { debug: true, requestId: 'parameterized-replay-diagnostics' }, + }, + ); assertRpcOk(replay); assert.equal(world.textInjectionCalls.at(-1)?.text, secret); const replayState = JSON.stringify(world.daemon.session()?.actions); - assert.equal(replayState.includes(secret), false); + assert.equal(replayState.includes(secret), false, replayState); assert.match(replayState, /\$\{SEARCH_TERM\}/); + assert.ok(opened.sessionStateDir); + const requestLog = fs.readFileSync( + path.join(opened.sessionStateDir, 'requests', 'parameterized-replay-diagnostics.ndjson'), + 'utf8', + ); + assert.match(requestLog, /provider_text_echo_regression/); + assert.match(requestLog, /\[REDACTED\]/); + assert.equal(requestLog.includes(secret), false); await client.sessions.close(); await client.apps.open({ app: 'settings', ...world.selection }); diff --git a/test/integration/provider-scenarios/android-world.ts b/test/integration/provider-scenarios/android-world.ts index 867061c4fe..0113dc2541 100644 --- a/test/integration/provider-scenarios/android-world.ts +++ b/test/integration/provider-scenarios/android-world.ts @@ -52,6 +52,7 @@ type AndroidSettingsWorld = { export async function createAndroidSettingsWorld(options?: { nativeTextInjection?: boolean; + onTextInjection?: (request: AndroidSettingsWorld['textInjectionCalls'][number]) => void; snapshotXml?: () => string; dumpsysWindow?: () => string; onAdbExec?: (args: string[]) => void; @@ -146,6 +147,7 @@ export async function createAndroidSettingsWorld(options?: { }; if (options?.nativeTextInjection) { adbProvider.text = async (request) => { + options.onTextInjection?.(request); textInjectionCalls.push({ ...request }); shellState.searchText = request.text; }; From 030eb81af29c8d48420b01c540d2f92a34db5d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 23 Jul 2026 14:29:01 +0200 Subject: [PATCH 3/7] fix: sanitize parameterized fill echoes --- .../adr/0017-parameterized-recorded-inputs.md | 16 +- .../__tests__/session-action-recorder.test.ts | 13 +- .../__tests__/interaction-common.test.ts | 73 +++++++++ src/daemon/parameterized-recorded-fill.ts | 147 +++++++++++++++++- .../active-session-script-publication.test.ts | 6 + 5 files changed, 239 insertions(+), 16 deletions(-) create mode 100644 src/daemon/handlers/__tests__/interaction-common.test.ts diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md index a80928a6c3..a88ab108bd 100644 --- a/docs/adr/0017-parameterized-recorded-inputs.md +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -53,14 +53,16 @@ The literal exists only on the live request path long enough to execute the inte addition to key-name redaction, because an opaque value need not look secret. 2. The platform interaction receives the literal. 3. Before response/event/recording retention, the fill result's semantic `text` field and an exact - post-fill `refLabel` become `${VAR}`. Stable selector/ref provenance and unrelated strings are - preserved byte-for-byte; post-fill selector-chain candidates are dropped only when a parsed - `text`, `label`, or `value` term exactly equals the supplied literal. + post-fill `refLabel` become `${VAR}`. Stable selector/ref provenance is preserved byte-for-byte. + Untrusted backend extras and nested settle output parameterize delimited occurrences of the + supplied literal; post-fill selector-chain candidates are dropped only when a parsed `text`, + `label`, or `value` term semantically contains it. 4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the - placeholder into `SessionAction`, and parameterizes the semantic result field again. ADR 0012 - `target-v1` evidence keeps its structure; exact value-bearing accessibility labels become the - placeholder without rewriting identity fragments that merely contain the same characters. The - `recordAs` control flag itself is not serialized into the script. + placeholder into `SessionAction`, and parameterizes the semantic result field plus arbitrary + backend/settle echoes again. ADR 0012 `target-v1` evidence keeps its structure; exact + value-bearing accessibility labels become the placeholder without rewriting identity fragments + that merely contain the same characters. The `recordAs` control flag itself is not serialized + into the script. 5. The existing ADR 0012/0016 writer receives an already-parameterized action. Its selector provenance, portability checks, same-directory temp write, and atomic publication algorithm are unchanged. diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index f354efe6dc..7046c1019c 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -97,9 +97,14 @@ test('parameterized fills keep literals out of recording state with deterministi flags: { recordAs: 'PASSWORD' }, result: { text: password, - message: 'Filled 21 chars', + message: `Filled ${password}`, selector: 'id="password"', selectorChain: ['id="password"', `value="${password}" editable=true`], + settle: { + diff: { + lines: [{ kind: 'added', text: `value=${password}` }], + }, + }, }, targetEvidence: { role: 'textinput', @@ -129,10 +134,16 @@ test('parameterized fills keep literals out of recording state with deterministi expect(serialized).not.toContain(password); expect(serialized).not.toContain(token); expect(session.actions[0]?.result?.text).toBe('${PASSWORD}'); + expect(session.actions[0]?.result?.message).toBe('Filled ${PASSWORD}'); expect(session.actions[0]?.result?.selector).toBe('id="password"'); expect(session.actions[0]?.targetEvidence?.label).toBe('${PASSWORD}'); expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credentials'); expect(session.actions[0]?.result?.selectorChain).toEqual(['id="password"']); + expect(session.actions[0]?.result?.settle).toEqual({ + diff: { + lines: [{ kind: 'added', text: 'value=${PASSWORD}' }], + }, + }); }); test('a one-character parameterized fill preserves unrelated response and selector text', () => { diff --git a/src/daemon/handlers/__tests__/interaction-common.test.ts b/src/daemon/handlers/__tests__/interaction-common.test.ts new file mode 100644 index 0000000000..a29103ae3c --- /dev/null +++ b/src/daemon/handlers/__tests__/interaction-common.test.ts @@ -0,0 +1,73 @@ +import { expect, test } from 'vitest'; +import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; +import { finalizeTouchInteraction } from '../interaction-common.ts'; + +test('parameterized fill scrubs backend and nested settle echoes at the response boundary', () => { + const secret = 'OpaqueValue1348'; + const placeholder = '${PASSWORD}'; + const sessionStore = makeSessionStore(); + const session = makeIosSession('parameterized-fill'); + session.recordSession = true; + sessionStore.set(session.name, session); + const payload = { + text: secret, + message: `Backend filled ${secret}`, + selector: 'id="password"', + selectorChain: ['id="password"', `value="${secret}" editable=true`], + backendEcho: { + id: secret, + status: `accepted ${secret}`, + nested: { kind: secret }, + }, + settle: { + settled: true, + waitedMs: 1, + captures: 2, + quietMs: 1, + timeoutMs: 100, + diff: { + summary: { additions: 1, removals: 0, unchanged: 0 }, + lines: [{ kind: 'added', text: `TextField value=${secret}` }], + }, + hint: `Observed ${secret}`, + }, + }; + + const response = finalizeTouchInteraction({ + session, + sessionStore, + command: 'fill', + positionals: ['id="password"', secret], + flags: { recordAs: 'PASSWORD' }, + result: payload, + responseData: payload, + actionStartedAt: 1, + actionFinishedAt: 2, + }); + + expect(response.ok).toBe(true); + if (!response.ok) return; + expect(response.data).toMatchObject({ + text: placeholder, + message: `Backend filled ${placeholder}`, + selector: 'id="password"', + selectorChain: ['id="password"'], + backendEcho: { + id: placeholder, + status: `accepted ${placeholder}`, + nested: { kind: placeholder }, + }, + settle: { + diff: { + lines: [{ kind: 'added', text: `TextField value=${placeholder}` }], + }, + hint: `Observed ${placeholder}`, + }, + }); + expect(JSON.stringify(response.data)).not.toContain(secret); + + const recordedResult = session.actions[0]?.result; + expect(recordedResult).toEqual(response.data); + expect(JSON.stringify(recordedResult)).not.toContain(secret); +}); diff --git a/src/daemon/parameterized-recorded-fill.ts b/src/daemon/parameterized-recorded-fill.ts index 13a68d18bc..5e327b8546 100644 --- a/src/daemon/parameterized-recorded-fill.ts +++ b/src/daemon/parameterized-recorded-fill.ts @@ -2,12 +2,35 @@ import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import { tryParseSelectorChain } from '../selectors/parse.ts'; const VALUE_BEARING_SELECTOR_KEYS = new Set(['text', 'label', 'value']); +const STABLE_ROOT_OUTPUT_STRING_KEYS = new Set([ + 'action', + 'backend', + 'gesture', + 'kind', + 'platform', + 'ref', + 'selector', + 'targetKind', +]); +const STRUCTURED_OUTPUT_KEYS = new Set(['cost', 'evidence', 'resolution', 'settle']); +const STRUCTURAL_NESTED_STRING_KEYS = new Set([ + 'digest', + 'foregroundApp', + 'id', + 'kind', + 'ref', + 'role', + 'source', + 'verification', +]); +const IDENTIFIER_CHAR = /^[A-Za-z0-9_]$/; /** * Parameterize only fields whose contract says they carry the fill value. - * Stable identity/provenance strings stay byte-for-byte unchanged; a derived - * selector candidate is dropped only when a parsed text/label/value term is - * exactly the supplied literal. + * Stable identity/provenance strings stay byte-for-byte unchanged. Untrusted + * backend extras and nested settle output are scrubbed recursively; a derived + * selector candidate is dropped only when a parsed text/label/value term + * semantically contains the supplied literal. */ export function parameterizeRecordedFillPayload< TPayload extends Record | undefined, @@ -15,7 +38,7 @@ export function parameterizeRecordedFillPayload< if (!payload) return payload; const selectorChain = readStringArray(payload.selectorChain); return { - ...payload, + ...parameterizeBackendOutput(payload, literal, placeholder), ...(typeof payload.text === 'string' ? { text: placeholder } : {}), ...(payload.refLabel === literal ? { refLabel: placeholder } : {}), ...(selectorChain @@ -61,14 +84,122 @@ function selectorCandidateCarriesFillValue(candidate: string, literal: string): (term) => VALUE_BEARING_SELECTOR_KEYS.has(term.key) && typeof term.value === 'string' && - term.value === literal, + parameterizeSensitiveString(term.value, literal, '') !== term.value, ), ) ?? false ); } +function parameterizeBackendOutput( + value: Record, + literal: string, + placeholder: string, +): Record { + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [ + key, + parameterizeRootOutputValue(entry, key, literal, placeholder), + ]), + ); +} + +function parameterizeRootOutputValue( + value: unknown, + key: string, + literal: string, + placeholder: string, +): unknown { + if (typeof value === 'string') { + if (STABLE_ROOT_OUTPUT_STRING_KEYS.has(key)) return value; + if (key === 'refLabel') return value === literal ? placeholder : value; + } + if (key === 'selectorChain' && isStringArray(value)) { + return filterSensitiveSelectorCandidates(value, literal); + } + return parameterizeBackendOutputValue( + value, + key, + literal, + placeholder, + STRUCTURED_OUTPUT_KEYS.has(key), + ); +} + +function parameterizeBackendOutputValue( + value: unknown, + key: string, + literal: string, + placeholder: string, + preserveStructuralStrings: boolean, +): unknown { + if (typeof value === 'string') { + if (preserveStructuralStrings && STRUCTURAL_NESTED_STRING_KEYS.has(key)) return value; + return parameterizeSensitiveString(value, literal, placeholder); + } + if (Array.isArray(value)) { + return value.map((entry) => + parameterizeBackendOutputValue(entry, key, literal, placeholder, preserveStructuralStrings), + ); + } + if (!value || typeof value !== 'object') return value; + return Object.fromEntries( + Object.entries(value).map(([nestedKey, entry]) => [ + nestedKey, + parameterizeBackendOutputValue( + entry, + nestedKey, + literal, + placeholder, + preserveStructuralStrings, + ), + ]), + ); +} + +function filterSensitiveSelectorCandidates(value: string[], literal: string): string[] { + return value.filter((candidate) => !selectorCandidateCarriesFillValue(candidate, literal)); +} + +function parameterizeSensitiveString(value: string, literal: string, placeholder: string): string { + if (!literal.trim()) return value === literal ? placeholder : value; + if (!isIdentifierLiteral(literal)) { + return value.replaceAll(literal, placeholder); + } + return parameterizeDelimitedLiteral(value, literal, placeholder); +} + +function isIdentifierLiteral(literal: string): boolean { + return Array.from(literal).every((character) => IDENTIFIER_CHAR.test(character)); +} + +function parameterizeDelimitedLiteral(value: string, literal: string, placeholder: string): string { + let cursor = 0; + let result = ''; + while (cursor < value.length) { + const matchIndex = value.indexOf(literal, cursor); + if (matchIndex === -1) return result + value.slice(cursor); + result += value.slice(cursor, matchIndex); + result += isDelimitedMatch(value, literal, matchIndex) ? placeholder : literal; + cursor = matchIndex + literal.length; + } + return result; +} + +function isDelimitedMatch(value: string, literal: string, matchIndex: number): boolean { + return ( + !isIdentifierCharacter(value[matchIndex - 1]) && + !isIdentifierCharacter(value[matchIndex + literal.length]) + ); +} + +function isIdentifierCharacter(value: string | undefined): boolean { + return value !== undefined && IDENTIFIER_CHAR.test(value); +} + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((entry) => typeof entry === 'string'); +} + function readStringArray(value: unknown): string[] | undefined { - return Array.isArray(value) && value.every((entry) => typeof entry === 'string') - ? value - : undefined; + return isStringArray(value) ? value : undefined; } diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 3cb25b67ff..8e40291c71 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -208,9 +208,15 @@ test('parameterized fill publishes only ${VAR} and replay resolves it immediatel ref: `@${search.ref}`, text: secret, recordAs: 'SEARCH_TERM', + settle: true, + settleQuietMs: 1, + timeoutMs: 1_000, ...world.selection, }); assert.equal(fill.text, '${SEARCH_TERM}'); + const settleOutput = JSON.stringify(fill.settle); + assert.equal(settleOutput.includes(secret), false, settleOutput); + assert.match(settleOutput, /\$\{SEARCH_TERM\}/); await client.command.wait({ selector: 'id=com.android.settings:id/search', ...world.selection, From f43a96827c1f389078667a64c162e1939ebb9f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 23 Jul 2026 16:05:37 +0200 Subject: [PATCH 4/7] fix: scrub embedded parameterized fill echoes --- .../adr/0017-parameterized-recorded-inputs.md | 12 +- .../__tests__/session-action-recorder.test.ts | 4 +- .../__tests__/interaction-common.test.ts | 128 +++++++++++- src/daemon/parameterized-recorded-fill.ts | 184 +++++++++++------- .../active-session-script-publication.test.ts | 13 +- 5 files changed, 256 insertions(+), 85 deletions(-) diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md index a88ab108bd..a3881815c8 100644 --- a/docs/adr/0017-parameterized-recorded-inputs.md +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -52,11 +52,13 @@ The literal exists only on the live request path long enough to execute the inte 1. The request scope registers it as an explicit diagnostics secret before platform work. This is in addition to key-name redaction, because an opaque value need not look secret. 2. The platform interaction receives the literal. -3. Before response/event/recording retention, the fill result's semantic `text` field and an exact - post-fill `refLabel` become `${VAR}`. Stable selector/ref provenance is preserved byte-for-byte. - Untrusted backend extras and nested settle output parameterize delimited occurrences of the - supplied literal; post-fill selector-chain candidates are dropped only when a parsed `text`, - `label`, or `value` term semantically contains it. +3. Before response/event/recording retention, the fill result's semantic `text` field becomes + `${VAR}` and every occurrence in a post-fill `refLabel` is parameterized. Stable selector/ref + provenance is preserved byte-for-byte. + Untrusted backend extras and nested settle output parameterize every occurrence of the supplied + literal in both keys and values; post-fill selector-chain candidates are dropped only when a + parsed `text`, `label`, or `value` term semantically contains it. Known response and settle + schema fields are the explicit structural boundary and retain their names and provenance values. 4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the placeholder into `SessionAction`, and parameterizes the semantic result field plus arbitrary backend/settle echoes again. ADR 0012 `target-v1` evidence keeps its structure; exact diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 7046c1019c..8612a64c93 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -146,7 +146,7 @@ test('parameterized fills keep literals out of recording state with deterministi }); }); -test('a one-character parameterized fill preserves unrelated response and selector text', () => { +test('a one-character parameterized fill preserves structural selector provenance', () => { const session = makeIosSession('default'); recordActionEntry(session, { command: 'fill', @@ -172,7 +172,7 @@ test('a one-character parameterized fill preserves unrelated response and select positionals: ['id="password"', '${LETTER}'], result: { text: '${LETTER}', - message: 'Already safe', + message: 'Alre${LETTER}dy s${LETTER}fe', selector: 'id="password"', selectorChain: ['id="password"', 'label="Account"'], }, diff --git a/src/daemon/handlers/__tests__/interaction-common.test.ts b/src/daemon/handlers/__tests__/interaction-common.test.ts index a29103ae3c..2ee0d66fc7 100644 --- a/src/daemon/handlers/__tests__/interaction-common.test.ts +++ b/src/daemon/handlers/__tests__/interaction-common.test.ts @@ -1,8 +1,28 @@ -import { expect, test } from 'vitest'; +import { beforeEach, expect, test, vi } from 'vitest'; import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; +import { attachRefs, type RawSnapshotNode } from '../../../kernel/snapshot.ts'; +import type { CommandFlags } from '../../../core/dispatch.ts'; +import { handleInteractionCommands } from '../interaction.ts'; import { finalizeTouchInteraction } from '../interaction-common.ts'; +vi.mock('../../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchCommand: vi.fn(async () => ({})), + }; +}); + +import { dispatchCommand } from '../../../core/dispatch.ts'; +const mockDispatch = vi.mocked(dispatchCommand); +const contextFromFlags = (_flags: CommandFlags | undefined) => ({}); + +beforeEach(() => { + mockDispatch.mockReset(); + mockDispatch.mockResolvedValue({}); +}); + test('parameterized fill scrubs backend and nested settle echoes at the response boundary', () => { const secret = 'OpaqueValue1348'; const placeholder = '${PASSWORD}'; @@ -12,13 +32,22 @@ test('parameterized fill scrubs backend and nested settle echoes at the response sessionStore.set(session.name, session); const payload = { text: secret, - message: `Backend filled ${secret}`, + message: `prefix${secret}suffix`, + refLabel: `prefix${secret}suffix`, selector: 'id="password"', - selectorChain: ['id="password"', `value="${secret}" editable=true`], + selectorChain: [ + 'id="password"', + `value="prefix${secret}suffix" editable=true`, + `label="${secret}4"`, + ], backendEcho: { id: secret, - status: `accepted ${secret}`, + status: `accepted${secret}suffix`, nested: { kind: secret }, + [secret]: true, + [`prefix${secret}suffix`]: { + [`${secret}4`]: true, + }, }, settle: { settled: true, @@ -28,9 +57,14 @@ test('parameterized fill scrubs backend and nested settle echoes at the response timeoutMs: 100, diff: { summary: { additions: 1, removals: 0, unchanged: 0 }, - lines: [{ kind: 'added', text: `TextField value=${secret}` }], + lines: [{ kind: 'added', text: `TextField value=prefix${secret}suffix` }], + }, + hint: `Observed ${secret}4`, + [`echo${secret}key`]: true, + backendEcho: { + id: secret, + [`key${secret}`]: `value${secret}`, }, - hint: `Observed ${secret}`, }, }; @@ -50,19 +84,29 @@ test('parameterized fill scrubs backend and nested settle echoes at the response if (!response.ok) return; expect(response.data).toMatchObject({ text: placeholder, - message: `Backend filled ${placeholder}`, + message: `prefix${placeholder}suffix`, + refLabel: `prefix${placeholder}suffix`, selector: 'id="password"', selectorChain: ['id="password"'], backendEcho: { id: placeholder, - status: `accepted ${placeholder}`, + status: `accepted${placeholder}suffix`, nested: { kind: placeholder }, + [placeholder]: true, + [`prefix${placeholder}suffix`]: { + [`${placeholder}4`]: true, + }, }, settle: { diff: { - lines: [{ kind: 'added', text: `TextField value=${placeholder}` }], + lines: [{ kind: 'added', text: `TextField value=prefix${placeholder}suffix` }], + }, + hint: `Observed ${placeholder}4`, + [`echo${placeholder}key`]: true, + backendEcho: { + id: placeholder, + [`key${placeholder}`]: `value${placeholder}`, }, - hint: `Observed ${placeholder}`, }, }); expect(JSON.stringify(response.data)).not.toContain(secret); @@ -71,3 +115,67 @@ test('parameterized fill scrubs backend and nested settle echoes at the response expect(recordedResult).toEqual(response.data); expect(JSON.stringify(recordedResult)).not.toContain(secret); }); + +test('parameterized fill scrubs concatenated backend values and object keys through the handler route', async () => { + const secret = 'TOKEN123'; + const placeholder = '${PASSWORD}'; + const sessionStore = makeSessionStore(); + const sessionName = 'parameterized-fill-handler-route'; + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + const nodes: RawSnapshotNode[] = [ + { + index: 0, + type: 'XCUIElementTypeTextField', + identifier: 'password', + label: 'Password', + rect: { x: 20, y: 40, width: 200, height: 44 }, + enabled: true, + hittable: true, + }, + ]; + session.recordSession = true; + session.snapshot = { + nodes: attachRefs(nodes), + createdAt: Date.now(), + backend: 'xctest', + }; + sessionStore.set(sessionName, session); + mockDispatch.mockImplementation(async (_device, command) => + command === 'fill' + ? { + message: `prefix${secret}suffix`, + [`prefix${secret}suffix`]: { + [`${secret}4`]: `echo${secret}`, + }, + } + : {}, + ); + + const response = await handleInteractionCommands({ + req: { + token: 't', + session: sessionName, + command: 'fill', + positionals: ['@e1', secret], + flags: { recordAs: 'PASSWORD' }, + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + expect(response, JSON.stringify(response)).toMatchObject({ ok: true }); + if (!response?.ok) return; + expect(response.data).toMatchObject({ + message: 'Filled 8 chars', + [`prefix${placeholder}suffix`]: { + [`${placeholder}4`]: `echo${placeholder}`, + }, + ref: 'e1', + text: placeholder, + }); + expect(JSON.stringify(response.data)).not.toContain(secret); + expect(JSON.stringify(session.actions)).not.toContain(secret); + expect(mockDispatch.mock.calls[0]?.[1]).toBe('fill'); + expect(mockDispatch.mock.calls[0]?.[2]).toContain(secret); +}); diff --git a/src/daemon/parameterized-recorded-fill.ts b/src/daemon/parameterized-recorded-fill.ts index 5e327b8546..0197f4cd4e 100644 --- a/src/daemon/parameterized-recorded-fill.ts +++ b/src/daemon/parameterized-recorded-fill.ts @@ -2,35 +2,112 @@ import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import { tryParseSelectorChain } from '../selectors/parse.ts'; const VALUE_BEARING_SELECTOR_KEYS = new Set(['text', 'label', 'value']); -const STABLE_ROOT_OUTPUT_STRING_KEYS = new Set([ +const STRUCTURAL_ROOT_OUTPUT_KEYS = new Set([ 'action', 'backend', + 'cost', + 'delayMs', + 'evidence', 'gesture', + 'hint', 'kind', + 'maestroFallbackReason', + 'maestroNonHittableCoordinateFallbackAllowed', + 'maestroNonHittableCoordinateFallbackUsed', + 'message', 'platform', 'ref', + 'referenceHeight', + 'referenceWidth', + 'refLabel', + 'resolution', 'selector', + 'selectorChain', + 'settle', + 'targetHittable', 'targetKind', + 'text', + 'warning', + 'x', + 'y', ]); -const STRUCTURED_OUTPUT_KEYS = new Set(['cost', 'evidence', 'resolution', 'settle']); -const STRUCTURAL_NESTED_STRING_KEYS = new Set([ - 'digest', - 'foregroundApp', - 'id', +const STABLE_ROOT_OUTPUT_STRING_KEYS = new Set([ + 'action', + 'backend', + 'gesture', 'kind', + 'platform', 'ref', - 'role', - 'source', - 'verification', + 'selector', + 'targetKind', +]); +const STRUCTURED_OUTPUT_KEYS = new Set(['cost', 'evidence', 'resolution', 'settle']); +const STRUCTURAL_KEYS_BY_PATH = new Map>([ + ['cost', new Set(['nodeCount', 'runnerRoundTrips', 'wallClockMs'])], + [ + 'evidence', + new Set(['changedFromBefore', 'digest', 'foregroundApp', 'interactiveNodeCount', 'nodeCount']), + ], + [ + 'resolution', + new Set([ + 'alternatives', + 'kind', + 'matchCount', + 'phase', + 'source', + 'tiebreak', + 'winnerDiagnostic', + ]), + ], + ['resolution.alternatives', new Set(['diagnosticRef', 'label', 'role'])], + ['resolution.winnerDiagnostic', new Set(['diagnosticRef', 'label', 'role'])], + [ + 'settle', + new Set([ + 'captures', + 'diff', + 'hint', + 'quietMs', + 'refs', + 'refsGeneration', + 'settled', + 'tail', + 'tailTruncated', + 'timeoutMs', + 'waitedMs', + ]), + ], + ['settle.diff', new Set(['lines', 'summary', 'truncated'])], + ['settle.diff.lines', new Set(['kind', 'ref', 'text'])], + ['settle.diff.summary', new Set(['additions', 'removals', 'unchanged'])], + ['settle.refs', new Set(['ref'])], + ['settle.tail', new Set(['label', 'ref', 'role'])], +]); +const STABLE_STRUCTURAL_STRING_PATHS = new Set([ + 'evidence.digest', + 'evidence.foregroundApp', + 'resolution.alternatives.diagnosticRef', + 'resolution.alternatives.role', + 'resolution.kind', + 'resolution.phase', + 'resolution.source', + 'resolution.tiebreak', + 'resolution.winnerDiagnostic.diagnosticRef', + 'resolution.winnerDiagnostic.role', + 'settle.diff.lines.kind', + 'settle.diff.lines.ref', + 'settle.refs.ref', + 'settle.tail.ref', + 'settle.tail.role', ]); -const IDENTIFIER_CHAR = /^[A-Za-z0-9_]$/; /** * Parameterize only fields whose contract says they carry the fill value. * Stable identity/provenance strings stay byte-for-byte unchanged. Untrusted - * backend extras and nested settle output are scrubbed recursively; a derived - * selector candidate is dropped only when a parsed text/label/value term - * semantically contains the supplied literal. + * backend extras and nested settle output keys and values are scrubbed + * recursively; a derived selector candidate is dropped only when a parsed + * text/label/value term semantically contains the supplied literal. */ export function parameterizeRecordedFillPayload< TPayload extends Record | undefined, @@ -40,7 +117,6 @@ export function parameterizeRecordedFillPayload< return { ...parameterizeBackendOutput(payload, literal, placeholder), ...(typeof payload.text === 'string' ? { text: placeholder } : {}), - ...(payload.refLabel === literal ? { refLabel: placeholder } : {}), ...(selectorChain ? { selectorChain: selectorChain.filter( @@ -96,10 +172,12 @@ function parameterizeBackendOutput( placeholder: string, ): Record { return Object.fromEntries( - Object.entries(value).map(([key, entry]) => [ - key, - parameterizeRootOutputValue(entry, key, literal, placeholder), - ]), + Object.entries(value).map(([key, entry]) => { + const outputKey = STRUCTURAL_ROOT_OUTPUT_KEYS.has(key) + ? key + : parameterizeSensitiveString(key, literal, placeholder); + return [outputKey, parameterizeRootOutputValue(entry, key, literal, placeholder)]; + }), ); } @@ -111,48 +189,51 @@ function parameterizeRootOutputValue( ): unknown { if (typeof value === 'string') { if (STABLE_ROOT_OUTPUT_STRING_KEYS.has(key)) return value; - if (key === 'refLabel') return value === literal ? placeholder : value; } if (key === 'selectorChain' && isStringArray(value)) { return filterSensitiveSelectorCandidates(value, literal); } return parameterizeBackendOutputValue( value, - key, literal, placeholder, - STRUCTURED_OUTPUT_KEYS.has(key), + STRUCTURED_OUTPUT_KEYS.has(key) ? key : undefined, ); } function parameterizeBackendOutputValue( value: unknown, - key: string, literal: string, placeholder: string, - preserveStructuralStrings: boolean, + structuralPath: string | undefined, ): unknown { if (typeof value === 'string') { - if (preserveStructuralStrings && STRUCTURAL_NESTED_STRING_KEYS.has(key)) return value; + if (structuralPath && STABLE_STRUCTURAL_STRING_PATHS.has(structuralPath)) return value; return parameterizeSensitiveString(value, literal, placeholder); } if (Array.isArray(value)) { return value.map((entry) => - parameterizeBackendOutputValue(entry, key, literal, placeholder, preserveStructuralStrings), + parameterizeBackendOutputValue(entry, literal, placeholder, structuralPath), ); } if (!value || typeof value !== 'object') return value; + const structuralKeys = structuralPath ? STRUCTURAL_KEYS_BY_PATH.get(structuralPath) : undefined; return Object.fromEntries( - Object.entries(value).map(([nestedKey, entry]) => [ - nestedKey, - parameterizeBackendOutputValue( - entry, - nestedKey, - literal, - placeholder, - preserveStructuralStrings, - ), - ]), + Object.entries(value).map(([nestedKey, entry]) => { + const isStructural = structuralKeys?.has(nestedKey) === true; + const outputKey = isStructural + ? nestedKey + : parameterizeSensitiveString(nestedKey, literal, placeholder); + return [ + outputKey, + parameterizeBackendOutputValue( + entry, + literal, + placeholder, + isStructural ? `${structuralPath}.${nestedKey}` : undefined, + ), + ]; + }), ); } @@ -162,38 +243,7 @@ function filterSensitiveSelectorCandidates(value: string[], literal: string): st function parameterizeSensitiveString(value: string, literal: string, placeholder: string): string { if (!literal.trim()) return value === literal ? placeholder : value; - if (!isIdentifierLiteral(literal)) { - return value.replaceAll(literal, placeholder); - } - return parameterizeDelimitedLiteral(value, literal, placeholder); -} - -function isIdentifierLiteral(literal: string): boolean { - return Array.from(literal).every((character) => IDENTIFIER_CHAR.test(character)); -} - -function parameterizeDelimitedLiteral(value: string, literal: string, placeholder: string): string { - let cursor = 0; - let result = ''; - while (cursor < value.length) { - const matchIndex = value.indexOf(literal, cursor); - if (matchIndex === -1) return result + value.slice(cursor); - result += value.slice(cursor, matchIndex); - result += isDelimitedMatch(value, literal, matchIndex) ? placeholder : literal; - cursor = matchIndex + literal.length; - } - return result; -} - -function isDelimitedMatch(value: string, literal: string, matchIndex: number): boolean { - return ( - !isIdentifierCharacter(value[matchIndex - 1]) && - !isIdentifierCharacter(value[matchIndex + literal.length]) - ); -} - -function isIdentifierCharacter(value: string | undefined): boolean { - return value !== undefined && IDENTIFIER_CHAR.test(value); + return value.replaceAll(literal, placeholder); } function isStringArray(value: unknown): value is string[] { diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 8e40291c71..32f076c0ed 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -161,16 +161,27 @@ test('a second successful open aborts publication and terminal save flags fail b test('parameterized fill publishes only ${VAR} and replay resolves it immediately before fill', async () => { const secret = 'OpaqueProviderValue1348'; + let injectedText: string | undefined; + let capturesAfterInjection = 0; await withProviderScenarioResource( async () => await createAndroidSettingsWorld({ nativeTextInjection: true, onTextInjection: (request) => { + injectedText = request.text; + capturesAfterInjection = 0; emitDiagnostic({ phase: 'provider_text_echo_regression', data: { text: request.text }, }); }, + snapshotXml: () => { + if (injectedText === undefined) return androidSettingsXml(''); + capturesAfterInjection += 1; + const visibleText = + capturesAfterInjection <= 3 ? injectedText : `prefix${injectedText}suffix`; + return androidSettingsXml(visibleText); + }, }), async (world) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-parameterized-script-')); @@ -216,7 +227,7 @@ test('parameterized fill publishes only ${VAR} and replay resolves it immediatel assert.equal(fill.text, '${SEARCH_TERM}'); const settleOutput = JSON.stringify(fill.settle); assert.equal(settleOutput.includes(secret), false, settleOutput); - assert.match(settleOutput, /\$\{SEARCH_TERM\}/); + assert.match(settleOutput, /prefix\$\{SEARCH_TERM\}suffix/); await client.command.wait({ selector: 'id=com.android.settings:id/search', ...world.selection, From 6a44acbe32a5b54a0add9a655abfdfd803d843ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 24 Jul 2026 08:38:48 +0200 Subject: [PATCH 5/7] fix: make recorded fill scrubbing idempotent --- .../adr/0017-parameterized-recorded-inputs.md | 7 +- .../__tests__/session-action-recorder.test.ts | 28 +++++ .../__tests__/interaction-common.test.ts | 114 ++++++++++++++++++ src/daemon/parameterized-recorded-fill.ts | 18 ++- 4 files changed, 163 insertions(+), 4 deletions(-) diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md index a3881815c8..56d11556b1 100644 --- a/docs/adr/0017-parameterized-recorded-inputs.md +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -59,12 +59,17 @@ The literal exists only on the live request path long enough to execute the inte literal in both keys and values; post-fill selector-chain candidates are dropped only when a parsed `text`, `label`, or `value` term semantically contains it. Known response and settle schema fields are the explicit structural boundary and retain their names and provenance values. + Because inline whitespace replacement cannot distinguish a sensitive value from ordinary + separators, an untrusted string or key containing a whitespace-only literal collapses to the + placeholder instead. 4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the placeholder into `SessionAction`, and parameterizes the semantic result field plus arbitrary backend/settle echoes again. ADR 0012 `target-v1` evidence keeps its structure; exact value-bearing accessibility labels become the placeholder without rewriting identity fragments that merely contain the same characters. The `recordAs` control flag itself is not serialized - into the script. + into the script. Parameterization is idempotent: this second pass preserves placeholders inserted + at the response boundary even when the literal is part of the placeholder's variable name or is + `$`. 5. The existing ADR 0012/0016 writer receives an already-parameterized action. Its selector provenance, portability checks, same-directory temp write, and atomic publication algorithm are unchanged. diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index 8612a64c93..d463b70976 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -198,3 +198,31 @@ test.each(['', ' '])( expect(session.actions[0]?.result?.text).toBe('${PASSWORD}'); }, ); + +test('whitespace-only fills collapse ambiguous recorder output and keys', () => { + const session = makeIosSession('default'); + const whitespace = ' '; + recordActionEntry(session, { + command: 'fill', + positionals: ['id="password"', whitespace], + flags: { recordAs: 'PASSWORD' }, + result: { + text: whitespace, + message: `prefix${whitespace}suffix`, + backend: { + [`key${whitespace}tail`]: `value${whitespace}tail`, + }, + selectorChain: ['id="password"', `value="prefix${whitespace}suffix"`], + }, + }); + + expect(session.actions[0]?.result).toEqual({ + text: '${PASSWORD}', + message: '${PASSWORD}', + backend: { + '${PASSWORD}': '${PASSWORD}', + }, + selectorChain: ['id="password"'], + }); + expect(JSON.stringify(session.actions)).not.toContain(whitespace); +}); diff --git a/src/daemon/handlers/__tests__/interaction-common.test.ts b/src/daemon/handlers/__tests__/interaction-common.test.ts index 2ee0d66fc7..e46c6dc96e 100644 --- a/src/daemon/handlers/__tests__/interaction-common.test.ts +++ b/src/daemon/handlers/__tests__/interaction-common.test.ts @@ -179,3 +179,117 @@ test('parameterized fill scrubs concatenated backend values and object keys thro expect(mockDispatch.mock.calls[0]?.[1]).toBe('fill'); expect(mockDispatch.mock.calls[0]?.[2]).toContain(secret); }); + +test('parameterized fill collapses whitespace-only backend echoes through the handler route', async () => { + const secret = ' '; + const placeholder = '${SPACES}'; + const sessionStore = makeSessionStore(); + const sessionName = 'parameterized-whitespace-fill-handler-route'; + const session = makeIosSession(sessionName, { appBundleId: 'com.example.app' }); + session.recordSession = true; + session.snapshot = { + nodes: attachRefs([ + { + index: 0, + type: 'XCUIElementTypeTextField', + identifier: 'password', + label: 'Password', + rect: { x: 20, y: 40, width: 200, height: 44 }, + enabled: true, + hittable: true, + }, + ]), + createdAt: Date.now(), + backend: 'xctest', + }; + sessionStore.set(sessionName, session); + mockDispatch.mockResolvedValue({ + [`prefix${secret}suffix`]: { + [`key${secret}tail`]: `value${secret}tail`, + }, + selectorChain: [`value="prefix${secret}suffix"`], + }); + + const response = await handleInteractionCommands({ + req: { + token: 't', + session: sessionName, + command: 'fill', + positionals: ['@e1', secret], + flags: { recordAs: 'SPACES' }, + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + expect(response, JSON.stringify(response)).toMatchObject({ ok: true }); + if (!response?.ok) return; + expect(response.data).toBeDefined(); + const responseData = response.data!; + expect(responseData).toMatchObject({ + [placeholder]: { + [placeholder]: placeholder, + }, + text: placeholder, + }); + expect(responseData.selectorChain).not.toContain(`value="prefix${secret}suffix"`); + expect(JSON.stringify(responseData)).not.toContain(secret); + expect(session.actions[0]?.result).toMatchObject({ + [placeholder]: { + [placeholder]: placeholder, + }, + text: placeholder, + }); + expect(session.actions[0]?.result?.selectorChain).not.toContain(`value="prefix${secret}suffix"`); + expect(JSON.stringify(session.actions)).not.toContain(secret); + expect(mockDispatch.mock.calls[0]?.[2]).toContain(secret); +}); + +test.each([ + { literal: 'PASSWORD', recordAs: 'PASSWORD' }, + { literal: '$', recordAs: 'DOLLAR' }, +])( + 'parameterization remains stable across response and recorder boundaries for $literal', + ({ literal, recordAs }) => { + const placeholder = `\${${recordAs}}`; + const sessionStore = makeSessionStore(); + const session = makeIosSession(`parameterized-overlap-${recordAs}`); + session.recordSession = true; + sessionStore.set(session.name, session); + const payload = { + text: literal, + refLabel: `prefix${literal}suffix`, + backendEcho: { + [`key${literal}`]: `value${literal}`, + }, + settle: { hint: `hint${literal}` }, + }; + + const response = finalizeTouchInteraction({ + session, + sessionStore, + command: 'fill', + positionals: ['id="password"', literal], + flags: { recordAs }, + result: payload, + responseData: payload, + actionStartedAt: 1, + actionFinishedAt: 2, + }); + + expect(response).toEqual({ + ok: true, + data: { + text: placeholder, + refLabel: `prefix${placeholder}suffix`, + backendEcho: { + [`key${placeholder}`]: `value${placeholder}`, + }, + settle: { hint: `hint${placeholder}` }, + }, + }); + if (!response.ok) return; + expect(session.actions[0]?.result).toEqual(response.data); + }, +); diff --git a/src/daemon/parameterized-recorded-fill.ts b/src/daemon/parameterized-recorded-fill.ts index 0197f4cd4e..3e18ff8711 100644 --- a/src/daemon/parameterized-recorded-fill.ts +++ b/src/daemon/parameterized-recorded-fill.ts @@ -107,7 +107,9 @@ const STABLE_STRUCTURAL_STRING_PATHS = new Set([ * Stable identity/provenance strings stay byte-for-byte unchanged. Untrusted * backend extras and nested settle output keys and values are scrubbed * recursively; a derived selector candidate is dropped only when a parsed - * text/label/value term semantically contains the supplied literal. + * text/label/value term semantically contains the supplied literal. Repeated + * application preserves placeholders already inserted by an earlier response + * boundary. */ export function parameterizeRecordedFillPayload< TPayload extends Record | undefined, @@ -242,8 +244,18 @@ function filterSensitiveSelectorCandidates(value: string[], literal: string): st } function parameterizeSensitiveString(value: string, literal: string, placeholder: string): string { - if (!literal.trim()) return value === literal ? placeholder : value; - return value.replaceAll(literal, placeholder); + if (!literal) return value === literal ? placeholder : value; + + const unparameterizedSegments = placeholder ? value.split(placeholder) : [value]; + if (unparameterizedSegments.every((segment) => !segment.includes(literal))) return value; + + // 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); } function isStringArray(value: unknown): value is string[] { From fceaa3fdad13906eebdf3a111771269b41225f04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 24 Jul 2026 16:36:21 +0200 Subject: [PATCH 6/7] fix: replay parameterized coordinate fills --- src/replay/__tests__/script.test.ts | 18 ++++++++++++++++++ src/replay/script.ts | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/src/replay/__tests__/script.test.ts b/src/replay/__tests__/script.test.ts index 8e5788b0a3..c2de1931fb 100644 --- a/src/replay/__tests__/script.test.ts +++ b/src/replay/__tests__/script.test.ts @@ -183,6 +183,24 @@ test('type and fill replay scripts round-trip typing delay flags', () => { assert.equal(parsed[1]?.flags.delayMs, 40); }); +test('coordinate fill replay scripts preserve both coordinates and parameterized text', () => { + const script = formatReplayScriptForTest([ + { + ts: Date.now(), + command: 'fill', + positionals: ['100', '482', '${PASSWORD}'], + flags: {}, + }, + ]); + + assert.match(script, /fill 100 482 "\$\{PASSWORD\}"/); + assert.deepEqual(parseReplayScriptDetailed(script).actions[0]?.positionals, [ + '100', + '482', + '${PASSWORD}', + ]); +}); + test('type replay script preserves literal delay flag tokens', () => { const parsed = parseReplayScriptDetailed('type "--delay-ms" "abc"\n').actions; assert.deepEqual(parsed[0]?.positionals, ['--delay-ms', 'abc']); diff --git a/src/replay/script.ts b/src/replay/script.ts index 8378e6104e..dafc8528a2 100644 --- a/src/replay/script.ts +++ b/src/replay/script.ts @@ -336,6 +336,11 @@ function parseReplayScriptLine(line: string): SessionAction | null { action.positionals = parsed.positionals; return action; } + const [maybeX, maybeY, ...coordinateText] = parsed.positionals; + if (isNumericToken(maybeX) && isNumericToken(maybeY)) { + action.positionals = [maybeX, maybeY, coordinateText.join(' ')]; + return action; + } const [target, text, ...textRest] = parsed.positionals; if (target.startsWith('@')) { const ref = stripRecordedRefGeneration(target); From 7d7bb1dadd71253374bb9be1e09df8118076ccdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 24 Jul 2026 20:14:52 +0200 Subject: [PATCH 7/7] test: align parameterized publication landmark --- .../active-session-script-publication.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 32f076c0ed..87844d29a6 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -229,12 +229,12 @@ test('parameterized fill publishes only ${VAR} and replay resolves it immediatel assert.equal(settleOutput.includes(secret), false, settleOutput); assert.match(settleOutput, /prefix\$\{SEARCH_TERM\}suffix/); await client.command.wait({ - selector: 'id=com.android.settings:id/search', + selector: 'id=android:id/title', ...world.selection, }); const recordedState = JSON.stringify(world.daemon.session()?.actions); - assert.equal(recordedState.includes(secret), false); + assert.equal(recordedState.includes(secret), false, recordedState); assert.match(recordedState, /\$\{SEARCH_TERM\}/); await client.sessions.saveScript({ path: scriptPath }); const script = fs.readFileSync(scriptPath, 'utf8');