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

fix: workaround for the issue with chrome.runtime.getURL introduced in Chrome 130 causing CSP rejecting script due to different origin (GUID instead of chrome extension id) #928

Merged
merged 1 commit into from
Oct 20, 2024

Conversation

onv
Copy link
Contributor

@onv onv commented Oct 17, 2024

fix: workaround for the issue with chrome.runtime.getURL introduced in Chrome 130 causing CSP rejecting script due to different origin (GUID instead of chrome extension id)

Starting Chrome 130 use_dynamic_url has taken an effect and as a result chrome.runtime.getURL(path) now returns url with random GUID chrome-extension://1d4b08e1-f2c5-48e5-839d-32c125e7a989/${path} instead of chrome-extension://${chrome.runtime.id}/${path}
Setting use_dynamic_url to false or replacing chrome.runtime.getURL() with template string chrome-extension://${chrome.runtime.id}/${path} solves the issue.

The related issue

image (14)

Copy link

changeset-bot bot commented Oct 17, 2024

🦋 Changeset detected

Latest commit: 9cdc3e7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@crxjs/vite-plugin Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Oct 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
vite-plugin-docs ⬜️ Ignored (Inspect) Visit Preview Oct 19, 2024 7:14am

@onv onv force-pushed the fix/chrome-runtime-get-url branch from 1f2dd1a to 58f5e0d Compare October 17, 2024 08:03
@onv
Copy link
Contributor Author

onv commented Oct 17, 2024

@jacksteamdev could you please take a look?

@pietrofxq
Copy link

shouldn't we also address use_dynamic_url value not being passed over to generated manifest.json?

@onv onv force-pushed the fix/chrome-runtime-get-url branch from 58f5e0d to 04855df Compare October 18, 2024 08:48
@goodideagiver
Copy link

goodideagiver commented Oct 18, 2024

I tried to use this changes in patch-package and manually editing vite-plugin/node_modules/dist but for me it didn't fix the issue

index c543f3e..654ed0d 100644
--- a/node_modules/@crxjs/vite-plugin/dist/index.mjs
+++ b/node_modules/@crxjs/vite-plugin/dist/index.mjs
@@ -1805,7 +1805,7 @@ function compileFileResources(fileName, {
 const defineManifest = (manifest) => manifest;
 const defineDynamicResource = ({
   matches = ["http://*/*", "https://*/*"],
-  use_dynamic_url = true
+  use_dynamic_url = false
 }) => ({
   matches,
   resources: [DYNAMIC_RESOURCE],
@@ -1839,7 +1839,7 @@ const pluginWebAccessibleResources = () => {
           // all resources are web accessible
           resources: ["**/*", "*"],
           // change the extension origin on every reload
-          use_dynamic_url: true
+          use_dynamic_url: false
         };
         if (browser === "firefox") {
           delete war.use_dynamic_url;

The iplementation from this PR works for me though xc2/follow-it-later#22

@pietrofxq
Copy link

I tried to use this changes in patch-package and manually editing vite-plugin/node_modules/dist but for me it didn't fix the issue

index c543f3e..654ed0d 100644
--- a/node_modules/@crxjs/vite-plugin/dist/index.mjs
+++ b/node_modules/@crxjs/vite-plugin/dist/index.mjs
@@ -1805,7 +1805,7 @@ function compileFileResources(fileName, {
 const defineManifest = (manifest) => manifest;
 const defineDynamicResource = ({
   matches = ["http://*/*", "https://*/*"],
-  use_dynamic_url = true
+  use_dynamic_url = false
 }) => ({
   matches,
   resources: [DYNAMIC_RESOURCE],
@@ -1839,7 +1839,7 @@ const pluginWebAccessibleResources = () => {
           // all resources are web accessible
           resources: ["**/*", "*"],
           // change the extension origin on every reload
-          use_dynamic_url: true
+          use_dynamic_url: false
         };
         if (browser === "firefox") {
           delete war.use_dynamic_url;

I think you updated the wrong lines. You should update the values on lines 1842 and 1924

@tance77
Copy link

tance77 commented Oct 18, 2024

We're excited about this patch. Our Extension is dead in the water till this is merged and deployed.

@pietrofxq
Copy link

@tance77 in the meantime you can patch the tool with patch-package, check #918 (comment)

@onv onv force-pushed the fix/chrome-runtime-get-url branch from 04855df to d6a90eb Compare October 18, 2024 21:00
@mr-menno
Copy link

mr-menno commented Oct 18, 2024

This is also in issue: #929

The simplest fix to get this working again is updating:
const ownOrigin = new URL(chrome.runtime.getURL(\"/\")).origin;
to
const ownOrigin = 'chrome-extension://'+chrome.runtime.id;
in
packages/vite-plugin/src/client/es/hmr-client-worker.ts

Although I've validate the above patch in this PR works for me as well.

…using CSP rejecting script due to different origin (GUID instead of chrome extension id)
@onv onv force-pushed the fix/chrome-runtime-get-url branch from d6a90eb to 9cdc3e7 Compare October 19, 2024 07:14
@mr-menno
Copy link

Thanks @onv !

When can this PR be merged and released? A lot of dev is at a standstill without manual patching.

@onv
Copy link
Contributor Author

onv commented Oct 19, 2024

@jacksteamdev, can this PR be merged? Looks like I still need an approval from maintainer. Thanks.

Copy link
Contributor

@jacksteamdev jacksteamdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @onv for putting this together!

@jacksteamdev jacksteamdev merged commit f4eb4d4 into crxjs:main Oct 20, 2024
4 of 5 checks passed
@jacksteamdev
Copy link
Contributor

This landed in 2.0.0-beta.26

@mr-menno
Copy link

mr-menno commented Oct 21, 2024 via email

@tance77
Copy link

tance77 commented Oct 21, 2024

2.0.0-beta26
seems to replace my background serverice worker path with service-worker-loader.js and that doesn't seem to exist for me.

@jacksteamdev
Copy link
Contributor

2.0.0-beta26 seems to replace my background serverice worker path with service-worker-loader.js and that doesn't seem to exist for me.

@tance77 If this is still happening for you, please create an issue so we can reproduce it.

@rahulbansal16
Copy link

This landed in 2.0.0-beta.26

What is the version of Vite to use for this?
I am using the 5.4.6 and I'm getting the errors about import _vite_plugin.crx is not a function.
image

import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'
import { crx } from '@crxjs/vite-plugin'
import manifest from './manifest.json'

export default defineConfig({
  plugins: [
    react(),
    crx({ manifest }),
  ],
})

Tried this configs #939 (comment) but the error is persistent.

@jacksteamdev

@rahulbansal16
Copy link

Anyone able to resolve the issues of #939 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants