feat(proxy): granular per-script privacy controls#611
Merged
Conversation
Replace binary `'proxy' | 'anonymize'` privacy mode with per-flag object model. Each script declares exactly what data it needs, anonymizing everything else. Privacy flags: `ip`, `userAgent`, `language`, `screen`, `timezone`, `hardware` - Untrusted ad networks (Meta, TikTok, X, Snapchat, Reddit): full anonymization - Analytics (GA): preserve UA/screen/timezone for reports, anonymize rest - Session recording (Clarity, Hotjar): preserve UA/screen/timezone for heatmaps - Trusted tools (PostHog, Segment, GTM): no anonymization (full fidelity) Users can override per-script defaults globally via `firstParty.privacy`.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
📝 WalkthroughWalkthroughThis PR replaces the two-mode first-party privacy model with a per-script, per-flag privacy object (ip, userAgent, language, screen, timezone, hardware). It adds types and utilities (ProxyPrivacy, ProxyPrivacyInput, ResolvedProxyPrivacy, resolvePrivacy, mergePrivacy), updates proxy configs with per-script defaults and a global override, threads resolved privacy into proxy request/response handling (route overrides, query/header/body stripping and normalization), updates public API typing for scripts.firstParty, and adjusts docs and tests to the new model. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
test/unit/proxy-configs.test.ts (2)
162-171:getAllProxyConfigspresence check is missing several PR-updated scripts.
xPixel,snapchatPixel,redditPixel, andposthogall have per-script privacy defaults added in this PR but are not asserted in this test.♻️ Proposed additions
expect(configs).toHaveProperty('googleAnalytics') expect(configs).toHaveProperty('googleTagManager') expect(configs).toHaveProperty('metaPixel') expect(configs).toHaveProperty('tiktokPixel') expect(configs).toHaveProperty('segment') expect(configs).toHaveProperty('clarity') expect(configs).toHaveProperty('hotjar') + expect(configs).toHaveProperty('xPixel') + expect(configs).toHaveProperty('snapchatPixel') + expect(configs).toHaveProperty('redditPixel')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/proxy-configs.test.ts` around lines 162 - 171, The test "returns all proxy configs" calls getAllProxyConfigs but omits assertions for newly added scripts; update that test to assert presence of the new proxy config keys by adding expect(configs).toHaveProperty('xPixel'), expect(configs).toHaveProperty('snapchatPixel'), expect(configs).toHaveProperty('redditPixel'), and expect(configs).toHaveProperty('posthog') so the test covers all PR-updated scripts.
173-182: "All configs have valid structure" has no coverage for the newprivacyfield.The
privacy: ProxyPrivacyInputproperty is the primary new surface area added by this PR toProxyConfig, yet the structural validation loop never asserts on it. Scripts likemetaPixel,tiktokPixel, andxPixelare expected to carry fully-anonymized privacy objects, whilesegmentandgoogleTagManagershould have no-op defaults. Without assertions here a missing or malformedprivacydeclaration inproxy-configs.tswould go undetected.Consider adding:
🧪 Proposed addition to the structural validation loop
for (const [key, config] of Object.entries(configs)) { expect(config, `${key} should have routes`).toHaveProperty('routes') expect(typeof config.routes, `${key}.routes should be an object`).toBe('object') if (config.rewrite) { expect(Array.isArray(config.rewrite), `${key}.rewrite should be an array`).toBe(true) } + if (config.privacy) { + expect(typeof config.privacy, `${key}.privacy should be an object`).toBe('object') + const knownFlags = ['ip', 'userAgent', 'language', 'screen', 'timezone', 'hardware'] + for (const flag of Object.keys(config.privacy)) { + expect(knownFlags, `${key}.privacy.${flag} is not a recognised flag`).toContain(flag) + expect(typeof config.privacy[flag], `${key}.privacy.${flag} should be boolean`).toBe('boolean') + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/proxy-configs.test.ts` around lines 173 - 182, The test "all configs have valid structure" should assert the new privacy field: after calling getAllProxyConfigs('/_scripts/c') and iterating entries, verify each config has a privacy property of type ProxyPrivacyInput (exists and is an object). Additionally assert that specific proxies have the expected privacy shapes: for 'metaPixel', 'tiktokPixel', and 'xPixel' assert the privacy object represents a fully-anonymized configuration (check presence/values that denote anonymization), and for 'segment' and 'googleTagManager' assert the privacy object is the no-op/default privacy shape; update the structural validation loop in that test to include these assertions so malformed or missing privacy declarations in proxy-configs.ts will fail.test/unit/proxy-privacy.test.ts (1)
149-150: Remove debugconsole.warnfrom committed unit tests.Lines [149], [198], and [218] add noisy test output without assertion value. Prefer keeping these tests silent in normal CI runs.
Also applies to: 198-198, 218-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/proxy-privacy.test.ts` around lines 149 - 150, Remove the three debug console.warn calls left in the unit test (the console.warn invocations that log "GA fingerprinting params found:" and "GA normalized params:"), deleting them from the test or replacing them with real assertions if the output needs to be validated; search for console.warn in proxy-privacy.test.ts and remove these noisy logs so tests remain silent in CI and ensure no other console.warns remain in that test file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/1.guides/2.first-party.md`:
- Line 118: The sentence "When a flag is active, data is **generalized** rather
than stripped — analytics endpoints still receive valid data, just with reduced
precision..." is inaccurate because some fields are redacted/emptied rather than
merely generalized; update that sentence to say flags either generalize (reduce
precision) or redact/empty sensitive fields, and keep the examples (screen
resolution and User-Agent) while adding hardware examples like
canvas/webgl/plugins/fonts being zeroed or cleared to illustrate redaction.
In `@docs/content/scripts/analytics/posthog.md`:
- Line 164: Replace the phrase "full fidelity data" with the hyphenated compound
modifier "full-fidelity data" in the sentence that reads "No privacy
anonymization is applied — PostHog is a trusted, open-source tool that requires
full fidelity data for GeoIP enrichment, feature flags, and session replay." so
the compound modifier is correctly hyphenated.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 161-171: The loop that handles client hints in proxy-handler.ts
only normalizes sec-ch-ua and sec-ch-ua-full-version-list; other high-entropy
hints (sec-ch-ua-platform-version, sec-ch-ua-arch, sec-ch-ua-model,
sec-ch-ua-bitness) are passed through unchanged. Update the header handling
where lowerKey is checked (the same block that references privacy.hardware,
headers[key], and value) so that when privacy.hardware is true you either strip
these headers or replace their values with a normalized/sanitized token (e.g.,
remove numeric/precise details or set to a fixed generic string) instead of
forwarding the raw value; keep the existing sec-ch-ua handling behavior for
consistency. Ensure the changes occur in the same header-processing loop so
lowerKey checks for 'sec-ch-ua-platform-version', 'sec-ch-ua-arch',
'sec-ch-ua-model', and 'sec-ch-ua-bitness' and apply the normalization/drop
logic when privacy.hardware is true.
- Around line 173-184: The current code overwrites any existing x-forwarded-for
chain with getRequestIP(...), losing intermediate proxies; change the logic in
proxy-handler.ts so you only assign headers['x-forwarded-for'] when it was not
already copied by the earlier header loop, and if privacy.ip is true anonymize
the existing header value (or each IP in the chain) instead of replacing it with
a single clientIP; use getRequestIP(event, { xForwardedFor: true }) and
anonymizeIP(...) only as fallbacks when no x-forwarded-for header exists.
- Around line 140-147: The header handling loop currently assigns IP-related
headers using the original casing (headers[key] = value) while later code
reads/writes headers['x-forwarded-for'] (lowercase), which can create duplicate
headers; update the loop in proxy-handler.ts to normalize IP-related header keys
to lowercase before assignment (use lowerKey instead of key for headers
assignment when lowerKey matches the IP set or remove any existing variant keys
first), ensuring checks use lowerKey and preserving the privacy.ip behavior so
downstream references to headers['x-forwarded-for'] see the single canonical
header.
- Around line 90-96: The privacy resolution (using matchedRoutePattern,
routePrivacy, resolvePrivacy, mergePrivacy, privacy, anyPrivacy) is being
performed before the route-not-found guard; move the block that computes
perScriptInput, perScriptResolved, privacy (and anyPrivacy) to after the
validation that ensures a matched route exists (the 404 check) so you only
compute privacy when matchedRoutePattern is defined and a route is found,
avoiding unnecessary work when responding with 404.
In `@src/runtime/server/utils/privacy.ts`:
- Around line 396-400: The code checks only for string values when anonymizing
language params, so array-valued keys like "languages" bypass p.language
normalization; update the block that uses NORMALIZE_PARAMS and p.language
(around the isLanguageParam check) to also detect Array.isArray(value) and, if
true, set result[key] to value.map(v => typeof v === 'string' ?
normalizeLanguage(v) : v), otherwise keep the existing string branch
(result[key] = normalizeLanguage(value)); preserve non-string/non-array values
unchanged and still continue after assigning to result.
In `@test/e2e/first-party.test.ts`:
- Around line 621-622: The current assertion only checks c.privacy is a non-null
object, which allows empty or malformed privacy flags; update the tests that use
the c.privacy check (the predicate referencing c.privacy) to assert the expected
flag schema instead — e.g., verify required keys exist and have correct types
(boolean or specific allowed values) for each privacy flag rather than just
typeof/object, by replacing the loose check with explicit assertions for the
individual flags on c.privacy (for example:
expect(c.privacy).toHaveProperty('shareUsage', 'boolean') / assert typeof
c.privacy.shareUsage === 'boolean', etc.) so regressions in per-flag values are
caught.
---
Nitpick comments:
In `@test/unit/proxy-configs.test.ts`:
- Around line 162-171: The test "returns all proxy configs" calls
getAllProxyConfigs but omits assertions for newly added scripts; update that
test to assert presence of the new proxy config keys by adding
expect(configs).toHaveProperty('xPixel'),
expect(configs).toHaveProperty('snapchatPixel'),
expect(configs).toHaveProperty('redditPixel'), and
expect(configs).toHaveProperty('posthog') so the test covers all PR-updated
scripts.
- Around line 173-182: The test "all configs have valid structure" should assert
the new privacy field: after calling getAllProxyConfigs('/_scripts/c') and
iterating entries, verify each config has a privacy property of type
ProxyPrivacyInput (exists and is an object). Additionally assert that specific
proxies have the expected privacy shapes: for 'metaPixel', 'tiktokPixel', and
'xPixel' assert the privacy object represents a fully-anonymized configuration
(check presence/values that denote anonymization), and for 'segment' and
'googleTagManager' assert the privacy object is the no-op/default privacy shape;
update the structural validation loop in that test to include these assertions
so malformed or missing privacy declarations in proxy-configs.ts will fail.
In `@test/unit/proxy-privacy.test.ts`:
- Around line 149-150: Remove the three debug console.warn calls left in the
unit test (the console.warn invocations that log "GA fingerprinting params
found:" and "GA normalized params:"), deleting them from the test or replacing
them with real assertions if the output needs to be validated; search for
console.warn in proxy-privacy.test.ts and remove these noisy logs so tests
remain silent in CI and ensure no other console.warns remain in that test file.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
docs/content/docs/1.guides/2.first-party.mddocs/content/scripts/analytics/google-analytics.mddocs/content/scripts/analytics/posthog.mddocs/content/scripts/marketing/clarity.mddocs/content/scripts/marketing/hotjar.mddocs/content/scripts/tracking/google-tag-manager.mddocs/content/scripts/tracking/meta-pixel.mddocs/content/scripts/tracking/reddit-pixel.mddocs/content/scripts/tracking/segment.mddocs/content/scripts/tracking/snapchat-pixel.mddocs/content/scripts/tracking/tiktok-pixel.mddocs/content/scripts/tracking/x-pixel.mdsrc/module.tssrc/proxy-configs.tssrc/runtime/server/proxy-handler.tssrc/runtime/server/utils/privacy.tstest/e2e/first-party.test.tstest/fixtures/first-party/nuxt.config.tstest/unit/proxy-configs.test.tstest/unit/proxy-privacy.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/e2e/first-party.test.ts (1)
110-142:⚠️ Potential issue | 🟠 Major
verifyFingerprintingAnonymized()will false-positive when inputs are already generalized (flaky e2e risk).Right now you only treat “already anonymized” as empty/0/empty array/object. But a lot of valid anonymized outputs are non-empty (e.g. timezone
"UTC", screen buckets like"1920x1080", UA already normalized, “major-only” version strings). If a provider already emits those values, the test will flag a “leak” even though nothing sensitive was forwarded.Concrete hardening approach
// Values considered already-anonymized (empty/zeroed) — not a leak even if unchanged - const isAnonymizedValue = (v: unknown) => + const SCREEN_BUCKETS = new Set(['1920x1080', '768x1024', '360x640']) + const isMajorOnlyVersion = (s: string) => /^\d+(?:[._-]0)+$/.test(s) // e.g. 143.0.0.0, 6.0.0 + const isNormalizedUA = (s: string) => /^Mozilla\/5\.0 \(compatible; (?:Chrome|Safari|Firefox|Edge|Opera)\/\d+\.0\)$/.test(s) + + const isAnonymizedValue = (v: unknown) => v === '' || v === 0 || (Array.isArray(v) && v.length === 0) || (typeof v === 'object' && v !== null && !Array.isArray(v) && Object.keys(v).length === 0) + || v === 'UTC' + || (typeof v === 'string' && SCREEN_BUCKETS.has(v)) + || (typeof v === 'string' && isMajorOnlyVersion(v)) + || (typeof v === 'string' && isNormalizedUA(v))(You can extend this with dimension buckets
1920,1080, etc., if those show up in captures.)Also applies to: 117-121, 122-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/first-party.test.ts` around lines 110 - 142, verifyFingerprintingAnonymized is giving false positives because isAnonymizedValue only treats empty/0/empty-collection as anonymized; update isAnonymizedValue (and keep ANONYMIZED_FINGERPRINT_PARAMS) to also recognize common already-generalized values (e.g., timezone strings like "UTC" or other fixed tz names, screen-size buckets or "WIDTHxHEIGHT" patterns and numeric bucket forms, UA normalizations or vendor-less tokens, and major-only version formats like "90" or "90.0" via regexes), and treat those string/number patterns as anonymized so the leak check (JSON.stringify comparison in verifyFingerprintingAnonymized) skips them; add pattern-based checks (regex) and explicit known-values list inside isAnonymizedValue so existing comparisons in verifyFingerprintingAnonymized remain unchanged.test/unit/proxy-configs.test.ts (1)
161-175:⚠️ Potential issue | 🟠 MajorTighten unit validation: ensure
privacyis non-null and includes all 6 boolean flags (passthrough currently allows{}).In the passthrough branch,
{}would pass becauseObject.values({})is empty. Alsotypeof null === 'object'.Proposed hardening
it('all configs have valid structure', () => { const configs = getAllProxyConfigs('/_scripts/c') const fullAnonymize = ['metaPixel', 'tiktokPixel', 'xPixel', 'snapchatPixel', 'redditPixel'] const passthrough = ['segment', 'googleTagManager', 'posthog'] + const PRIVACY_KEYS = ['ip', 'userAgent', 'language', 'screen', 'timezone', 'hardware'] as const for (const [key, config] of Object.entries(configs)) { expect(config, `${key} should have routes`).toHaveProperty('routes') expect(typeof config.routes, `${key}.routes should be an object`).toBe('object') if (config.rewrite) { expect(Array.isArray(config.rewrite), `${key}.rewrite should be an array`).toBe(true) } // Every config must declare a privacy object expect(config, `${key} should have privacy`).toHaveProperty('privacy') - expect(typeof config.privacy, `${key}.privacy should be an object`).toBe('object') + expect(config.privacy !== null && typeof config.privacy === 'object', `${key}.privacy should be a non-null object`).toBe(true) + for (const k of PRIVACY_KEYS) { + expect(typeof (config.privacy as any)[k], `${key}.privacy.${k} should be boolean`).toBe('boolean') + } if (fullAnonymize.includes(key)) { expect(config.privacy, `${key} should be fully anonymized`).toEqual({ ip: true, userAgent: true, language: true, screen: true, timezone: true, hardware: true, }) } if (passthrough.includes(key)) { // All flags should be false (no-op privacy) - for (const flag of Object.values(config.privacy)) { + for (const flag of Object.values(config.privacy)) { expect(flag, `${key} privacy flags should be false`).toBe(false) } } } })Also applies to: 177-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/proxy-configs.test.ts` around lines 161 - 175, The test for getAllProxyConfigs currently allows a passthrough {} or null privacy because Object.values({}) is empty and typeof null === 'object'; update the unit test(s) (the getAllProxyConfigs describe/it blocks) to assert that each proxy config's privacy property is non-null and an object and explicitly contains the six boolean flags (e.g., privacy.<flagName> for each expected flag) and that each flag's type is boolean, and apply the same stricter assertions to the other related test block(s) covering the passthrough branch.
🧹 Nitpick comments (6)
docs/content/docs/1.guides/2.first-party.md (1)
75-89: Move the table legend above the table rows.The key
✓ = anonymized, - = passed throughis placed at Line 89, after all the data rows. Readers encounter the symbols before seeing the legend, so they must scan ahead to understand the table. Moving the legend to just above the table header (before line 75) would remove that ambiguity.📝 Suggested placement
+✓ = anonymized, - = passed through + | Script | ip | userAgent | language | screen | timezone | hardware | Rationale | |--------|:--:|:---------:|:--------:|:------:|:--------:|:--------:|-----------| | Google Analytics | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for device, time, and OS reports | ... | Hotjar | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | - -✓ = anonymized, - = passed through🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/1.guides/2.first-party.md` around lines 75 - 89, Move the legend line "✓ = anonymized, - = passed through" so it appears immediately above the table header (the line starting with "| Script | ip | userAgent | language | screen | timezone | hardware | Rationale |") instead of after the final data row; update the markdown so readers see the key before encountering the symbols in the table rows (no other content changes needed).src/runtime/server/utils/privacy.ts (2)
1-70: AlignProxyPrivacyInputtyping/docs with runtime null-handling (or drop the null branches).
resolvePrivacy()/mergePrivacy()treatnullas “passthrough”, butProxyPrivacyInputdoesn’t includenull, which makes the API contract slightly ambiguous.Two reasonable options
- export type ProxyPrivacyInput = boolean | ProxyPrivacy + export type ProxyPrivacyInput = boolean | ProxyPrivacy | nullor (if you want to keep the type strict) remove
input === null/override === nullchecks and let callers normalize upstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/utils/privacy.ts` around lines 1 - 70, The code treats null as passthrough in resolvePrivacy and mergePrivacy but ProxyPrivacyInput doesn't include null; update the type to include null (export type ProxyPrivacyInput = boolean | ProxyPrivacy | null) so the runtime behavior matches the type signature, and adjust the doc comment if needed; alternatively, if you prefer strict typing, remove the input === null and override === null checks from resolvePrivacy and mergePrivacy instead—refer to the ProxyPrivacyInput type and the resolvePrivacy and mergePrivacy functions when making the change.
366-371: Consider centralizing the “all flags true” default to avoid drift.You inline
{ ip: true, userAgent: true, ... }as the default whenprivacyis omitted. A shared constant (orresolvePrivacy(true)) makes it harder to accidentally change one place but not others.Also applies to: 484-493
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/utils/privacy.ts` around lines 366 - 371, Centralize the “all flags true” default by introducing a single shared constant or using resolvePrivacy(true) instead of inlining { ip: true, userAgent: true, language: true, screen: true, timezone: true, hardware: true } in stripPayloadFingerprinting; update stripPayloadFingerprinting (and the other occurrence in this file that currently inlines the same object) to use that shared constant or resolvePrivacy(true) so both places reference the same canonical default ResolvedProxyPrivacy value.src/runtime/server/proxy-handler.ts (1)
33-45: AvoidString(value)for query params when values can be objects/arrays (safer serialization).If
stripPayloadFingerprintingever yields a non-primitive for a query key,String(value)will become"[object Object]", corrupting the forwarded query.Safer coercion
for (const [key, value] of Object.entries(stripped)) { if (value !== undefined && value !== null) { - params.set(key, String(value)) + params.set( + key, + typeof value === 'string' ? value + : typeof value === 'number' ? String(value) + : typeof value === 'boolean' ? String(value) + : Array.isArray(value) ? value.map(v => typeof v === 'string' ? v : JSON.stringify(v)).join(',') + : JSON.stringify(value), + ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/proxy-handler.ts` around lines 33 - 45, The current stripQueryFingerprinting function uses String(value) which turns objects/arrays into "[object Object]"; update stripQueryFingerprinting to serialize non-primitive values safely: after calling stripPayloadFingerprinting, for each entry use JSON.stringify for objects/arrays (e.g., typeof value === 'object' && value !== null) and value.toISOString() for Date instances, otherwise coerce primitives with String(value); keep the existing undefined/null filtering and set the resulting string into URLSearchParams. Reference: stripQueryFingerprinting and stripPayloadFingerprinting.test/e2e/first-party.test.ts (2)
444-465: Avoid unconditionally writing debug artifacts into the fixture directory.
proxy-test.jsonandsw-status.jsonare written even on success. This tends to dirty working trees locally and can create surprising artifacts in CI.One minimal pattern
- writeFileSync(join(fixtureDir, 'proxy-test.json'), JSON.stringify(response, null, 2)) + if (process.env.DEBUG_E2E === '1') { + writeFileSync(join(fixtureDir, 'proxy-test.json'), JSON.stringify(response, null, 2)) + } // ... - writeFileSync(join(fixtureDir, 'sw-status.json'), JSON.stringify({ - swStatus, - swLogs: swLogs.filter(l => l.includes('SW') || l.includes('service') || l.includes('worker')), - }, null, 2)) + if (process.env.DEBUG_E2E === '1') { + writeFileSync(join(fixtureDir, 'sw-status.json'), JSON.stringify({ + swStatus, + swLogs: swLogs.filter(l => l.includes('SW') || l.includes('service') || l.includes('worker')), + }, null, 2)) + }Also applies to: 500-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/first-party.test.ts` around lines 444 - 465, The test "proxy endpoint works directly" unconditionally writes debug artifacts (proxy-test.json and similarly sw-status.json) into the fixtureDir which dirties working trees; change the writeFileSync calls inside the it block(s) so they only write when needed (e.g. on failure: if (typeof response === 'object' && response.error) or when a deliberate env flag is set like process.env.UPDATE_FIXTURES === '1'), or alternatively write to os.tmpdir() instead of fixtureDir; update the code references to the writeFileSync calls in the 'proxy endpoint works directly' test (and the similar sw-status writing code around lines 500-505) to follow this conditional or temp-directory approach.
621-622: Extract ahasResolvedPrivacy()helper to avoid copy/paste drift.The repeated inline predicate is easy to accidentally edit inconsistently across providers; a small helper will keep the assertions uniform.
Proposed refactor
+const PRIVACY_KEYS = ['ip', 'userAgent', 'language', 'screen', 'timezone', 'hardware'] as const +function hasResolvedPrivacy(v: unknown): v is Record<(typeof PRIVACY_KEYS)[number], boolean> { + return typeof v === 'object' + && v !== null + && PRIVACY_KEYS.every(k => typeof (v as any)[k] === 'boolean') +} const hasValidCapture = captures.some(c => c.path?.startsWith('/_proxy/') && (isAllowedDomain(c.targetUrl, 'google-analytics.com') || isAllowedDomain(c.targetUrl, 'analytics.google.com')) - && c.privacy && typeof c.privacy === 'object' && typeof c.privacy.ip === 'boolean' && typeof c.privacy.userAgent === 'boolean' && typeof c.privacy.language === 'boolean' && typeof c.privacy.screen === 'boolean' && typeof c.privacy.timezone === 'boolean' && typeof c.privacy.hardware === 'boolean', + && hasResolvedPrivacy(c.privacy), )Also applies to: 655-656, 694-695, 731-732, 752-753, 788-789, 833-834, 873-874, 911-912, 949-950
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/first-party.test.ts` around lines 621 - 622, Extract the repeated inline predicate into a single helper function named hasResolvedPrivacy(obj) that returns true only when obj.privacy exists and each of privacy.ip, userAgent, language, screen, timezone, and hardware is a boolean; then replace all occurrences of the long inline check (currently using variable c) with calls to hasResolvedPrivacy(c) to avoid copy/paste drift and keep assertions consistent across providers (update all similar assertions in the test suite to use this helper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/1.guides/2.first-party.md`:
- Line 63: Update the docs table entry for the `userAgent` flag to reflect the
full normalized User-Agent output; replace the short example "Chrome/131.0" in
the `userAgent` row with the full format "Mozilla/5.0 (compatible;
Chrome/131.0)" so the table matches the actual normalization shown elsewhere in
the document.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 99-105: routePrivacy[matchedRoutePattern] can be undefined which
makes resolvePrivacy(undefined) default to all-false (privacy off); instead
fail-closed by treating a missing per-script entry as the most restrictive
privacy (e.g., set perScriptInput to a strict default with all strips enabled
before calling resolvePrivacy) and then proceed to merge with globalPrivacy as
before; optionally, if a debug flag is true, throw createError(500, ...) when
routePrivacy[matchedRoutePattern] is missing to surface misconfiguration
(referencing symbols: routePrivacy, matchedRoutePattern, perScriptInput,
resolvePrivacy, mergePrivacy, privacy, anyPrivacy, debug, createError).
---
Outside diff comments:
In `@test/e2e/first-party.test.ts`:
- Around line 110-142: verifyFingerprintingAnonymized is giving false positives
because isAnonymizedValue only treats empty/0/empty-collection as anonymized;
update isAnonymizedValue (and keep ANONYMIZED_FINGERPRINT_PARAMS) to also
recognize common already-generalized values (e.g., timezone strings like "UTC"
or other fixed tz names, screen-size buckets or "WIDTHxHEIGHT" patterns and
numeric bucket forms, UA normalizations or vendor-less tokens, and major-only
version formats like "90" or "90.0" via regexes), and treat those string/number
patterns as anonymized so the leak check (JSON.stringify comparison in
verifyFingerprintingAnonymized) skips them; add pattern-based checks (regex) and
explicit known-values list inside isAnonymizedValue so existing comparisons in
verifyFingerprintingAnonymized remain unchanged.
In `@test/unit/proxy-configs.test.ts`:
- Around line 161-175: The test for getAllProxyConfigs currently allows a
passthrough {} or null privacy because Object.values({}) is empty and typeof
null === 'object'; update the unit test(s) (the getAllProxyConfigs describe/it
blocks) to assert that each proxy config's privacy property is non-null and an
object and explicitly contains the six boolean flags (e.g., privacy.<flagName>
for each expected flag) and that each flag's type is boolean, and apply the same
stricter assertions to the other related test block(s) covering the passthrough
branch.
---
Nitpick comments:
In `@docs/content/docs/1.guides/2.first-party.md`:
- Around line 75-89: Move the legend line "✓ = anonymized, - = passed through"
so it appears immediately above the table header (the line starting with "|
Script | ip | userAgent | language | screen | timezone | hardware | Rationale
|") instead of after the final data row; update the markdown so readers see the
key before encountering the symbols in the table rows (no other content changes
needed).
In `@src/runtime/server/proxy-handler.ts`:
- Around line 33-45: The current stripQueryFingerprinting function uses
String(value) which turns objects/arrays into "[object Object]"; update
stripQueryFingerprinting to serialize non-primitive values safely: after calling
stripPayloadFingerprinting, for each entry use JSON.stringify for objects/arrays
(e.g., typeof value === 'object' && value !== null) and value.toISOString() for
Date instances, otherwise coerce primitives with String(value); keep the
existing undefined/null filtering and set the resulting string into
URLSearchParams. Reference: stripQueryFingerprinting and
stripPayloadFingerprinting.
In `@src/runtime/server/utils/privacy.ts`:
- Around line 1-70: The code treats null as passthrough in resolvePrivacy and
mergePrivacy but ProxyPrivacyInput doesn't include null; update the type to
include null (export type ProxyPrivacyInput = boolean | ProxyPrivacy | null) so
the runtime behavior matches the type signature, and adjust the doc comment if
needed; alternatively, if you prefer strict typing, remove the input === null
and override === null checks from resolvePrivacy and mergePrivacy instead—refer
to the ProxyPrivacyInput type and the resolvePrivacy and mergePrivacy functions
when making the change.
- Around line 366-371: Centralize the “all flags true” default by introducing a
single shared constant or using resolvePrivacy(true) instead of inlining { ip:
true, userAgent: true, language: true, screen: true, timezone: true, hardware:
true } in stripPayloadFingerprinting; update stripPayloadFingerprinting (and the
other occurrence in this file that currently inlines the same object) to use
that shared constant or resolvePrivacy(true) so both places reference the same
canonical default ResolvedProxyPrivacy value.
In `@test/e2e/first-party.test.ts`:
- Around line 444-465: The test "proxy endpoint works directly" unconditionally
writes debug artifacts (proxy-test.json and similarly sw-status.json) into the
fixtureDir which dirties working trees; change the writeFileSync calls inside
the it block(s) so they only write when needed (e.g. on failure: if (typeof
response === 'object' && response.error) or when a deliberate env flag is set
like process.env.UPDATE_FIXTURES === '1'), or alternatively write to os.tmpdir()
instead of fixtureDir; update the code references to the writeFileSync calls in
the 'proxy endpoint works directly' test (and the similar sw-status writing code
around lines 500-505) to follow this conditional or temp-directory approach.
- Around line 621-622: Extract the repeated inline predicate into a single
helper function named hasResolvedPrivacy(obj) that returns true only when
obj.privacy exists and each of privacy.ip, userAgent, language, screen,
timezone, and hardware is a boolean; then replace all occurrences of the long
inline check (currently using variable c) with calls to hasResolvedPrivacy(c) to
avoid copy/paste drift and keep assertions consistent across providers (update
all similar assertions in the test suite to use this helper).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/content/docs/1.guides/2.first-party.mddocs/content/scripts/analytics/posthog.mdsrc/runtime/server/proxy-handler.tssrc/runtime/server/utils/privacy.tstest/e2e/first-party.test.tstest/unit/proxy-configs.test.tstest/unit/proxy-privacy.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/content/scripts/analytics/posthog.md
Comment on lines
99
to
105
| // Resolve effective privacy: per-script is the base, global user override on top | ||
| const perScriptInput = routePrivacy[matchedRoutePattern] | ||
| const perScriptResolved = resolvePrivacy(perScriptInput) | ||
| // Global override: when set by user, it overrides per-script field-by-field | ||
| const privacy = globalPrivacy !== undefined ? mergePrivacy(perScriptResolved, globalPrivacy) : perScriptResolved | ||
| const anyPrivacy = privacy.ip || privacy.userAgent || privacy.language || privacy.screen || privacy.timezone || privacy.hardware | ||
|
|
There was a problem hiding this comment.
Fail-closed if routePrivacy[matchedRoutePattern] is missing (avoid “privacy silently off”).
If routePrivacy[matchedRoutePattern] is undefined due to misconfiguration, resolvePrivacy(undefined) becomes all-false, turning stripping off. For a privacy control, that’s a risky default.
Defensive fallback (minimal)
// Resolve effective privacy: per-script is the base, global user override on top
const perScriptInput = routePrivacy[matchedRoutePattern]
+ // Defensive: if routePrivacy mapping is missing, avoid silently disabling privacy.
+ // Prefer full anonymization (or throw when debug is enabled).
+ const perScriptResolved = resolvePrivacy(perScriptInput ?? true)
- const perScriptResolved = resolvePrivacy(perScriptInput)
// Global override: when set by user, it overrides per-script field-by-field
const privacy = globalPrivacy !== undefined ? mergePrivacy(perScriptResolved, globalPrivacy) : perScriptResolvedAlternative (stricter): if debug is true, throw createError(500, ...) when the routePrivacy entry is missing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/server/proxy-handler.ts` around lines 99 - 105,
routePrivacy[matchedRoutePattern] can be undefined which makes
resolvePrivacy(undefined) default to all-false (privacy off); instead
fail-closed by treating a missing per-script entry as the most restrictive
privacy (e.g., set perScriptInput to a strict default with all strips enabled
before calling resolvePrivacy) and then proceed to merge with globalPrivacy as
before; optionally, if a debug flag is true, throw createError(500, ...) when
routePrivacy[matchedRoutePattern] is missing to surface misconfiguration
(referencing symbols: routePrivacy, matchedRoutePattern, perScriptInput,
resolvePrivacy, mergePrivacy, privacy, anyPrivacy, debug, createError).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/server/proxy-handler.ts (1)
217-231:⚠️ Potential issue | 🟠 MajorTop-level JSON arrays can be corrupted during stripping.
At Line [217] and Line [231], arrays satisfy
typeof === 'object'and are cast toRecord<string, unknown>. That risks converting array payload shape into object-like output, which can break batch endpoints.🔧 Proposed fix
- let body: string | Record<string, unknown> | undefined + let body: string | Record<string, unknown> | unknown[] | undefined @@ - if (typeof rawBody === 'object') { + if (Array.isArray(rawBody)) { + body = rawBody.map(item => + typeof item === 'object' && item !== null + ? stripPayloadFingerprinting(item as Record<string, unknown>, privacy) + : item, + ) + } + else if (typeof rawBody === 'object') { // JSON body - strip fingerprinting recursively body = stripPayloadFingerprinting(rawBody as Record<string, unknown>, privacy) } @@ - if (parsed && typeof parsed === 'object') { + if (Array.isArray(parsed)) { + body = parsed.map(item => + typeof item === 'object' && item !== null + ? stripPayloadFingerprinting(item as Record<string, unknown>, privacy) + : item, + ) + } + else if (parsed && typeof parsed === 'object') { body = stripPayloadFingerprinting(parsed as Record<string, unknown>, privacy) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/proxy-handler.ts` around lines 217 - 231, The current checks treat any typeof rawBody === 'object' (and parsed JSON objects) as Record<string, unknown>, which will corrupt top-level arrays; update the branches in proxy-handler.ts around the rawBody handling (the if (typeof rawBody === 'object') and the parsed JSON branch) to first detect Array.isArray(rawBody) (and Array.isArray(parsed)) and handle arrays separately (either by iterating and applying stripPayloadFingerprinting to each element or by calling an array-aware variant), and avoid casting arrays to Record<string, unknown>; ensure stripPayloadFingerprinting is adjusted or an array-specific helper is used so top-level arrays are preserved.
🧹 Nitpick comments (1)
test/e2e/first-party.test.ts (1)
126-143:isAnonymizedValueallowlist is broad enough to mask real leaks.At Line [138], treating IANA timezone names as anonymized, and at Line [142], accepting many non-bucket numeric values, can create false negatives in leak detection. Tighten this predicate to match actual anonymizer outputs (or make checks conditional on active
capture.privacyflags).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/first-party.test.ts` around lines 126 - 143, The isAnonymizedValue predicate is too permissive: tighten the IANA timezone check and narrow/condition the numeric bucket allowlist. In function isAnonymizedValue, replace the loose timezone test (currently matching /^[A-Z][a-z]+\/[A-Z]/ or treating 'UTC' unconditionally) with a stricter pattern that matches full IANA names (e.g. ^[A-Za-z]+\/[A-Za-z_+-]+$) and only allow 'UTC' explicitly; and make the numeric bucket check (the array of screen widths/heights) either reduced to only true anonymizer-produced buckets or applied only when a privacy flag is active (e.g. consult capture.privacy or a capture.privacy.bucketing flag) so non-bucket numeric values are not falsely classified as anonymized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/1.guides/2.first-party.md`:
- Around line 75-90: The table has a stray legend row ("✓ = anonymized, - =
passed through") inside the Markdown table which breaks parsing (MD055/MD056);
remove that line from the table block and place the legend as plain text
immediately before or after the table, or alternatively convert it into a valid
8-column row matching the table schema; update the content around the table in
docs/content/docs/1.guides/2.first-party.md so the table contains only rows with
8 cells and the legend is outside the pipe-delimited table.
In `@src/runtime/server/utils/privacy.ts`:
- Around line 401-417: The exact-key checks for language and user-agent
normalization (isLanguageParam using NORMALIZE_PARAMS.language and
isUserAgentParam using NORMALIZE_PARAMS.userAgent) miss bracket-style keys like
"languages[0]" or "user_agent[0]"; replace those comparisons with the existing
matchesParam() utility (same approach used for other privacy params) so keys are
matched permissively, then apply p.language/p.userAgent gating and normalize via
normalizeLanguage (and the existing string/array handling) to ensure bracketed
keys are normalized as intended.
---
Outside diff comments:
In `@src/runtime/server/proxy-handler.ts`:
- Around line 217-231: The current checks treat any typeof rawBody === 'object'
(and parsed JSON objects) as Record<string, unknown>, which will corrupt
top-level arrays; update the branches in proxy-handler.ts around the rawBody
handling (the if (typeof rawBody === 'object') and the parsed JSON branch) to
first detect Array.isArray(rawBody) (and Array.isArray(parsed)) and handle
arrays separately (either by iterating and applying stripPayloadFingerprinting
to each element or by calling an array-aware variant), and avoid casting arrays
to Record<string, unknown>; ensure stripPayloadFingerprinting is adjusted or an
array-specific helper is used so top-level arrays are preserved.
---
Nitpick comments:
In `@test/e2e/first-party.test.ts`:
- Around line 126-143: The isAnonymizedValue predicate is too permissive:
tighten the IANA timezone check and narrow/condition the numeric bucket
allowlist. In function isAnonymizedValue, replace the loose timezone test
(currently matching /^[A-Z][a-z]+\/[A-Z]/ or treating 'UTC' unconditionally)
with a stricter pattern that matches full IANA names (e.g.
^[A-Za-z]+\/[A-Za-z_+-]+$) and only allow 'UTC' explicitly; and make the numeric
bucket check (the array of screen widths/heights) either reduced to only true
anonymizer-produced buckets or applied only when a privacy flag is active (e.g.
consult capture.privacy or a capture.privacy.bucketing flag) so non-bucket
numeric values are not falsely classified as anonymized.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/content/docs/1.guides/2.first-party.mdsrc/runtime/server/proxy-handler.tssrc/runtime/server/utils/privacy.tstest/e2e/first-party.test.tstest/unit/proxy-configs.test.ts
Comment on lines
+75
to
+90
| | Script | ip | userAgent | language | screen | timezone | hardware | Rationale | | ||
| |--------|:--:|:---------:|:--------:|:------:|:--------:|:--------:|-----------| | ||
| | Google Analytics | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for device, time, and OS reports | | ||
| | Google Tag Manager | - | - | - | - | - | - | Container script loading — no user data in requests | | ||
| | Meta Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | ||
| | TikTok Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | ||
| | X/Twitter Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | ||
| | Snapchat Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | ||
| | Reddit Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | ||
| | Segment | - | - | - | - | - | - | Trusted data pipeline — full fidelity required | | ||
| | PostHog | - | - | - | - | - | - | Trusted, open-source — full fidelity required | | ||
| | Microsoft Clarity | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | ||
| ✓ = anonymized, - = passed through | ||
|
|
||
| | Hotjar | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | ||
|
|
There was a problem hiding this comment.
Legend placement breaks table structure and markdownlint checks.
Line [87] is parsed as a malformed table row (MD055/MD056). Move the legend outside the table block (or format it as a valid 8-column row).
🔧 Proposed fix
| Microsoft Clarity | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering |
-✓ = anonymized, - = passed through
-
| Hotjar | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering |
+
+`✓` = anonymized, `-` = passed through📝 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
| | Script | ip | userAgent | language | screen | timezone | hardware | Rationale | | |
| |--------|:--:|:---------:|:--------:|:------:|:--------:|:--------:|-----------| | |
| | Google Analytics | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for device, time, and OS reports | | |
| | Google Tag Manager | - | - | - | - | - | - | Container script loading — no user data in requests | | |
| | Meta Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | TikTok Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | X/Twitter Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Snapchat Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Reddit Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Segment | - | - | - | - | - | - | Trusted data pipeline — full fidelity required | | |
| | PostHog | - | - | - | - | - | - | Trusted, open-source — full fidelity required | | |
| | Microsoft Clarity | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | |
| ✓ = anonymized, - = passed through | |
| | Hotjar | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | |
| | Script | ip | userAgent | language | screen | timezone | hardware | Rationale | | |
| |--------|:--:|:---------:|:--------:|:------:|:--------:|:--------:|-----------| | |
| | Google Analytics | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for device, time, and OS reports | | |
| | Google Tag Manager | - | - | - | - | - | - | Container script loading — no user data in requests | | |
| | Meta Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | TikTok Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | X/Twitter Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Snapchat Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Reddit Pixel | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | Untrusted ad network — full anonymization | | |
| | Segment | - | - | - | - | - | - | Trusted data pipeline — full fidelity required | | |
| | PostHog | - | - | - | - | - | - | Trusted, open-source — full fidelity required | | |
| | Microsoft Clarity | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | |
| | Hotjar | ✓ | - | ✓ | - | - | ✓ | UA/screen/timezone needed for heatmaps and device filtering | | |
| `✓` = anonymized, `-` = passed through |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 87-87: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe
(MD055, table-pipe-style)
[warning] 87-87: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe
(MD055, table-pipe-style)
[warning] 87-87: Table column count
Expected: 8; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/docs/1.guides/2.first-party.md` around lines 75 - 90, The table
has a stray legend row ("✓ = anonymized, - = passed through") inside the
Markdown table which breaks parsing (MD055/MD056); remove that line from the
table block and place the legend as plain text immediately before or after the
table, or alternatively convert it into a valid 8-column row matching the table
schema; update the content around the table in
docs/content/docs/1.guides/2.first-party.md so the table contains only rows with
8 cells and the legend is outside the pipe-delimited table.
Comment on lines
+401
to
+417
| const isLanguageParam = NORMALIZE_PARAMS.language.some(pm => lowerKey === pm.toLowerCase()) | ||
| if (isLanguageParam) { | ||
| if (Array.isArray(value)) { | ||
| result[key] = p.language ? value.map(v => typeof v === 'string' ? normalizeLanguage(v) : v) : value | ||
| } | ||
| else if (typeof value === 'string') { | ||
| result[key] = p.language ? normalizeLanguage(value) : value | ||
| } | ||
| else { | ||
| result[key] = value | ||
| } | ||
| continue | ||
| } | ||
|
|
||
| // User-agent params — controlled by userAgent flag | ||
| const isUserAgentParam = NORMALIZE_PARAMS.userAgent.some(pm => lowerKey === pm.toLowerCase()) | ||
| if (isUserAgentParam && typeof value === 'string') { |
There was a problem hiding this comment.
Bracket-style keys can bypass language/User-Agent normalization.
At Line [401] and Line [416], key matching is exact-only, so payload keys like languages[0] / user_agent[0] skip anonymization even when flags are enabled. Reuse matchesParam() here (same as other privacy params) to avoid bypasses.
🔧 Proposed fix
- const isLanguageParam = NORMALIZE_PARAMS.language.some(pm => lowerKey === pm.toLowerCase())
+ const isLanguageParam = matchesParam(key, NORMALIZE_PARAMS.language)
if (isLanguageParam) {
if (Array.isArray(value)) {
result[key] = p.language ? value.map(v => typeof v === 'string' ? normalizeLanguage(v) : v) : value
}
else if (typeof value === 'string') {
result[key] = p.language ? normalizeLanguage(value) : value
}
else {
result[key] = value
}
continue
}
- const isUserAgentParam = NORMALIZE_PARAMS.userAgent.some(pm => lowerKey === pm.toLowerCase())
- if (isUserAgentParam && typeof value === 'string') {
- result[key] = p.userAgent ? normalizeUserAgent(value) : value
- continue
- }
+ const isUserAgentParam = matchesParam(key, NORMALIZE_PARAMS.userAgent)
+ if (isUserAgentParam) {
+ if (Array.isArray(value)) {
+ result[key] = p.userAgent ? value.map(v => typeof v === 'string' ? normalizeUserAgent(v) : v) : value
+ }
+ else if (typeof value === 'string') {
+ result[key] = p.userAgent ? normalizeUserAgent(value) : value
+ }
+ else {
+ result[key] = value
+ }
+ continue
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/server/utils/privacy.ts` around lines 401 - 417, The exact-key
checks for language and user-agent normalization (isLanguageParam using
NORMALIZE_PARAMS.language and isUserAgentParam using NORMALIZE_PARAMS.userAgent)
miss bracket-style keys like "languages[0]" or "user_agent[0]"; replace those
comparisons with the existing matchesParam() utility (same approach used for
other privacy params) so keys are matched permissively, then apply
p.language/p.userAgent gating and normalize via normalizeLanguage (and the
existing string/array handling) to ensure bracketed keys are normalized as
intended.
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
1 participant
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.
🔗 Linked issue
N/A — new feature, no existing issue
❓ Type of change
📚 Description
The first-party proxy had a binary privacy model (
'proxy' | 'anonymize') that was too coarse — some scripts need real IPs for GeoIP but shouldn't get raw User-Agent strings, while trusted tools like PostHog need full-fidelity data. This replaces it with a per-flag object model (ip,userAgent,language,screen,timezone,hardware) where each script in the registry declares exactly what it needs, and users can override globally viafirstParty.privacy.Scripts are tiered: untrusted ad networks (Meta, TikTok, X, Snapchat, Reddit) get full anonymization, analytics tools (GA, Clarity, Hotjar) keep UA/screen/timezone for reports while anonymizing the rest, and trusted tools (PostHog, Segment, GTM) pass through. Missing per-script configs fail-closed to full anonymization. IP header handling preserves X-Forwarded-For chains and normalizes header casing to prevent duplicates. High-entropy client hints are stripped when the hardware flag is active.