Skip to content

Mask placeholder attributes when input masking is enabled - #1911

Closed
carlosdanieltt wants to merge 45 commits into
rrweb-io:mainfrom
mixpanel:fix/mask-placeholder-attributes
Closed

Mask placeholder attributes when input masking is enabled#1911
carlosdanieltt wants to merge 45 commits into
rrweb-io:mainfrom
mixpanel:fix/mask-placeholder-attributes

Conversation

@carlosdanieltt

Copy link
Copy Markdown

Summary

  • When maskAllInputs or maskInputOptions is enabled, placeholder attributes on <input> and <textarea> elements are now masked through the same maskInputValue() path as input values
  • Previously, placeholders were always recorded in plain text even with input masking on, which could leak PII if apps set placeholder text dynamically with user data (e.g., "Enter email: user@example.com")
  • Masking respects maskInputOptions granularity (per input type) and custom maskInputFn callbacks

Test plan

  • New integration test confirms placeholders are replaced with * characters when maskAllInputs: true
  • Existing maskInputOptions test confirms non-masked input types still show plain placeholders
  • Existing maskInputFn test confirms custom masking function applies to placeholders (textarea with data-unmask-example keeps its placeholder unmasked)
  • All 51 integration tests pass

🤖 Generated with Claude Code

jakewski and others added 30 commits August 8, 2025 20:30
Co-authored-by: Jakub Grzegorzewski <25271819+jakewski@users.noreply.github.com>
SRFE-8790 Address Iframe memory leaks
SRFE-8790 cleanup child nodes recursively
dhaval-valotia and others added 12 commits March 16, 2026 12:23
Ignore media interactions if not a supported media element during replay SR-490
When maskAllInputs or maskInputOptions is enabled, only input values
were masked while placeholder attributes were left in plain text. This
could leak PII if apps set placeholder text dynamically with user data.

Now placeholder attributes on input and textarea elements are masked
through the same maskInputValue() path, respecting maskInputOptions
granularity and custom maskInputFn callbacks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 20:20
@changeset-bot

changeset-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a10319d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR closes a masking gap by ensuring placeholder attributes on <input> and <textarea> are masked whenever input masking is enabled, preventing potential PII leakage via dynamically generated placeholder text.

Changes:

  • Mask placeholder values during full snapshot serialization (rrweb-snapshot) using the existing maskInputValue() path.
  • Mask placeholder attribute mutations during incremental recording (rrweb) alongside existing value masking.
  • Add an integration test for masked placeholders and update the shared form fixture + related snapshots.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/rrweb/test/integration.test.ts Adds integration coverage asserting placeholders are masked under maskAllInputs.
packages/rrweb/test/html/form.html Adds placeholder attributes to fixture inputs used across integration tests.
packages/rrweb/test/snapshots/integration.test.ts.snap Updates snapshots to reflect placeholder attributes and masking behavior.
packages/rrweb/src/record/mutation.ts Extends attribute-mutation masking to include placeholder.
packages/rrweb-snapshot/src/snapshot.ts Masks serialized placeholder attributes for inputs/textareas using maskInputValue().
Comments suppressed due to low confidence (1)

packages/rrweb/src/record/mutation.ts:588

  • When masking value/placeholder attribute mutations, the dedup guard compares the masked new value against the unmasked m.oldValue. This can cause redundant attribute-mutation records when the underlying value changes but masks to the same output (e.g., same-length placeholders, or a maskInputFn that returns a constant), increasing payload size and replay churn. Consider masking m.oldValue with the same inputs before comparing, so the comparison is done on the recorded representation.
        if (attributeName === 'value' || attributeName === 'placeholder') {
          const type = getInputType(target);

          value = maskInputValue({
            element: target,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@carlosdanieltt carlosdanieltt changed the title fix: mask placeholder attributes when input masking is enabled [MULTI-798] fix: mask placeholder attributes when input masking is enabled Jul 27, 2026
Carlos Trevino and others added 3 commits July 27, 2026 13:34
…utes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Required by mixpanel org policy. All actions now reference commit SHAs
with version comments for readability.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@eoghanmurray

Copy link
Copy Markdown
Collaborator

Hey Carlos, this looks sound enough; did you come up with an alternative fix?

@carlosdanieltt carlosdanieltt changed the title [MULTI-798] fix: mask placeholder attributes when input masking is enabled Mask placeholder attributes when input masking is enabled Jul 30, 2026
@carlosdanieltt

Copy link
Copy Markdown
Author

Closed in favor of a more cleaned up version here: #1912

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants