Skip to content

Commit

Permalink
fix: do not freeze process.env in dev (#12585)
Browse files Browse the repository at this point in the history
Co-authored-by: Emanuele Stoppa <[email protected]>
  • Loading branch information
florian-lefebvre and ematipico authored Dec 9, 2024
1 parent e21c7e6 commit a9373c0
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 81 deletions.
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@@
});

0 comments on commit a9373c0

Please sign in to comment.