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

Update dependency helmet to v4 #1668

Merged
merged 3 commits into from
Sep 17, 2020
Merged

Update dependency helmet to v4 #1668

merged 3 commits into from
Sep 17, 2020

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Aug 2, 2020

This PR contains the following updates:

Package Type Update Change
helmet (source) dependencies major 3.23.3 -> 4.0.0

Release Notes

helmetjs/helmet

v4.0.0

Compare Source

See the Helmet 4 upgrade guide for help upgrading from Helmet 3.

Added
  • helmet.contentSecurityPolicy:
    • If no default-src directive is supplied, an error is thrown
    • Directive lists can be any iterable, not just arrays
Changed
  • This package no longer has dependencies. This should have no effect on end users, other than speeding up installation time.
  • helmet.contentSecurityPolicy:
    • There is now a default set of directives if none are supplied
    • Duplicate keys now throw an error. See helmetjs/csp#​73
    • This middleware is more lenient, allowing more directive names or values
  • helmet.xssFilter now disables the buggy XSS filter by default. See #​230
Removed
  • Dropped support for old Node versions. Node 10+ is now required
  • helmet.featurePolicy. If you still need it, use the feature-policy package on npm.
  • helmet.hpkp. If you still need it, use the hpkp package on npm.
  • helmet.noCache. If you still need it, use the nocache package on npm.
  • helmet.contentSecurityPolicy:
    • Removed browser sniffing (including the browserSniff and disableAndroid parameters). See helmetjs/csp#​97
    • Removed conditional support. This includes directive functions and support for a function as the reportOnly. Read this if you need help.
    • Removed a lot of checks—you should be checking your CSP with a different tool
    • Removed support for legacy headers (and therefore the setAllHeaders parameter). Read this if you need help.
    • Removed the loose option
    • Removed support for functions as directive values. You must supply an iterable of strings
  • helmet.frameguard:
  • helmet.hidePoweredBy no longer accepts arguments. See this article to see how to replicate the removed behavior. See #​224.
  • helmet.hsts:
  • helmet.xssFilter no longer accepts options. Read "How to disable blocking with X–XSS–Protection" and "How to enable the report directive with X–XSS–Protection" if you need the legacy behavior.

Renovate configuration

📅 Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by WhiteSource Renovate. View repository job log here.

@bobsilverberg
Copy link
Contributor

I resolved one of the issues with this update, which was the removal of the browserSniff option for helmet.contentSecurityPolicy, but there is still a test failing.

The test for NODE_ENV=production › sets production CSP and security headers is now failing because header['x-xss-protection'] is now being set to 0 instead of 1; mode=block. This is happening because the project changed the default value that the product sets the x-xss-protection header to. It used to use 1; mode=block as a default, but switched to 0. There is a full discussion of this at helmetjs/helmet#230, which is a bit over my head. I gather that there are pros and cons to each option, as discussed at that issue. I believe we can still set the value to 1; mode=block if we want to override the default, but I'm not sure what we actually want to use.

@muffinresearch (or anyone else on the team) do you have an opinion on this?

@muffinresearch
Copy link
Contributor

muffinresearch commented Aug 17, 2020

I resolved one of the issues with this update, which was the removal of the browserSniff option for helmet.contentSecurityPolicy, but there is still a test failing.

The test for NODE_ENV=production › sets production CSP and security headers is now failing because header['x-xss-protection'] is now being set to 0 instead of 1; mode=block. This is happening because the project changed the default value that the product sets the x-xss-protection header to. It used to use 1; mode=block as a default, but switched to 0. There is a full discussion of this at helmetjs/helmet#230, which is a bit over my head. I gather that there are pros and cons to each option, as discussed at that issue. I believe we can still set the value to 1; mode=block if we want to override the default, but I'm not sure what we actually want to use.

@muffinresearch (or anyone else on the team) do you have an opinion on this?

+1 to taking Helmet's new default for x-xss-protection and turning it off. FWIW this header was never supported by Firefox.

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2020

Codecov Report

Merging #1668 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1668   +/-   ##
=======================================
  Coverage   98.04%   98.04%           
=======================================
  Files          63       63           
  Lines        2712     2712           
  Branches      702      702           
=======================================
  Hits         2659     2659           
  Misses         42       42           
  Partials       11       11           
Impacted Files Coverage Δ
src/server/index.tsx 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb316fa...5a2b15e. Read the comment docs.

@bobsilverberg
Copy link
Contributor

This is now fixed and can be merged!

@bobsilverberg bobsilverberg merged commit 4525454 into master Sep 17, 2020
@bobsilverberg bobsilverberg deleted the renovate/helmet-4.x branch September 17, 2020 16:02
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.

None yet

4 participants