bitwarden / clients

Bitwarden client apps (web, browser extension, desktop, and cli).
https://bitwarden.com
Other
9k stars 1.18k forks source link

bootstrap-content-message-handler.js prints to browser console on every window message, even those made by other extensions #7575

Closed OneNot closed 7 months ago

OneNot commented 8 months ago

Steps To Reproduce

Example of when the issue would happen: Using both this addon and Bitwarden's Firefox addon and navigating to youtube.com, the browser console gets flooded with constant debug messages.

Expected Result

bootstrap-content-message-handler.js should probably only log messages that originate from Bitwarden's own scripts.

Actual Result

bootstrap-content-message-handler.js floods the console with debug messages for every window message made by any addon.

Screenshots or Videos

No response

Additional Context

Anything that uses window messages is caught by Bitwarden's bootstrap-content-message-handler.js. As an example navigating to youtube.com with this addon and Bitwarden's addon enabled, the console gets constantly spammed with these two debug messages:

Handling window message [bootstrap-content-message-handler.js:167:21](moz-extension://ab53cc0e-d7fc-4dfe-8d84-71c9e7ffc4e0/content/bootstrap-content-message-handler.js)
Bad source or badly formatted message, skipping. [bootstrap-content-message-handler.js:171:25](moz-extension://ab53cc0e-d7fc-4dfe-8d84-71c9e7ffc4e0/content/bootstrap-content-message-handler.js)

by Bitwarden's bootstrap-content-message-handler.js

I only marked Windows 11 and Firefox in the issue template as that was where I saw the issue, but I assume the issue isn't exclusive to either.

Operating System

Windows

Operating System Version

11

Web Browser

Firefox

Browser Version

121.0.1

Build Version

2024.1.0

Issue Tracking Info

MidnightTinge commented 7 months ago

when you get to the point people are suggesting to downgrade and disable auto updates I really do gotta question some of the logic being thrown around to justify not reverting. I love the tool, but this makes me question whether or not I trust updates in the future to not disrupt my daily flow. the fact that this wasn't even considered to be put behind a flag is weird, doubly so considering this is in an "RC" that somehow got pushed out to the general public? or maybe I missed some context there. regardless, the fact that this won't be disabled until February is incredibly annoying, especially for a tool that I'm paying for. my options are to either disable bitwarden, disable my framework's devtools, or downgrade and disable updates. none of those are appealing options.

Filtering is not an option for you?

I need to be able to filter other logs (which makes the regex exclusion disruptive, I'm either filtering for one thing or filtering OUT the bitwarden stuff), and we log out to the debug channel (so I can't just exclude that channel). I'm in that group of people that are just altogether screwed 😄 while there are workarounds, it's incredibly disruptive to my flow either way unfortunately hence my frustration

edit: I'll also add that I shouldn't be expected to have to do this as a consumer, so if my options are "do this thing that pulls you out of your flow but technically works" or "downgrade" I'm going to downgrade

rathpc commented 7 months ago

This issue actually just started interfering with how github was loading some of my repositories (stalling to the point of an unresponsive script where I couldn't even open dev tools). I have chosen the option of downgrading for now but I am really surprised that this was ever pushed into production and upon seeing that it has had a large impact that an urgent hotfix is not yet being considered.

aminomancer commented 7 months ago

Hello, I'm wondering if this is known to have a performance impact. I noticed the logs while debugging some of my own code, but ignored it for a while by filtering it out. When I closed the tab, my browser froze for several seconds. Now everything's a bit laggy and stuttery. I'll restart of course (which I normally wouldn't do for weeks at a time 😛), and I've rolled back the update myself, so it's not the end of the world. And I'll most likely remember to re-enable updates.

But if this does have a major performance impact, which my experience at least hints at, I think that might be a reasonable justification for shipping a dot release ahead of schedule. If that is the case, then I believe filtering is not a solution to the performance issues.

Also, it's risky to have users roll back and disable automatic updates. They may forget to ever re-enable it, and then a segment of users will be unreachable by updates.

With that said, I don't know anything about your processes so I can't guess how disruptive a dot release would be for your release schedule. I just wanted to register my personal experience, as I'm not sure the performance impact for users has been considered.

CodeF53 commented 7 months ago

If you do roll back and disable automatic updates, just subscribe to the issue so you see it close so you get reminded to turn them back on.

laurengore42 commented 7 months ago

There's no way it doesn't have a performance impact - I'm seeing a debug log roughly every second (when I'm trying to work on my own code). Yes I have the debug channel turned off so I don't see the flood of messages, but the information is still in my browser.

aspiers commented 7 months ago

Tangential to the problem at hand, but I just wanted to say how refreshing it is to see such consistently polite and constructive discussion in this issue, despite the clearly significant amount of irritation this has caused a number of web developers (including myself). So many other communities would descend into mud-slinging and insults, so this speaks volumes to the quality of the Bitwarden user community I guess :heart:

And thanks a lot to @justindbaur for his transparent and honest communication "under fire" - this is great to see :clap:

That said, I will add my voice to the list of people who really think that an emergency hotfix release is warranted. We're talking about literally removing a single line of code, which surely could have been done with less time and effort than this whole issue has required? So @justindbaur, I would suggest you go back to the people referred to here:

Hi All, I am not the one that makes the go/no-go call on a release but I can assure you the people who do are aware of this issue and the comments you are making. It's not off the table but as of now we aren't planning on another release for this.

and ask them to think again. Or if there is some reason why cutting an emergency hotfix release to remove one line of code isn't as straightforward as it seems, then explaining that to the community would go a long way towards making people more patient with the status quo.

gammons commented 7 months ago

hi @justindbaur could you set our expectations for when this will be fixed? Thank you!

bacf5 commented 7 months ago

There's no way it doesn't have a performance impact - I'm seeing a debug log roughly every second (when I'm trying to work on my own code). Yes I have the debug channel turned off so I don't see the flood of messages, but the information is still in my browser.

