From 2b6ee79778b137c4d4daf8b76331a0bb34374590 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 6 Jul 2026 22:14:03 +0200 Subject: [PATCH] fix: all `_FILE` suffix secrets in env are cemented at startup (#12889) * fix: Resolve _FILE env secrets lazily to avoid clobbering third-party variables The Docker-style secrets support added in #11906 eagerly copied every *_FILE environment variable into its base variable at boot. This broke AWS SDK credential refresh on EKS Pod Identity: the rotating token in AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE was frozen into the static AWS_CONTAINER_AUTHORIZATION_TOKEN variable, which the SDK prefers and never re-reads, so S3 access failed once the boot-time token expired. It similarly resolved the standard OpenSSL SSL_CERT_FILE CA bundle into Outline's SSL_CERT setting, failing validation at startup. File secrets are now resolved lazily through a proxy when a variable is read off the environment export, which limits resolution to variables declared on the Environment classes, and SSL_CERT_FILE is explicitly reserved for OpenSSL. Fixes #12885 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF * refactor: Generalize reserved file variable documentation Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF * refactor: Clear SSL_CERT_FILE in test environment instead of reserving it Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF * docs: Trim withFileSecrets JSDoc Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF --------- Co-authored-by: Claude --- .env.sample | 7 +- server/utils/environment.test.ts | 125 ++++++++++++++++++++++--------- server/utils/environment.ts | 50 +++++++------ vitest.config.ts | 6 ++ 4 files changed, 124 insertions(+), 64 deletions(-) diff --git a/.env.sample b/.env.sample index cbdfc371b3..d56832fc34 100644 --- a/.env.sample +++ b/.env.sample @@ -4,9 +4,10 @@ NODE_ENV=production # ––––––––––– FILE-BASED SECRETS –––––––– # ––––––––––––––––––––––––––––––––––––––––– # -# Any environment variable can be loaded from a file by appending _FILE to the -# variable name and setting the value to the path of the file. This is useful -# for Docker secrets and other file-based secret management systems. +# Any environment variable read by Outline can be loaded from a file by +# appending _FILE to the variable name and setting the value to the path of the +# file. This is useful for Docker secrets and other file-based secret +# management systems. # # For example, instead of: # SECRET_KEY=your_secret_key diff --git a/server/utils/environment.test.ts b/server/utils/environment.test.ts index 90c859c18c..81ff2f6b44 100644 --- a/server/utils/environment.test.ts +++ b/server/utils/environment.test.ts @@ -1,9 +1,9 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { resolveFileSecrets } from "./environment"; +import { withFileSecrets } from "./environment"; -describe("resolveFileSecrets", () => { +describe("withFileSecrets", () => { let tmpDir: string; beforeEach(() => { @@ -18,11 +18,9 @@ describe("resolveFileSecrets", () => { const secretFile = path.join(tmpDir, "secret"); fs.writeFileSync(secretFile, "my-secret-value"); - const env: Record = { + const env = withFileSecrets({ TEST_SECRET_FILE: secretFile, - }; - - resolveFileSecrets(env); + }); expect(env.TEST_SECRET).toBe("my-secret-value"); }); @@ -31,11 +29,9 @@ describe("resolveFileSecrets", () => { const secretFile = path.join(tmpDir, "secret"); fs.writeFileSync(secretFile, " my-secret-value\n\n"); - const env: Record = { + const env = withFileSecrets({ TEST_TRIM_FILE: secretFile, - }; - - resolveFileSecrets(env); + }); expect(env.TEST_TRIM).toBe("my-secret-value"); }); @@ -44,12 +40,10 @@ describe("resolveFileSecrets", () => { const secretFile = path.join(tmpDir, "secret"); fs.writeFileSync(secretFile, "file-value"); - const env: Record = { + const env = withFileSecrets({ TEST_OVERRIDE: "direct-value", TEST_OVERRIDE_FILE: secretFile, - }; - - resolveFileSecrets(env); + }); expect(env.TEST_OVERRIDE).toBe("direct-value"); }); @@ -58,63 +52,120 @@ describe("resolveFileSecrets", () => { const secretFile = path.join(tmpDir, "secret"); fs.writeFileSync(secretFile, "file-value"); - const env: Record = { + const env = withFileSecrets({ TEST_OVERRIDE_EMPTY: "", TEST_OVERRIDE_EMPTY_FILE: secretFile, - }; - - resolveFileSecrets(env); + }); expect(env.TEST_OVERRIDE_EMPTY).toBe(""); }); - it("should skip a bare _FILE key with no base name", () => { + it("should return the file path when reading the _FILE variable itself", () => { const secretFile = path.join(tmpDir, "secret"); - fs.writeFileSync(secretFile, "value"); + fs.writeFileSync(secretFile, "my-secret-value"); - const env: Record = { - _FILE: secretFile, + const env = withFileSecrets({ + TEST_PATH_FILE: secretFile, + }); + + expect(env.TEST_PATH_FILE).toBe(secretFile); + }); + + it("should not materialize base variables that are never read", () => { + const tokenFile = path.join(tmpDir, "token"); + fs.writeFileSync(tokenFile, "rotating-token"); + + const record: Record = { + AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE: tokenFile, }; + withFileSecrets(record); - resolveFileSecrets(env); + expect(record.AWS_CONTAINER_AUTHORIZATION_TOKEN).toBeUndefined(); + expect( + Object.keys(record).includes("AWS_CONTAINER_AUTHORIZATION_TOKEN") + ).toBe(false); + }); - expect(env[""]).toBeUndefined(); + it("should cache the resolved value on the underlying record", () => { + const secretFile = path.join(tmpDir, "secret"); + fs.writeFileSync(secretFile, "first-value"); + + const record: Record = { + TEST_CACHE_FILE: secretFile, + }; + const env = withFileSecrets(record); + + expect(env.TEST_CACHE).toBe("first-value"); + expect(record.TEST_CACHE).toBe("first-value"); + + // Subsequent reads must not hit the filesystem again. + fs.rmSync(secretFile); + expect(env.TEST_CACHE).toBe("first-value"); }); it("should handle missing file gracefully", () => { - const env: Record = { + const env = withFileSecrets({ TEST_MISSING_FILE: path.join(tmpDir, "nonexistent"), - }; - - resolveFileSecrets(env); + }); expect(env.TEST_MISSING).toBeUndefined(); }); it("should skip _FILE entries with empty path", () => { - const env: Record = { + const env = withFileSecrets({ TEST_EMPTY_FILE: "", - }; - - resolveFileSecrets(env); + }); expect(env.TEST_EMPTY).toBeUndefined(); }); - it("should process multiple _FILE entries", () => { + it("should ignore a bare _FILE key with no base name", () => { + const secretFile = path.join(tmpDir, "secret"); + fs.writeFileSync(secretFile, "value"); + + const record: Record = { + _FILE: secretFile, + }; + const env = withFileSecrets(record); + + expect(env[""]).toBeUndefined(); + expect(record[""]).toBeUndefined(); + }); + + it("should resolve multiple _FILE entries independently", () => { const file1 = path.join(tmpDir, "secret1"); const file2 = path.join(tmpDir, "secret2"); fs.writeFileSync(file1, "value1"); fs.writeFileSync(file2, "value2"); - const env: Record = { + const env = withFileSecrets({ SECRET_KEY_FILE: file1, DATABASE_PASSWORD_FILE: file2, - }; - - resolveFileSecrets(env); + }); expect(env.SECRET_KEY).toBe("value1"); expect(env.DATABASE_PASSWORD).toBe("value2"); }); + + it("should resolve Outline _FILE secrets through Environment while leaving AWS SDK variables untouched", async () => { + const tokenFile = path.join(tmpDir, "eks-pod-identity-token"); + const secretFile = path.join(tmpDir, "smtp-password"); + fs.writeFileSync(tokenFile, "rotating-jwt-token"); + fs.writeFileSync(secretFile, "smtp-secret-value\n"); + + process.env.AWS_CONTAINER_CREDENTIALS_FULL_URI = + "http://169.254.170.23/v1/credentials"; + process.env.AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE = tokenFile; + process.env.SMTP_PASSWORD_FILE = secretFile; + + vi.resetModules(); + const env = (await import("../env")).default; + + // Outline's own variable is resolved from the file. + expect(env.SMTP_PASSWORD).toBe("smtp-secret-value"); + + // The AWS SDK's token variable is never materialized, so the SDK keeps + // re-reading the rotated token file on every credential refresh. + expect(process.env.AWS_CONTAINER_AUTHORIZATION_TOKEN).toBeUndefined(); + }); }); diff --git a/server/utils/environment.ts b/server/utils/environment.ts index 0117494a9e..3a8d353af7 100644 --- a/server/utils/environment.ts +++ b/server/utils/environment.ts @@ -37,44 +37,46 @@ process.env = { }; /** - * Process environment variables with _FILE suffix by reading the referenced - * file and setting the base variable. If the base variable is already set, the - * file is not read. File contents are trimmed of leading/trailing whitespace. + * Wraps an environment record so that Docker-style file secrets are resolved + * lazily. When a variable is read through the proxy and has no value, but a + * corresponding `_FILE` variable is set, the referenced file is read and + * its contents – trimmed of leading/trailing whitespace – are cached on the + * underlying record and returned. If the base variable is already set, the + * file is not read. * - * @param env - the environment record to process. + * @param env - the environment record to wrap. + * @returns a proxy over the record that resolves `_FILE` secrets on read. */ -export function resolveFileSecrets( +export function withFileSecrets( env: Record -): void { - for (const key of Object.keys(env)) { - if (key.endsWith("_FILE")) { - const baseKey = key.slice(0, -5); - if (!baseKey.length) { - continue; +): Record { + return new Proxy(env, { + get(target, prop, receiver) { + if (typeof prop !== "string" || !prop.length || prop.endsWith("_FILE")) { + return Reflect.get(target, prop, receiver); } - const filePath = env[key]; + const value = target[prop]; + if (value !== undefined) { + return value; + } + const filePath = target[`${prop}_FILE`]; if (!filePath) { - continue; - } - - if (env[baseKey] !== undefined) { - continue; + return undefined; } try { - env[baseKey] = fs.readFileSync(filePath, "utf8").trim(); + target[prop] = fs.readFileSync(filePath, "utf8").trim(); } catch (err) { // oxlint-disable-next-line no-console console.error( - `Failed to read file for ${key} (${filePath}): ${(err as Error).message}` + `Failed to read file for ${prop}_FILE (${filePath}): ${(err as Error).message}` ); } - } - } + return target[prop]; + }, + }); } -resolveFileSecrets(process.env); - -export default process.env; +export default withFileSecrets(process.env); diff --git a/vitest.config.ts b/vitest.config.ts index 5fdbc68feb..1b4c364684 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,6 +2,12 @@ import path from "node:path"; import swc from "unplugin-swc"; import { defineConfig } from "vitest/config"; +// SSL_CERT_FILE is OpenSSL's CA bundle variable and may be present in the +// host environment running the tests; clear it so it is not resolved into +// Outline's SSL_CERT setting. The config is evaluated before the global setup +// and before workers spawn, so this covers every test process. +delete process.env.SSL_CERT_FILE; + const aliases = { "@server": path.resolve(__dirname, "./server"), "@shared": path.resolve(__dirname, "./shared"),