Skip to content

Commit 49bd7e7

Browse files
authored
fix(replay): Ensure maskAttributes works with maskAllText=false (#20491)
This was found by claude security review, we did not look at `maskAttributes` properly when `maskAllText=false` was configured.
1 parent 1b33ddf commit 49bd7e7

2 files changed

Lines changed: 42 additions & 14 deletions

File tree

packages/replay-internal/src/util/maskAttribute.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ interface MaskAttributeParams {
1111

1212
/**
1313
* Masks an attribute if necessary, otherwise return attribute value as-is.
14+
* Keys listed in `maskAttributes` are masked even when `maskAllText` is false;
15+
* masking `value` on submit/button inputs without listing `value` still requires `maskAllText`.
1416
*/
1517
export function maskAttribute({
1618
el,
@@ -20,22 +22,20 @@ export function maskAttribute({
2022
privacyOptions,
2123
value,
2224
}: MaskAttributeParams): string {
23-
// We only mask attributes if `maskAllText` is true
24-
if (!maskAllText) {
25-
return value;
26-
}
27-
2825
// unmaskTextSelector takes precedence
2926
if (privacyOptions.unmaskTextSelector && el.matches(privacyOptions.unmaskTextSelector)) {
3027
return value;
3128
}
3229

33-
if (
34-
maskAttributes.includes(key) ||
35-
// Need to mask `value` attribute for `<input>` if it's a button-like
36-
// type
37-
(key === 'value' && el.tagName === 'INPUT' && ['submit', 'button'].includes(el.getAttribute('type') || ''))
38-
) {
30+
const masksNamedAttribute = maskAttributes.includes(key);
31+
// When `maskAllText` is enabled, also mask `value` on button-like inputs even if `value` is not listed.
32+
const masksSubmitButtonValue =
33+
maskAllText &&
34+
key === 'value' &&
35+
el.tagName === 'INPUT' &&
36+
['submit', 'button'].includes(el.getAttribute('type') || '');
37+
38+
if (masksNamedAttribute || masksSubmitButtonValue) {
3939
return value.replace(/[\S]/g, '*');
4040
}
4141

packages/replay-internal/test/unit/util/maskAttribute.test.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,15 @@ describe('maskAttribute', () => {
3333
test.each([
3434
['masks if `maskAllText` is true', defaultArgs, '***'],
3535
[
36-
'does not mask if `maskAllText` is false, despite `maskTextSelector` ',
37-
{ ...defaultArgs, maskAllText: false, maskTextSelector: 'classy' },
36+
'masks when key is in `maskAttributes` even if `maskAllText` is false',
37+
{ ...defaultArgs, maskAllText: false },
38+
'***',
39+
],
40+
[
41+
'does not mask when key is not in `maskAttributes` and `maskAllText` is false',
42+
{ ...defaultArgs, maskAllText: false, key: 'id', maskAttributes: ['title'] },
3843
'foo',
3944
],
40-
['does not mask if `maskAllText` is false', { ...defaultArgs, maskAllText: false }, 'foo'],
4145
[
4246
'does not mask if `unmaskTextSelector` matches',
4347
{ ...defaultArgs, privacyOptions: { ...privacyOptions, unmaskTextSelector: '.classy' } },
@@ -53,6 +57,30 @@ describe('maskAttribute', () => {
5357
{ ...defaultArgs, el: inputButton, value: 'input value' },
5458
'***** *****',
5559
],
60+
[
61+
'does not mask submit `value` when `maskAllText` is false unless `value` is in `maskAttributes`',
62+
{
63+
...defaultArgs,
64+
el: inputSubmit,
65+
key: 'value',
66+
maskAttributes: ['title'],
67+
maskAllText: false,
68+
value: 'input value',
69+
},
70+
'input value',
71+
],
72+
[
73+
'masks submit `value` when `maskAllText` is false if `value` is in `maskAttributes`',
74+
{
75+
...defaultArgs,
76+
el: inputSubmit,
77+
key: 'value',
78+
maskAttributes: ['value'],
79+
maskAllText: false,
80+
value: 'input value',
81+
},
82+
'***** *****',
83+
],
5684
])('%s', (_: string, input, output) => {
5785
expect(maskAttribute(input)).toEqual(output);
5886
});

0 commit comments

Comments
 (0)