Skip to content

Commit

Permalink
fix: Update edge-bundler and ensure only allowed environment variable…
Browse files Browse the repository at this point in the history
…s are supplied to edge functions (#4919)

* fix(deps): update dependency @netlify/edge-bundler to ^1.12.0

* fix: restrict environment variables available in edge functions

* chore: also allow variables from account and addons

* chore: disable DENO_DEPLOYMENT_ID

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
  • Loading branch information
danez and renovate[bot] committed Aug 10, 2022
1 parent 868ec26 commit df0c767
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 11 deletions.
16 changes: 9 additions & 7 deletions npm-shrinkwrap.json

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

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -222,7 +222,7 @@
"dependencies": {
"@netlify/build": "^27.8.1",
"@netlify/config": "^18.1.2",
"@netlify/edge-bundler": "^1.8.0",
"@netlify/edge-bundler": "^1.12.1",
"@netlify/framework-info": "^9.2.0",
"@netlify/local-functions-proxy": "^1.1.1",
"@netlify/plugins-list": "^6.37.0",
Expand Down
4 changes: 4 additions & 0 deletions src/commands/dev/dev.js
Expand Up @@ -233,6 +233,7 @@ const FRAMEWORK_PORT_TIMEOUT = 6e5
* @param {object} params
* @param {*} params.addonsUrls
* @param {import('../base-command').NetlifyOptions["config"]} params.config
* @param {import('../base-command').NetlifyOptions["cachedConfig"]['env']} params.env
* @param {InspectSettings} params.inspectSettings
* @param {() => Promise<object>} params.getUpdatedConfig
* @param {string} params.geolocationMode
Expand All @@ -247,6 +248,7 @@ const FRAMEWORK_PORT_TIMEOUT = 6e5
const startProxyServer = async ({
addonsUrls,
config,
env,
geoCountry,
geolocationMode,
getUpdatedConfig,
Expand All @@ -261,6 +263,7 @@ const startProxyServer = async ({
addonsUrls,
config,
configPath: site.configPath,
env,
geolocationMode,
geoCountry,
getUpdatedConfig,
Expand Down Expand Up @@ -479,6 +482,7 @@ const dev = async (options, command) => {
let url = await startProxyServer({
addonsUrls,
config,
env: command.netlify.cachedConfig.env,
geolocationMode: options.geo,
geoCountry: options.country,
getUpdatedConfig,
Expand Down
4 changes: 4 additions & 0 deletions src/lib/edge-functions/proxy.js
Expand Up @@ -56,6 +56,7 @@ const createSiteInfoHeader = (siteInfo = {}) => {
const initializeProxy = async ({
config,
configPath,
env: configEnv,
geoCountry,
geolocationMode,
getUpdatedConfig,
Expand All @@ -79,6 +80,7 @@ const initializeProxy = async ({
config,
configPath,
directories: [internalFunctionsPath, userFunctionsPath].filter(Boolean),
env: configEnv,
getUpdatedConfig,
importMaps: [importMap].filter(Boolean),
inspectSettings,
Expand Down Expand Up @@ -150,6 +152,7 @@ const prepareServer = async ({
config,
configPath,
directories,
env: configEnv,
getUpdatedConfig,
importMaps,
inspectSettings,
Expand Down Expand Up @@ -180,6 +183,7 @@ const prepareServer = async ({
config,
configPath,
directories,
env: configEnv,
getUpdatedConfig,
internalFunctions,
projectDir,
Expand Down
32 changes: 29 additions & 3 deletions src/lib/edge-functions/registry.js
Expand Up @@ -27,13 +27,14 @@ class EdgeFunctionsRegistry {
* @param {() => Promise<object>} opts.getUpdatedConfig
* @param {EdgeFunction[]} opts.internalFunctions
* @param {string} opts.projectDir
* @param {(functions: EdgeFunction[]) => Promise<object>} opts.runIsolate
* @param {(functions: EdgeFunction[], env?: NodeJS.ProcessEnv) => Promise<object>} opts.runIsolate
*/
constructor({
bundler,
config,
configPath,
directories,
env,
getUpdatedConfig,
internalFunctions,
projectDir,
Expand Down Expand Up @@ -65,7 +66,7 @@ class EdgeFunctionsRegistry {
this.internalFunctions = internalFunctions

/**
* @type {(functions: EdgeFunction[]) => Promise<object>}
* @type {(functions: EdgeFunction[], env?: NodeJS.ProcessEnv) => Promise<object>}
*/
this.runIsolate = runIsolate

Expand All @@ -79,6 +80,11 @@ class EdgeFunctionsRegistry {
*/
this.declarations = this.getDeclarations(config)

/**
* @type {Record<string, string>}
*/
this.env = EdgeFunctionsRegistry.getEnvironmentVariables(env)

/**
* @type {Map<string, import('chokidar').FSWatcher>}
*/
Expand Down Expand Up @@ -107,7 +113,7 @@ class EdgeFunctionsRegistry {
*/
async build(functions) {
try {
const { graph, success } = await this.runIsolate(functions)
const { graph, success } = await this.runIsolate(functions, this.env)

if (!success) {
throw new Error('Build error')
Expand Down Expand Up @@ -178,6 +184,26 @@ class EdgeFunctionsRegistry {
return declarations
}

static getEnvironmentVariables(envConfig) {
const env = Object.create(null)
Object.entries(envConfig).forEach(([key, variable]) => {
if (
variable.sources.includes('ui') ||
variable.sources.includes('account') ||
variable.sources.includes('addons')
) {
env[key] = variable.value
}
})

env.DENO_REGION = 'local'
// We use it in the bootstrap layer to detect whether we're running in production or not
// (see https://github.com/netlify/edge-functions-bootstrap/blob/main/src/bootstrap/environment.ts#L2)
// env.DENO_DEPLOYMENT_ID = 'xxx='

return env
}

getManifest() {
return this.bundler.generateManifest({ declarations: this.declarations, functions: this.functions })
}
Expand Down
2 changes: 2 additions & 0 deletions src/utils/proxy.js
Expand Up @@ -465,6 +465,7 @@ const startProxy = async function ({
addonsUrls,
config,
configPath,
env,
geoCountry,
geolocationMode,
getUpdatedConfig,
Expand All @@ -479,6 +480,7 @@ const startProxy = async function ({
const edgeFunctionsProxy = await edgeFunctions.initializeProxy({
config,
configPath,
env,
geolocationMode,
geoCountry,
getUpdatedConfig,
Expand Down
80 changes: 80 additions & 0 deletions tests/integration/100.command.dev.test.js
Expand Up @@ -536,4 +536,84 @@ test('should detect deleted edge functions', async (t) => {
})
})

test('should have only allowed environment variables set', async (t) => {
const siteInfo = {
account_slug: 'test-account',
id: 'site_id',
name: 'site-name',
build_settings: {
env: {
SECRET_ENV: 'true',
},
},
}

const routes = [
{ path: 'sites/site_id', response: siteInfo },
{ path: 'sites/site_id/service-instances', response: [] },
{
path: 'accounts',
response: [{ slug: siteInfo.account_slug }],
},
]
await withSiteBuilder('site-with-edge-functions-and-env', async (builder) => {
const publicDir = 'public'
builder
.withNetlifyToml({
config: {
build: {
publish: publicDir,
edge_functions: 'netlify/edge-functions',
},
edge_functions: [
{
function: 'env',
path: '/env',
},
],
},
})
.withEdgeFunction({
// eslint-disable-next-line no-undef
handler: () => new Response(`${JSON.stringify(Deno.env.toObject())}`),
name: 'env',
})

await builder.buildAsync()

await withMockApi(routes, async ({ apiUrl }) => {
await withDevServer(
{
cwd: builder.directory,
offline: false,
env: {
NETLIFY_API_URL: apiUrl,
NETLIFY_SITE_ID: 'site_id',
NETLIFY_AUTH_TOKEN: 'fake-token',
},
},
async ({ port }) => {
const response = await got(`http://localhost:${port}/env`).then((edgeResponse) =>
JSON.parse(edgeResponse.body),
)
const envKeys = Object.keys(response)

t.false(envKeys.includes('DENO_DEPLOYMENT_ID'))
// t.true(envKeys.includes('DENO_DEPLOYMENT_ID'))
// t.is(response.DENO_DEPLOYMENT_ID, 'xxx=')
t.true(envKeys.includes('DENO_REGION'))
t.is(response.DENO_REGION, 'local')
t.true(envKeys.includes('SECRET_ENV'))
t.is(response.SECRET_ENV, 'true')

t.false(envKeys.includes('NODE_ENV'))
t.false(envKeys.includes('NETLIFY_DEV'))
t.false(envKeys.includes('DEPLOY_URL'))
t.false(envKeys.includes('URL'))
},
)
})
})
})

/* eslint-enable require-await */

1 comment on commit df0c767

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

Package size: 228 MB

Please sign in to comment.