-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(graphql): add feature flag and CSP headers for GraphiQL endpoint #15364
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
Adds configuration to disable GraphiQL in air-gapped/production environments and implements Content Security Policy headers to address security scanner findings. Changes: - Add @ConditionalOnProperty to GraphiQLController for enabling/disabling endpoint - Add graphql.graphiql.enabled configuration property (defaults to true for backward compatibility) - Implement CSP headers (script-src, style-src, connect-src, img-src) - Add X-Content-Type-Options and X-Frame-Options security headers - Add spring-boot-autoconfigure dependency and update gradle lockfile Configuration: - Set GRAPHQL_GRAPHIQL_ENABLED=false to disable in air-gapped or production environments - Endpoint returns 404 when disabled Security improvements: - Content-Security-Policy header allows self and unpkg.com sources - X-Content-Type-Options: nosniff prevents MIME-sniffing attacks - X-Frame-Options: DENY prevents clickjacking Resolves air-gapped deployment blockers and satisfies OWASP ZAP security requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
cb738e6 to
2be6da6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
chriscollins3456
left a comment
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.
this makes enough sense, but not sure about the CSP headers
| () -> { | ||
| // Add Content Security Policy headers to allow unpkg.com for external resources | ||
| response.setHeader( | ||
| "Content-Security-Policy", | ||
| "default-src 'self'; " | ||
| + "script-src 'self' https://unpkg.com 'unsafe-inline'; " | ||
| + "style-src 'self' https://unpkg.com 'unsafe-inline'; " | ||
| + "img-src 'self' data:; " | ||
| + "connect-src 'self'"); | ||
|
|
||
| response.setHeader("X-Content-Type-Options", "nosniff"); | ||
| response.setHeader("X-Frame-Options", "DENY"); | ||
|
|
||
| return this.graphiqlHtml; | ||
| }, | ||
| this.getClass().getSimpleName(), | ||
| "graphiQL"); |
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 feel confident about these changes and what they're doing for us. i would recommend double checking with someone from the platform team
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 changes are not mandatory but are required to satisfy the security compliance issue for enterprise customers. Security scanners like OWASP flag the lack of CSP headers and reported them as security vulnerability.
This has been reported in community slack.
https://datahubspace.slack.com/archives/CV2KB471C/p1762287974158799?thread_ts=1761770048.446169&cid=CV2KB471C
Summary
This PR adds a configuration flag to disable the GraphiQL endpoint and implements Content Security Policy (CSP) headers to address security scanner findings. This resolves blockers for air-gapped deployments and satisfies OWASP ZAP security requirements.
Changes
1. Feature Flag for GraphiQL Endpoint
@ConditionalOnPropertyannotation toGraphiQLControllergraphql.graphiql.enabled=falsetruefor backward compatibility2. Security Headers Implementation
script-src 'self' https://unpkg.com 'unsafe-inline'style-src 'self' https://unpkg.com 'unsafe-inline'img-src 'self' data:connect-src 'self'3. Configuration Updates
graphql.graphiql.enabledproperty inapplication.yamlGRAPHQL_GRAPHIQL_ENABLED4. Dependency Updates
spring-boot-autoconfiguredependency to support@ConditionalOnPropertyUse Cases
Air-Gapped Deployments
Customers in air-gapped environments can now disable the GraphiQL endpoint that requires external CDN access:
export GRAPHQL_GRAPHIQL_ENABLED=falseProduction Security
Organizations can disable developer tools in production:
Security Compliance
CSP headers now satisfy security scanner requirements (OWASP ZAP, Burp Suite, etc.)
Testing
Files Modified
metadata-service/graphql-servlet-impl/src/main/java/com/datahub/graphql/GraphiQLController.javametadata-service/configuration/src/main/resources/application.yamlmetadata-service/graphql-servlet-impl/build.gradlemetadata-service/graphql-servlet-impl/gradle.lockfileRelated Issues
Addresses security and air-gapped deployment concerns mentioned in internal discussions.
🤖 Generated with Claude Code