Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions vtex/loaders/intelligentSearch/productListingPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,27 @@ export interface Props {
}
const searchArgsOf = (props: Props, url: URL) => {
const hideUnavailableItems = props.hideUnavailableItems;
const VALID_SIM_BEHAVIORS = new Set(["default", "skip", "only1P"]);
const rawSim = url.searchParams.get("simulationBehavior");
const simulationBehavior =
url.searchParams.get("simulationBehavior") as SimulationBehavior ||
props.simulationBehavior || "default";
(rawSim && VALID_SIM_BEHAVIORS.has(rawSim)
? rawSim
: props.simulationBehavior || "default") as SimulationBehavior;
Comment on lines +164 to +169

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize simulationBehavior consistently in loader and cache key.

Line 166–169 correctly normalizes invalid simulationBehavior to default, but cacheKey still includes raw query values (Line 475–478). Equivalent requests can generate different cache keys, enabling cache fragmentation with junk params.

Proposed fix (outside this hunk: cacheKey section)
+  const rawSimulationBehavior = url.searchParams.get("simulationBehavior");
+  const simulationBehavior = rawSimulationBehavior &&
+      VALID_SIM_BEHAVIORS.has(rawSimulationBehavior)
+    ? rawSimulationBehavior
+    : props.simulationBehavior || "default";
+
   const params = new URLSearchParams([
@@
-    [
-      "simulationBehavior",
-      url.searchParams.get("simulationBehavior") || props.simulationBehavior ||
-      "default",
-    ],
+    ["simulationBehavior", simulationBehavior],
   ]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vtex/loaders/intelligentSearch/productListingPage.ts` around lines 164 - 169,
The cacheKey currently uses the raw URL query value while the loader normalizes
simulationBehavior using VALID_SIM_BEHAVIORS and the simulationBehavior
variable, causing cache fragmentation; update the cacheKey construction to
reference the normalized simulationBehavior variable (the result of the
VALID_SIM_BEHAVIORS check / props fallback) instead of
url.searchParams.get("simulationBehavior") so equivalent requests produce the
same key and ensure the value is cast/typed as SimulationBehavior where needed.

const countFromSearchParams = url.searchParams.get("PS");
const count = Number(countFromSearchParams ?? props.count ?? 12);
const rawCount = countFromSearchParams ? Number(countFromSearchParams) : NaN;
const count = Number.isFinite(rawCount) && rawCount > 0
? Math.min(Math.floor(rawCount), 50)
: Number(props.count ?? 12);
const query = props.query ?? url.searchParams.get("q") ?? "";
const currentPageoffset = props.pageOffset ?? 1;
const rawPage = url.searchParams.get("page")
? Number(url.searchParams.get("page"))
: NaN;
const parsedPage = Number.isFinite(rawPage) && rawPage >= currentPageoffset
? rawPage - currentPageoffset
: 0;
Comment on lines +180 to +182

@cubic-dev-ai cubic-dev-ai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: page validation still accepts fractional values. Restrict to integer pages to avoid propagating non-integer pagination state.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vtex/loaders/intelligentSearch/productListingPage.ts, line 180:

<comment>`page` validation still accepts fractional values. Restrict to integer pages to avoid propagating non-integer pagination state.</comment>

<file context>
@@ -161,20 +161,27 @@ export interface Props {
+  const rawPage = url.searchParams.get("page")
+    ? Number(url.searchParams.get("page"))
+    : NaN;
+  const parsedPage = Number.isFinite(rawPage) && rawPage >= currentPageoffset
+    ? rawPage - currentPageoffset
+    : 0;
</file context>
Suggested change
const parsedPage = Number.isFinite(rawPage) && rawPage >= currentPageoffset
? rawPage - currentPageoffset
: 0;
const parsedPage = Number.isInteger(rawPage) && rawPage >= currentPageoffset
? rawPage - currentPageoffset
: 0;
Fix with Cubic

const page = props.page ??
Math.min(
url.searchParams.get("page")
? Number(url.searchParams.get("page")) - currentPageoffset
: 0,
VTEX_MAX_PAGES - currentPageoffset,
);
Math.min(parsedPage, VTEX_MAX_PAGES - currentPageoffset);
Comment on lines +177 to +184

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

page defensive parsing should enforce integer values.

Line 180 uses Number.isFinite, so ?page=1.5 is still accepted and converted into a fractional internal page index. Validate rawPage as an integer here as well.

Proposed fix
-  const parsedPage = Number.isFinite(rawPage) && rawPage >= currentPageoffset
+  const parsedPage = Number.isInteger(rawPage) && rawPage >= currentPageoffset
     ? rawPage - currentPageoffset
     : 0;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vtex/loaders/intelligentSearch/productListingPage.ts` around lines 177 - 184,
The current parsing allows fractional page values (rawPage) because it only
checks Number.isFinite; update the validation to require an integer: when
computing parsedPage use Number.isInteger(rawPage) (or equivalent integer check)
instead of Number.isFinite so values like ?page=1.5 are rejected; keep the
existing comparisons to currentPageoffset and VTEX_MAX_PAGES and ensure page
still falls back to 0 or props.page as before (refer to rawPage, parsedPage,
page, currentPageoffset, VTEX_MAX_PAGES).

const sort = (url.searchParams.get("sort")) ??
LEGACY_TO_IS[url.searchParams.get("O") ?? ""] ??
props.sort ??
Expand Down
26 changes: 20 additions & 6 deletions vtex/loaders/legacy/productListingPage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { LegacyItem } from "../../utils/types.ts";
import {
removeNonLatin1Chars,
removeScriptChars,
} from "../../../utils/normalize.ts";
import type { Filter, ProductListingPage } from "../../../commerce/types.ts";
import { STALE } from "../../../utils/fetch.ts";
import { AppContext } from "../../mod.ts";
Expand Down Expand Up @@ -194,7 +198,10 @@ const loader = async (
const filtersBehavior = props.filters || "dynamic";

const countFromSearchParams = url.searchParams.get("PS");
const count = Number(countFromSearchParams ?? props.count ?? 12);
const rawCount = countFromSearchParams ? Number(countFromSearchParams) : NaN;
const count = Number.isFinite(rawCount) && rawCount > 0
? Math.min(Math.floor(rawCount), 50)
: Number(props.count ?? 12);

const maybeMap = props.map || url.searchParams.get("map") || undefined;
let maybeTerm = props.term || url.pathname || "";
Expand All @@ -204,18 +211,25 @@ const loader = async (
maybeTerm = result?.possiblePaths[0] ?? maybeTerm;
}

const pageParam = url.searchParams.get("page")
? Number(url.searchParams.get("page")) - currentPageoffset
const rawPage = url.searchParams.get("page")
? Number(url.searchParams.get("page"))
: NaN;
const pageParam = Number.isFinite(rawPage) && rawPage >= currentPageoffset

@cubic-dev-ai cubic-dev-ai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: page validation still accepts fractional numbers, which can generate invalid pagination offsets (_from/_to). Restrict page to integers before computing offsets.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vtex/loaders/legacy/productListingPage.ts, line 217:

<comment>`page` validation still accepts fractional numbers, which can generate invalid pagination offsets (`_from`/`_to`). Restrict `page` to integers before computing offsets.</comment>

<file context>
@@ -204,18 +211,25 @@ const loader = async (
+  const rawPage = url.searchParams.get("page")
+    ? Number(url.searchParams.get("page"))
+    : NaN;
+  const pageParam = Number.isFinite(rawPage) && rawPage >= currentPageoffset
+    ? rawPage - currentPageoffset
     : 0;
</file context>
Fix with Cubic

? rawPage - currentPageoffset
: 0;
const page = props.page || pageParam;
const O: LegacySort = (url.searchParams.get("O") as LegacySort) ??
const page = props.page || Math.min(pageParam, MAX_ALLOWED_PAGES);
Comment on lines +214 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

page parsing still accepts non-integer values.

Line 217 validates only Number.isFinite, so inputs like ?page=1.5 pass and produce fractional _from/_to offsets later (Line 235–236). This should be integer-only in the loader too.

Proposed fix
-  const pageParam = Number.isFinite(rawPage) && rawPage >= currentPageoffset
+  const pageParam = Number.isInteger(rawPage) && rawPage >= currentPageoffset
     ? rawPage - currentPageoffset
     : 0;
📝 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 rawPage = url.searchParams.get("page")
? Number(url.searchParams.get("page"))
: NaN;
const pageParam = Number.isFinite(rawPage) && rawPage >= currentPageoffset
? rawPage - currentPageoffset
: 0;
const page = props.page || pageParam;
const O: LegacySort = (url.searchParams.get("O") as LegacySort) ??
const page = props.page || Math.min(pageParam, MAX_ALLOWED_PAGES);
const rawPage = url.searchParams.get("page")
? Number(url.searchParams.get("page"))
: NaN;
const pageParam = Number.isInteger(rawPage) && rawPage >= currentPageoffset
? rawPage - currentPageoffset
: 0;
const page = props.page || Math.min(pageParam, MAX_ALLOWED_PAGES);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vtex/loaders/legacy/productListingPage.ts` around lines 214 - 220, The loader
currently allows fractional pages because it only checks
Number.isFinite(rawPage); change the validation to require integers: parse the
query string into rawPage (keep the variable) then verify
Number.isInteger(rawPage) (or compare parseInt(pageStr,10) to the original
pageStr) and rawPage >= currentPageoffset before computing pageParam and page;
update the conditional that sets pageParam (uses rawPage and currentPageoffset)
so only integer rawPage values are accepted, and still clamp the result with
Math.min(..., MAX_ALLOWED_PAGES).

const rawSort = (url.searchParams.get("O") as LegacySort) ??
IS_TO_LEGACY[url.searchParams.get("sort") ?? ""] ??
props.sort ??
sortOptions[0].value as LegacySort;
const isSortValid = sortOptions.some((opt) => opt.value === rawSort);
const O = isSortValid ? rawSort : sortOptions[0].value as LegacySort;
const fq = [
...new Set([
...(props.fq ? [props.fq] : []),
...url.searchParams.getAll("fq"),
...url.searchParams.getAll("fq").map((v) =>

@cubic-dev-ai cubic-dev-ai Bot Mar 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: fq is sanitized for the backend request but not in cacheKey, causing cache-key fragmentation for equivalent sanitized queries.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vtex/loaders/legacy/productListingPage.ts, line 230:

<comment>`fq` is sanitized for the backend request but not in `cacheKey`, causing cache-key fragmentation for equivalent sanitized queries.</comment>

<file context>
@@ -204,18 +211,25 @@ const loader = async (
     ...new Set([
       ...(props.fq ? [props.fq] : []),
-      ...url.searchParams.getAll("fq"),
+      ...url.searchParams.getAll("fq").map((v) =>
+        removeScriptChars(removeNonLatin1Chars(v))
+      ),
</file context>
Fix with Cubic

removeScriptChars(removeNonLatin1Chars(v))
),
Comment on lines +230 to +232

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

fq sanitization is too destructive and can break valid filters.

Line 230–232 strips characters commonly used by legitimate VTEX filter syntax (e.g. separators/ranges/accents), which can silently alter queries and return wrong or empty PLPs. Use narrower sanitization (script tags/control chars) instead of broad character removal.

Safer direction
-      ...url.searchParams.getAll("fq").map((v) =>
-        removeScriptChars(removeNonLatin1Chars(v))
-      ),
+      ...url.searchParams.getAll("fq")
+        .map((v) =>
+          v
+            .replace(/<\/?script\b[^>]*>/gi, "")
+            .replace(/[\u0000-\u001F\u007F]/g, "")
+            .trim()
+        )
+        .filter(Boolean),
📝 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
...url.searchParams.getAll("fq").map((v) =>
removeScriptChars(removeNonLatin1Chars(v))
),
...url.searchParams.getAll("fq")
.map((v) =>
v
.replace(/<\/?script\b[^>]*>/gi, "")
.replace(/[\u0000-\u001F\u007F]/g, "")
.trim()
)
.filter(Boolean),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vtex/loaders/legacy/productListingPage.ts` around lines 230 - 232, The
current fq sanitization chain uses removeNonLatin1Chars which aggressively
strips valid VTEX filter characters and can break filters; update the map over
url.searchParams.getAll("fq") to only remove script/control characters by
calling removeScriptChars (and optional trim) and remove the
removeNonLatin1Chars call so separators/ranges/accents remain intact; locate the
code that maps getAll("fq") and replace the chained call
removeScriptChars(removeNonLatin1Chars(v)) with a narrower sanitizer such as
removeScriptChars(v).trim() (or an explicit control-char strip) so valid filter
syntax is preserved.

]),
];
const _from = page * count;
Expand Down
37 changes: 35 additions & 2 deletions vtex/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,36 @@ export const middleware = (
req: Request,
ctx: AppMiddlewareContext,
) => {
// Sanitize numeric query params early — invalid values (e.g. SSRF URLs
// passed as ?page=) cause NaN errors downstream in PLP loaders.
const url = new URL(req.url);
let dirty = false;

const page = url.searchParams.get("page");
if (page !== null) {
const num = Number(page);
if (!Number.isInteger(num) || num < 1) {
url.searchParams.delete("page");
dirty = true;
}
}

const ps = url.searchParams.get("PS");
if (ps !== null) {
const num = Number(ps);
if (!Number.isInteger(num) || num < 1 || num > 50) {
url.searchParams.delete("PS");
dirty = true;
}
}

if (dirty) {
return Promise.resolve(new Response(null, {
status: 301,
headers: { Location: url.pathname + url.search },
}));
}

const segment = getSegmentFromBag(ctx);
const isCookies = getISCookiesFromBag(ctx);
const cookies = getCookies(req.headers);
Expand All @@ -31,15 +61,18 @@ export const middleware = (

const isLoggedIn = Boolean(
cookies[VTEX_ID_CLIENT_COOKIE] ||
cookies[`${VTEX_ID_CLIENT_COOKIE}_${ctx.account}`]
cookies[`${VTEX_ID_CLIENT_COOKIE}_${ctx.account}`],
);

const cacheable = isCacheableSegment(ctx) && !isLoggedIn;

// 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");
ctx.response.headers.set(
"Cache-Control",
"no-store, no-cache, must-revalidate",
);
}

// PAGE_CACHE_ALLOWED_KEY: opts in to CDN page caching (VTEX-only)
Expand Down
Loading