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

Chrome Manifest v3 extensions may be rejected due to "obfuscated code" #1464

Open
pauldambra opened this issue Oct 14, 2024 · 12 comments
Open
Labels
SDK Support General SDK issues rather than being related to a feature team

Comments

@pauldambra
Copy link
Member

pauldambra commented Oct 14, 2024

follow-up to #1394

that issue is very long already so I want to have a little more space to breathe for what is a separate problem that will have a different solution

see #1394 (comment) from @oliverdunk

his comment carried here:

I took a quick look and didn't see this code present anymore, We had a lot of discussion about this internally, which is why your review took longer than normal - apologies for that. In summary:
Creating a worker using a blob URL (this is what the base64 string in your rejection email is used for) violates the script-src policy we intend to apply to MV3 extensions. Due to a Chrome bug, this currently works and would only be caught during review. However, we would like to change that in the future.
Once that bug is fixed, this would be dead code in violation of our policies. Our usual rule is to still enforce on this code as (while it may be less likely in this case) we have definitely seen code that looks like dead code become active across updates and used maliciously.
Given the above, and that understanding this code is quite hard during review, we have decided that this does violate our policies.

see also
image
from #1394 (comment)

other context

@pauldambra
Copy link
Member Author

first step is likely to get updated to latest rrweb so we can be sure we're not chasing shadows #1276

@jwarder
Copy link

jwarder commented Oct 15, 2024

Sorry if this question has already been asked but will session replay currently work with a chrome extension V3?

@ebloom19
Copy link

Unfortunately it will not pass the review by the Chrome Extension team yet check out the discussion here

#1394 (comment)

@ebloom19
Copy link

@pauldambra any new updates on the progress of this issue? We really missing having the session recording observability in our chrome extension.

@DophinL
Copy link

DophinL commented Oct 28, 2024

@pauldambra any new updates on the progress of this issue? We really missing having the session recording observability in our chrome extension.

Same question, I've paused session recording for a couple of weeks and really need it 😂 @pauldambra

@pauldambra
Copy link
Member Author

the rrweb upgrades took a bunch of attention so we haven't looked at this yet (i'll make the standard reminder that PRs are welcome if other folk have time to look at it - but i appreciate this is fairly deep in the guts of your dependencies dependency 😅)

it's been logged here already rrweb-io/rrweb#1578 without any engagement :/

@xrzhuang
Copy link

xrzhuang commented Nov 1, 2024

pls prioritize

@pauldambra
Copy link
Member Author

ok, pulling on this thread... it might be possible to fix this and avoid needing to remove Canvas recording support

we can test if rrweb-io/rrweb#1448 would remove this obfuscated code from the bundle in posthog's usage of rrweb, and if we see that folk's extensions start to pass google's checks, then we can prompt the rrweb maintainers and get this fixed upstream so everyone benefits

full disclosure to save people following all the links, that is a PostHog PR to rrweb implementing a change from Sentry's fork of rrweb they they offered to contribute rrweb-io/rrweb#1376. in both cases folk were only looking at bundle size since that predates google's changes to the review process.

@xrzhuang
Copy link

xrzhuang commented Nov 4, 2024

ok, pulling on this thread... it might be possible to fix this and avoid needing to remove Canvas recording support

we can test if rrweb-io/rrweb#1448 would remove this obfuscated code from the bundle in posthog's usage of rrweb, and if we see that folk's extensions start to pass google's checks, then we can prompt the rrweb maintainers and get this fixed upstream so everyone benefits

full disclosure to save people following all the links, that is a PostHog PR to rrweb implementing a change from Sentry's fork of rrweb they they offered to contribute rrweb-io/rrweb#1376. in both cases folk were only looking at bundle size since that predates google's changes to the review process.

let me know how i can publish this to the google store we can do this right away to test

@seawatts
Copy link

seawatts commented Nov 7, 2024

Thanks for looking into this @pauldambra. I'm going to remove posthog recording right now to get things published.

@pauldambra
Copy link
Member Author

Yep, unfortunately and super frustratingly that's the only solution at the moment. We really hate that that's the case

@robbie-c robbie-c added the SDK Support General SDK issues rather than being related to a feature team label Nov 29, 2024
@Juice10
Copy link

Juice10 commented Dec 12, 2024

We just fixed this upstream with this PR: rrweb-io/rrweb#1568

Specifically this fixes it in the bundling step as long as the DISABLE_WORKER_INLINING=true flag is set (which we only run while building the extension): https://github.com/rrweb-io/rrweb/blob/d09f561073304b2fcbd426576de1f03d46241e55/vite.config.default.ts#L164-L176

Also see the release.yaml for how this is used to release a new version of the extension to the Chrome store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SDK Support General SDK issues rather than being related to a feature team
Projects
None yet
Development

No branches or pull requests

8 participants