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

feat(core): introduce strict mode #483

Merged
merged 6 commits into from
Jun 28, 2024
Merged

feat(core): introduce strict mode #483

merged 6 commits into from
Jun 28, 2024

Conversation

vejja
Copy link
Collaborator

@vejja vejja commented Jun 26, 2024

Closes #470

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR adds a new strict option, which can take 2 possible values:

  • false (default): default settings are chosen to minimize the possibility of breaking the app. These default values are the same as in v1.
  • true: default settings are chosen to maximize security. These default values will usually require some additional fine-tuning to ensure the app will run smoothly.

With the new strict mode, the following headers are modified:
1- contentSecurityPolicy blocks everything by default with default-src: 'none'. In addition, all 'unsafe-inline' values are removed.
2- crossOriginEmbedderPolicy is set to require-corp
3- strictTransportSecurity has the preload flag
4- xFrameOptions is set to DENY

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

<!--- Provide a general summary of your changes in the title above -->

Closes #470

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (a non-breaking change which fixes an issue)
- [x] New feature (a non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Description
<!--- Describe your changes in detail -->
<!--- Why is this change required? What problem does it solve? -->
<!--- If it resolves an open issue, please link to the issue here. For example "Resolves: #137" -->

This PR adds a new `owaspDefaults` option, which can take 2 possible values:
- `compatibility` (default): OWASP default settings are chosen to minimize the possibility of breaking the app. These default values are the same as in v1.
- `security`: OWASP default settings are chosen to maximize security. These default values will usually require some additional fine-tuning to ensure the app will run smoothly.

With `security` OWASP level, the following headers are modified:
1- `contentSecurityPolicy` blocks everything by default with `default-src: 'none'`. In addition, all `'unsafe-inline'` values are removed.
2- `crossOriginEmbedderPolicy` is set to `require-corp`
3- `strictTransportSecurity` has the `preload` flag
4- 'xFrameOptions` is set to `DENY`

## Checklist:
<!--- Put an `x` in all the boxes that apply. -->
<!--- If your change requires a documentation PR, please link it appropriately -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes (if not applicable, please state why)
Copy link

vercel bot commented Jun 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuxt-security ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2024 5:46pm

@vejja vejja marked this pull request as ready for review June 27, 2024 14:29
@vejja
Copy link
Collaborator Author

vejja commented Jun 27, 2024

Hi @Baroshem
This is ready now with a full doc update
I'm not too sure about the owaspDefaults terminology, feel free to suggest otherwise
Cheers

@Baroshem
Copy link
Collaborator

Thanks @vejja for this amazing pull request. May I recommend using option

strict: boolean

That by default is set to false to be comptible and when set to true it will enable more strict options?

But I do wonder, what will happen if user selects strict and then changes the values of the headers manually?

@vejja
Copy link
Collaborator Author

vejja commented Jun 27, 2024

Thanks @vejja for this amazing pull request. May I recommend using option

strict: boolean

Yes, great idea

That by default is set to false to be comptible and when set to true it will enable more strict options?

But I do wonder, what will happen if user selects strict and then changes the values of the headers manually?

This is fine, the manual values will override the strict defaults

@hermes85pl
Copy link

How about replacing strict with mode or defaults or something similar (this would be more descriptive and it would read better with other options set next to it in the user config that might override it) and having it as a string to be used as an enum, so that in the future you could introduce other sets of default configuration to choose from while maintaining clean API and not breaking compatibility?

@hermes85pl
Copy link

Oh, I see that this was more or less @vejja's original proposal, hence I'm leaning towards that, although proper naming is the key, as usual 😅

@Baroshem
Copy link
Collaborator

The main question is, do we expect to have more presets? also, we could use the keyword preset and give it a value of strict or default. MAybe that would make sense? We could then have it as an enum and be more descriptive :)

@vejja
Copy link
Collaborator Author

vejja commented Jun 27, 2024

The main question is, do we expect to have more presets? also, we could use the keyword preset and give it a value of strict or default. MAybe that would make sense? We could then have it as an enum and be more descriptive :)

Personally I would lean towards not having more presets. Maintenance is the issue of course.
On naming issues, I would have loved preset, but Nitro already uses this terminology and we use it also via nitroPresets so I did eliminate it mentally.

@Baroshem
Copy link
Collaborator

Ok, so let's keep it strict then :)

@vejja vejja changed the title feat(core): introduce owasp default sets feat(core): introduce `strict mode Jun 27, 2024
@vejja vejja changed the title feat(core): introduce `strict mode feat(core): introduce strict mode Jun 27, 2024
Copy link
Collaborator

@Baroshem Baroshem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work @vejja !

Feel free to merge this PR and let me know so that I can publish a new version with it :)

@vejja vejja merged commit 90ad2d5 into main Jun 28, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The RCs break Turnstile with default configuration for nuxt-security
3 participants