Yep, definitely had a performance impact on my computer, had a few crashes. My mistake was leaving too many tabs open but who doesn't?

I hope they solve it soon.

CodeF53 commented 7 months ago

Or if there is some reason why cutting an emergency hotfix release to remove one line of code isn't as straightforward as it seems, then explaining that to the community would go a long way towards making people more patient with the status quo.

From what I have seen with extensions, they can take weeks to months to get updates approved. But that probably can be fast tracked given how big bitwarden is.

ITedInnovator commented 7 months ago

We are constantly developing and pushing code to main. When we are preparing for release we create a branch called rc. From that point on, that branch is only to be updated with the express approval of our QA, essentially only critical bug fixes. We were encountering what we consider a release blocking bug where our SSO flow in our Safari browser extension was not working. I spent hours trying to recreate the issue but was unable to. In a desperation attempt I requested, internally (to my manager, our QA team, and our devops team), approval to cherry pick a commit adding only logging to rc. That commit can be found here. It was decided to do it there because we didn't want it in main and to become a regular part of our code.

We knew the logging would be in released code, my manager asked me if it would be okay to release and I said yes it would be fine to release since we don't actually log any of the payloads of any message sent through the window messaging API, which I would have considered an issue. I admittedly underestimated the differing extensions that heavily utilize window.postMessage on websites and how many people utilize the debug channel of logs in dev tools.

Did your manager check with the security team that it isn't leaking any information it shouldn't? For a password manager I would expect more thorough procedures before logging to the live app.

fergalmoran commented 7 months ago

Hey all, give @justindbaur a break. He's being incredibly earnest and honest here, way more than I've personally come to expect from maintainers of commercial open source software.

I know it's disappointing that it's taking this long for a fix but we don't know the internal workings of the Bitwarden team. There are enough workarounds posted for the time being and it will be fixed when it's fixed.

aspiers commented 7 months ago

Or if there is some reason why cutting an emergency hotfix release to remove one line of code isn't as straightforward as it seems, then explaining that to the community would go a long way towards making people more patient with the status quo.

From what I have seen with extensions, they can take weeks to months to get updates approved.

