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

CSP inline script violation when loading Sentry SvelteKit #8925

Closed
3 tasks done
ncvc opened this issue Aug 31, 2023 · 12 comments · Fixed by #9969
Closed
3 tasks done

CSP inline script violation when loading Sentry SvelteKit #8925

ncvc opened this issue Aug 31, 2023 · 12 comments · Fixed by #9969
Assignees
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Bug

Comments

@ncvc
Copy link

ncvc commented Aug 31, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/sveltekit

SDK Version

7.66.0

Framework Version

7.66.0

Link to Sentry event

No response

SDK Setup

  Sentry.init({
    dsn: env.PUBLIC_SENTRY_SVELTEKIT_CLIENT_DSN,
    integrations: [
      new Sentry.BrowserTracing(),
      new CaptureConsoleIntegration({
        levels: ["warn", "error", "assert"],
      }),
    ],
    release,
    environment: env.PUBLIC_ENV_NAME,
    // Set tracesSampleRate to 1.0 to capture 100%
    // of transactions for performance monitoring.
    // We recommend adjusting this value in production
    tracesSampleRate: 1.0,
    attachStacktrace: true,
    ignoreErrors: [
      "TypeError: Failed to fetch dynamically imported module",
      "Load failed",
      "TypeError: Load failed",
      "TypeError: Failed to fetch",
      "Failed to fetch",
      "TypeError: NetworkError when attempting to fetch resource",
      "TypeError: Importing a module script failed",
      "TypeError: error loading dynamically imported module",
    ],
  });
  Sentry.setTags({
    server_name: env.PUBLIC_DATADOG_HOST_NAME,
    version: release,
    service: "agent-webapp",
  });

Steps to Reproduce

Open the browser console and load a page where Sentry is loaded

Expected Result

No CSP errors

Actual Result

On page load, I get the following CSP error in the console:

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'self' <the rest of my script-src>". Either the 'unsafe-inline' keyword, a hash ('sha256-+X7Z1KW2Vcl9pendYbp0FYL6F0HZek43aBP/14cwq+U='), or a nonce ('nonce-...') is required to enable inline execution.

I get this error in version 7.65.0 but not 7.64.0.

I believe this was introduced in this PR, where the following script tag is injected into the head tag:

<script>
    const f = window.fetch;
    if(f){
      window._sentryFetchProxy = function(...a){return f(...a)}
      window.fetch = function(...a){return window._sentryFetchProxy(...a)}
    }

  </script>

As a temporary workaround, I've added sha256-+X7Z1KW2Vcl9pendYbp0FYL6F0HZek43aBP/14cwq+U= to my script-src CSP, but ideally I wouldn't have to update this hash each time the injected script is changed.

@Lms24
Copy link
Member

Lms24 commented Sep 1, 2023

Hi @ncvc thanks for reporting!

Dang, this isn't great - we totally missed this aspect when implementing #8802. I need to think about a solution here as I really don't want to revert #8802 if possible at all. Fwiw, I have no plans to touch this script any time soon so the hash should stay pretty constant (famous last words I guess). However, I'd like to avoid putting that burden of adjusting CSP on users.

If you anyone has a suggestion, I'm all ears ;)

@Lms24
Copy link
Member

Lms24 commented Sep 1, 2023

I have an idea how we can fix this but I'm curious on the community's thoughts around the fix:

We currently inject our fetch proxy script in sentryHandle which is handler for Kit's server-side handle hook. This is done by using the transformPageChunk callback.

We can get around the CSP error if we adapt the handler to:

  1. Generate a nonce at each page load request
  2. Add this nonce to the <script nonce="_insert_here_"> tag of our fetch proxy script
  3. Add the script-src 'nonce-_insert_here_' directive to the response's CSP header

A more brittle alternative would be to just add the script's hash to the CSP header.

My concern is that users might not be happy about our SDK modifying their CSP header.

To mitigate this, we can add a option (e.g. allowFetchProxyScriptCSP: boolean) to sentryHandle. However, I'd still tend to enabeling this by default and making it an opt-out feature. The reason is that we need the <script> tag for our fetch instrumentation to work. So we need some way to work around this, ideally without any user-action. Making this opt-in, would require users to actively modify their Sentry config and it's safe to assume that this won't happen in many cases, especially if we release this in a minor version.

Additionally, we can also allow users passing an optional, custom nonce (or hash) to sentryHandle.

Needless to say, all of this only applies if users actually turned on CSP and a CSP header is already set on the response.

@getsantry
Copy link

getsantry bot commented Sep 23, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Sep 23, 2023
@cedricr
Copy link

cedricr commented Sep 23, 2023

This issue has gone three weeks without activity. In another week, I will close it

It’s totally preventing us to upgrade Sentry past 7.64.0, so let’s reset that clock ;-)

I think allowing to pass a custom nonce/hash to sentryHandle would be a nice solution here.

@Lms24
Copy link
Member

Lms24 commented Oct 2, 2023

I think allowing to pass a custom nonce/hash to sentryHandle would be a nice solution here.

I can see that as a compromise. Less intrusive than modifying the CSP header and it gives users full control over the the used method. However, it'd require manual action to adjust CSP. I think it's something we can live with for now and possibly add an automatic option later on if there's demand for it.

@mydea mydea changed the title CSP inline script violation when loading Sentry CSP inline script violation when loading Sentry SvelteKit Oct 18, 2023
@sergchernata
Copy link

Downgraded to 7.64.0 for this reason. Hoping for a better solution.

@Lms24
Copy link
Member

Lms24 commented Oct 31, 2023

Hi, sorry for the long wait. I'm aware it's not yet fixed but very busy with other tasks. Will try to get this fixed this or next week.

@binaryme
Copy link

binaryme commented Nov 8, 2023

7.64.0

how did downgrading fixed the problem for you? I'm still having this problem after downgrading.

Will wait for @Lms24 comments on addressing root cause.

@Lms24
Copy link
Member

Lms24 commented Nov 9, 2023

Downgrading to 7.64.0 should fix the issue as before that we didn't inject a custom <script> tag. If you need to downgrade for the moment, please check your lock file and ensure that all @sentry/* dependencies are on the same version.

@MathiasWP
Copy link

What's the status on this issue? This is blocking us from using CSP

@Lms24
Copy link
Member

Lms24 commented Dec 14, 2023

My friends behold, it's the time of the year,
where shipping big features sparks angst and fear.
With Astro and Spotlight out of the door,
and a couple of fixes, refactors and more,
the time has come to shift my eyes,
towards the sveltest of frameworks under the skies.

My fellow SvelteKit users rejoice,
rest assured we hear your voice.
Soon i shall get to work on this task,
"But when?" the avid SDK user might ask.
Over the holidays, maybe even tomorrow
to finally address your CSP sorrow


bottom line: We're currently in fixing mode (ok actually fixing and preparing our new major) so I finally have time to look at this. ETA Soon.
also sorry for delaying work on this!

let's just hope the fix is better than this poem ;)

@Lms24
Copy link
Member

Lms24 commented Dec 22, 2023

Hi everyone, at long last we finally just merged in two options to resolve this problem. See #9969 for instructions. I'll update our docs soon, too.

This fix will be released with 7.91.0 in the next hours.

Thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Type: Bug
Projects
Archived in project
7 participants