fix(analytics): validate telemetry event names and reject unknown body keys

[claude]

Requested by Olga Lopaci, Dimo Georgiev · Slack thread

What

Before. POST /api/analytics/send-event and POST /api/analytics/send-page accept requests from anyone — there is no authentication on either route — and whatever string the caller put in event became the event name in Segment, and from there the event name in Amplitude. The request body was not restricted to the fields the DTO declared, so callers could also send a traits object, which was spread verbatim into context.traits and landed in Amplitude as user properties, and a nonTracking: true flag, which made the backend send the event even when the user had not granted analytics consent. A vulnerability scanner found the endpoint and posted an out-of-band command-injection probe as the event name; those probe strings were ingested as real event names in the production Amplitude project and consumed the month's ingestion allowance.

After. Both endpoints only accept event names in the shape the application's own event names actually use, and reject any body field that is not part of the request contract. The scanner's probe payload now gets a 400, nothing reaches Segment, and the analytics consent decision can no longer be set by the caller. All real event names and page names continue to be accepted unchanged.

In one sentence: this validates the telemetry event name and closes the endpoints to unknown request-body fields.

How. event is now constrained with @Matches() — /^[A-Z0-9_]{1,128}$/ for events, which are SCREAMING_SNAKE_CASE constants, and a separate /^[A-Za-z0-9][A-Za-z0-9 /_-]{0,127}$/ for page views, which are human-readable labels like Search and Query and Pub/Sub. Because the two routes have genuinely different name shapes, send-page now binds a dedicated SendPageDto instead of reusing SendEventDto. The controller's ValidationPipe gains whitelist: true, forbidNonWhitelisted: true (matching database.controller.ts), so unknown keys are rejected rather than passed through. traits is removed from the request DTO and from the UI sender that was including the key on every request; the server-stamped telemetry: enabled|disabled trait is untouched, since dashboards depend on it. nonTracking is removed from the request DTO but stays on the internal ITelemetryEvent, so internal emitters that legitimately mark non-tracking events keep working.

One subtlety worth flagging for review: eventData had a @ValidateNested() decorator with no @Type() target. Under whitelist: true that combination makes class-validator reject every key inside eventData, which would have rejected all real events. It is now @IsObject(), which keeps the contents free-form while still requiring an object. There is a comment in the DTO recording why.

Deliberately not changed here, to keep this reviewable:

• eventData contents remain free-form. Real events carry many different shapes, so pinning them down needs its own pass.
• PATCH /api/settings has the same gap: settings.controller.ts also constructs its ValidationPipe without whitelist, so unknown keys sent to it are persisted into the settings record.
• app.enableCors() in main.ts is called with no origin allowlist, so any origin can make these calls from a browser.

Testing

Automated, added in this PR — both endpoints now assert that a malformed event/page name, a traits object, and a caller-supplied nonTracking flag each return 400 with the expected message. The pre-existing generated validation cases and the happy-path cases still pass unchanged.

# integration (against a local redis-server)
npm run test:api --prefix redisinsight/api   # scoped to test/api/analytics: 31 passing
                                             # scoped to test/api/settings:  45 passing

# unit
NODE_ENV=test jest -w 1 src/modules/analytics src/modules/settings   # 7 suites, 65 passing
jest 'redisinsight/ui/src/telemetry' -c jest.config.cjs              # 3 suites, 37 passing

# static
eslint [changed files]                                # clean
prettier --check [changed files]                      # clean
npm run type-check --prefix redisinsight/api          # no new errors vs .tscheck.rec.json
npm run type-check --prefix redisinsight/ui           # no new errors vs .tscheck.rec.json

The event-name pattern was checked against every event-name literal in the codebase rather than assumed: all 481 unique names in api/src/constants/telemetry-events.ts and ui/src/telemetry/events.ts match, and all 15 page names in ui/src/telemetry/pageViews.ts match the page pattern. Note the longest real event name is 66 characters (CONFIG_DATABASES_REDIS_CLOUD_AUTODISCOVERY_SUBSCRIPTIONS_SUCCEEDED), which is why the bound is 128 and not 64.

