Skip to content

Commit 7782a56

Browse files
waleedlatif1waleedlatif1
committed
fix(desktop): close three credential-disclosure gaps in agent page functions
Closed shadow roots. activeElementSecrecy is the only gate the driver consults before dispatching trusted CDP keystrokes, and it descended focus solely through `active.shadowRoot` — null by design for a closed root, while focus inside one retargets to the host, whose tagName is never INPUT. So a password field in a closed root reported 'safe', and Tab crosses closed boundaries natively. Now treated like a cross-origin frame. Detected by focusability rather than by tag: attachShadow accepts plain div/span/section as well as custom elements, so a tag test would miss half of them, whereas an element that is not focusable in its own right cannot be activeElement unless focus was retargeted out of a shadow tree. Multi-token autocomplete. isSecretField compared the whole attribute against 'current-password'/'new-password', but the spec allows space-separated detail tokens and WebAuthn recommends `current-password webauthn`. Those values fell through on exactly the type=text credential fields where autocomplete is the only signal. Now split into tokens, in all seven copies — the duplication is required by the `String(fn)` serialization contract, so consolidating was not an option. OTP and payment values. Deliberately NOT added to isSecretField: that helper also gates keystrokes, so folding them in would have stopped the agent completing a checkout or an OTP prompt — work it is legitimately asked to do. A separate predicate withholds only the value at the two emission sites, and the field is still reported with its real tag so the agent can fill it.
1 parent 9b8df93 commit 7782a56

2 files changed

Lines changed: 229 additions & 8 deletions

File tree

‎apps/desktop/src/main/browser-agent/page-functions.test.ts‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,20 @@ describe('secret-field detection', () => {
140140
],
141141
['new-password field', '<input type="text" autocomplete="new-password" />'],
142142
['uppercase autocomplete token', '<input type="text" autocomplete="Current-Password" />'],
143+
// The spec allows space-separated detail tokens and WebAuthn recommends
144+
// this exact value, so whole-string equality missed it.
145+
[
146+
'WebAuthn multi-token autocomplete',
147+
'<input type="text" autocomplete="current-password webauthn" />',
148+
],
149+
[
150+
'section-scoped autocomplete',
151+
'<input type="text" autocomplete="section-login current-password" />',
152+
],
153+
[
154+
'multi-token new-password with surrounding whitespace',
155+
'<input type="text" autocomplete=" new-password webauthn " />',
156+
],
143157
]
144158

145159
it.each(secretCases)('clickElement refuses a %s', (_label, html) => {
@@ -276,6 +290,24 @@ describe('collectSnapshot', () => {
276290
expect(outline).not.toContain('value=')
277291
})
278292

293+
it.each([
294+
['a one-time code', 'one-time-code', '123456'],
295+
['a card number', 'cc-number', '4111111111111111'],
296+
['a card security code', 'cc-csc', '737'],
297+
['a card expiry', 'cc-exp', '12/29'],
298+
])('withholds the value of %s while still listing the field', (_label, token, value) => {
299+
document.body.innerHTML = `<input type="text" autocomplete="${token}" value="${value}" aria-label="Field" />`
300+
visible(document.querySelector('input') as HTMLInputElement)
301+
302+
const outline = outlineOf(collectSnapshot())
303+
304+
// Not reported as a password-field: the agent must still be able to fill
305+
// these, it just never learns what is already there.
306+
expect(outline).not.toContain('password-field')
307+
expect(outline).not.toContain(value)
308+
expect(outline).toContain('value-withheld')
309+
})
310+
279311
it('withholds the value of a revealed password field', () => {
280312
document.body.innerHTML =
281313
'<input type="text" autocomplete="current-password" value="hunter2" aria-label="Password" />'
@@ -317,6 +349,23 @@ describe('readActiveElementState', () => {
317349
expect(readActiveElementState()).toMatchObject({ redacted: true, valuePreview: '' })
318350
})
319351

352+
it.each([
353+
['a one-time code', 'one-time-code', '123456'],
354+
['a card number', 'cc-number', '4111111111111111'],
355+
['a card security code', 'cc-csc', '737'],
356+
])('withholds %s on readback but keeps the real tag', (_label, token, value) => {
357+
document.body.innerHTML = `<input type="text" autocomplete="${token}" value="${value}" />`
358+
setActiveElement(document, document.querySelector('input'))
359+
360+
expect(readActiveElementState()).toEqual({
361+
activeElement: 'input',
362+
selectedChars: 0,
363+
valueLength: 0,
364+
valuePreview: '',
365+
redacted: true,
366+
})
367+
})
368+
320369
it('reports ordinary fields in full', () => {
321370
document.body.innerHTML = '<input type="text" value="tokyo" />'
322371
setActiveElement(document, document.querySelector('input'))
@@ -386,6 +435,46 @@ describe('activeElementSecrecy', () => {
386435
expect(activeElementSecrecy()).toBe('opaque')
387436
})
388437

438+
it('reports opaque for a password field inside a CLOSED shadow root', () => {
439+
const host = document.createElement('div')
440+
document.body.append(host)
441+
const shadow = host.attachShadow({ mode: 'closed' })
442+
shadow.innerHTML = '<input type="password" />'
443+
// Focus inside a closed root retargets to the host and `shadowRoot` reads
444+
// null, which is exactly what the browser reports and what made this 'safe'.
445+
setActiveElement(document, host)
446+
447+
expect(host.shadowRoot).toBeNull()
448+
expect(activeElementSecrecy()).toBe('opaque')
449+
})
450+
451+
it('reports opaque for a closed shadow root on a custom element', () => {
452+
const host = document.createElement('my-login')
453+
document.body.append(host)
454+
host.attachShadow({ mode: 'closed' }).innerHTML = '<input autocomplete="new-password" />'
455+
setActiveElement(document, host)
456+
457+
expect(activeElementSecrecy()).toBe('opaque')
458+
})
459+
460+
it('still reports safe for a focused element that is focusable in its own right', () => {
461+
// The false-positive guard: a div the page made focusable is focused
462+
// itself, not hiding a shadow tree, so keystrokes are not refused.
463+
document.body.innerHTML = '<div tabindex="0">menu</div>'
464+
setActiveElement(document, document.querySelector('div'))
465+
466+
expect(activeElementSecrecy()).toBe('safe')
467+
})
468+
469+
it('still reports safe for a focused contenteditable', () => {
470+
document.body.innerHTML = '<div contenteditable="true">note</div>'
471+
const editable = document.querySelector('div') as HTMLElement
472+
Object.defineProperty(editable, 'isContentEditable', { get: () => true })
473+
setActiveElement(document, editable)
474+
475+
expect(activeElementSecrecy()).toBe('safe')
476+
})
477+
389478
it('descends into a same-origin frame instead of calling it opaque', () => {
390479
const frame = document.createElement('iframe')
391480
document.body.append(frame)

‎apps/desktop/src/main/browser-agent/page-functions.ts‎

Lines changed: 140 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,39 @@ export function collectSnapshot(): unknown {
104104
// A reveal toggle flips the field to type="text" without making its
105105
// contents any less secret, and some forms never use type="password" at
106106
// all. The autocomplete token is the page's own declaration either way.
107+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
108+
// `current-password webauthn`, so whole-string equality missed real values
109+
// on exactly the type=text credential fields where autocomplete is the
110+
// only signal there is.
107111
const hint = String(el.getAttribute('autocomplete') || '').toLowerCase()
108-
return hint === 'current-password' || hint === 'new-password'
112+
return hint
113+
.split(/\s+/)
114+
.some((token) => token === 'current-password' || token === 'new-password')
115+
}
116+
117+
/**
118+
* Fields whose value is as sensitive as a password but which the agent must
119+
* still be able to FILL: one-time codes and payment details.
120+
*
121+
* Deliberately separate from isSecretField. That one also gates keystrokes
122+
* (activeElementSecrecy feeds the driver's press-key guard), so folding these
123+
* tokens into it would stop the agent completing a checkout or an OTP prompt —
124+
* work it is legitimately asked to do. Only the value is withheld here.
125+
*/
126+
const isSensitiveValueField = (el: Element | null): boolean => {
127+
if (!el || el.tagName !== 'INPUT') return false
128+
const hint = String(el.getAttribute('autocomplete') || '').toLowerCase()
129+
return hint
130+
.split(/\s+/)
131+
.some(
132+
(token) =>
133+
token === 'one-time-code' ||
134+
token === 'cc-number' ||
135+
token === 'cc-csc' ||
136+
token === 'cc-exp' ||
137+
token === 'cc-exp-month' ||
138+
token === 'cc-exp-year'
139+
)
109140
}
110141

111142
const roleFor = (el: Element): string => {
@@ -170,7 +201,8 @@ export function collectSnapshot(): unknown {
170201
// like any other. Redaction above is realm-safe and runs first, so
171202
// widening this cannot expose a credential field.
172203
const value = (el as HTMLInputElement).value
173-
if (value) parts.push(`value="${cut(String(value), 120)}"`)
204+
if (value && isSensitiveValueField(el)) parts.push('value-withheld')
205+
else if (value) parts.push(`value="${cut(String(value), 120)}"`)
174206
}
175207
if (el.tagName === 'A') {
176208
const href = el.getAttribute('href')
@@ -271,8 +303,14 @@ export function clickElement(id: number): unknown {
271303
const isSecretField = (node: Element | null): boolean => {
272304
if (!node || node.tagName !== 'INPUT') return false
273305
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
306+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
307+
// `current-password webauthn`, so whole-string equality missed real values
308+
// on exactly the type=text credential fields where autocomplete is the
309+
// only signal there is.
274310
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
275-
return hint === 'current-password' || hint === 'new-password'
311+
return hint
312+
.split(/\s+/)
313+
.some((token) => token === 'current-password' || token === 'new-password')
276314
}
277315

278316
const el = (window.__simAgentElements || [])[id]
@@ -321,8 +359,14 @@ export function focusElementForTyping(id: number): unknown {
321359
const isSecretField = (node: Element | null): boolean => {
322360
if (!node || node.tagName !== 'INPUT') return false
323361
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
362+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
363+
// `current-password webauthn`, so whole-string equality missed real values
364+
// on exactly the type=text credential fields where autocomplete is the
365+
// only signal there is.
324366
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
325-
return hint === 'current-password' || hint === 'new-password'
367+
return hint
368+
.split(/\s+/)
369+
.some((token) => token === 'current-password' || token === 'new-password')
326370
}
327371

328372
const el = (window.__simAgentElements || [])[id]
@@ -372,8 +416,39 @@ export function readActiveElementState(): unknown {
372416
const isSecretField = (node: Element | null): boolean => {
373417
if (!node || node.tagName !== 'INPUT') return false
374418
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
419+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
420+
// `current-password webauthn`, so whole-string equality missed real values
421+
// on exactly the type=text credential fields where autocomplete is the
422+
// only signal there is.
423+
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
424+
return hint
425+
.split(/\s+/)
426+
.some((token) => token === 'current-password' || token === 'new-password')
427+
}
428+
429+
/**
430+
* Fields whose value is as sensitive as a password but which the agent must
431+
* still be able to FILL: one-time codes and payment details.
432+
*
433+
* Deliberately separate from isSecretField. That one also gates keystrokes
434+
* (activeElementSecrecy feeds the driver's press-key guard), so folding these
435+
* tokens into it would stop the agent completing a checkout or an OTP prompt —
436+
* work it is legitimately asked to do. Only the value is withheld here.
437+
*/
438+
const isSensitiveValueField = (node: Element | null): boolean => {
439+
if (!node || node.tagName !== 'INPUT') return false
375440
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
376-
return hint === 'current-password' || hint === 'new-password'
441+
return hint
442+
.split(/\s+/)
443+
.some(
444+
(token) =>
445+
token === 'one-time-code' ||
446+
token === 'cc-number' ||
447+
token === 'cc-csc' ||
448+
token === 'cc-exp' ||
449+
token === 'cc-exp-month' ||
450+
token === 'cc-exp-year'
451+
)
377452
}
378453

379454
// Focus inside a frame or an open shadow root surfaces on the outer document
@@ -413,6 +488,17 @@ export function readActiveElementState(): unknown {
413488
redacted: true,
414489
}
415490
}
491+
// Reported as the real tag rather than 'password-field': the agent may
492+
// still type here, it just never learns what is already in the field.
493+
if (isSensitiveValueField(active)) {
494+
return {
495+
activeElement: active.tagName.toLowerCase(),
496+
selectedChars: 0,
497+
valueLength: 0,
498+
valuePreview: '',
499+
redacted: true,
500+
}
501+
}
416502
let value = ''
417503
let selectedChars = 0
418504
if (active.tagName === 'INPUT' || active.tagName === 'TEXTAREA') {
@@ -451,8 +537,14 @@ export function activeElementSecrecy(): string {
451537
const isSecretField = (node: Element | null): boolean => {
452538
if (!node || node.tagName !== 'INPUT') return false
453539
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
540+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
541+
// `current-password webauthn`, so whole-string equality missed real values
542+
// on exactly the type=text credential fields where autocomplete is the
543+
// only signal there is.
454544
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
455-
return hint === 'current-password' || hint === 'new-password'
545+
return hint
546+
.split(/\s+/)
547+
.some((token) => token === 'current-password' || token === 'new-password')
456548
}
457549

458550
let active = document.activeElement as HTMLElement | null
@@ -463,6 +555,34 @@ export function activeElementSecrecy(): string {
463555
active = shadow.activeElement as HTMLElement
464556
continue
465557
}
558+
// A CLOSED shadow root reports `shadowRoot === null` by design and cannot
559+
// be traversed from script, while focus inside it retargets to the HOST —
560+
// whose tagName is never INPUT, so the fallthrough below would call it
561+
// 'safe' and let a trusted CDP keystroke land on a password field the page
562+
// has hidden from us. Sequential focus navigation crosses closed boundaries
563+
// natively, so Tab alone is enough to get there. Treated like a
564+
// cross-origin frame: not inspectable is not safe.
565+
//
566+
// Detected by focusability rather than by tag: `attachShadow` accepts plain
567+
// div/span/section as well as custom elements, so a tag test would miss
568+
// half of them. An element that is not focusable in its own right cannot be
569+
// `activeElement` unless focus was retargeted out of a shadow tree, which
570+
// makes "not focusable yet focused" the reliable signal. Frames stay out of
571+
// it so the branch below still classifies them.
572+
const focusableItself =
573+
active === document.body ||
574+
active.isContentEditable ||
575+
active.hasAttribute('tabindex') ||
576+
active.tagName === 'INPUT' ||
577+
active.tagName === 'TEXTAREA' ||
578+
active.tagName === 'SELECT' ||
579+
active.tagName === 'BUTTON' ||
580+
active.tagName === 'A' ||
581+
active.tagName === 'AREA' ||
582+
active.tagName === 'SUMMARY' ||
583+
active.tagName === 'IFRAME' ||
584+
active.tagName === 'FRAME'
585+
if (!shadow && !focusableItself) return 'opaque'
466586
if (active.tagName === 'IFRAME' || active.tagName === 'FRAME') {
467587
let inner: Document | null = null
468588
try {
@@ -486,8 +606,14 @@ export function typeIntoElement(id: number, text: string, submit: boolean): unkn
486606
const isSecretField = (node: Element | null): boolean => {
487607
if (!node || node.tagName !== 'INPUT') return false
488608
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
609+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
610+
// `current-password webauthn`, so whole-string equality missed real values
611+
// on exactly the type=text credential fields where autocomplete is the
612+
// only signal there is.
489613
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
490-
return hint === 'current-password' || hint === 'new-password'
614+
return hint
615+
.split(/\s+/)
616+
.some((token) => token === 'current-password' || token === 'new-password')
491617
}
492618

493619
const el = (window.__simAgentElements || [])[id]
@@ -556,8 +682,14 @@ export function pressKeyOnPage(
556682
const isSecretField = (node: Element | null): boolean => {
557683
if (!node || node.tagName !== 'INPUT') return false
558684
if (String((node as HTMLInputElement).type || '').toLowerCase() === 'password') return true
685+
// Space-separated detail tokens are spec-legal and WebAuthn recommends
686+
// `current-password webauthn`, so whole-string equality missed real values
687+
// on exactly the type=text credential fields where autocomplete is the
688+
// only signal there is.
559689
const hint = String(node.getAttribute('autocomplete') || '').toLowerCase()
560-
return hint === 'current-password' || hint === 'new-password'
690+
return hint
691+
.split(/\s+/)
692+
.some((token) => token === 'current-password' || token === 'new-password')
561693
}
562694

563695
const target = (document.activeElement as HTMLElement | null) ?? document.body

0 commit comments

Comments
 (0)