diff --git a/server/src/__tests__/decisions-service.test.ts b/server/src/__tests__/decisions-service.test.ts index 7c0a546dbd..febd4223ab 100644 --- a/server/src/__tests__/decisions-service.test.ts +++ b/server/src/__tests__/decisions-service.test.ts @@ -1,4 +1,7 @@ import { randomUUID } from "node:crypto"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; import { eq } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -279,13 +282,38 @@ describePg("decisionService", () => { expect(result.executions[0]?.result).toMatchObject({ originReason: "deny_missing_membership" }); }); - it("refuses execution when the decision signing secret is unavailable", async () => { + it("fails closed when the configured signing secret is removed after proposal", async () => { const created = await createCommentDecision(); + const originalHome = process.env.PAPERCLIP_HOME; + const tempHome = mkdtempSync(path.join(tmpdir(), "paperclip-decision-rotate-")); + process.env.PAPERCLIP_HOME = tempHome; delete process.env.PAPERCLIP_DECISION_SIGNING_SECRET; process.env.PAPERCLIP_AGENT_JWT_SECRET = "agent-jwt-secret-must-not-sign-decisions"; - await expect(service().decide({ id: created.id, optionId: "yes", decidedByUserId, userActor: boardActor() })) - .rejects.toThrow("PAPERCLIP_DECISION_SIGNING_SECRET is required"); - expect(await db.select().from(issueComments).where(eq(issueComments.issueId, targetIssueId))).toHaveLength(0); + try { + await expect(service().decide({ id: created.id, optionId: "yes", decidedByUserId, userActor: boardActor() })) + .rejects.toThrow("Decision signature verification failed"); + expect(await db.select().from(issueComments).where(eq(issueComments.issueId, targetIssueId))).toHaveLength(0); + } finally { + if (originalHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalHome; + rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it("signs and verifies with an auto-generated key when no secret is configured", async () => { + const originalHome = process.env.PAPERCLIP_HOME; + const tempHome = mkdtempSync(path.join(tmpdir(), "paperclip-decision-generated-")); + process.env.PAPERCLIP_HOME = tempHome; + delete process.env.PAPERCLIP_DECISION_SIGNING_SECRET; + try { + const created = await createCommentDecision(); + const result = await service().decide({ id: created.id, optionId: "yes", decidedByUserId, userActor: boardActor() }); + expect(result.executionStatus).toBe("succeeded"); + } finally { + if (originalHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalHome; + rmSync(tempHome, { recursive: true, force: true }); + } }); it("records a failed effect and continues independent later effects", async () => { diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index 76a1ed981c..1abbefe45d 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; const ORIGINAL_PAPERCLIP_API_URL = process.env.PAPERCLIP_API_URL; const ORIGINAL_PAPERCLIP_RUNTIME_API_URL = process.env.PAPERCLIP_RUNTIME_API_URL; @@ -298,9 +301,92 @@ describe("startServer feedback export wiring", () => { process.env.BETTER_AUTH_SECRET = "test-secret"; }); - it("refuses startup when the decision signing secret is unavailable", async () => { + it("starts without PAPERCLIP_DECISION_SIGNING_SECRET by generating a persisted key", async () => { + const originalHome = process.env.PAPERCLIP_HOME; + const originalInstanceId = process.env.PAPERCLIP_INSTANCE_ID; + const tempHome = mkdtempSync(path.join(tmpdir(), "paperclip-decision-key-")); + process.env.PAPERCLIP_HOME = tempHome; + process.env.PAPERCLIP_INSTANCE_ID = "default"; delete process.env.PAPERCLIP_DECISION_SIGNING_SECRET; - await expect(startServer()).rejects.toThrow("PAPERCLIP_DECISION_SIGNING_SECRET is required"); + try { + const started = await startServer(); + expect(started.server).toBe(fakeServer); + const keyPath = path.join(tempHome, "instances", "default", "secrets", "decision-signing.key"); + expect(readFileSync(keyPath, "utf8").trim().length).toBeGreaterThanOrEqual(32); + if (process.platform !== "win32") { + expect(statSync(path.dirname(keyPath)).mode & 0o777).toBe(0o700); + expect(statSync(keyPath).mode & 0o777).toBe(0o600); + } + } finally { + if (originalHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalHome; + if (originalInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; + else process.env.PAPERCLIP_INSTANCE_ID = originalInstanceId; + rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it("repairs permissive permissions on an existing generated decision signing key", async () => { + const originalHome = process.env.PAPERCLIP_HOME; + const originalInstanceId = process.env.PAPERCLIP_INSTANCE_ID; + const tempHome = mkdtempSync(path.join(tmpdir(), "paperclip-decision-key-mode-")); + const keyPath = path.join(tempHome, "instances", "default", "secrets", "decision-signing.key"); + const existingKey = Buffer.alloc(32, 7).toString("base64"); + mkdirSync(path.dirname(keyPath), { recursive: true, mode: 0o777 }); + chmodSync(path.dirname(keyPath), 0o777); + writeFileSync(keyPath, existingKey, { encoding: "utf8", mode: 0o644 }); + chmodSync(keyPath, 0o644); + process.env.PAPERCLIP_HOME = tempHome; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + delete process.env.PAPERCLIP_DECISION_SIGNING_SECRET; + try { + const started = await startServer(); + expect(started.server).toBe(fakeServer); + expect(readFileSync(keyPath, "utf8")).toBe(existingKey); + if (process.platform !== "win32") { + expect(statSync(path.dirname(keyPath)).mode & 0o777).toBe(0o700); + expect(statSync(keyPath).mode & 0o777).toBe(0o600); + } + } finally { + if (originalHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalHome; + if (originalInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; + else process.env.PAPERCLIP_INSTANCE_ID = originalInstanceId; + rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it("refuses a symlink planted as the generated decision signing key", async () => { + if (process.platform === "win32") return; + + const originalHome = process.env.PAPERCLIP_HOME; + const originalInstanceId = process.env.PAPERCLIP_INSTANCE_ID; + const tempHome = mkdtempSync(path.join(tmpdir(), "paperclip-decision-key-symlink-")); + const keyPath = path.join(tempHome, "instances", "default", "secrets", "decision-signing.key"); + const plantedTarget = path.join(tempHome, "planted.key"); + const plantedKey = Buffer.alloc(32, 9).toString("base64"); + mkdirSync(path.dirname(keyPath), { recursive: true, mode: 0o777 }); + chmodSync(path.dirname(keyPath), 0o777); + writeFileSync(plantedTarget, plantedKey, { encoding: "utf8", mode: 0o600 }); + symlinkSync(plantedTarget, keyPath); + process.env.PAPERCLIP_HOME = tempHome; + process.env.PAPERCLIP_INSTANCE_ID = "default"; + delete process.env.PAPERCLIP_DECISION_SIGNING_SECRET; + try { + await expect(startServer()).rejects.toThrow("must be a regular file"); + expect(readFileSync(plantedTarget, "utf8")).toBe(plantedKey); + } finally { + if (originalHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = originalHome; + if (originalInstanceId === undefined) delete process.env.PAPERCLIP_INSTANCE_ID; + else process.env.PAPERCLIP_INSTANCE_ID = originalInstanceId; + rmSync(tempHome, { recursive: true, force: true }); + } + }); + + it("refuses startup when an explicit decision signing secret is too short", async () => { + process.env.PAPERCLIP_DECISION_SIGNING_SECRET = "too-short"; + await expect(startServer()).rejects.toThrow("PAPERCLIP_DECISION_SIGNING_SECRET must be at least 32 characters"); expect(loadConfigMock).not.toHaveBeenCalled(); }); diff --git a/server/src/index.ts b/server/src/index.ts index 734525a2f6..9ecc6a850b 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -72,7 +72,7 @@ import { getBoardClaimWarningUrl, initializeBoardClaimChallenge } from "./board- import { maybePersistWorktreeRuntimePorts } from "./worktree-config.js"; import { initTelemetry, getTelemetryClient } from "./telemetry.js"; import { conflict } from "./errors.js"; -import { validateDecisionSigningSecret } from "./services/decision-signing.js"; +import { ensureDecisionSigningSecret } from "./services/decision-signing.js"; import { createDecisionWakeOriginAgent } from "./services/decision-wakeup.js"; import { coordinateHeartbeatSchedulerShutdown } from "./shutdown.js"; import { systemdNotify } from "./services/systemd-notify.js"; @@ -123,7 +123,7 @@ export async function startServer(): Promise { // Tracing must be active (or have failed and logged) before the first DB // connection or the HTTP server exists — see instrumentation.ts. await instrumentationReady; - validateDecisionSigningSecret(); + ensureDecisionSigningSecret(); let config = loadConfig(); initTelemetry({ enabled: config.telemetryEnabled }); if (process.env.PAPERCLIP_SECRETS_PROVIDER === undefined) { diff --git a/server/src/services/decision-signing.ts b/server/src/services/decision-signing.ts index f2c38d4b18..365e20ac7b 100644 --- a/server/src/services/decision-signing.ts +++ b/server/src/services/decision-signing.ts @@ -1,11 +1,146 @@ -import { createHmac, timingSafeEqual } from "node:crypto"; +import { createHmac, randomBytes, timingSafeEqual } from "node:crypto"; +import { chmodSync, linkSync, lstatSync, mkdirSync, readFileSync, type Stats, unlinkSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { resolveDefaultSecretsKeyFilePath } from "../home-paths.js"; const VERSION = "decision-spec-v1"; +const MIN_SECRET_LENGTH = 32; -export function validateDecisionSigningSecret() { - const value = process.env.PAPERCLIP_DECISION_SIGNING_SECRET?.trim(); - if (!value || value.length < 32) throw new Error("PAPERCLIP_DECISION_SIGNING_SECRET is required and must be at least 32 characters"); - return value; +function resolveGeneratedSecretFilePath() { + return path.join(path.dirname(resolveDefaultSecretsKeyFilePath()), "decision-signing.key"); +} + +function assertOwnedByCurrentUser(stats: Stats, description: string) { + if (process.platform === "win32") return; + + const currentUserId = process.getuid?.(); + if (currentUserId !== undefined && stats.uid !== currentUserId) { + throw new Error(`${description} must be owned by the Paperclip process user`); + } +} + +function enforceKeyFilePermissions(keyPath: string) { + let stats = lstatSync(keyPath); + if (!stats.isFile()) { + throw new Error(`Decision signing key at ${keyPath} must be a regular file`); + } + assertOwnedByCurrentUser(stats, `Decision signing key at ${keyPath}`); + if (process.platform === "win32") return; + + const mode = stats.mode & 0o777; + if ((mode & 0o077) !== 0) { + chmodSync(keyPath, 0o600); + stats = lstatSync(keyPath); + if (!stats.isFile()) { + throw new Error(`Decision signing key at ${keyPath} must be a regular file`); + } + assertOwnedByCurrentUser(stats, `Decision signing key at ${keyPath}`); + if ((stats.mode & 0o077) !== 0) { + throw new Error(`Decision signing key at ${keyPath} must have permissions 0600`); + } + } +} + +function enforceSecretsDirectoryPermissions(directoryPath: string) { + let stats = lstatSync(directoryPath); + if (!stats.isDirectory()) { + throw new Error(`Decision signing secrets directory at ${directoryPath} must be a directory`); + } + assertOwnedByCurrentUser(stats, `Decision signing secrets directory at ${directoryPath}`); + if (process.platform === "win32") return; + + const mode = stats.mode & 0o777; + if ((mode & 0o077) !== 0) { + chmodSync(directoryPath, 0o700); + stats = lstatSync(directoryPath); + if (!stats.isDirectory()) { + throw new Error(`Decision signing secrets directory at ${directoryPath} must be a directory`); + } + assertOwnedByCurrentUser(stats, `Decision signing secrets directory at ${directoryPath}`); + if ((stats.mode & 0o077) !== 0) { + throw new Error(`Decision signing secrets directory at ${directoryPath} must have permissions 0700`); + } + } +} + +function readGeneratedSecret(keyPath: string): string { + enforceKeyFilePermissions(keyPath); + const existing = readFileSync(keyPath, "utf8").trim(); + if (existing.length < MIN_SECRET_LENGTH) { + throw new Error( + `Invalid decision signing key at ${keyPath} (must be at least ${MIN_SECRET_LENGTH} characters); remove the file to regenerate it or set PAPERCLIP_DECISION_SIGNING_SECRET`, + ); + } + return existing; +} + +function isAlreadyExists(error: unknown) { + return (error as NodeJS.ErrnoException).code === "EEXIST"; +} + +function isNotFound(error: unknown) { + return (error as NodeJS.ErrnoException).code === "ENOENT"; +} + +function loadOrCreateGeneratedSecret(): string { + const keyPath = resolveGeneratedSecretFilePath(); + const secretsDirectoryPath = path.dirname(keyPath); + try { + enforceSecretsDirectoryPermissions(secretsDirectoryPath); + return readGeneratedSecret(keyPath); + } catch (error) { + if (!isNotFound(error)) throw error; + } + + mkdirSync(secretsDirectoryPath, { recursive: true, mode: 0o700 }); + enforceSecretsDirectoryPermissions(secretsDirectoryPath); + const generated = randomBytes(32).toString("base64"); + const temporaryPath = `${keyPath}.${process.pid}.${randomBytes(8).toString("hex")}.tmp`; + + try { + writeFileSync(temporaryPath, generated, { encoding: "utf8", mode: 0o600, flag: "wx" }); + enforceKeyFilePermissions(temporaryPath); + + try { + // Publish only a complete key. A hard link is atomic and never replaces + // a key another server process created first. + linkSync(temporaryPath, keyPath); + enforceKeyFilePermissions(keyPath); + return generated; + } catch (error) { + if (!isAlreadyExists(error)) throw error; + return readGeneratedSecret(keyPath); + } + } finally { + try { + unlinkSync(temporaryPath); + } catch (error) { + if (!isNotFound(error)) throw error; + } + } +} + +export function resolveDecisionSigningSecret(): string { + const fromEnv = process.env.PAPERCLIP_DECISION_SIGNING_SECRET?.trim(); + if (fromEnv) { + if (fromEnv.length < MIN_SECRET_LENGTH) { + throw new Error( + `PAPERCLIP_DECISION_SIGNING_SECRET must be at least ${MIN_SECRET_LENGTH} characters when set (unset it to use an auto-generated key)`, + ); + } + return fromEnv; + } + return loadOrCreateGeneratedSecret(); +} + +/** + * Startup guard: resolves the signing secret once so an invalid explicit + * PAPERCLIP_DECISION_SIGNING_SECRET fails fast and the generated key file is + * materialized before the first decision write. A missing env var is not an + * error — the secret is auto-generated and persisted per instance. + */ +export function ensureDecisionSigningSecret() { + resolveDecisionSigningSecret(); } function canonical(value: unknown): string { @@ -18,7 +153,7 @@ function canonical(value: unknown): string { } export function signDecisionSpec(value: unknown) { - return `${VERSION}.${createHmac("sha256", validateDecisionSigningSecret()).update(`${VERSION}:${canonical(value)}`).digest("hex")}`; + return `${VERSION}.${createHmac("sha256", resolveDecisionSigningSecret()).update(`${VERSION}:${canonical(value)}`).digest("hex")}`; } export function verifyDecisionSpec(value: unknown, signature: string) {