diff --git a/server/src/__tests__/aws-secrets-manager-provider.test.ts b/server/src/__tests__/aws-secrets-manager-provider.test.ts index aa660961c48..488f3415998 100644 --- a/server/src/__tests__/aws-secrets-manager-provider.test.ts +++ b/server/src/__tests__/aws-secrets-manager-provider.test.ts @@ -270,6 +270,7 @@ describe("awsSecretsManagerProvider", () => { SecretId: "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key", SecretString: "rotated-secret-value", + VersionStages: ["PAPERCLIP_PENDING"], }, }, ]); @@ -750,4 +751,70 @@ describe("awsSecretsManagerProvider", () => { }, ]); }); + + it("archives pending Paperclip-managed AWS versions without deleting the secret", async () => { + const calls: Array<{ op: string; input: Record }> = []; + 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("not used"); + }, + async deleteSecret(input) { + calls.push({ op: "deleteSecret", input }); + return {}; + }, + async updateSecretVersionStage(input) { + calls.push({ op: "updateSecretVersionStage", input }); + return {}; + }, + }, + }); + + await provider.deleteOrArchive({ + mode: "archive", + 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:paperclip/prod-use1/company-1/openai-api-key", + versionId: "aws-version-2", + source: "managed", + }, + context: { + companyId: "company-1", + secretKey: "openai-api-key", + secretName: "OpenAI API Key", + version: 2, + }, + }); + + expect(calls).toEqual([ + { + op: "updateSecretVersionStage", + input: { + SecretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company-1/openai-api-key", + VersionStage: "PAPERCLIP_PENDING", + RemoveFromVersionId: "aws-version-2", + }, + }, + ]); + }); }); diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index 51238f2e646..6cbcf9cacdc 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -545,6 +545,121 @@ describeEmbeddedPostgres("secretService", () => { expect(JSON.stringify(resolveSpy.mock.calls[0]?.[0])).not.toContain("resolved-secret"); }); + it("cleans up managed provider secrets when create persistence fails", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const awsVault = await svc.createProviderConfig(companyId, { + provider: "aws_secrets_manager", + displayName: "AWS production", + config: { region: "us-east-1", namespace: "prod-use1" }, + }); + const prepared = { + material: { + scheme: "aws_secrets_manager_v1", + secretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/create-rollback", + versionId: "aws-version-1", + source: "managed", + }, + valueSha256: "value-sha-1", + fingerprintSha256: "fingerprint-sha-1", + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/create-rollback", + providerVersionRef: "aws-version-1", + }; + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockResolvedValue(prepared); + const deleteSpy = vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + vi.spyOn(db, "transaction").mockRejectedValueOnce(new Error("db insert failed")); + + await expect( + svc.create(companyId, { + name: "Create Rollback", + key: "create-rollback", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + value: "runtime-secret", + }), + ).rejects.toThrow("db insert failed"); + + expect(deleteSpy).toHaveBeenCalledWith(expect.objectContaining({ + material: prepared.material, + externalRef: prepared.externalRef, + mode: "delete", + providerConfig: expect.objectContaining({ id: awsVault.id }), + context: { + companyId, + secretKey: "create-rollback", + secretName: "Create Rollback", + version: 1, + }, + })); + }); + + it("archives managed provider versions when rotate persistence fails", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const awsVault = await svc.createProviderConfig(companyId, { + provider: "aws_secrets_manager", + displayName: "AWS production", + config: { region: "us-east-1", namespace: "prod-use1" }, + }); + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockResolvedValue({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/rotate-rollback", + versionId: "aws-version-1", + source: "managed", + }, + valueSha256: "value-sha-1", + fingerprintSha256: "fingerprint-sha-1", + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/rotate-rollback", + providerVersionRef: "aws-version-1", + }); + const secret = await svc.create(companyId, { + name: "Rotate Rollback", + key: "rotate-rollback", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + value: "runtime-secret", + }); + const prepared = { + material: { + scheme: "aws_secrets_manager_v1", + secretId: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/rotate-rollback", + versionId: "aws-version-2", + source: "managed", + }, + valueSha256: "value-sha-2", + fingerprintSha256: "fingerprint-sha-2", + externalRef: + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/company/rotate-rollback", + providerVersionRef: "aws-version-2", + }; + vi.spyOn(awsSecretsManagerProvider, "createVersion").mockResolvedValue(prepared); + const deleteSpy = vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + vi.spyOn(db, "transaction").mockRejectedValueOnce(new Error("db rotate failed")); + + await expect(svc.rotate(secret.id, { value: "rotated-runtime-secret" })).rejects.toThrow( + "db rotate failed", + ); + + expect(deleteSpy).toHaveBeenCalledWith(expect.objectContaining({ + material: prepared.material, + externalRef: prepared.externalRef, + mode: "archive", + providerConfig: expect.objectContaining({ id: awsVault.id }), + context: { + companyId, + secretKey: "rotate-rollback", + secretName: "Rotate Rollback", + version: 2, + }, + })); + }); + it("rejects rotation for non-active secrets", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/secrets/aws-secrets-manager-provider.ts b/server/src/secrets/aws-secrets-manager-provider.ts index b36439bf762..2a26b891bcc 100644 --- a/server/src/secrets/aws-secrets-manager-provider.ts +++ b/server/src/secrets/aws-secrets-manager-provider.ts @@ -19,6 +19,7 @@ const AWS_SECRETS_MANAGER_SCHEME = "aws_secrets_manager_v1"; const DEFAULT_PREFIX = "paperclip"; const DEFAULT_OWNER_TAG = "paperclip"; const DEFAULT_VERSION_STAGE = "AWSCURRENT"; +const PAPERCLIP_PENDING_VERSION_STAGE = "PAPERCLIP_PENDING"; const DEFAULT_DELETE_RECOVERY_WINDOW_DAYS = 30; const AWS_SECRETS_MANAGER_REQUEST_TIMEOUT_MS = 30_000; const AWS_CREDENTIAL_CACHE_TTL_MS = 5 * 60_000; @@ -95,6 +96,7 @@ interface AwsSecretsManagerGateway { putSecretValue(input: { SecretId: string; SecretString: string; + VersionStages?: string[]; }): Promise<{ ARN?: string; Name?: string; @@ -114,6 +116,12 @@ interface AwsSecretsManagerGateway { SecretId: string; RecoveryWindowInDays: number; }): Promise; + updateSecretVersionStage?(input: { + SecretId: string; + VersionStage: string; + RemoveFromVersionId?: string; + MoveToVersionId?: string; + }): Promise; listSecrets?(input: { MaxResults?: number; NextToken?: string; @@ -665,6 +673,7 @@ class AwsSecretsManagerJsonGateway implements AwsSecretsManagerGateway { putSecretValue(input: { SecretId: string; SecretString: string; + VersionStages?: string[]; }) { return this.call<{ ARN?: string; @@ -693,6 +702,15 @@ class AwsSecretsManagerJsonGateway implements AwsSecretsManagerGateway { return this.call("DeleteSecret", input); } + updateSecretVersionStage(input: { + SecretId: string; + VersionStage: string; + RemoveFromVersionId?: string; + MoveToVersionId?: string; + }) { + return this.call("UpdateSecretVersionStage", input); + } + listSecrets(input: { MaxResults?: number; NextToken?: string; @@ -904,6 +922,7 @@ export function createAwsSecretsManagerProvider( const created = await gateway.putSecretValue({ SecretId: secretId, SecretString: input.value, + VersionStages: [PAPERCLIP_PENDING_VERSION_STAGE], }); const normalizedSecretId = created.ARN ?? created.Name ?? secretId; return { @@ -990,7 +1009,7 @@ export function createAwsSecretsManagerProvider( ? asAwsSecretsManagerMaterial(input.material) : null; - if (input.mode !== "delete" || material?.source !== "managed") return; + if (material?.source !== "managed") return; const config = resolveConfig(input.providerConfig); const gateway = resolveGateway(config); @@ -1001,12 +1020,22 @@ export function createAwsSecretsManagerProvider( }); try { + if (input.mode === "archive") { + if (material.versionId && gateway.updateSecretVersionStage) { + await gateway.updateSecretVersionStage({ + SecretId: secretId, + VersionStage: PAPERCLIP_PENDING_VERSION_STAGE, + RemoveFromVersionId: material.versionId, + }); + } + return; + } await gateway.deleteSecret({ SecretId: secretId, RecoveryWindowInDays: config.deleteRecoveryWindowDays, }); } catch (error) { - normalizeAwsError("deleteSecret", error); + normalizeAwsError(input.mode === "archive" ? "updateSecretVersionStage" : "deleteSecret", error); } }, healthCheck, diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index df62846ca85..2060774b8ec 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -47,7 +47,9 @@ import type { PreparedSecretVersion, RemoteSecretListResult, SecretProviderHealthCheck, + SecretProviderModule, SecretProviderVaultRuntimeConfig, + SecretProviderWriteContext, } from "../secrets/types.js"; import { isSecretProviderClientError } from "../secrets/types.js"; @@ -130,6 +132,36 @@ function remoteImportRowFailureReason(error: unknown, fallback: string, context: return fallback; } +async function cleanupPreparedProviderWrite(input: { + provider: SecretProviderModule; + prepared: PreparedSecretVersion; + providerConfig: SecretProviderVaultRuntimeConfig | null; + context: SecretProviderWriteContext; + mode: "archive" | "delete"; + operation: string; +}) { + try { + await input.provider.deleteOrArchive({ + material: input.prepared.material, + externalRef: input.prepared.externalRef, + providerConfig: input.providerConfig, + context: input.context, + mode: input.mode, + }); + } catch (cleanupError) { + logger.warn( + { + err: cleanupError, + companyId: input.context.companyId, + provider: input.provider.id, + providerConfigId: input.providerConfig?.id ?? null, + operation: input.operation, + }, + "remote secret provider cleanup failed after db write failure", + ); + } +} + type CanonicalEnvBinding = | { type: "plain"; value: string } | { type: "secret_ref"; secretId: string; version: number | "latest" }; @@ -1462,67 +1494,77 @@ export function secretService(db: Db) { if (managedMode === "paperclip_managed" && !input.value?.trim()) { throw unprocessable("Managed secrets require value"); } + const providerWriteContext = { + companyId, + secretKey: key, + secretName: input.name, + version: 1, + }; const prepared = managedMode === "external_reference" ? await provider.linkExternalSecret({ externalRef: input.externalRef ?? "", providerVersionRef: input.providerVersionRef ?? null, providerConfig, - context: { - companyId, - secretKey: key, - secretName: input.name, - version: 1, - }, + context: providerWriteContext, }) : await provider.createSecret({ value: input.value ?? "", externalRef: null, providerConfig, - context: { - companyId, - secretKey: key, - secretName: input.name, - version: 1, - }, + context: providerWriteContext, }); - return db.transaction(async (tx) => { - const secret = await tx - .insert(companySecrets) - .values({ - companyId, - key, - name: input.name, - provider: input.provider, - providerConfigId: input.providerConfigId ?? null, - status: "active", - managedMode, - externalRef: prepared.externalRef, - providerMetadata: input.providerMetadata ?? null, - latestVersion: 1, - description: input.description ?? null, - lastRotatedAt: new Date(), + try { + return await db.transaction(async (tx) => { + const secret = await tx + .insert(companySecrets) + .values({ + companyId, + key, + name: input.name, + provider: input.provider, + providerConfigId: input.providerConfigId ?? null, + status: "active", + managedMode, + externalRef: prepared.externalRef, + providerMetadata: input.providerMetadata ?? null, + latestVersion: 1, + description: input.description ?? null, + lastRotatedAt: new Date(), + createdByAgentId: actor?.agentId ?? null, + createdByUserId: actor?.userId ?? null, + }) + .returning() + .then((rows) => rows[0]); + + await tx.insert(companySecretVersions).values({ + secretId: secret.id, + version: 1, + material: prepared.material, + valueSha256: prepared.valueSha256, + fingerprintSha256: prepared.fingerprintSha256 ?? prepared.valueSha256, + providerVersionRef: prepared.providerVersionRef ?? null, + status: "current", createdByAgentId: actor?.agentId ?? null, createdByUserId: actor?.userId ?? null, - }) - .returning() - .then((rows) => rows[0]); + }); - await tx.insert(companySecretVersions).values({ - secretId: secret.id, - version: 1, - material: prepared.material, - valueSha256: prepared.valueSha256, - fingerprintSha256: prepared.fingerprintSha256 ?? prepared.valueSha256, - providerVersionRef: prepared.providerVersionRef ?? null, - status: "current", - createdByAgentId: actor?.agentId ?? null, - createdByUserId: actor?.userId ?? null, + return secret; }); - - return secret; - }); + } catch (error) { + if (managedMode === "paperclip_managed") { + await cleanupPreparedProviderWrite({ + provider, + prepared, + providerConfig, + context: providerWriteContext, + mode: "delete", + operation: "create.rollback", + }); + } + throw error; + } }, rotate: async ( @@ -1557,64 +1599,74 @@ export function secretService(db: Db) { if (secret.managedMode !== "external_reference" && !input.value?.trim()) { throw unprocessable("Managed secrets require value"); } + const providerWriteContext = { + companyId: secret.companyId, + secretKey: secret.key, + secretName: secret.name, + version: nextVersion, + }; const prepared = secret.managedMode === "external_reference" ? await provider.linkExternalSecret({ externalRef: input.externalRef ?? secret.externalRef ?? "", providerVersionRef: input.providerVersionRef ?? null, providerConfig, - context: { - companyId: secret.companyId, - secretKey: secret.key, - secretName: secret.name, - version: nextVersion, - }, + context: providerWriteContext, }) : await provider.createVersion({ value: input.value ?? "", externalRef: secret.externalRef ?? null, providerConfig, - context: { - companyId: secret.companyId, - secretKey: secret.key, - secretName: secret.name, - version: nextVersion, - }, + context: providerWriteContext, }); - return db.transaction(async (tx) => { - await tx - .update(companySecretVersions) - .set({ status: "previous" }) - .where(eq(companySecretVersions.secretId, secret.id)); - await tx.insert(companySecretVersions).values({ - secretId: secret.id, - version: nextVersion, - material: prepared.material, - valueSha256: prepared.valueSha256, - fingerprintSha256: prepared.fingerprintSha256 ?? prepared.valueSha256, - providerVersionRef: prepared.providerVersionRef ?? null, - status: "current", - createdByAgentId: actor?.agentId ?? null, - createdByUserId: actor?.userId ?? null, + try { + return await db.transaction(async (tx) => { + await tx + .update(companySecretVersions) + .set({ status: "previous" }) + .where(eq(companySecretVersions.secretId, secret.id)); + await tx.insert(companySecretVersions).values({ + secretId: secret.id, + version: nextVersion, + material: prepared.material, + valueSha256: prepared.valueSha256, + fingerprintSha256: prepared.fingerprintSha256 ?? prepared.valueSha256, + providerVersionRef: prepared.providerVersionRef ?? null, + status: "current", + createdByAgentId: actor?.agentId ?? null, + createdByUserId: actor?.userId ?? null, + }); + + const updated = await tx + .update(companySecrets) + .set({ + latestVersion: nextVersion, + externalRef: prepared.externalRef, + providerConfigId, + lastRotatedAt: new Date(), + updatedAt: new Date(), + }) + .where(eq(companySecrets.id, secret.id)) + .returning() + .then((rows) => rows[0] ?? null); + + if (!updated) throw notFound("Secret not found"); + return updated; }); - - const updated = await tx - .update(companySecrets) - .set({ - latestVersion: nextVersion, - externalRef: prepared.externalRef, - providerConfigId, - lastRotatedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(companySecrets.id, secret.id)) - .returning() - .then((rows) => rows[0] ?? null); - - if (!updated) throw notFound("Secret not found"); - return updated; - }); + } catch (error) { + if (secret.managedMode !== "external_reference") { + await cleanupPreparedProviderWrite({ + provider, + prepared, + providerConfig, + context: providerWriteContext, + mode: "archive", + operation: "rotate.rollback", + }); + } + throw error; + } }, update: async (