From 95c545e6b8395090455cbe794627cf614d989545 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Fri, 11 Jul 2025 09:23:43 -0400 Subject: [PATCH 1/2] fix: pass all required env vars to edge function invocations We weren't passing "internally" defined (within this codebase) env vars through because when we copied the logic over from Netlify CLI we refactored some mechanisms that ended up changing some behaviours unintentionally. For "internal" env vars (currently, this is `NETLIFY_BLOBS_CONTEXT`, `NETLIFY_PURGE_API_TOKEN`, and `BRANCH`), we were mutating `process.env` to set these in an earlier code path than one where we determine the `source` of each env var, which affects all sorts of logic. This ended up making it tagged as `usedSource: 'process'`, which resulted in them being filtered from the env var bag passed to the deno bridge. (It thought that these were coming from the developer's own machine, i.e. the parent process running this dev server, and should be excluded.) --- package-lock.json | 2 +- packages/dev/src/lib/env.ts | 22 ++++ packages/dev/src/lib/runtime.ts | 1 + packages/dev/src/main.test.ts | 195 +++++++++++++++++++++++++++++--- packages/dev/src/main.ts | 24 ++-- 5 files changed, 214 insertions(+), 30 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9e62545f..043da0a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11738,7 +11738,7 @@ "node": "^20.6.1 || >=22" }, "peerDependencies": { - "vite": "^5 || ^6" + "vite": "^5 || ^6 || ^7" } }, "packages/vite-plugin/node_modules/@types/node": { diff --git a/packages/dev/src/lib/env.ts b/packages/dev/src/lib/env.ts index 7ee21723..01db44b9 100644 --- a/packages/dev/src/lib/env.ts +++ b/packages/dev/src/lib/env.ts @@ -42,6 +42,7 @@ interface InjectEnvironmentVariablesOptions { accountSlug?: string baseVariables: Record envAPI: EnvironmentVariables + envSnapshot: Record netlifyAPI?: NetlifyAPI siteID?: string } @@ -50,6 +51,7 @@ export const injectEnvVariables = async ({ accountSlug, baseVariables = {}, envAPI, + envSnapshot, netlifyAPI, siteID, }: InjectEnvironmentVariablesOptions) => { @@ -66,6 +68,26 @@ export const injectEnvVariables = async ({ }) } + // We've kept track of env vars we've set ourselves so far. These are "internal" env vars, + // part of the runtime emulation. They've already been populated on the actual env - we just + // need to record them in the results. + // TODO(serhalp): It would likely be cleaner to populate these on the env here rather than + // have them be a special case. + for (const key of Object.keys(envSnapshot)) { + const result: InjectedEnvironmentVariable = { + isInternal: true, + originalValue: 'unused', + overriddenSources: [], + usedSource: 'internal', + value: envAPI.get(key) ?? '', + } + + results[key] = result + } + + // Then we inject all the rest of the env vars, which come from multiple `source`s and have + // been collected from `@netlify/config` and/or Envelope. These have not been populated on the + // actual env yet, so we need to do that now too. for (const [key, variable] of Object.entries(variables)) { const existsInProcess = envAPI.has(key) const [usedSource, ...overriddenSources] = existsInProcess ? ['process', ...variable.sources] : variable.sources diff --git a/packages/dev/src/lib/runtime.ts b/packages/dev/src/lib/runtime.ts index 2bc14f28..9ea5db95 100644 --- a/packages/dev/src/lib/runtime.ts +++ b/packages/dev/src/lib/runtime.ts @@ -71,6 +71,7 @@ export const getRuntime = async ({ blobs, deployID, projectRoot, siteID }: GetRu return { env, + envSnapshot, stop: async () => { restoreEnvironment(envSnapshot) diff --git a/packages/dev/src/main.test.ts b/packages/dev/src/main.test.ts index e35a726a..672ae799 100644 --- a/packages/dev/src/main.test.ts +++ b/packages/dev/src/main.test.ts @@ -2,7 +2,7 @@ import { readFile } from 'node:fs/promises' import { resolve } from 'node:path' import { createImageServerHandler, Fixture, generateImage, getImageResponseSize, HTTPServer } from '@netlify/dev-utils' -import { describe, expect, test } from 'vitest' +import { afterEach, describe, expect, test, vi } from 'vitest' import { isFile } from './lib/fs.js' import { NetlifyDev } from './main.js' @@ -10,6 +10,10 @@ import { NetlifyDev } from './main.js' import { withMockApi } from '../test/mock-api.js' describe('Handling requests', () => { + afterEach(() => { + vi.unstubAllEnvs() + }) + describe('No linked site', () => { test('Same-site rewrite to a static file', async () => { const fixture = new Fixture() @@ -425,7 +429,7 @@ describe('Handling requests', () => { 'netlify/functions/hello.mjs', `export default async () => { const cache = await caches.open("my-cache"); - + await cache.put("https://example.com", new Response("Cached response")); return new Response("Hello world"); @@ -533,6 +537,130 @@ describe('Handling requests', () => { await dev.stop() await fixture.destroy() }) + + test('Invoking an edge function', async () => { + const fixture = new Fixture() + .withFile( + 'netlify.toml', + `[build] + publish = "public" + [context.dev.environment] + MY_TOKEN = "value from dev context" + [context.deploy-preview.environment] + MY_OTHER_TOKEN = "value from deploy preview context" + `, + ) + .withFile( + 'netlify/functions/hello.mjs', + `export default async (req, context) => new Response("Hello from function"); + + export const config = { path: "/hello/:a/*" };`, + ) + .withFile( + 'netlify/edge-functions/passthrough.mjs', + `export default async (req, context) => { + const res = await context.next(); + const text = await res.text(); + + return new Response(text.toUpperCase(), res); + }; + + export const config = { path: "/hello/passthrough/*" };`, + ) + .withFile( + 'netlify/edge-functions/terminate.mjs', + `export default async (req, context) => Response.json({ + runtimeEnv: { + NETLIFY_BLOBS_CONTEXT: Netlify.env.get("NETLIFY_BLOBS_CONTEXT"), + }, + platformEnv: { + DEPLOY_ID: Netlify.env.get("DEPLOY_ID"), + }, + configEnv: { + MY_TOKEN: Netlify.env.get("MY_TOKEN"), + MY_OTHER_TOKEN: Netlify.env.get("MY_OTHER_TOKEN"), + }, + parentProcessEnv: { + SOME_ZSH_THING_MAYBE: Netlify.env.get("SOME_ZSH_THING_MAYBE"), + }, + geo: context.geo, + params: context.params, + path: context.path, + server: context.server, + site: context.site, + url: context.url, + }); + + export const config = { path: "/hello/terminate/*" };`, + ) + const directory = await fixture.create() + + vi.stubEnv('SOME_ZSH_THING_MAYBE', 'value on developer machine') + + const dev = new NetlifyDev({ + apiToken: 'token', + projectRoot: directory, + }) + + const { serverAddress } = await dev.start() + + const req1 = new Request('https://site.netlify/hello/passthrough/two/three') + const res1 = await dev.handle(req1) + + expect(await res1?.text()).toBe('HELLO FROM FUNCTION') + + const req2 = new Request('https://site.netlify/hello/terminate/two/three') + const res2 = await dev.handle(req2) + const req2URL = new URL('/hello/terminate/two/three', serverAddress) + + expect(await res2?.json()).toStrictEqual({ + // Env vars emulating the EF runtime are present + runtimeEnv: { + NETLIFY_BLOBS_CONTEXT: expect.stringMatching(/\w+/) as unknown, + }, + // Env vars emulating the EF runtime are present + // Note that these originate from `@netlify/config` + platformEnv: { + DEPLOY_ID: '0', + }, + // Envs var set in `netlify.toml` for `dev` context only are passed to EFs + configEnv: { + MY_TOKEN: 'value from dev context', + // MY_OTHER_TOKEN is not present + }, + parentProcessEnv: { + // SOME_ZSH_THING_MAYBE is not present + }, + + geo: { + city: 'San Francisco', + country: { + code: 'US', + name: 'United States', + }, + latitude: 0, + longitude: 0, + subdivision: { + code: 'CA', + name: 'California', + }, + timezone: 'UTC', + }, + params: { + '0': 'two/three', + }, + server: { + region: 'dev', + }, + site: { + url: serverAddress, + }, + url: req2URL.toString(), + }) + + await dev.stop() + await fixture.destroy() + }) }) describe('With linked site', () => { @@ -587,7 +715,7 @@ describe('Handling requests', () => { `export default async (req, context) => Response.json({ env: { WITH_DEV_OVERRIDE: Netlify.env.get("WITH_DEV_OVERRIDE"), - WITHOUT_DEV_OVERRIDE: Netlify.env.get("WITHOUT_DEV_OVERRIDE") + WITHOUT_DEV_OVERRIDE: Netlify.env.get("WITHOUT_DEV_OVERRIDE") }, geo: context.geo, params: context.params, @@ -596,7 +724,7 @@ describe('Handling requests', () => { site: context.site, url: context.url }); - + export const config = { path: "/hello/:a/*" };`, ) .withStateFile({ siteId: 'site_id' }) @@ -659,13 +787,17 @@ describe('Handling requests', () => { .withFile( 'netlify.toml', `[build] - publish = "public" + publish = "public" + [context.dev.environment] + MY_TOKEN = "value from dev context" + [context.deploy-preview.environment] + MY_OTHER_TOKEN = "value from deploy preview context" `, ) .withFile( 'netlify/functions/hello.mjs', `export default async (req, context) => new Response("Hello from function"); - + export const config = { path: "/hello/:a/*" };`, ) .withFile( @@ -676,30 +808,45 @@ describe('Handling requests', () => { return new Response(text.toUpperCase(), res); }; - + export const config = { path: "/hello/passthrough/*" };`, ) .withFile( 'netlify/edge-functions/terminate.mjs', `export default async (req, context) => Response.json({ - env: { + siteEnv: { WITH_DEV_OVERRIDE: Netlify.env.get("WITH_DEV_OVERRIDE"), - WITHOUT_DEV_OVERRIDE: Netlify.env.get("WITHOUT_DEV_OVERRIDE") + WITHOUT_DEV_OVERRIDE: Netlify.env.get("WITHOUT_DEV_OVERRIDE"), + }, + runtimeEnv: { + NETLIFY_BLOBS_CONTEXT: Netlify.env.get("NETLIFY_BLOBS_CONTEXT"), + }, + platformEnv: { + DEPLOY_ID: Netlify.env.get("DEPLOY_ID"), + }, + configEnv: { + MY_TOKEN: Netlify.env.get("MY_TOKEN"), + MY_OTHER_TOKEN: Netlify.env.get("MY_OTHER_TOKEN"), + }, + parentProcessEnv: { + SOME_ZSH_THING_MAYBE: Netlify.env.get("SOME_ZSH_THING_MAYBE"), }, geo: context.geo, params: context.params, path: context.path, server: context.server, site: context.site, - url: context.url + url: context.url, }); - + export const config = { path: "/hello/terminate/*" };`, ) .withStateFile({ siteId: 'site_id' }) const directory = await fixture.create() await withMockApi(routes, async (context) => { + vi.stubEnv('SOME_ZSH_THING_MAYBE', 'value on developer machine') + const dev = new NetlifyDev({ apiURL: context.apiUrl, apiToken: 'token', @@ -718,10 +865,32 @@ describe('Handling requests', () => { const req2URL = new URL('/hello/terminate/two/three', serverAddress) expect(await res2?.json()).toStrictEqual({ - env: { + // Env vars set on the site ("UI") are passed to EFs + siteEnv: { WITH_DEV_OVERRIDE: 'value from dev context', WITHOUT_DEV_OVERRIDE: 'value from all context', }, + // Env vars emulating the EF runtime are present + // TODO(serhalp): Test conditionally injected `NETLIFY_PURGE_API_TOKEN` + // TODO(serhalp): Finish implementing and test conditionally injected `BRANCH` + runtimeEnv: { + NETLIFY_BLOBS_CONTEXT: expect.stringMatching(/\w+/) as unknown, + }, + // Env vars emulating the EF runtime are present + // Note that these originate from `@netlify/config` + platformEnv: { + DEPLOY_ID: '0', + }, + // Envs var set in `netlify.toml` for `dev` context only are passed to EFs + configEnv: { + MY_TOKEN: 'value from dev context', + // MY_OTHER_TOKEN is not present + }, + parentProcessEnv: { + // SOME_ZSH_THING_MAYBE is not present + }, + // TODO(serhalp): Implement and test support for `.env.*` files (exists in CLI) + geo: { city: 'San Francisco', country: { @@ -769,7 +938,7 @@ describe('Handling requests', () => { .withFile( 'netlify/functions/greeting.mjs', `export default async (req, context) => new Response(context.params.greeting + ", friend!"); - + export const config = { path: "/:greeting", preferStatic: true };`, ) .withFile('public/hello.html', 'Hello') diff --git a/packages/dev/src/main.ts b/packages/dev/src/main.ts index 1608f1f0..a168ba0d 100644 --- a/packages/dev/src/main.ts +++ b/packages/dev/src/main.ts @@ -447,28 +447,20 @@ export class NetlifyDev { accountSlug: config?.siteInfo?.account_slug, baseVariables: config?.env || {}, envAPI: runtime.env, + envSnapshot: runtime.envSnapshot, netlifyAPI: config?.api, siteID, }) } if (this.#features.edgeFunctions) { - const env = Object.entries(envVariables).reduce>((acc, [key, variable]) => { - if ( - variable.usedSource === 'account' || - variable.usedSource === 'addons' || - variable.usedSource === 'internal' || - variable.usedSource === 'ui' || - variable.usedSource.startsWith('.env') - ) { - return { - ...acc, - [key]: variable.value, - } - } - - return acc - }, {}) + const env = Object.entries(envVariables).reduce>( + (acc, [key, variable]) => ({ + ...acc, + [key]: variable.value, + }), + {}, + ) const edgeFunctionsHandler = new EdgeFunctionsHandler({ configDeclarations: this.#config?.config.edge_functions ?? [], From f4b3e1710246f1c8fc82a23a9de95704eff9c4ea Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Fri, 11 Jul 2025 14:31:12 -0400 Subject: [PATCH 2/2] refactor: move internal env var merging closer to EFs --- packages/dev/src/lib/env.ts | 35 +++++++++++++---------------------- packages/dev/src/main.ts | 31 ++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/packages/dev/src/lib/env.ts b/packages/dev/src/lib/env.ts index 01db44b9..8c25db14 100644 --- a/packages/dev/src/lib/env.ts +++ b/packages/dev/src/lib/env.ts @@ -42,16 +42,25 @@ interface InjectEnvironmentVariablesOptions { accountSlug?: string baseVariables: Record envAPI: EnvironmentVariables - envSnapshot: Record netlifyAPI?: NetlifyAPI siteID?: string } +/** + * Inject user-defined environment variables (from various sources, see `@netlify/config`) + * into the provided `envAPI` (which may be a proxy to `process.env`, affecting the current proc), + * if `siteID` and `accountSlug` are provided. + * @see {@link https://github.com/netlify/build/blob/8b7583e1890636bd64b54e20aee40ae5365edeaf/packages/config/src/env/main.ts#L92} + * + * This also injects and returns the documented runtime env vars: + * @see {@link https://docs.netlify.com/functions/environment-variables/#functions} + * + * @return Metadata about all injected environment variables + */ export const injectEnvVariables = async ({ accountSlug, baseVariables = {}, envAPI, - envSnapshot, netlifyAPI, siteID, }: InjectEnvironmentVariablesOptions) => { @@ -68,26 +77,8 @@ export const injectEnvVariables = async ({ }) } - // We've kept track of env vars we've set ourselves so far. These are "internal" env vars, - // part of the runtime emulation. They've already been populated on the actual env - we just - // need to record them in the results. - // TODO(serhalp): It would likely be cleaner to populate these on the env here rather than - // have them be a special case. - for (const key of Object.keys(envSnapshot)) { - const result: InjectedEnvironmentVariable = { - isInternal: true, - originalValue: 'unused', - overriddenSources: [], - usedSource: 'internal', - value: envAPI.get(key) ?? '', - } - - results[key] = result - } - - // Then we inject all the rest of the env vars, which come from multiple `source`s and have - // been collected from `@netlify/config` and/or Envelope. These have not been populated on the - // actual env yet, so we need to do that now too. + // Inject env vars which come from multiple `source`s and have been collected from + // `@netlify/config` and/or Envelope. These have not been populated on the actual env yet. for (const [key, variable] of Object.entries(variables)) { const existsInProcess = envAPI.has(key) const [usedSource, ...overriddenSources] = existsInProcess ? ['process', ...variable.sources] : variable.sources diff --git a/packages/dev/src/main.ts b/packages/dev/src/main.ts index a168ba0d..12b63ff9 100644 --- a/packages/dev/src/main.ts +++ b/packages/dev/src/main.ts @@ -447,25 +447,38 @@ export class NetlifyDev { accountSlug: config?.siteInfo?.account_slug, baseVariables: config?.env || {}, envAPI: runtime.env, - envSnapshot: runtime.envSnapshot, netlifyAPI: config?.api, siteID, }) } if (this.#features.edgeFunctions) { - const env = Object.entries(envVariables).reduce>( - (acc, [key, variable]) => ({ - ...acc, - [key]: variable.value, - }), - {}, - ) + const edgeFunctionsEnv = { + // User-defined env vars + documented runtime env vars + ...Object.entries(envVariables).reduce>( + (acc, [key, variable]) => ({ + ...acc, + [key]: variable.value, + }), + {}, + ), + // Add runtime env vars that we've set ourselves so far. These are "internal" env vars, + // part of the runtime emulation. They've already been populated on this process's env, which + // is needed to make other dev features work. These are different than the "documented" runtime + // env vars, in that they are implementation details, needed to make our features work. + ...Object.keys(runtime.envSnapshot).reduce>( + (acc, key) => ({ + ...acc, + [key]: runtime.env.get(key) ?? '', + }), + {}, + ), + } const edgeFunctionsHandler = new EdgeFunctionsHandler({ configDeclarations: this.#config?.config.edge_functions ?? [], directories: [this.#config?.config.build.edge_functions].filter(Boolean) as string[], - env, + env: edgeFunctionsEnv, geolocation: mockLocation, logger: this.#logger, siteID,