Skip to content

Commit ff980e9

Browse files
thymikeethymikee
committed
fix: harden parameterized replay recording
1 parent d98628b commit ff980e9

11 files changed

Lines changed: 261 additions & 75 deletions

‎docs/adr/0017-parameterized-recorded-inputs.md‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,15 @@ The literal exists only on the live request path long enough to execute the inte
5252
1. The request scope registers it as an explicit diagnostics secret before platform work. This is in
5353
addition to key-name redaction, because an opaque value need not look secret.
5454
2. The platform interaction receives the literal.
55-
3. Before response/event/recording retention, value-bearing interaction-result fields replace every
56-
occurrence of the literal with `${VAR}`. Stable selector/ref provenance is preserved byte-for-byte;
57-
post-fill selector-chain candidates derived from the new value are dropped rather than serialized.
55+
3. Before response/event/recording retention, the fill result's semantic `text` field and an exact
56+
post-fill `refLabel` become `${VAR}`. Stable selector/ref provenance and unrelated strings are
57+
preserved byte-for-byte; post-fill selector-chain candidates are dropped only when a parsed
58+
`text`, `label`, or `value` term exactly equals the supplied literal.
5859
4. At `recordActionEntry`, the recorder reconstructs the fill's literal positional, writes only the
59-
placeholder into `SessionAction`, and scrubs value-bearing result strings again. ADR 0012
60-
`target-v1` evidence keeps its structure, but any matching string is parameterized as well so a
61-
repeated fill cannot leak through a pre-action accessibility label. The `recordAs` control flag
62-
itself is not serialized into the script.
60+
placeholder into `SessionAction`, and parameterizes the semantic result field again. ADR 0012
61+
`target-v1` evidence keeps its structure; exact value-bearing accessibility labels become the
62+
placeholder without rewriting identity fragments that merely contain the same characters. The
63+
`recordAs` control flag itself is not serialized into the script.
6364
5. The existing ADR 0012/0016 writer receives an already-parameterized action. Its selector provenance,
6465
portability checks, same-directory temp write, and atomic publication algorithm are unchanged.
6566

@@ -71,9 +72,10 @@ the literal once this pre-publication boundary has run.
7172
Missing variables retain replay's existing fail-loud behavior: `${VAR}` resolution throws before the
7273
corresponding action is invoked. When replay dispatches an authored fill whose complete text token is a
7374
`${VAR}` placeholder, its daemon-only provenance channel derives `recordAs=VAR` for that live request.
74-
If the replay is itself being recorded or repaired, the recorder therefore retains the original
75-
placeholder rather than serializing the expanded value. Embedded interpolation remains ordinary script
76-
input; safe authoring emits one complete placeholder token.
75+
This derivation uses the unresolved source action, independently of whether the resolved value is empty
76+
or whitespace. If the replay is itself being recorded or repaired, the recorder therefore retains the
77+
original placeholder rather than serializing the expanded value. Embedded interpolation remains
78+
ordinary script input; safe authoring emits one complete placeholder token.
7779

7880
## Consequences
7981

