-
Notifications
You must be signed in to change notification settings - Fork 125
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
Unable to use with Manifest v3 due to remote code execution #1394
Comments
same issue here, seems to be occuring due to posthog.capture
Code snippet: popup.js: loadScript("/static/recorder.js?v=".concat(ar.LIB_VERSION), |
@damnmicrowave @seanwessmith have you tried those options https://posthog.com/docs/advanced/browser-extension ? |
I find this documentation a bit confusing. Don't fully understand what should I do. |
Right. It's not properly tree shaked. |
Is there any progress on this issue now? I have same issue |
have you tried to do what the docs tell here https://posthog.com/docs/advanced/browser-extension#session-replay about the |
The array.full.js should have all the bundled code so it should not do any remote code execution. Can you confirm if using |
Hi all, I work on the Chrome Extensions team at Google. We came across this issue as we've been seeing quite a number of violations along the lines described here. Just to clarify, the Additional requirements for Manifest V3 (usually referred to as our "Remote Hosted Code" policy) applies to all code included in the submission, even if it isn't executed. Tree shaking can often help to remove code but that depends a lot on how it is imported. Do feel free to reach out if you have any questions ([email protected]). Definitely keen to help make things work better out of the box. |
@oliverdunk I emailed you to start the conversation - happy to wait in case you're on summer leave but messaging here as well in case the email didn't make it :) |
However, the code is not actually executing. Please adjust the logic for code detection. I've had three consecutive versions rejected a total of nine times, and each time I have to manually appeal and wait over three days for a resolution. This is seriously impacting my product's iteration progress. |
@oliverdunk Please resolve this ASAP. Thank you! It really affects the development experience. 🫠 |
Just sharing for others - I migrated to https://www.npmjs.com/package/posthog-js-lite for my Chrome extension and that was approved by the Chrome Web Store team 👍 |
I managed to get it to work with the following. I was using:
I had to create custom replacements for PostHogProvider & usePostHog Included in the index.html inside the postHogProvider.tsx
usePostHog.ts
I am yet to publish these updates to the Chrome Web Store yet fingers crossed it does not get flagged! |
@DophinL rhe best solution I found is to use the HTTP endpoints |
@ryan-greenfieldlabs Have you encountered any issues while using the lite version? |
My solution was manually removing the code. import fs from "fs"
import path from "path"
const buildDir = path.join(__dirname, "..", "build", "chrome-mv3-prod")
const filesToModify = ["popup", "tabs/app", "tabs/unauthorized"]
function modifyFile(filePath: string) {
let content = fs.readFileSync(filePath, "utf-8")
content = content.replace(
/this\.rrwebRecord\?this\._onScriptLoaded\(\):e[so]\(this\.instance\.requestRouter\.endpointFor\("assets","\/static\/recorder\.js\?v="\s*\.concat\(g\.LIB_VERSION\)\),function\(t\)\{if\(t\)return K\.error\(r_\+"\s*could not load recorder\.js",t\);e\._onScriptLoaded\(\)\}\)/,
"this._onScriptLoaded()"
)
content = content.replace(
/&&e[so]\(this\.instance\.requestRouter\.endpointFor\("assets","\/static\/exception-autocapture\.js"\),function\(r\)\{if\(r\)return K\.error\("Could not load exception autocapture script",r\);_\.extendPostHogWithExceptionAutocapture\(t\.instance,e\)\}\)/,
""
)
content = content.replace(
/e[so]\(this\.instance\.requestRouter\.endpointFor\("assets","\/static\/toolbar\.js\?t="\s*\.concat\(a\)\),function\(e\)\{if\(e\)return K\.error\("Failed to load toolbar",e\),void\(t\._toolbarScriptLoaded=!1\);t\._callLoadToolbar\(n\)\}\),/,
""
)
content = content.replace(
/\|\|e[so]\(this\.instance\.requestRouter\.endpointFor\("assets","\/static\/surveys\.js"\),function\(t\)\{if\(t\)return K\.error\("Could not load surveys script",t\);H\.extendPostHogWithSurveys\(e\.instance\)\}\)/,
""
)
fs.writeFileSync(filePath, content, "utf-8")
console.log(`Modified: ${filePath}`)
}
function main() {
const files = fs.readdirSync(buildDir, { recursive: true })
filesToModify.forEach((filePattern) => {
const regex = new RegExp(
`${filePattern.replace("/", "\\/")}\\.[a-z0-9]+\\.js$`
)
const matchedFiles = files.filter((file) => regex.test(file.toString()))
if (matchedFiles.length > 0) {
matchedFiles.forEach((matchedFile) => {
const filePath = path.join(buildDir, matchedFile.toString())
modifyFile(filePath)
})
} else {
console.warn(`No files found matching pattern: ${filePattern}`)
}
})
}
main() |
Ah that's a really cool workaround @DophinL and leads into my update nicely 😊 I've been chatting to @oliverdunk. Their analysis means even if we allow you to configure posthog-js in such a way that it will never load remote code their analysis can't detect that. Frustrating with a posthog hat on, but totally understandable with an "extensions should be certified safe" hat on :) We're going to need to bundle a version of posthog-js that doesn't include the remote lazy-loading functionality. We should be able to do something very similar to the workaround above that lets rollup/terser shake the code away during bundling I'd normally be bullish about how fast we can do that but we're onboarding a new team member next week... Watch this space though! |
Nope. I am using |
After dealing with the remote loading code, I was fortunate enough to be rejected twice more, this time by PostHog, but for a different reason—code obfuscation issues. 💢 |
Thanks for sharing this - I've reached out to the team, and I'll try to follow up here and directly to Paul in the next day or so. I want to have a quick chat about this and make sure we're aligned. Generally, we allow minification, but don't allow obfuscation. As I hope you can appreciate from a review perspective this walks the line a little bit. I understand it isn't code you added though and comes from a reasonable set of steps (ultimately a dependency of PostHog seems to be using https://www.npmjs.com/package/rollup-plugin-web-worker-loader). I think we just need to make a decision about if this is something we consider reasonable or not. |
Thanks for replying @oliverdunk Folk can audit rrweb to see what it's doing - IIRC the web worker is to offload compression to a worker instead of blocking the main thread We're not using the compression right now but we plan to so it'd be painful to remove that - would need someone to contribute upstream or for us to start to maintain our own fork of rrweb Separately we're figuring out how we can provide a bundle with no remote execution here #1413 |
Thank you for the feedback. I look forward to further progress, and if there are any temporary workarounds that could help, please let me know. 🫡 @oliverdunk @pauldambra |
A good workaround is to use their HTTP api instead https://posthog.com/docs/api/capture import fetch from "node-fetch";
async function sendPosthogEvent() {
const url = "https://us.i.posthog.com/capture/";
const headers = {
"Content-Type": "application/json",
};
const payload = {
api_key: "xxx",
event: "event name",
distinct_id: "user distinct id",
properties: {
account_type: "pro",
},
timestamp: "[optional timestamp in ISO 8601 format]",
};
const response = await fetch(url, {
method: "POST",
headers: headers,
body: JSON.stringify(payload),
});
const data = await response.json();
console.log(data);
}
sendPosthogEvent() |
I mainly want to use the session replay feature. @pauldambra @oliverdunk Could you please provide any updates? It's been several days, and I noticed that my extension seems to be stuck with no progress in the review process. 🤣 |
If I'm using lite, how can I use session replay? Can I just import posthog-js/dist/recorder directly? @pauldambra |
This is my script to bypass this issue after the production build of my extension:
And it works, my extension is approved. |
Hey, no, you can't... that's why it's caled lite 🤣 We're still figuring out the best way to bundle it |
My extension has been stuck in review for over 7 days. Is there any way to expedite the process through alternative review methods? We have customers waiting for the new feature to be released. 🤣 @oliverdunk https://chromewebstore.google.com/detail/tidyread/gpfnpbpkadjgeoneammbiiidbkgkncaa |
This issue has 2174 words at 31 comments. Issues this long are hard to read or contribute to, and tend to take very long to reach a conclusion. Instead, why not:
Is this issue intended to be sprawling? Consider adding label |
Hey @pauldambra , thanking for pointing me to this issue, A quick question for you, Instead of sending these events from Thanks a lot. |
@Sachin-Malik that will work, posthog-node doesn't load any external scripts. (If you don't want to refactor things much, you can also consider posthog-js-lite in |
Hey all. I hope we finally have a good solution out for this with this PR. Preview docs here As of The important thing is you need to ensure you only ever import // posthog.ts
// No external code loading possible (this disables all extensions such as Replay, Surveys, Exceptions etc.)
import posthog from 'posthog-js/dist/module.no-external'
// No external code loading possible but all external dependencies pre-bundled
// import posthog from 'posthog-js/dist/module.full.no-external'
const posthogJs = posthog.init(...)
export default posthogJs Would love some feedback ASAP from people if this doesn't work or has issues. The hard part of splitting out the loader is done so any fixes to this new solution should hopefully be fast but if we get the feedback :D (edit: We had a CI issue so 1.64.1 will contain the fix...) |
Could you please confirm if obfuscation is also addressed? My extension was rejected again after 12 days of review due to this issue. 😂 @benjackwhite @pauldambra @oliverdunk |
Thanks so much for putting the updated bundle together. I took a quick look and everything I saw looked compliant with our policies - although the final decision is made during review, of course.
I took a quick look and didn't see this code present anymore, but maybe @benjackwhite or @pauldambra could confirm? We had a lot of discussion about this internally, which is why your review took longer than normal - apologies for that. In summary:
Given the above, and that understanding this code is quite hard during review, we have decided that this does violate our policies. Hopefully that is a non-issue if the code is gone from the new bundle, but if not, happy to work with you @benjackwhite / @pauldambra to see if we can come up with a solution. |
I’ve noticed that the obfuscated code still exists when using the PostHog recorder. This is a crucial feature for me and the primary reason I’m willing to pay for PostHog. It’s incredibly useful for user behavior analysis. However, to get through the review process and resolve the bugs my clients are encountering, I have no choice but to temporarily disable session replay, directly use @oliverdunk I have also submitted a new version of the extension for review that utilizes the new |
@DophinL @oliverdunk yep, the worker is part of rrweb that screen recording uses i'll figure out if we can patch the library so that we can shake that out, or contribute back upstream 🫠 :) |
But good to know that the "main" fix to remove the lazy loading has worked @oliverdunk - thanks for confirming |
Cool but it does not even support autocapture which is quite a important feature. |
Let us know if it works. Thanks! |
I tested it locally and everything seems to be working fine; it reports normally, including autocapture. I directly cloned the posthog-js repository and built a temporary version for local use. @twicer-is-coder . I'm using |
I have used Posthog-lite with a wrapper around it and my chrome extension is approved again, since my deadline was around the corner I just uploaded with this temp version until this mess is cleared up. The problem with this lite version is it does not even have autocapture. I would want to switch it with module.no-external.js once it's stable and people start getting approvals with it. |
Hey @benjackwhite or @pauldambra is there any updates on the progress of this issue. I have been using posthog-light as a work around but am missing a lot of key user analytics without the auto capture. 🙏 |
Same situation. |
Hey All, The latest versions of posthog-js can be used without external loading. We've seen some folk have their extensions approved. I believe session replay may still cause issues because the library we depend on has a base64 encoded web worker which counts as obfuscated code for Google. We've been trying to avoid forking the library so we can stay honest and contribute back upstream We don't use the webworker in posthog so will need to figure out how to build a version that does not include it. |
hey @pauldambra I'm using latest version of posthog:
and is integrated like so: import posthog from "posthog-js/dist/module.no-external" type PostHogProviderProps = { children: ReactNode } export function PostHogProvider({ children }: PostHogProviderProps) { return {children} but getting this error:
Any idea how to resolve it |
Hey @AndonMitev surprising... try setting If that doesn't resolve it and you're able to dig in to see what the initiator is for those network calls that would be great |
Hey thanks for the call, yeah can confirm with disable_external_dependency_loading: true` problem have been resolved and now logs are stored in posthog |
Hey @AndonMitev Since this is already a very long thread-history issue, it's best to add new issues or ideally with something so specific to you open a support request in-app. I'd like to keep this issue focussed since it's been around so long - and I've found very specific problems tend to be handled way easier in support queue than in github :) |
Given the length here I'm going to close this issue - since remote code execution is now solved (We're certainly getting reports of extensions being approved) I've opened #1464 so we can track the rrweb worker code issue I'd also like to update the docs here https://posthog.com/docs/advanced/browser-extension but would actually love if folk here could open a PR with how they've implemented the extension code since you all probably know how it works better than we do :) |
Bug Description
Bug description
I've created a simple reproduction for this issue. You can find it here.
Since 1st of August, we started getting rejections from Chrome Store for our browser extension. The reason for these rejections is that remote code execution is not allowed in Manifest v3 extensions.
Some posthog features are using remote code execution via injecting a script tag:
Unfortunately, they are included in the bundle even if not used.
Here are the emails that we got from the Chrome Store:
How to reproduce
pnpm i
and thenpnpm build
dist/content-scripts/main.js
/static/exception-autocapture.js
/static/surveys.js
/static/recorder.js
/static/toolbar.js
Additional context
This is not a bug, and I don't really know how this can be solved. Right now I've just forked the repo and removed all these features by hand.
Probably, the best way to enable them in the extensions would be to import the source code directly, instead of injecting script tags.
Debug info
No response
The text was updated successfully, but these errors were encountered: