getsentry / rrweb

record and replay the web
https://www.rrweb.io/
Other
9 stars 5 forks source link

feat: Ensure password inputs are always masked #78

Closed mydea closed 1 year ago

mydea commented 1 year ago

This PR does two things:

  1. Ensure you cannot opt-out of masking type="password" inputs
  2. Ensure inputs that used to have type=password and have this type changed (so e.g. through a "show password" toggle) always keep the password masked, even afterwards.

We should never record passwords, and e.g. toggle password type buttons are quite common and would easily leak the password into the replay as of now.

We do this by adding a rr_is_password attribute to the HTML input when the type is changed from password to something else. This is IMHO the easiest solution for this, and since we only add this when the type is really changed (so not eagerly for all inputs) IMHO it's an acceptable tradeoff to have this in the DOM.

Closes https://github.com/getsentry/rrweb/issues/34

mydea commented 1 year ago

Is this something we'd want to upstream? If so it should probably an option, though we could start an issue since 2.0 would be a good place to make that breaking change.

The password -> text is a good candidate to upstream though 👍

Yeah, I think the type change stuff makes sense to upstream. Not sure if they would like to have the "do not allow to opt-out of password" stuff upstream, but I guess I can also just ask!

mydea commented 1 year ago

Added upstream PR: https://github.com/rrweb-io/rrweb/pull/1170