Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not freeze process.env in dev #12585

Merged
merged 10 commits into from
Dec 9, 2024
5 changes: 5 additions & 0 deletions .changeset/nasty-parrots-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a case where `process.env` would be frozen despite changes made to environment variables in development
1 change: 0 additions & 1 deletion packages/astro/src/container/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ function createManifest(
i18n: manifest?.i18n,
checkOrigin: false,
middleware: manifest?.middleware ?? middlewareInstance,
envGetSecretEnabled: false,
key: createKey(),
};
}
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export type SSRManifest = {
i18n: SSRManifestI18n | undefined;
middleware?: () => Promise<AstroMiddlewareInstance> | AstroMiddlewareInstance;
checkOrigin: boolean;
envGetSecretEnabled: boolean;
};

export type SSRManifestI18n = {
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,5 @@ function createBuildManifest(
checkOrigin:
(settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false,
key,
envGetSecretEnabled: false,
};
}
3 changes: 0 additions & 3 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
};
}
6 changes: 4 additions & 2 deletions packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 = {
Expand All @@ -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(),
Expand Down
60 changes: 60 additions & 0 deletions packages/astro/src/env/env-loader.ts
Original file line number Diff line number Diff line change
@@ -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<string, string>,
astroConfig: AstroConfig,
): Record<string, string> {
const viteConfig = astroConfig.vite;
let envPrefixes: string[] = ['PUBLIC_'];
if (viteConfig.envPrefix) {
envPrefixes = Array.isArray(viteConfig.envPrefix)
? viteConfig.envPrefix
: [viteConfig.envPrefix];
}

const privateEnv: Record<string, string> = {};
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<string, string> = {};
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<typeof createEnvLoader>;
6 changes: 3 additions & 3 deletions packages/astro/src/env/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {};
Expand Down
32 changes: 23 additions & 9 deletions packages/astro/src/env/vite-plugin-env.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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;
}
}
}

Expand All @@ -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)};`,
};
},
Expand Down Expand Up @@ -122,6 +130,7 @@ function validatePublicVariables({
function getTemplates(
schema: EnvSchema,
validatedVariables: ReturnType<typeof validatePublicVariables>,
loadedEnv: Record<string, string> | null,
) {
let client = '';
let server = readFileSync(MODULE_TEMPLATE_URL, 'utf-8');
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
60 changes: 6 additions & 54 deletions packages/astro/src/vite-plugin-env/index.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> {
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<string, string> = {};
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<string, any>): Set<string> {
const references = new Set<string>();
Expand Down Expand Up @@ -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<string, string>;
let defaultDefines: Record<string, string>;
let isDev: boolean;
let devImportMetaEnvPrepend: string;
let viteConfig: vite.ResolvedConfig;
const { config: astroConfig } = settings;
return {
name: 'astro:vite-plugin-env',
config(_, { command }) {
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 12 additions & 6 deletions packages/astro/templates/env.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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@@
});
Loading