Feat: Add Stape server-side tagging integration #1411
Conversation
|
Warning Rate limit exceeded@yuriassuncx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds a new Stape server-side tagging app: app registration, app factory/preview, generated manifest, typed client/types, fetch/gdpr/tracking/security/event utilities, multiple loaders and server-side actions, server-only sections, and comprehensive README documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser as User
participant Loader as Section Loader
participant GDPR as GDPR Utils
participant Tracking as Tracking Utils
participant Action as Stape Action
participant Fetch as fetchStapeAPI
participant Stape as Stape Container
Browser->>Loader: page request
Loader->>GDPR: extractConsentFromHeaders(cookieHeader)
Loader->>Tracking: buildTrackingContext(req, enableGdprCompliance, consentCookieName)
Loader->>Tracking: createPageViewEvent(context,...)
alt consent granted
Loader->>Action: call tracking action (payload)
Action->>Fetch: fetchStapeAPI(containerUrl, payload, userAgent, clientIp, timeout)
Fetch->>Stape: POST /gtm (events, ids, consent)
Stape-->>Fetch: 2xx / error
Fetch-->>Action: FetchResult
Action-->>Loader: result
else consent denied
Loader-->>Loader: skip send, return metadata
end
Loader-->>Browser: HTML response (hidden config)
sequenceDiagram
autonumber
actor Client as Browser
participant ScriptLoader as analyticsScript Loader
participant Remote as Remote Script Host
participant Fallback as Fallback Generator
Client->>ScriptLoader: GET /analyticsScript
ScriptLoader->>Remote: resolve & fetch script URL
alt remote fetch OK
Remote-->>ScriptLoader: JS (200)
ScriptLoader-->>Client: JS (with cache headers)
else remote fetch fails or missing config
ScriptLoader->>Fallback: createFallbackScript()
Fallback-->>ScriptLoader: fallback JS
ScriptLoader-->>Client: fallback JS
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (12)
stape/loaders/getStapeConfig.ts (2)
37-40: Validate list response and handle empty/invalid bodies safelyProtect against non‑arrays and empty lists to avoid runtime errors on
.length.- const response = await api["GET /api/v2/containers"]({}); - const containers = await response.json(); - return containers.length > 0 ? containers[0] : null; + const response = await api["GET /api/v2/containers"]({}); + if (!response.ok) { + console.error("Stape containers list failed:", response.status, await response.text()); + return null; + } + const containers = await response.json(); + return Array.isArray(containers) && containers.length > 0 ? containers[0] : null;
1-2: Use type‑only import forContainerMinor tree‑shaking/TS hygiene: import as type.
-import { Container } from "../utils/types.ts"; +import type { Container } from "../utils/types.ts";stape/sections/Analytics/Stape.tsx (1)
60-70: Hide debug DOM in productionAvoid emitting hidden debug nodes unless
debugModeis enabled.- return ( + return debugMode ? ( <div style={{ display: "none" }} data-stape-config="true" data-container-url={containerUrl} data-gtm-id={gtmContainerId} data-gdpr-enabled={enableGdprCompliance?.toString()} data-server-tracking={enableServerSideTracking?.toString()} /> - ); + ) : null;stape/loaders/analyticsScript.ts (2)
20-33: Graceful “no container” response is fine; consider 204Not blocking, but returning 204 (No Content) instead of JS comment can reduce bytes on wire.
- return new Response( - "// Stape container URL not configured", - { - status: 200, + return new Response("", { + status: 204, headers: { - "Content-Type": "text/javascript", "Cache-Control": "public, max-age=300", }, - } - ); + });
50-59: Stream script body; add Vary and stronger content typeAvoid buffering large scripts; stream the body through and set
Vary: User-Agent.- const scriptContent = await response.text(); - - return new Response(scriptContent, { + const headers = new Headers(response.headers); + headers.set("Content-Type", "application/javascript"); + headers.set("Cache-Control", "public, max-age=3600"); + headers.set("Access-Control-Allow-Origin", "*"); + headers.set("Vary", "User-Agent"); + return new Response(response.body, { status: 200, - headers: { - "Content-Type": "text/javascript", - "Cache-Control": "public, max-age=3600", // Cache for 1 hour - "Access-Control-Allow-Origin": "*", - }, + headers, });stape/actions/sendEcommerceEvent.ts (2)
116-128: Improve client IP extraction and header propagationConsider common proxy headers and set a sane UA fallback.
- const stapeUrl = new URL('/gtm', containerUrl); - const userAgent = req.headers.get("user-agent") || ""; - const forwarded = req.headers.get("x-forwarded-for"); - const clientIp = forwarded ? forwarded.split(",")[0].trim() : "127.0.0.1"; + const stapeUrl = new URL("/gtm", containerUrl); + const userAgent = req.headers.get("user-agent") || "Deco-Stape-Server/1.0"; + const forwarded = req.headers.get("x-forwarded-for") + || req.headers.get("cf-connecting-ip") + || req.headers.get("true-client-ip") + || req.headers.get("x-real-ip"); + const clientIp = forwarded ? forwarded.split(",")[0].trim() : "127.0.0.1";
121-131: Add timeout and Accept header; avoid hanging request threadsBound the external call and advertise JSON.
- const response = await fetch(stapeUrl.toString(), { + const ac = new AbortController(); + const t = setTimeout(() => ac.abort(), 5000); + const response = await fetch(stapeUrl.toString(), { method: 'POST', headers: { 'Content-Type': 'application/json', + 'Accept': 'application/json', 'User-Agent': userAgent, 'X-Forwarded-For': clientIp, 'X-Real-IP': clientIp, }, body: JSON.stringify(payload), - }); + signal: ac.signal, + }); + clearTimeout(t);stape/actions/sendBasicEvent.ts (1)
58-68: Propagate client IPs and set UA fallback; add timeoutForwarding IPs improves attribution and anti‑fraud; add timeout to avoid long blocking.
- const stapeUrl = new URL('/gtm', containerUrl); - const userAgent = req.headers.get("user-agent") || ""; + const stapeUrl = new URL("/gtm", containerUrl); + const userAgent = req.headers.get("user-agent") || "Deco-Stape-Server/1.0"; + const forwarded = req.headers.get("x-forwarded-for") + || req.headers.get("cf-connecting-ip") + || req.headers.get("true-client-ip") + || req.headers.get("x-real-ip"); + const clientIp = forwarded ? forwarded.split(",")[0].trim() : "127.0.0.1"; - const response = await fetch(stapeUrl.toString(), { + const ac = new AbortController(); + const t = setTimeout(() => ac.abort(), 5000); + const response = await fetch(stapeUrl.toString(), { method: 'POST', headers: { 'Content-Type': 'application/json', - 'User-Agent': userAgent, + 'Accept': 'application/json', + 'User-Agent': userAgent, + 'X-Forwarded-For': clientIp, + 'X-Real-IP': clientIp, }, body: JSON.stringify(eventData), - }); + signal: ac.signal, + }); + clearTimeout(t);stape/actions/sendEvent.ts (2)
73-76: Support optional API key header and stronger IP derivationIf ctx.apiKey is configured, forward it. Prefer real IP headers when present.
- const userAgent = req.headers.get("user-agent") || ""; - const forwarded = req.headers.get("x-forwarded-for"); - const clientIp = forwarded ? forwarded.split(",")[0].trim() : "127.0.0.1"; + const userAgent = req.headers.get("user-agent") || ""; + const forwarded = req.headers.get("x-forwarded-for"); + const realIp = + req.headers.get("x-real-ip") || + req.headers.get("cf-connecting-ip") || + req.headers.get("true-client-ip") || + undefined; + const clientIp = (forwarded?.split(",")[0].trim() || realIp || "127.0.0.1"); @@ headers: { "Content-Type": "application/json", "User-Agent": userAgent, "X-Forwarded-For": clientIp, "X-Real-IP": clientIp, + ...(ctx.apiKey ? { "X-Api-Key": String(ctx.apiKey) } : {}), },Please confirm the exact header Stape expects for API auth (e.g., X-Api-Key vs Authorization: Bearer). I’ll adjust accordingly.
Also applies to: 93-99
78-86: Validate input: require at least one eventReturn a clear error if events are missing.
- const eventPayload: StapeEventRequest = { + if (!props.events?.length) { + return { success: false, message: "No events provided" }; + } + const eventPayload: StapeEventRequest = { events: props.events,stape/actions/trackEcommerceEvent.ts (2)
86-111: Avoid duplicating GA4 fields inside params; add event_id for dedupclient_id/user_id/timestamp_micros should be top-level. Include event_id in params; GA4 uses it for deduplication.
- const eventData = { + const eventData = { events: [{ name: eventName, params: { // Spread all event parameters ...eventParams, - // Add additional tracking data - client_id: clientId, - user_id: props.userId || undefined, - timestamp_micros: Date.now() * 1000, + // Recommended for deduplication + event_id: eventId, page_location: req.url, page_referrer: req.headers.get("referer") || "", ...props.additionalParams, }, }], gtm_container_id: gtmContainerId, client_id: clientId, user_id: props.userId || undefined, + timestamp_micros: Date.now() * 1000, consent: { ad_storage: hasConsent ? "granted" : "denied", analytics_storage: hasConsent ? "granted" : "denied", ad_user_data: hasConsent ? "granted" : "denied", ad_personalization: hasConsent ? "granted" : "denied", }, };If Stape’s /gtm client expects a different placement for timestamp/client_id, share the spec and I’ll align it precisely.
97-100: Consider sourcing page_location from the actual page contextreq.url is the API route URL, not the user-facing page URL. If available, pass page_location/page_referrer via props to improve attribution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
stape/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (16)
deco.ts(1 hunks)stape/README.md(1 hunks)stape/actions/sendBasicEvent.ts(1 hunks)stape/actions/sendEcommerceEvent.ts(1 hunks)stape/actions/sendEvent.ts(1 hunks)stape/actions/testConnection.ts(1 hunks)stape/actions/trackEcommerceEvent.ts(1 hunks)stape/actions/trackPageView.ts(1 hunks)stape/loaders/analyticsScript.ts(1 hunks)stape/loaders/getStapeConfig.ts(1 hunks)stape/manifest.gen.ts(1 hunks)stape/mod.ts(1 hunks)stape/sections/Analytics/Stape.tsx(1 hunks)stape/sections/StapeTracker.tsx(1 hunks)stape/utils/client.ts(1 hunks)stape/utils/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
stape/loaders/analyticsScript.ts (1)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)
stape/actions/sendEcommerceEvent.ts (2)
stape/utils/types.ts (2)
Item(79-102)EventData(47-50)stape/mod.ts (1)
AppContext(9-9)
stape/actions/trackEcommerceEvent.ts (2)
stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (2)
Props(11-45)AppContext(9-9)
stape/sections/StapeTracker.tsx (6)
stape/actions/sendBasicEvent.ts (1)
Props(3-27)stape/actions/sendEvent.ts (1)
Props(4-51)stape/actions/trackPageView.ts (1)
Props(3-39)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/sections/Analytics/Stape.tsx (2)
Props(4-31)loader(73-106)website/mod.ts (1)
SectionProps(31-33)
stape/mod.ts (3)
stape/manifest.gen.ts (1)
Manifest(37-37)utils/http.ts (1)
createHttpClient(134-240)stape/utils/client.ts (1)
StapeClient(3-68)
stape/sections/Analytics/Stape.tsx (4)
stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/sections/StapeTracker.tsx (2)
Props(4-37)loader(97-192)website/mod.ts (1)
SectionProps(31-33)
stape/actions/sendEvent.ts (2)
stape/mod.ts (1)
AppContext(9-9)stape/utils/types.ts (2)
StapeEventRequest(123-136)EventData(47-50)
stape/actions/sendBasicEvent.ts (2)
stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (2)
Props(11-45)AppContext(9-9)
stape/utils/client.ts (1)
stape/utils/types.ts (3)
Container(3-12)CapiGateway(14-24)StapeGateway(26-35)
stape/loaders/getStapeConfig.ts (2)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/utils/types.ts (1)
Container(3-12)
stape/actions/testConnection.ts (1)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)
stape/actions/trackPageView.ts (1)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)
🪛 Biome (2.1.2)
stape/mod.ts
[error] 59-59: Shouldn't redeclare 'App'. Consider to delete it or rename it.
'App' is defined here:
(lint/suspicious/noRedeclare)
🪛 markdownlint-cli2 (0.18.1)
stape/README.md
325-325: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
stape/actions/testConnection.ts (1)
55-67: Add timeout/abort and forward client IP; confirm API key usage
- Use
AbortControllerto abort the POST after ~8 s and clear the timer to prevent hanging.- Extract client IP from
req.headers(e.g.x-forwarded-for/x-real-ip) and forward it asX-Forwarded-For/X-Real-IP.- If your Stape container endpoint needs an auth header, include it here (e.g.
X-Api-KeyorAuthorization: Bearer …); please confirm the expected header name.stape/actions/trackEcommerceEvent.ts (1)
113-126: Add fetch timeout and optional API key header
- Wrap the Stape container request in an
AbortControllerwith a timeout (e.g. 8 s) and clear it after completion.- In the headers, spread
...(ctx.apiKey && { "X-Api-Key": ctx.apiKey })so secured containers work—confirm which header your container expects.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…default Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
stape/actions/sendEcommerceEvent.ts (2)
204-210: Redundant check: Line 205 re-checks analytics consent.Line 205 sets
events: []ifanalytics_storage === "denied", but lines 185-193 already return early in this case. The array will never be empty here—this condition is unreachable.Apply this diff to remove the redundant check:
const payload = { - events: consentData.analytics_storage === "denied" ? [] : [eventData], + events: [eventData], client_id: props.clientId || crypto.randomUUID(), user_id: props.userId, timestamp_micros: Date.now() * 1000, consent: consentData, };
88-99: Consider extracting event params builder to reduce complexity.The event parameter construction logic (lines 90-99) is straightforward but could be extracted into a small helper function if similar logic appears in other actions. This would reduce duplication and make testing easier.
stape/sections/Analytics/Stape.tsx (2)
105-105: Timestamp is computed twice; reuse pageViewData.eventParams.timestamp_micros.Lines 105 and 118 both call
Date.now() * 1000. The second call can drift slightly and creates unnecessary duplication.Apply this diff:
eventParams: { page_location: req.url, page_title: "Page View", timestamp_micros: Date.now() * 1000, ...props.customParameters, }, }; try { // Send basic event directly using Stape API const eventData = { events: [{ name: pageViewData.eventName, params: pageViewData.eventParams, }], client_id: crypto.randomUUID(), - timestamp_micros: Date.now() * 1000, + timestamp_micros: pageViewData.eventParams.timestamp_micros, };Also applies to: 118-118
73-164: Consider extracting shared server-side tracking logic into a utility.The loader in
stape/sections/Analytics/Stape.tsx(lines 73-164) and the loader instape/sections/StapeTracker.tsx(lines 97-228) share near-identical patterns:
- GDPR consent parsing from cookies
- client_id extraction/generation (though currently inconsistent)
- Event payload construction and dispatch to
/gtm- Error handling and debug logging
This duplication increases maintenance burden and the risk of divergence (e.g., the client_id stability fix needed in both places).
Refactor the common logic into a shared utility function (e.g.,
stape/utils/serverTracking.ts) that accepts:
req: Requestctx: AppContexteventName: stringeventParams: Record<string, unknown>debugMode?: booleanand returns
{ success: boolean, clientId?: string }. Both loaders can then call this utility, reducing duplication and ensuring consistent behavior.stape/sections/StapeTracker.tsx (2)
118-118: Cookie header is read twice; extract once and reuse.Line 118 reads
req.headers.get("cookie")for consent parsing, and line 132 reads it again for client_id extraction. Extract it once at the top of the consent block and reuse.Apply this diff:
// Automatically track page view if enabled if (props.enableAutoPageTracking !== false && containerUrl) { try { + const cookieHeader = req.headers.get("cookie") || ""; + // Check GDPR consent from cookies if enabled let hasConsent = true; if (enableGdprCompliance) { - const cookieHeader = req.headers.get("cookie") || ""; const consentCookie = cookieHeader .split("; ") .find((row) => row.startsWith(`${consentCookieName}=`)) ?.split("=")[1]; hasConsent = consentCookie === "true" || consentCookie === "granted"; } if (hasConsent) { // Generate stable client ID from existing cookies or create new one let clientId: string; - // Get cookie header for client ID extraction - const cookieHeader = req.headers.get("cookie") || ""; - // Try to extract from existing _ga cookie first const gaCookie = cookieHeaderAlso applies to: 132-132
43-95: Server-only section renders a hidden div with configuration data.The component returns a hidden
<div>with inline styles and data attributes. This is safe for server-side rendering but serves no functional purpose on the client. Consider whether this debugging aid is necessary in production or if it could be gated bydebugMode.If the configuration div is only for debugging, apply this diff:
} // Return a minimal hidden element for configuration tracking - return ( + return debugMode ? ( <div style={{ display: "none", position: "absolute", top: "-9999px", left: "-9999px", width: "1px", height: "1px", overflow: "hidden", }} data-stape-tracker="server-side" data-container-url={containerUrl} data-gtm-id={gtmContainerId} data-gdpr-enabled={enableGdprCompliance?.toString()} data-auto-page-tracking={enableAutoPageTracking?.toString()} data-ecommerce-tracking={enableEcommerceTracking?.toString()} data-page-url={pageUrl} data-user-agent={userAgent} data-client-ip={clientIp} aria-hidden="true" /> - ); + ) : null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
stape/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
stape/README.md(1 hunks)stape/actions/sendBasicEvent.ts(1 hunks)stape/actions/sendEcommerceEvent.ts(1 hunks)stape/actions/sendEvent.ts(1 hunks)stape/actions/testConnection.ts(1 hunks)stape/actions/trackEcommerceEvent.ts(1 hunks)stape/actions/trackPageView.ts(1 hunks)stape/loaders/analyticsScript.ts(1 hunks)stape/loaders/getStapeConfig.ts(1 hunks)stape/mod.ts(1 hunks)stape/sections/Analytics/Stape.tsx(1 hunks)stape/sections/StapeTracker.tsx(1 hunks)stape/utils/client.ts(1 hunks)stape/utils/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- stape/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- stape/utils/types.ts
- stape/utils/client.ts
- stape/actions/trackPageView.ts
- stape/actions/sendBasicEvent.ts
- stape/actions/sendEvent.ts
🧰 Additional context used
🧬 Code graph analysis (8)
stape/sections/StapeTracker.tsx (5)
stape/actions/sendBasicEvent.ts (1)
Props(3-27)stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/sections/Analytics/Stape.tsx (2)
Props(4-31)loader(73-164)website/mod.ts (1)
SectionProps(31-33)
stape/mod.ts (3)
stape/manifest.gen.ts (1)
Manifest(37-37)utils/http.ts (1)
createHttpClient(134-240)stape/utils/client.ts (1)
StapeClient(3-68)
stape/actions/sendEcommerceEvent.ts (2)
stape/utils/types.ts (2)
Item(79-102)EventData(47-50)stape/mod.ts (1)
AppContext(9-9)
stape/loaders/getStapeConfig.ts (2)
stape/mod.ts (1)
AppContext(9-9)stape/utils/types.ts (1)
Container(3-12)
stape/actions/trackEcommerceEvent.ts (2)
stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (1)
Props(11-45)
stape/sections/Analytics/Stape.tsx (5)
stape/actions/sendBasicEvent.ts (1)
Props(3-27)stape/actions/sendEvent.ts (1)
Props(4-51)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/sections/StapeTracker.tsx (1)
Props(4-37)website/mod.ts (1)
SectionProps(31-33)
stape/loaders/analyticsScript.ts (1)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)
stape/actions/testConnection.ts (1)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)
🔇 Additional comments (17)
stape/actions/sendEcommerceEvent.ts (3)
101-110: LGTM: transactionId validation implemented correctly.The purchase event now correctly requires
transactionIdand returns early with a clear error message if missing, addressing the previous review concern.
116-202: GDPR consent implementation is thorough and well-structured.The consent logic handles multiple formats (boolean, string, JSON object), parses cookies correctly, and blocks events when
analytics_storageis denied. The fallback to "unknown" when GDPR is disabled is appropriate.
212-226: Verify IP extraction handles proxy chains securely and validate deployment architecture.Line 215's approach of extracting the first IP from
X-Forwarded-Forusing comma-split handles IPv6 correctly (IPv6 addresses contain colons, not commas). However, taking the leftmost IP by default can be spoofed by malicious clients. The same pattern appears insendEvent.ts(line 92).In contrast,
trackPageView.tsandtrackEcommerceEvent.tsforward the entire header without parsing. Best practice is to walk the list right→left, skip known trusted proxies, and take the first non-trusted address.Verify your deployment ensures:
- Only trusted reverse proxies/load balancers can set
X-Forwarded-Forheaders- Untrusted clients cannot inject or manipulate these headers directly
If clients reach this code without a trusted proxy layer, consider implementing IP validation and a trusted-proxy whitelist.
stape/mod.ts (4)
62-64: API key resolution handles both string and Secret correctly.The logic properly extracts the API key whether it's a direct string or a Secret with a
.get()method, with a safe fallback to empty string.
75-79: State spreads Props, including apiKey Secret.Line 76 spreads
propsinto state, but line 77 overwritesapiKeywith the resolved string. SinceStateextendsOmit<Props, "apiKey">, this is correct. However, ensure that the Secret object is never accidentally exposed in logs or responses.
88-101: Preview configuration looks good.The preview metadata is complete with name, owner, description, logo, and placeholder arrays for images/tabs.
66-73: HTTP client Authorization header exposes empty Bearer token if apiKey is missing.If
stringApiKeyis empty, line 69 producesAuthorization: Bearer(with no token). This will cause all API requests to fail with 401. Consider validating thatapiKeyis provided before creating the client, or omitting the Authorization header entirely when absent.Apply this diff to omit the Authorization header when no API key is provided:
+ const headers = new Headers({ + "Content-Type": "application/json", + }); + + if (stringApiKey) { + headers.set("Authorization", `Bearer ${stringApiKey}`); + } + const api = createHttpClient<StapeClient>({ base: "https://api.app.stape.io", - headers: new Headers({ - "Authorization": `Bearer ${stringApiKey}`, - "Content-Type": "application/json", - }), + headers, fetcher: fetchSafe, });stape/loaders/analyticsScript.ts (7)
40-72: Script URL builder handles GTM and gtag correctly.The function correctly builds URLs with container IDs when available, and provides sensible fallbacks. The encoding of
gtmContainerIdprevents injection attacks.
74-127: Fallback scripts provide useful minimal functionality.Both GTM and gtag fallbacks initialize
dataLayerand push basic events, which is appropriate for graceful degradation.
201-221: Timeout handling is correct.The abort controller and timeout are properly set up and cleared, preventing request-thread blocking.
236-239: Content validation checks for common error patterns.Lines 237-239 detect HTML error pages in the response body. This is a useful sanity check to catch misconfigurations.
267-275: Detailed error logging includes sensitive headers.Line 273 logs
userAgentfrom the request. While this is generally safe, ensure no other sensitive headers (e.g., cookies, authorization) are logged elsewhere. The current logging is appropriate for debugging.
148-159: Error response leaks error message to client JavaScript.Line 149 interpolates
messagedirectly into the client-side JavaScript comment andconsole.errorcall. Ifmessagecontains sensitive information (e.g., internal paths, API details), this could leak to end users. Consider sanitizing or limiting the error message exposed to clients.Apply this diff to sanitize the error message:
return new Response( - `// Stape Error: ${message}\nconsole.error('Stape script loading failed: ${message}');`, + `// Stape Error: Script loading failed\nconsole.error('Stape script loading failed.');`, { status: 200, headers: { "Content-Type": "text/javascript", "Cache-Control": "no-cache", - "X-Stape-Error": message, + "X-Stape-Error": "Script loading failed", }, }, );
60-60: Fallback URLs without container ID will fail and should be handled.Stape's gtag.js and gtm.js endpoints require a container ID parameter to function properly. Lines 60 and 70 return script URLs without the
?id=parameter whengtmContainerIdis missing or doesn't match the expected prefix (G- or GTM-). These requests will likely return 404 errors, causing unnecessary failed HTTP requests before the fallback script activates.Consider adding validation in
buildScriptUrlor early in theanalyticsScriptloader to log a warning or return an error response whengtmContainerIdis required but missing or invalid, rather than attempting to fetch from an endpoint that will fail.stape/actions/trackEcommerceEvent.ts (1)
116-141: Verify thatadditionalParamsshould override standard tracking parameters.Line 129 spreads
additionalParamsafter standard params likeclient_id,user_id,timestamp_micros,page_location, andpage_referrer. This allowsadditionalParamsto override these standard fields, which could lead to data integrity issues if callers accidentally or maliciously override core tracking data.Consider whether this is intentional for flexibility, or if standard params should take precedence. If standard params should be protected, reorder the spread:
params: { // Spread all event parameters ...eventParams, + ...props.additionalParams, // Add additional tracking data client_id: clientId, user_id: props.userId || undefined, timestamp_micros: Date.now() * 1000, page_location: req.url, page_referrer: req.headers.get("referer") || "", - ...props.additionalParams, },stape/actions/testConnection.ts (1)
22-90: LGTM! Past review feedback properly addressed.The implementation correctly validates the Stape connection with an appropriate test payload. The previous concern about exposing
containerUrlin the return type has been resolved—it's now only used internally for the API call and logging. The error handling is robust, including the.catch()fallback onresponse.text()to handle parsing failures gracefully.stape/sections/StapeTracker.tsx (1)
128-161: LGTM: Stable client_id extraction implemented correctly.The logic correctly attempts to extract a stable
client_idfrom the_gacookie, falls back to_stape_client_id, and only generates a new UUID as a last resort. This addresses the attribution and session continuity concerns raised in the past review.
…rce event handling
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
stape/utils/types.ts (2)
3-35: Consider extracting common fields to reduce duplication.The
Container,CapiGateway, andStapeGatewayinterfaces share most fields (id, name, url, status, zone, plan, createdAt, updatedAt). This duplication makes maintenance harder if the common schema evolves.Apply this refactor to introduce a base interface:
+interface BaseGateway { + id: string; + name: string; + url: string; + status: "active" | "inactive" | "pending"; + zone: string; + plan: string; + createdAt: string; + updatedAt: string; +} + -export interface Container { - id: string; - name: string; - url: string; - status: "active" | "inactive" | "pending"; - zone: string; - plan: string; - createdAt: string; - updatedAt: string; -} +export interface Container extends BaseGateway {} -export interface CapiGateway { - id: string; - name: string; - url: string; - status: "active" | "inactive" | "pending"; - zone: string; - plan: string; +export interface CapiGateway extends BaseGateway { pixelIds: string[]; - createdAt: string; - updatedAt: string; } -export interface StapeGateway { - id: string; - name: string; - url: string; - status: "active" | "inactive" | "pending"; - zone: string; - plan: string; - createdAt: string; - updatedAt: string; -} +export interface StapeGateway extends BaseGateway {}
104-113: Consider adding "unknown" to ConsentStatus for uninitialized state.The
ConsentStatustype only includes"granted" | "denied", but consent can be in an uninitialized or unknown state before the user makes a choice. GA4's consent mode supports defaulting to "denied" but distinguishing "unknown" can be useful for analytics.Consider expanding the type:
-export type ConsentStatus = "granted" | "denied"; +export type ConsentStatus = "granted" | "denied" | "unknown";Then update default/fallback logic in
stape/utils/gdpr.tsaccordingly.stape/actions/sendEvent.ts (1)
13-55: Consider extending StapeEventRequest directly to reduce duplication.The
Propsinterface duplicates all fields fromStapeEventRequestwith added JSDoc. This creates maintenance overhead ifStapeEventRequestevolves.Refactor to extend the base type:
-export interface Props { - /** - * @title Event Data - * @description The event data to send to Stape - */ - events: EventData[]; - - /** - * @title Client ID - * @description Unique identifier for the client - */ - client_id?: string; - - /** - * @title User ID - * @description User identifier for cross-device tracking - */ - user_id?: string; - - /** - * @title Timestamp (microseconds) - * @description Event timestamp in microseconds since Unix epoch - */ - timestamp_micros?: number; - - /** - * @title User Properties - * @description Additional user properties - */ - user_properties?: Record<string, unknown>; - - /** - * @title Non-Personalized Ads - * @description Whether to disable personalized ads - */ - non_personalized_ads?: boolean; - - /** - * @title Consent Settings - * @description GDPR consent settings - */ - consent?: GdprConsentData; -} +export interface Props extends StapeEventRequest { + // JSDoc can be added via type annotations if needed by the Deco UI +}Alternatively, if JSDoc is required for each field by Deco's admin UI, document this requirement in a comment.
stape/actions/sendEcommerceEvent.ts (1)
133-164: Derive consent_status from actual consent data instead of hardcoding.Both success and error responses hardcode
consent_status: "granted"(lines 142, 156), but this doesn't accurately reflect the actual consent state. For better observability and compliance auditing, derive the status fromconsentData.Apply this refactor:
+ // Determine consent status + const consentStatus = isAnalyticsAllowed(consentData) ? "granted" : "denied"; + try { const response = await fetchWithTimeout( // ... fetch logic ); return { status: "success", event_name: finalEventName, request_data: stapeRequest, response_data: { status: response.status || 0, statusText: response.statusText || "", body: response.data || "", }, - consent_status: "granted", + consent_status: consentStatus, request_info: { url: req.url, method: req.method, user_agent: req.headers.get("user-agent") || "", ip: req.headers.get("x-forwarded-for") || "", }, }; } catch (error) { return { status: "error", event_name: finalEventName, request_data: stapeRequest, error: error instanceof Error ? error.message : String(error), - consent_status: "granted", + consent_status: consentStatus, request_info: { url: req.url, method: req.method, user_agent: req.headers.get("user-agent") || "", ip: req.headers.get("x-forwarded-for") || "", }, }; }stape/utils/fetch.ts (1)
37-41: Ensure timeout is always cleared.The timeout is cleared in the success path (line 52) and error path (line 84), but consider using a
finallyblock to guarantee cleanup in all code paths.Apply this diff:
const controller = new AbortController(); const timeoutId = setTimeout(() => { controller.abort(); }, timeoutMs); try { + // ... existing fetch logic + } catch (error) { + // ... existing error handling + } finally { + clearTimeout(timeoutId); + } - - // Clear timeout on successful response - clearTimeout(timeoutId);stape/actions/trackEcommerceEvent.ts (1)
145-163: Consider usingfetchStapeAPIfromstape/utils/fetch.ts.The fetch logic here duplicates what
fetchStapeAPIalready provides (building the URL, setting headers, timeout handling). Using the utility would reduce duplication and improve consistency.Apply this diff:
+import { fetchStapeAPI, extractRequestInfo } from "../utils/fetch.ts"; + const trackEcommerceEvent = async ( // ... - // Send to Stape container - const stapeUrl = new URL("/gtm", containerUrl); - - const response = await fetch(stapeUrl.toString(), { - method: "POST", - headers: { - "Content-Type": "application/json", - "User-Agent": req.headers.get("user-agent") || "Deco-Stape-Server/1.0", - "X-Forwarded-For": req.headers.get("x-forwarded-for") || - req.headers.get("x-real-ip") || - "127.0.0.1", - }, - body: JSON.stringify(eventData), - }); - - if (!response.ok) { - throw new Error( - `Stape API error: ${response.status} ${response.statusText}`, - ); - } + const { userAgent, clientIp } = extractRequestInfo(req); + const result = await fetchStapeAPI(containerUrl, eventData, userAgent, clientIp); + + if (!result.success) { + throw new Error(result.error || "Unknown Stape API error"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
stape/actions/sendEcommerceEvent.ts(1 hunks)stape/actions/sendEvent.ts(1 hunks)stape/actions/trackEcommerceEvent.ts(1 hunks)stape/utils/fetch.ts(1 hunks)stape/utils/gdpr.ts(1 hunks)stape/utils/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
stape/actions/trackEcommerceEvent.ts (4)
stape/actions/sendEvent.ts (1)
Props(13-55)stape/actions/sendBasicEvent.ts (1)
Props(3-27)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/utils/types.ts (1)
EcommerceEvents(150-162)
stape/utils/gdpr.ts (1)
stape/utils/types.ts (1)
GdprConsentData(108-113)
stape/actions/sendEvent.ts (4)
stape/mod.ts (1)
AppContext(9-9)stape/utils/types.ts (3)
EventData(47-50)GdprConsentData(108-113)StapeEventRequest(164-172)stape/utils/gdpr.ts (2)
extractConsentFromHeaders(101-114)isAnalyticsAllowed(120-122)stape/utils/fetch.ts (2)
extractRequestInfo(137-161)fetchStapeAPI(110-132)
stape/actions/sendEcommerceEvent.ts (4)
stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/utils/types.ts (5)
EcommerceEvents(150-162)Item(79-102)StapeEventRequest(164-172)EventParams(52-77)EventData(47-50)stape/utils/gdpr.ts (2)
parseConsentCookie(50-95)isAnalyticsAllowed(120-122)stape/utils/fetch.ts (1)
fetchWithTimeout(26-104)
🔇 Additional comments (26)
stape/utils/types.ts (5)
47-77: LGTM! Well-structured event parameter types.The
EventDataandEventParamsinterfaces provide good flexibility with the index signature while maintaining strong typing for common fields. The snake_case naming aligns with GA4 conventions.
79-102: LGTM! Comprehensive GA4-aligned item schema.The
Iteminterface correctly requires core fields (item_id, item_name, price, quantity) and provides extensive optional fields matching GA4 enhanced e-commerce specifications.
115-147: LGTM! Comprehensive GA4 standard events.The
StandardEventstype union provides excellent type safety for GA4-recommended events, covering e-commerce, engagement, gaming, and lifecycle categories.
149-162: LGTM! Appropriate e-commerce event subset.The
EcommerceEventstype correctly isolates e-commerce-specific events from the broaderStandardEvents, enabling better type safety for e-commerce tracking functions.
164-172: Consider adding an index signature to support custom fields.Stape's server-side tagging API accepts and forwards arbitrary top-level fields in event requests, which can then be mapped to GA4 parameters via configuration. The current interface definition prevents TypeScript users from adding such custom fields, even though the underlying API supports them.
If your use case requires custom top-level fields beyond the standard ones, consider:
export interface StapeEventRequest { events: EventData[]; client_id?: string; user_id?: string; timestamp_micros?: number; user_properties?: Record<string, unknown>; non_personalized_ads?: boolean; consent?: GdprConsentData; + [key: string]: unknown; }If you prefer a strict schema that only allows predefined fields, the current definition is appropriate.
stape/actions/sendEvent.ts (4)
66-89: LGTM! GDPR consent gate properly implemented.The consent check correctly gates event dispatch when
enableGdprComplianceis true, using the configured cookie name and returning a clear denial message when analytics is not allowed.
91-104: LGTM! Payload construction with sensible defaults.The payload construction correctly defaults
client_idto a random UUID andtimestamp_microsto current time in microseconds, ensuring required fields are always populated.
126-136: LGTM! Robust error handling.The catch block safely extracts error messages with a type guard and returns a consistent error response structure while logging for debugging.
106-125: Timeout and error handling properly implemented.The code correctly delegates timeout handling to
fetchStapeAPI, which usesfetchWithTimeoutwith a 5-second timeout (DEFAULT_STAPE_TIMEOUT_MS = 5000). This is a reasonable value for external API calls. Error handling includes proper logging and user-friendly error messages.stape/actions/sendEcommerceEvent.ts (3)
1-30: LGTM! Well-structured e-commerce event props.The
Propsinterface appropriately uses theEcommerceEventstype for type safety and provides a configurable timeout. The structure aligns well with GA4 e-commerce event requirements.
32-49: LGTM! Comprehensive response interface for observability.The
EcommerceEventResponseinterface provides excellent observability with three status states (success/error/skipped), full request/response data, and consent tracking. The "skipped" status is particularly useful for GDPR compliance monitoring.
85-105: Enforce transaction_id requirement for purchase events.Although a past review requested that
transaction_idbe mandatory for purchase events (critical for GA4 deduplication), the current code still treats it as optional (line 94). This can cause attribution and deduplication issues.Apply this fix before building eventParams:
+ // Require transaction_id for purchase events + if (event_name === "purchase" && !transaction_id) { + return { + status: "error", + event_name: custom_event_name || event_name, + error: "transaction_id is required for purchase events", + request_info: { + url: req.url, + method: req.method, + user_agent: req.headers.get("user-agent") || "", + ip: req.headers.get("x-forwarded-for") || "", + }, + }; + } + const eventParams: EventParams = { ...custom_parameters };stape/utils/fetch.ts (6)
1-14: LGTM!The interface definitions are well-structured and provide clear contracts for fetch operations with timeout support and comprehensive error handling.
16-20: LGTM!The timeout constants are reasonable (10s default, 5s for Stape API) and well-documented.
55-71: LGTM!Excellent error handling that captures response bodies for debugging non-OK responses, with proper fallback when response body is unreadable.
86-92: LGTM!Proper detection and handling of AbortError for timeout scenarios.
110-132: LGTM!The Stape API helper correctly builds the URL path, sets appropriate headers for server-side tracking (User-Agent, X-Forwarded-For, X-Real-IP), and delegates to
fetchWithTimeout.
137-161: LGTM!The request info extraction logic correctly parses forwarded IPs, handles both X-Forwarded-For and X-Real-IP headers, and provides sensible defaults.
stape/actions/trackEcommerceEvent.ts (3)
4-34: LGTM!The Props interface is well-documented and correctly uses the
EcommerceEventstype for type safety on eventName.
40-52: LGTM!Proper validation that containerUrl is configured before proceeding.
54-66: LGTM!Event name validation and UUID generation for eventId and clientId are correct.
stape/utils/gdpr.ts (5)
1-4: LGTM!Clear documentation references and attribution to Google Consent Mode documentation.
11-44: LGTM!The factory functions for creating consent objects are clear, well-documented, and follow the GA4 consent model correctly.
50-95: LGTM!Excellent consent parsing logic that handles multiple formats (JSON, boolean strings, object-based consent) with appropriate fallbacks. The URL decoding and trimming ensure robust parsing.
101-114: LGTM!The header extraction correctly parses the cookie header, handles URL encoding, and delegates to
parseConsentCookiefor format handling.
120-139: LGTM!The consent checking functions provide clear, focused APIs for determining allowed tracking types based on consent state.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
stape/actions/sendEcommerceEvent.ts (1)
92-94: Consider using stable client_id extraction pattern.While the current approach generates a client_id from timestamp and random string, using the same stable cookie extraction pattern as
stape/sections/StapeTracker.tsx(checking_gaand_stape_client_idcookies) would provide better cross-session attribution and consistency across the codebase.This is less critical for action-based events than loader-based tracking, but would improve attribution accuracy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
stape/actions/sendEcommerceEvent.ts(1 hunks)stape/actions/trackEcommerceEvent.ts(1 hunks)stape/loaders/getStapeConfig.ts(1 hunks)stape/sections/Analytics/Stape.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- stape/loaders/getStapeConfig.ts
🧰 Additional context used
🧬 Code graph analysis (3)
stape/sections/Analytics/Stape.tsx (4)
stape/actions/sendEvent.ts (1)
Props(13-55)stape/mod.ts (2)
Props(11-45)AppContext(9-9)website/mod.ts (1)
SectionProps(31-33)stape/utils/gdpr.ts (2)
extractConsentFromHeaders(101-114)isAnyTrackingAllowed(136-139)
stape/actions/sendEcommerceEvent.ts (4)
stape/mod.ts (1)
Props(11-45)stape/utils/types.ts (5)
EcommerceEvents(150-162)Item(79-102)StapeEventRequest(164-172)EventParams(52-77)EventData(47-50)stape/utils/gdpr.ts (2)
extractConsentFromHeaders(101-114)isAnalyticsAllowed(120-122)stape/utils/fetch.ts (1)
fetchWithTimeout(26-104)
stape/actions/trackEcommerceEvent.ts (4)
stape/actions/sendEvent.ts (1)
Props(13-55)stape/mod.ts (2)
Props(11-45)AppContext(9-9)stape/utils/types.ts (1)
EcommerceEvents(150-162)stape/utils/gdpr.ts (2)
extractConsentFromHeaders(101-114)isAnyTrackingAllowed(136-139)
🔇 Additional comments (17)
stape/actions/trackEcommerceEvent.ts (7)
1-6: LGTM!The imports are clean and correctly bring in the GDPR utilities as recommended in previous reviews.
8-38: LGTM!The Props interface is well-structured with clear JSDoc annotations and appropriate type constraints.
44-56: LGTM!The function signature and early configuration validation follow best practices for defensive programming.
72-89: LGTM! GDPR utilities properly integrated.The code now correctly uses
extractConsentFromHeadersandisAnyTrackingAllowedutilities as recommended in previous reviews, eliminating the duplicate consent parsing logic.
110-115: Verify consent defaults when GDPR compliance is disabled.When
enableGdprComplianceisfalse,consentDataremainsnulland the fallback sets all consent fields to"denied". This may not reflect the intended behavior—when GDPR compliance is disabled, tracking should likely default to"granted"(no consent mechanism active).Consider this logic:
- let hasConsent = true; - let consentData = null; + let hasConsent = true; + let consentData = enableGdprCompliance ? null : { + ad_storage: "granted", + analytics_storage: "granted", + ad_user_data: "granted", + ad_personalization: "granted", + }; if (enableGdprCompliance) { const cookieHeader = req.headers.get("cookie") || ""; consentData = extractConsentFromHeaders( cookieHeader, ctx.consentCookieName || "cookie_consent", ); hasConsent = isAnyTrackingAllowed(consentData); }Or simplify the consent block:
consent: consentData || { - ad_storage: "denied", - analytics_storage: "denied", - ad_user_data: "denied", - ad_personalization: "denied", + ad_storage: enableGdprCompliance ? "denied" : "granted", + analytics_storage: enableGdprCompliance ? "denied" : "granted", + ad_user_data: enableGdprCompliance ? "denied" : "granted", + ad_personalization: enableGdprCompliance ? "denied" : "granted", },
91-144: LGTM! Event data construction and API call are well-implemented.The event payload follows GA4 conventions, headers properly forward client identity (User-Agent, X-Forwarded-For), and error handling is robust. The timestamp calculation correctly converts milliseconds to microseconds.
145-155: LGTM!Error handling is comprehensive and the default export is correctly declared.
stape/sections/Analytics/Stape.tsx (5)
1-35: LGTM: Clean interface definition.The imports and Props interface are well-structured with appropriate JSDoc annotations for the server-side tracking configuration.
41-75: LGTM: Configuration component is correctly implemented.The component appropriately renders server-side configuration without client-side JavaScript overhead, aligning with the PR's goals.
77-100: LGTM: GDPR consent handling is correct.The loader properly gates page view tracking behind consent checks using the appropriate utility functions from the GDPR module.
102-159: Event dispatch implementation is correct (aside from client_id issue).The page_view event construction, fetch headers (User-Agent, X-Forwarded-For), error handling, and debug logging are all properly implemented. The only critical issue is the client_id generation flagged separately.
121-121: Critical: Generate stable client_id instead of fresh UUID per request.Every page view generates a new
client_id, causing GA4/Stape to treat each hit as a distinct visitor. This breaks attribution, funnels, and session tracking.The companion file
stape/sections/StapeTracker.tsxalready implements stable client_id extraction from_gaor_stape_client_idcookies. Apply the same pattern here.Apply this diff to derive a stable client_id from cookies:
if (hasConsent) { + // Generate stable client ID from existing cookies or create new one + let clientId: string; + + // Get cookie header for client ID extraction + const cookieHeader = req.headers.get("cookie") || ""; + + // Try to extract from existing _ga cookie first + const gaCookie = cookieHeader + .split("; ") + .find((row: string) => row.startsWith("_ga=")) + ?.split("=")[1]; + + if (gaCookie) { + // Extract client ID from GA cookie format: GA1.1.clientId.timestamp + const gaParts = gaCookie.split("."); + if (gaParts.length >= 3) { + clientId = `${gaParts[2]}.${gaParts[3] || Date.now()}`; + } else { + clientId = crypto.randomUUID(); + } + } else { + // Try to find existing Stape client ID cookie + const stapeClientCookie = cookieHeader + .split("; ") + .find((row: string) => row.startsWith("_stape_client_id=")) + ?.split("=")[1]; + + if (stapeClientCookie) { + clientId = stapeClientCookie; + } else { + // Generate new stable ID and it should be set as cookie client-side + clientId = crypto.randomUUID(); + } + } + // Use sendBasicEvent action via context to track page view server-side const pageViewData = { eventName: "page_view" as const, eventParams: { page_location: req.url, page_title: "Page View", timestamp_micros: Date.now() * 1000, ...props.customParameters, }, }; try { // Send basic event directly using Stape API const eventData = { events: [{ name: pageViewData.eventName, params: pageViewData.eventParams, }], - client_id: crypto.randomUUID(), + client_id: clientId, timestamp_micros: Date.now() * 1000, };stape/actions/sendEcommerceEvent.ts (5)
1-52: LGTM: Well-defined interfaces.Both Props and EcommerceEventResponse interfaces are comprehensive and appropriately typed for handling ecommerce events with proper response metadata.
72-89: LGTM: GDPR consent handling is correct.The code properly uses
extractConsentFromHeadersand gates event dispatch behindisAnalyticsAllowed, resolving the previous concern about consent parsing.
113-141: LGTM: Request construction and headers are correct.The Stape request is properly constructed with all required fields, and the fetch includes appropriate headers for server-side attribution (User-Agent, X-Forwarded-For, X-Real-IP), resolving the previous concern about missing IP forwarding.
143-174: LGTM: Comprehensive response handling.Both success and error paths return well-structured responses with appropriate metadata, consent status, and request information for debugging and monitoring.
91-111: Add validation: require transaction_id for purchase events.GA4 deduplication and attribution rely on
transaction_idfor purchase events. The code should validate thattransaction_idis provided whenevent_name === "purchase"and return an error if missing.Apply this diff:
const finalEventName = custom_event_name || event_name; + + // Validate required fields for purchase events + if (event_name === "purchase" && !transaction_id) { + return { + status: "error", + event_name: finalEventName, + error: "transaction_id is required for purchase events", + consent_status: consentData.analytics_storage, + request_info: { + url: req.url, + method: req.method, + user_agent: req.headers.get("user-agent") || "", + ip: req.headers.get("x-forwarded-for") || "", + }, + }; + } + const clientIdCookie = cookieHeader.match(/client_id=([^;]+)/)?.[1];
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
stape/utils/security.ts (1)
17-24: Consider strengthening API key validation.The minimum length of 10 characters is weak for modern API keys, which typically use 32+ characters. Consider raising the threshold to improve security.
Apply this diff:
export const isValidApiKey = (apiKey: string): boolean => { // API key should be non-empty and not contain obvious placeholders return typeof apiKey === "string" && - apiKey.length > 10 && + apiKey.length >= 32 && !apiKey.includes("your-api-key") && !apiKey.includes("placeholder") && !apiKey.includes("example"); };stape/actions/sendBasicEvent.ts (1)
68-74: Strengthen event name validation.Line 70 checks if
safeEventName !== props.eventName, which will reject valid event names that happen to differ after sanitization. However, an emptysafeEventNameindicates invalid input. Consider clarifying the validation logic.- if (!safeEventName || safeEventName !== props.eventName) { + if (!safeEventName) { return createErrorResult(new Error("Invalid event name format")); }This change accepts event names that are sanitized but still valid, while rejecting those that become empty after sanitization. If you want to enforce that callers provide pre-sanitized names, keep the current logic but update the error message to clarify this requirement.
stape/actions/testConnection.ts (1)
94-94: Avoid logging sensitive containerUrl to console.Line 94 logs the full
containerUrl, which may be considered internal configuration. If this action is exposed to untrusted callers, consider redacting or hashing the URL in logs.- console.log(`Testing Stape connection to: ${containerUrl}/gtm`); + console.log(`Testing Stape connection...`);Or log only the hostname:
+ const urlHost = new URL(containerUrl).hostname; + console.log(`Testing Stape connection to: ${urlHost}`);stape/utils/events.ts (2)
42-60: Consider structured logging for production.Lines 42-60 use
console.log/console.errorfor debug output. While adequate for initial development, consider a structured logging library (e.g.,std/log) for production to support log levels, formatting, and observability integrations.
73-100: Clarify GA cookie extraction logic and fallback.Line 78 constructs a clientId from
gaParts[2]andgaParts[3], but ifgaParts[3]is undefined, it falls back toDate.now(). This fallback introduces a timestamp-based ID that may not align with GA's actual client ID format.Consider one of the following:
- Reject malformed GA cookies: Return
nullifgaParts.length < 4to avoid mixing formats.- Document the fallback: If the timestamp fallback is intentional, add a comment explaining why.
Example for strict validation:
export const extractClientIdFromGaCookie = ( gaCookie: string, ): string | null => { const gaParts = gaCookie.split("."); - return gaParts.length >= 3 - ? `${gaParts[2]}.${gaParts[3] || Date.now()}` - : null; + // GA cookie format: GA1.2.clientId1.clientId2 + return gaParts.length >= 4 + ? `${gaParts[2]}.${gaParts[3]}` + : null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
stape/actions/sendBasicEvent.ts(1 hunks)stape/actions/testConnection.ts(1 hunks)stape/actions/trackPageView.ts(1 hunks)stape/loaders/getStapeConfig.ts(1 hunks)stape/mod.ts(1 hunks)stape/sections/Analytics/Stape.tsx(1 hunks)stape/sections/StapeTracker.tsx(1 hunks)stape/utils/events.ts(1 hunks)stape/utils/security.ts(1 hunks)stape/utils/tracking.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- stape/mod.ts
- stape/sections/Analytics/Stape.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
stape/utils/security.ts (1)
vtex/utils/openapi/orders.openapi.gen.ts (1)
Headers(6147-6149)
stape/sections/StapeTracker.tsx (5)
stape/mod.ts (2)
Props(12-46)AppContext(10-10)website/mod.ts (1)
SectionProps(31-33)stape/utils/fetch.ts (1)
extractRequestInfo(137-161)stape/utils/tracking.ts (3)
buildTrackingContext(9-36)createPageViewEvent(39-83)sendEventSafely(105-131)stape/utils/events.ts (3)
logDebugMessage(42-50)logDebugError(52-60)TrackingContext(9-15)
stape/actions/sendBasicEvent.ts (5)
stape/mod.ts (2)
Props(12-46)AppContext(10-10)stape/utils/events.ts (7)
EventResult(4-7)createErrorResult(33-36)sanitizeEventName(119-122)sanitizeEventParams(124-147)createSuccessResult(23-26)isTimeoutError(38-39)createTimeoutResult(28-31)stape/utils/security.ts (2)
validateStapeConfig(129-149)isRateLimited(154-175)stape/utils/tracking.ts (2)
buildTrackingContext(9-36)createBasicEventPayload(86-102)stape/utils/fetch.ts (1)
fetchStapeAPI(110-132)
stape/actions/trackPageView.ts (2)
stape/mod.ts (2)
Props(12-46)AppContext(10-10)stape/actions/sendEvent.ts (1)
Props(13-55)
stape/utils/tracking.ts (2)
stape/utils/events.ts (3)
TrackingContext(9-15)hasTrackingConsent(103-116)generateClientId(82-100)stape/utils/fetch.ts (2)
extractRequestInfo(137-161)fetchStapeAPI(110-132)
stape/utils/events.ts (1)
stape/utils/gdpr.ts (2)
extractConsentFromHeaders(101-114)isAnyTrackingAllowed(136-139)
stape/loaders/getStapeConfig.ts (2)
stape/mod.ts (1)
AppContext(10-10)stape/utils/types.ts (1)
Container(3-12)
stape/actions/testConnection.ts (2)
stape/mod.ts (1)
AppContext(10-10)stape/utils/fetch.ts (2)
extractRequestInfo(137-161)fetchStapeAPI(110-132)
🔇 Additional comments (39)
stape/loaders/getStapeConfig.ts (4)
13-30: LGTM! Well-structured utility functions.The type guard and error handling utilities are clean and cover the key cases: credential validation, logging, and API error handling with appropriate 404 special-casing.
32-50: LGTM! Robust error handling.The function properly validates HTTP status before parsing JSON, handles errors gracefully, and provides clear logging. Previous concerns about status checking have been addressed.
52-68: LGTM! Consistent error handling.The function follows the same robust error handling pattern as
fetchSpecificContainer, properly validates HTTP status, and correctly handles the empty containers case.
74-91: LGTM! Clean and well-structured.The main function has a clear flow: validates credentials early, logs appropriate warnings, and routes to the correct fetch function. The implementation addresses all previous review concerns.
stape/utils/security.ts (5)
4-15: LGTM!The container URL validation correctly enforces HTTPS and blocks localhost/loopback addresses.
26-32: LGTM!Event name validation correctly implements GA4 naming conventions.
58-88: LGTM!URL sanitization correctly removes sensitive query parameters and fails safely by returning an empty string on parse errors.
91-101: LGTM!User input sanitization removes dangerous HTML characters and enforces length limits.
129-149: LGTM!Configuration validation correctly checks required fields and enforces security constraints.
stape/actions/trackPageView.ts (4)
1-42: LGTM!Props interface is well-defined with clear documentation for all fields.
48-65: LGTM!Configuration extraction correctly applies documented defaults for GDPR compliance settings. Early return on missing config is appropriate.
67-97: LGTM!GDPR consent handling correctly uses the configured cookie name and blocks events when consent is denied.
99-189: LGTM!Event payload construction, timeout handling, and error handling are well-implemented. The consent object correctly reflects the actual consent state.
stape/sections/StapeTracker.tsx (4)
1-51: LGTM!Imports and Props interface are well-organized with clear documentation.
57-109: LGTM!Server-side component correctly renders a minimal hidden element for configuration tracking.
195-210: LGTM!Helper function correctly merges props and context for the component.
111-156: Client ID extraction verified—stable identifiers are used.The
generateClientIdfunction instape/utils/events.tscorrectly extracts stable client IDs from the_gacookie when consent is present. The implementation:
- Searches for the
_gacookie in the request headers- Extracts the client ID portion using
extractClientIdFromGaCookie- Only falls back to
crypto.randomUUID()when consent is absent (appropriate for privacy compliance)This ensures accurate attribution and funnel tracking when users have granted consent.
stape/actions/sendBasicEvent.ts (6)
1-19: LGTM: Clean imports and configuration.The module correctly imports all necessary utilities for security, GDPR compliance, and event handling. The 5-second timeout is appropriate for server-side tracking.
21-45: LGTM: Well-documented Props interface.The interface is properly typed with clear JSDoc annotations for each field.
58-66: LGTM: Security validation protects against misconfigurations.The configuration validation correctly gates the action and returns a structured error on failure.
76-96: LGTM: Comprehensive GDPR and rate-limiting checks.The code properly:
- Builds a tracking context with consent checking
- Returns early if consent is not granted
- Applies rate limiting per client
This addresses the past review concern about GDPR consent gating.
98-117: LGTM: Safe event payload creation and transmission.The payload uses sanitized inputs and the fetch includes proper timeout handling with structured result types.
118-129: LGTM: Robust error handling with privacy protection.Error logging avoids exposing sensitive data, and the timeout vs. general error distinction is appropriate.
stape/utils/tracking.ts (6)
1-7: LGTM: Appropriate imports and sensible default.The DEFAULT_USER_AGENT provides a clear fallback identifier for server-side requests.
9-36: Consider the privacy implications of generating clientId without consent.Lines 23-27 generate a clientId even when
hasConsentis false. While using a random UUID (not persisted) is safer than reading cookies, the downstream code may still send this ephemeral ID to Stape. If the intent is to avoid any tracking without consent, consider whether generating and potentially sending even a random ID aligns with your privacy policy.If your interpretation is that a random, non-persistent ID is acceptable for logging/debugging even without consent, document this clearly. Otherwise, consider returning a fixed placeholder (e.g.,
"no-consent") or handling consent failures earlier in the call chain so no event is constructed.
48-58: Good: Sensitive query parameters are stripped.This protects against accidental leakage of tokens and secrets in page URLs.
86-102: LGTM: Straightforward basic event payload.The function correctly defaults
clientIdto a new UUID if not provided and includes a timestamp.
105-131: LGTM: Safe event sending with error suppression.The function logs errors without exposing sensitive data and rethrows for upstream handling.
134-159: LGTM: Referrer sanitization prevents sensitive data leakage.The function strips sensitive query parameters and safely handles malformed URLs.
stape/actions/testConnection.ts (5)
1-20: LGTM: Clean configuration and well-documented Props.The Props interface provides optional test customization with sensible defaults.
23-36: LGTM: Appropriate return type excludes containerUrl.The TestResult interface correctly omits
containerUrl, addressing the prior security concern.
39-51: LGTM: Test event payload is safe and descriptive.The
createTestEventhelper builds a clear test payload with appropriate labels.
53-77: LGTM: Result factory functions are consistent and clear.These helpers provide uniform result objects and proper timeout detection.
83-117: Verify containerUrl is non-empty before use.
containerUrlis retrieved fromctxat line 88. Although typed asstringin Props,sanitizeUrl(called during initialization atmod.ts:80) returns an empty string if URL parsing fails. An empty or malformedcontainerUrlwill causefetchStapeAPIto fail at line 97.Consider adding a guard:
export default async function testStapeConnection( props: Props, req: Request, ctx: AppContext, ): Promise<TestResult> { const { containerUrl } = ctx; + + if (!containerUrl) { + return createErrorResult(new Error("Container URL is not configured")); + } try {stape/utils/events.ts (5)
1-20: LGTM: Clean type definitions for event handling.The interfaces provide clear contracts for event results, tracking context, and client ID sources.
23-39: LGTM: Consistent result factory functions.These helpers ensure uniform error and success responses across actions.
63-70: LGTM: Simple cookie extraction.The function safely splits the cookie header and extracts the requested cookie value.
103-116: LGTM: GDPR consent check delegates to utility.The function correctly short-circuits when compliance is disabled and delegates cookie parsing to
extractConsentFromHeaders.
119-147: LGTM: Effective sanitization against injection.Both
sanitizeEventNameandsanitizeEventParamsstrip unsafe characters and limit lengths, protecting against injection and exfiltration attacks.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
stape/utils/security.ts (5)
4-15: Consider validating against private IP ranges.The function blocks
localhostand127.0.0.1, but doesn't prevent other private IP ranges (e.g.,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16) or IP addresses that bypass the localhost string check. An attacker could potentially usehttps://10.0.0.1to target internal services.Apply this diff to add comprehensive private IP validation:
export const isValidContainerUrl = (url: string): boolean => { try { const parsedUrl = new URL(url); + // Check for private IP ranges + const hostname = parsedUrl.hostname.toLowerCase(); + const privateIpPatterns = [ + /^localhost$/i, + /^127\./, + /^10\./, + /^172\.(1[6-9]|2\d|3[01])\./, + /^192\.168\./, + /^\[::1\]$/, // IPv6 localhost + /^\[fe80:/i, // IPv6 link-local + ]; + + if (privateIpPatterns.some(pattern => pattern.test(hostname))) { + return false; + } + // Only allow HTTPS for security return parsedUrl.protocol === "https:" && - parsedUrl.hostname.length > 0 && - !parsedUrl.hostname.includes("localhost") && - !parsedUrl.hostname.includes("127.0.0.1"); + parsedUrl.hostname.length > 0; } catch { return false; } };
17-24: Strengthen API key validation.The current validation has two weaknesses:
- Placeholder strings are case-sensitive (e.g.,
"YOUR-API-KEY"or"PLACEHOLDER"would pass).- A minimum length of 10 characters may be insufficient for secure API keys (most are 32+ characters).
Apply this diff to improve validation:
export const isValidApiKey = (apiKey: string): boolean => { // API key should be non-empty and not contain obvious placeholders + const lowerKey = apiKey.toLowerCase(); return typeof apiKey === "string" && - apiKey.length > 10 && - !apiKey.includes("your-api-key") && - !apiKey.includes("placeholder") && - !apiKey.includes("example"); + apiKey.length >= 32 && + !lowerKey.includes("your-api-key") && + !lowerKey.includes("placeholder") && + !lowerKey.includes("example") && + !lowerKey.includes("test") && + !lowerKey.includes("dummy"); };
35-55: Headers type ambiguity has been resolved.The previous concern about
Headerstype conflict has been properly addressed by usingglobalThis.Headers. The allowlist approach and length limitation are good security practices.Optionally, consider adding
"x-forwarded-host"to the allowed headers list if you need to capture the original host for analytics purposes.
57-88: Consider case-insensitive parameter matching and logging parse failures.Two improvements to consider:
- The sensitive parameter check is case-sensitive, so variations like
"apiKey","API_KEY", or"AccessToken"would not be filtered.- Returning an empty string on URL parse failure could mask legitimate URLs that have minor formatting issues, making debugging difficult.
Apply this diff:
export const sanitizeUrl = (url: string): string => { try { const parsedUrl = new URL(url); // Remove sensitive query parameters const sensitiveParams = [ "token", "key", "password", "secret", "auth", "session", "api_key", "access_token", "refresh_token", "jwt", "client_secret", "private_key", "credential", ]; + // Remove parameters case-insensitively + const allParams = Array.from(parsedUrl.searchParams.keys()); - sensitiveParams.forEach((param) => { - parsedUrl.searchParams.delete(param); + allParams.forEach((param) => { + const lowerParam = param.toLowerCase(); + if (sensitiveParams.some(sensitive => lowerParam.includes(sensitive))) { + parsedUrl.searchParams.delete(param); + } }); return parsedUrl.toString(); - } catch { + } catch (error) { + console.warn("Failed to parse URL for sanitization:", url, error); // If URL parsing fails, return empty string for security return ""; } };
91-101: Enhance input sanitization for additional attack vectors.The current implementation handles basic XSS characters but misses other potentially dangerous patterns:
- Null bytes (
\0) can truncate strings in some contexts- Control characters can break parsing or logging
- Unicode homoglyphs and normalization attacks
Apply this diff:
export const sanitizeUserInput = ( input: string, maxLength: number = 100, ): string => { if (typeof input !== "string") return ""; return input .replace(/[<>\"'&]/g, "") // Remove potentially dangerous characters + .replace(/[\x00-\x1F\x7F]/g, "") // Remove control characters + .normalize("NFC") // Normalize Unicode to prevent homoglyph attacks .substring(0, maxLength) .trim(); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
stape/utils/security.ts(1 hunks)stape/utils/tracking.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- stape/utils/tracking.ts
🧰 Additional context used
🧬 Code graph analysis (1)
stape/utils/security.ts (1)
vtex/utils/openapi/orders.openapi.gen.ts (1)
Headers(6147-6149)
🔇 Additional comments (4)
stape/utils/security.ts (4)
26-32: LGTM!The event name validation correctly implements GA4 naming conventions with appropriate length and character constraints.
129-149: LGTM!The configuration validation provides clear error reporting and appropriately reuses the individual validation functions. The structured return value makes it easy for callers to handle validation failures.
104-126: IPv6 anonymization still fails for compressed addresses.This concern from the previous review remains unresolved. The current implementation cannot handle IPv6 addresses with
::compression (e.g.,2001:db8::1,::1,fe80::) because splitting on:produces fewer than 8 groups. This causes the function to fall back to127.0.0.1, losing legitimate data.Apply this diff to properly handle IPv6 compression:
export const anonymizeIp = (ip: string): string => { try { // For IPv4, zero out the last octet if (ip.includes(".")) { const parts = ip.split("."); if (parts.length === 4) { return `${parts[0]}.${parts[1]}.${parts[2]}.0`; } } // For IPv6, zero out the last 4 groups if (ip.includes(":")) { - const parts = ip.split(":"); - if (parts.length >= 4) { - return parts.slice(0, 4).join(":") + "::"; + // Expand compressed IPv6 addresses + let fullIp = ip; + if (ip.includes("::")) { + const [left, right] = ip.split("::"); + const leftParts = left ? left.split(":") : []; + const rightParts = right ? right.split(":") : []; + const missingGroups = 8 - leftParts.length - rightParts.length; + const zeros = Array(missingGroups).fill("0"); + fullIp = [...leftParts, ...zeros, ...rightParts].join(":"); } + + const parts = fullIp.split(":"); + if (parts.length === 8) { + // Zero out last 4 groups + return parts.slice(0, 4).concat(["0", "0", "0", "0"]).join(":"); + } } return "127.0.0.1"; // Fallback to localhost } catch { return "127.0.0.1"; } };
152-185: Rate limiting remains bypassable and lacks production readiness.The concerns from the previous review remain unaddressed:
- Bypassable limits:
clientIdis user-controlled, allowing attackers to generate fresh IDs to circumvent rate limits.- Memory leaks:
cleanupRateLimitDatarequires manual invocation; without periodic cleanup, the Map grows unbounded.- Not distributed: In-memory state is lost on restart/redeploy and doesn't work across multiple instances.
For production use, consider these improvements:
Short-term (if Redis is not available):
- Switch to IP-based rate limiting (extract from
x-forwarded-fororx-real-ipheaders)- Set up a periodic cleanup timer using
setInterval- Document that this is single-instance only
Long-term (recommended):
- Migrate to Redis or similar distributed store with built-in TTL
- Use a battle-tested rate-limiting library (e.g.,
rate-limiter-flexible)Apply this minimal diff to add automatic cleanup:
// Rate limiting utilities (prevent abuse) const requestCounts = new Map<string, { count: number; resetTime: number }>(); + +// Set up automatic cleanup every minute +if (typeof setInterval !== "undefined") { + setInterval(() => { + cleanupRateLimitData(); + }, 60000); +} export const isRateLimited = (However, this does not address the bypass or distributed deployment issues.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
stape/utils/tracking.ts (1)
90-107: Consider parameter ordering and consent handling.Two observations:
Event parameters can override timestamp_micros: The
eventParamsspread at line 101 allows callers to override thetimestamp_microsset at line 100. Consider moving the spread before the timestamp, or validate that critical fields are not present in eventParams.Missing consent object: Unlike
createPageViewEvent, this payload doesn't include a consent object. If this helper is used for GA4/Stape events that require consent indication, consider adding an optional consent parameter.export const createBasicEventPayload = ( eventName: string, eventParams: Record<string, unknown> = {}, clientId?: string, userId?: string, + consent?: { ad_storage: string; analytics_storage: string; ad_user_data: string; ad_personalization: string }, ) => ({ events: [{ name: eventName, params: { - timestamp_micros: Date.now() * 1000, ...eventParams, + timestamp_micros: Date.now() * 1000, }, }], client_id: clientId || crypto.randomUUID(), user_id: userId, timestamp_micros: Date.now() * 1000, + ...(consent && { consent }), });stape/sections/StapeTracker.tsx (1)
140-147: Simplify redundant consent parameter.Line 146 passes
trackingContext.hasConsentas thehasConsentparameter tosendPageViewEvent, but thecontextparameter already contains this value (line 141). Consider removing the redundant parameter from thesendPageViewEventsignature.await sendPageViewEvent( trackingContext, props, containerUrl, gtmContainerId, req, - trackingContext.hasConsent, );And update the
sendPageViewEventsignature:const sendPageViewEvent = async ( context: TrackingContext, props: Props, containerUrl: string, gtmContainerId: string | undefined, req: Request, - hasConsent: boolean, ) => { const eventData = createPageViewEvent( context, props.customParameters, gtmContainerId, props.userId, - hasConsent, + context.hasConsent, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
stape/sections/StapeTracker.tsx(1 hunks)stape/utils/security.ts(1 hunks)stape/utils/tracking.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- stape/utils/security.ts
🧰 Additional context used
🧬 Code graph analysis (2)
stape/sections/StapeTracker.tsx (5)
stape/mod.ts (2)
Props(12-46)AppContext(10-10)website/mod.ts (1)
SectionProps(31-33)stape/utils/fetch.ts (1)
extractRequestInfo(137-161)stape/utils/tracking.ts (3)
buildTrackingContext(9-36)createPageViewEvent(39-88)sendEventSafely(110-136)stape/utils/events.ts (3)
logDebugMessage(42-50)logDebugError(52-60)TrackingContext(9-15)
stape/utils/tracking.ts (2)
stape/utils/events.ts (3)
TrackingContext(9-15)hasTrackingConsent(103-116)generateClientId(82-100)stape/utils/fetch.ts (2)
extractRequestInfo(137-161)fetchStapeAPI(110-132)
🔇 Additional comments (4)
stape/utils/tracking.ts (2)
8-36: LGTM: Privacy-first tracking context builder.The consent-gated client ID generation correctly prevents persistent tracking without user consent. The fallback to a random UUID for non-consented requests is appropriate for privacy compliance.
109-136: LGTM: Robust error handling with sanitized logging.The error handling correctly logs failures without exposing sensitive data, and re-throws to allow upstream handling.
stape/sections/StapeTracker.tsx (2)
198-213: LGTM: Clean loader result builder.The helper correctly merges configuration from props and context for downstream consumption.
26-31: Ignore unused prop suggestion –enableEcommerceTrackingis passed through the loader (line 78) and rendered as thedata-ecommerce-trackingattribute (line 102), so it’s already in use. No changes required.Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
stape/sections/StapeTracker.tsx (2)
72-83: Remove client IP from debug logs.Line 81 logs
clientIpto the console, exposing personal data (IP addresses are considered personal data under GDPR) to any client-side script or browser extension. Remove this field from debug output or pseudonymise it server-side before logging.Apply this diff:
if (debugMode) { console.log("Stape Server Tracker Configuration:", { containerUrl, gtmContainerId, enableGdprCompliance, enableAutoPageTracking, enableEcommerceTracking, pageUrl, userAgent, - clientIp, }); }
86-108: Remove client IP from DOM attributes.Line 105 embeds
clientIpin adata-client-ipattribute, making personal data accessible to any client-side JavaScript, browser extensions, or dev tools. This violates GDPR data minimisation principles and creates unnecessary exposure. Remove this attribute entirely.Apply this diff:
return ( <div style={{ display: "none", position: "absolute", top: "-9999px", left: "-9999px", width: "1px", height: "1px", overflow: "hidden", }} data-stape-tracker="server-side" data-container-url={containerUrl} data-gtm-id={gtmContainerId} data-gdpr-enabled={enableGdprCompliance?.toString()} data-auto-page-tracking={enableAutoPageTracking?.toString()} data-ecommerce-tracking={enableEcommerceTracking?.toString()} data-page-url={pageUrl} data-user-agent={userAgent} - data-client-ip={clientIp} aria-hidden="true" /> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
stape/sections/StapeTracker.tsx(1 hunks)stape/utils/tracking.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
stape/sections/StapeTracker.tsx (4)
stape/mod.ts (2)
Props(12-46)AppContext(10-10)stape/utils/fetch.ts (1)
extractRequestInfo(137-161)stape/utils/tracking.ts (5)
buildTrackingContext(9-36)createPageViewEvent(39-88)createPageViewEvent(150-185)extractSafeReferrer(188-203)sendEventSafely(110-136)stape/utils/events.ts (3)
logDebugMessage(42-50)logDebugError(52-60)TrackingContext(9-15)
stape/utils/tracking.ts (2)
stape/utils/events.ts (3)
TrackingContext(9-15)hasTrackingConsent(103-116)generateClientId(82-100)stape/utils/fetch.ts (2)
extractRequestInfo(137-161)fetchStapeAPI(110-132)
🪛 Biome (2.1.2)
stape/sections/StapeTracker.tsx
[error] 175-176: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
stape/utils/tracking.ts
[error] 150-150: Shouldn't redeclare 'createPageViewEvent'. Consider to delete it or rename it.
'createPageViewEvent' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Actions: ci
stape/sections/StapeTracker.tsx
[error] 176-176: Deno fmt error: 'import' and 'export' cannot be used outside of module code. This caused a formatting/linting issue in StapeTracker.tsx.
🔇 Additional comments (2)
stape/utils/tracking.ts (1)
187-203: LGTM: Safe referrer extraction.The
extractSafeReferrerfunction correctly sanitizes referrer URLs by removing sensitive query parameters and handles parsing failures securely.stape/sections/StapeTracker.tsx (1)
220-235: LGTM: Loader result construction.The
createLoaderResulthelper cleanly merges props with runtime context and request-derived metadata.
…nhanced privacy compliance
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…d IP pseudonymization
1 participant
Stape Integration for Deco.cx
📖 Summary
This PR introduces a native Stape integration for Deco.cx, enabling 100% server-side tracking.
It replaces fragile client-side scripts with a reliable, GDPR-compliant, and ad blocker–resistant solution, ensuring accurate data collection and attribution across major ad platforms.
✨ What’s Included
Stape App for Deco.cx with configuration via
deco.tsServer-side actions:
trackPageView– page views with real referrers, user agentstrackEcommerceEvent– purchase, add_to_cart, begin_checkout, etc.sendEvent&sendBasicEvent– analytics and custom eventssendEcommerceEvent– structured GA4 commerce eventstestConnection– validate container setupSections & Loaders:
StapeTracker– automatic server-side tracking sectionAnalytics/Stape– configuration displaygetStapeConfig&analyticsScript– container and script setupReal-time integrations with:
✅ Benefits
🔗 Resources
Summary by CodeRabbit
New Features
Documentation