Merged
Conversation
Remove server-side Set-Cookie for IS cookies (vtex_is_anonymous, vtex_is_session) and move persistence to client-side via document.cookie. Skip vtex_segment Set-Cookie when user is anonymous (deterministic segment). Clear dirty flag in middleware for anonymous users so deco runtime can set Cache-Control: public headers, enabling CDN caching.
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds segment cacheability detection and uses it in middleware to avoid cookie writes and to mark non-cacheable pages; moves analytics ISC cookie persistence client-side; removes intelligent-search cookie side effects; exposes Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Middleware as Middleware
participant SegmentUtil as SegmentUtil
participant Cookies as Cookies
participant Analytics as AnalyticsSnippet
Client->>Middleware: HTTP request
Middleware->>Cookies: getCookies(req)
Middleware->>SegmentUtil: set/check segment (setSegmentBag)
SegmentUtil->>Cookies: decide whether to write segment cookies (isCacheableSegment)
Cookies-->>SegmentUtil: skip or set cookies
SegmentUtil-->>Middleware: return segment info
Middleware->>Middleware: compute isLoggedIn, cacheable
alt cacheable
Middleware->>Middleware: set PAGE_CACHE_ALLOWED_KEY
else not cacheable
Middleware->>Middleware: set PAGE_DIRTY_KEY
Middleware->>Client: add Cache-Control: no-store,no-cache,must-revalidate
end
Middleware->>Analytics: continue request processing / render
Analytics->>Client: client-side snippet persists ISC cookies (anonymous/session sliding expiration)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vtex/middleware.ts`:
- Line 2: Remove the invalid import of PAGE_DIRTY_KEY from "@deco/deco/blocks"
and instead add a local definition at the top of the file: declare
PAGE_DIRTY_KEY as a Symbol (e.g. const PAGE_DIRTY_KEY = Symbol("pageDirty")) so
uses of PAGE_DIRTY_KEY in this module reference the local symbol; ensure the
import line is deleted and the new Symbol declaration appears before any code
that references PAGE_DIRTY_KEY.
In `@vtex/sections/Analytics/Vtex.tsx`:
- Around line 25-35: The cookie values are written directly to document.cookie
(via setCookieIfMissing) using cookies.anonymous and cookies.session without
validation or encoding, which can create invalid/malformed cookies; update
setCookieIfMissing to validate that the provided value is a string (or coerce
safely), URL-encode it (e.g. encodeURIComponent), and guard against
undefined/null/objects before building the cookie string, then use the sanitized
value when calling setCookieIfMissing for ANONYMOUS_COOKIE and SESSION_COOKIE;
ensure any non-string inputs are rejected or converted and that you only write
the cookie when the sanitized value is present.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
vtex/middleware.tsvtex/sections/Analytics/Vtex.tsxvtex/utils/intelligentSearch.tsvtex/utils/segment.ts
💤 Files with no reviewable changes (1)
- vtex/utils/intelligentSearch.ts
vtex/middleware.ts
Outdated
| @@ -1,10 +1,15 @@ | |||
| import { getCookies } from "std/http/cookie.ts"; | |||
| import { PAGE_DIRTY_KEY } from "@deco/deco/blocks"; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the import and check for PAGE_DIRTY_KEY in the codebase and upstream module
# 1. Check import usage in the codebase
echo "=== Checking for PAGE_DIRTY_KEY and `@deco/deco/blocks` imports ==="
rg -n "PAGE_DIRTY_KEY|@deco/deco/blocks" -C2
echo ""
echo "=== Checking dependency files for `@deco/deco` version ==="
fd 'deno.json*|import_map*|deps.ts|mod.ts' -x sed -n '1,220p' {}
echo ""
echo "=== Checking upstream `@deco/deco`@1.170.0 blocks module exports ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/blocks/mod.ts" | sed -n '1,240p'Repository: deco-cx/apps
Length of output: 50369
🏁 Script executed:
#!/bin/bash
# Check the pinned version of `@deco/deco` and verify exports
echo "=== Checking deno.lock for `@deco/deco` version ==="
cat deno.lock | grep -A5 '"jsr.io/@deco/deco"' | head -20
echo ""
echo "=== Checking deno.json for `@deco/deco` imports/version ==="
cat deno.json | grep -A2 -B2 'deco/deco' || echo "Not found in deno.json"
echo ""
echo "=== Fetching `@deco/deco`@1.170.0 blocks module - just first 100 lines ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/blocks/mod.ts" | head -100
echo ""
echo "=== Checking if PAGE_DIRTY_KEY is exported anywhere in `@deco/deco`@1.170.0 ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/blocks/mod.ts" | grep -i "PAGE_DIRTY_KEY"Repository: deco-cx/apps
Length of output: 1924
🏁 Script executed:
#!/bin/bash
# Search for PAGE_DIRTY_KEY more broadly
echo "=== Searching for PAGE_DIRTY_KEY in `@deco/deco` JSR ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/mod.ts" | grep -i "PAGE_DIRTY_KEY" | head -5
echo ""
echo "=== Checking `@deco/deco` main exports ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/mod.ts" | head -50
echo ""
echo "=== Check if PAGE_DIRTY_KEY is in utils or engine ==="
curl -fsSL "https://jsr.io/@deco/deco/1.170.0/engine/mod.ts" 2>/dev/null | grep -i "PAGE_DIRTY_KEY" | head -3
echo ""
echo "=== Try checking earlier version like 1.127.0 (current pin) ==="
curl -fsSL "https://jsr.io/@deco/deco/1.127.0/blocks/mod.ts" 2>/dev/null | grep -i "PAGE_DIRTY_KEY"Repository: deco-cx/apps
Length of output: 1829
🏁 Script executed:
#!/bin/bash
# Search the codebase for PAGE_DIRTY_KEY definition or similar patterns
echo "=== Search for PAGE_DIRTY_KEY definition anywhere in repo ==="
rg "PAGE_DIRTY_KEY\s*=" -n
echo ""
echo "=== Search for createBagKey usage (exported from `@deco/deco`) ==="
rg "createBagKey" -n
echo ""
echo "=== Search for bag.set patterns to understand correct usage ==="
rg "bag\.set\(" -B2 -A1 | head -30Repository: deco-cx/apps
Length of output: 1008
🏁 Script executed:
#!/bin/bash
# Check where IS_ANONYMOUS, IS_SESSION, DEVICE_ID are imported/defined
echo "=== Search for IS_ANONYMOUS definition ==="
rg "IS_ANONYMOUS" -n | head -10
echo ""
echo "=== Search for IS_SESSION definition ==="
rg "IS_SESSION" -n | head -10
echo ""
echo "=== Search for DEVICE_ID definition ==="
rg "DEVICE_ID" -n | head -10
echo ""
echo "=== Check what's actually imported in vtex/middleware.ts ==="
head -15 vtex/middleware.tsRepository: deco-cx/apps
Length of output: 1674
Remove broken import and define PAGE_DIRTY_KEY locally.
Line 2 imports PAGE_DIRTY_KEY from @deco/deco/blocks, but this is not exported. Define it locally as a Symbol instead, following the pattern used elsewhere in the codebase (see vtex/utils/intelligentSearch.ts for IS_ANONYMOUS and IS_SESSION, or linx-impulse/utils/deviceId.ts for DEVICE_ID):
const PAGE_DIRTY_KEY = Symbol("pageDirty");Remove the broken import and add this definition at the top of the file.
🧰 Tools
🪛 GitHub Actions: ci
[error] 2-2: TS2305: Module '"https://jsr.io/@deco/deco/1.170.0/blocks/mod.ts"' has no exported member 'PAGE_DIRTY_KEY'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/middleware.ts` at line 2, Remove the invalid import of PAGE_DIRTY_KEY
from "@deco/deco/blocks" and instead add a local definition at the top of the
file: declare PAGE_DIRTY_KEY as a Symbol (e.g. const PAGE_DIRTY_KEY =
Symbol("pageDirty")) so uses of PAGE_DIRTY_KEY in this module reference the
local symbol; ensure the import line is deleted and the new Symbol declaration
appears before any code that references PAGE_DIRTY_KEY.
vtex/sections/Analytics/Vtex.tsx
Outdated
Comment on lines
+25
to
+35
| const setCookieIfMissing = (name: string, value: string, maxAge: number) => { | ||
| document.cookie = | ||
| `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`; | ||
| }; | ||
|
|
||
| if (!document.cookie.includes(`${ANONYMOUS_COOKIE}=`)) { | ||
| setCookieIfMissing(ANONYMOUS_COOKIE, cookies.anonymous, ONE_YEAR_SECS); | ||
| } | ||
|
|
||
| // Always re-set session cookie to simulate sliding expiration | ||
| setCookieIfMissing(SESSION_COOKIE, cookies.session, THIRTY_MIN_SECS); |
Contributor
There was a problem hiding this comment.
Validate and encode cookie values before writing document.cookie.
At Line [31] and Line [35], values are written from any without validation/encoding. This can emit invalid cookies (undefined, objects) and malformed cookie strings if values contain reserved separators.
🔧 Proposed fix
const persistISCookies = (cookies: ISCookies | null) => {
if (!cookies) return;
- const setCookieIfMissing = (name: string, value: string, maxAge: number) => {
- document.cookie =
- `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`;
- };
+ const toCookieValue = (value: unknown): string | null => {
+ if (typeof value !== "string" || value.length === 0) return null;
+ return encodeURIComponent(value);
+ };
+
+ const setCookie = (name: string, rawValue: unknown, maxAge: number) => {
+ const value = toCookieValue(rawValue);
+ if (!value) return;
+ document.cookie =
+ `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`;
+ };
if (!document.cookie.includes(`${ANONYMOUS_COOKIE}=`)) {
- setCookieIfMissing(ANONYMOUS_COOKIE, cookies.anonymous, ONE_YEAR_SECS);
+ setCookie(ANONYMOUS_COOKIE, cookies.anonymous, ONE_YEAR_SECS);
}
// Always re-set session cookie to simulate sliding expiration
- setCookieIfMissing(SESSION_COOKIE, cookies.session, THIRTY_MIN_SECS);
+ setCookie(SESSION_COOKIE, cookies.session, THIRTY_MIN_SECS);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const setCookieIfMissing = (name: string, value: string, maxAge: number) => { | |
| document.cookie = | |
| `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`; | |
| }; | |
| if (!document.cookie.includes(`${ANONYMOUS_COOKIE}=`)) { | |
| setCookieIfMissing(ANONYMOUS_COOKIE, cookies.anonymous, ONE_YEAR_SECS); | |
| } | |
| // Always re-set session cookie to simulate sliding expiration | |
| setCookieIfMissing(SESSION_COOKIE, cookies.session, THIRTY_MIN_SECS); | |
| const persistISCookies = (cookies: ISCookies | null) => { | |
| if (!cookies) return; | |
| const toCookieValue = (value: unknown): string | null => { | |
| if (typeof value !== "string" || value.length === 0) return null; | |
| return encodeURIComponent(value); | |
| }; | |
| const setCookie = (name: string, rawValue: unknown, maxAge: number) => { | |
| const value = toCookieValue(rawValue); | |
| if (!value) return; | |
| document.cookie = | |
| `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`; | |
| }; | |
| if (!document.cookie.includes(`${ANONYMOUS_COOKIE}=`)) { | |
| setCookie(ANONYMOUS_COOKIE, cookies.anonymous, ONE_YEAR_SECS); | |
| } | |
| // Always re-set session cookie to simulate sliding expiration | |
| setCookie(SESSION_COOKIE, cookies.session, THIRTY_MIN_SECS); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/sections/Analytics/Vtex.tsx` around lines 25 - 35, The cookie values are
written directly to document.cookie (via setCookieIfMissing) using
cookies.anonymous and cookies.session without validation or encoding, which can
create invalid/malformed cookies; update setCookieIfMissing to validate that the
provided value is a string (or coerce safely), URL-encode it (e.g.
encodeURIComponent), and guard against undefined/null/objects before building
the cookie string, then use the sanitized value when calling setCookieIfMissing
for ANONYMOUS_COOKIE and SESSION_COOKIE; ensure any non-string inputs are
rejected or converted and that you only write the cookie when the sanitized
value is present.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="vtex/utils/segment.ts">
<violation number="1" location="vtex/utils/segment.ts:76">
P2: isCacheableSegment ignores channelPrivacy, so private-channel segments can be treated as cacheable and skip Set-Cookie, risking cached private pricing/content. Include channelPrivacy in the cacheability check.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Private channels (B2B) require login to view prices/products. Without this check, private-channel segments could be cached publicly, risking exposure of restricted pricing or content.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
vtex/utils/segment.ts (1)
246-258: Avoid redundantVTEXSCwrites and keep the comment in sync.On Line 251,
VTEXSCis set wheneverscexists in the request, even if the cookie already has the same value. Add a diff check (like Line 260 does forvtex_segment) and update the comment to includechannelPrivacy.Proposed patch
- // Only set cookies when content-affecting fields differ (campaigns, - // non-default sales channel, price tables, region). + // Only set cookies when content-affecting fields differ (campaigns, + // non-default sales channel, price tables, region, channel privacy). if (!isCacheableSegment(ctx)) { - if (segmentFromRequest.channel) { + const requestedChannel = segmentFromRequest.channel; + const currentChannel = cookies[SALES_CHANNEL_COOKIE]?.split("=")[1]; + if (requestedChannel && requestedChannel !== currentChannel) { setCookie(ctx.response.headers, { - value: `sc=${segmentFromRequest.channel}`, + value: `sc=${requestedChannel}`, name: SALES_CHANNEL_COOKIE, path: "/", secure: true, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vtex/utils/segment.ts` around lines 246 - 258, The code currently always writes SALES_CHANNEL_COOKIE when segmentFromRequest.channel exists even if the existing VTEXSC cookie already matches; update the logic in the non-cacheable branch (where setCookie is called for SALES_CHANNEL_COOKIE) to first read the current SALES_CHANNEL_COOKIE value from ctx.request.headers/cookies and only call setCookie if it differs (mirror the existing diff-check used for vtex_segment), and also update the preceding comment to mention channelPrivacy along with UTMs so the comment stays in sync with the behavior; touch the isCacheableSegment check, segmentFromRequest.channel usage, SALES_CHANNEL_COOKIE constant, and setCookie invocation to implement this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@vtex/utils/segment.ts`:
- Around line 246-258: The code currently always writes SALES_CHANNEL_COOKIE
when segmentFromRequest.channel exists even if the existing VTEXSC cookie
already matches; update the logic in the non-cacheable branch (where setCookie
is called for SALES_CHANNEL_COOKIE) to first read the current
SALES_CHANNEL_COOKIE value from ctx.request.headers/cookies and only call
setCookie if it differs (mirror the existing diff-check used for vtex_segment),
and also update the preceding comment to mention channelPrivacy along with UTMs
so the comment stays in sync with the behavior; touch the isCacheableSegment
check, segmentFromRequest.channel usage, SALES_CHANNEL_COOKIE constant, and
setCookie invocation to implement this change.
UTMs can affect pricing in VTEX, so by default they make pages non-cacheable (delegates to isAnonymous). Only with the opt-in removeUTMFromCacheKey flag are UTMs ignored for caching decisions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vtex/utils/segment.ts`:
- Around line 244-267: The code skips Set-Cookie for cacheable segments
(isCacheableSegment) but there is no client-side persistence for segment
cookies; update the Analytics snippet (vtex/sections/Analytics/Vtex.tsx) to
persist the same segment values the server would set: when page context exposes
vtex_segment (token) or segmentFromRequest.channel, write client-side cookies
with names SEGMENT_COOKIE_NAME and SALES_CHANNEL_COOKIE respectively, using
path="/" and secure=true (note httpOnly cannot be set client-side), and ensure
values mirror those produced by setCookie in the server logic so cacheable
segments still persist on the client.
Comment on lines
+244
to
267
| // Skip Set-Cookie when the segment only differs by UTMs. | ||
| // UTMs don't affect page content, so the response can still be cached. | ||
| // Only set cookies when content-affecting fields differ (campaigns, | ||
| // non-default sales channel, price tables, region). | ||
| if (!isCacheableSegment(ctx)) { | ||
| if (segmentFromRequest.channel) { | ||
| setCookie(ctx.response.headers, { | ||
| value: `sc=${segmentFromRequest.channel}`, | ||
| name: SALES_CHANNEL_COOKIE, | ||
| path: "/", | ||
| secure: true, | ||
| }); | ||
| } | ||
|
|
||
| // Avoid setting cookie when segment from request matches the one generated | ||
| if (vtex_segment !== token) { | ||
| setCookie(ctx.response.headers, { | ||
| value: token, | ||
| name: SEGMENT_COOKIE_NAME, | ||
| path: "/", | ||
| secure: true, | ||
| httpOnly: true, | ||
| }); | ||
| if (vtex_segment !== token) { | ||
| setCookie(ctx.response.headers, { | ||
| value: token, | ||
| name: SEGMENT_COOKIE_NAME, | ||
| path: "/", | ||
| secure: true, | ||
| httpOnly: true, | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all TypeScript/JavaScript files that might contain Analytics snippet or client-side cookie handling
fd -t f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) | head -20Repository: deco-cx/apps
Length of output: 286
🏁 Script executed:
#!/bin/bash
# Search for Analytics-related files
find . -type f -name "*analytics*" -o -name "*Analytics*" | grep -v node_modules | head -20Repository: deco-cx/apps
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Search for document.cookie usage across the codebase
rg "document\.cookie" -t ts -t tsx -t js -A 2 -B 2 | head -100Repository: deco-cx/apps
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Search for segment cookie handling (vtex_segment, SEGMENT_COOKIE_NAME, etc.)
rg "vtex_segment|SEGMENT_COOKIE_NAME|SALES_CHANNEL_COOKIE" -t ts -t tsx -t js -A 2 -B 2 | head -100Repository: deco-cx/apps
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Check the current file to understand the cookie constants being used
cat -n vtex/utils/segment.ts | sed -n '1,50p'Repository: deco-cx/apps
Length of output: 1586
🏁 Script executed:
#!/bin/bash
# View lines 244-280 of segment.ts to see full context around the reviewed code
cat -n vtex/utils/segment.ts | sed -n '244,280p'Repository: deco-cx/apps
Length of output: 966
🏁 Script executed:
#!/bin/bash
# Find the isCacheableSegment function definition
rg "isCacheableSegment" vtex/utils/segment.ts -B 5 -A 10Repository: deco-cx/apps
Length of output: 1280
🏁 Script executed:
#!/bin/bash
# Check the VTEX Analytics section implementation
find ./vtex/sections/Analytics -type f \( -name "*.ts" -o -name "*.js" \) | xargs catRepository: deco-cx/apps
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Search for client-side cookie persistence in VTEX-related analytics
rg "document\.cookie" . -g "vtex*" -B 2 -A 2 2>/dev/null | head -80Repository: deco-cx/apps
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check the main DecoAnalytics component for client-side cookie handling
cat analytics/components/DecoAnalytics.tsx 2>/dev/null | head -150Repository: deco-cx/apps
Length of output: 3138
🏁 Script executed:
#!/bin/bash
# Check the analytics loader script
cat analytics/loaders/DecoAnalyticsScript.ts 2>/dev/nullRepository: deco-cx/apps
Length of output: 2977
🏁 Script executed:
#!/bin/bash
# Search for any client-side cookie logic across analytics files
rg "document\.cookie|localStorage|sessionStorage" . -g "analytics*" -g "*Analytics*" -B 2 -A 2 2>/dev/null | head -80Repository: deco-cx/apps
Length of output: 475
🏁 Script executed:
#!/bin/bash
# Check if there are any TypeScript/JavaScript files in the VTEX Analytics section
find ./vtex/sections/Analytics -type f 2>/dev/null | xargs ls -laRepository: deco-cx/apps
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Look for how segment token is handled in client-side code
rg "vtex_segment|SEGMENT_COOKIE_NAME" . --type js --type ts -B 3 -A 3 2>/dev/null | head -100Repository: deco-cx/apps
Length of output: 6261
🏁 Script executed:
#!/bin/bash
# Check the VTEX Analytics implementation for segment cookie handling
cat ./vtex/sections/Analytics/Vtex.tsxRepository: deco-cx/apps
Length of output: 3902
🏁 Script executed:
#!/bin/bash
# Search for any reference to setting cookies client-side in the entire codebase
rg "setCookie|document\.cookie\s*=" -t ts -t js --max-count=20 2>/dev/null | grep -v node_modulesRepository: deco-cx/apps
Length of output: 6856
🏁 Script executed:
#!/bin/bash
# Check if there's any client-side segment handling at all
rg "vtex_segment|SALES_CHANNEL_COOKIE|SEGMENT_COOKIE_NAME" . -t ts -t js --type-not minified -B 2 -A 2 2>/dev/null | grep -v "utils/segment.ts" | head -80Repository: deco-cx/apps
Length of output: 3802
Critical: Client-side cookie persistence is not implemented for segment cookies.
The server-side logic correctly skips Set-Cookie headers for cacheable segments (lines 248-267). However, the original review claims that "client-side cookie persistence has moved to the Analytics snippet," but the VTEX Analytics implementation (vtex/sections/Analytics/Vtex.tsx) only handles vtex_is_anonymous and vtex_is_session cookies—not vtex_segment or VTEXSC.
This creates a gap: For cacheable segments (UTM-only or default), cookies are neither set server-side nor persisted client-side. Clarify the intended behavior:
- Is missing client-side persistence intentional (cacheable segments are stateless)?
- Or does the segment handling need to be added to the Analytics snippet alongside the existing Intelligent Search cookie persistence?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/utils/segment.ts` around lines 244 - 267, The code skips Set-Cookie for
cacheable segments (isCacheableSegment) but there is no client-side persistence
for segment cookies; update the Analytics snippet
(vtex/sections/Analytics/Vtex.tsx) to persist the same segment values the server
would set: when page context exposes vtex_segment (token) or
segmentFromRequest.channel, write client-side cookies with names
SEGMENT_COOKIE_NAME and SALES_CHANNEL_COOKIE respectively, using path="/" and
secure=true (note httpOnly cannot be set client-side), and ensure values mirror
those produced by setCookie in the server logic so cacheable segments still
persist on the client.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="vtex/utils/segment.ts">
<violation number="1" location="vtex/utils/segment.ts:79">
P1: isCacheableSegment no longer excludes private channels, so channelPrivacy="private" segments can be cached and shared across users. This risks leaking private-channel content through CDN caching. Reintroduce a channelPrivacy check in both the removeUTMFromCacheKey branch and the default path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Private channels may not always generate session cookies, so explicitly reject caching when channelPrivacy is "private" to prevent leaking restricted content through CDN.
- VTEX middleware opts in to page caching via PAGE_CACHE_ALLOWED_KEY - Check VtexIdclientAutCookie to detect logged-in users - Move IS cookie generation to client-side for cache-safe HTML - Mark deterministic matchers as cacheable (queryString, pathname, host, device, site, environment)
guitavano
approved these changes
Mar 3, 2026
vtex/sections/Analytics/Vtex.tsx
Outdated
Comment on lines
+17
to
+18
| const ANONYMOUS_COOKIE = "vtex_is_anonymous"; | ||
| const SESSION_COOKIE = "vtex_is_session"; |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
vtex/middleware.ts (1)
2-2:⚠️ Potential issue | 🔴 Critical
PAGE_DIRTY_KEYimport still appears unresolved and may fail type-check.This looks like the same unresolved import problem already reported earlier (
TS2305risk), and it can block CI/build.Run this read-only verification to confirm exports for your pinned
@deco/decoversion:#!/bin/bash set -euo pipefail echo "== Import in vtex/middleware.ts ==" sed -n '1,20p' vtex/middleware.ts | rg -n "PAGE_(DIRTY|CACHE_ALLOWED)_KEY|@deco/deco/blocks" echo echo "== Detect pinned `@deco/deco` version from deno.lock ==" VER="$(rg -o 'jsr\.io/@deco/deco/[0-9]+\.[0-9]+\.[0-9]+' deno.lock | head -n1 | sed 's#jsr.io/@deco/deco/##')" if [ -z "${VER}" ]; then VER="1.170.0" fi echo "Using version: ${VER}" echo echo "== Check blocks/mod.ts exports for that version ==" curl -fsSL "https://jsr.io/@deco/deco/${VER}/blocks/mod.ts" | rg -n "PAGE_DIRTY_KEY|PAGE_CACHE_ALLOWED_KEY"Expected:
PAGE_CACHE_ALLOWED_KEYandPAGE_DIRTY_KEYshould both be exported.
IfPAGE_DIRTY_KEYis missing, define it locally in this module and remove it from the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vtex/middleware.ts` at line 2, Import of PAGE_DIRTY_KEY from `@deco/deco/blocks` is unresolved and may fail type-checking; check whether PAGE_DIRTY_KEY is actually exported for the pinned `@deco/deco` version and if it is missing remove PAGE_DIRTY_KEY from the import line and define a local constant (e.g., export const PAGE_DIRTY_KEY = "page:dirty" or the project-specific key value) inside the vtex middleware module, leaving PAGE_CACHE_ALLOWED_KEY imported as before so code referencing PAGE_DIRTY_KEY uses the newly defined local constant.vtex/sections/Analytics/Vtex.tsx (1)
21-24:⚠️ Potential issue | 🟠 MajorSanitize and encode cookie values before writing
document.cookie(still unresolved).
setCookiestill writes raw values. SincegetOrCreateISCookiescan reuse pre-existing cookie values, malformed values can propagate into cookie writes.Proposed fix
const getCookie = (name: string): string | null => { const match = document.cookie.match(new RegExp(`(?:^|; )${name}=([^;]*)`)); - return match ? match[1] : null; + if (!match) return null; + try { + return decodeURIComponent(match[1]); + } catch { + return null; + } }; -const setCookie = (name: string, value: string, maxAge: number) => { +const toCookieValue = (value: unknown): string | null => { + if (typeof value !== "string" || value.length === 0) return null; + return encodeURIComponent(value); +}; + +const setCookie = (name: string, rawValue: unknown, maxAge: number) => { + const value = toCookieValue(rawValue); + if (!value) return; document.cookie = `${name}=${value};path=/;max-age=${maxAge};secure;SameSite=Lax`; };Also applies to: 27-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vtex/sections/Analytics/Vtex.tsx` around lines 21 - 24, The setCookie function writes raw cookie values; update setCookie (and any callers like getOrCreateISCookies) to validate and percent-encode name and value before writing document.cookie (e.g., use encodeURIComponent for both), ensure maxAge is a non-negative integer, and reject or normalize values containing forbidden characters (semicolon, comma, whitespace) to prevent malformation and injection; make sure getOrCreateISCookies passes sanitized/encoded strings to setCookie so reused cookie values are safe.
🧹 Nitpick comments (1)
vtex/sections/Analytics/Vtex.tsx (1)
16-18: Escape cookie name in regex matcher for future-safe parsing.
getCookieinterpolatesnamedirectly intoRegExp. Escaping the name avoids regex edge cases if names change later.Proposed hardening
const getCookie = (name: string): string | null => { - const match = document.cookie.match(new RegExp(`(?:^|; )${name}=([^;]*)`)); + const escapedName = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const match = document.cookie.match( + new RegExp(`(?:^|; )${escapedName}=([^;]*)`), + ); return match ? match[1] : null; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vtex/sections/Analytics/Vtex.tsx` around lines 16 - 18, The getCookie function interpolates the cookie name directly into a RegExp which is brittle if the name contains regex-special chars; add an escape helper (e.g., escapeRegex that uses name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) and use it when building the RegExp in getCookie (replace new RegExp(`(?:^|; )${name}=([^;]*)`) with new RegExp(`(?:^|; )${escapeRegex(name)}=([^;]*)`) so cookie names are safely matched).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vtex/middleware.ts`:
- Line 42: The file vtex/middleware.ts fails deno fmt --check due to formatting;
run the Deno formatter (deno fmt) on the repository or specifically on
vtex/middleware.ts, verify the change around the ctx.response.headers.set(...)
line is applied, and commit the formatted file so CI passes the formatting
check.
---
Duplicate comments:
In `@vtex/middleware.ts`:
- Line 2: Import of PAGE_DIRTY_KEY from `@deco/deco/blocks` is unresolved and may
fail type-checking; check whether PAGE_DIRTY_KEY is actually exported for the
pinned `@deco/deco` version and if it is missing remove PAGE_DIRTY_KEY from the
import line and define a local constant (e.g., export const PAGE_DIRTY_KEY =
"page:dirty" or the project-specific key value) inside the vtex middleware
module, leaving PAGE_CACHE_ALLOWED_KEY imported as before so code referencing
PAGE_DIRTY_KEY uses the newly defined local constant.
In `@vtex/sections/Analytics/Vtex.tsx`:
- Around line 21-24: The setCookie function writes raw cookie values; update
setCookie (and any callers like getOrCreateISCookies) to validate and
percent-encode name and value before writing document.cookie (e.g., use
encodeURIComponent for both), ensure maxAge is a non-negative integer, and
reject or normalize values containing forbidden characters (semicolon, comma,
whitespace) to prevent malformation and injection; make sure
getOrCreateISCookies passes sanitized/encoded strings to setCookie so reused
cookie values are safe.
---
Nitpick comments:
In `@vtex/sections/Analytics/Vtex.tsx`:
- Around line 16-18: The getCookie function interpolates the cookie name
directly into a RegExp which is brittle if the name contains regex-special
chars; add an escape helper (e.g., escapeRegex that uses
name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) and use it when building the RegExp
in getCookie (replace new RegExp(`(?:^|; )${name}=([^;]*)`) with new
RegExp(`(?:^|; )${escapeRegex(name)}=([^;]*)`) so cookie names are safely
matched).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
vtex/middleware.tsvtex/sections/Analytics/Vtex.tsxwebsite/matchers/device.tswebsite/matchers/environment.tswebsite/matchers/host.tswebsite/matchers/pathname.tswebsite/matchers/queryString.tswebsite/matchers/site.ts
✅ Files skipped from review due to trivial changes (2)
- website/matchers/site.ts
- website/matchers/pathname.ts
| // PAGE_DIRTY_KEY: marks page dirty for section-level caching and other consumers | ||
| if (!cacheable) { | ||
| ctx.bag.set(PAGE_DIRTY_KEY, true); | ||
| ctx.response.headers.set("Cache-Control", "no-store, no-cache, must-revalidate"); |
Contributor
There was a problem hiding this comment.
Formatting check is currently failing CI.
The pipeline reports deno fmt --check failure; please run formatter and commit the result.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/middleware.ts` at line 42, The file vtex/middleware.ts fails deno fmt
--check due to formatting; run the Deno formatter (deno fmt) on the repository
or specifically on vtex/middleware.ts, verify the change around the
ctx.response.headers.set(...) line is applied, and commit the formatted file so
CI passes the formatting check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this Contribution About?
Please provide a brief description of the changes or enhancements you are proposing in this pull request.
Issue Link
Please link to the relevant issue that this pull request addresses:
Loom Video
Demonstration Link
Summary by cubic
Enable CDN caching for anonymous VTEX pages. Middleware opts into caching for cacheable, logged-out requests and sets no-store for others.
Written for commit 344a85d. Summary will update on new commits.
Summary by CodeRabbit
Performance Improvements
Behavior Changes