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

%sveltekit.nonce% not being replaced #47

Open
EdwardLemayDev opened this issue Jan 12, 2024 · 9 comments
Open

%sveltekit.nonce% not being replaced #47

EdwardLemayDev opened this issue Jan 12, 2024 · 9 comments

Comments

@EdwardLemayDev
Copy link

Good day,

I don't know if it was something that was noticed, but it seems like the nonce isn't being replaced when added dynamically with svelte:head.
I was taking inspiration on your project for a personal alternative that would change a data attribute instead, and initially thought it was a mistake on my end for the nonce.
After trying mode-watcher, I found out that it was behaving the same way as my attempt.
If that's normal could somebody explain, I don't see it replaced even when building the app and it got me confused.

@huntabyte
Copy link
Member

I'm not exactly sure why this isn't being replaced. We're only using this as a workaround due to an issue with the eslint plugin for svelte. I'd raise an issue with kit about the nonce.

@huntabyte huntabyte closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2024
@ollema
Copy link
Collaborator

ollema commented Jan 29, 2024

I wonder if this ever worked? or maybe it only works in app.html

@sksar
Copy link

sksar commented Jan 30, 2024

Maybe something to do with this sveltejs/kit#7945 and this sveltejs/kit#8427

@ollema
Copy link
Collaborator

ollema commented Feb 22, 2024

I actually think there is more we can do here on this issue.

I believe, based on:

  1. in the docs, they mention:

To add a nonce for scripts and links manually included in src/app.html, you may use the placeholder %sveltekit.nonce% (for example <script nonce="%sveltekit.nonce%">).

notice how only src/app.html is mentioned here!

  1. in the source code, notice how only the app template does the replacement for %sveltekit.nonce%:
	templates: {
		app: ({ head, body, assets, nonce, env }) => ${s(template)
			.replace('%sveltekit.head%', '" + head + "')
			.replace('%sveltekit.body%', '" + body + "')
			.replace(/%sveltekit\.assets%/g, '" + assets + "')
			.replace(/%sveltekit\.nonce%/g, '" + nonce + "')
			.replace(
				/%sveltekit\.env\.([^%]+)%/g,
				(_match, capture) => `" + (env[${s(capture)}] ?? "") + "`
			)},
		error: ({ status, message }) => ${s(error_page)
			.replace(/%sveltekit\.status%/g, '" + status + "')
			.replace(/%sveltekit\.error\.message%/g, '" + message + "')}
	},

I tried to search the kit codebase for other replacements/usages of %sveltekit.nonce% or even just nonce but could not find any. please let me know if I missed anything!


my conclusion is that %sveltekit.nonce% only works in src/app.html.

therefore, I think we should:

  • remove %sveltekit.nonce% from mode-watcher since it just leads to confusion. both in this issue and potentially in issues like FOUC #39 (comment) as well
  • include instructions for you could configure mode-watcher to work with more restrictive CSP settings. in practice I think this means manually copying and pasting the FOUC prevention snippet to your src/app.html along with a %sveltekit.nonce% nonce property (that actually does get replaced!). would need to think about how to handle minimization here or even if mode-watcher should publish the snippet to some CDN like jsdelivr, not sure here.

thoughts? @huntabyte

@ollema
Copy link
Collaborator

ollema commented Feb 22, 2024

aha, or is the idea that we use <svelte:head> to inject our FOUC prevention snippet into %sveltekit.head% and therefore it should indeed be present before the %sveltekit.nonce% replacement happens?

if so, that does sound like a bug in kit. sorry for the false alarm 🤦

I wonder if the %sveltekit.nonce% is not correctly escaped or something like that?

created an issue in kit:
sveltejs/kit#11882

@ollema
Copy link
Collaborator

ollema commented Feb 22, 2024

actually, scratch that last comment. I learned in a conversation on discord that the conclusion I drew earlier was correct. it has never worked and it will never work to have %sveltekit.nonce% in <svelte:head> as that is not safe.
https://discord.com/channels/457912077277855764/1196782211375042580/1210204641942446140

so we are back to:

  • remove %sveltekit.nonce% from mode-watcher since it just leads to confusion. both in this issue and potentially in issues like FOUC #39 (comment) as well
  • include instructions for you could configure mode-watcher to work with more restrictive CSP settings. in practice I think this means manually copying and pasting the FOUC prevention snippet to your src/app.html along with a %sveltekit.nonce% nonce property (that actually does get replaced!). would need to think about how to handle minimization here or even if mode-watcher should publish the snippet to some CDN like jsdelivr, not sure here.

we could also look into hashes but that also complicates things when it comes to minimisation.

overall, personally I think it makes sense to:

  • have an "easy mode" for users who do not configure CSP. then mode-watcher just requires two lines of code in +layout.svelte
  • have an "advanced mode" for users who do configure CSP. then we should not insert anything into svelte:head in the <ModeWatcher /> component and instead ask users to add the FOUC prevention snippet themselves to src/app.html. how we do that is up for debate:
    • ask them to copy and paste a <script nonce="%sveltekit.nonce%">... snippet. does script tags in app.html get minimised by default?
    • ask them to include a <script src="..." integrity="sha256-xyz..."> from a CDN and also configure CSP directives in kit to include sha256-xyz.... then we will also have to host this somewhere and calculate and publish the hash (sha256-xyz...)

for the advanced mode, I think I prefer the first option which leads to more lines of code in src/app.html but no extra configuration of kit

I am happy to start working on some or all of these changes after your approval

@ollema ollema reopened this Feb 22, 2024
@huntabyte
Copy link
Member

Once I get back from traveling this week I really want to take a good look at how we could best tackle this. Part of me doesn't want to ignore the fact that it doesn't get replaced, but at the same time I want adding mode-watcher to projects to remain as simple as possible.

I think I'm leaning towards your approach of the advanced/vs basic usage, but will further investigate this weekend.

@ollema
Copy link
Collaborator

ollema commented Mar 19, 2024

Part of me doesn't want to ignore the fact that it doesn't get replaced, but at the same time I want adding mode-watcher to projects to remain as simple as possible.

yeah I think that sounds great! I do think that if we want to keep it simple we should just remove nonce="%sveltekit.nonce%" since it does nothing and instead it just leads to more confusion (and a false sense of security perhaps?)

I think I'm leaning towards your approach of the advanced/vs basic usage, but will further investigate this weekend.

👍

@vyconm
Copy link

vyconm commented Apr 22, 2024

hey guys, just ran into this in a production app.

Would you mind sharing a workaround until this gets fixed? Alternatively I could just implement different dark mode switching, but this package is pretty neat.

thank you!

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

No branches or pull requests

5 participants