Add implementation plan for linking PostHog to anonymous extension usage#128
Add implementation plan for linking PostHog to anonymous extension usage#128pedroheyerdahl wants to merge 5 commits intomainfrom
Conversation
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/functions/limitations |
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (1 files)
|
pedroheyerdahl
force-pushed
the
feat/link-posthog-distinct-id-to-anon-usage
branch
from
f9a5576 to
15286b3
Compare
…sers Add security note explaining that PostHog's distinct_id uses email for authenticated users (set via posthog.identify()) and header value for anonymous users. Documents that PostHog links both states via alias() call for cross-state query support.
| let anonymousId = context.globalState.get<string>(STORAGE_KEY); | ||
|
|
||
| if (!anonymousId) { | ||
| const machineId = vscode.env.machineId; |
There was a problem hiding this comment.
WARNING: Using vscode.env.machineId (even hashed) can still be treated as personal data / a stable identifier
machineId is designed to be stable across time and can enable long-lived tracking. Even though you hash and truncate it, it’s still a deterministic per-machine identifier and may raise privacy / store-policy concerns. If the goal is just to correlate anonymous sessions, consider generating a random UUID once (stored in globalState) instead of deriving it from machineId, unless you explicitly need cross-reinstall stability and have confirmed policy compliance.
Remove regex character restrictions from distinct_id validation, keeping only length limit (255 chars) to prevent DB bloat. This supports all PostHog distinct_id formats including ULIDs (e.g., "01JFKX...") while maintaining security against malicious payloads. Also fix SQL query to prevent division by zero in coverage calculation.
| // NEW - line ~250 | ||
| // Validate PostHog distinct_id from header (length limit only to prevent bloat) | ||
| const rawDistinctId = request.headers.get('x-posthog-distinct-id'); | ||
| const posthogDistinctIdFromHeader = rawDistinctId && rawDistinctId.length <= 255 |
There was a problem hiding this comment.
WARNING: trim() can produce an empty string that still gets persisted
In the suggested backend parsing, rawDistinctId.trim() can become "" (e.g., header value is only whitespace). Because "" is not nullish, the later posthogDistinctIdFromHeader ?? undefined keeps it, so you can end up storing empty posthog_distinct_id values. Consider treating "" as undefined after trimming to avoid polluting analytics/coverage queries.
1 participant
No description provided.