From a9373c0c9a3c2e1773fc11bb14e156698b0d9d38 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Mon, 9 Dec 2024 13:29:44 +0100 Subject: [PATCH] fix: do not freeze process.env in dev (#12585) Co-authored-by: Emanuele Stoppa --- .changeset/nasty-parrots-cry.md | 5 ++ packages/astro/src/container/index.ts | 1 - packages/astro/src/core/app/types.ts | 1 - packages/astro/src/core/build/generate.ts | 1 - .../src/core/build/plugins/plugin-manifest.ts | 3 - packages/astro/src/core/create-vite.ts | 6 +- packages/astro/src/env/env-loader.ts | 60 +++++++++++++++++++ packages/astro/src/env/runtime.ts | 6 +- packages/astro/src/env/vite-plugin-env.ts | 32 +++++++--- .../src/vite-plugin-astro-server/plugin.ts | 1 - packages/astro/src/vite-plugin-env/index.ts | 60 ++----------------- packages/astro/templates/env.mjs | 18 ++++-- 12 files changed, 113 insertions(+), 81 deletions(-) create mode 100644 .changeset/nasty-parrots-cry.md create mode 100644 packages/astro/src/env/env-loader.ts diff --git a/.changeset/nasty-parrots-cry.md b/.changeset/nasty-parrots-cry.md new file mode 100644 index 0000000000..be25a64c2a --- /dev/null +++ b/.changeset/nasty-parrots-cry.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a case where `process.env` would be frozen despite changes made to environment variables in development \ No newline at end of file diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index 9ca9d23001..7a31af8a7a 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -155,7 +155,6 @@ function createManifest( i18n: manifest?.i18n, checkOrigin: false, middleware: manifest?.middleware ?? middlewareInstance, - envGetSecretEnabled: false, key: createKey(), }; } diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 0fb627f718..2417902500 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -69,7 +69,6 @@ export type SSRManifest = { i18n: SSRManifestI18n | undefined; middleware?: () => Promise | AstroMiddlewareInstance; checkOrigin: boolean; - envGetSecretEnabled: boolean; }; export type SSRManifestI18n = { diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 47d6ef3def..75b138c27e 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -559,6 +559,5 @@ function createBuildManifest( checkOrigin: (settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false, key, - envGetSecretEnabled: false, }; } diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 12fdf65b17..caebb470d5 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -274,8 +274,5 @@ function buildManifest( (settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false, serverIslandNameMap: Array.from(settings.serverIslandNameMap), key: encodedKey, - envGetSecretEnabled: - (unwrapSupportKind(settings.adapter?.supportedAstroFeatures.envGetSecret) ?? - 'unsupported') !== 'unsupported', }; } diff --git a/packages/astro/src/core/create-vite.ts b/packages/astro/src/core/create-vite.ts index 7b680c4055..784ef5f83f 100644 --- a/packages/astro/src/core/create-vite.ts +++ b/packages/astro/src/core/create-vite.ts @@ -41,6 +41,7 @@ import { vitePluginMiddleware } from './middleware/vite-plugin.js'; import { joinPaths } from './path.js'; import { vitePluginServerIslands } from './server-islands/vite-plugin-server-islands.js'; import { isObject } from './util.js'; +import { createEnvLoader } from '../env/env-loader.js'; type CreateViteOptions = { settings: AstroSettings; @@ -123,6 +124,7 @@ export async function createVite( }); const srcDirPattern = glob.convertPathToPattern(fileURLToPath(settings.config.srcDir)); + const envLoader = createEnvLoader(); // Start with the Vite configuration that Astro core needs const commonConfig: vite.InlineConfig = { @@ -146,8 +148,8 @@ export async function createVite( // The server plugin is for dev only and having it run during the build causes // the build to run very slow as the filewatcher is triggered often. command === 'dev' && vitePluginAstroServer({ settings, logger, fs, manifest, ssrManifest }), // ssrManifest is only required in dev mode, where it gets created before a Vite instance is created, and get passed to this function - envVitePlugin({ settings }), - astroEnv({ settings, mode, sync }), + envVitePlugin({ envLoader }), + astroEnv({ settings, mode, sync, envLoader }), markdownVitePlugin({ settings, logger }), htmlVitePlugin(), astroPostprocessVitePlugin(), diff --git a/packages/astro/src/env/env-loader.ts b/packages/astro/src/env/env-loader.ts new file mode 100644 index 0000000000..f33878a652 --- /dev/null +++ b/packages/astro/src/env/env-loader.ts @@ -0,0 +1,60 @@ +import { loadEnv } from 'vite'; +import type { AstroConfig } from '../types/public/index.js'; +import { fileURLToPath } from 'node:url'; + +// Match valid JS variable names (identifiers), which accepts most alphanumeric characters, +// except that the first character cannot be a number. +const isValidIdentifierRe = /^[_$a-zA-Z][\w$]*$/; + +function getPrivateEnv( + fullEnv: Record, + astroConfig: AstroConfig, +): Record { + const viteConfig = astroConfig.vite; + let envPrefixes: string[] = ['PUBLIC_']; + if (viteConfig.envPrefix) { + envPrefixes = Array.isArray(viteConfig.envPrefix) + ? viteConfig.envPrefix + : [viteConfig.envPrefix]; + } + + const privateEnv: Record = {}; + for (const key in fullEnv) { + // Ignore public env var + if (isValidIdentifierRe.test(key) && envPrefixes.every((prefix) => !key.startsWith(prefix))) { + if (typeof process.env[key] !== 'undefined') { + let value = process.env[key]; + // Replacements are always strings, so try to convert to strings here first + if (typeof value !== 'string') { + value = `${value}`; + } + // Boolean values should be inlined to support `export const prerender` + // We already know that these are NOT sensitive values, so inlining is safe + if (value === '0' || value === '1' || value === 'true' || value === 'false') { + privateEnv[key] = value; + } else { + privateEnv[key] = `process.env.${key}`; + } + } else { + privateEnv[key] = JSON.stringify(fullEnv[key]); + } + } + } + return privateEnv; +} + +export const createEnvLoader = () => { + let privateEnv: Record = {}; + return { + load: (mode: string, config: AstroConfig) => { + const loaded = loadEnv(mode, config.vite.envDir ?? fileURLToPath(config.root), ''); + privateEnv = getPrivateEnv(loaded, config); + return loaded; + }, + getPrivateEnv: () => { + return privateEnv; + }, + }; +}; + +export type EnvLoader = ReturnType; diff --git a/packages/astro/src/env/runtime.ts b/packages/astro/src/env/runtime.ts index a2017b617f..25a87d4bcf 100644 --- a/packages/astro/src/env/runtime.ts +++ b/packages/astro/src/env/runtime.ts @@ -4,14 +4,14 @@ import type { ValidationResultInvalid } from './validators.js'; export { validateEnvVariable, getEnvFieldType } from './validators.js'; export type GetEnv = (key: string) => string | undefined; -type OnSetGetEnv = (reset: boolean) => void; +type OnSetGetEnv = () => void; let _getEnv: GetEnv = (key) => process.env[key]; -export function setGetEnv(fn: GetEnv, reset = false) { +export function setGetEnv(fn: GetEnv) { _getEnv = fn; - _onSetGetEnv(reset); + _onSetGetEnv(); } let _onSetGetEnv: OnSetGetEnv = () => {}; diff --git a/packages/astro/src/env/vite-plugin-env.ts b/packages/astro/src/env/vite-plugin-env.ts index 816f460b31..c8ec5b8533 100644 --- a/packages/astro/src/env/vite-plugin-env.ts +++ b/packages/astro/src/env/vite-plugin-env.ts @@ -1,6 +1,5 @@ import { readFileSync } from 'node:fs'; -import { fileURLToPath } from 'node:url'; -import { type Plugin, loadEnv } from 'vite'; +import type { Plugin } from 'vite'; import { AstroError, AstroErrorData } from '../core/errors/index.js'; import type { AstroSettings } from '../types/astro.js'; import { @@ -11,26 +10,35 @@ import { import { type InvalidVariable, invalidVariablesToError } from './errors.js'; import type { EnvSchema } from './schema.js'; import { getEnvFieldType, validateEnvVariable } from './validators.js'; +import type { EnvLoader } from './env-loader.js'; interface AstroEnvPluginParams { settings: AstroSettings; mode: string; sync: boolean; + envLoader: EnvLoader; } -export function astroEnv({ settings, mode, sync }: AstroEnvPluginParams): Plugin { +export function astroEnv({ settings, mode, sync, envLoader }: AstroEnvPluginParams): Plugin { const { schema, validateSecrets } = settings.config.env; + let isDev: boolean; let templates: { client: string; server: string; internal: string } | null = null; return { name: 'astro-env-plugin', enforce: 'pre', + config(_, { command }) { + isDev = command !== 'build'; + }, buildStart() { - const loadedEnv = loadEnv(mode, fileURLToPath(settings.config.root), ''); - for (const [key, value] of Object.entries(loadedEnv)) { - if (value !== undefined) { - process.env[key] = value; + const loadedEnv = envLoader.load(mode, settings.config); + + if (!isDev) { + for (const [key, value] of Object.entries(loadedEnv)) { + if (value !== undefined) { + process.env[key] = value; + } } } @@ -42,7 +50,7 @@ export function astroEnv({ settings, mode, sync }: AstroEnvPluginParams): Plugin }); templates = { - ...getTemplates(schema, validatedVariables), + ...getTemplates(schema, validatedVariables, isDev ? loadedEnv : null), internal: `export const schema = ${JSON.stringify(schema)};`, }; }, @@ -122,6 +130,7 @@ function validatePublicVariables({ function getTemplates( schema: EnvSchema, validatedVariables: ReturnType, + loadedEnv: Record | null, ) { let client = ''; let server = readFileSync(MODULE_TEMPLATE_URL, 'utf-8'); @@ -142,10 +151,15 @@ function getTemplates( } server += `export let ${key} = _internalGetSecret(${JSON.stringify(key)});\n`; - onSetGetEnv += `${key} = reset ? undefined : _internalGetSecret(${JSON.stringify(key)});\n`; + onSetGetEnv += `${key} = _internalGetSecret(${JSON.stringify(key)});\n`; } server = server.replace('// @@ON_SET_GET_ENV@@', onSetGetEnv); + if (loadedEnv) { + server = server.replace('// @@GET_ENV@@', `return (${JSON.stringify(loadedEnv)})[key];`); + } else { + server = server.replace('// @@GET_ENV@@', 'return _getEnv(key);'); + } return { client, diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 871a123160..b706f967d3 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -191,7 +191,6 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest i18n: i18nManifest, checkOrigin: (settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false, - envGetSecretEnabled: false, key: hasEnvironmentKey() ? getEnvironmentKey() : createKey(), middleware() { return { diff --git a/packages/astro/src/vite-plugin-env/index.ts b/packages/astro/src/vite-plugin-env/index.ts index f68012927c..76d295089f 100644 --- a/packages/astro/src/vite-plugin-env/index.ts +++ b/packages/astro/src/vite-plugin-env/index.ts @@ -1,63 +1,14 @@ -import { fileURLToPath } from 'node:url'; import { transform } from 'esbuild'; import MagicString from 'magic-string'; import type * as vite from 'vite'; -import { loadEnv } from 'vite'; -import type { AstroSettings } from '../types/astro.js'; -import type { AstroConfig } from '../types/public/config.js'; +import type { EnvLoader } from '../env/env-loader.js'; interface EnvPluginOptions { - settings: AstroSettings; + envLoader: EnvLoader; } // Match `import.meta.env` directly without trailing property access const importMetaEnvOnlyRe = /\bimport\.meta\.env\b(?!\.)/; -// Match valid JS variable names (identifiers), which accepts most alphanumeric characters, -// except that the first character cannot be a number. -const isValidIdentifierRe = /^[_$a-zA-Z][\w$]*$/; - -function getPrivateEnv( - viteConfig: vite.ResolvedConfig, - astroConfig: AstroConfig, -): Record { - let envPrefixes: string[] = ['PUBLIC_']; - if (viteConfig.envPrefix) { - envPrefixes = Array.isArray(viteConfig.envPrefix) - ? viteConfig.envPrefix - : [viteConfig.envPrefix]; - } - - // Loads environment variables from `.env` files and `process.env` - const fullEnv = loadEnv( - viteConfig.mode, - viteConfig.envDir ?? fileURLToPath(astroConfig.root), - '', - ); - - const privateEnv: Record = {}; - for (const key in fullEnv) { - // Ignore public env var - if (isValidIdentifierRe.test(key) && envPrefixes.every((prefix) => !key.startsWith(prefix))) { - if (typeof process.env[key] !== 'undefined') { - let value = process.env[key]; - // Replacements are always strings, so try to convert to strings here first - if (typeof value !== 'string') { - value = `${value}`; - } - // Boolean values should be inlined to support `export const prerender` - // We already know that these are NOT sensitive values, so inlining is safe - if (value === '0' || value === '1' || value === 'true' || value === 'false') { - privateEnv[key] = value; - } else { - privateEnv[key] = `process.env.${key}`; - } - } else { - privateEnv[key] = JSON.stringify(fullEnv[key]); - } - } - } - return privateEnv; -} function getReferencedPrivateKeys(source: string, privateEnv: Record): Set { const references = new Set(); @@ -114,13 +65,12 @@ async function replaceDefine( }; } -export default function envVitePlugin({ settings }: EnvPluginOptions): vite.Plugin { +export default function envVitePlugin({ envLoader }: EnvPluginOptions): vite.Plugin { let privateEnv: Record; let defaultDefines: Record; let isDev: boolean; let devImportMetaEnvPrepend: string; let viteConfig: vite.ResolvedConfig; - const { config: astroConfig } = settings; return { name: 'astro:vite-plugin-env', config(_, { command }) { @@ -152,7 +102,9 @@ export default function envVitePlugin({ settings }: EnvPluginOptions): vite.Plug } // Find matches for *private* env and do our own replacement. - privateEnv ??= getPrivateEnv(viteConfig, astroConfig); + // Env is retrieved before process.env is populated by astro:env + // so that import.meta.env is first replaced by values, not process.env + privateEnv ??= envLoader.getPrivateEnv(); // In dev, we can assign the private env vars to `import.meta.env` directly for performance if (isDev) { diff --git a/packages/astro/templates/env.mjs b/packages/astro/templates/env.mjs index 4144dde5b7..4461ed035b 100644 --- a/packages/astro/templates/env.mjs +++ b/packages/astro/templates/env.mjs @@ -2,14 +2,23 @@ import { schema } from 'virtual:astro:env/internal'; import { createInvalidVariablesError, - getEnv, + getEnv as _getEnv, getEnvFieldType, setOnSetGetEnv, validateEnvVariable, } from 'astro/env/runtime'; +// @ts-expect-error +/** @returns {string} */ +// used while generating the virtual module +// biome-ignore lint/correctness/noUnusedFunctionParameters: `key` is used by the generated code +// biome-ignore lint/correctness/noUnusedVariables: `key` is used by the generated code +const getEnv = (key) => { + // @@GET_ENV@@ +}; + export const getSecret = (key) => { - return getEnv(key); + return getEnv(key) }; const _internalGetSecret = (key) => { @@ -25,9 +34,6 @@ const _internalGetSecret = (key) => { throw createInvalidVariablesError(key, type, result); }; -// used while generating the virtual module -// biome-ignore lint/correctness/noUnusedFunctionParameters: `reset` is used by the generated code -// biome-ignore lint/correctness/noUnusedVariables: `reset` is used by the generated code -setOnSetGetEnv((reset) => { +setOnSetGetEnv(() => { // @@ON_SET_GET_ENV@@ });