diff --git a/.changeset/shaggy-moons-sort.md b/.changeset/shaggy-moons-sort.md new file mode 100644 index 000000000000..27aa5690bdee --- /dev/null +++ b/.changeset/shaggy-moons-sort.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +security: Stop implicitly tracking URLs as dependencies in server-side `load`s diff --git a/documentation/docs/20-core-concepts/20-load.md b/documentation/docs/20-core-concepts/20-load.md index 7a332ae07471..46dd991f7c85 100644 --- a/documentation/docs/20-core-concepts/20-load.md +++ b/documentation/docs/20-core-concepts/20-load.md @@ -537,7 +537,7 @@ Dependency tracking does not apply _after_ the `load` function has returned — ### Manual invalidation -You can also re-run `load` functions that apply to the current page using [`invalidate(url)`](modules#$app-navigation-invalidate), which re-runs all `load` functions that depend on `url`, and [`invalidateAll()`](modules#$app-navigation-invalidateall), which re-runs every `load` function. +You can also re-run `load` functions that apply to the current page using [`invalidate(url)`](modules#$app-navigation-invalidate), which re-runs all `load` functions that depend on `url`, and [`invalidateAll()`](modules#$app-navigation-invalidateall), which re-runs every `load` function. Server load functions will never automatically depend on a fetched `url` to avoid leaking secrets to the client. A `load` function depends on `url` if it calls `fetch(url)` or `depends(url)`. Note that `url` can be a custom identifier that starts with `[a-z]:`: @@ -585,7 +585,7 @@ To summarize, a `load` function will re-run in the following situations: - It references a property of `params` whose value has changed - It references a property of `url` (such as `url.pathname` or `url.search`) whose value has changed. Properties in `request.url` are _not_ tracked - It calls `await parent()` and a parent `load` function re-ran -- It declared a dependency on a specific URL via [`fetch`](#making-fetch-requests) or [`depends`](types#public-types-loadevent), and that URL was marked invalid with [`invalidate(url)`](modules#$app-navigation-invalidate) +- It declared a dependency on a specific URL via [`fetch`](#making-fetch-requests) (universal load only) or [`depends`](types#public-types-loadevent), and that URL was marked invalid with [`invalidate(url)`](modules#$app-navigation-invalidate) - All active `load` functions were forcibly re-run with [`invalidateAll()`](modules#$app-navigation-invalidateall) `params` and `url` can change in response to a `` link click, a [`
` interaction](form-actions#get-vs-post), a [`goto`](modules#$app-navigation-goto) invocation, or a [`redirect`](modules#sveltejs-kit-redirect). diff --git a/packages/kit/src/core/config/index.spec.js b/packages/kit/src/core/config/index.spec.js index 6fc1066b1279..3388f149b7df 100644 --- a/packages/kit/src/core/config/index.spec.js +++ b/packages/kit/src/core/config/index.spec.js @@ -69,6 +69,9 @@ const get_defaults = (prefix = '') => ({ csrf: { checkOrigin: true }, + dangerZone: { + trackServerFetches: false + }, embedded: false, env: { dir: process.cwd(), diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js index e039060cc17c..3a6c862267bd 100644 --- a/packages/kit/src/core/config/options.js +++ b/packages/kit/src/core/config/options.js @@ -111,6 +111,11 @@ const options = object( checkOrigin: boolean(true) }), + dangerZone: object({ + // TODO 2.0: Remove this + trackServerFetches: boolean(false) + }), + embedded: boolean(false), env: object({ diff --git a/packages/kit/src/core/sync/write_server.js b/packages/kit/src/core/sync/write_server.js index bf119dcfa7ac..647aa5924237 100644 --- a/packages/kit/src/core/sync/write_server.js +++ b/packages/kit/src/core/sync/write_server.js @@ -34,6 +34,7 @@ export const options = { app_template_contains_nonce: ${template.includes('%sveltekit.nonce%')}, csp: ${s(config.kit.csp)}, csrf_check_origin: ${s(config.kit.csrf.checkOrigin)}, + track_server_fetches: ${s(config.kit.dangerZone.trackServerFetches)}, embedded: ${config.kit.embedded}, env_public_prefix: '${config.kit.env.publicPrefix}', hooks: null, // added lazily, via \`get_hooks\` diff --git a/packages/kit/src/runtime/server/data/index.js b/packages/kit/src/runtime/server/data/index.js index 1fbec114a26d..8d38b1e10720 100644 --- a/packages/kit/src/runtime/server/data/index.js +++ b/packages/kit/src/runtime/server/data/index.js @@ -76,7 +76,8 @@ export async function render_data( } } return data; - } + }, + track_server_fetches: options.track_server_fetches }); } catch (e) { aborted = true; diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js index fdf7713be962..b7009a27b9fe 100644 --- a/packages/kit/src/runtime/server/page/index.js +++ b/packages/kit/src/runtime/server/page/index.js @@ -150,7 +150,8 @@ export async function render_page(event, page, options, manifest, state, resolve if (parent) Object.assign(data, await parent.data); } return data; - } + }, + track_server_fetches: options.track_server_fetches }); } catch (e) { load_error = /** @type {Error} */ (e); diff --git a/packages/kit/src/runtime/server/page/load_data.js b/packages/kit/src/runtime/server/page/load_data.js index 5db45afdc673..ced84ab9918c 100644 --- a/packages/kit/src/runtime/server/page/load_data.js +++ b/packages/kit/src/runtime/server/page/load_data.js @@ -10,10 +10,18 @@ import { validate_depends } from '../../shared.js'; * state: import('types').SSRState; * node: import('types').SSRNode | undefined; * parent: () => Promise>; + * track_server_fetches: boolean; * }} opts * @returns {Promise} */ -export async function load_server_data({ event, state, node, parent }) { +export async function load_server_data({ + event, + state, + node, + parent, + // TODO 2.0: Remove this + track_server_fetches +}) { if (!node?.server) return null; let done = false; @@ -51,7 +59,10 @@ export async function load_server_data({ event, state, node, parent }) { ); } - uses.dependencies.add(url.href); + // TODO 2.0: Remove this + if (track_server_fetches) { + uses.dependencies.add(url.href); + } return event.fetch(info, init); }, diff --git a/packages/kit/src/runtime/server/page/respond_with_error.js b/packages/kit/src/runtime/server/page/respond_with_error.js index 10607a09d8ad..2567b7e768e8 100644 --- a/packages/kit/src/runtime/server/page/respond_with_error.js +++ b/packages/kit/src/runtime/server/page/respond_with_error.js @@ -44,7 +44,8 @@ export async function respond_with_error({ event, state, node: default_layout, - parent: async () => ({}) + parent: async () => ({}), + track_server_fetches: options.track_server_fetches }); const server_data = await server_data_promise; diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index bf8a748c0668..b95076c4520e 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -456,14 +456,15 @@ test.describe('Invalidation', () => { expect(shared).not.toBe(next_shared); }); - test('fetch in server load can be invalidated', async ({ page, app, request }) => { + test('fetch in server load cannot be invalidated', async ({ page, app, request }) => { + // TODO 2.0: Can remove this test after `dangerZone.trackServerFetches` and associated code is removed await request.get('/load/invalidation/server-fetch/count.json?reset'); await page.goto('/load/invalidation/server-fetch'); const selector = '[data-testid="count"]'; expect(await page.textContent(selector)).toBe('1'); await app.invalidate('/load/invalidation/server-fetch/count.json'); - expect(await page.textContent(selector)).toBe('2'); + expect(await page.textContent(selector)).toBe('1'); }); test('+layout.js is re-run when shared dep is invalidated', async ({ page }) => { diff --git a/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.server.js b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.server.js new file mode 100644 index 000000000000..fc6a5abe8eac --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.server.js @@ -0,0 +1,6 @@ +// TODO 2.0: Delete +/** @type {import('./$types').PageServerLoad} */ +export async function load({ fetch }) { + const res = await fetch('/path-base/server-fetch-invalidate/count.json'); + return res.json(); +} diff --git a/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.svelte b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.svelte new file mode 100644 index 000000000000..3cadb6924412 --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/+page.svelte @@ -0,0 +1,6 @@ + + +

{data.count}

diff --git a/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/count.json/+server.js b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/count.json/+server.js new file mode 100644 index 000000000000..443894c3dcf5 --- /dev/null +++ b/packages/kit/test/apps/options/source/pages/server-fetch-invalidate/count.json/+server.js @@ -0,0 +1,10 @@ +// TODO 2.0: Delete +import { json } from '@sveltejs/kit'; + +let count = 0; + +/** @type {import('./$types').RequestHandler} */ +export function GET({ url }) { + if (url.searchParams.has('reset')) count = 0; + return json({ count: count++ }); +} diff --git a/packages/kit/test/apps/options/svelte.config.js b/packages/kit/test/apps/options/svelte.config.js index 63a9e77106aa..ce787f38d5e9 100644 --- a/packages/kit/test/apps/options/svelte.config.js +++ b/packages/kit/test/apps/options/svelte.config.js @@ -9,6 +9,9 @@ const config = { 'require-trusted-types-for': ['script'] } }, + dangerZone: { + trackServerFetches: true + }, files: { assets: 'public', lib: 'source/components', diff --git a/packages/kit/test/apps/options/test/test.js b/packages/kit/test/apps/options/test/test.js index 295985858f70..43277cade6bc 100644 --- a/packages/kit/test/apps/options/test/test.js +++ b/packages/kit/test/apps/options/test/test.js @@ -297,3 +297,22 @@ test.describe('Routing', () => { await expect(page.locator('h2')).toHaveText('target: 0'); }); }); + +test.describe('load', () => { + // TODO 2.0: Remove this test + test('fetch in server load can be invalidated when `dangerZone.trackServerFetches` is set', async ({ + page, + app, + request, + javaScriptEnabled + }) => { + test.skip(!javaScriptEnabled, 'JavaScript is disabled'); + await request.get('/path-base/server-fetch-invalidate/count.json?reset'); + await page.goto('/path-base/server-fetch-invalidate'); + const selector = '[data-testid="count"]'; + + expect(await page.textContent(selector)).toBe('1'); + await app.invalidate('/path-base/server-fetch-invalidate/count.json'); + expect(await page.textContent(selector)).toBe('2'); + }); +}); diff --git a/packages/kit/test/prerendering/basics/test/tests.spec.js b/packages/kit/test/prerendering/basics/test/tests.spec.js index 7a63454c6429..433be62c5b24 100644 --- a/packages/kit/test/prerendering/basics/test/tests.spec.js +++ b/packages/kit/test/prerendering/basics/test/tests.spec.js @@ -170,9 +170,7 @@ test('fetches data from local endpoint', () => { { type: 'data', data: [{ message: 1 }, 'hello'], - uses: { - dependencies: ['http://example.com/origin/message.json'] - } + uses: {} } ] }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 55c4589ec8a4..7d541a4377f3 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -340,6 +340,16 @@ export interface KitConfig { */ checkOrigin?: boolean; }; + /** + * Here be dragons. Enable at your peril. + */ + dangerZone?: { + /** + * Automatically add server-side `fetch`ed URLs to the `dependencies` map of `load` functions. This will expose secrets + * to the client if your URL contains them. + */ + trackServerFetches?: boolean; + }; /** * Whether or not the app is embedded inside a larger app. If `true`, SvelteKit will add its event listeners related to navigation etc on the parent of `%sveltekit.body%` instead of `window`, and will pass `params` from the server rather than inferring them from `location.pathname`. * @default false diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index c2dce68abcd4..379328678073 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -333,6 +333,7 @@ export interface SSROptions { app_template_contains_nonce: boolean; csp: ValidatedConfig['kit']['csp']; csrf_check_origin: boolean; + track_server_fetches: boolean; embedded: boolean; env_public_prefix: string; hooks: ServerHooks;