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: astro:env secret in middleware #12326

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fluffy-paws-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a case where using an `astro:env` secret inside a middleware would crash in SSR
12 changes: 4 additions & 8 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import type {
StylesheetAsset,
} from './types.js';
import { getTimeStat, shouldAppendForwardSlash } from './util.js';
import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js';

function createEntryURL(filePath: string, outFolder: URL) {
return new URL('./' + filePath + `?time=${Date.now()}`, outFolder);
Expand All @@ -65,14 +66,9 @@ export async function generatePages(options: StaticBuildOptions, internals: Buil
const baseDirectory = getOutputDirectory(options.settings.config);
const renderersEntryUrl = new URL('renderers.mjs', baseDirectory);
const renderers = await import(renderersEntryUrl.toString());
let middleware: MiddlewareHandler = (_, next) => next();
try {
// middleware.mjs is not emitted if there is no user middleware
// in which case the import fails with ERR_MODULE_NOT_FOUND, and we fall back to a no-op middleware
middleware = await import(new URL('middleware.mjs', baseDirectory).toString()).then(
(mod) => mod.onRequest,
);
} catch {}
const middleware: MiddlewareHandler = internals.middlewareEntryPoint
? await import(internals.middlewareEntryPoint.toString()).then((mod) => mod.onRequest)
: NOOP_MIDDLEWARE_FN;
manifest = createBuildManifest(
options.settings,
internals,
Expand Down
16 changes: 7 additions & 9 deletions packages/astro/src/core/build/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,13 @@ export class BuildPipeline extends Pipeline {
const renderersEntryUrl = new URL(`renderers.mjs?time=${Date.now()}`, baseDirectory);
const renderers = await import(renderersEntryUrl.toString());

const middleware = await import(new URL('middleware.mjs', baseDirectory).toString())
.then((mod) => {
return function () {
return { onRequest: mod.onRequest };
};
})
// middleware.mjs is not emitted if there is no user middleware
// in which case the import fails with ERR_MODULE_NOT_FOUND, and we fall back to a no-op middleware
.catch(() => manifest.middleware);
const middleware = internals.middlewareEntryPoint
? await import(internals.middlewareEntryPoint.toString()).then((mod) => {
return function () {
return { onRequest: mod.onRequest };
};
})
: manifest.middleware;

if (!renderers) {
throw new Error(
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/plugins/plugin-pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V
imports.push(`import { renderers } from "${RENDERERS_MODULE_ID}";`);
exports.push(`export { renderers };`);

return `${imports.join('\n')}${exports.join('\n')}`;
return `/* patch secret here */ ${imports.join('\n')}${exports.join('\n')}`;
}
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/build/plugins/plugin-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ function generateSSRCode(settings: AstroSettings, adapter: AstroAdapter, middlew
`import { renderers } from '${RENDERERS_MODULE_ID}';`,
`import * as serverEntrypointModule from '${ADAPTER_VIRTUAL_MODULE_ID}';`,
`import { manifest as defaultManifest } from '${SSR_MANIFEST_VIRTUAL_MODULE_ID}';`,
edgeMiddleware ? `` : `import { onRequest as middleware } from '${middlewareId}';`,
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
settings.config.experimental.serverIslands
? `import { serverIslandMap } from '${VIRTUAL_ISLAND_MAP_ID}';`
: '',
Expand Down
16 changes: 2 additions & 14 deletions packages/astro/src/core/middleware/vite-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@ export const MIDDLEWARE_MODULE_ID = '\0astro-internal:middleware';
const NOOP_MIDDLEWARE = '\0noop-middleware';

export function vitePluginMiddleware({ settings }: { settings: AstroSettings }): VitePlugin {
let isCommandBuild = false;
let resolvedMiddlewareId: string | undefined = undefined;
const hasIntegrationMiddleware =
settings.middlewares.pre.length > 0 || settings.middlewares.post.length > 0;
let userMiddlewareIsPresent = false;

return {
name: '@astro/plugin-middleware',
config(_, { command }) {
isCommandBuild = command === 'build';
},
async resolveId(id) {
if (id === MIDDLEWARE_MODULE_ID) {
const middlewareId = await this.resolve(
Expand Down Expand Up @@ -53,20 +49,12 @@ export function vitePluginMiddleware({ settings }: { settings: AstroSettings }):
if (!userMiddlewareIsPresent && settings.config.i18n?.routing === 'manual') {
throw new AstroError(MissingMiddlewareForInternationalization);
}
// In the build, tell Vite to emit this file
if (isCommandBuild) {
this.emitFile({
type: 'chunk',
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
preserveSignature: 'strict',
fileName: 'middleware.mjs',
id,
});
}

const preMiddleware = createMiddlewareImports(settings.middlewares.pre, 'pre');
const postMiddleware = createMiddlewareImports(settings.middlewares.post, 'post');

const source = `
/* patch secrets here only when we are building */
${
userMiddlewareIsPresent
? `import { onRequest as userOnRequest } from '${resolvedMiddlewareId}';`
Expand Down Expand Up @@ -124,7 +112,7 @@ export function vitePluginMiddlewareBuild(

writeBundle(_, bundle) {
for (const [chunkName, chunk] of Object.entries(bundle)) {
if (chunk.type !== 'asset' && chunk.fileName === 'middleware.mjs') {
if (chunk.type !== 'asset' && chunk.facadeModuleId === MIDDLEWARE_MODULE_ID) {
const outputDirectory = getOutputDirectory(opts.settings.config);
internals.middlewareEntryPoint = new URL(chunkName, outputDirectory);
}
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/env/runtime-constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const ENV_SYMBOL = Symbol.for('astro:env/dev');
6 changes: 5 additions & 1 deletion packages/astro/src/env/runtime.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { AstroError, AstroErrorData } from '../core/errors/index.js';
import { invalidVariablesToError } from './errors.js';
import { ENV_SYMBOL } from './runtime-constants.js';
import type { ValidationResultInvalid } from './validators.js';
export { validateEnvVariable, getEnvFieldType } from './validators.js';

export type GetEnv = (key: string) => string | undefined;
type OnSetGetEnv = (reset: boolean) => void;

let _getEnv: GetEnv = (key) => process.env[key];
let _getEnv: GetEnv = (key) => {
const env = (globalThis as any)[ENV_SYMBOL] ?? {};
return env[key];
};

export function setGetEnv(fn: GetEnv, reset = false) {
_getEnv = fn;
Expand Down
7 changes: 2 additions & 5 deletions packages/astro/src/env/vite-plugin-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
import { type InvalidVariable, invalidVariablesToError } from './errors.js';
import type { EnvSchema } from './schema.js';
import { getEnvFieldType, validateEnvVariable } from './validators.js';
import { ENV_SYMBOL } from './runtime-constants.js';

interface AstroEnvVirtualModPluginParams {
settings: AstroSettings;
Expand Down Expand Up @@ -41,11 +42,7 @@ export function astroEnv({
fileURLToPath(settings.config.root),
'',
);
for (const [key, value] of Object.entries(loadedEnv)) {
if (value !== undefined) {
process.env[key] = value;
}
}
(globalThis as any)[ENV_SYMBOL] = loadedEnv;

const validatedVariables = validatePublicVariables({
schema,
Expand Down
34 changes: 21 additions & 13 deletions packages/astro/test/env-secret.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict';
import { afterEach, describe, it } from 'node:test';
import { afterEach, before, describe, it } from 'node:test';
import * as cheerio from 'cheerio';
import testAdapter from './test-adapter.js';
import { loadFixture } from './test-utils.js';
Expand All @@ -15,6 +15,9 @@ describe('astro:env secret variables', () => {
if (process.env.KNOWN_SECRET) {
delete process.env.KNOWN_SECRET;
}
if (process.env.UNKNOWN_SECRET) {
delete process.env.UNKNOWN_SECRET;
}
});

it('works in dev', async () => {
Expand All @@ -28,30 +31,24 @@ describe('astro:env secret variables', () => {
});

it('builds without throwing', async () => {
process.env.KNOWN_SECRET = '123456';
process.env.UNKNOWN_SECRET = 'abc';
fixture = await loadFixture({
root: './fixtures/astro-env-server-secret/',
output: 'server',
adapter: testAdapter({
env: {
KNOWN_SECRET: '123456',
UNKNOWN_SECRET: 'abc',
},
}),
adapter: testAdapter(),
});
await fixture.build();
assert.equal(true, true);
});

it('adapter can set how env is retrieved', async () => {
process.env.KNOWN_SECRET = '123456';
process.env.UNKNOWN_SECRET = 'abc';
fixture = await loadFixture({
root: './fixtures/astro-env-server-secret/',
output: 'server',
adapter: testAdapter({
env: {
KNOWN_SECRET: '123456',
UNKNOWN_SECRET: 'abc',
},
}),
adapter: testAdapter(),
});
await fixture.build();
const app = await fixture.loadTestAdapterApp();
Expand Down Expand Up @@ -87,4 +84,15 @@ describe('astro:env secret variables', () => {
assert.equal(error.message.includes('KNOWN_SECRET is missing'), true);
}
});

it('should render using env secrets in static pages', async () => {
process.env.KNOWN_SECRET = '123456';
fixture = await loadFixture({
root: './fixtures/astro-env-secret-static-pages/',
});
await fixture.build();
const indexHtml = await fixture.readFile('/index.html');

assert.match(indexHtml, /123456/);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { defineConfig, envField } from 'astro/config';

// https://astro.build/config
export default defineConfig({
experimental: {
env: {
schema: {
KNOWN_SECRET: envField.number({ context: "server", access: "secret" })
}
}
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/astro-env-secret-static-pages",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
import { getSecret, KNOWN_SECRET } from "astro:env/server"

const UNKNOWN_SECRET = getSecret("UNKNOWN_SECRET")
---

<pre id="data">{JSON.stringify({ KNOWN_SECRET, UNKNOWN_SECRET })}</pre>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "astro/tsconfigs/base"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineMiddleware } from 'astro/middleware';
import { KNOWN_SECRET } from 'astro:env/server';

export const onRequest = defineMiddleware(async (ctx, next) => {
console.log({ KNOWN_SECRET });
return await next();
});
15 changes: 2 additions & 13 deletions packages/astro/test/test-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { viteID } from '../dist/core/util.js';
* setEntryPoints?: (entryPoints: EntryPoints) => void;
* setMiddlewareEntryPoint?: (middlewareEntryPoint: MiddlewareEntryPoint) => void;
* setRoutes?: (routes: Routes) => void;
* env: Record<string, string | undefined>;
* }} param0
* @returns {AstroIntegration}
*/
Expand All @@ -26,7 +25,6 @@ export default function ({
setEntryPoints,
setMiddlewareEntryPoint,
setRoutes,
env,
} = {}) {
return {
name: 'my-ssr-adapter',
Expand All @@ -49,18 +47,9 @@ export default function ({
return `
import { App } from 'astro/app';
import fs from 'fs';
import { setGetEnv } from 'astro/env/setup';

${
env
? `
await import('astro/env/setup')
.then(mod => mod.setGetEnv((key) => {
const data = ${JSON.stringify(env)};
return data[key];
}))
.catch(() => {});`
: ''
}
setGetEnv((key) => process.env[key]);

class MyApp extends App {
#manifest = null;
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading