feat(apv): union the query-param allowlist with a customer-configured list

[mattbodle]

Depends on #1375 — merge that first. This branch is stacked on it, so the diff shown here includes it until #1375 lands.

Summary

Adds the autoLogPageViewQueryParams feature flag. The dashboard sends a comma-separated list of extra query param names; the SDK unions it with the built-in ALLOWED_QUERY_PARAMS.

effective allowlist = ALLOWED_QUERY_PARAMS ∪ sort(customer additions)

Absent or empty behaves exactly as 2.80.x. That is the kill switch, and it means rolling this back is clearing a text field rather than cutting an SDK release — the whole reason it lives in config and not in the constant.

// flags: { autoLogPageViewQueryParams: 'promo_code, affiliate_id' }
data: { hostname, title, path: '/deal', utm_source: 'google', promo_code: 'SAVE20' }

No new window.mParticle surface.

Built-ins keep their position; customs are sorted

pageKey walks the effective list, so preserving the built-in prefix means a customer who opts in gets byte-identical dedup keys for pages that use none of their custom params. Reorder that prefix and adding a single param changes every key at once, firing one spurious page view per page on the first navigation after rollout.

The custom suffix is sorted, for the same reason one step further in. Appended in config order, the suffix depends on the sequence the customer typed rather than the set — so a customer who merely swaps two co-occurring custom params in the Advanced Settings field changes the key of every page carrying both, and each of those fires one spurious view. Sorting makes the suffix depend on the set. Built-ins keep their positions, and custom params do not exist in 2.80.x, so no key that exists today moves.

Correction from the first revision, which said "There is a test stating exactly that property" about byte-identity. The test it referred to called the new allowedQueryParams on both sides, so it compared new against new: if the whole union were sorted, both sides move together and it still passes. It is now asserted against a literal ('/p?utm_source=google&gclid=abc'), which is what byte-identical means. The built-in ordering was genuinely pinned, by 'should keep the built-ins first, in their existing order'.

Ordered pairs instead of a dictionary

pageKey and describePage previously took their ordering from the module constant via capturedNames(). With a configurable list, each would need the allowlist threaded in. Capturing the order at snapshot time means neither needs it at all, and capturedNames() is deleted.

Correction: the first revision framed this as forced. It was not. Threading the allowlist into pageKey's two call sites was a workable alternative. This was a preference between two shapes, and the pair list won on the grounds that fewer functions need to know about the allowlist — not because the alternative did not work.

Validation runs once, at the config boundary

processFlags parses and validates the customer's string as it arrives, and stores the whole verdict — the accepted names, the positions it rejected, and how many entries it dropped past the cap. Nothing downstream re-validates.

That is a change from the first revision of this branch, and it fixes a real bug (below).

The SDK validates at all — rather than trusting the dashboard — because remote config is untrusted input: it arrives over the network into a third-party embed running on the customer's page, and the SDK cannot assume anything checked it.

Rule	Reason
/^[a-z0-9_][a-z0-9_.-]{0,63}$/	excludes &, =, %, whitespace — what would let a name distort the dedup key or an attribute name
not hostname / title / path	the three core event fields
not constructor / __proto__ / prototype	resolve to something truthy through the prototype chain
not true / false	every other flag in this SDK is compared === 'True'. An operator who sets this one the same way sends the literal True, which is a legal param name — so it would silently pass validation and the SDK would start capturing ?true=.
max 25 entries	mParticle caps attributes per event; 24 built-ins + 25 leaves headroom
duplicate of a built-in → dropped, not rejected	asking for something you already have is not an error

Two bugs from the first revision of this branch

1. The rejected-name warning was dead in production

Found by Cursor Bugbot, and it was right. processFlags stored only parseQueryParamAllowlist(raw).allowed, and readCustomQueryParams then re-parsed that already-clean array — so rejected was always [] and the warning could never fire. Measured:

processFlags stores: ["promo_code"]     (from 'promo_code, bad name')
tracker re-parse -> rejected: []
warning fires in production? false

The tracker tests passed only because they mocked getFeatureFlag to return a raw comma-separated string, which production never sends.

The fix is not to move the parse. parseQueryParamAllowlist now returns one value carrying every part of its verdict, processFlags stores it whole, and the tracker reports from what it was handed. There is exactly one validator, at one boundary — which is also what AGENTS.md asks for ("only validate at system boundaries"). And the tracker test now hands the tracker the shape production sends, so it cannot pass for the wrong reason again.

2. The sanitiser leaked values

The first revision claimed the rejected-entry label could not echo a value. That was false. ., - and _ are all in the legal name class, so cutting at "the first illegal character" does not cut at all for most inputs; and a leading - or . is illegal only in first position, so /^[a-z0-9_.-]*/ consumed the entry whole and the truncation branch never ran. Measured against the old code:

entry	logged label
password=hunter2	password...	safe — the only case originally tested
token.hunter2 x	token.hunter2...	leaked
user.email.hunter2@x	user.email.hunter2...	leaked
-secret-value-abcdef	-secret-value-abcdef	leaked whole, no ellipsis
.hunter2	.hunter2	leaked whole

This goes to Logger.warning → console, which session-replay tooling ships off-domain — the exact scenario the function's own comment named.

Rejections are now reported as 1-based positions and never as text:

mParticle APV: ignoring invalid additional page view query parameters at positions 2, 4

A number cannot leak. It also fixes the useless empty label a non-ASCII entry produced (émail reported an empty name, because every character is illegal and the legal prefix is ""). Positions count every comma-separated slot including blanks, so a reported position lines up with what the customer typed.

Entries past the cap are now reported

The cap check used to return at the top of the loop, before validation — so entry 26 and beyond were neither accepted nor reported, and a customer who pasted 40 names got 25 with no indication that 15 vanished. The check moved below the duplicate and blank filters, so neither consumes headroom, and the losers are counted:

mParticle APV: ignoring 15 additional page view query parameters beyond the limit of 25

The value cap applies to custom params only

Built-in behaviour is untouched, so no existing customer loses a long utm_content on upgrade. Custom params are where unbounded choice enters: a param holding a base64 blob would bloat every APV event and every dedup key.

Dropped, not truncated. Truncating makes two different long values produce the same key, which silently swallows a real page view.

No aggregate payload bound exists. 25 custom params × 512 bytes ≈ 12.8 KB per APV event on top of the built-ins, in the worst case. There is no cap on the total, only per-value. I have deliberately not added one — a global bound needs a number someone owns, and the per-value cap plus the 25-entry cap is a reasonable first fence — but it should be a known number rather than a surprise.

effectiveAllowlist and a mis-shaped flag

Correction: the first revision said "A mis-shaped flag value threw inside logPageView". processFlags makes that unreachable — every path into SDKConfig.flags goes through it. effectiveAllowlist no longer accepts a raw string at all.

What it does still need to survive is the null that getFeatureFlag returns when the flag is absent, which is the common case and which a default parameter does not catch (defaults fire only for undefined). Hence || [], with a test for null specifically.

An array is trusted as pre-validated, which is what lets the tests inject hostile names directly and prove the own-property check in allowedQueryParams is a real backstop rather than dead code sitting behind validation.

getFeatureFlag's declared type

getFeatureFlag was declared boolean | string while IFeatureFlags already held a non-scalar for this flag, which forced an as unknown as double cast in events.ts. The declared return type (in helpers.ts and SDKHelpersApi) is widened to cover what processFlags actually stores, and the double cast is gone.

This carries #1375's missing test

#1375 switched capturedNames() from in to an own-property check and shipped without a test: with a hardcoded allowlist the two are unobservable through the public helpers, because no built-in names a prototype member. A configured list makes it observable without touching module state. Reverting that line now fails three tests here.

Verification

Measured on this branch's tip:

• jest — 768 pass / 0 fail, up from 700 on fix: do not resolve query-param keys that name inherited prototype members #1375 (+68)
• karma ChromeHeadless — 1089 pass / 0 fail, 10 skipped, after a fresh npm run build && npm run build:test-bundle
• tsc -p . --noEmit clean, eslint src/ test/src/ clean

Mutation-checked — each behaviour deliberately broken, and the named test(s) that catch it:

mutation	test(s) that fail
processFlags stores only .allowed (the Bugbot bug, restored)	Store #processFlags should return default featureFlags…, …should return featureFlags if featureFlags are passed in, Store #processConfig should process feature flags (karma)
tracker re-parses the list it was handed	5 tests in PageViewTracker › configured additional query params, incl. should warn with positions for what it rejected, never text
parser records the entry text instead of the position	26 tests, incl. all five should report only a position for … cases
.sort() dropped from the custom suffix	#effectiveAllowlist › should append extras sorted, whatever order they were given in
whole union sorted, built-ins included	#pageKey › should be this exact key with no custom params configured (+6 others). The old byte-identity test passed under this mutation, which is why it was replaced.
cap checked at the top of the loop again	should count the entries dropped past the cap, should not let duplicates consume cap headroom, should warn about entries dropped past the cap
true/false removed from the reserved list	the four should reject the boolean-looking value … cases
|| [] removed from effectiveAllowlist	#effectiveAllowlist › should tolerate null

One of Becca's reported leak cases, api-key-abc123def, turned out not to be a leak: it is a legal param name (- is in the body class), so the parser accepts it and never labels it. The old rejectedLabel did return it whole, but only when called directly — it was unreachable through parseQueryParamAllowlist. Noted for accuracy; the other five were real.

Follow-ups

Part 2 of 4:

1. fix: do not resolve query-param keys that name inherited prototype members #1375 — hardening
2. this PR — flag + union
3. mPServer: SettingTemplate row
4. mPServer: Advanced Settings UI field

Then #1374, which removes code, state, nonce and the other OAuth/OIDC params from the built-in list. That only stops being a capability loss once this PR lands.

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Changes APV attribution and SPA dedup behavior for configured params, and parses untrusted remote config; mitigations include validation at the config boundary and reserved-name checks, but misconfiguration could still alter which URL data reaches events.

Overview
Adds the autoLogPageViewQueryParams remote flag so customers can extend auto page view (APV) capture beyond the built-in query param list. processFlags parses the comma-separated config once into an IQueryParamAllowlist (accepted names, rejected positions, over-limit count); landing logPageView and PageViewTracker union those names with ALLOWED_QUERY_PARAMS when building page view attributes and dedup keys.

allowedQueryParams now returns ordered ICapturedParam[] (custom suffix sorted; built-in prefix unchanged so existing dedup keys stay stable when custom params are absent). Custom param values are capped at 512 chars (dropped, not truncated); invalid config entries trigger position-only warnings at tracker init, not re-parsed text in logs.

queryStringParser / hasOwnProp harden lookups against prototype pollution when the allowlist is customer-controlled.

^{Reviewed by Cursor Bugbot for commit b792672. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 0217059. Configure here.}

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud