Don't upgrade insecure requests when the page is served over HTTP

[tamoreton]

Duplicate of github/secure_headers#348 but for helmet.

tl;dr Helmet sets upgrade-insecure-requests header by default. This is fine, but Safari honours that header even if the website is being served over HTTP, which means that if I'm doing local web development, assets (styles, scripts etc.) get upgraded to HTTPS and don't get loaded.

This behaviour is quite hard to reason about—I've encountered it a few times before and only just (with the help of this Stack Overflow thread) figured out why it's happening.

The current workaround is to put some branching in the consumer layer:

app.use(
  helmet.contentSecurityPolicy({
    directives: {
      defaultSrc: ["'self'"],
      scriptSrc: [
        "'self'",
        (req, res) => `'nonce-${res.locals.cspNonce}'`
      ],
      imgSrc: ["'self'"],
      connectSrc: ["'self'"],
      ...(process.env.NODE_ENV === 'development' && {
        upgradeInsecureRequests: null
      })
    }
  })
)

[EvanHahn]

I'm surprised I haven't heard about this before. I guess most people develop using Chromium or Firefox.

I'd like to address this if we can ensure upgrade-insecure-requests will be set in production. A lot of people just dump app.use(helmet()) in their code and I want to continue to support that.

My first thought would be to add a process.env.NODE_ENV check to Helmet, similar to what you have. However, I don't know if I feel comfortable trusting that across all of Helmet's deployments. I worry that someone has NODE_ENV set to the wrong value in production, and I don't necessarily want to impact their security.

Can you think of a way to address this without breaking anything? Or can you convince me that the NODE_ENV check is actually fine?

[tamoreton]

There'll be something on req that will tell us whether a page is being served over HTTP or HTTPS – could that be an option? As long as we do it in a way that doesn't effectively disable the behaviour of this header.

[EvanHahn]

I don't think we can use that for two reasons:

1. Helmet is designed to work with any Node.js web framework, not just Express. req.secure, which is the flag you're describing, is an Express exclusive.

2. Even if we could use it, I'm not sure how it would help. How would we decide that someone is using HTTP that we shouldn't upgrade?