simplify CSP directive generation

[danielszuk]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch @kindspells/astro-shield@1.3.5 for the project I'm working on.

My task

I had to implement the CSP headers for an astro web app appropriately to the predefined (and not negotiable) company policy.
Where I ran into obstacles:

• I have to add https: to the script-src
• I have to remove self from the script-src
  Both of them seemed to be impossible with the current implementation of astro-shield:
• if I add 'https:', astro-shield adds quotation marks around it, which makes the rule invalid
• 'self' is added by astro-shield by default, could not prevent it (related: Add 'self' to CSP script-src directive only when strictly necessary #54)

My suggestions

Checking the code it seems to me, that it tries to add some defaults to the CSP directives.
IMO the rule of quotation mark usage in CSP directives is chaotic enough, astro-shield should not complicate it further:) (I mean adding and removing quotation marks from the values the developer set in the config file).
My suggestion is to leave the developer-set values as it is to avoid any confusion and expand them only with the hash values of the resources.

Here is the diff that solved my problem:

@@ -16,16 +16,6 @@ export const serialiseHashes = hashes =>
 		.map(h => `'${h}'`)
 		.join(' ')
 
-/**
- * @param {Set<string>} hashes
- * @returns {string}
- */
-export const safeSerialiseHashes = hashes =>
-	Array.from(hashes)
-		.sort()
-		.map(h => (h.match(/^'[^']+'$/i) ? h : `'${h}'`))
-		.join(' ')
-
 /**
  * @param {CSPDirectives} directives
  * @returns {string}
@@ -45,15 +35,9 @@ export const serialiseCspDirectives = directives =>
 export const setSrcDirective = (directives, srcType, hashes) => {
 	const baseSrcDirective = directives[srcType]
 	if (baseSrcDirective) {
-		const srcDirective = new Set(
-			baseSrcDirective.split(/\s+/).filter(v => v !== "'self'"),
-		)
-		for (const hash of hashes) {
-			srcDirective.add(`'${hash}'`)
-		}
-		directives[srcType] = `'self' ${safeSerialiseHashes(srcDirective)}`
+		directives[srcType] = `${baseSrcDirective} ${serialiseHashes(hashes)}`
 	} else {
-		directives[srcType] = `'self' ${serialiseHashes(hashes)}`
+		directives[srcType] = `${serialiseHashes(hashes)}`
 	}
 }
 

If it makes sense to you, I would like to contribute by implementing this patch fully (modify the tests and create the PR)

This issue body was partially generated by patch-package.

[castarco]

Hi @danielszuk . It seems you are not the only one having problems with this specific part of the integration.

I agree that it needs some changes, but I should look at it with care. From what I see there, I suspect that your patch would break other systems.

[danielszuk]

Hey @castarco, thanks for your contribution! We are eagerly waiting for your insights about this, ping me if you need any help.

[castarco]

Morning @danielszuk , you can take a look on this PR #79 .

It's very similar to what you did, but I kept having 'self' by default (it's possible to remove it by explicitly setting the CSP directive).

I also kept part of that "complex" logic you removed, to ensure that it gets rid of duplicates and always presents the directives in the same form (so equivalent directives should look the same).

[danielszuk]

Thank you @castarco! Works well now in 1.3.6 :)