fix: do not resolve query-param keys that name inherited prototype members

[mattbodle]

Summary

queryStringParser takes a caller-supplied list of keys and looks each one up on a plain object. obj[name] walks the prototype chain, so a key named constructor resolves to Object.prototype.constructor, passes the truthiness gate, and is copied into the result as though the URL had supplied it.

queryStringParser('https://x.com?foo=bar', ['constructor'])
// before: { constructor: 'function Object() { [native code] }' }
// after:  {}

A real ?constructor=legitimate in the URL still resolves — the guard is an own-property check, not a name ban. There is a test for that.

Reachability, stated plainly

Not reachable in production today. Both callers pass hardcoded key lists — integrationCapture's click IDs and pageViewTracker's ALLOWED_QUERY_PARAMS — and neither contains a prototype member name.

It is a latent bug in a shared exported util, and it becomes reachable input the moment either list can be extended from configuration. That is the next PR, which is why this one goes first and on its own.

Object.prototype is not polluted by any of this, before or after. I verified. This is a data-hygiene and dedup-integrity bug, not a prototype-pollution vulnerability, and I would rather label it accurately than escalate it.

Why guard the lookup instead of Object.create(null)

Object.create(null) on the lookup map looks like the smaller fix and kills the whole class at the root. It is the wrong call here for two concrete reasons:

1. queryStringParser returns that same map directly when no keys are passed (if (isEmpty(keys)) return lowerCaseUrlParams), so the null prototype would escape to callers.
2. integrationCapture.ts:289 calls .hasOwnProperty() on the result, which throws on a null-prototype object.

So the returned object stays plain, and a test pins that contract.

The other half, and its missing test

This also switches pageViewTracker's capturedNames() from in to the same own-property check.

That half has no test in this PR, deliberately and with an explanation. capturedNames() only ever tests names drawn from ALLOWED_QUERY_PARAMS; with a hardcoded list containing no prototype member name, in and hasOwnProperty cannot be told apart by any test. I wrote one, mutation-checked it, found it passed with the fix reverted, and deleted it rather than ship a test that implies coverage it does not have. It gains a real regression test in the follow-up PR, where the allowlist becomes configurable and the distinction becomes observable.

Verification

• jest — 698 pass / 0 fail (from 692; +6)
• karma ChromeHeadless — 1089 pass / 0 fail
• tsc -p . clean, eslint src/ test/src/ clean
• Mutation-checked: removing the guard fails queryStringParser › does not resolve keys naming inherited Object.prototype members. The two vacuous tests were found this way and removed or re-labelled.

Follow-ups

Part 1 of 4 for customer-configurable APV query params:

1. this PR — hardening
2. flag + union of ALLOWED_QUERY_PARAMS with a customer list
3. mPServer: SettingTemplate row
4. mPServer: Advanced Settings UI field

🤖 Generated with Claude Code

[cursor]

PR Summary

Low Risk
Localized query-param hygiene and dedup key logic; no auth or payment paths, and current callers use hardcoded key lists.

Overview
Hardens query-string parsing and auto page-view dedup so caller-supplied key names cannot pick up inherited Object.prototype members (e.g. constructor) as if they came from the URL.

Adds shared hasOwnProp in utils and uses it in queryStringParser before copying requested keys—real params like ?constructor=legitimate still work. pageViewTracker’s capturedNames switches from in to the same own-property check ahead of configurable allowlists.

New Jest cases cover false prototype-key resolution, legitimate same-name params, and plain-object results for callers that use .hasOwnProperty() on the parser output.

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

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
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

[mattbodle]

Superseded by #1375 — identical commit, branch renamed from hardening/ to fix/ so it passes the semantic branch-name check. Closing to avoid two PRs for one change.