[🐞] nonce is missing on scripts #4411

tzdesign · 2023-06-05T12:12:13Z

Which component is affected?

Qwik City (routing)

Describe the bug

I'm not sure what exactly this code is. It looks like something in qwik city.

These two tags are rendered in preview without a nonce.

    <script>
    (function() {
        const l = location
        , c = l.pathname + l.search
        , t = "_qCityPopstateFallback"
        , o = "_qCityHistory";
        window[t] || (window[t] = ()=>{
            window[o] || c === (l.pathname + l.search) || l.reload()
        }
        ,
        setTimeout(()=>{
            addEventListener("popstate", window[t])
        }
        , 0))
    }
    )();
</script>
<!--/qv-->
<!--/qv-->
<script q:key="1Z_0">
    ((s,a,i,r)=>{
        i = (e,t)=>{
            t = document.querySelector("[q\\:base]"),
            t && a.active && a.active.postMessage({
                type: "qprefetch",
                base: t.getAttribute("q:base"),
                ...e
            })
        }
        ,
        document.addEventListener("qprefetch", e=>{
            const t = e.detail;
            a ? i(t) : t.bundles && s.push(...t.bundles)
        }
        ),
        navigator.serviceWorker.register("/service-worker.js").then(e=>{
            r = ()=>{
                a = e,
                i({
                    bundles: s
                })
            }
            ,
            e.installing ? e.installing.addEventListener("statechange", t=>{
                t.target.state == "activated" && r()
            }
            ) : e.active && r()
        }
        ).catch(e=>console.error(e))
    }
    )([])
</script>

Reproduction

https://github.com/the-zimmermann/csp-nonce-issue

Steps to reproduce

Just run preview

Run a qwik app with a plugin middleware:

plugin@csp.ts

import type { RequestHandler } from '@builder.io/qwik-city';

export const onRequest: RequestHandler = (ev) => {
  const nonce = Date.now().toString(55);
  ev.sharedMap.set('@nonce', nonce);

  const csp = [
    `default-src 'self' 'unsafe-inline'`,
    `font-src 'self' `,
    `img-src 'self' `,
    `script-src 'strict-dynamic' 'unsafe-inline' 'nonce-${nonce}' `,
    `style-src 'self' 'unsafe-inline'`,
    `frame-src 'self' 'nonce-${nonce}'`,
    `object-src 'none'`,
    `base-uri 'self'`,
    `require-trusted-types-for 'script'`
  ];

  ev.headers.set(
    'Content-Security-Policy',
    csp.join('; ')
  );
};

System Info

Binaries:
    Node: 16.17.0 - /usr/local/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 8.15.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 113.0.5672.126
    Edge: 113.0.1774.57
    Firefox: 108.0.2
    Safari: 16.4
  npmPackages:
    @builder.io/qwik: ^1.1.4 => 1.1.4 
    @builder.io/qwik-city: ^1.1.4 => 1.1.4 
    undici: 5.22.0 => 5.22.0 
    vite: 4.3.3 => 4.3.3

Additional Information

No response

manucorporat · 2023-06-05T16:10:57Z

mind creating a new PR?! would be very happy to merge it!

tzdesign · 2023-06-06T09:57:51Z

@manucorporat I still have no idea what the flow of qwik-city is.
I looked up the code and the first script implements clientNavigate, but no clue where it is embedded.

The second one might be sw-prefetch.ts

But somehow, this looks correctly implemented:

export const ServiceWorkerRegister = (props: { nonce?: string }) =>
  jsx('script', { dangerouslySetInnerHTML: swRegister, nonce: props.nonce });

PS: In the next weeks I will read the whole project through, would be awesome to contribute a bit in code and not only in words ;)

DustinJSilk · 2023-06-06T19:25:13Z

@tzdesign can you share a link to a repository or stackblitz to reproduce the issue and i will try and help

tzdesign · 2023-06-07T08:31:40Z

@DustinJSilk Stackblitz fails somehow. These guys seem to be having trouble.

See https://github.com/the-zimmermann/csp-nonce-issue

I just added the plugin the scripts are reported and one odd thing a temporary script or smt?

DustinJSilk · 2023-06-07T08:38:07Z

I see you havent passed the nonce to the ServiceWorkerRegister. Try updating your root.tsx file to include this:

  const nonce = useServerData<string | undefined>('nonce')
 
  return <QwikCityProvider>
      <head>
        <meta charSet="utf-8" />
        <link rel="manifest" href="/manifest.json" />
        <RouterHead />
      </head>
      <body lang="en">
        <RouterOutlet />
        <ServiceWorkerRegister nonce={nonce} />
      </body>
    </QwikCityProvider>

tzdesign · 2023-06-07T08:49:38Z

@DustinJSilk oh, I did not know that. Isn't there a way to do this under the hood?
This kind of thing will pop up for almost everyone using CSP. Alternatively, we should really document CSP to make everything clear.

Thanks for your help.

tzdesign · 2023-06-07T08:50:42Z

@DustinJSilk if you do this in my repository do you still see the issue pointing to a missing script (closing <style/>)?

How do you deal with This document requires 'TrustedScriptURL' assignment. ?
Do you set the origin as trusted script url?