If true, that would not be a reason to avoid doing a quick hotfix; in fact, if anything it would be a reason to do one ASAP instead of waiting for the next normal release.

noelhibbard commented 7 months ago

For those saying this extra logging should only apply to people testing. It's possible that the bug they are trying to catch with the extra logging is so rare that they couldn't restrict the logging to just a subset of testers. They probably needed a larger pool. Also, the average user isn't in the dev console and developers should already know how to filter unwanted messages from the console without hand holding. The outrage here is a little overblown.

laurengore42 commented 7 months ago

'If you accidentally push code to production, you should take it out again' is not really outrage. I was pleased to find a thread that explained why my browser console was doing that.

therealstein commented 7 months ago

Especially when dealing with iframes and window messages, this extension causes the browser to crash. Screenshot 2024-01-22 at 11 12 59

nick-shmyrev commented 7 months ago

I know it's disappointing that it's taking this long for a fix but we don't know the internal workings of the Bitwarden team. There are enough workarounds posted for the time being and it will be fixed when it's fixed.

True, there are workarounds, and "fixed when it's fixed" is probably good enough for a free product. But is "waiting 3 weeks to fix a known issue caused by a single line of code" good enough for a paid tier? For me, the answer is no.

JDuchniewicz commented 7 months ago

Very interesting finding, launched my app and was dumbstruck but then I remembered installing this extension.

JxEngel commented 7 months ago

Not sure if this helps others or has been mentioned above... I was experiencing this and found that if I login to the extension it goes away.

justindbaur commented 7 months ago

Hey all, happy update, we are preparing a release that will remove the logging. You can see the diff from our previous release here. This should remove all the logging you see from us on your various sites. Please note that updates to browser extensions go through a review on their respective stores. This process can sometimes take several days but we will be pushing this update to 100% of users right away so you should get the update soon after approval.

If we are to ever release any logging in the content scripts we will be sure to make it opt-in so that none of our fellow developers have to have this affect them but I also assure you that is was a very one-off scenario and I don't foresee the need for logging in this part of the application to be needed ever again.

JannesMeyer commented 7 months ago

Thank you, @justindbaur. I really appreciate your hard work on Bitwarden.

It's an amazing product that I couldn't live without!

sneakers-the-rat commented 7 months ago

hell ya ty. appreciate your responsiveness, thx for handling this <3

Akila-I commented 7 months ago

just came across this issue, and read through the whole thread. Appreciate the hard work on open source. Happily waiting for the update.

schulzjona commented 7 months ago

Any ETA for the Firefox extension update?

soon

oieeaaaa commented 7 months ago

i uninstalled bitwarden - problem solved lol

njrardin commented 7 months ago

Hey all, happy update, we are preparing a release that will remove the logging. You can see the diff from our previous release here. This should remove all the logging you see from us on your various sites. Please note that updates to browser extensions go through a review on their respective stores. This process can sometimes take several days but we will be pushing this update to 100% of users right away so you should get the update soon after approval.

If we are to ever release any logging in the content scripts we will be sure to make it opt-in so that none of our fellow developers have to have this affect them but I also assure you that is was a very one-off scenario and I don't foresee the need for logging in this part of the application to be needed ever again.

As a web developer who has certainly pushed worse things to main, I don't think I can throw stones here. Appreciate the candidness and look forward to being able to turn my debug channel back on!

NotSaviru commented 7 months ago

image

cranberry3148 commented 7 months ago

@NotSaviru Update to 2024.1.1

NotSaviru commented 7 months ago

updated but sometime it appears

noc2spam commented 7 months ago

Hey all, happy update, we are preparing a release that will remove the logging. You can see the diff from our previous release here. This should remove all the logging you see from us on your various sites. Please note that updates to browser extensions go through a review on their respective stores. This process can sometimes take several days but we will be pushing this update to 100% of users right away so you should get the update soon after approval.

If we are to ever release any logging in the content scripts we will be sure to make it opt-in so that none of our fellow developers have to have this affect them but I also assure you that is was a very one-off scenario and I don't foresee the need for logging in this part of the application to be needed ever again.

Thank you so much for the update! Really appreciated.