Server-side fetch function treats credentials: 'include' the same as credentials: 'same-origin'

[lachlancollins]

Describe the bug

Hi there, earlier today I ran into an issue automatically passing cookies into fetch requests, despite having credentials: 'include' set (which worked with browser fetch). Once I saw the JSDoc hint that I needed a sub-subdomain, I changed the endpoint and created this PR: #10421

However, I have a feeling this isn't actually expected behaviour. The source code in fetch.js has the following logic for including cookies:

if (`.${url.hostname}`.endsWith(`.${event.url.hostname}`) && credentials !== 'omit') {
	const cookie = get_cookie_header(url, request.headers.get('cookie'));
	if (cookie) request.headers.set('cookie', cookie);
}

This only checks if credentials !== 'omit', but does not differentiate 'include' from 'same-origin'. According to MDN, the cookies should be sent regardless of origin for 'include'.

I think the correct logic would look something like this:

if (
	credentials === 'include' ||
	(credentials === 'same-origin' && `.${url.hostname}`.endsWith(`.${event.url.hostname}`))
) {
	const cookie = get_cookie_header(url, request.headers.get('cookie'));
	if (cookie) request.headers.set('cookie', cookie);
}

Reproduction

Very hard to share a reproduction - it would need a separate server...

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 12.01 GB / 15.58 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 18.12.0 - ~/.nvm/versions/node/v18.12.0/bin/node
    npm: 9.8.0 - ~/.nvm/versions/node/v18.12.0/bin/npm
    pnpm: 8.6.6 - ~/.nvm/versions/node/v18.12.0/bin/pnpm
  npmPackages:
    @sveltejs/kit: 1.22.3 => 1.22.3 
    @sveltejs/vite-plugin-svelte-inspector: 1.0.3 => 1.0.3 
    svelte: 4.1.1 => 4.1.1 
    vite: 4.4.5 => 4.4.5

Severity

serious, but I can work around it

Additional Information

Logic initially implemented in #1847

[dummdidumm]

I think that is a bug, yes, and the proposed change makes sense. The only thing I'm wondering is whether or not we need to manually filter out cookies with SameSite: strict or if the server/browser will take care of this for us automatically. You can amend your existing PR about docs with the fix, and hopefully we can solve the SameSite question along the way.

[lachlancollins]

Thanks @dummdidumm, I've updated the PR!

[lachlancollins]

Closing - potential security issues discussed on #10421