Skip to content

fix(cdp): guard against malformed invocation globals in cyclotron-hog worker - #73076

Merged
meikelmosby merged 9 commits into
masterfrom
posthog-code/fix-cdp-cyclotron-malformed-globals
Jul 23, 2026
Merged

fix(cdp): guard against malformed invocation globals in cyclotron-hog worker#73076
meikelmosby merged 9 commits into
masterfrom
posthog-code/fix-cdp-cyclotron-malformed-globals

Conversation

@meikelmosby

@meikelmosby meikelmosby commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

A single malformed cdp_cyclotron_hog invocation — one whose state.globals is present but missing both project and event — crash-looped the cyclotron-hog worker and stalled the partition it owned. In loadHogFunctions, two enrichment calls run in one Promise.all: addGroupsToGlobals dereferences globals.project.id, and the person-stub branch dereferences globals.event.distinct_id. With those fields absent, both throw TypeError: Cannot read properties of undefined, which surfaces as an unhandled rejection → the worker exits → it restarts, re-reads the same offset, and crashes again. One poison-pill message takes down the consumer and stops the partition from draining.

The dereferences themselves have been unchanged for months — this is a latent bug triggered by data, not a code rollout.

Root cause context: every invocation deserialize path (Kafka/WarpStream, cyclotron postgres v1/v2, and the rerun rehydration path) casts the parsed JSON to its type with as and performs no shape validation. The live producer always sets project/event, but a message that arrives without them — a legacy/differently-shaped queued message, or a rerun rehydrated from an intentionally-minimized ClickHouse invocation_globals snapshot via a blind cast — flows straight into the unguarded derefs.

Changes

