Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: mask password by default #1334

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Conversation

daibhin
Copy link
Contributor

@daibhin daibhin commented Jul 31, 2024

Changes

By default rrweb only masks password fields by setting maskAllInputs: false and maskInputOptions: { password: true }

Currently PostHog JS masks all inputs by default (maskAllInputs: true) and because of that has no maskInputOptions e.g. maskInputOptions: {}

Given the sensitivity of passwords I think we should still use the same default as rrweb for maskInputOptions. It might be a little confusing to a customer why their passwords are still masked when they explicitly set maskAllInputs: false but I would rather confusion and protection over unintentionally disabling password masking.

There is a little bit of an overlap in these two parameters which is the source of confusion, but as I said above, a customer should really know when they're explicitly revealing password fields. I would argue we go a step further and always mask password fields because I can't think of a good reason to allow passwords to be captured but we can follow up with that

@daibhin daibhin added the bump minor Bump minor version when this PR gets merged label Jul 31, 2024
Copy link

vercel bot commented Jul 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Jul 31, 2024 5:07pm

Copy link

github-actions bot commented Jul 31, 2024

Size Change: +655 B (+0.06%)

Total Size: 1.16 MB

Filename Size Change
dist/array.full.js 332 kB +166 B (+0.05%)
dist/array.js 154 kB +163 B (+0.11%)
dist/main.js 154 kB +163 B (+0.11%)
dist/module.js 154 kB +163 B (+0.11%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 10.4 kB
dist/recorder-v2.js 110 kB
dist/recorder.js 110 kB
dist/surveys-preview.js 59.5 kB
dist/surveys.js 65.6 kB
dist/tracing-headers.js 8.26 kB
dist/web-vitals.js 5.79 kB

compressed-size-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants