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..a3881815c --- /dev/null +++ b/docs/adr/0017-parameterized-recorded-inputs.md @@ -0,0 +1,91 @@ +# 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, 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 + 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. + +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. +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 + +- 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..8612a64c9 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -86,3 +86,115 @@ 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}`, + selector: 'id="password"', + selectorChain: ['id="password"', `value="${password}" editable=true`], + settle: { + diff: { + lines: [{ kind: 'added', text: `value=${password}` }], + }, + }, + }, + targetEvidence: { + role: 'textinput', + label: password, + ancestry: [{ role: 'form', label: 'Credentials' }], + 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 18 chars' }, + }); + + 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(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 structural selector provenance', () => { + 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: 'Alre${LETTER}dy s${LETTER}fe', + 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/__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/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__/interaction-common.test.ts b/src/daemon/handlers/__tests__/interaction-common.test.ts new file mode 100644 index 000000000..2ee0d66fc --- /dev/null +++ b/src/daemon/handlers/__tests__/interaction-common.test.ts @@ -0,0 +1,181 @@ +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}'; + const sessionStore = makeSessionStore(); + const session = makeIosSession('parameterized-fill'); + session.recordSession = true; + sessionStore.set(session.name, session); + const payload = { + text: secret, + message: `prefix${secret}suffix`, + refLabel: `prefix${secret}suffix`, + selector: 'id="password"', + selectorChain: [ + 'id="password"', + `value="prefix${secret}suffix" editable=true`, + `label="${secret}4"`, + ], + backendEcho: { + id: secret, + status: `accepted${secret}suffix`, + nested: { kind: secret }, + [secret]: true, + [`prefix${secret}suffix`]: { + [`${secret}4`]: true, + }, + }, + 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=prefix${secret}suffix` }], + }, + hint: `Observed ${secret}4`, + [`echo${secret}key`]: true, + backendEcho: { + id: secret, + [`key${secret}`]: `value${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: `prefix${placeholder}suffix`, + refLabel: `prefix${placeholder}suffix`, + selector: 'id="password"', + selectorChain: ['id="password"'], + backendEcho: { + id: placeholder, + status: `accepted${placeholder}suffix`, + nested: { kind: placeholder }, + [placeholder]: true, + [`prefix${placeholder}suffix`]: { + [`${placeholder}4`]: true, + }, + }, + settle: { + diff: { + lines: [{ kind: 'added', text: `TextField value=prefix${placeholder}suffix` }], + }, + hint: `Observed ${placeholder}4`, + [`echo${placeholder}key`]: true, + backendEcho: { + id: placeholder, + [`key${placeholder}`]: `value${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); +}); + +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/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 96968d836..a90bf823d 100644 --- a/src/daemon/handlers/interaction-common.ts +++ b/src/daemon/handlers/interaction-common.ts @@ -14,6 +14,9 @@ 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 } from '../../replay/recorded-input.ts'; +import { parameterizeRecordedFillPayload } from '../parameterized-recorded-fill.ts'; export type ContextFromFlags = ( flags: CommandFlags | undefined, @@ -61,13 +64,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 +99,34 @@ 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, + }); + const placeholder = recordedInputPlaceholder(params.flags.recordAs); + return [ + parameterizeRecordedFillPayload(params.result, literal, placeholder), + parameterizeRecordedFillPayload(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/parameterized-recorded-fill.ts b/src/daemon/parameterized-recorded-fill.ts new file mode 100644 index 000000000..0197f4cd4 --- /dev/null +++ b/src/daemon/parameterized-recorded-fill.ts @@ -0,0 +1,255 @@ +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 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 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_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', +]); + +/** + * 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 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, +>(payload: TPayload, literal: string, placeholder: string): TPayload { + if (!payload) return payload; + const selectorChain = readStringArray(payload.selectorChain); + return { + ...parameterizeBackendOutput(payload, literal, placeholder), + ...(typeof payload.text === 'string' ? { text: 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' && + 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]) => { + const outputKey = STRUCTURAL_ROOT_OUTPUT_KEYS.has(key) + ? key + : parameterizeSensitiveString(key, literal, placeholder); + return [outputKey, 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 === 'selectorChain' && isStringArray(value)) { + return filterSensitiveSelectorCandidates(value, literal); + } + return parameterizeBackendOutputValue( + value, + literal, + placeholder, + STRUCTURED_OUTPUT_KEYS.has(key) ? key : undefined, + ); +} + +function parameterizeBackendOutputValue( + value: unknown, + literal: string, + placeholder: string, + structuralPath: string | undefined, +): unknown { + if (typeof value === 'string') { + 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, 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]) => { + const isStructural = structuralKeys?.has(nestedKey) === true; + const outputKey = isStructural + ? nestedKey + : parameterizeSensitiveString(nestedKey, literal, placeholder); + return [ + outputKey, + parameterizeBackendOutputValue( + entry, + literal, + placeholder, + isStructural ? `${structuralPath}.${nestedKey}` : undefined, + ), + ]; + }), + ); +} + +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; + return value.replaceAll(literal, placeholder); +} + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((entry) => typeof entry === 'string'); +} + +function readStringArray(value: unknown): string[] | undefined { + return isStringArray(value) ? value : undefined; +} diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index 978059f72..8bae3e1c5 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 { @@ -248,6 +250,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { if (!timingSafeStringEqual(req.token, token)) { return unauthorizedResponse(); } + registerParameterizedFillDiagnosticValue(req); let childScope: RequestExecutionScope | undefined; try { @@ -301,6 +304,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..d337403c9 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -4,6 +4,15 @@ 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, + validateRecordedInputVariableName, +} from '../replay/recorded-input.ts'; +import { + parameterizeRecordedFillPayload, + parameterizeRecordedFillTargetEvidence, +} from './parameterized-recorded-fill.ts'; export type RecordActionEntry = { command: string; @@ -84,14 +93,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 +115,52 @@ 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}`. 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; + const variableName = validateRecordedInputVariableName(entry.flags.recordAs); + const placeholder = recordedInputPlaceholder(variableName); + const literal = inferFillText({ + ts: 0, + command: entry.command, + positionals: entry.positionals, + flags: entry.flags, + }); + return { + ...entry, + positionals: replaceFillText(entry.positionals, placeholder), + result: parameterizeRecordedFillPayload(entry.result, literal, placeholder), + targetEvidence: parameterizeRecordedFillTargetEvidence( + 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..febd71740 --- /dev/null +++ b/src/replay/recorded-input.ts @@ -0,0 +1,31 @@ +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]; +} 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..67cd37b75 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -2,9 +2,10 @@ 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'; +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 () => { @@ -87,3 +88,141 @@ 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'; + 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-')); + const scriptPath = path.join(root, 'parameterized-search.ad'); + const client = world.daemon.client(); + try { + const opened = 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', + 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, /prefix\$\{SEARCH_TERM\}suffix/); + 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}`], + 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, 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 }); + 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); 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; };