Reject managed secret externalRef overrides

Co-Authored-By: Paperclip <noreply@paperclip.ing>

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");
+  });
+});

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,

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<string, unknown> }> = [];
     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,
         },
       },

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();
+  });
 });

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,
+    );
+  });
 });

server/src/secrets/aws-secrets-manager-provider.ts

@@ -216,11 +216,20 @@ function resolveManagedSecretRef(input: {
   context: ManagedSecretNamespaceContext | undefined;
   externalRefs: Array<string | null | undefined>;
 }) {
+  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);
 }
 

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)