DustinJSilk · 2023-06-07T09:00:33Z

Feel free to submit a PR with documentation.
We manually pass the none to the service worker since it is a lite component (not created with component$) and was not easy to pass the value to the component automatically.

I don't use require-trusted-types-for in my CSP policy. Removing that solves the TrustedScriptURL issue.

tzdesign · 2023-06-08T08:47:33Z

@DustinJSilk I've removed require-trusted-types-for and you're right about that, but would you please check again?

There are still bugs like when there are some temporary scripts, if you follow the link in dev-tools, you end up at the end of a style tag and there are no script tags anymore.

@jordanw66 thanks for clarifying

DustinJSilk · 2023-06-08T08:55:20Z

@tzdesign i dont have much capacity right now, have you checked if this issue is a regression yet? Try installing qwik/qwik-city v1.0.0. If it works, it would help to know which version of qwik it stopped working.

tzdesign · 2023-06-08T11:25:55Z

@DustinJSilk I'm pretty busy too, but I checked it out. v1.0.0 works fine, not a single report in the console.

I added a page to the docs in this pr #4440.
I will check another version after lunch and let you know what I find.

DustinJSilk · 2023-06-08T11:32:26Z

Nice PR and thanks for checking v1! I’ll try dig into it soon as well as it’ll probably block my next production release

tzdesign · 2023-06-08T14:56:08Z

@DustinJSilk looks like version 1.1 is already having this issue.

I think it's the new RouterOutlet by @manucorporat with a plain script.
https://github.com/BuilderIO/qwik/blob/6dd1b47923b62f9673c93040cdd19778f3a4ce92/packages/qwik-city/runtime/src/router-outlet-component.tsx#L33

Would it be save to add useServerData here?

/**
 * @public
 */
export const RouterOutlet = component$(() => {
  _jsxBranch();

  const nonce = useServerData<string | undefined>('nonce');

  const { value } = useContext(ContentInternalContext);
  if (value && value.length > 0) {
    const contentsLen = value.length;
    let cmp: JSXNode | null = null;
    for (let i = contentsLen - 1; i >= 0; i--) {
      cmp = jsx(value[i].default, {
        children: cmp,
      });
    }
    return (
      <>
        {cmp}
        <script dangerouslySetInnerHTML={popStateScript} nonce={nonce}></script>
      </>
    );
  }
  return SkipRender;
});

If yes let me know, I can create another PR

DustinJSilk · 2023-06-09T10:58:12Z

Yes that looks like it would work. Have you managed to test if it solves the problem in your repo?

tzdesign · 2023-06-09T18:55:34Z

@DustinJSilk In my pr I changed the router-outlet, but there is a discussion if it should be changed to a prop instead of useServerData. Let's see what Manu thinks as he has rewritten this file.

DustinJSilk · 2023-06-11T13:11:15Z

~~@tzdesign is there a PR open for this change yet? I havent seen one.~~

Found it under the docs pr

tzdesign added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels Jun 5, 2023

tzdesign changed the title ~~[🐞] nonce is missing on a script~~ [🐞] nonce is missing on scripts Jun 5, 2023

tzdesign closed this as completed Jun 7, 2023

tzdesign reopened this Jun 7, 2023

DustinJSilk mentioned this issue Jun 12, 2023

fix: add missing nonce to popstate script #4468

Merged

7 tasks

manucorporat closed this as completed in #4468 Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[🐞] nonce is missing on scripts #4411

[🐞] nonce is missing on scripts #4411

tzdesign commented Jun 5, 2023 •

edited

manucorporat commented Jun 5, 2023

tzdesign commented Jun 6, 2023 •

edited

DustinJSilk commented Jun 6, 2023

tzdesign commented Jun 7, 2023

DustinJSilk commented Jun 7, 2023

tzdesign commented Jun 7, 2023

tzdesign commented Jun 7, 2023 •

edited

DustinJSilk commented Jun 7, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 8, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 8, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 9, 2023

tzdesign commented Jun 9, 2023

DustinJSilk commented Jun 11, 2023 •

edited

[🐞] nonce is missing on scripts #4411

[🐞] nonce is missing on scripts #4411

Comments

tzdesign commented Jun 5, 2023 • edited

Which component is affected?

Describe the bug

Reproduction

Steps to reproduce

System Info

Additional Information

manucorporat commented Jun 5, 2023

tzdesign commented Jun 6, 2023 • edited

DustinJSilk commented Jun 6, 2023

tzdesign commented Jun 7, 2023

DustinJSilk commented Jun 7, 2023

tzdesign commented Jun 7, 2023

tzdesign commented Jun 7, 2023 • edited

DustinJSilk commented Jun 7, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 8, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 8, 2023

tzdesign commented Jun 8, 2023

DustinJSilk commented Jun 9, 2023

tzdesign commented Jun 9, 2023

DustinJSilk commented Jun 11, 2023 • edited

tzdesign commented Jun 5, 2023 •

edited

tzdesign commented Jun 6, 2023 •

edited

tzdesign commented Jun 7, 2023 •

edited

DustinJSilk commented Jun 11, 2023 •

edited