From d98628b2a7646bfa0ce48f1db88c91999cf246b5 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/4] 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 6956b4816..0750d7942 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 fc14468fe..0665cee9e 100644 --- a/docs/adr/0016-active-session-script-publication.md +++ b/docs/adr/0016-active-session-script-publication.md @@ -111,15 +111,14 @@ absence of `close` changes neither action dispatch nor the success response shap ### 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 @@ -167,8 +166,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. ## Alternatives Considered @@ -215,7 +214,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 000000000..9ece01064 --- /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 b956a8f95..2b7a1ce1d 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 3a2e95bfc..191c4ec54 100644 --- a/scripts/integration-progress-model.ts +++ b/scripts/integration-progress-model.ts @@ -154,6 +154,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 d1af133cd..486bf8c69 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 ca7b579e8..887ebc407 100644 --- a/src/cli/parser/__tests__/cli-help-command-usage.test.ts +++ b/src/cli/parser/__tests__/cli-help-command-usage.test.ts @@ -237,6 +237,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 b1334335b..b32ee7bff 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 8a2e75160..5526eb7e8 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. A duration wait, wait stable, or wait @ref 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 a126185ee..34e5208f2 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 b255c3d7c..45bb82d94 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 548e3df9f..05db325a5 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 631148f0d..ae3cd69df 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 bd501a114..486858084 100644 --- a/src/commands/interaction/interactions.test.ts +++ b/src/commands/interaction/interactions.test.ts @@ -19,3 +19,14 @@ test('swipe writes only typed daemon input', () => { pattern: 'ping-pong', }); }); + +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 f2a785fc7..8c0919c84 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 b5daebd55..437c35007 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 330a8494b..f769afc2c 100644 --- a/src/contracts/cli-flags.ts +++ b/src/contracts/cli-flags.ts @@ -92,6 +92,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 85b8d53b8..92849d8ad 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 fff7f7ba5..0d1e1e557 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 96968d836..1a422b9ac 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 000000000..b7e0c76e8 --- /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 e1bde84e4..bbda1e4a6 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 afb1734e6..badb9dca2 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 978059f72..061b73880 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 45cf7f8c6..cb752eae2 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 8478fbf03..6e939fcfd 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 000000000..10ca40a38 --- /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 0a5e27e1c..256fa8a60 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 7428b3ca7..131790355 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 ee5a68c7e..12914c941 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -87,3 +87,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 ff980e93fd961abaf5ca3f13fdb3d896f6deaa33 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/4] 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 9ece01064..a80928a6c 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 92849d8ad..f354efe6d 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 f34e8f17b..0a50c2489 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 000000000..953fbb3d2 --- /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 1a422b9ac..a90bf823d 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 000000000..13a68d18b --- /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 061b73880..8bae3e1c5 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 cb752eae2..d337403c9 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 10ca40a38..febd71740 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 12914c941..6f192e735 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 { createAndroidSettingsWorld } from './android-world.ts'; @@ -89,15 +90,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, @@ -152,15 +166,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 867061c4f..0113dc254 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 d8f0071a4ebca203c26c93ec0decfd39b058db4c 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/4] 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 a80928a6c..a88ab108b 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 f354efe6d..7046c1019 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 000000000..a29103ae3 --- /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 13a68d18b..5e327b854 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 6f192e735..7dbabb02b 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -138,9 +138,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 8af3931146d0749c925dffe8290e9ba26ee9f050 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/4] 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 | 15 +- 5 files changed, 257 insertions(+), 86 deletions(-) diff --git a/docs/adr/0017-parameterized-recorded-inputs.md b/docs/adr/0017-parameterized-recorded-inputs.md index a88ab108b..a3881815c 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 7046c1019..8612a64c9 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 a29103ae3..2ee0d66fc 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 5e327b854..0197f4cd4 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 7dbabb02b..67cd37b75 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -5,7 +5,7 @@ import path from 'node:path'; import { emitDiagnostic } from '../../../src/utils/diagnostics.ts'; import { test } from 'vitest'; import { assertRpcError, assertRpcOk } from './assertions.ts'; -import { createAndroidSettingsWorld } from './android-world.ts'; +import { androidSettingsXml, createAndroidSettingsWorld } from './android-world.ts'; import { withProviderScenarioResource } from './harness.ts'; test('provider route publishes and replays an open-to-destination script with a live handoff', async () => { @@ -91,16 +91,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-')); @@ -146,7 +157,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,