Manually, booting the controller over HTTP: real events with arbitrary eventData, the 66-character event name, and page names containing spaces and / all return 204; probe-style event names, traits, and nonTracking all return 400. Against main the same payloads all returned 204 and reached the analytics service with the attacker-controlled event name, the injected user properties, and nonTracking: true.

---

Membership validation against an allowlist of known event and page names was attempted on this branch and reverted — it is not part of this diff. See the deferral note for why it is follow-up work.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[github-actions]

Code Coverage - Backend unit tests

St.❔	Category	Percentage	Covered / Total
🟢	Statements	93.21%	16643/17855
🟡	Branches	75.66%	5389/7123
🟢	Functions	87.58%	2552/2914
🟢	Lines	93.07%	15919/17104

Test suite run success

3779 tests passing in 325 suites.

Report generated by 🧪jest coverage report action from a4a17f2

[github-actions]

Code Coverage - Integration Tests

Status	Category	Percentage	Covered / Total
🟡	Statements	79.88%	18448/23094
🟡	Branches	62.12%	8605/13850
🟡	Functions	67.9%	2509/3695
🟡	Lines	79.5%	17378/21858

[github-actions]

Code Coverage - Frontend unit tests

St.❔	Category	Percentage	Covered / Total
🟢	Statements	83.57%	28877/34553
🟡	Branches	69.68%	12291/17639
🟡	Functions	78.65%	7627/9697
🟢	Lines	84.03%	28070/33404

Test suite run success

8049 tests passing in 872 suites.

Report generated by 🧪jest coverage report action from a4a17f2

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4a17f2c27

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Enforce an allowlist for unauthenticated event names

When analytics consent is enabled, an unauthenticated caller can still submit arbitrary names such as UNRECOGNIZED_EVENT_123, because this pattern validates only their syntax; the controller then forwards them to Segment and returns 204. The same issue exists for the page-name pattern below, so an adapted client can continue polluting Amplitude with invented events and consuming the ingestion allowance this change is intended to protect. Validate against the application's actual event/page allowlists, or add authentication/rate limiting rather than relying only on name shape.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ceb926cae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[claude]

On the review point that the event-name patterns validate shape but not membership: confirmed, and deferring the fix to a follow-up rather than doing it here.

The finding is real. I booted the real AnalyticsController over HTTP and {"event": "UNRECOGNIZED_EVENT_123"} returns 204 and reaches analytics.track() with the caller's eventData intact; send-page behaves the same for any well-formed label. So this PR closes the injection-shaped and consent-override holes but not the unknown-name one.

I attempted a genuine allowlist on this branch and reverted it — the branch is back at a4a17f2. Two constraints a proper implementation has to meet, both of which my attempt got wrong:

1. The catalogue must live in a UI-neutral shared module that Vite can resolve, not in API source. I moved the enums into api/src/constants and had the UI re-export them through an apiSrc/* alias. That resolves for tsc, jest and the main UI Vite build, but .storybook/vite.config.ts aliases only uiSrc and apiClient, so the Storybook bundle cannot resolve the specifier — a tsconfig path mapping fixes type resolution but not bundling. It also puts the frontend importing backend source, against AGENTS.md:105 ("UI must not import from backend directly").

2. The HTTP route should validate against the client event catalogue only. I used the union of the client catalogue and the API's own TelemetryEvents enum (481 names). That admits ~72 backend-only names such as APPLICATION_STARTED, letting an unauthenticated caller forge events only trusted backend flows should emit — a new hole in place of the old one. The UI is already type-constrained to the client enum, so the union bought nothing.

Worth recording for the follow-up: an allowlist bounds the set of names but not the volume. The endpoint stays unauthenticated and unthrottled, and the API binds 0.0.0.0 by default, so replaying a legitimate name is still unbounded in self-hosted deployments. That is separate work from name validation.

---

Generated by Claude Code