-
Notifications
You must be signed in to change notification settings - Fork 397
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(gnoweb): add secure headers by default & timeout configuration #3619
Conversation
Signed-off-by: gfanton <[email protected]>
Signed-off-by: gfanton <[email protected]>
🛠 PR Checks Summary🔴 Must not contain the "don't merge" label Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Signed-off-by: gfanton <[email protected]>
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Overall LGTM 👍 I just asked a question bellow about your strict mode
Co-authored-by: Antoine Eddi <[email protected]>
Can a user upload a .js file as part of their realm and use this to bypass CSP projection due to |
This is a good resource for security headers: Notwithstanding my above query, the other headers set look good. Another header we might want to consider:
Once this is deployed somewhere we should double check headers with https://securityheaders.com/ |
@gfanton I fixed the inline CSS issue. It should be better now, even if the render should not be as fast as before (If this causes a FOUSVG issue, I will create a dedicated PR 👍) |
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.
good after changes
…security` header Co-authored-by: Morgan <[email protected]>
just waiting for @sw360cab for last review, then we can merge |
LGTM 👍 Headers should be automatically forwarded back and forth in case Gnoweb is behind Proxy/Gateway |
Related to #3619, this PR is a hotfix for blog images from [gno.land](https://gnolang.github.io/) until a proper generic whitelisted image provider is implemented. Or do we want to use base64 and replace all images from blog and realm instead?
This PR adds the following secure headers by default
strict=true
to gnoweb:I've also enforced a timeout on read/write/idle (default to 1 minute).
cc @kristovatlas