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

Includes remotely-hosted code, so cannot be used within a Chrome extension #1383

Closed
raindrift opened this issue Aug 9, 2024 · 9 comments · Fixed by #1386 or #1407
Closed

Includes remotely-hosted code, so cannot be used within a Chrome extension #1383

raindrift opened this issue Aug 9, 2024 · 9 comments · Fixed by #1386 or #1407
Assignees
Labels
bug js @honeybadger-io/js

Comments

@raindrift
Copy link

raindrift commented Aug 9, 2024

What are the steps to reproduce this issue?

  1. Create a chrome extension using manifest v3
  2. Set up honeybadger in accordance with https://docs.honeybadger.io/lib/javascript/integration/chrome-extension/
  3. Submit extension to the Chrome web store

What happens?

Extension is rejected after review. I received the following notice by email:

Violation reference ID: Blue Argon

Technical Requirements - Additional Requirements for Manifest V3:

Violation:
Including remotely hosted code in a Manifest V3 item.
Violating Content:
Code snippet: service_worker.js: t.getUserFeedbackScriptUrl = function(e) { var t = e.split(".").slice(0, 2).join("."); return "https://js.honeybadger.io/v".concat(t, "/honeybadger-feedback-form.js")
Code snippet: extension.js: }; t.getUserFeedbackScriptUrl = function(e) { var t = e.split(".").slice(0, 2).join("."); return "https://js.honeybadger.io/v".concat(t, "/honeybadger-feedback-form.js") };

What were you expecting to happen?

Extension would be accepted, on account of not breaking the rules.

Any logs, error output, etc?

No

Any other comments?

It took a few extension versions for them to catch this issue. Because of this, I know that Honeybadger works great in our extension! I am hoping we don't have to stop using it. I know this library has worked fine with mv3 for years. The policy isn't new, but perhaps there have been recent updates to Google's code analysis tools?

I am installing the npm version and packaging with webpack, but the same issue exists in the minified js.

It seems like this would be solvable by:

  1. Include the feedback form js with the rest of the honeybadger javascript. It's not very much code, so it seems like lazy-loading it (if that's why it is separate) isn't gaining much.
  2. This problem will occur again in the form itself, since it submits data by injecting a <script> tag into the page. Instead, send the form data to the backend with fetch()

For the moment, I will try to work around this by forking the Honeybadger library and removing the feedback form entirely, since we aren't currently using it.

What versions are you using?

Operating System: osx
Package Name: honeybadger-js
Package Version: 6.9.3
Browser Version: Chrome 127.0.6533.89 / arm64

@raindrift
Copy link
Author

raindrift commented Aug 9, 2024

Update: this appears to be happening because Google made updates to their static analysis tools. They also found "remotely-hosted code" in a README that was mistakenly packed with a different extension I work on. That was obviously an error.

It seems unlikely that they will allow the honeybadger library in any Chrome extension going forward, now that their tooling can detect that it is executing code they can't audit.

For anyone else who runs into this issue, my workaround was to grab dist/browser/honeybadger.js from the npm package, comment out the definition for Honeybadger.prototype.appendUserFeedbackScriptTag and its callsite (there's only one), and explicitly load it as part of each entrypoint that uses it (we're using webpack) instead of importing it.

I don't know if it will work, but I'll find out in the next day or two!

@subzero10
Copy link
Member

Hey @raindrift, thanks for submitting this issue and good catch on figuring out the updates on their static analysis tools!

Include the feedback form js with the rest of the honeybadger javascript. It's not very much code, so it seems like lazy-loading it (if that's why it is separate) isn't gaining much.

The feedback form is an optional feature, not heavily used and only for the browser integration (note that honeybadger-io/js is an isomorphic package). That's probably why we are lazy-loading it. @joshuap do you know if there's any other reason for taking this approach?

This problem will occur again in the form itself, since it submits data by injecting a <script> tag into the page. Instead, send the form data to the backend with fetch()

I think the reason it was made like this is because a fetch with HTTP POST would be rejected due to CORS from Honeybadger's API server - but I'm not 100% sure, it's been some time since I worked on this.