Two layers: harden the read sites so a bad message can't crash-loop the worker, and validate the shape at the most plausible source instead of blind-casting it.

  • cdp-cyclotron-worker.consumer.ts: loadHogFunctions validates globals.project/globals.event up front. A malformed invocation is logged, captureException'd, and routed to the existing dequeue path so it's dropped and the partition drains — instead of throwing out of the Promise.all. The guard sits before both crash sites.
  • groups-manager.service.ts: addGroupsToGlobals skips enrichment (sets groups = {}) when project/event is absent, as defense-in-depth (it's also called from the hogflow path).
  • schema/cyclotron.ts + rerun-paginator.service.ts: replace the blind parsedGlobals as HogFunctionInvocationGlobalsWithInputs cast in the rerun rehydration path with a zod safeParse. The new HogFunctionInvocationGlobalsSchema validates the invariants the worker relies on (project.{id,url}, event.{distinct_id,properties}) and stays permissive (passthrough) on everything else so drift on non-critical fields never rejects a good message. A row that fails validation is logged and skipped (counted as rehydrate_failed) rather than re-enqueued as a poison pill.

Still open (flagged, not in scope here): the Kafka/postgres deserialize paths remain unvalidated casts, and the equivalent loadHogFlows path in the hogflow worker lacks the same guard.

How did you test this code?

Test-first for the worker guard: added failing tests, confirmed they reproduced the exact TypeErrors, then applied the fix and confirmed green.

  • groups-manager.service.test.ts — two parameterized cases (missing project, missing event) asserting addGroupsToGlobals resolves with empty groups. Before the fix they failed with Cannot read properties of undefined (reading 'id') and (reading 'properties'); after, the suite passes. Regression caught: a partial globals reaching enrichment throws — no existing test exercised it, because the fixture always backfills project/event.
  • schema/cyclotron.test.ts — 8 cases: valid globals accepted; missing project/event and wrong-typed critical fields rejected; non-critical fields passed through. Regression caught: loosening the schema (or switching to strict) so a poison-pill shape validates, or so reruns silently drop inputs/request.
  • cdp-cyclotron-worker.test.ts — two cases asserting a malformed invocation is dequeued (DLQ'd), not executed. Regression caught: the worker crash-looping on a poison pill instead of dropping it. This test is DB-backed (resetTestDatabase/createHub) and could not run in the sandbox (no Postgres) — it mirrors the existing dequeue if the hog function cannot be found pattern and will run in CI.

Typecheck clean on all changed files; eslint clean. Runnable suites (groups-manager, schema/cyclotron) pass locally.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Investigated from a production incident thread: a cyclotron-hog worker stuck Progressing, one partition backed up. Confirmed the thesis before fixing — wrote the failing unit test that reproduces the crash, watched it fail with the reported stack traces, then implemented the guard and watched it pass. Traced the produce→serialize→enqueue→dequeue→deserialize path to establish that no deserialize step validates shape; on a follow-up pass, replaced the rerun path's blind as cast — the most plausible producer of malformed re-enqueued state — with schema validation. Chose to keep the schema permissive (validate only the dereferenced invariants) so it can't reject valid messages on incidental drift.

Skills invoked: /writing-tests.


Created with PostHog from a Slack thread

… worker

A cyclotron-hog invocation whose state.globals is present but missing
project or event caused unguarded dereferences (globals.project.id in
addGroupsToGlobals, globals.event.distinct_id in the person-stub branch)
to throw an unhandled rejection out of the loadHogFunctions Promise.all,
crash-looping the worker on a single poison-pill message and stalling the
partition it owned.

Guard both read sites: loadHogFunctions now drops a malformed invocation
via the existing dequeue path (logged + captured) instead of throwing, and
addGroupsToGlobals skips enrichment when project/event is absent.

Generated-By: PostHog Code
Task-Id: e16d9add-5ec4-4db3-9c24-cb3bdb036b4d
@meikelmosby
meikelmosby requested a review from a team July 23, 2026 07:23
@meikelmosby
meikelmosby marked this pull request as ready for review July 23, 2026 07:23
@trunk-io

trunk-io Bot commented Jul 23, 2026

Copy link
Copy Markdown

😎 Merged manually by @meikelmosby - details.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @meikelmosby! 👋

It looks like your git author email on this PR isn't your @posthog.com address (meikel-ratz@web.de). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "fix(cdp): guard against malformed invoca..." | Re-trigger Greptile

The rerun paginator re-enqueues invocations by casting a persisted (and
intentionally minimized) ClickHouse globals snapshot with `as`, trusting it
without checking shape. A snapshot written by an older serializer — or
otherwise drifted — can be missing project/event, which the cyclotron-hog
worker then dereferences unguarded: the source of the malformed poison-pill
state the worker guards hardened against.

Add a zod schema validating the invariants the worker relies on
(project.{id,url}, event.{distinct_id,properties}) while staying permissive
(passthrough) on non-critical fields, and use safeParse at the rehydration
boundary — a row that fails validation is logged and skipped rather than
re-enqueued as a poison pill.

Generated-By: PostHog Code
Task-Id: e16d9add-5ec4-4db3-9c24-cb3bdb036b4d
Generated-By: PostHog Code
Task-Id: e16d9add-5ec4-4db3-9c24-cb3bdb036b4d
@dmarchuk
dmarchuk enabled auto-merge (squash) July 23, 2026 07:52
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
auto-merge was automatically disabled July 23, 2026 08:08

Pull Request is not mergeable

@hex-security-app

Copy link
Copy Markdown

Warning

Finding on products/experiments/backend/session_context.py:124 — could not attach an inline comment (line is not part of the diff), so reporting it here.

🟡 Invalidate session-context cache on access changes

This cache is keyed only by user ID, so it returns a previously authorized result before the view evaluates the newly filtered experiments queryset. If a user loads session context containing a private experiment and an admin then revokes that user's experiment access, the same user can keep retrieving that private experiment's name, flag/variant, exposure time, and metric hits for five minutes. Cache entries need an authorization/version component or invalidation when experiment/resource access changes.

Prompt To Fix With AI
Make the session-context cache authorization-safe. Include a monotonically changing per-user/team experiment-access version in the cache key, or invalidate all relevant `experiment_session_context_{team}_{user}_*` entries whenever experiment access controls or membership/property access are changed. Add a regression test that primes the cache with access to a private experiment, revokes access for the same user, then verifies the endpoint immediately omits the experiment.

Severity: medium | Confidence: 96% | React with 👍 if useful or 👎 if not

@meikelmosby
meikelmosby enabled auto-merge (squash) July 23, 2026 08:20
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
auto-merge was automatically disabled July 23, 2026 08:46

Pull Request is not mergeable

@meikelmosby
meikelmosby enabled auto-merge (squash) July 23, 2026 09:42
Samielakkad

This comment was marked as resolved.

The rehydration schema validation added in this PR rejects the minimal
globals the routing tests construct, so the rerunnable-type cases would get
null back before their assertion. Give the fixture the project/event shape a
real invocation always carries so those cases pass validation and the tests
exercise the type gate as intended.

Generated-By: PostHog Code
Task-Id: e16d9add-5ec4-4db3-9c24-cb3bdb036b4d
@meikelmosby
meikelmosby merged commit 06066f0 into master Jul 23, 2026
183 checks passed
@meikelmosby
meikelmosby deleted the posthog-code/fix-cdp-cyclotron-malformed-globals branch July 23, 2026 10:34
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-23 10:57 UTC Run
prod-us ✅ Deployed 2026-07-23 11:17 UTC Run
prod-eu ✅ Deployed 2026-07-23 11:18 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants