Repository navigation
fix: harden server event and attribution boundaries - #35
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Add review records for sourcebuster, Odoo, Matomo, PostHog, Chatwoot, and trace-ids, following the existing oss-contributions format and index. Each record states the observed seam, the smallest useful contribution, the host-owned boundaries, and the evidence required before proposing. Only the sourcebuster change is implemented; the remainder are unposted reviews. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f074a1dfa
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ?? data['formId'] | ||
| ?? data['form_id'] | ||
| ?? `${input.identity.visitorId ?? ''}:${input.identity.sessionId ?? ''}`; |
There was a problem hiding this comment.
When two leads are submitted from the same form during one visitor session, formId (or the visitor/session fallback) is identical for both calls, so this produces the same event_id and a deduplicating collector silently discards the later conversion. This is also the path shown in the server README, where trackLead supplies only formId; callers without a unique lead ID should receive a per-occurrence ID or be required to provide an explicit stable key rather than deriving it from session/form identity.
Useful? React with 👍 / 👎.
| input.identity.visitorId ?? '', | ||
| input.identity.sessionId ?? '', | ||
| input.now ?? '', |
There was a problem hiding this comment.
When the same purchase, booking, or lead is retried with its stable business ID but a refreshed now, visitor cookie, or session cookie, these fields change the derived event_id, so the collector cannot deduplicate the retry. Once stableInput is a transaction/booking/lead identifier, the derivation should depend only on stable occurrence data such as site, canonical event name, and that identifier—not request-time identity or timestamp context.
Useful? React with 👍 / 👎.
| const payload = | ||
| typeof payloadRaw === 'string' && payloadRaw | ||
| ? ((safeJsonParse(payloadRaw) as AttributionPayload | null) ?? {}) | ||
| ? filterServerAttributionPayload(safeJsonParse(payloadRaw)) |
There was a problem hiding this comment.
When an older ct_attribution cookie contains the documented first_*/last_* field names, this new canonical-only filter drops every aliased attribution value even though parseIdentityFromCookies explicitly retains support for LEGACY_ATTRIBUTION_KEY. The browser storage read path normalizes these aliases before allowlisting; applying the same normalization here is necessary to avoid silently losing attribution for upgraded installations.
Useful? React with 👍 / 👎.
7f074a1 to
92f90e5
Compare
1 participant
Acceptance criteria
Every canonical server event has a non-empty stable event ID, untrusted cookie attribution is limited to canonical non-identity fields, and browser envelope routing/consent values cannot be overridden by nested caller data.
Changes
Validation
All passed in the isolated worktree. This PR intentionally leaves duplicated framework server adapters and generic integration builders for a follow-up parity change.