‎src/daemon/__tests__/session-action-recorder.test.ts‎

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@ test('parameterized fills keep literals out of recording state with deterministi
9797
flags: { recordAs: 'PASSWORD' },
9898
result: {
9999
text: password,
100-
message: `Filled ${password}`,
100+
message: 'Filled 21 chars',
101+
selector: 'id="password"',
101102
selectorChain: ['id="password"', `value="${password}" editable=true`],
102-
settle: { diff: `value=${password}` },
103103
},
104104
targetEvidence: {
105105
role: 'textinput',
106-
label: `Current value ${password}`,
107-
ancestry: [{ role: 'form', label: `Credential ${password}` }],
106+
label: password,
107+
ancestry: [{ role: 'form', label: 'Credentials' }],
108108
sibling: 0,
109109
viewportOrder: 0,
110110
verification: 'verified' as const,
@@ -117,7 +117,7 @@ test('parameterized fills keep literals out of recording state with deterministi
117117
command: 'fill',
118118
positionals: ['id="token"', token],
119119
flags: { recordAs: 'API_TOKEN' },
120-
result: { text: token, message: `Filled ${token}` },
120+
result: { text: token, message: 'Filled 18 chars' },
121121
});
122122

123123
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
128128
const serialized = JSON.stringify(session.actions);
129129
expect(serialized).not.toContain(password);
130130
expect(serialized).not.toContain(token);
131-
expect(serialized).toContain('value=${PASSWORD}');
132-
expect(session.actions[0]?.targetEvidence?.label).toBe('Current value ${PASSWORD}');
133-
expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credential ${PASSWORD}');
131+
expect(session.actions[0]?.result?.text).toBe('${PASSWORD}');
132+
expect(session.actions[0]?.result?.selector).toBe('id="password"');
133+
expect(session.actions[0]?.targetEvidence?.label).toBe('${PASSWORD}');
134+
expect(session.actions[0]?.targetEvidence?.ancestry[0]?.label).toBe('Credentials');
134135
expect(session.actions[0]?.result?.selectorChain).toEqual(['id="password"']);
135136
});
137+
138+
test('a one-character parameterized fill preserves unrelated response and selector text', () => {
139+
const session = makeIosSession('default');
140+
recordActionEntry(session, {
141+
command: 'fill',
142+
positionals: ['id="password"', 'a'],
143+
flags: { recordAs: 'LETTER' },
144+
result: {
145+
text: 'a',
146+
message: 'Already safe',
147+
selector: 'id="password"',
148+
selectorChain: ['id="password"', 'label="Account"', 'value="a" editable=true'],
149+
},
150+
targetEvidence: {
151+
role: 'textinput',
152+
label: 'Password',
153+
ancestry: [{ role: 'form', label: 'Credentials area' }],
154+
sibling: 0,
155+
viewportOrder: 0,
156+
verification: 'verified',
157+
},
158+
});
159+
160+
expect(session.actions[0]).toMatchObject({
161+
positionals: ['id="password"', '${LETTER}'],
162+
result: {
163+
text: '${LETTER}',
164+
message: 'Already safe',
165+
selector: 'id="password"',
166+
selectorChain: ['id="password"', 'label="Account"'],
167+
},
168+
targetEvidence: {
169+
label: 'Password',
170+
ancestry: [{ role: 'form', label: 'Credentials area' }],
171+
},
172+
});
173+
});
174+
175+
test.each(['', ' '])(
176+
'parameterized fills preserve the placeholder when the resolved value is %j',
177+
(value) => {
178+
const session = makeIosSession('default');
179+
recordActionEntry(session, {
180+
command: 'fill',
181+
positionals: ['id="password"', value],
182+
flags: { recordAs: 'PASSWORD' },
183+
result: { text: value },
184+
});
185+
186+
expect(session.actions[0]?.positionals).toEqual(['id="password"', '${PASSWORD}']);
187+
expect(session.actions[0]?.result?.text).toBe('${PASSWORD}');
188+
},
189+
);

‎src/daemon/action-utils.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,20 @@ import type { SessionAction } from './types.ts';
22

33
export function inferFillText(action: SessionAction): string {
44
const resultText = action.result?.text;
5-
if (typeof resultText === 'string' && resultText.trim().length > 0) {
6-
return resultText;
7-
}
5+
if (typeof resultText === 'string') return resultText;
86
const positionals = action.positionals ?? [];
97
if (positionals.length === 0) return '';
108
const first = positionals[0];
119
if (first?.startsWith('@')) {
12-
if (positionals.length >= 3) return positionals.slice(2).join(' ').trim();
13-
return positionals.slice(1).join(' ').trim();
10+
if (positionals.length >= 3) return positionals.slice(2).join(' ');
11+
return positionals.slice(1).join(' ');
1412
}
1513
if (
1614
positionals.length >= 3 &&
1715
!Number.isNaN(Number(positionals[0])) &&
1816
!Number.isNaN(Number(positionals[1]))
1917
) {
20-
return positionals.slice(2).join(' ').trim();
18+
return positionals.slice(2).join(' ');
2119
}
22-
return positionals.slice(1).join(' ').trim();
20+
return positionals.slice(1).join(' ');
2321
}

‎src/daemon/handlers/__tests__/session-replay-action-runtime.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { expect, test } from 'vitest';
2+
import { makeIosSession } from '../../../__tests__/test-utils/index.ts';
3+
import { recordActionEntry } from '../../session-action-recorder.ts';
4+
import type { DaemonRequest, SessionAction } from '../../types.ts';
5+
import { invokeReplayAction } from '../session-replay-action-runtime.ts';
6+
7+
const REPLAY_REQUEST: DaemonRequest = {
8+
token: 'token',
9+
session: 'default',
10+
command: 'replay',
11+
positionals: ['login.ad'],
12+
flags: {},
13+
};
14+
15+
test.each(['', ' '])(
16+
'replay keeps source placeholder provenance when PASSWORD resolves to %j',
17+
async (value) => {
18+
const session = makeIosSession('default');
19+
const sourceAction: SessionAction = {
20+
ts: 0,
21+
command: 'fill',
22+
positionals: ['id="password"', '${PASSWORD}'],
23+
flags: {},
24+
};
25+
const response = await invokeReplayAction({
26+
req: REPLAY_REQUEST,
27+
sessionName: 'default',
28+
action: sourceAction,
29+
scope: { values: { PASSWORD: value } },
30+
filePath: 'login.ad',
31+
line: 1,
32+
step: 1,
33+
invoke: async (request) => {
34+
recordActionEntry(session, {
35+
command: request.command,
36+
positionals: request.positionals ?? [],
37+
flags: request.flags ?? {},
38+
result: { text: request.positionals?.at(-1) },
39+
});
40+
return { ok: true, data: {} };
41+
},
42+
});
43+
44+
expect(response.ok).toBe(true);
45+
expect(session.actions[0]?.positionals).toEqual(['id="password"', '${PASSWORD}']);
46+
expect(session.actions[0]?.result?.text).toBe('${PASSWORD}');
47+
},
48+
);