For anyone else who runs into this issue, my workaround was to grab dist/browser/honeybadger.js from the npm package, > comment out the definition for Honeybadger.prototype.appendUserFeedbackScriptTag and its callsite (there's only one), > and explicitly load it as part of each entrypoint that uses it (we're using webpack) instead of importing it.

I don't know if it will work, but I'll find out in the next day or two!

Considering they found "remotely-hosted code" in a README file, maybe it would be better to remove this part of the code (instead of commenting out)? In any case, let us know how it goes and we'll see how to proceed moving forward! 🤔

@raindrift
Copy link
Author

Hey @raindrift, thanks for submitting this issue and good catch on figuring out the updates on their static analysis tools!

No prob! fwiw, I think a number of companies will be running into this in the coming weeks. For example, mixpanel's library is also no longer compatible.

This problem will occur again in the form itself, since it submits data by injecting a <script> tag into the page. Instead, send the form data to the backend with fetch()

I think the reason it was made like this is because a fetch with HTTP POST would be rejected due to CORS from Honeybadger's API server - but I'm not 100% sure, it's been some time since I worked on this.

I think adding a <script> tag to the dom is often used as a fallback when fetch or xhr are blocked for some reason? But I have the impression that more recent browser features for blocking content make this workaround largely obsolete now. There could be some other reason, though.

FWIW, you may be able to get around your CORS issue by setting Content-type: text/plain on the POST request. This puts it in "legacy CORS" mode, which doesn't do a preflight and skips a bunch of the other checks. But since you control the infrastructure that you're connecting to, you can also just allow all origins on the server side.

If the issue is that you're trying to get around the page's content security policy, I'd be surprised if this trick works. But even so, your user controls the page and can presumably change the CSP at least in most cases?

I guess you have to support a lot of different browsers, including older ones. If the script tag is working around an issue that still exists, I wonder if it would be super difficult to have a build for extensions, in addition to the server and browser builds. It looks like it would be a slightly nontrivial refactor, but maybe possible since it's already an isomorphic package anyway?

Considering they found "remotely-hosted code" in a README file, maybe it would be better to remove this part of the code (instead of commenting out)? In any case, let us know how it goes and we'll see how to proceed moving forward! 🤔

Commenting the code does seem to have worked, inasmuch as I got a second rejection for some different code that I had not yet commented. So I guess their tools can understand code comments at least. Specifically, I also removed the body of the isUserFeedbackScriptUrlAlreadyVisible function.

It hasn't been accepted yet, but it also hasn't been automatically rejected. Fingers crossed.

@subzero10
Copy link
Member

@raindrift I'm not exactly sure why we went for the script tags instead of accepting all origins on the server side; maybe this was a standard approach when it was first introduced. I will see how it works by trying your suggestions (thanks for the "legacy CORS" mode switch!). The chrome extension build is a possible approach which I already considered, I will look into that as well.

In any case, please keep us posted on the status of your extension review, it will help us move in the right direction.

@raindrift
Copy link
Author

raindrift commented Aug 16, 2024 via email

@subzero10
Copy link
Member

Thanks keeping us updated. I will work on a solution asap!

@subzero10 subzero10 self-assigned this Aug 19, 2024
@subzero10 subzero10 added bug js @honeybadger-io/js labels Aug 19, 2024
subzero10 added a commit that referenced this issue Aug 19, 2024
@joshuap
Copy link
Member

joshuap commented Aug 19, 2024

The feedback form is an optional feature, not heavily used and only for the browser integration (note that honeybadger-io/js is an isomorphic package). That's probably why we are lazy-loading it. @joshuap do you know if there's any other reason for taking this approach?

No, I don't recall at the moment.

@subzero10
Copy link
Member

subzero10 commented Aug 25, 2024

Hey @raindrift, we have published a build on our CDN that does not include the code which caused the rejection on Google Play Store. The build is available at this URL: https://js.honeybadger.io/v6.10/honeybadger.ext.min.js

It'd be great if you could try it and let us know if it worked for you :)

@akshaysmurthy
Copy link

@subzero10 We had the same problem. In https://js.honeybadger.io/v6.10/honeybadger.ext.min.js, I still see a document.createElement("script"). I think that is enough for the Chrome store to reject the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug js @honeybadger-io/js
Projects
None yet
4 participants