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

Runtime Config for Security Headers #369

Closed
marcel-wtfoxtrot opened this issue Feb 5, 2024 · 12 comments · Fixed by #370
Closed

Runtime Config for Security Headers #369

marcel-wtfoxtrot opened this issue Feb 5, 2024 · 12 comments · Fixed by #370
Labels
bug Something isn't working

Comments

@marcel-wtfoxtrot
Copy link

Version

nuxt-security: 1.1.0
nuxt: 3.10

Reproduction Link

https://stackblitz.com/edit/nuxt-starter-tjhotm?file=modules%2Fsecurity%2Findex.ts

Hi.
I want to actually use the new "runtimeHooks" but there must be something i'm missing.

I use ENV Variables to define my Headers.
Within the new 'nuxt-security:ready' hook i want to read the values from the runtimeConfig and pass them to the 'nuxt-security:headers' hook.

Unfortunately none of these values are being applied. Some internal defaults are still being used.
Do i have to disable some default behaviour?

@marcel-wtfoxtrot marcel-wtfoxtrot added the bug Something isn't working label Feb 5, 2024
@Baroshem
Copy link
Owner

Baroshem commented Feb 5, 2024

Hey @marcel-wtfoxtrot

Thanks for reporting this issue.

@huang-julien could you take a look? I think you are the best person for that question :)

@quarkus
Copy link

quarkus commented Feb 5, 2024

I was just trying to disable some CSP headers via a boolean ENV flag and can't get it work either :(

@quarkus
Copy link

quarkus commented Feb 6, 2024

@huang-julien i created a quick stackblitz for reproduction:
https://stackblitz.com/edit/nuxt-starter-p4acfm?file=server%2Fplugins%2Fsecurity-plugin.ts

Would be interesting to know if its just a documentation issue / misundertstanding or if its not working at all .. which would surprise me as there is a test for that.

In the Stackblitz the Headers are not removed from the documents response which prevents the preview to be rendered in the iframe.

@huang-julien
Copy link
Contributor

hmmm i don't see any issue, can you add more step so i can reproduce it ?
image

@quarkus
Copy link

quarkus commented Feb 6, 2024

Hey @huang-julien! Thanks for the reply!
The hooks are called and i can also access values from the runtime config.
They just do not seem to change anything to the response headers send along with the document.

screenshot_mh 2024-02-06 um 14 06 55

I assumed it would be possible to change / remove the headers by calling the hooks.

@marcel-wtfoxtrot
Copy link
Author

Hi @huang-julien

i've updated my repo to give you some more insight.

https://stackblitz.com/edit/nuxt-starter-tjhotm?file=modules%2Fsecurity%2Findex.ts,app.vue,nuxt.config.ts,modules%2Fsecurity%2Fruntime%2Fplugins%2Fnitro.ts

If you enable the headers definition within the Module Options (modules/security/index.ts) the app is able to load and will render the welcome screen.

The same Options are being passed via 'nuxt-security:headers'. You can see them in the console.

in short:

using 'nuxt-security:headers' will not apply custom rules
using module options will apply our custom rules

Does this make sense?

@huang-julien
Copy link
Contributor

I couldn't reproduce the render issue locally but i reproduced the issue with the headers thank you !

@Baroshem
Copy link
Owner

Baroshem commented Feb 7, 2024

Hey guys, I have merged a PR from @huang-julien to fix this issue and released it with 1.1.1 version.

Could you check if everything works now?

Thanks @huang-julien for the work! 💚

@quarkus
Copy link

quarkus commented Feb 7, 2024

Thanks a lot guys.
I am afraid it does not change too much for me :( .

https://stackblitz.com/edit/nuxt-starter-p4acfm?file=server%2Fplugins%2Fsecurity-plugin.ts

I was tinkering around with the settings and tried to add the exact headers from @huang-julien s test but i can't get them to show up in the documents response.

i try to add the following headers in the plugin:

contentSecurityPolicy: {
          'script-src': ["'self'", "'unsafe-inline'", 'some-value.com'],
          'frame-ancestors': "'*'",
        },

But the CSP header in the documents response is not altered:

base-uri 'none'; font-src 'self' https: data:; form-action 'self'; frame-ancestors 'self'; img-src 'self' data:; object-src 'none'; script-src-attr 'none'; style-src 'self' https: 'unsafe-inline'; script-src 'self' https: 'unsafe-inline' 'strict-dynamic';
(dev and production build).

I really don't understand whats going on here as, by looking at the tests, this is supposed to work just fine.

@huang-julien
Copy link
Contributor

huang-julien commented Feb 7, 2024

image

i don't have this issue, i do have some-value for script-src and * for frame-ancestors 🤔 (i cloned your repro locally)

@quarkus
Copy link

quarkus commented Feb 9, 2024

Ok .. i will try that as well. Thanks for looking into!

@quarkus
Copy link

quarkus commented Feb 14, 2024

Hey @huang-julien just did.
Pulled the stackblitz, yarn && yarn dev -> did not work.
Checked out the nuxt-security yarn && yarn dev -> works like a charm in the playground project.

Went back to the stackblitz and debugged into :
https://github.com/Baroshem/nuxt-security/blob/main/src/runtime/nitro/plugins/00-context.ts#L41

Where i found a silent Error being thrown due to a missing import (setHeader -> undefined).
I can fix it by adding the missing imports.

Came back to comment.
Found this issue and PR that will fix it :) !

#373
#374

I believe it is still worth examining the differences in installing the module from its sources or via pgk manager as it obviously makes a difference in how dependencies & utils are installed / indexed.
Also i am not sure if the Tests work correct in that case ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants