mirror of
https://github.com/outline/outline.git
synced 2026-08-03 13:27:25 +03:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF * refactor: Generalize reserved file variable documentation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF * docs: Trim withFileSecrets JSDoc Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqP3Gb29iG5bJmhYuEAAjF --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
3cc39e61c0
commit
2b6ee79778
+4
-3
@@ -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
|
||||
|
||||
@@ -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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
_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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
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<string, string | undefined> = {
|
||||
_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<string, string | undefined> = {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
+26
-24
@@ -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 `<NAME>_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<string, string | undefined>
|
||||
): void {
|
||||
for (const key of Object.keys(env)) {
|
||||
if (key.endsWith("_FILE")) {
|
||||
const baseKey = key.slice(0, -5);
|
||||
if (!baseKey.length) {
|
||||
continue;
|
||||
): Record<string, string | undefined> {
|
||||
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);
|
||||
|
||||
@@ -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"),
|
||||
|
||||
Reference in New Issue
Block a user