feat(first-party): exclude fingerprinting scripts, fix per-script opt-out#662
Open
feat(first-party): exclude fingerprinting scripts, fix per-script opt-out#662
Conversation
…-out Scripts requiring browser fingerprinting for fraud/bot detection (Stripe, PayPal, Google reCAPTCHA, Google Sign-In) now set proxy: false and scriptBundling: false, excluding them from first-party mode entirely. Per-script firstParty: false opt-out now works in registry config entries, fixing the reported issue where Plausible autoInject ignored the opt-out. Both applyAutoInject and finalizeFirstParty check the entry before injecting proxy endpoints or registering domains. Updated docs, architecture reference, and tests to match the current domain-based proxy config format.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe diff replaces a pattern-based first-party proxy (InterceptRule/routes/rewrites) with a domain-driven model: proxy configs now declare domains and a single proxyPrefix. Build-time AST rewrites are derived from domains; runtime interceptors and Partytown resolve now rewrite non-same-origin URLs to origin + proxyPrefix + "/" + host + path. Exports and types for Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/first-party/proxy-configs.ts (1)
30-30: Unused parameter_proxyPrefixinbuildProxyConfig.The
_proxyPrefixparameter is passed but never used withinbuildProxyConfig. TheautoInject.computeValuefunctions receiveproxyPrefixas an argument when they're called (at runtime), not frombuildProxyConfig. Consider removing this parameter if it's truly unused, or document why it's retained.🧹 Option A: Remove unused parameter
-function buildProxyConfig(_proxyPrefix: string) { +function buildProxyConfig() { return {And update the caller:
export function getAllProxyConfigs(proxyPrefix: string): Partial<Record<RegistryScriptKey, ProxyConfig>> { - return buildProxyConfig(proxyPrefix) + return buildProxyConfig() }🧹 Option B: Add comment explaining future use
-function buildProxyConfig(_proxyPrefix: string) { +// proxyPrefix reserved for future static config generation +function buildProxyConfig(_proxyPrefix: string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/first-party/proxy-configs.ts` at line 30, The parameter `_proxyPrefix` on function buildProxyConfig is unused — remove it from the buildProxyConfig signature and update any callers to stop passing that argument, or if you intentionally reserve it for future use, keep the parameter but add a one-line comment above buildProxyConfig explaining it is retained intentionally for future API compatibility (and that actual runtime values come from autoInject.computeValue callbacks which receive proxyPrefix). Ensure references to buildProxyConfig and the autoInject.computeValue usage remain consistent after the change.FIRST_PARTY.md (1)
77-84: Add language specifiers to fenced code blocks.The code blocks are missing language specifiers, which aids syntax highlighting and satisfies markdownlint MD040.
📝 Proposed fix
-``` +```ts scripts.registry.plausibleAnalytics = { domain: 'mysite.com', firstParty: false }Or in tuple form with scriptOptions:
-+ts
scripts.registry.plausibleAnalytics = [{ domain: 'mysite.com' }, { firstParty: false }]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@FIRST_PARTY.md` around lines 77 - 84, The fenced code blocks showing examples for scripts.registry.plausibleAnalytics lack language specifiers; update both blocks to include a language (e.g., add "ts" after the opening ```), so the examples "scripts.registry.plausibleAnalytics = { domain: 'mysite.com', firstParty: false }" and "scripts.registry.plausibleAnalytics = [{ domain: 'mysite.com' }, { firstParty: false }]" are fenced as ```ts to satisfy markdownlint MD040 and enable syntax highlighting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playground/plugins/error-overlay.client.ts`:
- Around line 86-90: safeStringify currently JSON.stringify's Error objects into
{} because Error.message and Error.stack are non-enumerable; update
safeStringify to first check if the input is an instance of Error (or has
Error-like shape) and return a stringified representation that includes
error.message and error.stack (e.g., combine message and stack or build an
object with these properties) before falling back to JSON.stringify/String(a);
reference the safeStringify function to locate where to add the instanceof Error
extraction and ensure the final return remains a string.
- Around line 133-149: Update the MutationObserver usage (observer) to also
observe attribute changes by passing { childList: true, subtree: true,
attributes: true, attributeFilter: ['src','href'] } and extend the observer
callback to handle mutation.type === 'attributes' by calling checkElement on
mutation.target (or the element whose src/href changed) and/or reading the new
attribute value and passing it to reportIfThirdParty; keep existing childList
handling intact and use the existing checkElement and reportIfThirdParty helpers
to perform the same validation for attribute mutations.
In `@src/first-party/intercept-plugin.ts`:
- Around line 6-8: The server proxy currently allows fetching any host even when
privacy filtering is applied, creating an open relay; update the proxy handler
that implements __nuxtScripts.fetch to perform an explicit allowlist check
against domainPrivacy (or its helper) before proceeding to proxyPrefix logic and
return an HTTP 403/error for disallowed domains so requests are never forwarded
to hosts not listed in domainPrivacy.
In `@src/module.ts`:
- Around line 517-520: The current guard only checks config.partytown?.length
and misses registry-enabled Partytown flags; update the conditional around
hasNuxtModule('@nuxtjs/partytown') to also detect registry-level Partytown by
checking your registry entries for scriptOptions.partytown === true (or truthy)
before setting (nuxt.options as any).partytown.resolveUrl via
generatePartytownResolveUrl(proxyPrefix); specifically, replace the single
config.partytown?.length check with a predicate that is true if config.partytown
has entries OR any registry entry has scriptOptions.partytown truthy, then set
partytownConfig.resolveUrl as before.
In `@src/runtime/server/proxy-handler.ts`:
- Around line 87-101: The handler currently defaults to proxying even when a
domain has no entry in domainPrivacy (perScriptInput === undefined), enabling
SSRF; change the logic in proxy-handler.ts so that if perScriptInput is
undefined you reject the request immediately (e.g., return a 403/400 Response or
throw an error) rather than proceeding to build targetBase or proxying; update
the block that checks perScriptInput (and any later use of targetBase) to
enforce this allowlist check and log the rejection when debug is true.
---
Nitpick comments:
In `@FIRST_PARTY.md`:
- Around line 77-84: The fenced code blocks showing examples for
scripts.registry.plausibleAnalytics lack language specifiers; update both blocks
to include a language (e.g., add "ts" after the opening ```), so the examples
"scripts.registry.plausibleAnalytics = { domain: 'mysite.com', firstParty: false
}" and "scripts.registry.plausibleAnalytics = [{ domain: 'mysite.com' }, {
firstParty: false }]" are fenced as ```ts to satisfy markdownlint MD040 and
enable syntax highlighting.
In `@src/first-party/proxy-configs.ts`:
- Line 30: The parameter `_proxyPrefix` on function buildProxyConfig is unused —
remove it from the buildProxyConfig signature and update any callers to stop
passing that argument, or if you intentionally reserve it for future use, keep
the parameter but add a one-line comment above buildProxyConfig explaining it is
retained intentionally for future API compatibility (and that actual runtime
values come from autoInject.computeValue callbacks which receive proxyPrefix).
Ensure references to buildProxyConfig and the autoInject.computeValue usage
remain consistent after the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef75c7c0-42e1-4799-bb61-a3f586597abb
📒 Files selected for processing (31)
FIRST_PARTY.mddocs/content/docs/1.guides/2.first-party.mdplayground/nuxt.config.tsplayground/pages/third-parties/clarity/nuxt-scripts.vueplayground/pages/third-parties/google-analytics/datalayers.vueplayground/pages/third-parties/google-analytics/nuxt-scripts.vueplayground/pages/third-parties/google-maps/center.vueplayground/pages/third-parties/hotjar.vueplayground/pages/third-parties/intercom/use-script.vueplayground/pages/third-parties/lemon-squeezy/script.vueplayground/pages/third-parties/meta-pixel.vueplayground/pages/third-parties/plausible-analytics.vueplayground/pages/third-parties/reddit-pixel/nuxt-scripts.vueplayground/pages/third-parties/snapchat/nuxt-scripts.vueplayground/pages/third-parties/vercel-analytics/nuxt-scripts.vueplayground/pages/third-parties/x-pixel/nuxt-scripts.vueplayground/plugins/error-overlay.client.tssrc/first-party/index.tssrc/first-party/intercept-plugin.tssrc/first-party/partytown-resolve.tssrc/first-party/proxy-configs.tssrc/first-party/setup.tssrc/first-party/types.tssrc/module.tssrc/plugins/transform.tssrc/registry.tssrc/runtime/server/proxy-handler.tstest/unit/auto-inject.test.tstest/unit/first-party.test.tstest/unit/gravatar-proxy.test.tstest/unit/proxy-configs.test.ts
Comment on lines
+133
to
+149
| // observe DOM for script/img/link/iframe elements with third-party src | ||
| const observer = new MutationObserver((mutations) => { | ||
| for (const mutation of mutations) { | ||
| for (const node of mutation.addedNodes) { | ||
| if (node.nodeType !== Node.ELEMENT_NODE) continue | ||
| const el = node as Element | ||
| checkElement(el) | ||
| for (const child of el.querySelectorAll('[src],[href]')) | ||
| checkElement(child) | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| function checkElement(el: Element) { | ||
| const src = el.getAttribute('src') || (el.tagName === 'LINK' ? el.getAttribute('href') : '') | ||
| if (src) reportIfThirdParty(src) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
With MutationObserver, does observing { childList: true, subtree: true } detect later src/href changes on existing elements, or do you need { attributes: true, attributeFilter: ["src","href"] } to catch them?
💡 Result:
No, observing { childList: true, subtree: true } does NOT detect src or href changes on existing elements. You need { attributes: true, attributeFilter: ["src", "href"] } (or { attributes: true } without filter) to catch attribute changes on existing nodes. childList only monitors addition/removal of child nodes, not attribute modifications on existing nodes. subtree extends childList monitoring to descendants but does not add attribute watching. Attribute changes produce mutation records of type "attributes", which require the attributes option. Examples from sources: - To detect img src change: observer.observe(img, {attributes: true}) [1st search result 1] - MDN: { childList: true } monitors "addition of new child nodes or removal of existing child nodes" [1st search result 6, 2nd search result 8] - MDN example checks if (mutation.type === "attributes") separately from "childList" [1st search result 7] - javascript.info: childList for "changes in the direct children", attributes for "attributes of node" [1st search result 9] - StackOverflow confirms attributes: true needed for attribute changes like src [2nd search result 16]
Citations:
- 1: https://stackoverflow.com/questions/10655531/detect-a-img-src-change
- 2: https://stackoverflow.com/questions/10560356/mutation-observers-subtree
- 3: https://stackoverflow.com/questions/44557417/change-img-src-using-mutationobserver-without-recursion
- 4: https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver/observe
- 5: https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver
- 6: https://developer.mozilla.org/docs/Web/API/MutationObserver/observe
- 7: https://javascript.info/mutation-observer
- 8: https://stackoverflow.com/questions/30946829/mutationobserver-not-working/30947010
- 9: https://udn.realityripple.com/docs/Web/API/MutationObserverInit
- 10: https://stackoverflow.com/questions/41424989/javascript-listen-for-attribute-change/41425087
- 11: https://developer.mozilla.org/en-US/docs/Web/API/MutationRecord/type
🏁 Script executed:
cat -n playground/plugins/error-overlay.client.ts | sed -n '130,170p'Repository: nuxt/scripts
Length of output: 1683
Observe src/href attribute mutations as well as inserted nodes.
The observer at line 164 subscribes only to childList, so elements appended first and assigned src/href afterward will bypass detection. This causes false negatives in third-party request checks.
Add attributes: true with attributeFilter: ['src', 'href'] to the observer options, and handle attribute mutations in the callback:
Suggested fix
const observer = new MutationObserver((mutations) => {
for (const mutation of mutations) {
+ if (mutation.type === 'attributes') {
+ checkElement(mutation.target as Element)
+ continue
+ }
for (const node of mutation.addedNodes) {
if (node.nodeType !== Node.ELEMENT_NODE) continue
const el = node as Element
checkElement(el)
for (const child of el.querySelectorAll('[src],[href]'))
checkElement(child)
}
}
})
...
- observer.observe(document.documentElement, { childList: true, subtree: true })
+ observer.observe(document.documentElement, {
+ childList: true,
+ subtree: true,
+ attributes: true,
+ attributeFilter: ['src', 'href'],
+ })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/plugins/error-overlay.client.ts` around lines 133 - 149, Update
the MutationObserver usage (observer) to also observe attribute changes by
passing { childList: true, subtree: true, attributes: true, attributeFilter:
['src','href'] } and extend the observer callback to handle mutation.type ===
'attributes' by calling checkElement on mutation.target (or the element whose
src/href changed) and/or reading the new attribute value and passing it to
reportIfThirdParty; keep existing childList handling intact and use the existing
checkElement and reportIfThirdParty helpers to perform the same validation for
attribute mutations.
Comment on lines
+517
to
+520
| if (config.partytown?.length && hasNuxtModule('@nuxtjs/partytown')) { | ||
| const partytownConfig = (nuxt.options as any).partytown || {} | ||
| if (!partytownConfig.resolveUrl) { | ||
| partytownConfig.resolveUrl = generatePartytownResolveUrl(interceptRules) | ||
| partytownConfig.resolveUrl = generatePartytownResolveUrl(proxyPrefix) |
There was a problem hiding this comment.
Auto resolveUrl check misses registry-level Partytown scripts.
Line 517 only checks config.partytown?.length. If users enable Partytown per script via registry scriptOptions.partytown = true (without shorthand), Line 520 never runs and first-party rewriting in the worker is skipped.
🔧 Proposed fix
- if (config.partytown?.length && hasNuxtModule('@nuxtjs/partytown')) {
+ const hasPartytownScripts = !!config.partytown?.length
+ || Object.values(config.registry || {}).some((entry: any) =>
+ Array.isArray(entry) && !!entry[1]?.partytown)
+ if (hasPartytownScripts && hasNuxtModule('@nuxtjs/partytown')) {
const partytownConfig = (nuxt.options as any).partytown || {}
if (!partytownConfig.resolveUrl) {
partytownConfig.resolveUrl = generatePartytownResolveUrl(proxyPrefix)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/module.ts` around lines 517 - 520, The current guard only checks
config.partytown?.length and misses registry-enabled Partytown flags; update the
conditional around hasNuxtModule('@nuxtjs/partytown') to also detect
registry-level Partytown by checking your registry entries for
scriptOptions.partytown === true (or truthy) before setting (nuxt.options as
any).partytown.resolveUrl via generatePartytownResolveUrl(proxyPrefix);
specifically, replace the single config.partytown?.length check with a predicate
that is true if config.partytown has entries OR any registry entry has
scriptOptions.partytown truthy, then set partytownConfig.resolveUrl as before.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/stats.ts (1)
520-522: Deduplicate extracted domains before sorting.Line [521] sorts but does not dedupe, so repeated domains in config can inflate
proxyEndpointsand duplicateproxyDomainsin stats.♻️ Proposed change
function extractDomains(proxyDomains: string[]): string[] { - return [...proxyDomains].sort() + return [...new Set(proxyDomains)].sort() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stats.ts` around lines 520 - 522, The extractDomains function currently returns a sorted copy of proxyDomains but doesn't remove duplicates; update extractDomains to deduplicate entries before sorting (e.g., convert proxyDomains to a Set or use Array.from(new Set(...)) then sort) so proxyEndpoints and proxyDomains in the stats logic no longer contain repeated domains; ensure the function name extractDomains is modified in-place and tested to produce a unique, alphabetically-sorted array.test/unit/rewrite-ast.test.ts (1)
6-8: Consider extracting the sharedderiveRewriteshelper to a test utility.This helper is duplicated in both
rewrite-ast.test.tsandthird-party-proxy-replacements.test.ts. Extracting it to a shared test utility (e.g.,test/utils/proxy-test-helpers.ts) would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/rewrite-ast.test.ts` around lines 6 - 8, Extract the duplicated deriveRewrites helper into a shared test utility by creating a new module (e.g., test/utils/proxy-test-helpers.ts) that exports the function deriveRewrites(domains: string[], proxyPrefix: string): ProxyRewrite[] with the same signature and return shape; remove the local deriveRewrites definitions from rewrite-ast.test.ts and third-party-proxy-replacements.test.ts and replace them with imports from the new utility, and ensure any required types (ProxyRewrite) are imported or re-exported from the utility so both tests compile without other changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/stats.ts`:
- Around line 520-522: The extractDomains function currently returns a sorted
copy of proxyDomains but doesn't remove duplicates; update extractDomains to
deduplicate entries before sorting (e.g., convert proxyDomains to a Set or use
Array.from(new Set(...)) then sort) so proxyEndpoints and proxyDomains in the
stats logic no longer contain repeated domains; ensure the function name
extractDomains is modified in-place and tested to produce a unique,
alphabetically-sorted array.
In `@test/unit/rewrite-ast.test.ts`:
- Around line 6-8: Extract the duplicated deriveRewrites helper into a shared
test utility by creating a new module (e.g., test/utils/proxy-test-helpers.ts)
that exports the function deriveRewrites(domains: string[], proxyPrefix:
string): ProxyRewrite[] with the same signature and return shape; remove the
local deriveRewrites definitions from rewrite-ast.test.ts and
third-party-proxy-replacements.test.ts and replace them with imports from the
new utility, and ensure any required types (ProxyRewrite) are imported or
re-exported from the utility so both tests compile without other changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4d2d08c-d642-4ffc-9e14-7964373ab8b4
📒 Files selected for processing (3)
src/stats.tstest/unit/rewrite-ast.test.tstest/unit/third-party-proxy-replacements.test.ts
The proxy handler previously forwarded requests to any domain, even those not in the domainPrivacy allowlist. Now returns 403 for domains not registered via proxy configs, preventing use as an open relay. Also fix Error serialization in playground error overlay.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
playground/plugins/error-overlay.client.ts (1)
135-145:⚠️ Potential issue | 🟠 MajorStill missing attribute mutation tracking for
src/hrefchanges after insertion.Line 165 only observes
childList, so elements appended first and updated later (el.src = ...) are not detected. This is still the same unresolved issue previously reported.Suggested fix
const observer = new MutationObserver((mutations) => { for (const mutation of mutations) { + if (mutation.type === 'attributes') { + checkElement(mutation.target as Element) + continue + } for (const node of mutation.addedNodes) { if (node.nodeType !== Node.ELEMENT_NODE) continue const el = node as Element checkElement(el) for (const child of el.querySelectorAll('[src],[href]')) checkElement(child) } } }) @@ - observer.observe(document.documentElement, { childList: true, subtree: true }) + observer.observe(document.documentElement, { + childList: true, + subtree: true, + attributes: true, + attributeFilter: ['src', 'href'], + })#!/bin/bash # Verify whether attribute mutation handling exists for src/href. rg -n "MutationObserver|observer\\.observe|mutation\\.type === 'attributes'|attributeFilter" playground/plugins/error-overlay.client.tsAlso applies to: 165-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/plugins/error-overlay.client.ts` around lines 135 - 145, The MutationObserver callback and/or its observe call currently only handles childList mutations so later attribute updates (el.src/href after insertion) are missed; update the observer.observe options to include attributes: true and attributeFilter: ['src','href'] (and keep childList/subtree:true), and extend the MutationObserver callback (the function referenced as observer) to handle mutation.type === 'attributes' by calling checkElement(mutation.target as Element) for each attribute mutation; keep the existing addedNodes handling but add this attributes branch so src/href changes after insertion are detected.
🧹 Nitpick comments (1)
src/runtime/server/proxy-handler.ts (1)
109-110: Unreachable fallback and misleading comment.The
?? truefallback on line 110 is now unreachable since lines 96-103 throw a 403 error whenperScriptInput === undefined. The comment on line 109 ("missing domain → full anonymization") is also inconsistent with the current control flow where missing domains are rejected, not anonymized.🧹 Suggested cleanup
- // Resolve effective privacy: per-script is the base, global user override on top - // Fail-closed: missing domain → full anonymization (most restrictive) - const perScriptResolved = resolvePrivacy(perScriptInput ?? true) + // Resolve effective privacy: per-script is the base, global user override on top + const perScriptResolved = resolvePrivacy(perScriptInput)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/proxy-handler.ts` around lines 109 - 110, The fallback "?? true" and the comment about "missing domain → full anonymization" are inconsistent because perScriptInput is already rejected earlier; remove the unreachable fallback and update the comment to reflect that missing domains are rejected, or if you want anonymization instead, move the fallback logic before the 403 throw and call resolvePrivacy(perScriptInput ?? true) there; locate the perScriptInput check that throws 403 and the subsequent const perScriptResolved = resolvePrivacy(perScriptInput ?? true) (symbols: perScriptInput, resolvePrivacy, perScriptResolved) and either delete the "?? true" and change the comment to note rejection, or shift the fallback so anonymization happens before the authorization check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playground/plugins/error-overlay.client.ts`:
- Around line 70-76: The tooltip content is only built when the overlay click
handler runs, so it becomes stale while visible; refactor the tooltip text
construction into a reusable helper (e.g., updateTooltipContent or
buildTooltipText) that computes lines from the errors and warnings arrays and
sets tooltip.textContent (and tooltip.style.display as needed), then call that
helper both from the overlay click listener (currently building the text) and
from render() (the code that updates counters at render) so the tooltip is
refreshed whenever new events arrive while it is visible.
---
Duplicate comments:
In `@playground/plugins/error-overlay.client.ts`:
- Around line 135-145: The MutationObserver callback and/or its observe call
currently only handles childList mutations so later attribute updates
(el.src/href after insertion) are missed; update the observer.observe options to
include attributes: true and attributeFilter: ['src','href'] (and keep
childList/subtree:true), and extend the MutationObserver callback (the function
referenced as observer) to handle mutation.type === 'attributes' by calling
checkElement(mutation.target as Element) for each attribute mutation; keep the
existing addedNodes handling but add this attributes branch so src/href changes
after insertion are detected.
---
Nitpick comments:
In `@src/runtime/server/proxy-handler.ts`:
- Around line 109-110: The fallback "?? true" and the comment about "missing
domain → full anonymization" are inconsistent because perScriptInput is already
rejected earlier; remove the unreachable fallback and update the comment to
reflect that missing domains are rejected, or if you want anonymization instead,
move the fallback logic before the 403 throw and call
resolvePrivacy(perScriptInput ?? true) there; locate the perScriptInput check
that throws 403 and the subsequent const perScriptResolved =
resolvePrivacy(perScriptInput ?? true) (symbols: perScriptInput, resolvePrivacy,
perScriptResolved) and either delete the "?? true" and change the comment to
note rejection, or shift the fallback so anonymization happens before the
authorization check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccb77ef0-d4c1-45d6-87cd-f53ecf1e53ac
📒 Files selected for processing (2)
playground/plugins/error-overlay.client.tssrc/runtime/server/proxy-handler.ts
Comment on lines
+70
to
+76
| overlay.addEventListener('click', () => { | ||
| if (tooltip.style.display === 'none') { | ||
| const lines: string[] = [] | ||
| for (const msg of errors) lines.push(`[ERR] ${msg}`) | ||
| for (const msg of warnings) lines.push(`[WARN] ${msg}`) | ||
| tooltip.textContent = lines.length > 0 ? lines.join('\n') : 'No issues' | ||
| tooltip.style.display = 'block' |
There was a problem hiding this comment.
Tooltip content becomes stale while it is open.
While render() updates counters on new events (Lines 93-113), tooltip text is only rebuilt when opening (Lines 70-76). If the tooltip is visible, fresh issues are not shown until it is closed/reopened.
Suggested fix
+ function renderTooltip() {
+ const lines: string[] = []
+ for (const msg of errors) lines.push(`[ERR] ${msg}`)
+ for (const msg of warnings) lines.push(`[WARN] ${msg}`)
+ tooltip.textContent = lines.length > 0 ? lines.join('\n') : 'No issues'
+ }
+
function render() {
@@
overlay.style.boxShadow = '0 2px 8px rgba(0,0,0,0.3)'
overlay.textContent = parts.join(' | ')
+ if (tooltip.style.display !== 'none')
+ renderTooltip()
}
@@
overlay.addEventListener('click', () => {
if (tooltip.style.display === 'none') {
- const lines: string[] = []
- for (const msg of errors) lines.push(`[ERR] ${msg}`)
- for (const msg of warnings) lines.push(`[WARN] ${msg}`)
- tooltip.textContent = lines.length > 0 ? lines.join('\n') : 'No issues'
+ renderTooltip()
tooltip.style.display = 'block'
}Also applies to: 93-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playground/plugins/error-overlay.client.ts` around lines 70 - 76, The tooltip
content is only built when the overlay click handler runs, so it becomes stale
while visible; refactor the tooltip text construction into a reusable helper
(e.g., updateTooltipContent or buildTooltipText) that computes lines from the
errors and warnings arrays and sets tooltip.textContent (and
tooltip.style.display as needed), then call that helper both from the overlay
click listener (currently building the text) and from render() (the code that
updates counters at render) so the tooltip is refreshed whenever new events
arrive while it is visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
❓ Type of change
📚 Description
Scripts requiring browser fingerprinting for fraud/bot detection (Stripe, PayPal, Google reCAPTCHA, Google Sign-In) were being included in first-party mode, which could strip fingerprinting data and break their core functionality. These now set
proxy: falseandscriptBundling: falseto exclude them entirely.Per-script
firstParty: falseopt-out in registry config was not respected byautoInject, causing Plausible (and other scripts with auto-inject) to still route through the proxy even when opted out. BothapplyAutoInjectandfinalizeFirstPartynow check forfirstParty: falsebefore injecting proxy endpoints or registering domains.Changes
Fingerprinting script exclusion:
proxy: false+scriptBundling: falsein registryproxy-configs.tsPer-script firstParty opt-out fix:
applyAutoInjectnow checksfirstParty: falsein entry input and scriptOptionsfinalizeFirstPartyskips domain registration for opted-out entriesplausibleAnalytics: { domain: 'mysite.com', firstParty: false }Docs:
FIRST_PARTY.mdarchitecture referencedocs/content/docs/1.guides/2.first-party.mduser guideTests:
Playground fixes:
$windowreference in vercel-analytics pagesetupGoogleMapshandler in google-maps/center page