nonce is missing when rendering a <NuxtIsland> component during SSR
Version
nuxt-security: 2.0.0-rc.9 nuxt: 3.12.3
Reproduction Link
https://stackblitz.com/edit/github-5zfscl?file=pages%2Findex.vue
Steps to reproduce
Check the network log for the initial document request. You will see that the nonce- is missing from the script-src.
What is Expected?
The nonce- should be there. You can remove the <ServerComponent /> and reload the page. The nonce will be there again.
What is actually happening?
I added some loggings and it seems like the rendering of the page and the server component share the nonce. The nonce is then removed when rendering the page, because it already exists.
Hey @P4sca1
Thanks for reporting it.
As you have already did some reasearch, maybe you would be interested in implementing a fix for that? :)
Also CC @huang-julien
The last time I wasn't able to fix the issue, due to missing information about how NuxtIsland rendering works internally. I think rendering a page during SSR with NuxtIsland components shares the ssrContext. I will investigate further using a minimal reproduction and work on a fix.
@Baroshem I was able to add a test case, which reproduces the behavior. I can confirm that the root cause is, that Nuxt calls the request and the render:html hook multiple times when rendering NuxtIsland components with different events, that share the same context.
I updated the nonce generation so that nonces are only generated if they do not already exist in the context. There may be other places in the code, where additional care needs to be taken if a context is shared between multiple requests / renders.
When rendering the page with a server-only component, the CSP header already includes a script-src entry with a resolved nonce. However, already resolved nonces are filtered away in this line: https://github.com/Baroshem/nuxt-security/blob/2d5128244e3c53ecc100cdca41e45f593ec086c4/src/runtime/nitro/plugins/50-updateCsp.ts#L34 Just removing the line resolves the issue. I am wondering why it was added in the first place. Could you give some insights?
I just noticed that the resolveSecurityRules functionality (using different rules depending on the request path) will also break when using server-only components.
The NuxtIsland component will be rendered first. Rules are created for a path similar to /__nuxt_island/ServerComponent_cv0OmafuIw.json?props=%7B%7D locFIxi6Z4wKnnO8L6MuHw==. Afterwards, the page will be rendered, but because the rules already exist in the context, they will never be generated for the actual page path.
Before implementing a fix, I created a discussion in the nuxt repo to verify if context sharing is expected behavior: https://github.com/nuxt/nuxt/discussions/28340
I think we would need opinion from @vejja about it :)
Will look at it
I'm proposing a very simple fix in PR #500 Bottom line is right now we do not support Nuxt Islands, and this PR is a tentative to introduce support for Islands.
It is only a tentative proposal at this stage, because I do not use Islands myself, so I'm not very knowledgeable of all potential edge cases.
@P4sca1 would you like to test the fix, and also add some tests to verify if it works ?
Just removing the line resolves the issue. I am wondering why it was added in the first place. Could you give some insights?
That is a very valid remark. At first sight it looks like it's a legacy from the old code base when we didn't have the security context. I will need to double-check but this might have become superfluous and we should probably remove.
Update: So it is code legacy, but still I'd like to keep it because it ensures that nobody can set a constant nonce via options
Before implementing a fix, I created a discussion in the nuxt repo to verify if context sharing is expected behavior: nuxt/nuxt#28340
It rings a bell. I remember I had a hard time working around what the context was supposed to be, and I figured out it's not the same thing if the request is coming from the client (browser) or if it's a server-generated request. Would love to know the answer of the Nuxt core team also
Hey @vejja,
thank you for the update. I already worked on a PR for this issue and published it in https://github.com/Baroshem/nuxt-security/pull/502.
Changes are similar to your PR. Additional things that are included:
- Log warning when removing a static nonce from the header
- Add a test case for server-only components
- Don't generate nonce multiple times when rendering island components
My PR does not include the dependency updates, the changes to 60-recombineHtml, and the playground changes. Maybe you could remove the changes for 50-updateCsp from your PR and we can merge both?
Hey @P4sca1 Sorry, I hadn't seen your PR. Will merge both
Released in 2.0.0 :)