Skip to content

Commit fe86029

Browse files
danielroedanielroe
authored
fix(nitro): validate island request hash matches props (#35077)
1 parent 04eca90 commit fe86029

7 files changed

Lines changed: 201 additions & 23 deletions

File tree

‎packages/nitro-server/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"mlly": "^1.8.2",
4848
"mocked-exports": "^0.1.1",
4949
"nitro": "^3.0.260311-beta",
50+
"ohash": "^2.0.11",
5051
"pathe": "^2.0.3",
5152
"srvx": "^0.11.15",
5253
"std-env": "^4.1.0",

‎packages/nitro-server/src/runtime/handlers/island.ts‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { HTTPError, defineEventHandler, getQuery, readBody } from 'nitro/h3'
77
import { resolveUnrefHeadInput } from '@unhead/vue'
88
import { getRequestDependencies } from 'vue-bundle-renderer/runtime'
99
import { getQuery as getURLQuery } from 'ufo'
10+
import { computeIslandHash, filterIslandProps } from '#app/island-hash'
1011
import type { NuxtIslandContext, NuxtIslandResponse } from 'nuxt/app'
1112
import { islandCache, islandPropCache } from '../utils/cache'
1213
import { createSSRContext } from '../utils/renderer/app'
@@ -115,7 +116,8 @@ async function getIslandContext (event: H3Event): Promise<NuxtIslandContext> {
115116
let url = event.url.pathname + event.url.search + event.url.hash
116117
const islandPath = event.url.pathname
117118
if (import.meta.prerender && await islandPropCache!.hasItem(islandPath)) {
118-
// rehydrate props from cache so we can rerender island if cache does not have it any more
119+
// for prerender, the original request URL (with query) is rehydrated from cache
120+
// so that re-renders of the same island path use the original props
119121
url = await islandPropCache!.getItem(islandPath) as string
120122
}
121123

@@ -131,14 +133,33 @@ async function getIslandContext (event: H3Event): Promise<NuxtIslandContext> {
131133
throw new HTTPError({ status: 400, statusText: 'Invalid island component name' })
132134
}
133135

134-
const context = event.req.method === 'GET' ? getQuery<NuxtIslandContext>(event) : await readBody<NuxtIslandContext>(event)
136+
const rawContext = event.req.method === 'GET' ? getQuery<NuxtIslandContext>(event) : await readBody<NuxtIslandContext>(event)
137+
const rawProps = destr<Record<string, any> | null | undefined>(rawContext?.props) || {}
138+
const filteredProps = filterIslandProps(rawProps)
139+
140+
// Reconstruct the `context` object as the client computed its hash over.
141+
// `<NuxtIsland>` sends `{ ...props.context, props: JSON.stringify(props.props) }`
142+
const clientContext: Record<string, any> = {}
143+
if (rawContext && typeof rawContext === 'object') {
144+
for (const key in rawContext) {
145+
if (key !== 'props') {
146+
clientContext[key] = (rawContext as Record<string, any>)[key]
147+
}
148+
}
149+
}
150+
151+
// Bind the response to the URL: a request whose URL-resident `hashId` does not match
152+
// the actual (name, props, context) is rejected.
153+
const expectedHash = computeIslandHash(componentName, filteredProps, clientContext, undefined)
154+
if (!hashId || hashId !== expectedHash) {
155+
throw new HTTPError({ status: 400, statusText: 'Invalid island request hash' })
156+
}
135157

136-
// Only extract known context fields to prevent arbitrary data injection
137158
return {
138-
url: typeof context?.url === 'string' ? context.url : '/',
159+
url: typeof rawContext?.url === 'string' ? rawContext.url : '/',
139160
id: hashId,
140161
name: componentName,
141-
props: destr(context?.props) || {},
162+
props: rawProps,
142163
slots: {},
143164
components: {},
144165
}

‎packages/nuxt/src/app/components/nuxt-island.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { Component, PropType, RendererNode, VNode } from 'vue'
22
import { Fragment, Teleport, computed, createStaticVNode, createVNode, defineComponent, getCurrentInstance, h, nextTick, onBeforeUnmount, onMounted, ref, shallowRef, toRaw, watch, withMemo } from 'vue'
33
import { debounce } from 'perfect-debounce'
4-
import { hash } from 'ohash'
54
import type { ActiveHeadEntry, SerializableHead } from '@unhead/vue'
65
import { randomUUID } from 'uncrypto'
76
import { joinURL, withQuery } from 'ufo'
@@ -12,6 +11,7 @@ import { createError } from '../composables/error'
1211
import { prerenderRoutes, useRequestEvent } from '../composables/ssr'
1312
import { injectHead } from '../composables/head'
1413
import { getFragmentHTML, isEndFragment, isStartFragment } from './utils'
14+
import { computeIslandHash, filterIslandProps } from '../island-hash'
1515

1616
// @ts-expect-error virtual file
1717
import { appBaseURL, remoteComponentIslands, selectiveClient } from '#build/nuxt.config.mjs'
@@ -85,8 +85,8 @@ export default defineComponent({
8585
const error = ref<unknown>(null)
8686
const config = useRuntimeConfig()
8787
const nuxtApp = useNuxtApp()
88-
const filteredProps = computed(() => props.props ? Object.fromEntries(Object.entries(props.props).filter(([key]) => !key.startsWith('data-v-'))) : {})
89-
const hashId = computed(() => hash([props.name, filteredProps.value, props.context, props.source]).replace(/[-_]/g, ''))
88+
const filteredProps = computed(() => filterIslandProps(props.props))
89+
const hashId = computed(() => computeIslandHash(props.name, filteredProps.value, props.context, props.source))
9090
const instance = getCurrentInstance()!
9191
const event = useRequestEvent()
9292

‎packages/nuxt/src/app/island-hash.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { hash } from 'ohash'
2+
3+
/**
4+
* Strip Vue scoped-style attributes (`data-v-*`) from island props before hashing
5+
* or rendering. Scoped-id markers leak in from parent components and are not part
6+
* of the logical island input.
7+
*
8+
* Used by both `<NuxtIsland>` (client) and the `/__nuxt_island/*` handler (server)
9+
* to derive the URL-resident `hashId`.
10+
*
11+
* @internal
12+
*/
13+
export function filterIslandProps (props: Record<string, any> | null | undefined): Record<string, any> {
14+
if (!props) { return {} }
15+
const out: Record<string, any> = {}
16+
for (const key in props) {
17+
if (!key.startsWith('data-v-')) {
18+
out[key] = props[key]
19+
}
20+
}
21+
return out
22+
}
23+
24+
/**
25+
* Compute the `hashId` segment embedded in an island URL (`/__nuxt_island/<Name>_<hashId>.json`).
26+
*
27+
* The hash binds the response to the requested `(name, props, context, source)` tuple,
28+
* so the server can reject requests whose URL hash does not match the supplied query/body.
29+
*
30+
* @internal
31+
*/
32+
export function computeIslandHash (
33+
name: string,
34+
filteredProps: Record<string, any>,
35+
context: Record<string, any>,
36+
source: string | undefined,
37+
): string {
38+
return hash([name, filteredProps, context, source]).replace(/[-_]/g, '')
39+
}

‎packages/nuxt/test/island-hash.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { hash } from 'ohash'
3+
import { computeIslandHash, filterIslandProps } from '#app/island-hash'
4+
5+
describe('filterIslandProps', () => {
6+
it('returns an empty object for nullish input', () => {
7+
expect(filterIslandProps(undefined)).toEqual({})
8+
expect(filterIslandProps(null)).toEqual({})
9+
})
10+
11+
it('passes through ordinary props', () => {
12+
expect(filterIslandProps({ a: 1, b: 'x', c: { nested: true } })).toEqual({
13+
a: 1,
14+
b: 'x',
15+
c: { nested: true },
16+
})
17+
})
18+
19+
it('strips Vue scoped-style markers', () => {
20+
expect(filterIslandProps({
21+
'data-v-abc123': '',
22+
'data-v-def456': '',
23+
'count': 3,
24+
'label': 'hi',
25+
})).toEqual({ count: 3, label: 'hi' })
26+
})
27+
28+
it('preserves keys that merely contain "data-v-"', () => {
29+
// Only the prefix is stripped — keys like `extra-data-v-x` are legitimate.
30+
expect(filterIslandProps({ 'extra-data-v-x': 1, 'data-v-x': 2 })).toEqual({ 'extra-data-v-x': 1 })
31+
})
32+
})
33+
34+
describe('computeIslandHash', () => {
35+
it('matches the ohash-based shape the client embeds in the URL', () => {
36+
const name = 'PureComponent'
37+
const props = { count: 3, label: 'hi' }
38+
const context = { url: '/foo' }
39+
const expected = hash([name, props, context, undefined]).replace(/[-_]/g, '')
40+
expect(computeIslandHash(name, props, context, undefined)).toBe(expected)
41+
})
42+
43+
it('changes when props change', () => {
44+
const a = computeIslandHash('X', { n: 1 }, {}, undefined)
45+
const b = computeIslandHash('X', { n: 2 }, {}, undefined)
46+
expect(a).not.toBe(b)
47+
})
48+
49+
it('changes when context changes', () => {
50+
const a = computeIslandHash('X', {}, { url: '/a' }, undefined)
51+
const b = computeIslandHash('X', {}, { url: '/b' }, undefined)
52+
expect(a).not.toBe(b)
53+
})
54+
55+
it('changes when name changes', () => {
56+
const a = computeIslandHash('A', {}, {}, undefined)
57+
const b = computeIslandHash('B', {}, {}, undefined)
58+
expect(a).not.toBe(b)
59+
})
60+
61+
it('changes when source changes', () => {
62+
const a = computeIslandHash('X', {}, {}, undefined)
63+
const b = computeIslandHash('X', {}, {}, 'https://remote.example')
64+
expect(a).not.toBe(b)
65+
})
66+
67+
it('produces URL-safe output (no - or _)', () => {
68+
for (let i = 0; i < 20; i++) {
69+
const h = computeIslandHash('Comp', { i, salt: `${i}-${i}` }, {}, undefined)
70+
expect(h).not.toMatch(/[-_]/)
71+
}
72+
})
73+
})

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/server-components.test.ts‎

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,20 @@ import { isWindows } from 'std-env'
55
import { normalize } from 'pathe'
66
import { $fetch, fetch, setup, startServer } from '@nuxt/test-utils/e2e'
77
import type { NuxtIslandResponse } from 'nuxt/app'
8+
import { computeIslandHash, filterIslandProps } from '../packages/nuxt/src/app/island-hash'
89

910
import { isDev, isWebpack } from './matrix'
1011
import { renderPage } from './utils'
1112

13+
function islandURL (name: string, opts: { props?: Record<string, any>, context?: Record<string, any> } = {}) {
14+
const filtered = filterIslandProps(opts.props ?? {})
15+
const ctx = opts.context ?? {}
16+
const hashId = computeIslandHash(name, filtered, ctx, undefined)
17+
const query: Record<string, any> = { ...ctx }
18+
if (opts.props) { query.props = JSON.stringify(opts.props) }
19+
return withQuery(`/__nuxt_island/${name}_${hashId}.json`, query)
20+
}
21+
1222
await setup({
1323
rootDir: fileURLToPath(new URL('./fixtures/server-components', import.meta.url)),
1424
dev: isDev,
@@ -171,7 +181,7 @@ describe('server components/islands', () => {
171181

172182
describe('component islands', () => {
173183
it('renders components with route', async () => {
174-
const result = await $fetch<NuxtIslandResponse>('/__nuxt_island/RouteComponent.json?url=/foo')
184+
const result = await $fetch<NuxtIslandResponse>(islandURL('RouteComponent', { context: { url: '/foo' } }))
175185

176186
result.html = result.html.replace(/ data-island-uid="[^"]*"/g, '')
177187
if (isDev) {
@@ -180,6 +190,7 @@ describe('component islands', () => {
180190

181191
result.head.link ||= []
182192
result.head.style ||= []
193+
delete result.id
183194

184195
expect(result).toMatchInlineSnapshot(`
185196
{
@@ -194,18 +205,15 @@ describe('component islands', () => {
194205
})
195206

196207
it('render async component', async () => {
197-
const result = await $fetch<NuxtIslandResponse>(withQuery('/__nuxt_island/LongAsyncComponent.json', {
198-
props: JSON.stringify({
199-
count: 3,
200-
}),
201-
}))
208+
const result = await $fetch<NuxtIslandResponse>(islandURL('LongAsyncComponent', { props: { count: 3 } }))
202209
if (isDev) {
203210
result.head.link = result.head.link?.filter(l => typeof l.href !== 'string' || (!l.href.includes('_nuxt/components/islands/LongAsyncComponent') && !l.href.includes('PureComponent') /* TODO: fix dev bug triggered by previous fetch of /islands */))
204211
}
205212

206213
result.head.link ||= []
207214
result.head.style ||= []
208215
result.html = result.html.replaceAll(/ (?:data-island-uid|data-island-component)="[^"]*"/g, '')
216+
delete result.id
209217
expect(result).toMatchInlineSnapshot(`
210218
{
211219
"head": {
@@ -255,11 +263,7 @@ describe('component islands', () => {
255263
})
256264

257265
it('render .server async component', async () => {
258-
const result = await $fetch<NuxtIslandResponse>(withQuery('/__nuxt_island/AsyncServerComponent.json', {
259-
props: JSON.stringify({
260-
count: 2,
261-
}),
262-
}))
266+
const result = await $fetch<NuxtIslandResponse>(islandURL('AsyncServerComponent', { props: { count: 2 } }))
263267
if (isDev) {
264268
result.head.link = result.head.link?.filter(l => typeof l.href === 'string' && !l.href.includes('PureComponent') /* TODO: fix dev bug triggered by previous fetch of /islands */ && (!l.href.startsWith('_nuxt/components/islands/') || l.href.includes('AsyncServerComponent')))
265269
}
@@ -270,6 +274,7 @@ describe('component islands', () => {
270274
result.components = {}
271275
result.slots = {}
272276
result.html = result.html.replaceAll(/ (?:data-island-uid|data-island-component)="[^"]*"/g, '')
277+
delete result.id
273278

274279
expect(result).toMatchInlineSnapshot(`
275280
{
@@ -287,7 +292,7 @@ describe('component islands', () => {
287292

288293
if (!isWebpack) {
289294
it('render server component with selective client hydration', async () => {
290-
const result = await $fetch<NuxtIslandResponse>('/__nuxt_island/ServerWithClient')
295+
const result = await $fetch<NuxtIslandResponse>(islandURL('ServerWithClient'))
291296
if (isDev) {
292297
result.head.link = result.head.link?.filter(l => typeof l.href !== 'string' || (!l.href.includes('_nuxt/components/islands/LongAsyncComponent') && !l.href.includes('PureComponent') /* TODO: fix dev bug triggered by previous fetch of /islands */))
293298

@@ -304,6 +309,7 @@ describe('component islands', () => {
304309

305310
result.head.link ||= []
306311
result.head.style ||= []
312+
delete result.id
307313

308314
expect(result).toMatchInlineSnapshot(`
309315
{
@@ -327,13 +333,13 @@ describe('component islands', () => {
327333
}
328334

329335
it('renders pure components', async () => {
330-
const result = await $fetch<NuxtIslandResponse>(withQuery('/__nuxt_island/PureComponent.json', {
331-
props: JSON.stringify({
336+
const result = await $fetch<NuxtIslandResponse>(islandURL('PureComponent', {
337+
props: {
332338
bool: false,
333339
number: 3487,
334340
str: 'something',
335341
obj: { foo: 42, bar: false, me: 'hi' },
336-
}),
342+
},
337343
}))
338344
result.html = result.html.replace(/ data-island-uid="[^"]*"/g, '')
339345

@@ -461,6 +467,41 @@ describe('component islands', () => {
461467
})
462468
})
463469

470+
describe('hash binding', () => {
471+
it('accepts a request whose URL hash matches the props', async () => {
472+
const res = await fetch(islandURL('PureComponent', {
473+
props: { bool: false, number: 1, str: 's', obj: {} },
474+
}))
475+
expect(res.status).toBe(200)
476+
})
477+
478+
it('rejects a request whose URL hash was computed over different props', async () => {
479+
// Compute a valid hash for one set of props, then swap the actual query props.
480+
const url = islandURL('PureComponent', {
481+
props: { bool: false, number: 1, str: 's', obj: {} },
482+
})
483+
const tampered = url.replace(/props=[^&]+/, 'props=' + encodeURIComponent(JSON.stringify({
484+
bool: true, number: 999, str: '<script>x</script>', obj: { evil: true },
485+
})))
486+
const res = await fetch(tampered)
487+
expect(res.status).toBe(400)
488+
})
489+
490+
it('rejects a request with a fabricated hash', async () => {
491+
const res = await fetch(withQuery('/__nuxt_island/PureComponent_deadbeefcafef00d.json', {
492+
props: JSON.stringify({ bool: false, number: 1, str: 's', obj: {} }),
493+
}))
494+
expect(res.status).toBe(400)
495+
})
496+
497+
it('rejects a request with no hash segment in the URL', async () => {
498+
const res = await fetch(withQuery('/__nuxt_island/PureComponent.json', {
499+
props: JSON.stringify({ bool: false, number: 1, str: 's', obj: {} }),
500+
}))
501+
expect(res.status).toBe(400)
502+
})
503+
})
504+
464505
describe.skipIf(isDev || isWebpack)('regressions', () => {
465506
// https://github.com/nuxt/nuxt/issues/26527
466507
it.fails('renders <Counter nuxt-client /> when nested two levels deep in server components', async () => {

0 commit comments

Comments
 (0)