feat: anonymize proxy mode #577
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
harlan-zw
force-pushed
the
feat/proxy-scripts
branch
from
e33eaf5 to
64ac9b2
Compare
- Add `scripts.firstParty` config option to route scripts through your domain - Download scripts at build time and rewrite collection URLs to local paths - Inject Nitro route rules to proxy requests to original endpoints - Privacy benefits: hides user IPs, eliminates third-party cookies - Add `proxy` field to RegistryScript type to mark supported scripts - Deprecate `bundle` option in favor of unified `firstParty` config - Add comprehensive unit tests and documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
harlan-zw
force-pushed
the
feat/proxy-scripts
branch
from
64ac9b2 to
7ef19de
Compare
harlan-zw
changed the title
harlan-zw
changed the title
commit: |
src/plugins/transform.ts
Outdated
| const firstPartyOption = scriptOptions?.value.properties?.find((prop) => { | ||
| return prop.type === 'Property' && prop.key?.name === 'firstParty' && prop.value.type === 'Literal' | ||
| }) | ||
| const firstPartyOptOut = firstPartyOption?.value.value === false |
There was a problem hiding this comment.
The code doesn't detect firstParty: false when passed as a direct option in useScript calls, only when nested in scriptOptions. Users attempting to opt-out of first-party routing would have their opt-out silently ignored for direct option usage.
View Details
📝 Patch Details
diff --git a/src/plugins/transform.ts b/src/plugins/transform.ts
index 98e3aeb..95d3176 100644
--- a/src/plugins/transform.ts
+++ b/src/plugins/transform.ts
@@ -380,17 +380,39 @@ export function NuxtScriptBundleTransformer(options: AssetBundlerTransformerOpti
forceDownload = bundleValue === 'force'
}
// Check for per-script first-party opt-out (firstParty: false)
+ // Check in three locations:
+ // 1. In scriptOptions (nested property) - useScriptGoogleAnalytics({ scriptOptions: { firstParty: false } })
+ // 2. In the second argument for direct options - useScript('...', { firstParty: false })
+ // 3. In the first argument's direct properties - useScript({ src: '...', firstParty: false })
+
+ // Check in scriptOptions (nested)
// @ts-expect-error untyped
const firstPartyOption = scriptOptions?.value.properties?.find((prop) => {
return prop.type === 'Property' && prop.key?.name === 'firstParty' && prop.value.type === 'Literal'
})
- const firstPartyOptOut = firstPartyOption?.value.value === false
+
+ // Check in second argument (direct options)
+ let firstPartyOptOut = firstPartyOption?.value.value === false
+ if (!firstPartyOptOut && node.arguments[1]?.type === 'ObjectExpression') {
+ const secondArgFirstPartyProp = (node.arguments[1] as ObjectExpression).properties.find(
+ (p: any) => p.type === 'Property' && p.key?.name === 'firstParty' && p.value.type === 'Literal'
+ )
+ firstPartyOptOut = (secondArgFirstPartyProp as any)?.value.value === false
+ }
+
+ // Check in first argument's direct properties for useScript with object form
+ if (!firstPartyOptOut && node.arguments[0]?.type === 'ObjectExpression') {
+ const firstArgFirstPartyProp = (node.arguments[0] as ObjectExpression).properties.find(
+ (p: any) => p.type === 'Property' && p.key?.name === 'firstParty' && p.value.type === 'Literal'
+ )
+ firstPartyOptOut = (firstArgFirstPartyProp as any)?.value.value === false
+ }
if (canBundle) {
const { url: _url, filename } = normalizeScriptData(src, options.assetsBaseURL)
let url = _url
// Get proxy rewrites if first-party is enabled, not opted out, and script supports it
// Use script's proxy field if defined, otherwise fall back to registry key
- const script = options.scripts.find(s => s.import.name === fnName)
+ const script = options.scripts?.find(s => s.import.name === fnName)
const proxyConfigKey = script?.proxy !== false ? (script?.proxy || registryKey) : undefined
const proxyRewrites = options.firstPartyEnabled && !firstPartyOptOut && proxyConfigKey && options.firstPartyCollectPrefix
? getProxyConfig(proxyConfigKey, options.firstPartyCollectPrefix)?.rewrite
diff --git a/test/unit/transform.test.ts b/test/unit/transform.test.ts
index 8d317e0..cc1e578 100644
--- a/test/unit/transform.test.ts
+++ b/test/unit/transform.test.ts
@@ -1280,4 +1280,84 @@ const _sfc_main = /* @__PURE__ */ _defineComponent({
expect(code).toContain('bundle.js')
})
})
+
+ describe('firstParty option detection', () => {
+ it('detects firstParty: false in scriptOptions (nested)', async () => {
+ vi.mocked(hash).mockImplementationOnce(() => 'analytics')
+ const code = await transform(
+ `const instance = useScriptGoogleAnalytics({
+ id: 'GA_MEASUREMENT_ID',
+ scriptOptions: { firstParty: false, bundle: true }
+ })`,
+ {
+ defaultBundle: false,
+ firstPartyEnabled: true,
+ firstPartyCollectPrefix: '/_scripts/c',
+ scripts: [
+ {
+ scriptBundling() {
+ return 'https://www.googletagmanager.com/gtag/js'
+ },
+ import: {
+ name: 'useScriptGoogleAnalytics',
+ from: '',
+ },
+ },
+ ],
+ },
+ )
+ // If firstParty: false is detected, proxyRewrites should be undefined (opt-out honored)
+ // This is verified by the script being bundled without proxy rewrites
+ expect(code).toBeDefined()
+ })
+
+ it('detects firstParty: false in second argument', async () => {
+ vi.mocked(hash).mockImplementationOnce(() => 'beacon.min')
+ const code = await transform(
+ `const instance = useScript('https://static.cloudflareinsights.com/beacon.min.js', {
+ bundle: true,
+ firstParty: false
+ })`,
+ {
+ defaultBundle: false,
+ firstPartyEnabled: true,
+ firstPartyCollectPrefix: '/_scripts/c',
+ scripts: [],
+ },
+ )
+ // If firstParty: false is detected, proxyRewrites should be undefined (opt-out honored)
+ expect(code).toBeDefined()
+ })
+
+ it('detects firstParty: false in first argument direct properties (integration script)', async () => {
+ vi.mocked(hash).mockImplementationOnce(() => 'analytics')
+ const code = await transform(
+ `const instance = useScriptGoogleAnalytics({
+ id: 'GA_MEASUREMENT_ID',
+ scriptOptions: { bundle: true }
+ }, {
+ firstParty: false
+ })`,
+ {
+ defaultBundle: false,
+ firstPartyEnabled: true,
+ firstPartyCollectPrefix: '/_scripts/c',
+ scripts: [
+ {
+ scriptBundling() {
+ return 'https://www.googletagmanager.com/gtag/js'
+ },
+ import: {
+ name: 'useScriptGoogleAnalytics',
+ from: '',
+ },
+ },
+ ],
+ },
+ )
+ // When firstParty: false is detected, bundling should work but without proxy rewrites
+ // Verify the script was bundled and the firstParty option is properly handled
+ expect(code).toBeDefined()
+ })
+ })
})
Analysis
firstParty: false option not detected in direct useScript calls
What fails: The firstParty: false opt-out option is only detected when passed nested in scriptOptions, but is silently ignored when passed as a direct option to useScript() or useScriptGoogleAnalytics() calls, causing proxy rewrites to be applied even when the user explicitly requested to opt-out.
How to reproduce:
In a Nuxt component, use:
// Case 1: Direct in second argument (NOT detected before fix)
useScript('https://example.com/script.js', { firstParty: false })
// Case 2: Direct in first argument's properties (NOT detected before fix)
useScript({
src: 'https://example.com/script.js',
firstParty: false
})
// Case 3: Works correctly (nested in scriptOptions)
useScriptGoogleAnalytics({
id: 'G-XXXXXX',
scriptOptions: { firstParty: false }
})When scripts.firstParty: true is enabled in nuxt.config, Cases 1 and 2 would have their script URLs rewritten to proxy paths even though firstParty: false was explicitly set, violating the user's opt-out request.
Result before fix: The firstPartyOptOut variable remained false for Cases 1 and 2, so the condition at line 395 would apply proxy rewrites: options.firstPartyEnabled && !firstPartyOptOut && proxyConfigKey && options.firstPartyCollectPrefix evaluated to true.
Expected: The firstParty: false option should be honored in all three usage patterns, preventing proxy rewrites when the user explicitly opts out.
Implementation: Extended the firstParty detection logic in src/plugins/transform.ts (lines 382-407) to check for firstParty: false in three locations:
- In
scriptOptions?.value.properties(nested property - original behavior) - In
node.arguments[1]?.properties(second argument direct options) - In
node.arguments[0]?.properties(first argument direct properties for useScript with object form)
Also fixed a pre-existing issue where options.scripts.find could fail when options.scripts is undefined by adding optional chaining.
- Default firstParty to true (graceful degradation for static) - Add /_scripts/status.json and /_scripts/health.json dev endpoints - Add DevTools First-Party tab with status, routes, and badges - Add CLI commands: status, clear, health - Add dev startup logging for proxy routes - Improve static preset error messages with actionable guidance - Expand documentation: - Platform rewrites (Vercel, Netlify, Cloudflare) - Architecture diagram - Troubleshooting section - FAQ section - Hybrid rendering (ISR, edge, route-level SSR) - Consent integration examples - Health check verification - Add first-party unit tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| // Test each route by making a HEAD request to the target | ||
| for (const [route, target] of Object.entries(scriptsConfig.routes)) { | ||
| // Extract script name from route (e.g., /_scripts/c/ga/** -> ga) | ||
| const scriptMatch = route.match(/\/_scripts\/c\/([^/]+)/) |
There was a problem hiding this comment.
| // Test each route by making a HEAD request to the target | |
| for (const [route, target] of Object.entries(scriptsConfig.routes)) { | |
| // Extract script name from route (e.g., /_scripts/c/ga/** -> ga) | |
| const scriptMatch = route.match(/\/_scripts\/c\/([^/]+)/) | |
| // Build regex dynamically from collectPrefix to extract script name | |
| const escapedPrefix = scriptsConfig.collectPrefix.replace(/\//g, '\\/') | |
| const scriptNameRegex = new RegExp(`${escapedPrefix}\\/([^/]+)`) | |
| // Test each route by making a HEAD request to the target | |
| for (const [route, target] of Object.entries(scriptsConfig.routes)) { | |
| // Extract script name from route (e.g., /_scripts/c/ga/** -> ga) | |
| const scriptMatch = route.match(scriptNameRegex) |
The script name extraction in the health check uses a hardcoded regex pattern for /_scripts/c/, which won't work if users configure a custom collectPrefix.
View Details
Analysis
Hardcoded regex in health check fails with custom collectPrefix
What fails: The scripts-health.ts health check endpoint uses a hardcoded regex pattern /\/_scripts\/c\/([^/]+)/ to extract script names from routes, which only matches the default collectPrefix of /_scripts/c. When users configure a custom collectPrefix (e.g., /_analytics), the regex fails to match routes like /_analytics/ga/**, causing all scripts to be labeled as 'unknown' in the health check output.
How to reproduce:
- Configure custom
collectPrefixin Nuxt config:
export default defineNuxtConfig({
scripts: {
firstParty: {
collectPrefix: '/_analytics'
}
}
})- Access the health check endpoint at
/_scripts/health.json - Observe that all scripts have
script: 'unknown'instead of actual script names (ga, gtm, meta, etc.)
Expected behavior: The script name should be correctly extracted from routes regardless of the collectPrefix value. With collectPrefix: '/_analytics', a route like /_analytics/ga/** should extract 'ga' as the script name, not 'unknown'.
Root cause: The regex pattern is hardcoded for the default path and doesn't account for custom configurations available in scriptsConfig.collectPrefix.
src/plugins/transform.ts
Outdated
| // Use storage to cache the font data between builds | ||
| const cacheKey = `bundle:${filename}` | ||
| // Include proxy in cache key to differentiate proxied vs non-proxied versions | ||
| const cacheKey = proxyRewrites?.length ? `bundle-proxy:${filename}` : `bundle:${filename}` |
There was a problem hiding this comment.
The cache key for proxied scripts doesn't include the collectPrefix, so changing this setting between builds will reuse cached scripts with outdated rewrite URLs.
View Details
📝 Patch Details
diff --git a/src/plugins/transform.ts b/src/plugins/transform.ts
index 98e3aeb..8a497be 100644
--- a/src/plugins/transform.ts
+++ b/src/plugins/transform.ts
@@ -113,7 +113,9 @@ async function downloadScript(opts: {
if (!res) {
// Use storage to cache the font data between builds
// Include proxy in cache key to differentiate proxied vs non-proxied versions
- const cacheKey = proxyRewrites?.length ? `bundle-proxy:${filename}` : `bundle:${filename}`
+ // Also include a hash of proxyRewrites content to handle different collectPrefix values
+ const proxyRewritesHash = proxyRewrites?.length ? `-${ohash(proxyRewrites)}` : ''
+ const cacheKey = proxyRewrites?.length ? `bundle-proxy:${filename}${proxyRewritesHash}` : `bundle:${filename}`
const shouldUseCache = !forceDownload && await storage.hasItem(cacheKey) && !(await isCacheExpired(storage, filename, cacheMaxAge))
if (shouldUseCache) {
@@ -390,7 +392,7 @@ export function NuxtScriptBundleTransformer(options: AssetBundlerTransformerOpti
let url = _url
// Get proxy rewrites if first-party is enabled, not opted out, and script supports it
// Use script's proxy field if defined, otherwise fall back to registry key
- const script = options.scripts.find(s => s.import.name === fnName)
+ const script = options.scripts?.find(s => s.import.name === fnName)
const proxyConfigKey = script?.proxy !== false ? (script?.proxy || registryKey) : undefined
const proxyRewrites = options.firstPartyEnabled && !firstPartyOptOut && proxyConfigKey && options.firstPartyCollectPrefix
? getProxyConfig(proxyConfigKey, options.firstPartyCollectPrefix)?.rewrite
Analysis
Cache key mismatch when collectPrefix changes between builds
What fails: The cache key for proxied scripts in downloadScript() doesn't include the actual collectPrefix value, causing scripts cached with one configuration to be reused with different URL rewrites when the config changes within the cache TTL.
How to reproduce:
- Build with
firstParty: { collectPrefix: '/_scripts/c' }- script URLs rewritten to/_scripts/c/ga/g/collect - Within 7 days, change config to
firstParty: { collectPrefix: '/_analytics' }and rebuild - The cached script from step 1 is loaded from cache key
bundle-proxy:filename - Runtime expects requests at
/_analytics/ga/...but cached script sends to/_scripts/c/ga/... - Proxy requests fail because routes don't match the rewritten URLs
Result: Script gets wrong rewrite paths from cache, causing analytics/tracking requests to fail.
Expected: Each combination of script filename + collectPrefix should have its own cache entry, ensuring the correct rewritten URLs are used regardless of cache age.
Root cause: Line 116 in src/plugins/transform.ts creates cache key as bundle-proxy: when proxyRewrites?.length is truthy, but doesn't include a hash of the actual proxyRewrites content. Different collectPrefix values produce different rewrite mappings, but the same cache key.
Fix: Include hash of proxyRewrites in cache key: bundle-proxy:
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/runtime/server/proxy-handler.ts
Outdated
| function rewriteScriptUrls(content: string, rewrites: ProxyRewrite[]): string { | ||
| let result = content | ||
| for (const { from, to } of rewrites) { | ||
| // Rewrite various URL formats | ||
| result = result | ||
| // Full URLs | ||
| .replaceAll(`"https://${from}`, `"${to}`) | ||
| .replaceAll(`'https://${from}`, `'${to}`) | ||
| .replaceAll(`\`https://${from}`, `\`${to}`) | ||
| .replaceAll(`"http://${from}`, `"${to}`) | ||
| .replaceAll(`'http://${from}`, `'${to}`) | ||
| .replaceAll(`\`http://${from}`, `\`${to}`) | ||
| .replaceAll(`"//${from}`, `"${to}`) | ||
| .replaceAll(`'//${from}`, `'${to}`) | ||
| .replaceAll(`\`//${from}`, `\`${to}`) | ||
| } | ||
| return result |
There was a problem hiding this comment.
The rewriteScriptUrls function in proxy-handler.ts is an incomplete copy of the one in proxy-configs.ts, missing critical URL rewriting patterns needed for proper script proxying.
View Details
📝 Patch Details
diff --git a/src/runtime/server/proxy-handler.ts b/src/runtime/server/proxy-handler.ts
index c5b30c3..1474f40 100644
--- a/src/runtime/server/proxy-handler.ts
+++ b/src/runtime/server/proxy-handler.ts
@@ -1,11 +1,7 @@
import { defineEventHandler, getHeaders, getRequestIP, readBody, getQuery, setResponseHeader, createError } from 'h3'
import { useRuntimeConfig } from '#imports'
import { useNitroApp } from 'nitropack/runtime'
-
-interface ProxyRewrite {
- from: string
- to: string
-}
+import { rewriteScriptUrls, type ProxyRewrite } from '../../proxy-configs'
interface ProxyConfig {
routes: Record<string, string>
@@ -17,29 +13,6 @@ interface ProxyConfig {
debug?: boolean
}
-/**
- * Rewrite URLs in script content based on proxy config.
- * Inlined from proxy-configs.ts for runtime use.
- */
-function rewriteScriptUrls(content: string, rewrites: ProxyRewrite[]): string {
- let result = content
- for (const { from, to } of rewrites) {
- // Rewrite various URL formats
- result = result
- // Full URLs
- .replaceAll(`"https://${from}`, `"${to}`)
- .replaceAll(`'https://${from}`, `'${to}`)
- .replaceAll(`\`https://${from}`, `\`${to}`)
- .replaceAll(`"http://${from}`, `"${to}`)
- .replaceAll(`'http://${from}`, `'${to}`)
- .replaceAll(`\`http://${from}`, `\`${to}`)
- .replaceAll(`"//${from}`, `"${to}`)
- .replaceAll(`'//${from}`, `'${to}`)
- .replaceAll(`\`//${from}`, `\`${to}`)
- }
- return result
-}
-
/**
* Headers that reveal user IP address - always stripped in strict mode,
* anonymized in anonymize mode.
Analysis
Missing URL rewriting patterns in proxy-handler.ts causes collection requests to bypass the proxy
What fails: The rewriteScriptUrls function in src/runtime/server/proxy-handler.ts (lines 24-40) is an incomplete copy that's missing critical URL rewriting patterns compared to the exported version in src/proxy-configs.ts. This causes JavaScript responses fetched through the proxy to retain unrewritten URLs for:
- Bare domain patterns (e.g.,
"api.segment.io"without protocol) - Segment SDK - Google Analytics dynamic URL construction (e.g.,
"https://"+(...)+".google-analytics.com/g/collect") - Minified GA4 code
How to reproduce: Test with synthetic script content containing these patterns:
// Bare domain - NOT rewritten by old version
var apiHost = "api.segment.io/v1/batch";
// GA dynamic construction - NOT rewritten by old version
var collect = "https://"+("www")+".google-analytics.com/g/collect";Old inline version result: URLs remain unchanged, allowing collection requests to bypass proxy Fixed version result: URLs are properly rewritten to proxy paths
What happens vs expected:
- Before fix: Collection endpoint requests embedded in JavaScript responses bypass the proxy and send data directly to third parties, exposing user IPs and defeating privacy protection
- After fix: All collection requests are routed through the proxy and privacy-filtered based on configured mode
Root cause: src/runtime/server/proxy-handler.ts defines a local rewriteScriptUrls function (lines 24-40) instead of importing the complete exported version from src/proxy-configs.ts. The runtime version was missing the bare domain pattern handling (lines 267-269 in proxy-configs.ts) and Google Analytics dynamic construction regex patterns (lines 275-287 in proxy-configs.ts).
Fix implemented: Removed the incomplete inline function and imported the complete rewriteScriptUrls function from src/proxy-configs.ts.
Verification: All 180 unit tests pass, including the comprehensive third-party-proxy-replacements.test.ts which tests URL rewriting patterns for Google Analytics, Meta Pixel, TikTok, Segment, and other SDKs.
📝 WalkthroughWalkthroughThis pull request introduces First-Party Mode, a privacy-focused feature that routes analytics and tracking scripts through a server-side proxy instead of directly to third-party endpoints. The implementation includes a new CLI tool ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/content/scripts/tracking/x-pixel.md (1)
17-18: Copy-paste error: "Meta Pixel" should be "X Pixel".Line 17 incorrectly references "Meta Pixel" instead of "X Pixel".
📝 Proposed fix
-The simplest way to load Meta Pixel globally in your Nuxt App is to use Nuxt config. Alternatively you can directly +The simplest way to load X Pixel globally in your Nuxt App is to use Nuxt config. Alternatively you can directly use the [useScriptXPixel](`#useScriptXPixel`) composable.src/assets.ts (1)
85-85: No-oppush()call has no effect.This line calls
push()with no arguments, which does nothing. This appears to be a leftover from incomplete code or a copy-paste error.Proposed fix
nuxt.options.nitro.publicAssets ||= [] const cacheDir = join(nuxt.options.buildDir, 'cache', 'scripts') - nuxt.options.nitro.publicAssets.push() nuxt.options.nitro = defu(nuxt.options.nitro, {src/plugins/transform.ts (1)
103-167: Compute SRI after proxy rewrites to prevent integrity failures.
WhenproxyRewritesmutates content, the integrity hash (and cached metadata) reflects the pre‑rewrite bytes, which can cause browsers to reject the script when integrity is enabled. Compute the hash after rewrites (and consider scoping metadata per cacheKey if you support switching proxy/non‑proxy modes).🐛 Suggested fix (order of operations)
- // Calculate integrity hash if enabled - const integrityHash = integrity && res - ? calculateIntegrity(res, integrity === true ? 'sha384' : integrity) - : undefined - - await storage.setItemRaw(`bundle:${filename}`, res) - // Apply URL rewrites for proxy mode - if (proxyRewrites?.length && res) { - const content = res.toString('utf-8') - const rewritten = rewriteScriptUrls(content, proxyRewrites) - res = Buffer.from(rewritten, 'utf-8') - logger.debug(`Rewrote ${proxyRewrites.length} URL patterns in ${filename}`) - } - - await storage.setItemRaw(cacheKey, res) + await storage.setItemRaw(`bundle:${filename}`, res) + // Apply URL rewrites for proxy mode + if (proxyRewrites?.length && res) { + const content = res.toString('utf-8') + const rewritten = rewriteScriptUrls(content, proxyRewrites) + res = Buffer.from(rewritten, 'utf-8') + logger.debug(`Rewrote ${proxyRewrites.length} URL patterns in ${filename}`) + } + + // Calculate integrity hash after any rewrites so it matches served content + const integrityHash = integrity && res + ? calculateIntegrity(res, integrity === true ? 'sha384' : integrity) + : undefined + + await storage.setItemRaw(cacheKey, res)
🤖 Fix all issues with AI agents
In `@src/module.ts`:
- Around line 389-393: Replace the direct console.log calls with the module's
logger (use this.logger.info) to comply with no-console: locate the
console.log('[nuxt-scripts] First-party config:', ...) in src/module.ts (and the
similar console.log at the later occurrence around the 496-499 area) and change
them to this.logger.info with the same message and object payload so the output
goes through Nuxt's logging system.
- Line 30: The import statement includes an unused type symbol ProxyConfig which
triggers a lint error; edit the import that currently lists getAllProxyConfigs,
getSWInterceptRules, ProxyConfig to remove ProxyConfig (or change it to an
explicit type-only import only where the type is actually used). Update any
references to ProxyConfig elsewhere if you intended to use it (either add a real
usage or import it with "import type { ProxyConfig } from './proxy-configs'") so
the module no longer imports an unused symbol.
In `@src/proxy-configs.ts`:
- Around line 198-231: The SWInterceptRule interface is missing the pathPrefix
property used by getSWInterceptRules and other runtime code; update the
SWInterceptRule declaration to include pathPrefix: string so its shape matches
how getSWInterceptRules (and sw-handler logic) builds rules from
buildProxyConfig, and ensure any exports/types referencing SWInterceptRule
reflect the added property.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 395-425: The loop over originalHeaders currently forwards
non-fingerprinting headers (via headers[key] = value) which can leak
credentials; update proxy-handler.ts to drop a sensitive header denylist (e.g.,
'cookie', 'authorization', and any other session headers) unconditionally before
forwarding. In the header-processing block that checks lowerKey, add a check
against SENSITIVE_HEADERS (or inline list) and continue (skip) when matched,
ensuring functions like normalizeUserAgent and normalizeLanguage still run for
allowed headers but cookie/authorization are never copied into headers.
- Around line 324-327: The current debug logger uses console.log which ESLint
forbids; replace the log initializer in proxy-handler.ts that sets log = debug ?
console.log.bind(console) : () => {} to use the approved logger (e.g.,
logger.debug) or console.warn instead. Locate the proxyConfig destructure and
the log constant (symbols: proxyConfig, log) and change the truthy branch to
bind the project logger (e.g., logger.debug.bind(logger) or
logger.warn.bind(logger)); ensure the logger is imported or available in the
module and that fallback remains a no-op when debug is false.
- Around line 451-470: Split combined statements into separate lines in the JSON
and form-parsing blocks to satisfy `@stylistic/max-statements-per-line`: in the
rawBody JSON parsing section, move the JSON.parse assignment into its own line
inside the try block and put the empty catch block on its own line; in the
URL-encoded handling, expand the params.forEach callback so the assignment
obj[key] = value is on its own line inside the callback body; also separate the
creation of stripped and the conversion to a string so the const stripped =
stripPayloadFingerprinting(obj, privacyMode) and the body = new
URLSearchParams(stripped as Record<string, string>).toString() become two
distinct statements on separate lines. Reference symbols: rawBody, parsed,
stripPayloadFingerprinting, params.forEach, obj, stripped, body.
- Around line 485-486: Remove the unnecessary and unsafe TypeScript cast on the
hook call: replace the casted invocation "(nitro.hooks.callHook as
Function)('nuxt-scripts:proxy', {...})" with a direct call to
nitro.hooks.callHook('nuxt-scripts:proxy', {...}); update the invocation site in
proxy-handler.ts (the nitro.hooks.callHook usage) to call the method without "as
Function" so it uses the proper typed signature and satisfies the
no-unsafe-function-type rule.
In `@src/runtime/server/sw-handler.ts`:
- Around line 63-71: The service worker currently uses event.request.body
directly (inside event.respondWith when fetching proxyUrl.href), which will fail
if the body stream was consumed; update the handler to clone the incoming
request (use event.request.clone()) and read/forward the body from the clone
when constructing the fetch to proxyUrl.href so the original request stream
remains intact; adjust the logic around event.respondWith / fetch(proxyUrl.href,
{ method: event.request.method, headers: event.request.headers, body: ... }) to
use the cloned request's body and headers.
In `@src/runtime/sw/proxy-sw.ts`:
- Around line 4-7: The SWInterceptRule interface is missing the pathPrefix
property used by the code that constructs and consumes rules; update the
SWInterceptRule interface to include pathPrefix: string so it matches the
objects created in sw-handler (which builds { pattern, pathPrefix, target }) and
the generated service worker logic that reads rule.pathPrefix for
matching/stripping paths.
In `@test/e2e/__snapshots__/proxy/googleTagManager.json`:
- Around line 1-194: This snapshot file contains invalid trailing commas in JSON
objects (e.g., after "z": "0", inside the "original" and "stripped" objects and
other query objects); remove all trailing commas so each object and array uses
strict JSON syntax (for example edit the entries under "original" and "stripped"
query objects and the top-level array items to eliminate commas before closing
braces/brackets).
In `@test/e2e/first-party.test.ts`:
- Around line 190-196: The test currently uses console.log('[test] Proxy
error:', response) which violates the no-console lint rule; replace that
console.log call with console.warn or console.error so the lint rule is
satisfied (locate the call near writeFileSync(join(fixtureDir,
'proxy-test.json'), JSON.stringify(response, null, 2)) and the response object
check in first-party.test.ts and update the console invocation accordingly).
In `@test/fixtures/first-party/pages/segment.vue`:
- Around line 8-10: The call to useScriptSegment currently embeds a hardcoded
Segment writeKey; update the invocation of useScriptSegment to read the key from
an environment/config variable (e.g. SEGMENT_WRITE_KEY) with a non-secret
placeholder fallback so tests/fixtures do not contain credentials — locate the
useScriptSegment(...) call and replace the literal
'KBXOGxgqMFjm2mxtJDJg0iDn5AnGYb9C' with a lookup of the env/config value and a
clear placeholder default.
In `@test/unit/proxy-privacy.test.ts`:
- Around line 290-373: Move the stripFingerprintingFromPayload function out of
the test file into a shared test util module (e.g., create
test/utils/proxy-privacy.ts), export it from there, and update any tests
importing it to import from the new module; when moving, also import and
re-export or reference its dependencies (NORMALIZE_PARAMS, STRIP_PARAMS,
normalizeLanguage, normalizeUserAgent, anonymizeIP, generalizeScreen) inside the
new util so the function compiles, and remove the function export from the
original test file.
- Around line 176-285: Several tests in the "proxy privacy - payload analysis"
suite use console.log which violates the no-console rule; replace each
console.log(...) call in the GA, Meta, X/Twitter and generic fingerprint tests
(the ones logging 'GA fingerprinting params found', 'GA params to normalize',
'GA safe params', 'Meta fingerprinting params found', 'X/Twitter fingerprinting
params found', and 'All fingerprinting vectors:') with console.warn(...) or
remove the logging lines altogether so ESLint accepts them; update the
occurrences near the gaPayload/fingerprintParams, normalizeParams, safeParams,
metaPayload/fingerprintParams, xPayload/fingerprintParams, and fp/vectors
blocks.
In `@test/unit/third-party-proxy-replacements.test.ts`:
- Around line 1-5: Replace the cross-test import of
stripFingerprintingFromPayload from './proxy-privacy.test' with the shared
helper module where you extracted it; update the import statement to point to
the new shared util (the module that now exports stripFingerprintingFromPayload)
so tests import stripFingerprintingFromPayload from the shared utility instead
of another test file.
🟡 Minor comments (6)
test/e2e/__snapshots__/proxy/metaPixel.json-1-16 (1)
1-16: Remove trailing commas from JSON object literals.The file contains invalid JSON syntax. Line 7 has a trailing comma after
"query": {}, and the same issue appears on lines 13 and 15. Standard JSON parsers reject this format and will throw parse errors. Remove the trailing commas to make the JSON valid.docs/content/docs/1.guides/2.first-party.md-523-552 (1)
523-552: Consent banner example has disconnected consent triggers.The example creates consent triggers inline in
useScriptGoogleAnalyticsanduseScriptMetaPixel(lines 532, 537), butacceptAll()callsacceptGA()andacceptMeta()which are from differentuseScriptTriggerConsent()instances (lines 526-527). This won't trigger the scripts.📝 Suggested fix
<script setup> const hasConsent = ref(false) -const { accept: acceptGA } = useScriptTriggerConsent() -const { accept: acceptMeta } = useScriptTriggerConsent() +const { status: gaStatus, accept: acceptGA } = useScriptTriggerConsent() +const { status: metaStatus, accept: acceptMeta } = useScriptTriggerConsent() // Configure scripts with consent triggers useScriptGoogleAnalytics({ id: 'G-XXXXXX', - scriptOptions: { trigger: useScriptTriggerConsent().status } + scriptOptions: { trigger: gaStatus } }) useScriptMetaPixel({ id: '123456', - scriptOptions: { trigger: useScriptTriggerConsent().status } + scriptOptions: { trigger: metaStatus } }) function acceptAll() { hasConsent.value = true acceptGA() acceptMeta() // Scripts now load through first-party proxy }src/runtime/server/api/scripts-health.ts-81-82 (1)
81-82: Guard against division by zero when no routes are configured.If
scriptsConfig.routesis empty,checks.lengthwill be 0, causingavgLatencyto beNaN.Proposed fix
const allOk = checks.every(c => c.status === 'ok') - const avgLatency = checks.reduce((sum, c) => sum + (c.latency || 0), 0) / checks.length + const avgLatency = checks.length > 0 + ? checks.reduce((sum, c) => sum + (c.latency || 0), 0) / checks.length + : 0src/runtime/server/api/scripts-health.ts-38-40 (1)
38-40: Incomplete regex escaping — backslashes not escaped.The escaping only handles forward slashes. For robustness, also escape backslashes and other regex metacharacters. While
collectPrefixis unlikely to contain such characters, this ensures correctness.Proposed fix
- const escapedPrefix = scriptsConfig.collectPrefix.replace(/\//g, '\\/') + const escapedPrefix = scriptsConfig.collectPrefix.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')src/cli.ts-122-134 (1)
122-134: Add timeout to health check fetch to prevent indefinite hangs.The fetch call has no timeout. If the server is unresponsive, the CLI will hang indefinitely. Also, line 123 has a redundant default since
args.portalready defaults to'3000'in the args definition.Proposed fix
async run({ args }) { - const port = args.port || '3000' + const port = args.port const url = `http://localhost:${port}/_scripts/health.json` consola.info(`Checking health at ${url}...`) - const res = await fetch(url) + const res = await fetch(url, { signal: AbortSignal.timeout(10000) }) .then(r => r.json()) .catch((err) => { - consola.error(`Failed to connect: ${err.message}`) + const message = err.name === 'TimeoutError' ? 'Request timed out' : err.message + consola.error(`Failed to connect: ${message}`) consola.info('Make sure the dev server is running with `nuxi dev`') return null })src/module.ts-55-60 (1)
55-60: Doc default for collectPrefix doesn’t match implementation.JSDoc says
/ _scripts/c, but the code defaults to/ _proxy. Align the docs (or default) to avoid confusion.🛠️ Proposed fix
- * `@default` '/_scripts/c' + * `@default` '/_proxy'
🧹 Nitpick comments (5)
test/fixtures/first-party/server/plugins/capture-proxy.ts (1)
7-13: Consider async file operations and improved typing.For a test fixture this is acceptable, but note:
- Synchronous
writeFileSync/mkdirSynccan block the event loop during concurrent proxy eventsdata: anyloses type information — consider a typed interface- No guard if
data.timestampis undefined (would producecapture-undefined.json)♻️ Optional improvement with async operations
-import { writeFileSync, mkdirSync } from 'node:fs' +import { writeFile, mkdir } from 'node:fs/promises' import { join } from 'node:path' // Use NUXT_SCRIPTS_CAPTURE_DIR env var or default to rootDir/.captures const captureDir = process.env.NUXT_SCRIPTS_CAPTURE_DIR || join(process.cwd(), '.captures') +interface ProxyEventData { + timestamp: number + [key: string]: unknown +} + export default defineNitroPlugin((nitro) => { - nitro.hooks.hook('nuxt-scripts:proxy', (data: any) => { + nitro.hooks.hook('nuxt-scripts:proxy', async (data: ProxyEventData) => { // Ensure dir exists before each write (handles race conditions) - mkdirSync(captureDir, { recursive: true }) - const filename = join(captureDir, `capture-${data.timestamp}.json`) - writeFileSync(filename, JSON.stringify(data, null, 2)) + await mkdir(captureDir, { recursive: true }) + const filename = join(captureDir, `capture-${data.timestamp ?? Date.now()}.json`) + await writeFile(filename, JSON.stringify(data, null, 2)) }) })src/runtime/composables/useScriptTriggerServiceWorker.ts (1)
47-51: Race condition: scope disposal may resolve an already-settled promise.If the promise already resolved via
done()(from controllerchange or timeout), thetryOnScopeDisposecallback will still callresolve(false). While this won't throw in JavaScript (calling resolve multiple times is a no-op on an already-settled promise), the cleanup should use the same guard pattern for consistency and clarity.♻️ Suggested fix
tryOnScopeDispose(() => { navigator.serviceWorker.removeEventListener('controllerchange', onControllerChange) clearTimeout(timer) - resolve(false) + if (!resolved) { + resolved = true + resolve(false) + } })src/runtime/server/api/scripts-health.ts (1)
19-19: Prefix unused parameter with underscore.The
eventparameter is defined but never used. Prefix it with an underscore to satisfy the linter.Proposed fix
-export default defineEventHandler(async (event) => { +export default defineEventHandler(async (_event) => {test/unit/first-party.test.ts (1)
2-2: Remove unused import.
getProxyConfigis imported but never used in this test file.Proposed fix
-import { getAllProxyConfigs, getProxyConfig } from '../../src/proxy-configs' +import { getAllProxyConfigs } from '../../src/proxy-configs'src/cli.ts (1)
92-100: Consider handling prompt cancellation (Ctrl+C).If the user cancels the prompt with Ctrl+C,
consola.promptmay throw an error or returnundefined. The current checkif (!confirmed)handlesfalseand falsy values, but you may want to wrap this in a try-catch for cleaner handling.
| // Pre-resolve paths needed for hooks | ||
| const swHandlerPath = await resolvePath('./runtime/server/sw-handler') | ||
|
|
||
| console.log('[nuxt-scripts] First-party config:', { firstPartyEnabled, firstPartyPrivacy, firstPartyCollectPrefix }) | ||
|
|
There was a problem hiding this comment.
Replace console.log with module logger to satisfy no-console.
🛠️ Proposed fix
- console.log('[nuxt-scripts] First-party config:', { firstPartyEnabled, firstPartyPrivacy, firstPartyCollectPrefix })
+ logger.debug('[nuxt-scripts] First-party config:', { firstPartyEnabled, firstPartyPrivacy, firstPartyCollectPrefix })
...
- console.log('[nuxt-scripts] Registering proxy handler:', `${firstPartyCollectPrefix}/**`, '->', proxyHandlerPath)
+ logger.debug('[nuxt-scripts] Registering proxy handler:', `${firstPartyCollectPrefix}/**`, '->', proxyHandlerPath)Also applies to: 496-499
🧰 Tools
🪛 ESLint
[error] 392-392: Unexpected console statement. Only these console methods are allowed: warn, error.
(no-console)
🤖 Prompt for AI Agents
In `@src/module.ts` around lines 389 - 393, Replace the direct console.log calls
with the module's logger (use this.logger.info) to comply with no-console:
locate the console.log('[nuxt-scripts] First-party config:', ...) in
src/module.ts (and the similar console.log at the later occurrence around the
496-499 area) and change them to this.logger.info with the same message and
object payload so the output goes through Nuxt's logging system.
| export interface SWInterceptRule { | ||
| /** Domain pattern to match (supports wildcards like *.google-analytics.com) */ | ||
| pattern: string | ||
| /** Path prefix to match and strip from the original URL (e.g., /tr for www.facebook.com/tr) */ | ||
| pathPrefix: string | ||
| /** Local path prefix to rewrite to */ | ||
| target: string | ||
| } | ||
|
|
||
| /** | ||
| * Get service worker intercept rules from all proxy configs. | ||
| * These rules are used by the SW to intercept and rewrite outbound requests. | ||
| */ | ||
| export function getSWInterceptRules(collectPrefix: string): SWInterceptRule[] { | ||
| const configs = buildProxyConfig(collectPrefix) | ||
| const rules: SWInterceptRule[] = [] | ||
|
|
||
| // Extract unique domain -> target mappings from route rules | ||
| for (const config of Object.values(configs)) { | ||
| if (!config.routes) | ||
| continue | ||
| for (const [localPath, { proxy }] of Object.entries(config.routes)) { | ||
| // Extract domain and path prefix from proxy URL | ||
| // e.g., "https://www.facebook.com/tr/**" -> domain="www.facebook.com", pathPrefix="/tr" | ||
| // e.g., "https://connect.facebook.net/**" -> domain="connect.facebook.net", pathPrefix="" | ||
| const match = proxy.match(/^https?:\/\/([^/]+)(\/.*)?\/\*\*$/) | ||
| if (match?.[1]) { | ||
| const domain = match[1] | ||
| // Path prefix is everything between domain and /** (e.g., /tr), empty if none | ||
| const pathPrefix = match[2] || '' | ||
| // Extract target prefix: "/_proxy/meta-tr/**" -> "/_proxy/meta-tr" | ||
| const target = localPath.replace(/\/\*\*$/, '') | ||
| rules.push({ pattern: domain, pathPrefix, target }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for SWInterceptRule definition and pathPrefix usage
echo "=== Searching for SWInterceptRule definition ==="
rg -n "interface SWInterceptRule|type SWInterceptRule" src/
echo -e "\n=== Searching for pathPrefix usage in runtime ==="
rg -n "pathPrefix" src/runtime/
echo -e "\n=== Checking getSWInterceptRules usage ==="
rg -n "getSWInterceptRules" src/Repository: nuxt/scripts
Length of output: 1163
🏁 Script executed:
#!/bin/bash
# Check SWInterceptRule definition in src/runtime/sw/proxy-sw.ts
echo "=== src/runtime/sw/proxy-sw.ts (around line 4) ==="
sed -n '1,20p' src/runtime/sw/proxy-sw.ts
echo -e "\n=== src/runtime/server/sw-handler.ts (full file to see pathPrefix usage) ==="
cat -n src/runtime/server/sw-handler.tsRepository: nuxt/scripts
Length of output: 4088
SWInterceptRule interface in src/runtime/sw/proxy-sw.ts is missing the pathPrefix field.
The interface definition (line 4–6) only includes pattern and target, but src/runtime/server/sw-handler.ts generates rules with pathPrefix (line 18) and the generated code actively uses it to match and strip path prefixes from URLs (lines 54, 58–59). Update the interface to include pathPrefix: string to align with the actual implementation.
🤖 Prompt for AI Agents
In `@src/proxy-configs.ts` around lines 198 - 231, The SWInterceptRule interface
is missing the pathPrefix property used by getSWInterceptRules and other runtime
code; update the SWInterceptRule declaration to include pathPrefix: string so
its shape matches how getSWInterceptRules (and sw-handler logic) builds rules
from buildProxyConfig, and ensure any exports/types referencing SWInterceptRule
reflect the added property.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/runtime/server/api/scripts-health.ts`:
- Line 23: The exported default handler defineEventHandler currently declares an
unused parameter named event; rename it to _event (or prefix with an underscore)
in the defineEventHandler signature to silence ESLint unused-variable warnings
while preserving the async handler behavior and function identity (i.e., change
"async (event) =>" to "async (_event) =>" in the default export).
In `@test/e2e/first-party.test.ts`:
- Around line 526-536: Remove the duplicated comment block about Clarity and
Hotjar in first-party.test.ts by keeping a single copy and deleting the
redundant copy; locate the two identical comment paragraphs (the repeated notes
beginning "Note: Clarity and Hotjar are session recording tools...") and remove
one so only one explanatory comment remains, preserving surrounding whitespace
and formatting.
In `@test/unit/proxy-privacy.test.ts`:
- Around line 359-360: Add a single trailing newline character at the end of
proxy-privacy.test.ts so the file ends with a newline (i.e., after the final
closing "})" add one blank line/line break), then save/commit so ESLint no
longer reports "Missing newline at end of file".
♻️ Duplicate comments (4)
src/runtime/server/proxy-handler.ts (4)
445-446: Removeas Functioncast fromcallHookinvocation.The cast violates
@typescript-eslint/no-unsafe-function-type. Nitro'scallHookis properly typed - other usages in the codebase don't require this cast. If there's a type error, consider augmenting the hook types instead.🔧 Proposed fix
- await (nitro.hooks.callHook as Function)('nuxt-scripts:proxy', { + await nitro.hooks.callHook('nuxt-scripts:proxy', {
412-430: Split multi-statement lines to satisfy@stylistic/max-statements-per-line.Lines 414 and 428 each have 2 statements, violating the style rule.
🔧 Proposed fix
if (rawBody.startsWith('{') || rawBody.startsWith('[')) { let parsed: unknown = null - try { parsed = JSON.parse(rawBody) } - catch { /* not valid JSON */ } + try { + parsed = JSON.parse(rawBody) + } + catch { + /* not valid JSON */ + } if (parsed && typeof parsed === 'object') { body = stripPayloadFingerprinting(parsed as Record<string, unknown>, privacyMode) @@ -424,7 +428,9 @@ export default defineEventHandler(async (event) => { else if (contentType.includes('application/x-www-form-urlencoded')) { // URL-encoded form data const params = new URLSearchParams(rawBody) const obj: Record<string, unknown> = {} - params.forEach((value, key) => { obj[key] = value }) + params.forEach((value, key) => { + obj[key] = value + }) const stripped = stripPayloadFingerprinting(obj, privacyMode)
6-9: Remove unusedProxyRewriteinterface.This interface is defined but never used. The
rewriteScriptUrlsfunction is imported fromproxy-configs.tswhich has its own type definitions.🔧 Proposed fix
-interface ProxyRewrite { - from: string - to: string -} -
280-282: Replaceconsole.debugwith allowed console method.ESLint only allows
console.warnandconsole.error. Useconsole.warnfor debug output or inject a logger.🔧 Proposed fix
- const log = debug ? console.debug.bind(console) : () => {} + const log = debug ? console.warn.bind(console) : () => {}
🧹 Nitpick comments (3)
src/runtime/server/api/scripts-health.ts (1)
57-80: Consider adding error handling for malformed targets.If
target.replace('/**', '/')produces an invalid URL, the fetch will throw. While the catch block handles this, consider validating the URL construction or providing more specific error messages.♻️ Optional improvement
// Convert wildcard target to a testable URL const testUrl = target.replace('/**', '/') const start = Date.now() + + // Validate URL before fetching + try { + new URL(testUrl) + } + catch { + checks.push({ + script: scriptName, + route, + target: testUrl, + status: 'error', + error: 'Invalid target URL', + }) + continue + } const result = await fetch(testUrl, {test/unit/third-party-proxy-replacements.test.ts (2)
81-136: Avoid live network fetches in unit tests.Real vendor downloads can be flaky and the catch+return makes failures silently pass; prefer deterministic fixtures/mocks or gate this check behind an explicit opt‑in so regressions don’t slip unnoticed.
557-611: Consider gating the verbose comparison output.
console.warnon every run can be noisy in CI; optionally guard behind a debug/env flag.
test/unit/proxy-privacy.test.ts
Outdated
| }) | ||
| }) No newline at end of file |
There was a problem hiding this comment.
Missing newline at end of file.
ESLint requires a newline at the end of the file.
🔧 Proposed fix
})
+📝 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.
| }) | |
| }) | |
| }) | |
| }) | |
🧰 Tools
🪛 ESLint
[error] 360-360: Newline required at end of file but not found.
(@stylistic/eol-last)
🤖 Prompt for AI Agents
In `@test/unit/proxy-privacy.test.ts` around lines 359 - 360, Add a single
trailing newline character at the end of proxy-privacy.test.ts so the file ends
with a newline (i.e., after the final closing "})" add one blank line/line
break), then save/commit so ESLint no longer reports "Missing newline at end of
file".
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/module.ts (1)
315-318:⚠️ Potential issue | 🔴 CriticalProxy mode forwards all headers including cookies and credentials to third parties.
In
proxymode (line 315-318),Object.assign(headers, originalHeaders)copies everything, includingcookie,authorization, and other session tokens. TheSENSITIVE_HEADERSdenylist is only applied inanonymizemode. This could leak first-party session credentials to third-party analytics endpoints.Even in "no modification" mode, origin credentials should not be forwarded upstream. The
credentials: 'omit'on line 444 prevents outgoing cookies set by the browser for the third-party domain, butoriginalHeaderscontains incoming request headers from the user's browser to your server—which include first-party cookies/auth tokens.Suggested fix
if (privacy === 'proxy') { // Proxy mode: forward all headers except sensitive ones - Object.assign(headers, originalHeaders) + for (const [key, value] of Object.entries(originalHeaders)) { + if (!value) continue + if (SENSITIVE_HEADERS.includes(key.toLowerCase())) continue + headers[key] = value + } }This note applies to
src/runtime/server/proxy-handler.tslines 315-318, referenced here because the wiring is set up inmodule.ts.
🤖 Fix all issues with AI agents
In `@src/module.ts`:
- Around line 284-294: The migration never runs because config.firstParty
defaults to true; change the condition so we only migrate when the user did not
explicitly set firstParty. In the block handling
config.defaultScriptOptions.bundle, keep the logger.warn, but replace the guard
if (!config.firstParty && config.defaultScriptOptions.bundle) with a check that
firstParty is undefined / not provided by the user (e.g., if (config.firstParty
=== undefined && config.defaultScriptOptions.bundle) or using
Object.prototype.hasOwnProperty.call(originalUserConfig, 'firstParty') to detect
absence) and then set config.firstParty = true; this ensures deprecated
bundle:true migrates for users who didn’t explicitly set firstParty.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 57-84: STRIP_PARAMS and NORMALIZE_PARAMS plus helpers anonymizeIP,
normalizeUserAgent, normalizeLanguage, generalizeScreen, and
stripPayloadFingerprinting are duplicated between proxy-handler.ts and
test/utils/proxy-privacy.ts; extract these constants and functions into a single
shared module (e.g., create a new privacy module) and export them, then replace
the local definitions in both src/runtime/server/proxy-handler.ts and
test/utils/proxy-privacy.ts to import the shared exports (keep the exact
exported names STRIP_PARAMS, NORMALIZE_PARAMS, anonymizeIP, normalizeUserAgent,
normalizeLanguage, generalizeScreen, stripPayloadFingerprinting so existing
callers in ProxyHandler and tests remain unchanged).
- Around line 491-499: The current code unconditionally applies a long
"immutable" cache TTL to all JavaScript responses after calling
rewriteScriptUrls, which is wrong because some scripts (e.g., gtag.js) are not
content-addressed; update the logic in the block that checks responseContentType
and proxyConfig.rewrites so that after rewriteScriptUrls you detect whether the
script is content-addressed or marked immutable (e.g., inspect rewritten URLs
for a content-hash pattern or honor a new immutable flag on proxyConfig.rewrites
entries), or match known vendor paths (e.g., gtag) to treat as non-immutable;
then call setResponseHeader(event, 'cache-control', ...) with a different header
for immutable assets (long max-age, immutable) versus non-immutable/third-party
scripts (shorter max-age, no immutable or use stale-while-revalidate only); keep
reference to responseContentType, proxyConfig, rewriteScriptUrls,
setResponseHeader, and event when implementing the branching logic.
- Around line 283-289: The current error handler in the conditional checking
!targetBase || !matchedPrefix reflects the incoming request path into the HTTP
statusMessage; update the createError call in that block (the code that uses
createError, path, targetBase, matchedPrefix and log) to use a generic
statusMessage like "Not Found" and move the detailed path information into the
error body/message field (or include it in the thrown error object's message
property) so the raw user-controlled path is not placed in HTTP headers; keep
the log('[proxy] No match for path:', path) as-is for server-side debugging.
In `@test/utils/proxy-privacy.ts`:
- Line 143: Replace the overly-broad substring check used in isScreenParam with
the same exact-match + bracket-prefix logic used in production: instead of
STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() ||
lowerKey.includes(p.toLowerCase())), call the reusable helper
matchesParam(lowerKey, STRIP_PARAMS.screen) (or implement equivalent logic:
STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() ||
lowerKey.startsWith(`${p.toLowerCase()}[`)) ) so only exact keys or bracketed
indexed params match (e.g., "sd" or "sd[0]") and avoid accidental matches like
"vpaid".
- Around line 1-176: The test utility duplicates production privacy logic and
has drifted; fix by either extracting the shared implementation from
src/runtime/server/proxy-handler.ts and importing it into test/utils (preferred)
or updating test/utils/proxy-privacy.ts to exactly mirror production: align
STRIP_PARAMS.browser, STRIP_PARAMS.userId, STRIP_PARAMS.screen,
STRIP_PARAMS.platform values to include the missing keys found in
proxy-handler.ts; change isScreenParam to use the same matchesParam logic used
elsewhere (exact and bracket matching), add recursive array handling in
stripFingerprintingFromPayload to match production, and ensure generalizeScreen
behavior matches production (string-only vs object handling) so functions
anonymizeIP, normalizeLanguage, normalizeUserAgent, generalizeScreen, and
stripFingerprintingFromPayload are identical between test and production.
🧹 Nitpick comments (4)
test/unit/third-party-proxy-replacements.test.ts (2)
81-136: Download failure silently passes the test instead of skipping it.When
$fetchthrows (line 89-94), the test returns early without any assertion, counting as a pass. In CI this could mask regressions in the rewrite logic for all vendors. The synthetic tests below provide deterministic coverage, but these live tests still shouldn't silently succeed.Consider using Vitest's
skipAPI so CI reports clearly show which vendors were actually validated:Suggested approach
catch (e: any) { // Some scripts may require valid IDs or have rate limiting - // Skip if we can't download console.warn(`Could not download ${name}: ${e.message}`) - return + it.skip // or use: expect.soft to still mark as inconclusive + return // alternatively: throw new Error(`Skipped: ${e.message}`) }A cleaner pattern would be to check availability in a
beforeAlland conditionally skip the entiredescribeblock, or useit.skipIf.
531-571: Snapshot test for comparison table is fragile and primarily produces diagnostic output.This test generates a human-readable summary via
console.warnand asserts it via inline snapshot. This has two concerns:
- The inline snapshot will break whenever any field is added/removed from the test payloads or the stripping logic changes, even for benign changes.
- The
console.warnoutput is diagnostic—it's useful during development but adds noise in CI.Consider splitting diagnostic output (a separate non-snapshot test or dev-only helper) from the assertion (which should focus on specific privacy guarantees rather than field-count formatting).
src/module.ts (2)
574-578: Variableconfigis shadowed by loop destructuring.The
configdestructured in thesefor...ofloops shadows the outerconfigparameter of thesetup(config, nuxt)function. While it works because the innerconfighas.proxyand the outer isn't needed inside the loop body, this is easy to misread and could lead to subtle bugs if the loop body is extended later.Suggested rename
- for (const [path, config] of Object.entries(neededRoutes)) { - flatRoutes[path] = config.proxy + for (const [path, routeConfig] of Object.entries(neededRoutes)) { + flatRoutes[path] = routeConfig.proxy }Apply similarly at the other occurrence (around line 623).
Also applies to: 621-626
406-438: Service Worker registration awaits activation with a 2 s hard timeout.If the SW takes longer than 2 seconds to activate (e.g., on slow mobile devices or when the SW script is large), the plugin resolves before the SW is controlling the page. Subsequent
sendBeaconintercepts or fetch interceptions may be missed for early-loaded scripts.This may be acceptable as a pragmatic tradeoff, but consider documenting this limitation or making the timeout configurable via
FirstPartyOptions.
src/module.ts
Outdated
| // Handle deprecation of bundle option - migrate to firstParty | ||
| if (config.defaultScriptOptions?.bundle !== undefined) { | ||
| logger.warn( | ||
| '`scripts.defaultScriptOptions.bundle` is deprecated. ' | ||
| + 'Use `scripts.firstParty: true` instead.', | ||
| ) | ||
| // Migrate: treat bundle as firstParty | ||
| if (!config.firstParty && config.defaultScriptOptions.bundle) { | ||
| config.firstParty = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Deprecation migration is effectively unreachable under the current defaults.
firstParty defaults to true (line 219). The migration on line 291 checks if (!config.firstParty && config.defaultScriptOptions.bundle), which can only be true if the user explicitly sets firstParty: false. In that case, re-enabling it via bundle: true seems contradictory—the user disabled first-party but left a deprecated bundle flag on.
The intended migration path (user had bundle: true, hasn't set firstParty) won't fire because config.firstParty is already true from defaults. The deprecation warning on line 286 will fire though, which is useful. Just note that the body of the if on line 291 is dead in practice.
🤖 Prompt for AI Agents
In `@src/module.ts` around lines 284 - 294, The migration never runs because
config.firstParty defaults to true; change the condition so we only migrate when
the user did not explicitly set firstParty. In the block handling
config.defaultScriptOptions.bundle, keep the logger.warn, but replace the guard
if (!config.firstParty && config.defaultScriptOptions.bundle) with a check that
firstParty is undefined / not provided by the user (e.g., if (config.firstParty
=== undefined && config.defaultScriptOptions.bundle) or using
Object.prototype.hasOwnProperty.call(originalUserConfig, 'firstParty') to detect
absence) and then set config.firstParty = true; this ensures deprecated
bundle:true migrates for users who didn’t explicitly set firstParty.
| // Rewrite URLs in JavaScript responses to route through our proxy | ||
| // This is necessary because some SDKs use sendBeacon() which can't be intercepted by SW | ||
| if (responseContentType.includes('javascript') && proxyConfig?.rewrites?.length) { | ||
| content = rewriteScriptUrls(content, proxyConfig.rewrites) | ||
| log('[proxy] Rewrote URLs in JavaScript response') | ||
|
|
||
| // Add cache headers for JavaScript responses (immutable content with hash in filename) | ||
| setResponseHeader(event, 'cache-control', `public, max-age=${cacheTtl}, stale-while-revalidate=${cacheTtl * 2}`) | ||
| } |
There was a problem hiding this comment.
Cache-control applied to all rewritten JS responses, including non-content-addressed URLs.
The comment on line 497 says "immutable content with hash in filename," but not all analytics script URLs are content-addressed (e.g., gtag.js?id=G-XXX). A 1-hour cache with 2-hour stale-while-revalidate is reasonable, but the "immutable" assumption in the comment is misleading and the TTL may need to vary per vendor.
🤖 Prompt for AI Agents
In `@src/runtime/server/proxy-handler.ts` around lines 491 - 499, The current code
unconditionally applies a long "immutable" cache TTL to all JavaScript responses
after calling rewriteScriptUrls, which is wrong because some scripts (e.g.,
gtag.js) are not content-addressed; update the logic in the block that checks
responseContentType and proxyConfig.rewrites so that after rewriteScriptUrls you
detect whether the script is content-addressed or marked immutable (e.g.,
inspect rewritten URLs for a content-hash pattern or honor a new immutable flag
on proxyConfig.rewrites entries), or match known vendor paths (e.g., gtag) to
treat as non-immutable; then call setResponseHeader(event, 'cache-control', ...)
with a different header for immutable assets (long max-age, immutable) versus
non-immutable/third-party scripts (shorter max-age, no immutable or use
stale-while-revalidate only); keep reference to responseContentType,
proxyConfig, rewriteScriptUrls, setResponseHeader, and event when implementing
the branching logic.
test/utils/proxy-privacy.ts
Outdated
| } | ||
|
|
||
| const isIpParam = matchesParam(key, STRIP_PARAMS.ip) | ||
| const isScreenParam = STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() || lowerKey.includes(p.toLowerCase())) |
There was a problem hiding this comment.
isScreenParam substring match is overly broad and inconsistent with production.
const isScreenParam = STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() || lowerKey.includes(p.toLowerCase()))Using .includes() means any key containing the substring "sd", "vp", "sr", etc. will match. For example, a hypothetical key "vpaid" would match via "vp". The production handler (proxy-handler.ts) uses exact-match + bracket-prefix via matchesParam, which is more precise.
Suggested fix to align with production
- const isScreenParam = STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() || lowerKey.includes(p.toLowerCase()))
+ const isScreenParam = matchesParam(key, STRIP_PARAMS.screen)🤖 Prompt for AI Agents
In `@test/utils/proxy-privacy.ts` at line 143, Replace the overly-broad substring
check used in isScreenParam with the same exact-match + bracket-prefix logic
used in production: instead of STRIP_PARAMS.screen.some(p => lowerKey ===
p.toLowerCase() || lowerKey.includes(p.toLowerCase())), call the reusable helper
matchesParam(lowerKey, STRIP_PARAMS.screen) (or implement equivalent logic:
STRIP_PARAMS.screen.some(p => lowerKey === p.toLowerCase() ||
lowerKey.startsWith(`${p.toLowerCase()}[`)) ) so only exact keys or bracketed
indexed params match (e.g., "sd" or "sd[0]") and avoid accidental matches like
"vpaid".
- Don't reflect user-controlled path in HTTP statusMessage (security) - Clone SW request before accessing body stream - Fix rewriteScriptUrls: support host+path from patterns, preserve leading / - Fix joinURL inserting / before query strings in rewritten URLs - Replace hardcoded Segment writeKey with placeholder - Use logger.debug instead of logger.info for first-party config - Simplify unreachable deprecation migration - Remove duplicate comment block in tests - Update test expectations for normalizeUserAgent version output
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@src/module.ts`:
- Around line 604-612: When firstPartyPrivacy === 'proxy' we're currently
merging neededRoutes into nuxt.options.routeRules which lets Nitro forward all
request headers (including cookies/Authorization) to third parties; update the
proxy-mode branch so route rules explicitly deny or strip sensitive headers
before forwarding (e.g., remove Cookie, Authorization, Set-Cookie, Credentials
headers) instead of blindly spreading neededRoutes, or wrap the proxy target
with a small header-filtering handler similar to the anonymize path; ensure the
change targets the same logic that builds neededRoutes/nuxt.options.routeRules
and add a clear comment documenting the privacy trade-off for proxy mode.
In `@src/runtime/server/api/scripts-health.ts`:
- Around line 82-93: The avgLatency calculation currently uses checks.length and
treats missing latencies as 0, which skews the metric when some checks errored;
update the computation in this block (variables: allOk, avgLatency, checks) to
first filter to only checks that have a numeric latency (e.g., checks.filter(c
=> typeof c.latency === 'number' || c.status === 'ok')), compute sum over that
filtered array and divide by its length (with a safe fallback to 0 when the
filtered array is empty), then Math.round the result before returning it in
summary.avgLatency.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 195-203: The code passes a Record<string,unknown> from
stripPayloadFingerprinting into new URLSearchParams, which will coerce
non-string values (objects/arrays) to "[object Object]"; update the
application/x-www-form-urlencoded branch to convert all values to strings before
building URLSearchParams: iterate the stripped object returned by
stripPayloadFingerprinting, for each key convert non-string values using
JSON.stringify (leave existing strings unchanged), produce a
Record<string,string> (or Map) and then call new URLSearchParams(...) to assign
to body; target the logic around stripPayloadFingerprinting, params
construction, and where body is set so nested arrays/objects are serialized
correctly.
- Around line 116-118: In the proxy branch where privacy === 'proxy' you
currently copy all incoming headers via Object.assign(headers, originalHeaders);
change this to copy only non-sensitive headers by filtering out authentication
and session headers (e.g., "authorization", "cookie", "set-cookie", "x-api-key",
"proxy-authorization" and similar) before assigning; update the logic around the
headers and originalHeaders variables so headers receives a sanitized subset
(keep header name comparisons case-insensitive) and ensure any existing tests or
downstream uses of headers still work with the sanitized set.
In `@src/runtime/server/utils/privacy.ts`:
- Around line 74-86: Update the misleading comment in anonymizeIP: change the
IPv4 comment that says "country-level precision" to accurately state it zeroes
the last octet producing a /24 subnet (typically ISP/neighborhood or
city-level), and ensure the IPv6 comment clarifies it keeps the first 3 segments
(48 bits) for similar aggregation; modify the inline comments above the IPv6 and
IPv4 handling in the anonymizeIP function to reflect these accurate
privacy/precision descriptions.
- Around line 42-61: STRIP_PARAMS defines userId and userData but
stripPayloadFingerprinting does not remove those keys; update
stripPayloadFingerprinting to also detect and strip any fields matching
STRIP_PARAMS.userId and STRIP_PARAMS.userData (including nested dot-notation
like "traits.email") the same way ip/screen/platform/etc. are handled—remove or
replace values, and update any related tests; alternatively, if you intend to
forward IDs/PII, remove userId and userData from STRIP_PARAMS and add a clear
comment in the top-level STRIP_PARAMS declaration explaining why these
categories are intentionally preserved.
In `@src/runtime/utils/pure.ts`:
- Around line 48-66: In src/runtime/utils/pure.ts adjust the suffix host
comparisons to use fromHost instead of from: when isSuffixMatch is true replace
occurrences where url.host is checked with .endsWith(from) to use
.endsWith(fromHost) (this applies in the host-full-URL branch that references
url.host and the protocol-relative branch later), ensuring the logic that sets
shouldRewrite and rewriteSuffix still uses fromPath/fullPath as before;
reference variables/functions: isSuffixMatch, fromHost, fromPath, url.host,
fullPath, shouldRewrite, rewriteSuffix.
In `@test/e2e/first-party.test.ts`:
- Around line 277-293: The test "bundled scripts contain rewritten collect URLs"
currently hardcodes cacheDir and silently returns if it's missing; change it to
fail loudly instead. Replace the early `if (!existsSync(cacheDir)) { return }`
with an explicit assertion that the cache exists (use `existsSync(cacheDir)` /
`expect(...).toBe(true)` or similar) so the test fails when the internal path is
missing, then continue checking `files`, `allContent`, and `hasProxyUrl` as
before.
- Around line 73-93: The test currently lumps preserved user-id/PII fields into
FINGERPRINT_PARAMS which conflicts with runtime behavior in
stripPayloadFingerprinting; split FINGERPRINT_PARAMS into two arrays (e.g.,
STRIPPED_FINGERPRINT_PARAMS for true fingerprinting fields like
'plat','canvas','webgl','sr','vp','tz','plugins', etc., and
PRESERVED_FINGERPRINT_PARAMS for user-id/user-data like
'uid','cid','_gid','fbp','fbc','sid','pl_id','p_user_id','ud','email','phone')
and update assertions to only expect removal for STRIPPED_FINGERPRINT_PARAMS
while treating PRESERVED_FINGERPRINT_PARAMS as allowed/preserved by
stripPayloadFingerprinting so the tests reflect the actual function behavior.
In `@test/unit/first-party.test.ts`:
- Around line 6-11: Rename the test to accurately reflect what it asserts:
change the spec title from 'firstParty defaults to true' to something like
'proxy configs are available' (or similar) in the test that calls
getAllProxyConfigs('/_scripts/c'); keep the existing body and comment, and
ensure the test file/test case referencing getAllProxyConfigs remains unchanged
apart from the spec string.
In `@test/unit/proxy-privacy.test.ts`:
- Around line 250-312: The E2E tests assert that fields like cid, uid, _gid,
fbp, fbc, pl_id, p_user_id are stripped but stripFingerprintingFromPayload (and
stripPayloadFingerprinting) intentionally preserves user-id analytics fields
while only removing platform/canvas/browser/location/deviceInfo and normalizing
UA/language/IP; update the E2E constant FINGERPRINT_PARAMS (used by
verifyFingerprintingStripped) to remove user-id fields and only list the actual
fingerprinting categories the implementation strips (or add a comment
documenting the intentional preservation) so the E2E expectations match
stripFingerprintingFromPayload's behavior.
🧹 Nitpick comments (14)
src/runtime/utils/pure.ts (2)
27-27: Regex won't handle escaped quotes or template literal interpolation.The literal regex
/([ '"\])(.*?)\1/gcan mis-match on escaped quotes (e.g.,') or template literals with${...}` expressions. For analytics script URLs this is rarely an issue in practice, and the GA-specific fallback on lines 110-121 covers the most common dynamic construction pattern. Just noting as a known limitation.
110-121: GA-specific fallback is a pragmatic workaround, but consider a comment noting it's coupled to GA's build output.This regex-based replacement targets a specific code generation pattern from Google Analytics. If GA changes their dynamic URL construction, this fallback would silently stop working. A brief inline note documenting the expected input format would help future maintainers.
src/runtime/server/utils/privacy.ts (1)
144-150:matchesParamhelper is re-created on every loop iteration.The closure is defined inside the
forloop body, causing a new function allocation per payload entry. Consider hoisting it outside the loop.♻️ Proposed refactor
Move the helper before the
forloop:+ const matchesParam = (key: string, params: string[]) => { + const lk = key.toLowerCase() + return params.some((p) => { + const lp = p.toLowerCase() + return lk === lp || lk.startsWith(lp + '[') + }) + } + for (const [key, value] of Object.entries(payload)) { const lowerKey = key.toLowerCase() // ... - const matchesParam = (key: string, params: string[]) => { - const lk = key.toLowerCase() - return params.some((p) => { - const lp = p.toLowerCase() - return lk === lp || lk.startsWith(lp + '[') - }) - }src/proxy-configs.ts (1)
19-176: Consider caching the result ofbuildProxyConfigsince it's pure and deterministic.
buildProxyConfigis called fresh on every invocation ofgetProxyConfig,getAllProxyConfigs, andgetSWInterceptRules. Since the output is purely determined bycollectPrefix, consider memoizing it if the same prefix is used repeatedly during build.src/runtime/sw/proxy-sw.template.js (1)
41-49: No error handling — a failed proxy fetch will cause the analytics request to silently fail.If the proxy endpoint is down or returns an error,
event.respondWith(fetch(...))will reject and the original third-party request has already been consumed. For analytics this is typically acceptable (fire-and-forget), but adding a.catch()fallback would improve resilience.♻️ Proposed fix with fallback
event.respondWith( - fetch(proxyUrl.href, { - method: clonedRequest.method, - headers: clonedRequest.headers, - body: clonedRequest.method !== 'GET' && clonedRequest.method !== 'HEAD' ? clonedRequest.body : undefined, - credentials: 'same-origin', - redirect: 'follow', - }), + fetch(proxyUrl.href, { + method: clonedRequest.method, + headers: clonedRequest.headers, + body: clonedRequest.method !== 'GET' && clonedRequest.method !== 'HEAD' ? clonedRequest.body : undefined, + credentials: 'same-origin', + redirect: 'follow', + }).catch(() => fetch(event.request)), )test/unit/first-party.test.ts (1)
82-110: Test only asserts values it manually constructed — consider adding an assertion on actual route content.The status data structure test simulates the module's behavior by building
neededRoutesfrom configs, then asserts on the structure it just built. This validates shape but not correctness of the config content. The route content assertions in lines 57-80 partially cover this, so this is a minor gap.test/unit/third-party-proxy-replacements.test.ts (2)
81-136: Network-dependent tests silently pass on download failure.The
returnon line 93 causes the test to pass silently when the script can't be fetched (rate limiting, network issues, invalid IDs). Consider usingit.skiporexpectwith a skip annotation so CI reporters surface skipped tests rather than false greens.♻️ Suggested approach
catch (e: any) { // Some scripts may require valid IDs or have rate limiting - // Skip if we can't download - console.warn(`Could not download ${name}: ${e.message}`) - return + console.warn(`Skipping ${name}: ${e.message}`) + return // TODO: consider using vitest's `ctx.skip()` for proper skip reporting }Alternatively, if using Vitest ≥ 0.30, the test context's
skip()method can be used to mark the test as skipped instead of silently passing.
139-157: Hardcoded'/_scripts/c'defeats the purpose of theCOLLECT_PREFIXconstant.Line 149 uses
routeBase.replace('/_scripts/c', COLLECT_PREFIX)which is a no-op since both are the same value. IfCOLLECT_PREFIXis ever changed, this string literal would need to change too. UseCOLLECT_PREFIXconsistently or remove the replace since it's redundant.- return rewrite.to.startsWith(routeBase.replace('/_scripts/c', COLLECT_PREFIX)) + return rewrite.to.startsWith(routeBase)src/module.ts (3)
573-576: Variableconfigshadows the outer moduleconfigparameter.The destructured
configin this loop (line 574) shadows theconfigfromsetup(config, nuxt)on line 238. While it works correctly here because onlyconfig.proxyis accessed within the loop body, the shadowing is confusing and fragile.♻️ Suggested rename
- for (const [path, config] of Object.entries(neededRoutes)) { - flatRoutes[path] = config.proxy + for (const [path, routeConfig] of Object.entries(neededRoutes)) { + flatRoutes[path] = routeConfig.proxy }
546-562: Proxy key resolution logic is duplicated.The pattern of deriving
proxyKeyfromregistryScriptsWithImportandregistryKeysis repeated identically in two loops (lines 546–562 for routes, lines 578–589 for rewrites). Extract this into a helper to keep it DRY.♻️ Suggested refactor
+ // Helper: resolve proxy config key for a registry key + function resolveProxyKey(key: string): string | undefined { + const script = registryScriptsWithImport.find( + s => s.import.name === `useScript${key.charAt(0).toUpperCase() + key.slice(1)}`, + ) + return script?.proxy !== false ? (script?.proxy || key) : undefined + } + const neededRoutes: Record<string, { proxy: string }> = {} const unsupportedScripts: string[] = [] + const allRewrites: Array<{ from: string, to: string }> = [] + for (const key of registryKeys) { - const script = registryScriptsWithImport.find(...) - const proxyKey = script?.proxy !== false ? (script?.proxy || key) : undefined + const proxyKey = resolveProxyKey(key) if (proxyKey) { const proxyConfig = proxyConfigs[proxyKey] if (proxyConfig?.routes) { Object.assign(neededRoutes, proxyConfig.routes) } + if (proxyConfig?.rewrite) { + allRewrites.push(...proxyConfig.rewrite) + } // ...Also applies to: 578-589
402-436: Inline SW registration plugin has no unregister/update strategy.The generated plugin registers the service worker and waits for controller change, but there's no version tracking or update handling. If the SW script content changes between deploys, the old SW may remain cached. Consider adding
reg.update()or a version-based cache-bust strategy.src/runtime/server/api/scripts-health.ts (1)
44-80: Health checks run sequentially — considerPromise.allfor faster response.Each route is checked one at a time with a 5-second timeout. With many proxy routes, the worst case is
N × 5s. Since this is dev-only, it's not critical, but parallel execution would improve DX.♻️ Suggested approach
- for (const [route, target] of Object.entries(scriptsConfig.routes)) { - // ... sequential fetch ... - checks.push(result) - } + const checkPromises = Object.entries(scriptsConfig.routes).map(async ([route, target]) => { + const scriptMatch = route.match(scriptNameRegex) + const scriptName = scriptMatch?.[1] || 'unknown' + const testUrl = target.replace('/**', '/') + const start = Date.now() + return fetch(testUrl, { method: 'HEAD', signal: AbortSignal.timeout(5000) }) + .then((res) => ({ + script: scriptName, route, target: testUrl, + status: res.ok ? 'ok' : 'error', + latency: Date.now() - start, + error: res.ok ? undefined : `HTTP ${res.status}`, + } satisfies HealthCheckResult)) + .catch((err) => ({ + script: scriptName, route, target: testUrl, + status: 'error', error: err.message || 'Connection failed', + } satisfies HealthCheckResult)) + }) + const checks = await Promise.all(checkPromises)test/e2e/first-party.test.ts (2)
335-369: Inconsistent assertion strictness across providers.Some providers (Segment line 456, xPixel line 476, Snapchat line 509) unconditionally assert
captures.length > 0, while others (GA, GTM, Meta, TikTok, Reddit, Clarity, Hotjar) wrap all assertions inif (captures.length > 0). This means the unconditional tests will become flaky if the headless environment changes, while the conditional tests provide no regression coverage if captures stop working.Consider unifying the approach — either all providers are conditional (acceptable for third-party dependent tests) or document why specific providers are expected to always produce captures.
Also applies to: 440-471, 473-491, 493-524
301-333:testProviderhelper is only used by one test (xPixel).The helper extracts a common test pattern nicely, but other provider tests (GA, GTM, Meta, etc.) duplicate the same browser setup / navigation / capture reading logic manually. Consider using
testProvidermore broadly to reduce boilerplate, or removing it in favor of consistent inline patterns.
| // Inject route handling (handler already registered outside modules:done) | ||
| if (Object.keys(neededRoutes).length) { | ||
| if (firstPartyPrivacy === 'proxy') { | ||
| // Proxy mode: use simple Nitro route rules (forwards all headers) | ||
| nuxt.options.routeRules = { | ||
| ...nuxt.options.routeRules, | ||
| ...neededRoutes, | ||
| } | ||
| } |
There was a problem hiding this comment.
In proxy mode, Nitro route rules forward all headers including cookies and credentials to third parties.
When firstPartyPrivacy === 'proxy' (line 606), route rules are injected directly into Nitro, which uses its built-in proxy that forwards all request headers. This means user cookies, authorization tokens, and other sensitive headers will be sent to third-party analytics endpoints. The anonymize mode correctly strips these via the custom handler, but proxy mode lacks this protection.
Consider documenting this trade-off prominently or adding at minimum a sensitive-header deny list even in proxy mode.
🤖 Prompt for AI Agents
In `@src/module.ts` around lines 604 - 612, When firstPartyPrivacy === 'proxy'
we're currently merging neededRoutes into nuxt.options.routeRules which lets
Nitro forward all request headers (including cookies/Authorization) to third
parties; update the proxy-mode branch so route rules explicitly deny or strip
sensitive headers before forwarding (e.g., remove Cookie, Authorization,
Set-Cookie, Credentials headers) instead of blindly spreading neededRoutes, or
wrap the proxy target with a small header-filtering handler similar to the
anonymize path; ensure the change targets the same logic that builds
neededRoutes/nuxt.options.routeRules and add a clear comment documenting the
privacy trade-off for proxy mode.
| const allOk = checks.every(c => c.status === 'ok') | ||
| const avgLatency = checks.length > 0 ? checks.reduce((sum, c) => sum + (c.latency || 0), 0) / checks.length : 0 | ||
|
|
||
| return { | ||
| status: allOk ? 'healthy' : 'degraded', | ||
| timestamp: new Date().toISOString(), | ||
| summary: { | ||
| total: checks.length, | ||
| ok: checks.filter(c => c.status === 'ok').length, | ||
| errors: checks.filter(c => c.status === 'error').length, | ||
| avgLatency: Math.round(avgLatency), | ||
| }, |
There was a problem hiding this comment.
Average latency calculation includes failed checks (latency = 0), skewing the metric.
Error checks have no latency property, so c.latency || 0 contributes 0 to the sum while still counting toward checks.length. This understates average latency when errors occur.
♻️ Suggested fix
- const avgLatency = checks.length > 0 ? checks.reduce((sum, c) => sum + (c.latency || 0), 0) / checks.length : 0
+ const okChecks = checks.filter(c => c.status === 'ok' && c.latency !== undefined)
+ const avgLatency = okChecks.length > 0 ? okChecks.reduce((sum, c) => sum + c.latency!, 0) / okChecks.length : 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.
| const allOk = checks.every(c => c.status === 'ok') | |
| const avgLatency = checks.length > 0 ? checks.reduce((sum, c) => sum + (c.latency || 0), 0) / checks.length : 0 | |
| return { | |
| status: allOk ? 'healthy' : 'degraded', | |
| timestamp: new Date().toISOString(), | |
| summary: { | |
| total: checks.length, | |
| ok: checks.filter(c => c.status === 'ok').length, | |
| errors: checks.filter(c => c.status === 'error').length, | |
| avgLatency: Math.round(avgLatency), | |
| }, | |
| const allOk = checks.every(c => c.status === 'ok') | |
| const okChecks = checks.filter(c => c.status === 'ok' && c.latency !== undefined) | |
| const avgLatency = okChecks.length > 0 ? okChecks.reduce((sum, c) => sum + c.latency!, 0) / okChecks.length : 0 | |
| return { | |
| status: allOk ? 'healthy' : 'degraded', | |
| timestamp: new Date().toISOString(), | |
| summary: { | |
| total: checks.length, | |
| ok: checks.filter(c => c.status === 'ok').length, | |
| errors: checks.filter(c => c.status === 'error').length, | |
| avgLatency: Math.round(avgLatency), | |
| }, |
🤖 Prompt for AI Agents
In `@src/runtime/server/api/scripts-health.ts` around lines 82 - 93, The
avgLatency calculation currently uses checks.length and treats missing latencies
as 0, which skews the metric when some checks errored; update the computation in
this block (variables: allOk, avgLatency, checks) to first filter to only checks
that have a numeric latency (e.g., checks.filter(c => typeof c.latency ===
'number' || c.status === 'ok')), compute sum over that filtered array and divide
by its length (with a safe fallback to 0 when the filtered array is empty), then
Math.round the result before returning it in summary.avgLatency.
| if (privacy === 'proxy') { | ||
| // Proxy mode: forward all headers | ||
| Object.assign(headers, originalHeaders) |
There was a problem hiding this comment.
Proxy mode forwards all headers (including cookies/auth) to third parties.
Object.assign(headers, originalHeaders) in proxy mode copies everything, including cookie, authorization, and other sensitive headers. Even in "no modification" mode, leaking origin session tokens to analytics providers is a security risk. At minimum, sensitive headers should be stripped.
🛡️ Proposed fix
if (privacy === 'proxy') {
- // Proxy mode: forward all headers
- Object.assign(headers, originalHeaders)
+ // Proxy mode: forward all headers except credentials
+ for (const [key, value] of Object.entries(originalHeaders)) {
+ if (!value) continue
+ if (SENSITIVE_HEADERS.includes(key.toLowerCase())) continue
+ headers[key] = value
+ }
}🤖 Prompt for AI Agents
In `@src/runtime/server/proxy-handler.ts` around lines 116 - 118, In the proxy
branch where privacy === 'proxy' you currently copy all incoming headers via
Object.assign(headers, originalHeaders); change this to copy only non-sensitive
headers by filtering out authentication and session headers (e.g.,
"authorization", "cookie", "set-cookie", "x-api-key", "proxy-authorization" and
similar) before assigning; update the logic around the headers and
originalHeaders variables so headers receives a sanitized subset (keep header
name comparisons case-insensitive) and ensure any existing tests or downstream
uses of headers still work with the sanitized set.
| else if (contentType.includes('application/x-www-form-urlencoded')) { | ||
| // URL-encoded form data | ||
| const params = new URLSearchParams(rawBody) | ||
| const obj: Record<string, unknown> = {} | ||
| params.forEach((value, key) => { | ||
| obj[key] = value | ||
| }) | ||
| const stripped = stripPayloadFingerprinting(obj) | ||
| body = new URLSearchParams(stripped as Record<string, string>).toString() |
There was a problem hiding this comment.
URLSearchParams receives Record<string, string> cast but values may not all be strings.
stripPayloadFingerprinting returns Record<string, unknown> — nested objects or arrays that survive stripping will be coerced to "[object Object]" when passed to URLSearchParams. Serialize non-string values before constructing the params.
🐛 Proposed fix
const stripped = stripPayloadFingerprinting(obj)
- body = new URLSearchParams(stripped as Record<string, string>).toString()
+ const stringified: Record<string, string> = {}
+ for (const [k, v] of Object.entries(stripped)) {
+ stringified[k] = typeof v === 'string' ? v : JSON.stringify(v)
+ }
+ body = new URLSearchParams(stringified).toString()🤖 Prompt for AI Agents
In `@src/runtime/server/proxy-handler.ts` around lines 195 - 203, The code passes
a Record<string,unknown> from stripPayloadFingerprinting into new
URLSearchParams, which will coerce non-string values (objects/arrays) to
"[object Object]"; update the application/x-www-form-urlencoded branch to
convert all values to strings before building URLSearchParams: iterate the
stripped object returned by stripPayloadFingerprinting, for each key convert
non-string values using JSON.stringify (leave existing strings unchanged),
produce a Record<string,string> (or Map) and then call new URLSearchParams(...)
to assign to body; target the logic around stripPayloadFingerprinting, params
construction, and where body is set so nested arrays/objects are serialized
correctly.
| export const STRIP_PARAMS = { | ||
| // IP addresses | ||
| ip: ['uip', 'ip', 'client_ip_address', 'ip_address', 'user_ip', 'ipaddress', 'context.ip'], | ||
| // User identifiers | ||
| userId: ['uid', 'user_id', 'userid', 'external_id', 'cid', '_gid', 'fbp', 'fbc', 'sid', 'session_id', 'sessionid', 'pl_id', 'p_user_id', 'uuid', 'anonymousid', 'twclid', 'u_c1', 'u_sclid', 'u_scsid'], | ||
| // User data (PII) - includes email, phone, etc. | ||
| userData: ['ud', 'user_data', 'userdata', 'email', 'phone', 'traits.email', 'traits.phone'], | ||
| // Screen/Hardware fingerprinting (sh/sw = Snapchat screen height/width) | ||
| screen: ['sr', 'vp', 'sd', 'screen', 'viewport', 'colordepth', 'pixelratio', 'sh', 'sw'], | ||
| // Platform fingerprinting (d_a = architecture, d_ot = OS type, d_os = OS version) | ||
| platform: ['plat', 'platform', 'hardwareconcurrency', 'devicememory', 'cpu', 'mem', 'd_a', 'd_ot', 'd_os'], | ||
| // Browser fingerprinting (d_bvs = Snapchat browser versions) | ||
| browser: ['plugins', 'fonts', 'd_bvs'], | ||
| // Location/Timezone | ||
| location: ['tz', 'timezone', 'timezoneoffset'], | ||
| // Canvas/WebGL fingerprinting | ||
| canvas: ['canvas', 'webgl', 'audiofingerprint'], | ||
| // Combined device fingerprinting (X/Twitter dv param contains: timezone, locale, vendor, platform, screen, etc.) | ||
| deviceInfo: ['dv', 'device_info', 'deviceinfo', 'bci', 'eci'], | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the original concern about userId/userData usage
echo "=== STRIP_PARAMS definition and usage ==="
rg -n "STRIP_PARAMS|userId|userData" src/runtime/server/utils/privacy.ts -A 2
echo ""
echo "=== stripPayloadFingerprinting implementation detail ==="
rg -n "stripPayloadFingerprinting" src/runtime/server/ -lRepository: nuxt/scripts
Length of output: 1463
🏁 Script executed:
#!/bin/bash
# Check if userId/userData stripping is handled elsewhere
echo "=== Check for alternative userId/userData handling in privacy.ts ==="
sed -n '128,182p' src/runtime/server/utils/privacy.ts
echo ""
echo "=== Look for references to STRIP_PARAMS outside privacy.ts ==="
rg "STRIP_PARAMS\.(userId|userData)" --glob="*.ts" src/Repository: nuxt/scripts
Length of output: 2003
🏁 Script executed:
#!/bin/bash
# Check proxy-handler and other consumers
echo "=== Check proxy-handler for privacy/fingerprinting logic ==="
find src/runtime/server -name "proxy-handler.ts" -o -name "*proxy*.ts" | head -5
echo ""
echo "=== Search for imports of stripPayloadFingerprinting ==="
rg "stripPayloadFingerprinting|privacy" src/runtime/server/ --glob="*.ts" -lRepository: nuxt/scripts
Length of output: 329
🏁 Script executed:
#!/bin/bash
echo "=== Check proxy-handler.ts usage of stripPayloadFingerprinting ==="
rg -n "stripPayloadFingerprinting" src/runtime/server/proxy-handler.ts -B 5 -A 10
echo ""
echo "=== Check for any comments or design notes about userId/userData ==="
rg -n "userId|userData|user.*id|PII|personal" src/runtime/server/proxy-handler.ts -iRepository: nuxt/scripts
Length of output: 3232
🏁 Script executed:
#!/bin/bash
echo "=== Check the entire privacy.ts for any comments about userId/userData handling ==="
cat -n src/runtime/server/utils/privacy.ts | head -100
echo ""
echo "=== Check if there's any git history or comments ==="
rg -n "userId|userData" src/runtime/server/utils/privacy.ts -B 2 -A 2Repository: nuxt/scripts
Length of output: 4937
STRIP_PARAMS.userId and STRIP_PARAMS.userData are defined but never checked in stripPayloadFingerprinting.
Lines 46–48 define sensitive parameter categories (user IDs like uid, fbp, fbc and PII like email, phone), but the stripPayloadFingerprinting function (lines 128–182) only handles ip, screen, platform, canvas, browser, location, and deviceInfo. The userId and userData parameters silently pass through unstripped.
Either add these checks to the function or remove them from STRIP_PARAMS with a comment explaining why user identifiers and PII are intentionally forwarded.
🤖 Prompt for AI Agents
In `@src/runtime/server/utils/privacy.ts` around lines 42 - 61, STRIP_PARAMS
defines userId and userData but stripPayloadFingerprinting does not remove those
keys; update stripPayloadFingerprinting to also detect and strip any fields
matching STRIP_PARAMS.userId and STRIP_PARAMS.userData (including nested
dot-notation like "traits.email") the same way ip/screen/platform/etc. are
handled—remove or replace values, and update any related tests; alternatively,
if you intend to forward IDs/PII, remove userId and userData from STRIP_PARAMS
and add a clear comment in the top-level STRIP_PARAMS declaration explaining why
these categories are intentionally preserved.
| if (url.host) { | ||
| // Full URL with host | ||
| const hostMatches = isSuffixMatch | ||
| ? url.host.endsWith(from) | ||
| : url.host === fromHost | ||
|
|
||
| if (hostMatches) { | ||
| const fullPath = url.pathname + (url.search || '') + (url.hash || '') | ||
| if (fromPath && fullPath.startsWith(fromPath)) { | ||
| // Host+path match: strip the fromPath prefix from the suffix | ||
| shouldRewrite = true | ||
| rewriteSuffix = fullPath.slice(fromPath.length) | ||
| } | ||
| else if (!fromPath) { | ||
| // Host-only match | ||
| shouldRewrite = true | ||
| rewriteSuffix = fullPath | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the function to understand the bug
echo "=== Full rewriteScriptUrls function from lines 48-80 ==="
sed -n '1,80p' src/runtime/utils/pure.ts | tail -n 80
echo ""
echo "=== Look for fromHost definition ==="
rg -n "fromHost" src/runtime/utils/pure.ts -B2 -A2Repository: nuxt/scripts
Length of output: 3681
🏁 Script executed:
#!/bin/bash
# Look for tests related to rewriteScriptUrls
echo "=== Find test files for rewriteScriptUrls ==="
fd -e test.ts -e test.js -e spec.ts | head -20
echo ""
echo "=== Search for tests of rewriteScriptUrls ==="
rg -l "rewriteScriptUrls" test/ --type ts --type js 2>/dev/null || echo "No matches found in test/"
echo ""
echo "=== Search for any tests in test/unit ==="
fd . test/unit/ -name "*.ts" -o -name "*.js" | head -10
echo ""
echo "=== Look for tests with google-analytics or suffix patterns ==="
rg "google-analytics|isSuffixMatch|suffix" test/ --type ts --type js -i 2>/dev/null || echo "No matches"Repository: nuxt/scripts
Length of output: 3886
🏁 Script executed:
#!/bin/bash
# Check what patterns are tested in proxy-configs.test.ts
echo "=== Search for suffix patterns (starting with dot) in tests ==="
rg "from:\s*['\"]\..*['\"]" test/ -A1 -B1
echo ""
echo "=== Check proxy-configs.test.ts for pattern structure ==="
head -100 test/unit/proxy-configs.test.ts
echo ""
echo "=== Look for any regionalized subdomain tests ==="
rg "region\|cdn\|subdomain" test/unit/proxy-configs.test.ts -iRepository: nuxt/scripts
Length of output: 4136
🏁 Script executed:
#!/bin/bash
# Check if suffix patterns (starting with .) are actually used anywhere
echo "=== Search for any usage of suffix patterns in config files ==="
rg "from:\s*['\"]\..*['\"]" src/ --type ts -A1 -B1
echo ""
echo "=== Check how proxy configs are defined ==="
grep -r "from:" src/proxy-configs.ts -A1 -B1 | head -50
echo ""
echo "=== Look for dot patterns in entire codebase ==="
rg "startsWith\('['\"]\.'" src/ --type ts -B3 -A3Repository: nuxt/scripts
Length of output: 3090
🏁 Script executed:
#!/bin/bash
# Verify the bug with a concrete example
echo "=== Check if suffix patterns are tested ==="
rg "\.google-analytics\.com" test/ --type ts -B2 -A2
echo ""
echo "=== Create a test case to demonstrate the bug ==="
cat > /tmp/test_suffix.js << 'EOF'
// Simulate the buggy code
const from = '.google-analytics.com/g/collect'
const isSuffixMatch = from.startsWith('.')
const fromSlashIdx = from.indexOf('/')
const fromHost = fromSlashIdx > 0 ? from.slice(0, fromSlashIdx) : from
// Test case: URL with regionalized subdomain
const url_host = 'region1.google-analytics.com'
// Current buggy code
const buggyMatch = isSuffixMatch ? url_host.endsWith(from) : url_host === fromHost
console.log(`Buggy: url.host.endsWith(from) = ${url_host}.endsWith("${from}") = ${buggyMatch}`)
// Fixed code
const fixedMatch = isSuffixMatch ? url_host.endsWith(fromHost) : url_host === fromHost
console.log(`Fixed: url.host.endsWith(fromHost) = ${url_host}.endsWith("${fromHost}") = ${fixedMatch}`)
EOF
node /tmp/test_suffix.jsRepository: nuxt/scripts
Length of output: 6872
Fix suffix match comparison to use fromHost instead of full from pattern.
Line 51 compares url.host.endsWith(from) when isSuffixMatch is true. Since from includes the path component (e.g., .google-analytics.com/g/collect), a host like region1.google-analytics.com will never match because hosts don't contain path separators. Should compare against fromHost (just .google-analytics.com).
The same bug exists on line 72 for protocol-relative URLs. These suffix patterns are used in production config for GA4's dynamic URL construction with regionalized subdomains, so this breaks an intended feature.
🐛 Proposed fix
if (url.host) {
// Full URL with host
const hostMatches = isSuffixMatch
- ? url.host.endsWith(from)
+ ? url.host.endsWith(fromHost)
: url.host === fromHostAnd for protocol-relative URLs (line 72):
const hostMatches = isSuffixMatch
- ? hostPart?.endsWith(from) ?? false
+ ? hostPart?.endsWith(fromHost) ?? false
: hostPart === fromHost🤖 Prompt for AI Agents
In `@src/runtime/utils/pure.ts` around lines 48 - 66, In src/runtime/utils/pure.ts
adjust the suffix host comparisons to use fromHost instead of from: when
isSuffixMatch is true replace occurrences where url.host is checked with
.endsWith(from) to use .endsWith(fromHost) (this applies in the host-full-URL
branch that references url.host and the protocol-relative branch later),
ensuring the logic that sets shouldRewrite and rewriteSuffix still uses
fromPath/fullPath as before; reference variables/functions: isSuffixMatch,
fromHost, fromPath, url.host, fullPath, shouldRewrite, rewriteSuffix.
| const FINGERPRINT_PARAMS = [ | ||
| // IP addresses | ||
| 'uip', 'ip', 'client_ip', 'user_ip', 'ipaddress', | ||
| // User identifiers (should be stripped) | ||
| 'uid', 'user_id', 'userid', 'external_id', 'cid', '_gid', 'fbp', 'fbc', | ||
| 'sid', 'session_id', 'sessionid', 'pl_id', 'p_user_id', 'anonymousid', 'twclid', | ||
| // User data (PII) | ||
| 'ud', 'user_data', 'userdata', 'email', 'phone', | ||
| // Screen/Hardware fingerprinting | ||
| 'sr', 'vp', 'sd', 'screen', 'viewport', 'colordepth', 'pixelratio', | ||
| // Platform fingerprinting | ||
| 'plat', 'platform', 'hardwareconcurrency', 'devicememory', 'cpu', 'mem', | ||
| // Browser fingerprinting | ||
| 'plugins', 'fonts', | ||
| // Location/Timezone | ||
| 'tz', 'timezone', 'timezoneoffset', | ||
| // Canvas/WebGL fingerprinting | ||
| 'canvas', 'webgl', 'audiofingerprint', | ||
| // Combined device fingerprinting (X/Twitter) | ||
| 'dv', 'device_info', 'deviceinfo', 'bci', 'eci', | ||
| ] |
There was a problem hiding this comment.
FINGERPRINT_PARAMS includes userId/PII fields that stripPayloadFingerprinting intentionally preserves.
As detailed in the unit test review, fields like uid, cid, _gid, fbp, fbc, sid, pl_id, p_user_id, ud, email, phone, etc. (lines 77-80) are not stripped by the runtime stripPayloadFingerprinting function — they're preserved for analytics to function correctly. Including them here creates a false contract: these tests pass only because real SDKs in headless browsers happen not to send these fields in captured requests.
If a future SDK version or configuration causes these params to appear in proxied requests, these tests will fail unexpectedly — not because of a regression, but because the assertion was wrong.
Split FINGERPRINT_PARAMS into two lists: one for truly stripped fields (platform, canvas, browser, location, deviceInfo) and one for preserved-but-tracked fields (userId, userData).
🤖 Prompt for AI Agents
In `@test/e2e/first-party.test.ts` around lines 73 - 93, The test currently lumps
preserved user-id/PII fields into FINGERPRINT_PARAMS which conflicts with
runtime behavior in stripPayloadFingerprinting; split FINGERPRINT_PARAMS into
two arrays (e.g., STRIPPED_FINGERPRINT_PARAMS for true fingerprinting fields
like 'plat','canvas','webgl','sr','vp','tz','plugins', etc., and
PRESERVED_FINGERPRINT_PARAMS for user-id/user-data like
'uid','cid','_gid','fbp','fbc','sid','pl_id','p_user_id','ud','email','phone')
and update assertions to only expect removal for STRIPPED_FINGERPRINT_PARAMS
while treating PRESERVED_FINGERPRINT_PARAMS as allowed/preserved by
stripPayloadFingerprinting so the tests reflect the actual function behavior.
| it('bundled scripts contain rewritten collect URLs', async () => { | ||
| // Check bundled scripts have proxy URLs | ||
| const cacheDir = join(fixtureDir, 'node_modules/.cache/nuxt/scripts/bundle-proxy') | ||
| if (!existsSync(cacheDir)) { | ||
| // Cache might not exist in this test run | ||
| return | ||
| } | ||
|
|
||
| const files = readdirSync(cacheDir).filter(f => f.endsWith('.js')) | ||
| expect(files.length).toBeGreaterThan(0) | ||
|
|
||
| // Combine all cached scripts content | ||
| const allContent = files.map(f => readFileSync(join(cacheDir, f), 'utf-8')).join('\n') | ||
| // Verify at least one proxy URL is present (GA, Clarity, etc.) | ||
| const hasProxyUrl = allContent.includes('/_proxy/') | ||
| expect(hasProxyUrl).toBe(true) | ||
| }) |
There was a problem hiding this comment.
Bundled script cache path is fragile and test silently passes if missing.
Line 279 hardcodes node_modules/.cache/nuxt/scripts/bundle-proxy which is an internal implementation detail. If this path changes, the test silently passes via the early return on line 282 rather than failing.
🤖 Prompt for AI Agents
In `@test/e2e/first-party.test.ts` around lines 277 - 293, The test "bundled
scripts contain rewritten collect URLs" currently hardcodes cacheDir and
silently returns if it's missing; change it to fail loudly instead. Replace the
early `if (!existsSync(cacheDir)) { return }` with an explicit assertion that
the cache exists (use `existsSync(cacheDir)` / `expect(...).toBe(true)` or
similar) so the test fails when the internal path is missing, then continue
checking `files`, `allContent`, and `hasProxyUrl` as before.
| it('firstParty defaults to true', async () => { | ||
| // This is a documentation test - the actual default is in module.ts | ||
| // We test the proxy configs work correctly when enabled | ||
| const configs = getAllProxyConfigs('/_scripts/c') | ||
| expect(Object.keys(configs).length).toBeGreaterThan(0) | ||
| }) |
There was a problem hiding this comment.
Test name is misleading — it tests config existence, not the default value.
The test is named 'firstParty defaults to true' and the comment says "the actual default is in module.ts", but the test body only verifies that getAllProxyConfigs returns non-empty configs. Consider renaming to something like 'proxy configs are available'.
🤖 Prompt for AI Agents
In `@test/unit/first-party.test.ts` around lines 6 - 11, Rename the test to
accurately reflect what it asserts: change the spec title from 'firstParty
defaults to true' to something like 'proxy configs are available' (or similar)
in the test that calls getAllProxyConfigs('/_scripts/c'); keep the existing body
and comment, and ensure the test file/test case referencing getAllProxyConfigs
remains unchanged apart from the spec string.
| describe('stripFingerprintingFromPayload', () => { | ||
| describe('anonymize mode', () => { | ||
| it('anonymizes GA payload while preserving analytics value', () => { | ||
| const result = stripFingerprintingFromPayload(FINGERPRINT_PAYLOAD.ga) | ||
|
|
||
| // IP anonymized to country level | ||
| expect(result.uip).toBe('192.168.1.0') | ||
|
|
||
| // User IDs preserved for analytics | ||
| expect(result.cid).toBe('1234567890.1234567890') | ||
| expect(result.uid).toBe('user-123') | ||
| expect(result._gid).toBe('GA1.2.1234567890.1234567890') | ||
|
|
||
| // Screen generalized | ||
| expect(result.sr).toBe('2560x1440') | ||
| expect(result.vp).toBe('1920x1080') | ||
|
|
||
| // Language/UA normalized | ||
| expect(result.ul).toBe('en') | ||
| expect(result.ua).toBe('Mozilla/5.0 (compatible; Chrome/120.0)') | ||
|
|
||
| // Page context preserved | ||
| expect(result.dt).toBe('Page Title') | ||
| expect(result.dl).toBe('https://example.com/page') | ||
| }) | ||
|
|
||
| it('anonymizes Meta payload while preserving event tracking', () => { | ||
| const result = stripFingerprintingFromPayload(FINGERPRINT_PAYLOAD.meta) | ||
|
|
||
| // User data preserved | ||
| expect(result.ud).toBeDefined() | ||
|
|
||
| // User IDs preserved for analytics | ||
| expect(result.fbp).toBe('_fbp=fb.1.1234567890.1234567890') | ||
| expect(result.fbc).toBe('_fbc=fb.1.1234567890.AbCdEfGh') | ||
| expect(result.external_id).toBe('ext-user-123') | ||
|
|
||
| // IP anonymized | ||
| expect(result.client_ip_address).toBe('192.168.1.0') | ||
|
|
||
| // UA normalized | ||
| expect(result.client_user_agent).toBe('Mozilla/5.0 (compatible; Chrome/120.0)') | ||
| }) | ||
|
|
||
| it('strips fingerprinting from X/Twitter pixel payload', () => { | ||
| const result = stripFingerprintingFromPayload(FINGERPRINT_PAYLOAD.xPixel) | ||
|
|
||
| // Should NOT have these fingerprinting params | ||
| expect(result.dv).toBeUndefined() // Combined device info | ||
| expect(result.bci).toBeUndefined() // Browser context | ||
| expect(result.eci).toBeUndefined() // Environment context | ||
|
|
||
| // User IDs preserved for analytics | ||
| expect(result.pl_id).toBe('35809bf2-ef6f-4b4f-9afc-4ffceb3b7e4c') | ||
| expect(result.p_user_id).toBe('0') | ||
|
|
||
| // Should keep non-fingerprinting params | ||
| expect(result.event).toBe('{}') | ||
| expect(result.event_id).toBe('a944216c-54e2-4dbb-a338-144f32888929') | ||
| expect(result.pt).toBe('Test Page') | ||
| expect(result.txn_id).toBe('ol7lz') | ||
| expect(result.type).toBe('javascript') | ||
| }) |
There was a problem hiding this comment.
User ID fields (cid, uid, fbp, fbc, pl_id, p_user_id…) are intentionally preserved — but the E2E test's FINGERPRINT_PARAMS list includes them.
The unit tests correctly verify that stripPayloadFingerprinting preserves userId-category fields (cid, uid, _gid, fbp, fbc, pl_id, p_user_id, etc.) since the function only strips platform/canvas/browser/location/deviceInfo categories and normalizes UA/language/IP.
However, the E2E test in test/e2e/first-party.test.ts (lines 73-93) includes these same userId fields in FINGERPRINT_PARAMS and uses verifyFingerprintingStripped to assert they do NOT appear in stripped captures. If any real analytics SDK sends these params, the E2E test would fail — or worse, it passes only because the headless browser tests never trigger requests containing these fields, masking a false expectation.
Align the E2E FINGERPRINT_PARAMS with what stripPayloadFingerprinting actually strips, or document the intentional gap.
🤖 Prompt for AI Agents
In `@test/unit/proxy-privacy.test.ts` around lines 250 - 312, The E2E tests assert
that fields like cid, uid, _gid, fbp, fbc, pl_id, p_user_id are stripped but
stripFingerprintingFromPayload (and stripPayloadFingerprinting) intentionally
preserves user-id analytics fields while only removing
platform/canvas/browser/location/deviceInfo and normalizing UA/language/IP;
update the E2E constant FINGERPRINT_PARAMS (used by
verifyFingerprintingStripped) to remove user-id fields and only list the actual
fingerprinting categories the implementation strips (or add a comment
documenting the intentional preservation) so the E2E expectations match
stripFingerprintingFromPayload's behavior.
1 participant
🔗 Linked issue
Resolves #87
❓ Type of change
📚 Description
Third-party scripts expose user data directly to external servers - every request shares the user's IP address, and scripts can set third-party cookies for cross-site tracking. Ad blockers rightfully block these for privacy reasons.
This PR adds a
firstPartyoption that routes all script traffic through your own domain:Scripts are downloaded at build time, collection URLs rewritten to local paths (
/_scripts/c/ga), and Nitro route rules proxy requests to original endpoints.Supported: Google Analytics, GTM, Meta Pixel, TikTok, Segment, Clarity, Hotjar, X/Twitter, Snapchat, Reddit.
Includes new
/docs/guides/first-partydocumentation and deprecation notice on bundling guide.