From bd1d4aaf8262187b4f132d7fe0365902131ddf1a Mon Sep 17 00:00:00 2001 From: Matthew Phillips <matthew@skypack.dev> Date: Fri, 6 Sep 2024 12:41:39 -0400 Subject: [PATCH] Allow passing into the crypto key via ASTRO_KEY (#11879) * Allow passing into the crypto key via ASTRO_KEY * Add a changeset * Add test * Use the node package * omg * Create a new create-key command * linting * lint again * Update the changeset --- .changeset/four-tips-accept.md | 23 +++++++++++++ .../server-islands-key/astro.config.mjs | 12 +++++++ .../fixtures/server-islands-key/package.json | 12 +++++++ .../src/components/Island.astro | 6 ++++ .../server-islands-key/src/pages/index.astro | 14 ++++++++ packages/astro/e2e/server-islands-key.test.js | 33 +++++++++++++++++++ packages/astro/e2e/test-utils.js | 4 +++ packages/astro/src/cli/create-key/index.ts | 31 +++++++++++++++++ packages/astro/src/cli/index.ts | 8 +++++ packages/astro/src/core/build/index.ts | 7 ++-- packages/astro/src/core/encryption.ts | 29 ++++++++++++++++ packages/astro/src/core/logger/core.ts | 1 + .../server-islands/ssr/astro.config.mjs | 2 ++ pnpm-lock.yaml | 9 +++++ 14 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 .changeset/four-tips-accept.md create mode 100644 packages/astro/e2e/fixtures/server-islands-key/astro.config.mjs create mode 100644 packages/astro/e2e/fixtures/server-islands-key/package.json create mode 100644 packages/astro/e2e/fixtures/server-islands-key/src/components/Island.astro create mode 100644 packages/astro/e2e/fixtures/server-islands-key/src/pages/index.astro create mode 100644 packages/astro/e2e/server-islands-key.test.js create mode 100644 packages/astro/src/cli/create-key/index.ts diff --git a/.changeset/four-tips-accept.md b/.changeset/four-tips-accept.md new file mode 100644 index 0000000000..bfab504736 --- /dev/null +++ b/.changeset/four-tips-accept.md @@ -0,0 +1,23 @@ +--- +'astro': patch +--- + +Allow passing a cryptography key via ASTRO_KEY + +For Server islands Astro creates a cryptography key in order to hash props for the islands, preventing accidental leakage of secrets. + +If you deploy to an environment with rolling updates then there could be multiple instances of your app with different keys, causing potential key mismatches. + +To fix this you can now pass the `ASTRO_KEY` environment variable to your build in order to reuse the same key. + +To generate a key use: + +``` +astro create-key +``` + +This will print out an environment variable to set like: + +``` +ASTRO_KEY=PIAuyPNn2aKU/bviapEuc/nVzdzZPizKNo3OqF/5PmQ= +``` diff --git a/packages/astro/e2e/fixtures/server-islands-key/astro.config.mjs b/packages/astro/e2e/fixtures/server-islands-key/astro.config.mjs new file mode 100644 index 0000000000..db1a7b4524 --- /dev/null +++ b/packages/astro/e2e/fixtures/server-islands-key/astro.config.mjs @@ -0,0 +1,12 @@ +import { defineConfig } from 'astro/config'; +import node from '@astrojs/node'; + +// https://astro.build/config +export default defineConfig({ + output: 'server', + adapter: node({ mode: 'standalone' }), + integrations: [], + experimental: { + serverIslands: true, + } +}); diff --git a/packages/astro/e2e/fixtures/server-islands-key/package.json b/packages/astro/e2e/fixtures/server-islands-key/package.json new file mode 100644 index 0000000000..b03c575c7e --- /dev/null +++ b/packages/astro/e2e/fixtures/server-islands-key/package.json @@ -0,0 +1,12 @@ +{ + "name": "@e2e/server-islands-key", + "version": "0.0.0", + "private": true, + "scripts": { + "dev": "astro dev" + }, + "dependencies": { + "astro": "workspace:*", + "@astrojs/node": "^8.3.3" + } +} diff --git a/packages/astro/e2e/fixtures/server-islands-key/src/components/Island.astro b/packages/astro/e2e/fixtures/server-islands-key/src/components/Island.astro new file mode 100644 index 0000000000..5eab0dc4df --- /dev/null +++ b/packages/astro/e2e/fixtures/server-islands-key/src/components/Island.astro @@ -0,0 +1,6 @@ +--- +const { secret } = Astro.props; +--- +<h2 id="island">I am an island</h2> +<slot /> +<h3 id="secret">{secret}</h3> diff --git a/packages/astro/e2e/fixtures/server-islands-key/src/pages/index.astro b/packages/astro/e2e/fixtures/server-islands-key/src/pages/index.astro new file mode 100644 index 0000000000..a19aa8a275 --- /dev/null +++ b/packages/astro/e2e/fixtures/server-islands-key/src/pages/index.astro @@ -0,0 +1,14 @@ +--- +import Island from '../components/Island.astro'; +--- + +<html> + <head> + <!-- Head Stuff --> + </head> + <body> + <Island server:defer secret="test"> + <h3 id="children">children</h3> + </Island> + </body> +</html> diff --git a/packages/astro/e2e/server-islands-key.test.js b/packages/astro/e2e/server-islands-key.test.js new file mode 100644 index 0000000000..2120d1368f --- /dev/null +++ b/packages/astro/e2e/server-islands-key.test.js @@ -0,0 +1,33 @@ +import { expect } from '@playwright/test'; +import { testFactory } from './test-utils.js'; + +const test = testFactory(import.meta.url, { root: './fixtures/server-islands-key/' }); + +test.describe('Server islands - Key reuse', () => { + test.describe('Production', () => { + let previewServer; + + test.beforeAll(async ({ astro }) => { + // Playwright's Node version doesn't have these functions, so stub them. + process.stdout.clearLine = () => {}; + process.stdout.cursorTo = () => {}; + process.env.ASTRO_KEY = 'eKBaVEuI7YjfanEXHuJe/pwZKKt3LkAHeMxvTU7aR0M='; + await astro.build(); + previewServer = await astro.preview(); + }); + + test.afterAll(async () => { + await previewServer.stop(); + delete process.env.ASTRO_KEY; + }); + + test('Components render properly', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/')); + + let el = page.locator('#island'); + + await expect(el, 'element rendered').toBeVisible(); + await expect(el, 'should have content').toHaveText('I am an island'); + }); + }); +}); diff --git a/packages/astro/e2e/test-utils.js b/packages/astro/e2e/test-utils.js index 933186a718..7ae2e552a5 100644 --- a/packages/astro/e2e/test-utils.js +++ b/packages/astro/e2e/test-utils.js @@ -14,6 +14,10 @@ const testFileToPort = new Map(); for (let i = 0; i < testFiles.length; i++) { const file = testFiles[i]; if (file.endsWith('.test.js')) { + // Port 4045 is an unsafe port in Chrome, so skip it. + if((4000 + i) === 4045) { + i++; + } testFileToPort.set(file, 4000 + i); } } diff --git a/packages/astro/src/cli/create-key/index.ts b/packages/astro/src/cli/create-key/index.ts new file mode 100644 index 0000000000..55091d5059 --- /dev/null +++ b/packages/astro/src/cli/create-key/index.ts @@ -0,0 +1,31 @@ +import { type Flags, flagsToAstroInlineConfig } from '../flags.js'; +import { createNodeLogger } from '../../core/config/logging.js'; +import { createKey as createCryptoKey,encodeKey } from '../../core/encryption.js'; + +interface CreateKeyOptions { + flags: Flags; +} + +export async function createKey({ flags }: CreateKeyOptions): Promise<0 | 1> { + try { + const inlineConfig = flagsToAstroInlineConfig(flags); + const logger = createNodeLogger(inlineConfig); + + const keyPromise = createCryptoKey(); + const key = await keyPromise; + const encoded = await encodeKey(key); + + logger.info('crypto', `Generated a key to encrypt props passed to Server islands. To reuse the same key across builds, set this value as ASTRO_KEY in an environment variable on your build server. + +ASTRO_KEY=${encoded}`); + } catch(err: unknown) { + if(err != null) { + // eslint-disable-next-line no-console + console.error(err.toString()); + } + return 1; + } + + + return 0; +} diff --git a/packages/astro/src/cli/index.ts b/packages/astro/src/cli/index.ts index c767569fde..23486f938a 100644 --- a/packages/astro/src/cli/index.ts +++ b/packages/astro/src/cli/index.ts @@ -7,6 +7,7 @@ type CLICommand = | 'help' | 'version' | 'add' + | 'create-key' | 'docs' | 'dev' | 'build' @@ -30,6 +31,7 @@ async function printAstroHelp() { ['add', 'Add an integration.'], ['build', 'Build your project and write it to disk.'], ['check', 'Check your project for errors.'], + ['create-key', 'Create a cryptography key'], ['db', 'Manage your Astro database.'], ['dev', 'Start the development server.'], ['docs', 'Open documentation in your web browser.'], @@ -78,6 +80,7 @@ function resolveCommand(flags: yargs.Arguments): CLICommand { 'build', 'preview', 'check', + 'create-key', 'docs', 'db', 'info', @@ -111,6 +114,11 @@ async function runCommand(cmd: string, flags: yargs.Arguments) { await printInfo({ flags }); return; } + case 'create-key': { + const { createKey } = await import('./create-key/index.js'); + const exitCode = await createKey({ flags }); + return process.exit(exitCode); + } case 'docs': { const { docs } = await import('./docs/index.js'); await docs({ flags }); diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 70d2401284..4253b8802f 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -23,7 +23,7 @@ import { resolveConfig } from '../config/config.js'; import { createNodeLogger } from '../config/logging.js'; import { createSettings } from '../config/settings.js'; import { createVite } from '../create-vite.js'; -import { createKey } from '../encryption.js'; +import { createKey, getEnvironmentKey, hasEnvironmentKey } from '../encryption.js'; import type { Logger } from '../logger/core.js'; import { levels, timerMessage } from '../logger/core.js'; import { apply as applyPolyfill } from '../polyfill.js'; @@ -188,6 +188,9 @@ class AstroBuilder { green(`✓ Completed in ${getTimeStat(this.timer.init, performance.now())}.`), ); + const hasKey = hasEnvironmentKey(); + const keyPromise = hasKey ? getEnvironmentKey() : createKey(); + const opts: StaticBuildOptions = { allPages, settings: this.settings, @@ -198,7 +201,7 @@ class AstroBuilder { pageNames, teardownCompiler: this.teardownCompiler, viteConfig, - key: createKey(), + key: keyPromise, }; const { internals, ssrOutputChunkNames, contentFileNames } = await viteBuild(opts); diff --git a/packages/astro/src/core/encryption.ts b/packages/astro/src/core/encryption.ts index ccfc9bdd27..7cfba99d94 100644 --- a/packages/astro/src/core/encryption.ts +++ b/packages/astro/src/core/encryption.ts @@ -20,6 +20,35 @@ export async function createKey() { return key; } +// The environment variable name that can be used to provide the encrypted key. +const ENVIRONMENT_KEY_NAME = 'ASTRO_KEY' as const; + +/** + * Get the encoded value of the ASTRO_KEY env var. + */ +export function getEncodedEnvironmentKey(): string { + return process.env[ENVIRONMENT_KEY_NAME] || ''; +} + +/** + * See if the environment variable key ASTRO_KEY is set. + */ +export function hasEnvironmentKey(): boolean { + return getEncodedEnvironmentKey() !== ''; +} + +/** + * Get the environment variable key and decode it into a CryptoKey. + */ +export async function getEnvironmentKey(): Promise<CryptoKey> { + // This should never happen, because we always check `hasEnvironmentKey` before this is called. + if(!hasEnvironmentKey()) { + throw new Error(`There is no environment key defined. If you see this error there is a bug in Astro.`) + } + const encodedKey = getEncodedEnvironmentKey(); + return decodeKey(encodedKey); +} + /** * Takes a key that has been serialized to an array of bytes and returns a CryptoKey */ diff --git a/packages/astro/src/core/logger/core.ts b/packages/astro/src/core/logger/core.ts index c06866df5c..51ebd9325b 100644 --- a/packages/astro/src/core/logger/core.ts +++ b/packages/astro/src/core/logger/core.ts @@ -18,6 +18,7 @@ export type LoggerLabel = | 'check' | 'config' | 'content' + | 'crypto' | 'deprecated' | 'markdown' | 'router' diff --git a/packages/astro/test/fixtures/server-islands/ssr/astro.config.mjs b/packages/astro/test/fixtures/server-islands/ssr/astro.config.mjs index 8eb474b048..79ce4c497a 100644 --- a/packages/astro/test/fixtures/server-islands/ssr/astro.config.mjs +++ b/packages/astro/test/fixtures/server-islands/ssr/astro.config.mjs @@ -1,7 +1,9 @@ import svelte from '@astrojs/svelte'; import { defineConfig } from 'astro/config'; +import testAdapter from '../../../test-adapter.js'; export default defineConfig({ + adapter: testAdapter(), output: 'server', integrations: [ svelte() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 34b6143194..29ef628484 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1615,6 +1615,15 @@ importers: specifier: ^18.3.1 version: 18.3.1(react@18.3.1) + packages/astro/e2e/fixtures/server-islands-key: + dependencies: + '@astrojs/node': + specifier: ^8.3.3 + version: 8.3.3(astro@packages+astro) + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/e2e/fixtures/solid-circular: dependencies: '@astrojs/solid-js':