Require authentication for Forge analytics report endpoint#15814
Require authentication for Forge analytics report endpoint#15814jamesfredley wants to merge 2 commits into
Conversation
The Forge analytics `POST /analytics/report` endpoint previously accepted unauthenticated writes, letting anyone inject fabricated project-generation statistics. Secure it with a Google-issued bearer JWT that is validated by signature (Google JWKS), issuer, audience, and a caller-subject claim pinned to the Forge web service account's unique id, so only the intended caller can report analytics. Public read endpoints (`GET /top/*`) stay anonymous. Also add an HSTS response filter to the Forge web (Netty) app and wire the analytics audience and caller-subject env vars through the Cloud Run deploy workflows and README. Assisted-by: claude-code:claude-opus-4-8
There was a problem hiding this comment.
Pull request overview
This PR hardens the Grails Forge analytics ingestion path by requiring authenticated writes to POST /analytics/report (preventing unauthenticated injection of fabricated stats), while keeping the “top/*” read endpoints publicly accessible. It also adds an HSTS response filter to the Forge web Netty service and wires new env vars through Cloud Run deploy workflows and documentation.
Changes:
- Require bearer JWT authentication for
POST /analytics/report, and explicitly markGET /analytics/top/*endpoints as anonymous. - Add an (enabled-by-default, configurable) HSTS response filter to the Forge web Netty app, with a new spec verifying the header.
- Update deploy workflows + README to provide
GRAILS_FORGE_ANALYTICS_AUDIENCEandGRAILS_FORGE_ANALYTICS_CALLER_SUBJECT.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-forge/README.md | Documents new analytics auth env vars for Forge + analytics services. |
| grails-forge/grails-forge-web-netty/src/test/groovy/org/grails/forge/netty/security/HstsHeaderFilterSpec.groovy | Adds test coverage verifying the HSTS header is applied. |
| grails-forge/grails-forge-web-netty/src/main/resources/application.yml | Introduces HSTS config defaults and passes analytics audience config to the GCP HTTP client auth config. |
| grails-forge/grails-forge-web-netty/src/main/java/org/grails/forge/netty/security/HstsHeaderFilter.java | Adds a global response filter that sets Strict-Transport-Security when enabled. |
| grails-forge/grails-forge-web-netty/build.gradle | Adds Groovy/Spock test dependencies needed for the new Spock spec. |
| grails-forge/grails-forge-analytics-postgres/src/test/groovy/org/grails/forge/analytics/postgres/StoreGeneratedProjectStatsSpec.groovy | Updates integration-style test to send an authorized bearer token. |
| grails-forge/grails-forge-analytics-postgres/src/test/groovy/org/grails/forge/analytics/postgres/AnalyticsControllerSecuritySpec.groovy | Adds security-focused tests for rejecting unauthenticated/invalid tokens and allowing anonymous reads. |
| grails-forge/grails-forge-analytics-postgres/src/main/resources/application.yml | Enables bearer JWT auth + Google JWKS verification config and configures caller subject pinning. |
| grails-forge/grails-forge-analytics-postgres/src/main/java/org/grails/forge/analytics/postgres/AnalyticsController.java | Secures the report endpoint and explicitly allows anonymous access to read endpoints. |
| grails-forge/grails-forge-analytics-postgres/src/main/java/org/grails/forge/analytics/postgres/AnalyticsCallerJwtClaimsValidator.java | Adds a custom JWT claims validator to pin sub to the expected caller subject. |
| grails-forge/grails-forge-analytics-postgres/build.gradle | Adds Micronaut JWT security dependency and Groovy/Spock test dependencies. |
| .github/workflows/forge-deploy-snapshot.yml | Wires GRAILS_FORGE_ANALYTICS_AUDIENCE and sets GRAILS_FORGE_ANALYTICS_CALLER_SUBJECT for analytics deployment. |
| .github/workflows/forge-deploy-release.yml | Wires GRAILS_FORGE_ANALYTICS_AUDIENCE and sets GRAILS_FORGE_ANALYTICS_CALLER_SUBJECT for analytics deployment (latest + versioned). |
| .github/workflows/forge-deploy-prev.yml | Wires GRAILS_FORGE_ANALYTICS_AUDIENCE and sets GRAILS_FORGE_ANALYTICS_CALLER_SUBJECT for analytics deployment. |
| .github/workflows/forge-deploy-prev-snapshot.yml | Wires GRAILS_FORGE_ANALYTICS_AUDIENCE and sets GRAILS_FORGE_ANALYTICS_CALLER_SUBJECT for analytics deployment. |
| .github/workflows/forge-deploy-next.yml | Wires GRAILS_FORGE_ANALYTICS_AUDIENCE and sets GRAILS_FORGE_ANALYTICS_CALLER_SUBJECT for analytics deployment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @MockBean(ApplicationRepository) | ||
| @NonNull | ||
| ApplicationRepository applicationRepository() { | ||
| Mock(ApplicationRepository) | ||
| } | ||
|
|
||
| @MockBean(JdbcOperations) | ||
| @NonNull | ||
| JdbcOperations jdbcOperations() { | ||
| Mock(JdbcOperations) | ||
| } |
| void 'top analytics data is not rejected as unauthenticated'() { | ||
| when: | ||
| client.toBlocking().exchange(HttpRequest.GET('/top/features'), String) | ||
|
|
||
| then: | ||
| HttpClientResponseException e = thrown(HttpClientResponseException) | ||
| e.status == HttpStatus.INTERNAL_SERVER_ERROR | ||
| 0 * applicationRepository._ | ||
| } |
| } | ||
|
|
||
| @Get("/top/features") | ||
| @Secured(SecurityRule.IS_ANONYMOUS) |
There was a problem hiding this comment.
I'm confused - these indicate they're anonymous but your'e requiring authentication?
There was a problem hiding this comment.
@jamesfredley isn't the point of this to ensure we're authenticated? Shouldn't all of these IS_ANONYMOUS be removed for the analytics endpoint?
Assisted-by: opencode:gpt-5.5
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #15814 +/- ##
==================================================
- Coverage 49.4842% 49.3657% -0.1185%
- Complexity 16697 16782 +85
==================================================
Files 1947 1986 +39
Lines 92474 93336 +862
Branches 16152 16337 +185
==================================================
+ Hits 45760 46076 +316
- Misses 39606 40135 +529
- Partials 7108 7125 +17 🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: fa5214f Learn more about TestLens at testlens.app. |
|
The tests are failing to authenticate: AnalyticsControllerSecuritySpec > top analytics data is not rejected as unauthenticated FAILED StoreGeneratedProjectStatsSpec > test save generation data FAILED |
The Forge analytics POST /analytics/report endpoint previously accepted unauthenticated writes, allowing anyone to inject fabricated project-generation statistics into the analytics database.
Changes
Tests
Both focused specs pass locally.