-
Notifications
You must be signed in to change notification settings - Fork 34
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
Fix: enable secure header middleware by default #1601
Conversation
🦋 Changeset detectedLatest commit: caaece5 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Related: #1602 |
Hoping for other opinions, but I don't mind the change. I'd like your thoughts on whether adding some form of warning / documentation linking is suitable here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etaoins do you have any thoughts as Koala BDFL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind this either. This is the big one because it can break loading all HTTP resources hosted on the same domain:
The app's domain does not mix HTTP and HTTPS
However, that's become even less common in the 5 years since this documentation was written. The other caveats only have a local effect and would presumably be caught during testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a patch changeset, please?
Co-authored-by: Aaron Moat <2937187+AaronMoat@users.noreply.github.com>
Idk why we wouldn't have this?
https://github.com/seek-oss/koala/tree/master/src/secureHeaders