PostHog / posthog-js

Send usage data from your web app or site to PostHog, with autocapture.
https://posthog.com/docs/libraries/js
Other
304 stars 127 forks source link

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

Open pauldambra opened 1 month ago

pauldambra commented 1 month ago

follow-up to https://github.com/PostHog/posthog-js/issues/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 https://github.com/PostHog/posthog-js/issues/1394#issuecomment-2371497392 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 https://github.com/PostHog/posthog-js/issues/1394#issuecomment-2370278219

other context

pauldambra commented 1 month ago

first step is likely to get updated to latest rrweb so we can be sure we're not chasing shadows https://github.com/PostHog/posthog-js/pull/1276

jwarder commented 1 month ago

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

ebloom19 commented 1 month ago
  • Unable to use with Manifest v3 due to remote code execution #1394 (comment)

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

https://github.com/PostHog/posthog-js/issues/1394#issuecomment-2399245692

ebloom19 commented 1 month ago

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

DophinL commented 1 month ago

@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 commented 1 month ago

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 https://github.com/rrweb-io/rrweb/issues/1578 without any engagement :/

xrzhuang commented 1 month ago

pls prioritize

pauldambra commented 1 month ago

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

we can test if https://github.com/rrweb-io/rrweb/pull/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 https://github.com/rrweb-io/rrweb/issues/1376. in both cases folk were only looking at bundle size since that predates google's changes to the review process.

xrzhuang commented 4 weeks ago

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 commented 3 weeks ago

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

pauldambra commented 3 weeks ago

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