-
Notifications
You must be signed in to change notification settings - Fork 9
[COMP-1143]Missing HTTP Security Header #955
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: munishchouhan <[email protected]>
|
testing locally: |
|
Check all headers: |
Signed-off-by: munishchouhan <[email protected]>
|
tested in dev: |
|
tested augmentation: |
Signed-off-by: munishchouhan <[email protected]>
|
@claude review |
|
Claude finished @pditommaso's task —— View job Code Review Complete ✅Todo List:
Overall AssessmentThis is a well-implemented, secure, and production-ready HTTP security headers feature. The implementation follows Micronaut best practices and Wave's coding conventions. ✅ StrengthsExcellent Architecture
Comprehensive Security CoverageAll critical security headers implemented with sensible defaults:
Robust Configuration
Excellent Test CoverageThe test suite (
Production Quality Code
📝 Minor ObservationsCode Quality Notes
Security Best Practices Verified
Testing NotesThe tests properly verify:
🔍 Code-Specific Comments
|
| @Value('${wave.security.headers.hsts.max-age:31536000}') | ||
| Long hstsMaxAge |
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.
How this value has been chosen (please clarify it in the javadoc)?
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.
these are number of seconds in one year
i will add that in the javadocs
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.
done
| */ | ||
| @Nullable | ||
| @Value('${wave.security.headers.csp:default-src \'self\'; frame-ancestors \'none\'}') | ||
| String csp |
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.
Worth expanding to contentSecurityPolicy
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.
done
Signed-off-by: munishchouhan <[email protected]>
Signed-off-by: munishchouhan <[email protected]>
Overview
This PR implements HTTP security headers for all Wave service responses to improve security posture and protect against common web vulnerabilities.
Changes
1. HTTP Security Headers Implementation
New Files Created:
src/main/groovy/io/seqera/wave/configuration/SecurityHeadersConfig.groovysrc/main/groovy/io/seqera/wave/filter/SecurityHeadersFilter.groovysrc/test/groovy/io/seqera/wave/filter/SecurityHeadersFilterTest.groovyModified Files:
src/main/groovy/io/seqera/wave/filter/FilterOrder.groovysrc/main/resources/application.ymlSummary:
Implemented a new HTTP filter that adds industry-standard security headers to all Wave service responses. The filter runs with the highest priority (-120) to ensure security headers are always present.
Security Headers Added:
Strict-Transport-Securitymax-age=31536000; includeSubDomainsX-Frame-OptionsDENYX-Content-Type-OptionsnosniffReferrer-Policystrict-origin-when-cross-originPermissions-Policycamera=(), microphone=(), geolocation=()Content-Security-Policydefault-src 'self'; frame-ancestors 'none'Architecture:
Key Features:
application.ymlwave.security.headers.enabled: falseOrderedinterface for proper filter execution orderConfiguration:
Default configuration added to
application.yml:Filter Order:
Security Benefits