feat: endpoint opt-in, path standardization, proxy consolidation#644
Merged
feat: endpoint opt-in, path standardization, proxy consolidation#644
Conversation
…e-origin enforcement - Registry scripts with server handlers (bluesky, x, instagram, gravatar, google maps) now require explicit opt-in via `scripts.registry.<key>: true` in nuxt.config - Components throw clear errors via `requireRegistryEndpoint()` when not enabled - Standardize all endpoint paths from `/api/_scripts/` to `/_scripts/` - Unify first-party collection prefix from `/_proxy` to `/_scripts/c` - Consolidate 4 image proxy handlers into `createImageProxyHandler()` factory - Add `validateSameOrigin()` to all proxy/embed endpoints and collection handler - Add `serverHandlers` field to `RegistryScript` type for generic handler registration - Replace ~60 lines of manual handler registration in module.ts with registry-driven loop - Add Google Maps geocode proxy (`/_scripts/google-maps-geocode-proxy`) - Wire geocode proxy into ScriptGoogleMaps.vue for server-side address resolution
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR standardizes embed API paths from Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
📝 Coding Plan
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/registry/bluesky-embed.ts (1)
81-85:⚠️ Potential issue | 🔴 CriticalFix the capture-group typing before merge.
Line 85 returns
match[1]andmatch[2]directly, which TypeScript types asstring | undefineddue to indexed access onRegExpMatchArray. The return type requires bothactorandrkeyto bestring, causing a TS2322 mismatch.Suggested fix
export function extractBlueskyPostId(url: string): { actor: string, rkey: string } | undefined { const match = url.match(BSKY_POST_URL_RE) if (!match) return undefined - return { actor: match[1], rkey: match[2] } + const [, actor, rkey] = match + if (!actor || !rkey) + return undefined + return { actor, rkey } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/registry/bluesky-embed.ts` around lines 81 - 85, The function extractBlueskyPostId currently returns match[1] and match[2] which are typed string | undefined; after the existing null-check on match, narrow the group types before returning (e.g., assert or validate that match[1] and match[2] are present) so the returned actor and rkey are plain strings. Update the return to use a non-null assertion or explicit check/cast for match[1] and match[2] (referencing extractBlueskyPostId, the local match variable, and BSKY_POST_URL_RE) so TypeScript no longer reports TS2322.
🧹 Nitpick comments (3)
src/runtime/server/instagram-embed-image.ts (1)
4-5: Redundant domain check.The condition
hostname === 'scontent.cdninstagram.com'is redundant since'scontent.cdninstagram.com'.endsWith('.cdninstagram.com')is alreadytrue. The first condition alone handles all cases.💡 Simplified predicate
- allowedDomains: hostname => - hostname.endsWith('.cdninstagram.com') || hostname === 'scontent.cdninstagram.com', + allowedDomains: hostname => hostname.endsWith('.cdninstagram.com'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/instagram-embed-image.ts` around lines 4 - 5, The predicate in allowedDomains is redundant: remove the explicit check hostname === 'scontent.cdninstagram.com' and keep only the endsWith('.cdninstagram.com') check; update the allowedDomains arrow function (the hostname parameter) to return hostname.endsWith('.cdninstagram.com') so the logic remains correct and simpler.src/runtime/server/utils/image-proxy.ts (1)
96-98: Consider adding aVaryheader for consistency.Other proxy handlers in this PR (e.g.,
gravatar-proxy.ts,google-static-maps-proxy.ts) setVary: Accept-Encoding. While not critical for image proxies, adding it would ensure consistent caching behavior across CDNs and proxies.💡 Optional improvement
setHeader(event, 'Content-Type', response.headers.get('content-type') || contentType) setHeader(event, 'Cache-Control', `public, max-age=${cacheMaxAge}, s-maxage=${cacheMaxAge}`) +setHeader(event, 'Vary', 'Accept-Encoding')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/utils/image-proxy.ts` around lines 96 - 98, The response from image proxy handlers (inside image-proxy.ts where setHeader(event, 'Content-Type', ...), setHeader(event, 'Cache-Control', ...), using variables response, contentType, cacheMaxAge) should also include a Vary header for consistency with other proxies; add setHeader(event, 'Vary', 'Accept-Encoding') after the existing header calls so CDNs and intermediate caches honor different encodings and match behavior in gravatar-proxy.ts and google-static-maps-proxy.ts.test/e2e/basic.test.ts (1)
491-492: These assertions don't prove the image proxies still return 200.
img.src.includes(...)andgetAttribute('src')?.includes(...)only verify URL rewriting. If the new same-origin guard starts returning403for browser image requests, all of these tests still pass because the attribute is set before the fetch completes. Assertimg.completewith a non-zeronaturalWidth(or observe the proxy responses) so this suite actually covers the new proxy behavior.Also applies to: 532-533, 567-576
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/basic.test.ts` around lines 491 - 492, The current assertion using [...imgs].every(img => img.src.includes('/_scripts/x-embed-image')) only checks URL rewriting, not that the proxy actually serves images; update the tests that use the imgs collection (the predicate using img.src.includes(...)) to also assert the image loaded successfully by checking either img.complete && img.naturalWidth > 0 for each img or performing a fetch(img.src) and asserting response.status === 200; apply the same change to the other similar checks around the file (the assertions at the locations referenced in the review: the blocks near lines 532-533 and 567-576) so the suite verifies real proxy responses rather than just rewritten attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue`:
- Around line 252-266: The cache currently mixes google.maps.LatLng and plain
{lat,lng} (queryToLatLngCache is Map<string, google.maps.LatLng>) which is why
the proxy branch casts to any; fix by making the cache and function types accept
both shapes (e.g., Map<string, google.maps.LatLng | google.maps.LatLngLiteral>
and adjust the function return type) so you can store the proxy result without
as any, or if you prefer a single concrete type, convert the proxy result to a
google.maps.LatLng instance before caching/returning (create new
google.maps.LatLng(lat, lng) when google.maps is available); update references
to queryToLatLngCache, the proxy $fetch call to
'/_scripts/google-maps-geocode-proxy', and any consumers (e.g., setCenter) to
accept the widened union type or handle both forms.
In `@src/runtime/components/ScriptBlueskyEmbed.vue`:
- Around line 29-33: The unconditional call
requireRegistryEndpoint('ScriptBlueskyEmbed', 'blueskyEmbed') prevents use of
fully custom endpoints; change the guard so it only runs when one of the default
built-in endpoints is being used (e.g., check if apiEndpoint starts with
'/_scripts/' or imageProxyEndpoint starts with '/_scripts/' before calling
requireRegistryEndpoint). Update the logic around the apiEndpoint and
imageProxyEndpoint defaults and call
requireRegistryEndpoint('ScriptBlueskyEmbed', 'blueskyEmbed') only when those
values indicate the built-in '/_scripts/*' endpoints are still selected.
In `@src/runtime/components/ScriptInstagramEmbed.vue`:
- Around line 28-32: The component always calls
requireRegistryEndpoint('ScriptInstagramEmbed', 'instagramEmbed') even when a
caller passed a custom apiEndpoint; change this so the registry opt-in check
runs only for the default internal route—i.e., inspect the component prop/option
apiEndpoint (or whatever local variable holds the endpoint) and only invoke
requireRegistryEndpoint('ScriptInstagramEmbed', 'instagramEmbed') when
apiEndpoint is exactly '/_scripts/instagram-embed' (or unset), otherwise skip
the call so custom backends are allowed.
In `@src/runtime/server/utils/image-proxy.ts`:
- Around line 78-94: Update the $fetch.raw call in image-proxy.ts to include
ignoreResponseError: true so 3xx responses aren't thrown as errors;
specifically, when invoking $fetch.raw(url, { timeout: 5000, redirect:
followRedirects ? 'follow' : 'manual', headers }), add ignoreResponseError: true
so the returned response (used later in the redirect check that throws
createError with statusCode 403) can be inspected when followRedirects is false.
---
Outside diff comments:
In `@src/runtime/registry/bluesky-embed.ts`:
- Around line 81-85: The function extractBlueskyPostId currently returns
match[1] and match[2] which are typed string | undefined; after the existing
null-check on match, narrow the group types before returning (e.g., assert or
validate that match[1] and match[2] are present) so the returned actor and rkey
are plain strings. Update the return to use a non-null assertion or explicit
check/cast for match[1] and match[2] (referencing extractBlueskyPostId, the
local match variable, and BSKY_POST_URL_RE) so TypeScript no longer reports
TS2322.
---
Nitpick comments:
In `@src/runtime/server/instagram-embed-image.ts`:
- Around line 4-5: The predicate in allowedDomains is redundant: remove the
explicit check hostname === 'scontent.cdninstagram.com' and keep only the
endsWith('.cdninstagram.com') check; update the allowedDomains arrow function
(the hostname parameter) to return hostname.endsWith('.cdninstagram.com') so the
logic remains correct and simpler.
In `@src/runtime/server/utils/image-proxy.ts`:
- Around line 96-98: The response from image proxy handlers (inside
image-proxy.ts where setHeader(event, 'Content-Type', ...), setHeader(event,
'Cache-Control', ...), using variables response, contentType, cacheMaxAge)
should also include a Vary header for consistency with other proxies; add
setHeader(event, 'Vary', 'Accept-Encoding') after the existing header calls so
CDNs and intermediate caches honor different encodings and match behavior in
gravatar-proxy.ts and google-static-maps-proxy.ts.
In `@test/e2e/basic.test.ts`:
- Around line 491-492: The current assertion using [...imgs].every(img =>
img.src.includes('/_scripts/x-embed-image')) only checks URL rewriting, not that
the proxy actually serves images; update the tests that use the imgs collection
(the predicate using img.src.includes(...)) to also assert the image loaded
successfully by checking either img.complete && img.naturalWidth > 0 for each
img or performing a fetch(img.src) and asserting response.status === 200; apply
the same change to the other similar checks around the file (the assertions at
the locations referenced in the review: the blocks near lines 532-533 and
567-576) so the suite verifies real proxy responses rather than just rewritten
attributes.
🪄 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: 658d54e3-58b9-47ef-944f-a3ba8a9dcddf
📒 Files selected for processing (31)
docs/content/scripts/bluesky-embed.mddocs/content/scripts/x-embed.mdsrc/first-party/setup.tssrc/first-party/types.tssrc/module.tssrc/registry-types.jsonsrc/registry.tssrc/runtime/components/GoogleMaps/ScriptGoogleMaps.vuesrc/runtime/components/ScriptBlueskyEmbed.vuesrc/runtime/components/ScriptInstagramEmbed.vuesrc/runtime/components/ScriptXEmbed.vuesrc/runtime/registry/bluesky-embed.tssrc/runtime/registry/schemas.tssrc/runtime/registry/x-embed.tssrc/runtime/server/bluesky-embed-image.tssrc/runtime/server/bluesky-embed.tssrc/runtime/server/google-maps-geocode-proxy.tssrc/runtime/server/google-static-maps-proxy.tssrc/runtime/server/gravatar-proxy.tssrc/runtime/server/instagram-embed-asset.tssrc/runtime/server/instagram-embed-image.tssrc/runtime/server/instagram-embed.tssrc/runtime/server/proxy-handler.tssrc/runtime/server/utils/image-proxy.tssrc/runtime/server/utils/same-origin.tssrc/runtime/server/x-embed-image.tssrc/runtime/server/x-embed.tssrc/runtime/types.tssrc/runtime/utils.tstest/e2e/basic.test.tstest/unit/instagram-embed.test.ts
- Add `ignoreResponseError` to image proxy when redirects disabled - Make `requireRegistryEndpoint` conditional on default endpoint usage - Fix cache type to `LatLng | LatLngLiteral`, remove `as any` cast - Fix strict null assertions in bluesky-embed registry/server - Fix null check in first-party test
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/runtime/components/ScriptBlueskyEmbed.vue (1)
33-34:⚠️ Potential issue | 🟠 MajorRegistry guard still misses default image-proxy usage.
On Line 33, the check only considers
apiEndpoint. IfapiEndpointis custom butimageProxyEndpointremains/_scripts/bluesky-embed-image, the component still relies on registry handlers but skipsrequireRegistryEndpoint, leading to delayed runtime failures (broken proxied images) instead of an immediate config error.Suggested fix
+const defaultApiEndpoint = '/_scripts/bluesky-embed' +const defaultImageProxyEndpoint = '/_scripts/bluesky-embed-image' + const props = withDefaults(defineProps<{ // ... }>(), { - apiEndpoint: '/_scripts/bluesky-embed', - imageProxyEndpoint: '/_scripts/bluesky-embed-image', + apiEndpoint: defaultApiEndpoint, + imageProxyEndpoint: defaultImageProxyEndpoint, }) -if (!props.apiEndpoint || props.apiEndpoint === '/_scripts/bluesky-embed') +if (props.apiEndpoint === defaultApiEndpoint || props.imageProxyEndpoint === defaultImageProxyEndpoint) requireRegistryEndpoint('ScriptBlueskyEmbed', 'blueskyEmbed')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/ScriptBlueskyEmbed.vue` around lines 33 - 34, The guard currently only checks props.apiEndpoint and skips requireRegistryEndpoint when a custom apiEndpoint is provided, but it misses the case where props.imageProxyEndpoint is still the default '/_scripts/bluesky-embed-image', causing broken proxied images; update the conditional that calls requireRegistryEndpoint('ScriptBlueskyEmbed', 'blueskyEmbed') to also trigger when props.imageProxyEndpoint is falsy or equals '/_scripts/bluesky-embed-image' (i.e., call requireRegistryEndpoint unless both apiEndpoint and imageProxyEndpoint are explicitly customized), so include checks for props.imageProxyEndpoint alongside props.apiEndpoint in the if expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue`:
- Around line 255-266: The current proxy branch in the geocoding flow (the
$fetch to '/_scripts/google-maps-geocode-proxy' that uses queryToLatLngCache)
throws on any non-'OK' response which prevents falling back to the client-side
Places lookup; change the logic in the function (the block that calls $fetch and
reads data.status/results) to treat a non-'OK' as a non-fatal miss (i.e., don’t
throw there), only return the lat/lng on status 'OK' and otherwise fall through
so the Places lookup runs, and only throw an error at the end if both the proxy
and the Places attempts fail. Ensure you still cache successful results via
queryToLatLngCache.set(query, latLng).
---
Duplicate comments:
In `@src/runtime/components/ScriptBlueskyEmbed.vue`:
- Around line 33-34: The guard currently only checks props.apiEndpoint and skips
requireRegistryEndpoint when a custom apiEndpoint is provided, but it misses the
case where props.imageProxyEndpoint is still the default
'/_scripts/bluesky-embed-image', causing broken proxied images; update the
conditional that calls requireRegistryEndpoint('ScriptBlueskyEmbed',
'blueskyEmbed') to also trigger when props.imageProxyEndpoint is falsy or equals
'/_scripts/bluesky-embed-image' (i.e., call requireRegistryEndpoint unless both
apiEndpoint and imageProxyEndpoint are explicitly customized), so include checks
for props.imageProxyEndpoint alongside props.apiEndpoint in the if expression.
🪄 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: 0026ae70-899a-4174-8900-782998d252fc
📒 Files selected for processing (8)
src/runtime/components/GoogleMaps/ScriptGoogleMaps.vuesrc/runtime/components/ScriptBlueskyEmbed.vuesrc/runtime/components/ScriptInstagramEmbed.vuesrc/runtime/components/ScriptXEmbed.vuesrc/runtime/registry/bluesky-embed.tssrc/runtime/server/bluesky-embed.tssrc/runtime/server/utils/image-proxy.tstest/e2e-dev/first-party.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/runtime/server/bluesky-embed.ts
- src/runtime/server/utils/image-proxy.ts
- src/runtime/components/ScriptInstagramEmbed.vue
- src/runtime/components/ScriptXEmbed.vue
Comment on lines
+255
to
+266
| if (endpoints?.googleMaps) { | ||
| const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/google-maps-geocode-proxy', { | ||
| params: { address: query }, | ||
| }) | ||
| if (data.status === 'OK' && data.results?.[0]?.geometry?.location) { | ||
| const loc = data.results[0].geometry.location | ||
| const latLng = { lat: loc.lat, lng: loc.lng } | ||
| queryToLatLngCache.set(query, latLng) | ||
| return latLng | ||
| } | ||
| throw new Error(`No location found for ${query}`) | ||
| } |
There was a problem hiding this comment.
Keep the Places lookup as a real fallback.
src/runtime/server/google-maps-geocode-proxy.ts already throws on upstream failures, and this branch throws on any non-OK geocode response, so once endpoints?.googleMaps is enabled the code never reaches the client-side Places path below. That makes the proxy a hard dependency instead of a best-effort optimization.
Suggested fix
if (endpoints?.googleMaps) {
- const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/google-maps-geocode-proxy', {
- params: { address: query },
- })
- if (data.status === 'OK' && data.results?.[0]?.geometry?.location) {
- const loc = data.results[0].geometry.location
- const latLng = { lat: loc.lat, lng: loc.lng }
- queryToLatLngCache.set(query, latLng)
- return latLng
- }
- throw new Error(`No location found for ${query}`)
+ try {
+ const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/google-maps-geocode-proxy', {
+ params: { address: query },
+ })
+ if (data.status === 'OK' && data.results?.[0]?.geometry?.location) {
+ const loc = data.results[0].geometry.location
+ const latLng = { lat: loc.lat, lng: loc.lng }
+ queryToLatLngCache.set(query, latLng)
+ return latLng
+ }
+ }
+ catch {
+ // Fall through to the existing client-side Places lookup.
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (endpoints?.googleMaps) { | |
| const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/google-maps-geocode-proxy', { | |
| params: { address: query }, | |
| }) | |
| if (data.status === 'OK' && data.results?.[0]?.geometry?.location) { | |
| const loc = data.results[0].geometry.location | |
| const latLng = { lat: loc.lat, lng: loc.lng } | |
| queryToLatLngCache.set(query, latLng) | |
| return latLng | |
| } | |
| throw new Error(`No location found for ${query}`) | |
| } | |
| if (endpoints?.googleMaps) { | |
| try { | |
| const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/google-maps-geocode-proxy', { | |
| params: { address: query }, | |
| }) | |
| if (data.status === 'OK' && data.results?.[0]?.geometry?.location) { | |
| const loc = data.results[0].geometry.location | |
| const latLng = { lat: loc.lat, lng: loc.lng } | |
| queryToLatLngCache.set(query, latLng) | |
| return latLng | |
| } | |
| } | |
| catch { | |
| // Fall through to the existing client-side Places lookup. | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue` around lines 255 -
266, The current proxy branch in the geocoding flow (the $fetch to
'/_scripts/google-maps-geocode-proxy' that uses queryToLatLngCache) throws on
any non-'OK' response which prevents falling back to the client-side Places
lookup; change the logic in the function (the block that calls $fetch and reads
data.status/results) to treat a non-'OK' as a non-fatal miss (i.e., don’t throw
there), only return the lat/lng on status 'OK' and otherwise fall through so the
Places lookup runs, and only throw an error at the end if both the proxy and the
Places attempts fail. Ensure you still cache successful results via
queryToLatLngCache.set(query, latLng).
The endpoint opt-in mechanism requires registry scripts to be explicitly enabled. Add xEmbed, instagramEmbed, blueskyEmbed to the basic test fixture config so server handlers are registered.
Each embed component (Bluesky, X, Instagram) and Google Maps now requires explicit enablement via `scripts.registry` in nuxt.config. Add Setup sections to all relevant docs and update migration guide. Also enable registry scripts in test fixture for e2e tests.
Browser CORS already prevents cross-origin reads without explicit Access-Control-Allow-Origin headers — no need for server-side Origin/Referer checks.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/server/google-static-maps-proxy.ts (1)
6-53:⚠️ Potential issue | 🟠 MajorAdd Referer validation and strict parameter allowlisting to prevent proxy abuse.
This handler lacks server-side controls that other Nuxt Scripts proxies (e.g., Gravatar) include. Without Referer validation and parameter allowlisting, the endpoint is vulnerable to direct HTTP requests from non-browser clients (curl, scripts, servers) consuming your Google Maps API quota.
Specific gaps:
- No
Referer/Hostheader validation to detect cross-site abuse- All query parameters are forwarded as-is (
safeQuery) without validating allowed keys/values/ranges- CORS provides no protection against non-browser clients or server-to-server requests
Since Google Maps API is metered and paid, this creates cost exposure and an abuse vector. Add:
- Referer validation (compare against request origin)
- Strict allowlist of permitted parameters (size, center, zoom, markers only; reject oversized images, excessive markers)
- Optional: rate limiting per IP or session
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/google-static-maps-proxy.ts` around lines 6 - 53, The handler (defineEventHandler) currently trusts all incoming requests and forwards every query param to Google (via withQuery) which enables quota abuse; add server-side Referer/Origin/Host validation, a strict parameter allowlist and value checks, and reject non-conforming requests. Specifically: inside the exported defineEventHandler, read request headers (referer/origin/host) and compare against configured allowedOrigins in runtime config (reject with createError 403 if mismatched); replace spreading of getQuery() into safeQuery by whitelisting only allowed keys (e.g., size, center, zoom, markers) and validate each value (max image dimensions, numeric zoom ranges, limit number/length of markers) rejecting requests that exceed limits; ensure the server-side apiKey is still injected and responses for rejected requests return a clear 4xx error; optionally add a simple rate-limit hook (per-IP) before forwarding to withQuery to mitigate automated abuse.
🧹 Nitpick comments (1)
src/runtime/server/google-static-maps-proxy.ts (1)
37-46: Consider disabling redirect following for SSRF hardening.The
$fetchcall follows redirects by default. While the target URL is hardcoded to Google's API, thecreateImageProxyHandlerutility includes an explicitfollowRedirectsoption for SSRF protection. For defense-in-depth, consider addingredirect: 'manual'orredirect: 'error'to reject unexpected redirects.🛡️ Optional hardening
const response = await $fetch.raw(googleMapsUrl, { + redirect: 'manual', headers: { 'User-Agent': 'Nuxt Scripts Google Static Maps Proxy', }, }).catch((error: any) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/server/google-static-maps-proxy.ts` around lines 37 - 46, The $fetch.raw call should disable automatic redirect following to harden against SSRF: add a redirect option (e.g. redirect: 'error' or 'manual') to the $fetch.raw options where googleMapsUrl is fetched, and handle redirect responses explicitly (for example, if response.status is in the 3xx range throw a createError with a 502/400 status and message) so the createImageProxyHandler flow rejects unexpected redirects; update the $fetch.raw invocation and its catch logic accordingly to reference googleMapsUrl, $fetch.raw, and the surrounding createImageProxyHandler usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/server/instagram-embed.ts`:
- Around line 308-311: The RSRC rewriting currently in the combinedCss.replace
using RSRC_RE builds the proxy URL inline and bypasses shared normalization;
change that replacement callback to call the existing proxyAssetUrl helper
instead of constructing `/_scripts/instagram-embed-asset?url=...` directly —
compute the original asset URL as
`https://static.cdninstagram.com/rsrc.php${path}`, pass it through proxyAssetUrl
(and still encode if proxyAssetUrl expects encoded input) so the resulting URL
uses the shared normalization/encoding logic; update the replacement in the
combinedCss.replace(RSRC_RE, ...) callback to return the proxyAssetUrl result.
---
Outside diff comments:
In `@src/runtime/server/google-static-maps-proxy.ts`:
- Around line 6-53: The handler (defineEventHandler) currently trusts all
incoming requests and forwards every query param to Google (via withQuery) which
enables quota abuse; add server-side Referer/Origin/Host validation, a strict
parameter allowlist and value checks, and reject non-conforming requests.
Specifically: inside the exported defineEventHandler, read request headers
(referer/origin/host) and compare against configured allowedOrigins in runtime
config (reject with createError 403 if mismatched); replace spreading of
getQuery() into safeQuery by whitelisting only allowed keys (e.g., size, center,
zoom, markers) and validate each value (max image dimensions, numeric zoom
ranges, limit number/length of markers) rejecting requests that exceed limits;
ensure the server-side apiKey is still injected and responses for rejected
requests return a clear 4xx error; optionally add a simple rate-limit hook
(per-IP) before forwarding to withQuery to mitigate automated abuse.
---
Nitpick comments:
In `@src/runtime/server/google-static-maps-proxy.ts`:
- Around line 37-46: The $fetch.raw call should disable automatic redirect
following to harden against SSRF: add a redirect option (e.g. redirect: 'error'
or 'manual') to the $fetch.raw options where googleMapsUrl is fetched, and
handle redirect responses explicitly (for example, if response.status is in the
3xx range throw a createError with a 502/400 status and message) so the
createImageProxyHandler flow rejects unexpected redirects; update the $fetch.raw
invocation and its catch logic accordingly to reference googleMapsUrl,
$fetch.raw, and the surrounding createImageProxyHandler usage.
🪄 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: efa4c992-7015-45c1-91f3-315187de0fd5
📒 Files selected for processing (6)
src/runtime/server/bluesky-embed.tssrc/runtime/server/google-maps-geocode-proxy.tssrc/runtime/server/google-static-maps-proxy.tssrc/runtime/server/gravatar-proxy.tssrc/runtime/server/instagram-embed.tssrc/runtime/server/utils/image-proxy.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/runtime/server/bluesky-embed.ts
- src/runtime/server/utils/image-proxy.ts
- src/runtime/server/google-maps-geocode-proxy.ts
Comment on lines
308
to
311
| combinedCss = combinedCss.replace( | ||
| RSRC_RE, | ||
| (_m, path) => `url(/api/_scripts/instagram-embed-asset?url=${encodeURIComponent(`https://static.cdninstagram.com/rsrc.php${path}`)})`, | ||
| (_m, path) => `url(/_scripts/instagram-embed-asset?url=${encodeURIComponent(`https://static.cdninstagram.com/rsrc.php${path}`)})`, | ||
| ) |
There was a problem hiding this comment.
Use proxyAssetUrl() for RSRC rewriting to avoid drift and normalize URLs consistently.
Line 310 duplicates asset-proxy URL construction instead of using the existing helper. This bypasses shared normalization (& handling) and can drift if endpoint paths change again.
Suggested fix
combinedCss = combinedCss.replace(
RSRC_RE,
- (_m, path) => `url(/_scripts/instagram-embed-asset?url=${encodeURIComponent(`https://static.cdninstagram.com/rsrc.php${path}`)})`,
+ (_m, path) => `url(${proxyAssetUrl(`https://static.cdninstagram.com/rsrc.php${path}`)})`,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| combinedCss = combinedCss.replace( | |
| RSRC_RE, | |
| (_m, path) => `url(/api/_scripts/instagram-embed-asset?url=${encodeURIComponent(`https://static.cdninstagram.com/rsrc.php${path}`)})`, | |
| (_m, path) => `url(/_scripts/instagram-embed-asset?url=${encodeURIComponent(`https://static.cdninstagram.com/rsrc.php${path}`)})`, | |
| ) | |
| combinedCss = combinedCss.replace( | |
| RSRC_RE, | |
| (_m, path) => `url(${proxyAssetUrl(`https://static.cdninstagram.com/rsrc.php${path}`)})`, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/server/instagram-embed.ts` around lines 308 - 311, The RSRC
rewriting currently in the combinedCss.replace using RSRC_RE builds the proxy
URL inline and bypasses shared normalization; change that replacement callback
to call the existing proxyAssetUrl helper instead of constructing
`/_scripts/instagram-embed-asset?url=...` directly — compute the original asset
URL as `https://static.cdninstagram.com/rsrc.php${path}`, pass it through
proxyAssetUrl (and still encode if proxyAssetUrl expects encoded input) so the
resulting URL uses the shared normalization/encoding logic; update the
replacement in the combinedCss.replace(RSRC_RE, ...) callback to return the
proxyAssetUrl result.
- /_scripts/assets/ — bundled/downloaded scripts - /_scripts/embed/ — social embed endpoints (bluesky, x, instagram) - /_scripts/proxy/ — proxy endpoints (gravatar, google-static-maps, geocode) - /_scripts/c/ — first-party collection (unchanged) Fixes dev 404s caused by assets dev handler intercepting embed routes. Also skips social embed e2e tests (depend on live external APIs).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/runtime/components/ScriptBlueskyEmbed.vue (1)
30-34:⚠️ Potential issue | 🟠 MajorGuard registry checks when either built-in endpoint is still selected.
Line 33 currently gates only
apiEndpoint. If a consumer sets a custom API but leaves the defaultimageProxyEndpoint, image proxy calls can still hit disabled built-in routes.Suggested fix
+const DEFAULT_API_ENDPOINT = '/_scripts/embed/bluesky' +const DEFAULT_IMAGE_PROXY_ENDPOINT = '/_scripts/embed/bluesky-image' + const props = withDefaults(defineProps<{ @@ }>(), { - apiEndpoint: '/_scripts/embed/bluesky', - imageProxyEndpoint: '/_scripts/embed/bluesky-image', + apiEndpoint: DEFAULT_API_ENDPOINT, + imageProxyEndpoint: DEFAULT_IMAGE_PROXY_ENDPOINT, }) -if (!props.apiEndpoint || props.apiEndpoint === '/_scripts/embed/bluesky') +if ( + props.apiEndpoint === DEFAULT_API_ENDPOINT + || props.imageProxyEndpoint === DEFAULT_IMAGE_PROXY_ENDPOINT +) requireRegistryEndpoint('ScriptBlueskyEmbed', 'blueskyEmbed')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/ScriptBlueskyEmbed.vue` around lines 30 - 34, The guard currently checks only props.apiEndpoint and may miss cases where props.imageProxyEndpoint remains the default; update the condition so requireRegistryEndpoint('ScriptBlueskyEmbed','blueskyEmbed') is called when either props.apiEndpoint is missing or equals '/_scripts/embed/bluesky' OR props.imageProxyEndpoint is missing or equals '/_scripts/embed/bluesky-image'. Locate the check around props.apiEndpoint and props.imageProxyEndpoint and change the boolean expression to cover both defaults before calling requireRegistryEndpoint.src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue (1)
255-266:⚠️ Potential issue | 🟠 MajorProxy geocode branch still blocks fallback on misses/errors.
At Line 255, once
endpoints?.googleMapsis true, Line 265 throws for any non-OK/miss, so the Places fallback below never runs in those cases.🐛 Proposed fix to preserve fallback behavior
if (endpoints?.googleMaps) { - const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/proxy/google-maps-geocode', { - params: { address: query }, - }) - if (data.status === 'OK' && data.results?.[0]?.geometry?.location) { - const loc = data.results[0].geometry.location - const latLng = { lat: loc.lat, lng: loc.lng } - queryToLatLngCache.set(query, latLng) - return latLng - } - throw new Error(`No location found for ${query}`) + try { + const data = await $fetch<{ results: Array<{ geometry: { location: { lat: number, lng: number } } }>, status: string }>('/_scripts/proxy/google-maps-geocode', { + params: { address: query }, + }) + if (data.status === 'OK' && data.results?.[0]?.geometry?.location) { + const loc = data.results[0].geometry.location + const latLng = { lat: loc.lat, lng: loc.lng } + queryToLatLngCache.set(query, latLng) + return latLng + } + } + catch { + // fall through to Places API fallback + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue` around lines 255 - 266, The proxy geocode branch currently throws on any non-OK/miss which prevents the Places fallback from running; in the block that checks endpoints?.googleMaps and calls $fetch, only set queryToLatLngCache and return a latLng when data.status === 'OK' and results exist, but do not throw on other statuses—instead return undefined (or null) so the caller can continue to the Places fallback; remove or replace the throw new Error(`No location found for ${query}`) with a non-throwing return and keep error handling scoped to the fetch call only.
🧹 Nitpick comments (1)
test/e2e/basic.test.ts (1)
456-458: Prefer conditional gating over permanently skipped social embed tests.This keeps CI stable while allowing opt-in live coverage (e.g., via env flag) in nightly or manual runs.
♻️ Suggested adjustment
+const runLiveSocialEmbeds = process.env.NUXT_SCRIPTS_E2E_LIVE === 'true' -describe.skip('social-embeds', () => { +(runLiveSocialEmbeds ? describe : describe.skip)('social-embeds', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/basic.test.ts` around lines 456 - 458, Replace the permanent skip on the test suite by gating it behind an opt-in env flag: locate the describe.skip call for 'social-embeds' in test/e2e/basic.test.ts and change it so the suite runs only when an environment variable (e.g. RUN_SOCIAL_EMBEDS) is truthy; implement this by selecting describe vs describe.skip at runtime (for example via a local describeIf = process.env.RUN_SOCIAL_EMBEDS ? describe : describe.skip) and use describeIf('social-embeds', ...). Ensure the rest of the suite body and its nested tests remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registry-types.json`:
- Line 844: Add a missing EOF newline to the end of registry-types.json by
ensuring the final closing brace '}' is followed by a single newline character;
update the file so it ends with a newline to satisfy the style/eol-last lint
rule and unblock CI.
---
Duplicate comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue`:
- Around line 255-266: The proxy geocode branch currently throws on any
non-OK/miss which prevents the Places fallback from running; in the block that
checks endpoints?.googleMaps and calls $fetch, only set queryToLatLngCache and
return a latLng when data.status === 'OK' and results exist, but do not throw on
other statuses—instead return undefined (or null) so the caller can continue to
the Places fallback; remove or replace the throw new Error(`No location found
for ${query}`) with a non-throwing return and keep error handling scoped to the
fetch call only.
In `@src/runtime/components/ScriptBlueskyEmbed.vue`:
- Around line 30-34: The guard currently checks only props.apiEndpoint and may
miss cases where props.imageProxyEndpoint remains the default; update the
condition so requireRegistryEndpoint('ScriptBlueskyEmbed','blueskyEmbed') is
called when either props.apiEndpoint is missing or equals
'/_scripts/embed/bluesky' OR props.imageProxyEndpoint is missing or equals
'/_scripts/embed/bluesky-image'. Locate the check around props.apiEndpoint and
props.imageProxyEndpoint and change the boolean expression to cover both
defaults before calling requireRegistryEndpoint.
---
Nitpick comments:
In `@test/e2e/basic.test.ts`:
- Around line 456-458: Replace the permanent skip on the test suite by gating it
behind an opt-in env flag: locate the describe.skip call for 'social-embeds' in
test/e2e/basic.test.ts and change it so the suite runs only when an environment
variable (e.g. RUN_SOCIAL_EMBEDS) is truthy; implement this by selecting
describe vs describe.skip at runtime (for example via a local describeIf =
process.env.RUN_SOCIAL_EMBEDS ? describe : describe.skip) and use
describeIf('social-embeds', ...). Ensure the rest of the suite body and its
nested tests remain unchanged.
🪄 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: 72c1b1c4-9afd-4d63-b63d-47f3b05015bf
📒 Files selected for processing (23)
docs/content/scripts/bluesky-embed.mddocs/content/scripts/google-maps.mddocs/content/scripts/instagram-embed.mddocs/content/scripts/x-embed.mdplayground/nuxt.config.tssrc/assets.tssrc/first-party/setup.tssrc/first-party/types.tssrc/plugins/transform.tssrc/registry-types.jsonsrc/registry.tssrc/runtime/components/GoogleMaps/ScriptGoogleMaps.vuesrc/runtime/components/ScriptBlueskyEmbed.vuesrc/runtime/components/ScriptInstagramEmbed.vuesrc/runtime/components/ScriptXEmbed.vuesrc/runtime/registry/bluesky-embed.tssrc/runtime/registry/gravatar.tssrc/runtime/registry/schemas.tssrc/runtime/registry/x-embed.tssrc/runtime/server/instagram-embed.tssrc/stats.tstest/e2e/basic.test.tstest/unit/instagram-embed.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/runtime/components/ScriptXEmbed.vue
- src/first-party/setup.ts
- src/runtime/components/ScriptInstagramEmbed.vue
- docs/content/scripts/bluesky-embed.md
- src/first-party/types.ts
- test/unit/instagram-embed.test.ts
- src/runtime/registry/bluesky-embed.ts
src/registry-types.json
Outdated
| } | ||
| ] | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fix missing EOF newline to unblock lint.
Line 844 is missing a trailing newline, and CI is already failing on style/eol-last.
Suggested 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.
Suggested change
| } | |
| } | |
🧰 Tools
🪛 GitHub Actions: Test
[error] 844-844: ESLint: style/eol-last - Newline required at end of file but not found.
🪛 GitHub Check: test
[failure] 844-844:
Newline required at end of file but not found
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/registry-types.json` at line 844, Add a missing EOF newline to the end of
registry-types.json by ensuring the final closing brace '}' is followed by a
single newline character; update the file so it ends with a newline to satisfy
the style/eol-last lint rule and unblock CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Related to #594
❓ Type of change
📚 Description
Server-side embed components (Bluesky, X, Instagram) previously registered their server handlers unconditionally, exposing proxy endpoints on every app whether used or not. This PR introduces an explicit opt-in via
scripts.registry.<key>: truein nuxt.config, with a clear runtime error (requireRegistryEndpoint) when a component is used without being enabled.Additionally consolidates 4 duplicate image proxy handlers into a
createImageProxyHandler()factory, standardizes all endpoint paths from/api/_scripts/to/_scripts/, unifies first-party collection under/_scripts/c, and relies on browser CORS for cross-origin protection (no server-side Origin checks needed).New
serverHandlersfield onRegistryScriptreplaces ~60 lines of manualaddServerHandler()calls with a generic registration loop. Also adds a geocode proxy (/_scripts/google-maps-geocode-proxy) soScriptGoogleMapscan resolve addresses server-side without exposing the API key.scripts.registry.blueskyEmbed: true(andxEmbed,instagramEmbed)/api/_scripts/*to/_scripts/*/_proxyto/_scripts/c(override viafirstParty.collectPrefix)📝 Migration
If using custom
apiEndpointprops pointing to your own routes, therequireRegistryEndpointcheck is skipped automatically.