sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.72k stars 1.94k forks source link

Disable cookie secure option when using vite dev --host #10438

Open hyunbinseo opened 1 year ago

hyunbinseo commented 1 year ago

Describe the problem

Thanks to the localhost exception, cookies can be set in the development environment with ease.

The httpOnly and secure options are true by default (except on http://localhost/, where secure is false)

https://kit.svelte.dev/docs/types#public-types-cookies


However, the exception is not applied if the Vite server is exposed over the network, and is accessed using an IP address.

npx vite dev --host

# VITE v4.4.4  ready in 612 ms
cookies.set('name', 'value');

//  Local:   http://localhost:5173/ ← works
//  Network: http://172.30.1.83:5173/ ← does not work
//  Network: http://100.116.137.49:5173/ ← does not work

Describe the proposed solution

Disable the secure option in the cookies.set() API if it is a Vite dev server. (and possibly in the preview server)

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

hyunbinseo commented 1 year ago

Current implementation checks for http://localhost

https://github.com/sveltejs/kit/blob/8c4b74cdb9f9bc4de16e7a7cb2dfe38fc9e92db6/packages/kit/src/runtime/server/cookie.js#L33-L38

Why not check for the http:// protocol and the dev value in the $app/environment module?

Whether the dev server is running. This is not guaranteed to correspond to NODE_ENV or MODE. — https://kit.svelte.dev/docs/modules#$app-environment-dev

Would there be security issues with this?

mihaimiculescu commented 10 months ago

Ran also into this because I'm encapsulating my project in a docker container, so --host is a must. The barbaric workaround is to edit the cookie.js file before running in dev mode

sed -i "/secure:/c\\\t\tsecure:false" $(find . -name "cookie.js")

and then restore it to its original form afterwards

sed -i "/secure:/c\\\t\tsecure: url.hostname === 'localhost' && url.protocol === 'http:' ? false : true" $(find . -name "cookie.js")

This works if using GNU sed and might need adjusments if running on mac

hyunbinseo commented 10 months ago

I've settled on this workaround for the dev environment.

import { sveltekit } from '@sveltejs/kit/vite';
import basicSsl from '@vitejs/plugin-basic-ssl';
import { defineConfig } from 'vitest/config';

export default defineConfig({
  plugins: [
    ...(process.argv.includes('--host') ? [basicSsl()] : []), //
    sveltekit(),
  ],
});

It conditionally loads the @vitejs/plugin-basic-ssl plugin.

pnpm dev

#  VITE v5.0.10  ready in 533 ms
#
#  ➜  Local:   http://localhost:5174/
#  ➜  Network: use --host to expose

pnpm dev --host

#  VITE v5.0.10  ready in 574 ms
#
#  ➜  Local:   https://localhost:5173/
#  ➜  Network: https://172.30.1.84:5173/
#  ➜  Network: https://100.116.137.49:5173/
#  ➜  press h + enter to show help
hyunbinseo commented 9 months ago

The above workaround does not work in vite preview or npm preview.

Since they run the app in Node.js, all Node.js adapter related restrictions apply as well.

HTTP doesn't give SvelteKit a reliable way to know the URL that is currently being requested. The simplest way to tell SvelteKit where the app is being served is to set the ORIGIN environment variable:

Here is the updated vite.config.ts file.

Note that the isPreview check is broken as of now.

import { sveltekit } from '@sveltejs/kit/vite';
import basicSsl from '@vitejs/plugin-basic-ssl';
import { defineConfig } from 'vitest/config';

export default defineConfig(({ command, isPreview }) => {
  const exposed = command === 'serve' && process.argv.includes('--host');

  if (exposed && isPreview) {
    console.error('🔴 In the preview mode, the Node.js server cannot determine the request URL.');
    console.error('🔴 If the CSRF protection is enabled (default) POST form submits will fail.');
  }

  return {
    plugins: [
      //
      ...(exposed && !isPreview ? [basicSsl()] : []),
      sveltekit(),
    ],
  };
});
hyunbinseo commented 1 month ago

Vite does expose dev, preview server URLs in plugins:

import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vite';

export default defineConfig({
  plugins: [
    sveltekit(),
    {
      name: 'urls',
      configureServer: (server) => {
        server.httpServer?.on('listening', () => {
          setTimeout(() => {
            console.log(server.resolvedUrls);
            // { local: ['http://localhost:5173/'],
            // network: ['http://192.168.0.2:5173/'] };
          }, 10); // Setting this to 0 results in null
        });
      }
    }
  ]
});

But this cannot be shared using import.meta.env:

And process.env only supports plain-text values.

Would be nice if SvelteKit disables secure cookie in dev / preview servers.

eltigerchino commented 1 month ago

Is using the dev variable good enough for a short term workaround? e.g, secure: !dev to only enable secure in production https://kit.svelte.dev/docs/modules#$app-environment-dev

hyunbinseo commented 1 month ago

@eltigerchino It is a viable, shortterm workaround.

import { dev } from '$app/environment';

export const load = ({ cookies }) => {
  cookies.set('name', 'value', { path: '/', secure: !dev });
};

The limitations are:

Should I add vite preview in the issue title and body?

dominikg commented 1 month ago

you can have a secure dev server too, so this setting would have to be derived from the url ( secure = url.startsWith('https://')

hyunbinseo commented 1 month ago

Updated workaround:

import { dev } from '$app/environment';

export const load = ({ cookies, url }) => {
  cookies.set('name', 'value', { path: '/', secure: !dev || url.protocol === 'https:' });
};