‎src/daemon/handlers/interaction-common.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@ import {
1515
import { markPostGestureStabilization } from '../post-gesture-stabilization.ts';
1616
import { computeTargetEvidence, type RecordedTargetCapture } from '../session-target-evidence.ts';
1717
import { inferFillText } from '../action-utils.ts';
18-
import {
19-
recordedInputPlaceholder,
20-
replaceRecordedInputLiteral,
21-
} from '../../replay/recorded-input.ts';
18+
import { recordedInputPlaceholder } from '../../replay/recorded-input.ts';
19+
import { parameterizeRecordedFillPayload } from '../parameterized-recorded-fill.ts';
2220

2321
export type ContextFromFlags = (
2422
flags: CommandFlags | undefined,
@@ -126,10 +124,9 @@ function parameterizeFillPayloads(params: {
126124
flags: params.flags,
127125
result: params.result,
128126
});
129-
if (!literal) return [params.result, params.responseData];
130127
const placeholder = recordedInputPlaceholder(params.flags.recordAs);
131128
return [
132-
replaceRecordedInputLiteral(params.result, literal, placeholder),
133-
replaceRecordedInputLiteral(params.responseData, literal, placeholder),
129+
parameterizeRecordedFillPayload(params.result, literal, placeholder),
130+
parameterizeRecordedFillPayload(params.responseData, literal, placeholder),
134131
];
135132
}

‎src/daemon/parameterized-recorded-fill.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import type { TargetAnnotationV1 } from '../replay/target-identity.ts';
2+
import { tryParseSelectorChain } from '../selectors/parse.ts';
3+
4+
const VALUE_BEARING_SELECTOR_KEYS = new Set(['text', 'label', 'value']);
5+
6+
/**
7+
* Parameterize only fields whose contract says they carry the fill value.
8+
* Stable identity/provenance strings stay byte-for-byte unchanged; a derived
9+
* selector candidate is dropped only when a parsed text/label/value term is
10+
* exactly the supplied literal.
11+
*/
12+
export function parameterizeRecordedFillPayload<
13+
TPayload extends Record<string, unknown> | undefined,
14+
>(payload: TPayload, literal: string, placeholder: string): TPayload {
15+
if (!payload) return payload;
16+
const selectorChain = readStringArray(payload.selectorChain);
17+
return {
18+
...payload,
19+
...(typeof payload.text === 'string' ? { text: placeholder } : {}),
20+
...(payload.refLabel === literal ? { refLabel: placeholder } : {}),
21+
...(selectorChain
22+
? {
23+
selectorChain: selectorChain.filter(
24+
(candidate) => !selectorCandidateCarriesFillValue(candidate, literal),
25+
),
26+
}
27+
: {}),
28+
} as TPayload;
29+
}
30+
31+
/** Parameterize exact accessibility labels without rewriting identity fragments. */
32+
export function parameterizeRecordedFillTargetEvidence(
33+
evidence: TargetAnnotationV1 | undefined,
34+
literal: string,
35+
placeholder: string,
36+
): TargetAnnotationV1 | undefined {
37+
if (!evidence) return evidence;
38+
return {
39+
...evidence,
40+
...(evidence.label === literal ? { label: placeholder } : {}),
41+
ancestry: evidence.ancestry.map((entry) => ({
42+
...entry,
43+
...(entry.label === literal ? { label: placeholder } : {}),
44+
})),
45+
...(evidence.scrollRegion
46+
? {
47+
scrollRegion: {
48+
...evidence.scrollRegion,
49+
...(evidence.scrollRegion.label === literal ? { label: placeholder } : {}),
50+
},
51+
}
52+
: {}),
53+
};
54+
}
55+
56+
function selectorCandidateCarriesFillValue(candidate: string, literal: string): boolean {
57+
const parsed = tryParseSelectorChain(candidate);
58+
return (
59+
parsed?.selectors.some((selector) =>
60+
selector.terms.some(
61+
(term) =>
62+
VALUE_BEARING_SELECTOR_KEYS.has(term.key) &&
63+
typeof term.value === 'string' &&
64+
term.value === literal,
65+
),
66+
) ?? false
67+
);
68+
}
69+
70+
function readStringArray(value: unknown): string[] | undefined {
71+
return Array.isArray(value) && value.every((entry) => typeof entry === 'string')
72+
? value
73+
: undefined;
74+
}

‎src/daemon/request-router.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn {
250250
if (!timingSafeStringEqual(req.token, token)) {
251251
return unauthorizedResponse();
252252
}
253+
registerParameterizedFillDiagnosticValue(req);
253254

254255
let childScope: RequestExecutionScope | undefined;
255256
try {

‎src/daemon/session-action-recorder.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@ import type { TargetAnnotationV1 } from '../replay/target-identity.ts';
77
import { inferFillText } from './action-utils.ts';
88
import {
99
recordedInputPlaceholder,
10-
replaceRecordedInputLiteral,
1110
validateRecordedInputVariableName,
1211
} from '../replay/recorded-input.ts';
12+
import {
13+
parameterizeRecordedFillPayload,
14+
parameterizeRecordedFillTargetEvidence,
15+
} from './parameterized-recorded-fill.ts';
1316

1417
export type RecordActionEntry = {
1518
command: string;
@@ -115,12 +118,11 @@ export function recordActionEntry(
115118
/**
116119
* #1348: the recorder is the first durable boundary. The live request keeps
117120
* the literal fill value through device execution, but the SessionAction gets
118-
* only `${VAR}`. Value-bearing result strings are scrubbed at the same boundary
119-
* so backend echoes or settled diffs cannot smuggle the literal into recording
120-
* state, the event display, or the eventual publication temp/target file;
121-
* ADR 0012 identity evidence keeps its structure but is scrubbed too, covering
122-
* repeated fills where the pre-action accessibility label already echoes the
123-
* same sensitive value.
121+
* only `${VAR}`. The result's semantic fill-value field is parameterized at
122+
* the same boundary, while selector/ref provenance is preserved byte-for-byte.
123+
* Exact value-bearing selector candidates are omitted, and exact accessibility
124+
* value labels in ADR 0012 evidence are parameterized without rewriting
125+
* unrelated identity fragments.
124126
*/
125127
function parameterizeRecordedFill(entry: RecordActionEntry): RecordActionEntry {
126128
if (entry.command !== 'fill' || typeof entry.flags.recordAs !== 'string') return entry;
@@ -132,12 +134,15 @@ function parameterizeRecordedFill(entry: RecordActionEntry): RecordActionEntry {
132134
positionals: entry.positionals,
133135
flags: entry.flags,
134136
});
135-
if (!literal) return entry;
136137
return {
137138
...entry,
138139
positionals: replaceFillText(entry.positionals, placeholder),
139-
result: replaceRecordedInputLiteral(entry.result, literal, placeholder),
140-
targetEvidence: replaceRecordedInputLiteral(entry.targetEvidence, literal, placeholder),
140+
result: parameterizeRecordedFillPayload(entry.result, literal, placeholder),
141+
targetEvidence: parameterizeRecordedFillTargetEvidence(
142+
entry.targetEvidence,
143+
literal,
144+
placeholder,
145+
),
141146
};
142147
}
143148

‎src/replay/recorded-input.ts‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,3 @@ export function readRecordedInputVariableName(value: string): string | undefined
2929
if (!match?.[1] || match[1].startsWith('AD_')) return undefined;
3030
return match[1];
3131
}
32-
33-
export function replaceRecordedInputLiteral<T>(value: T, literal: string, placeholder: string): T {
34-
if (typeof value === 'string') return value.replaceAll(literal, placeholder) as T;
35-
if (Array.isArray(value)) {
36-
return value.map((entry) => replaceRecordedInputLiteral(entry, literal, placeholder)) as T;
37-
}
38-
if (!value || typeof value !== 'object') return value;
39-
return Object.fromEntries(
40-
Object.entries(value).map(([key, entry]) => {
41-
if (key === 'ref') return [key, entry];
42-
if (key === 'selector' && typeof entry === 'string') {
43-
return [key, entry.includes(literal) ? undefined : entry];
44-
}
45-
if (key === 'selectorChain' && Array.isArray(entry)) {
46-
return [
47-
key,
48-
entry.filter(
49-
(candidate) => typeof candidate !== 'string' || !candidate.includes(literal),
50-
),
51-
];
52-
}
53-
return [key, replaceRecordedInputLiteral(entry, literal, placeholder)];
54-
}),
55-
) as T;
56-
}

0 commit comments

Comments
 (0)