Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion secrets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,12 @@ of those too; add a matching `secrets:` entry in `docker-compose.yml` (or a
Everything in this directory except this README is gitignored. `scripts/selfhost-init-secrets.sh`
generates a real random value for each self-generatable file (so `docker compose build`/`up`
never fails on a missing file, and boots without any manual `openssl` step) and creates only an
**empty** placeholder for the four externally-issued ones it can't generate a usable value for. Either
**empty** placeholder for the four externally-issued ones it can't generate a usable value for. Those
four placeholders are safe to leave empty: the app treats an empty file for `GITHUB_APP_PRIVATE_KEY_FILE`,
`ORB_ENROLLMENT_SECRET_FILE`, `PAGERDUTY_ROUTING_KEY_FILE`, or `CLAUDE_CODE_OAUTH_TOKEN_FILE` as "not
configured", skips it, and logs a `selfhost_secret_file_empty_optional` warning naming the variable. An empty
file for any OTHER secret is still a hard boot failure -- there it can only mean a truncated write, which
would otherwise leave you running with a credential that silently reads as unset. Either
way, it only ever touches the *permissions* of a file that is still empty, never its content — the
moment a real value lands in one (written by the script or by you), both the content and whatever
mode you set are left alone on every future run. Always safe to re-run.
39 changes: 39 additions & 0 deletions src/selfhost/load-file-secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,32 @@ import { readFileSync } from "node:fs";
// named exactly one of these.
const COMPOSE_RESERVED_FILE_VARS = new Set(["COMPOSE_FILE", "COMPOSE_ENV_FILE"]);

/**
* The secrets `scripts/selfhost-init-secrets.sh` deliberately leaves EMPTY, for which empty therefore means
* "not configured yet" rather than "truncated".
*
* WHY THIS EXISTS: #9487 made an empty secret file fatal at boot, which is right for a secret the init
* script fills with a real random value -- an empty one there can only mean a truncated write, and the bug
* it fixed (a truncated GITHUB_WEBHOOK_SECRET booting an instance that silently rejected every webhook) is
* exactly that. But these four come from an EXTERNAL party, so the init script cannot generate them and
* creates a zero-byte placeholder instead (secrets/README.md says so explicitly). Compose also requires the
* file to exist before the stack will start. The result was that running the documented setup and starting
* the container crash-looped it -- observed on the ORB, where an unused GitHub App key did precisely that.
*
* So for these four ONLY, an empty file is skipped rather than fatal, and loudly logged. That is strictly
* better than the pre-#9487 behavior it superficially resembles: back then an empty file silently became an
* empty env var that every `nonBlank()` downstream read as unconfigured, with no signal at all. Here the
* target var is left genuinely unset and the operator gets a named warning at boot.
*
* Every other secret keeps #9487's fail-closed behavior unchanged.
*/
const OPTIONAL_WHEN_EMPTY_FILE_VARS = new Set([
"GITHUB_APP_PRIVATE_KEY_FILE",
"ORB_ENROLLMENT_SECRET_FILE",
"PAGERDUTY_ROUTING_KEY_FILE",
"CLAUDE_CODE_OAUTH_TOKEN_FILE",
]);

/** `env` and `readFile` are injectable purely for testability -- every real caller uses the defaults
* (`process.env`, `node:fs`'s `readFileSync`), so this is byte-identical to a hardcoded version at
* runtime while letting tests pass a plain object and a mock reader instead of mutating global state. */
Expand Down Expand Up @@ -59,6 +85,19 @@ export function loadFileSecrets(
// re-reported as "unreadable", collapsing two genuinely different operator problems (a bad path/permission
// vs a truncated write) into one misleading message and the wrong log event.
if (value === "") {
// An externally-issued secret the init script could only stub out: empty is "not configured", so skip
// it and leave the target var genuinely unset -- but say so, loudly and by name.
if (OPTIONAL_WHEN_EMPTY_FILE_VARS.has(key)) {
console.warn(
JSON.stringify({
level: "warn",
event: "selfhost_secret_file_empty_optional",
var: key,
message: `${key} points at an empty file (${path}); treating it as not configured. Write the issued value if you need this capability.`,
}),
);
continue;
}
console.error(
JSON.stringify({
level: "error",
Expand Down
59 changes: 59 additions & 0 deletions test/unit/selfhost-load-file-secrets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,62 @@ describe("loadFileSecrets return value (#9543 — which vars actually came from
expect(loadFileSecrets(env, () => "unused")).toEqual([]);
});
});

describe("empty placeholders for externally-issued secrets (#9487 follow-up)", () => {
// scripts/selfhost-init-secrets.sh creates a ZERO-BYTE placeholder for the four secrets it cannot
// generate, and compose refuses to start unless the file exists. #9487 then made an empty file fatal, so
// running the documented setup and starting the container crash-looped it. Observed live on the ORB: an
// unused GitHub App key took the whole instance down on upgrade.
const EXTERNALLY_ISSUED = [
"GITHUB_APP_PRIVATE_KEY_FILE",
"ORB_ENROLLMENT_SECRET_FILE",
"PAGERDUTY_ROUTING_KEY_FILE",
"CLAUDE_CODE_OAUTH_TOKEN_FILE",
];

it.each(EXTERNALLY_ISSUED)("REGRESSION: an empty %s is skipped, not fatal", (key) => {
const env: Record<string, string | undefined> = { [key]: "/run/secrets/placeholder" };
expect(() => loadFileSecrets(env, () => "")).not.toThrow();
// Left genuinely UNSET rather than set to "" -- the pre-#9487 bug was an empty string that read as
// configured-but-blank to anything checking presence rather than truthiness.
expect(env[key.slice(0, -"_FILE".length)]).toBeUndefined();
});

it("REGRESSION: reproduces the exact ORB boot crash and shows it is fixed", () => {
const env: Record<string, string | undefined> = {
GITHUB_APP_PRIVATE_KEY_FILE: "/run/secrets/github_app_private_key",
GITHUB_WEBHOOK_SECRET_FILE: "/run/secrets/github_webhook_secret",
};
const files: Record<string, string> = {
"/run/secrets/github_app_private_key": "",
"/run/secrets/github_webhook_secret": "a-real-webhook-secret",
};
expect(() => loadFileSecrets(env, (path) => files[path] ?? "")).not.toThrow();
expect(env.GITHUB_APP_PRIVATE_KEY).toBeUndefined();
expect(env.GITHUB_WEBHOOK_SECRET).toBe("a-real-webhook-secret");
});

it("INVARIANT: #9487's fail-closed behavior is UNCHANGED for a self-generated secret", () => {
// The bug #9487 actually fixed: a truncated webhook secret booting an instance that silently rejected
// every webhook. The init script writes a real random value here, so empty can only mean truncation.
const env: Record<string, string | undefined> = { GITHUB_WEBHOOK_SECRET_FILE: "/run/secrets/github_webhook_secret" };
expect(() => loadFileSecrets(env, () => "")).toThrow(/is empty/);
});

it("whitespace-only is treated the same as empty for an optional secret", () => {
const env: Record<string, string | undefined> = { PAGERDUTY_ROUTING_KEY_FILE: "/run/secrets/pagerduty_routing_key" };
expect(() => loadFileSecrets(env, () => " \n\t ")).not.toThrow();
expect(env.PAGERDUTY_ROUTING_KEY).toBeUndefined();
});

it("a REAL value in an optional secret still loads normally", () => {
const env: Record<string, string | undefined> = { CLAUDE_CODE_OAUTH_TOKEN_FILE: "/run/secrets/claude_code_oauth_token" };
expect(loadFileSecrets(env, () => " real-token ")).toEqual(["CLAUDE_CODE_OAUTH_TOKEN"]);
expect(env.CLAUDE_CODE_OAUTH_TOKEN).toBe("real-token");
});

it("an UNREADABLE optional secret is still fatal — missing is not the same as deliberately empty", () => {
const env: Record<string, string | undefined> = { GITHUB_APP_PRIVATE_KEY_FILE: "/run/secrets/gone" };
expect(() => loadFileSecrets(env, () => { throw new Error("ENOENT"); })).toThrow(/Failed to read secret file/);
});
});