Add test to exercise override opting out without default_src

[rafaelfranca]

On secure_headers 5.x it was possible to override the CSP directives when optin out without having to define a default_src.

Now on 6.x it is required to set the default_src when overriding other directives.

It is not clear in the CHANGELOG/upgrade guide if this change is by design or if it is just a side effect of other changes.

I could not find anything in the spec that says that default_src is required or not, so I decided to open a PR with a test to get feedback on that.

If this is undesirable behavior I'm willing to change this PR to fix the problem.

Let me know what are the next steps.

Thanks.

[oreoshake]

👋 hello, thanks for raising this.

It is not clear in the CHANGELOG/upgrade guide if this change is by design or if it is just a side effect of other changes.

It appears to have been fixed as a side effect of other changes, not exactly sure when. default_src has been required for base policies since 3.0.

https://github.com/github/secure_headers/blob/f3d3f9d6b09c925ba75aa1d7fea1978b7dbe14ce/spec/lib/secure_headers/headers/policy_management_spec.rb#L59:L63

[rafaelfranca]

That makes sense. So maybe override_content_security_policy_directives when the csp is opt-out, should build a new csp directive based on the default config, instead of a blank one?

[oreoshake]

So maybe override_content_security_policy_directives when the csp is opt-out, should build a new csp directive based on the default config, instead of a blank one?

I think that behavior would be a little surprising. Declaring OPT_OUT and then overriding (to me) indicates that you wouldn't to build on the default policy that you declared you didn't want to use.

This feels a bit edge casey so I'm trying to keep an open mind. It seems in the example provided, the "fix" to get the desired policy isn't too burdensome to add and would be obvious/self-documenting.

I'd be curious to see what others think.

[rafaelfranca]

I think the problem with asking the override the define default_url on the override_content_security_policy_directives call is that if your are using OPT_OUT you will get your desired override and the default url specified, but if you are using something that already have a default_url the override_content_security_policy_directives will also override the default_url even if you don't want that to happen.

This requires your code to have to check if default_url is set and only override the default_url it it is not already set.

In other words, my method call in the app needs now to be something like:

if SecureHeaders.config_for(request).csp['default_src']
  SecureHeaders.override_content_security_policy_directives(request, { frame_ancestors: %w('none') }, :enforced)
else
  SecureHeaders.override_content_security_policy_directives(request, { frame_ancestors: %w('none'), default_src: %w('self' https:), script_src: %w(https:) }, :enforced)
end

[oreoshake]

Interesting! I see your point. Can you describe your use a little more? Or does this about sum it up:

1. Your default policy is opt-out
2. You have various hooks to set a unique CSP per domain
3. You have various hooks to override part of that domain-specific CSP

That's an interesting strategy and but I will not say it's wrong. That being said, I still don't think adding the surprise default CSP that can change over time is the right answer. I'll have to think on it some more but I'm not seeing an obvious path forward. Did you have anything in mind?

[oreoshake]

Adding this as a public commitment to resolve this issue in the near future.