diff --git a/packages/shared/src/validators/secret.test.ts b/packages/shared/src/validators/secret.test.ts new file mode 100644 index 00000000000..689bf9b06b1 --- /dev/null +++ b/packages/shared/src/validators/secret.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; +import { createSecretSchema } from "./secret.js"; + +describe("secret validators", () => { + it("rejects externalRef on managed secrets", () => { + expect(() => + createSecretSchema.parse({ + name: "OpenAI API Key", + managedMode: "paperclip_managed", + value: "secret-value", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/other", + }), + ).toThrow(/Managed secrets cannot set externalRef/); + }); + + it("allows externalRef on external reference secrets", () => { + const parsed = createSecretSchema.parse({ + name: "Shared Secret", + managedMode: "external_reference", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/other", + }); + + expect(parsed.externalRef).toContain(":secret:shared/other"); + }); +}); diff --git a/packages/shared/src/validators/secret.ts b/packages/shared/src/validators/secret.ts index 74ec04fe500..12d38e52fde 100644 --- a/packages/shared/src/validators/secret.ts +++ b/packages/shared/src/validators/secret.ts @@ -47,6 +47,13 @@ export const createSecretSchema = z.object({ } return; } + if (value.externalRef?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["externalRef"], + message: "Managed secrets cannot set externalRef", + }); + } if (!value.value?.trim()) { ctx.addIssue({ code: z.ZodIssueCode.custom, diff --git a/server/src/__tests__/aws-secrets-manager-provider.test.ts b/server/src/__tests__/aws-secrets-manager-provider.test.ts index 0baa0a4fd30..23354776ec7 100644 --- a/server/src/__tests__/aws-secrets-manager-provider.test.ts +++ b/server/src/__tests__/aws-secrets-manager-provider.test.ts @@ -140,7 +140,7 @@ describe("awsSecretsManagerProvider", () => { expect(prepared.providerVersionRef).toBe("aws-version-2"); }); - it("ignores out-of-namespace refs for managed AWS secret version writes", async () => { + it("rejects out-of-namespace refs for managed AWS secret version writes", async () => { const calls: Array<{ op: string; input: Record }> = []; const provider = createAwsSecretsManagerProvider({ config: { @@ -170,26 +170,20 @@ describe("awsSecretsManagerProvider", () => { }, }); - await provider.createVersion({ - value: "rotated-secret-value", - externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", - context: { - companyId: "company-1", - secretKey: "openai-api-key", - secretName: "OpenAI API Key", - version: 2, - }, - }); - - expect(calls).toEqual([ - { - op: "putSecretValue", - input: { - SecretId: "paperclip/prod-use1/company-1/openai-api-key", - SecretString: "rotated-secret-value", + await expect( + provider.createVersion({ + value: "rotated-secret-value", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", + context: { + companyId: "company-1", + secretKey: "openai-api-key", + secretName: "OpenAI API Key", + version: 2, }, - }, - ]); + }), + ).rejects.toThrow(/drifted outside the derived deployment\/company scope/i); + + expect(calls).toEqual([]); }); it("stores linked external references as metadata-only provider material", async () => { @@ -269,6 +263,56 @@ describe("awsSecretsManagerProvider", () => { ]); }); + it("rejects managed resolve attempts when stored refs drift outside the derived scope", async () => { + const provider = createAwsSecretsManagerProvider({ + config: { + region: "us-east-1", + endpoint: "https://secretsmanager.us-east-1.amazonaws.com", + deploymentId: "prod-use1", + prefix: "paperclip", + kmsKeyId: "arn:aws:kms:us-east-1:123456789012:key/test", + environmentTag: "production", + providerOwnerTag: "paperclip", + deleteRecoveryWindowDays: 30, + }, + gateway: { + async createSecret() { + throw new Error("not used"); + }, + async putSecretValue() { + throw new Error("not used"); + }, + async getSecretValue() { + throw new Error("should not be called"); + }, + async deleteSecret() { + throw new Error("not used"); + }, + }, + }); + + await expect( + provider.resolveVersion({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-2/openai-api-key", + versionId: "aws-version-2", + source: "managed", + }, + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-2/openai-api-key", + providerVersionRef: "aws-version-2", + context: { + companyId: "company-1", + secretId: "secret-1", + secretKey: "openai-api-key", + version: 2, + }, + }), + ).rejects.toThrow(/drifted outside the derived deployment\/company scope/i); + }); + it("warns when AWS provider configuration is incomplete and blocks managed writes", async () => { delete process.env.PAPERCLIP_SECRETS_AWS_REGION; delete process.env.AWS_REGION; @@ -325,10 +369,12 @@ describe("awsSecretsManagerProvider", () => { await provider.deleteOrArchive({ mode: "delete", - externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key", material: { scheme: "aws_secrets_manager_v1", - secretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", + secretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key", versionId: null, source: "managed", }, @@ -339,6 +385,24 @@ describe("awsSecretsManagerProvider", () => { version: 2, }, }); + await expect( + provider.deleteOrArchive({ + mode: "delete", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", + material: { + scheme: "aws_secrets_manager_v1", + secretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/attacker", + versionId: null, + source: "managed", + }, + context: { + companyId: "company-1", + secretKey: "openai-api-key", + secretName: "OpenAI API Key", + version: 2, + }, + }), + ).rejects.toThrow(/drifted outside the derived deployment\/company scope/i); await provider.deleteOrArchive({ mode: "delete", externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/external", @@ -360,7 +424,8 @@ describe("awsSecretsManagerProvider", () => { { op: "deleteSecret", input: { - SecretId: "paperclip/prod-use1/company-1/openai-api-key", + SecretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key", RecoveryWindowInDays: 30, }, }, diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index 140e77c5761..228bcf7e788 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -7,6 +7,7 @@ import { errorHandler } from "../middleware/error-handler.js"; const mockSecretService = vi.hoisted(() => ({ listProviders: vi.fn(), checkProviders: vi.fn(), + create: vi.fn(), })); const mockLogActivity = vi.hoisted(() => vi.fn()); @@ -37,6 +38,7 @@ describe("secret routes", () => { beforeEach(() => { mockSecretService.listProviders.mockReset(); mockSecretService.checkProviders.mockReset(); + mockSecretService.create.mockReset(); mockLogActivity.mockReset(); }); @@ -64,4 +66,17 @@ describe("secret routes", () => { ], }); }); + + it("rejects managed secret creation when externalRef is supplied", async () => { + const res = await request(createApp()).post("/api/companies/company-1/secrets").send({ + name: "OpenAI API Key", + managedMode: "paperclip_managed", + value: "secret-value", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:shared/other", + }); + + expect(res.status).toBe(400); + expect(JSON.stringify(res.body)).toMatch(/Managed secrets cannot set externalRef/); + expect(mockSecretService.create).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index 2d3571dd247..1241dde790c 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { mkdirSync, rmSync } from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { eq } from "drizzle-orm"; import { companies, @@ -13,6 +13,7 @@ import { secretAccessEvents, } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { awsSecretsManagerProvider } from "../secrets/aws-secrets-manager-provider.js"; import { secretService } from "../services/secrets.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); @@ -39,6 +40,7 @@ describeEmbeddedPostgres("secretService", () => { }); afterEach(async () => { + vi.restoreAllMocks(); await db.delete(secretAccessEvents); await db.delete(companySecretBindings); await db.delete(companySecretVersions); @@ -188,4 +190,113 @@ describeEmbeddedPostgres("secretService", () => { }), ).rejects.toThrow(/not bound/i); }); + + it("rejects externalRef overrides on managed secrets", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const secret = await svc.create(companyId, { + name: `managed-${randomUUID()}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + + await expect( + svc.update(secret.id, { + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod/company-b/openai-api-key", + }), + ).rejects.toThrow(/Managed secrets cannot override externalRef/i); + + await expect( + svc.rotate(secret.id, { + value: "rotated-runtime-secret", + externalRef: "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod/company-b/openai-api-key", + }), + ).rejects.toThrow(/Managed secrets cannot override externalRef/i); + }); + + it("passes managed AWS secret context into provider delete during removal", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const externalRef = + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key"; + + const secret = await db + .insert(companySecrets) + .values({ + companyId, + key: "openai-api-key", + name: "OpenAI API Key", + provider: "aws_secrets_manager", + managedMode: "paperclip_managed", + externalRef, + latestVersion: 1, + status: "active", + }) + .returning() + .then((rows) => rows[0]); + + await db.insert(companySecretVersions).values({ + secretId: secret.id, + version: 1, + material: { + scheme: "aws_secrets_manager_v1", + secretId: externalRef, + versionId: "aws-version-1", + source: "managed", + }, + valueSha256: "value-sha-1", + fingerprintSha256: "fingerprint-sha-1", + providerVersionRef: "aws-version-1", + status: "current", + }); + + const deleteSpy = vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + + const removed = await svc.remove(secret.id); + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.id, secret.id)) + .then((rows) => rows[0] ?? null); + + expect(removed?.id).toBe(secret.id); + expect(deleteSpy).toHaveBeenCalledTimes(1); + expect(deleteSpy).toHaveBeenCalledWith({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: externalRef, + versionId: "aws-version-1", + source: "managed", + }, + externalRef, + context: { + companyId, + secretKey: "openai-api-key", + secretName: "OpenAI API Key", + version: 1, + }, + mode: "delete", + }); + expect(persisted).toBeNull(); + }); + + it("refuses to resolve secrets once they are disabled or archived", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const secret = await svc.create(companyId, { + name: `managed-${randomUUID()}`, + provider: "local_encrypted", + value: "runtime-secret", + }); + + await svc.update(secret.id, { status: "disabled" }); + await expect(svc.resolveSecretValue(companyId, secret.id, "latest")).rejects.toThrow( + /not active/i, + ); + + await svc.update(secret.id, { status: "archived" }); + await expect(svc.resolveSecretValue(companyId, secret.id, "latest")).rejects.toThrow( + /not active/i, + ); + }); }); diff --git a/server/src/secrets/aws-secrets-manager-provider.ts b/server/src/secrets/aws-secrets-manager-provider.ts index 48241888267..257250611fb 100644 --- a/server/src/secrets/aws-secrets-manager-provider.ts +++ b/server/src/secrets/aws-secrets-manager-provider.ts @@ -216,11 +216,20 @@ function resolveManagedSecretRef(input: { context: ManagedSecretNamespaceContext | undefined; externalRefs: Array; }) { + let sawNonEmptyExternalRef = false; for (const externalRef of input.externalRefs) { + if (externalRef?.trim()) { + sawNonEmptyExternalRef = true; + } if (externalRef?.trim() && isManagedSecretRefForContext(input.config, input.context, externalRef)) { return externalRef.trim(); } } + if (sawNonEmptyExternalRef) { + throw unprocessable( + "AWS Secrets Manager managed secret ref drifted outside the derived deployment/company scope", + ); + } return buildManagedSecretId(input.config, input.context); } diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 24a2f01c5ff..7af1cf8f36f 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -369,6 +369,9 @@ export function secretService(db: Db) { if (managedMode === "external_reference" && !input.externalRef?.trim()) { throw unprocessable("External reference secrets require externalRef"); } + if (managedMode === "paperclip_managed" && input.externalRef?.trim()) { + throw unprocessable("Managed secrets cannot override externalRef"); + } if (managedMode === "paperclip_managed" && !input.value?.trim()) { throw unprocessable("Managed secrets require value"); } @@ -444,6 +447,9 @@ export function secretService(db: Db) { if (secret.managedMode === "external_reference" && !(input.externalRef ?? secret.externalRef)?.trim()) { throw unprocessable("External reference secrets require externalRef"); } + if (secret.managedMode !== "external_reference" && input.externalRef?.trim()) { + throw unprocessable("Managed secrets cannot override externalRef"); + } if (secret.managedMode !== "external_reference" && !input.value?.trim()) { throw unprocessable("Managed secrets require value"); } @@ -537,6 +543,9 @@ export function secretService(db: Db) { } } const deleting = patch.status === "deleted"; + if (secret.managedMode !== "external_reference" && patch.externalRef !== undefined) { + throw unprocessable("Managed secrets cannot override externalRef"); + } return db .update(companySecrets)