kit: $env/dynamic/public becomes $env/static/public for pre-rendered pages, breaking app and exposing information

Describe the bug

The docs for $env/dynamic/public state:

This module provides access to runtime environment variables, as defined by the platform you’re running on.

For pre-rendered pages though, this is incorrect and misleading. They actually behave more like $env/static/public with values coming from build-time .env files used. Not only does this risk exposing information that wasn’t intended to be exposed but the behavior of the app becomes indeterminate because the values used in the app depend on the order of page navigation.

As an example, I’m using $env/dynamic/public to read Firebase config from runtime environment values set in Google Cloud Run. Every so often Firebase auth on the client would fail, but a page refresh would always fix it. It depended on which page was initially viewed and exposed development-use configuration that was not intended to be exposed.

Reproduction

See https://github.com/CaptainCodeman/svelte-kit-10008

Logs

No response

System Info

System:
    OS: macOS 13.3.1
    CPU: (6) x64 Intel(R) Core(TM) i5-8500B CPU @ 3.00GHz
    Memory: 36.28 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.10.0 - ~/Library/pnpm/node
    npm: 8.19.2 - ~/Library/pnpm/npm
  Browsers:
    Brave Browser: 106.1.44.112
    Chrome: 113.0.5672.126
    Chrome Canary: 115.0.5786.0
    Firefox: 111.0.1
    Safari: 16.4
    Safari Technology Preview: 16.4
  npmPackages:
    @sveltejs/adapter-node: ^1.2.4 => 1.2.4 
    @sveltejs/kit: ^1.18.0 => 1.18.0 
    svelte: ^3.59.1 => 3.59.1 
    vite: ^4.3.8 => 4.3.8

Severity

annoyance (had to disable pre-rendered pages for correct functionality)

Additional Information

About this issue

Original URL
State: closed
Created a year ago
Reactions: 2
Comments: 15 (14 by maintainers)

Most upvoted comments

Thinking out loud:

yes, the public env values are embedded in the HTML response, and then retrieved from an external JS module. This sounds silly but like many such things it makes more sense when you dig into it. That external module is part of the build, and in many cases will be inlined into a larger chunk. Those chunks are loaded eagerly (with modulepreload) and cached forever (since they are immutable). If we instead imported those values from a module, then it couldn’t be part of the build (obviously) so would have to be an endpoint, as @benmccann says. Not only would this mean less efficient chunking, but the dynamic module couldn’t be cached immutably, so at minimum you’d have to wait for a 304 response for that module on every single page load before you could boot the app. That would be bad. The current approach is very deliberate
in general, it doesn’t make sense to repopulate $env/dynamic/public on navigation — it would be weird if it changed value during the session (unless a new version of the app is deployed while the session is ongoing, but that’s a whole other story)
prerendering is an exception to that — you don’t want to land on a prerendered page and have $env/dynamic/public populated with build-time variables, but equally you don’t want to land on a dynamically rendered page and have it be populated with request-time variables that are unexpectedly used on a prerendered page that you later navigate to
if something is prerendered, you know you’re going to be dealing with build-time variables. but in that case, you can just use $env/static/public, which is more efficient anyway.
conclusion: we should prevent you from using $env/dynamic/public during prerendering

That solves the easy part of the problem. The trickier part is subsequently populating $env/dynamic/public with request-time variables.

Since a prerendered page could access $env/dynamic/public before a navigation (in an event handler, or even in some browser-only code that runs immediately upon rendering), it doesn’t seem sufficient to e.g. smuggle the env vars into the data for the next request.

Which means that in the prerendered case I don’t think we have a choice other than to serve env vars dynamically, from a ${base}/_env.js module (configurable via config.kit.env.publicModule). This is a bummer — as described, this undermines the performance characteristics of prerendering by forcing you to make a request to a (potentially distant) origin server before you can hydrate an otherwise-fully-prerendered page.

Mitigations:

in the adapter-static case, the module would also be ‘prerendered’, so even though you have to wait for a 304, you’re at least dealing with a CDN rather than an origin server
we could skip this stuff if $env/dynamic/public is never imported in the app. this would happen ‘for free’ if that module basically just re-exports from ../../_env.js if no request-time env vars exist in the HTML
we can inject a modulepreload for ${base}/_env.js into the prerendered HTML if $env/dynamic/public is imported somewhere in the app

Rich-Harris on Dec 12, 2023

It’s not really whether there should be a pre-render option or not, I don’t think there is any case to be made for it - unless the app is built on the server it will run on (which would be a bit weird) or you happen to have a local .env file that matches production, the values will always be wrong if they are pre-rendered.

Pre-rendering stops the “dynamic” part doing what it says on the tin. The purpose of dynamic should always be to reflect that values set on the server, not anything that is compiled into the app, otherwise you’d just use the static option.

CaptainCodeman on Aug 9, 2023