diff --git a/cli/src/__tests__/secrets.test.ts b/cli/src/__tests__/secrets.test.ts index 8ae2a77ff5c..cef60632867 100644 --- a/cli/src/__tests__/secrets.test.ts +++ b/cli/src/__tests__/secrets.test.ts @@ -1,3 +1,7 @@ +import { randomBytes } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { Command } from "commander"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Agent, CompanySecret } from "@paperclipai/shared"; @@ -262,6 +266,47 @@ describe("secrets CLI helpers", () => { expect(result.message).toContain("prod-us-1"); expect(result.message).toContain("AWS_PROFILE/shared config"); }); + + describe("local encrypted key file permissions", () => { + let tempDir: string; + let keyFilePath: string; + + beforeEach(() => { + delete process.env.PAPERCLIP_SECRETS_MASTER_KEY; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-secrets-check-")); + keyFilePath = path.join(tempDir, "master.key"); + fs.writeFileSync(keyFilePath, randomBytes(32).toString("base64"), "utf8"); + process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE = keyFilePath; + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it.skipIf(process.platform === "win32")( + "warns when the key file is readable by group or others", + () => { + fs.chmodSync(keyFilePath, 0o644); + + const result = secretsCheck(configWithSecretsProvider("local_encrypted")); + + expect(result.status).toBe("warn"); + expect(result.message).toContain("key file permissions are 644"); + }, + ); + + it("does not warn about POSIX permission bits on Windows", () => { + fs.chmodSync(keyFilePath, 0o666); + vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + const result = secretsCheck(configWithSecretsProvider("local_encrypted")); + + expect(result.status).toBe("pass"); + expect(result.message).not.toContain("chmod 600"); + expect(result.repairHint).toBeUndefined(); + }); + }); }); describe("secrets API parity commands", () => { diff --git a/cli/src/checks/secrets-check.ts b/cli/src/checks/secrets-check.ts index 73f9c0402c1..6cd64f1d0dc 100644 --- a/cli/src/checks/secrets-check.ts +++ b/cli/src/checks/secrets-check.ts @@ -141,9 +141,11 @@ export function secretsCheck(config: PaperclipConfig, configPath?: string): Chec }; } + // Windows has no POSIX permission bits: stat() reports 0o666 for any writable + // file and chmod cannot restrict it, so this check would always warn there. const keyMode = fs.statSync(keyFilePath).mode & 0o777; const permissionWarning = - (keyMode & 0o077) !== 0 + process.platform !== "win32" && (keyMode & 0o077) !== 0 ? `; key file permissions are ${keyMode.toString(8)} (run chmod 600 ${keyFilePath})` : ""; diff --git a/server/src/__tests__/secret-provider-registry.test.ts b/server/src/__tests__/secret-provider-registry.test.ts index 326fd40682a..8059fe486fc 100644 --- a/server/src/__tests__/secret-provider-registry.test.ts +++ b/server/src/__tests__/secret-provider-registry.test.ts @@ -2,7 +2,7 @@ import { randomBytes } from "node:crypto"; import { chmodSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { checkSecretProviders, listSecretProviders } from "../secrets/provider-registry.js"; describe("secret provider registry", () => { @@ -11,6 +11,7 @@ describe("secret provider registry", () => { const tmpDirs: string[] = []; afterEach(() => { + vi.restoreAllMocks(); if (previousKeyFile === undefined) { delete process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE; } else { @@ -47,7 +48,7 @@ describe("secret provider registry", () => { ); }); - it("warns when the local encrypted key file is readable by group or others", async () => { + it.skipIf(process.platform === "win32")("warns when the local encrypted key file is readable by group or others", async () => { const dir = path.join(os.tmpdir(), `paperclip-secret-provider-${randomBytes(6).toString("hex")}`); tmpDirs.push(dir); mkdirSync(dir, { recursive: true }); @@ -67,4 +68,22 @@ describe("secret provider registry", () => { expect(local?.warnings?.join("\n")).toContain("chmod 600"); expect(local?.backupGuidance?.join("\n")).toContain("database"); }); + + it("does not warn about POSIX permission bits on Windows", async () => { + const dir = path.join(os.tmpdir(), `paperclip-secret-provider-${randomBytes(6).toString("hex")}`); + tmpDirs.push(dir); + mkdirSync(dir, { recursive: true }); + const keyFile = path.join(dir, "master.key"); + writeFileSync(keyFile, randomBytes(32).toString("base64"), "utf8"); + chmodSync(keyFile, 0o666); + process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE = keyFile; + delete process.env.PAPERCLIP_SECRETS_MASTER_KEY; + vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + + const checks = await checkSecretProviders(); + const local = checks.find((check) => check.provider === "local_encrypted"); + + expect(local).toMatchObject({ status: "ok", details: { keyFilePath: keyFile } }); + expect(local?.warnings ?? []).toEqual([]); + }); }); diff --git a/server/src/secrets/local-encrypted-provider.ts b/server/src/secrets/local-encrypted-provider.ts index bc4820a3d38..ce7f4572194 100644 --- a/server/src/secrets/local-encrypted-provider.ts +++ b/server/src/secrets/local-encrypted-provider.ts @@ -199,8 +199,10 @@ async function inspectLocalEncryptedHealth(): Promise }; } + // Windows has no POSIX permission bits: stat() reports 0o666 for any writable + // file and chmod cannot restrict it, so this check would always warn there. const warnings = - mode !== null && (mode & 0o077) !== 0 + process.platform !== "win32" && mode !== null && (mode & 0o077) !== 0 ? [`Secrets key file permissions are ${mode.toString(8)}; run chmod 600 ${keyPath}`] : []; return {