diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index fc878c0280..8c984d48b6 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -106,6 +106,54 @@ describe("secret routes", () => { expect(mockSecretService.create).not.toHaveBeenCalled(); }); + it("returns sanitized AWS provider errors when managed secret creation fails", async () => { + mockSecretService.create.mockRejectedValue( + new HttpError( + 403, + "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret.create", + providerConfigId: "11111111-1111-4111-8111-111111111111", + region: "us-east-1", + credentialPath: "Paperclip server runtime/provider credential path", + requiredCapability: "secretsmanager:CreateSecret", + actionableMessage: + "AWS managed secret creation needs secretsmanager:CreateSecret in the selected region for this provider vault.", + safeAlternative: + "If the secret already exists in AWS, link it as an external reference instead of creating a Paperclip-managed value.", + }, + ), + ); + + const res = await request(createApp()).post("/api/companies/company-1/secrets").send({ + name: "Vercel token", + key: "vercel_token", + provider: "aws_secrets_manager", + providerConfigId: "11111111-1111-4111-8111-111111111111", + managedMode: "paperclip_managed", + value: "vcp_test", + }); + + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ + code: "access_denied", + error: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + details: { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret.create", + providerConfigId: "11111111-1111-4111-8111-111111111111", + region: "us-east-1", + requiredCapability: "secretsmanager:CreateSecret", + }, + }); + expect(JSON.stringify(res.body)).not.toContain("arn:aws"); + expect(JSON.stringify(res.body)).not.toContain("123456789012"); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("restricts user secret definition management to company admins", async () => { const res = await request(createApp({ type: "board", diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index bd85967228..7dee6201f9 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -654,6 +654,127 @@ describeEmbeddedPostgres("secretService", () => { expect(rows.filter((row) => row.ownerUserId === "user-1" && row.status === "active")).toHaveLength(1); }); + it("reports current-user secret rollback failures when AWS create cleanup cannot remove the reserved row", async () => { + const companyId = await seedCompany(); + await seedCompanyMember(companyId, "user-1", "owner"); + 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 definition = await svc.createUserSecretDefinition(companyId, { + key: "github_token", + name: "GitHub token", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + }); + + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockRejectedValueOnce( + new SecretProviderClientError({ + code: "access_denied", + provider: "aws_secrets_manager", + operation: "createSecret", + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + rawMessage: + "AccessDeniedException: arn:aws:sts::123456789012:assumed-role/prod/Paperclip cannot create secret", + }), + ); + vi.spyOn(db, "delete").mockImplementationOnce(() => { + throw new Error("reserved row delete failed"); + }); + + await expect( + svc.createCurrentUserSecretValue(companyId, "user-1", { + definitionId: definition.id, + value: "runtime-secret", + }), + ).rejects.toMatchObject({ + status: 500, + message: "Secret create failed and Paperclip could not roll back the local secret reservation.", + details: { + code: "secret_create_rollback_failed", + provider: "aws_secrets_manager", + operation: "secret.create", + providerConfigId: awsVault.id, + providerError: { + status: 403, + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + details: { + code: "access_denied", + requiredCapability: "secretsmanager:CreateSecret", + }, + }, + }, + }); + + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.companyId, companyId)); + expect(persisted).toHaveLength(1); + expect(JSON.stringify(persisted)).not.toContain("runtime-secret"); + }); + + it("reports current-user secret persistence rollback failures when local cleanup cannot remove the reserved row", async () => { + const companyId = await seedCompany(); + await seedCompanyMember(companyId, "user-1", "owner"); + 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 definition = await svc.createUserSecretDefinition(companyId, { + key: "github_token", + name: "GitHub token", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + }); + const externalRef = + "arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1/user/github-token"; + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockResolvedValue({ + material: { + scheme: "aws_secrets_manager_v1", + secretId: externalRef, + versionId: "aws-version-1", + source: "managed", + }, + valueSha256: "value-sha-1", + fingerprintSha256: "fingerprint-sha-1", + externalRef, + providerVersionRef: "aws-version-1", + }); + vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + vi.spyOn(db, "transaction").mockRejectedValueOnce(new Error("db activate failed")); + vi.spyOn(db, "delete").mockImplementationOnce(() => { + throw new Error("reserved row delete failed"); + }); + + await expect( + svc.createCurrentUserSecretValue(companyId, "user-1", { + definitionId: definition.id, + value: "runtime-secret", + }), + ).rejects.toMatchObject({ + status: 500, + message: "Secret create failed and Paperclip could not roll back the local secret reservation.", + details: { + code: "secret_create_rollback_failed", + provider: "aws_secrets_manager", + operation: "user_secret_value.create_rollback", + providerConfigId: awsVault.id, + }, + }); + + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.companyId, companyId)); + expect(persisted).toHaveLength(1); + expect(JSON.stringify(persisted)).not.toContain("runtime-secret"); + }); + it("returns conflict when concurrent user secret definition creation races the unique index", async () => { const companyId = await seedCompany(); const svc = secretService(db); @@ -1665,7 +1786,17 @@ describeEmbeddedPostgres("secretService", () => { providerConfigId: awsVault.id, value: "runtime-secret", }), - ).rejects.toThrow("db activate failed"); + ).rejects.toMatchObject({ + status: 500, + message: "Secret create failed and Paperclip could not clean up the remote provider secret.", + details: { + code: "secret_create_provider_cleanup_failed", + provider: "aws_secrets_manager", + operation: "create.rollback", + providerConfigId: awsVault.id, + localCleanupHandle: true, + }, + }); const persisted = await svc.getByName(companyId, "Create Cleanup Handle"); expect(persisted).toMatchObject({ @@ -1687,6 +1818,62 @@ describeEmbeddedPostgres("secretService", () => { }); }); + it("reports managed secret persistence rollback failures when local cleanup cannot remove the reserved row", 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-local-cleanup", + 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-local-cleanup", + providerVersionRef: "aws-version-1", + }; + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockResolvedValue(prepared); + vi.spyOn(awsSecretsManagerProvider, "deleteOrArchive").mockResolvedValue(); + vi.spyOn(db, "transaction").mockRejectedValueOnce(new Error("db activate failed")); + vi.spyOn(db, "delete").mockImplementationOnce(() => { + throw new Error("reserved row delete failed"); + }); + + await expect( + svc.create(companyId, { + name: "Create Local Cleanup", + key: "create-local-cleanup", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + value: "runtime-secret", + }), + ).rejects.toMatchObject({ + status: 500, + message: "Secret create failed and Paperclip could not roll back the local secret reservation.", + details: { + code: "secret_create_rollback_failed", + provider: "aws_secrets_manager", + operation: "create.rollback", + providerConfigId: awsVault.id, + }, + }); + + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.companyId, companyId)); + expect(persisted).toHaveLength(1); + expect(JSON.stringify(persisted)).not.toContain("runtime-secret"); + }); + it("archives managed provider versions when rotate persistence fails", async () => { const companyId = await seedCompany(); const svc = secretService(db); @@ -2002,6 +2189,123 @@ describeEmbeddedPostgres("secretService", () => { expect(thrown instanceof Error ? thrown.message : String(thrown)).not.toContain("arn:aws"); }); + it("sanitizes AWS managed secret create failures and removes the reserved row", 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 rawProviderMessage = + "AccessDeniedException: User: arn:aws:sts::123456789012:assumed-role/prod/Paperclip is not authorized to perform secretsmanager:CreateSecret on arn:aws:secretsmanager:us-east-1:123456789012:secret:paperclip/prod-use1"; + + vi.spyOn(awsSecretsManagerProvider, "createSecret").mockRejectedValueOnce( + new SecretProviderClientError({ + code: "access_denied", + provider: "aws_secrets_manager", + operation: "createSecret", + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + rawMessage: rawProviderMessage, + }), + ); + + let thrown: unknown; + try { + await svc.create(companyId, { + name: "Vercel token", + key: "vercel_token", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + managedMode: "paperclip_managed", + value: "vcp_test", + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toMatchObject({ + status: 403, + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + details: { + code: "access_denied", + provider: "aws_secrets_manager", + operation: "secret.create", + providerConfigId: awsVault.id, + region: "us-east-1", + requiredCapability: "secretsmanager:CreateSecret", + }, + }); + expect(JSON.stringify(thrown)).not.toContain("arn:aws"); + expect(JSON.stringify(thrown)).not.toContain("123456789012"); + expect(thrown instanceof Error ? thrown.message : String(thrown)).not.toContain("arn:aws"); + + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.companyId, companyId)); + expect(persisted).toHaveLength(0); + }); + + it("reports rollback failures when AWS managed secret create cleanup cannot remove the reserved row", 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").mockRejectedValueOnce( + new SecretProviderClientError({ + code: "access_denied", + provider: "aws_secrets_manager", + operation: "createSecret", + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + rawMessage: + "AccessDeniedException: arn:aws:sts::123456789012:assumed-role/prod/Paperclip cannot create secret", + }), + ); + vi.spyOn(db, "delete").mockImplementationOnce(() => { + throw new Error("reserved row delete failed"); + }); + + await expect( + svc.create(companyId, { + name: "Vercel token", + key: "vercel_token", + provider: "aws_secrets_manager", + providerConfigId: awsVault.id, + managedMode: "paperclip_managed", + value: "vcp_test", + }), + ).rejects.toMatchObject({ + status: 500, + message: "Secret create failed and Paperclip could not roll back the local secret reservation.", + details: { + code: "secret_create_rollback_failed", + provider: "aws_secrets_manager", + operation: "secret.create", + providerConfigId: awsVault.id, + providerError: { + status: 403, + message: "AWS Secrets Manager denied the request. Check IAM permissions for this provider vault.", + details: { + code: "access_denied", + requiredCapability: "secretsmanager:CreateSecret", + }, + }, + }, + }); + + const persisted = await db + .select() + .from(companySecrets) + .where(eq(companySecrets.companyId, companyId)); + expect(persisted).toHaveLength(1); + expect(JSON.stringify(persisted)).not.toContain("vcp_test"); + }); + it("previews AWS provider vault discovery from draft config without persisting a provider vault", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 65cdbed8db..1a2454c2d5 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -131,6 +131,119 @@ function remoteProviderHttpError(error: unknown, context: { return new HttpError(502, "Remote secret provider request failed.", safeRemoteProviderErrorDetails(null, context)); } +function remoteProviderWriteHttpError(error: unknown, context: { + companyId: string; + provider: SecretProvider; + providerConfigId?: string | null; + providerConfig: SecretProviderVaultRuntimeConfig | null; + operation: string; +}): HttpError { + return remoteProviderHttpError(error, { + companyId: context.companyId, + provider: context.provider, + providerConfigId: context.providerConfig?.id ?? context.providerConfigId ?? "deployment-default", + operation: context.operation, + providerConfig: context.providerConfig?.config ?? null, + }); +} + +async function throwProviderWriteOrReservedRowRollbackError(input: { + error: unknown; + rollbackReservedRow: () => Promise; + companyId: string; + provider: SecretProvider; + providerConfigId?: string | null; + providerConfig: SecretProviderVaultRuntimeConfig | null; + operation: string; +}): Promise { + const providerError = remoteProviderWriteHttpError(input.error, input); + try { + await input.rollbackReservedRow(); + } catch (rollbackError) { + const providerConfigId = input.providerConfig?.id ?? input.providerConfigId ?? "deployment-default"; + logger.warn( + { + err: rollbackError, + providerErr: providerError, + companyId: input.companyId, + provider: input.provider, + providerConfigId, + operation: input.operation, + }, + "remote secret provider write failed and reserved secret rollback failed", + ); + throw new HttpError(500, "Secret create failed and Paperclip could not roll back the local secret reservation.", { + code: "secret_create_rollback_failed", + provider: input.provider, + operation: input.operation, + providerConfigId, + providerError: { + status: providerError.status, + message: providerError.message, + details: providerError.details ?? null, + }, + }); + } + throw providerError; +} + +function providerConfigIdentifier(input: { + providerConfigId?: string | null; + providerConfig: SecretProviderVaultRuntimeConfig | null; +}) { + return input.providerConfig?.id ?? input.providerConfigId ?? "deployment-default"; +} + +async function deleteLocalSecretCreateReservationOrThrow(input: { + db: Pick; + secretId: string; + companyId: string; + provider: SecretProvider; + providerConfigId?: string | null; + providerConfig: SecretProviderVaultRuntimeConfig | null; + operation: string; +}) { + try { + await input.db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, input.secretId)); + await input.db.delete(companySecrets).where(eq(companySecrets.id, input.secretId)); + } catch (rollbackError) { + const providerConfigId = providerConfigIdentifier(input); + logger.warn( + { + err: rollbackError, + companyId: input.companyId, + provider: input.provider, + providerConfigId, + operation: input.operation, + }, + "secret create failed and local reserved secret rollback failed", + ); + throw new HttpError(500, "Secret create failed and Paperclip could not roll back the local secret reservation.", { + code: "secret_create_rollback_failed", + provider: input.provider, + operation: input.operation, + providerConfigId, + }); + } +} + +function throwProviderCleanupFailedAfterCreateRollback(input: { + companyId: string; + provider: SecretProvider; + providerConfigId?: string | null; + providerConfig: SecretProviderVaultRuntimeConfig | null; + operation: string; +}): never { + const providerConfigId = providerConfigIdentifier(input); + throw new HttpError(500, "Secret create failed and Paperclip could not clean up the remote provider secret.", { + code: "secret_create_provider_cleanup_failed", + provider: input.provider, + operation: input.operation, + providerConfigId, + localCleanupHandle: true, + }); +} + function safeRemoteProviderErrorDetails( error: { code: string } | null, context: { @@ -144,7 +257,32 @@ function safeRemoteProviderErrorDetails( context.provider !== "aws_secrets_manager" || context.operation !== "secret_provider_config.discovery.preview" ) { - return { code: error?.code ?? "provider_error" }; + if (context.provider !== "aws_secrets_manager") { + return { code: error?.code ?? "provider_error" }; + } + const details: Record = { + code: error?.code ?? "provider_error", + provider: context.provider, + operation: context.operation, + providerConfigId: context.providerConfigId, + }; + const region = safeString(context.providerConfig?.region); + if (region) details.region = region; + details.credentialPath = "Paperclip server runtime/provider credential path"; + if (error?.code === "access_denied") { + if (context.operation === "secret.create") { + details.requiredCapability = "secretsmanager:CreateSecret"; + details.actionableMessage = + "AWS managed secret creation needs secretsmanager:CreateSecret in the selected region for this provider vault. If the vault config uses a KMS key, the runtime credentials also need KMS write permissions for that key."; + details.safeAlternative = + "If the secret already exists in AWS, link it as an external reference instead of creating a Paperclip-managed value."; + } else if (context.operation === "secret.rotate") { + details.requiredCapability = "secretsmanager:PutSecretValue"; + details.actionableMessage = + "AWS managed secret rotation needs secretsmanager:PutSecretValue for the selected provider vault and managed secret path."; + } + } + return details; } const details: Record = { code: error?.code ?? "provider_error", @@ -1591,8 +1729,15 @@ export function secretService(db: Db) { context: providerWriteContext, }); } catch (error) { - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); - throw error; + throw await throwProviderWriteOrReservedRowRollbackError({ + error, + rollbackReservedRow: () => db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)), + companyId, + provider: provider.id, + providerConfigId, + providerConfig, + operation: "secret.create", + }); } try { @@ -1625,17 +1770,33 @@ export function secretService(db: Db) { }); } catch (error) { if (managedMode === "paperclip_managed") { - await cleanupPreparedProviderWrite({ + const cleaned = await cleanupPreparedProviderWrite({ provider, prepared, providerConfig, context: providerWriteContext, mode: "delete", operation: "user_secret_value.create_rollback", - }).catch(() => false); + }); + if (!cleaned) { + throwProviderCleanupFailedAfterCreateRollback({ + companyId, + provider: provider.id, + providerConfigId, + providerConfig, + operation: "user_secret_value.create_rollback", + }); + } } - await db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, reservedSecret.id)).catch(() => undefined); - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); + await deleteLocalSecretCreateReservationOrThrow({ + db, + secretId: reservedSecret.id, + companyId, + provider: provider.id, + providerConfigId, + providerConfig, + operation: "user_secret_value.create_rollback", + }); throw error; } } @@ -2855,8 +3016,15 @@ export function secretService(db: Db) { context: providerWriteContext, }); } catch (error) { - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); - throw error; + throw await throwProviderWriteOrReservedRowRollbackError({ + error, + rollbackReservedRow: () => db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)), + companyId, + provider: provider.id, + providerConfigId: input.providerConfigId ?? null, + providerConfig, + operation: "secret.create", + }); } try { @@ -2889,14 +3057,25 @@ export function secretService(db: Db) { mode: "delete", operation: "create.prepare_rollback", }); - if (cleaned) { - await db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, reservedSecret.id)).catch(() => undefined); - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); + if (!cleaned) { + throwProviderCleanupFailedAfterCreateRollback({ + companyId, + provider: provider.id, + providerConfigId: input.providerConfigId ?? null, + providerConfig, + operation: "create.prepare_rollback", + }); } - } else { - await db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, reservedSecret.id)).catch(() => undefined); - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); } + await deleteLocalSecretCreateReservationOrThrow({ + db, + secretId: reservedSecret.id, + companyId, + provider: provider.id, + providerConfigId: input.providerConfigId ?? null, + providerConfig, + operation: "create.prepare_rollback", + }); throw error; } @@ -2936,14 +3115,25 @@ export function secretService(db: Db) { mode: "delete", operation: "create.rollback", }); - if (cleaned) { - await db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, reservedSecret.id)).catch(() => undefined); - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); + if (!cleaned) { + throwProviderCleanupFailedAfterCreateRollback({ + companyId, + provider: provider.id, + providerConfigId: input.providerConfigId ?? null, + providerConfig, + operation: "create.rollback", + }); } - } else { - await db.delete(companySecretVersions).where(eq(companySecretVersions.secretId, reservedSecret.id)).catch(() => undefined); - await db.delete(companySecrets).where(eq(companySecrets.id, reservedSecret.id)).catch(() => undefined); } + await deleteLocalSecretCreateReservationOrThrow({ + db, + secretId: reservedSecret.id, + companyId, + provider: provider.id, + providerConfigId: input.providerConfigId ?? null, + providerConfig, + operation: "create.rollback", + }); throw error; } }, @@ -2986,20 +3176,31 @@ export function secretService(db: Db) { 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: providerWriteContext, - }) - : await provider.createVersion({ - value: input.value ?? "", - externalRef: secret.externalRef ?? null, - providerConfig, - context: providerWriteContext, - }); + let prepared: PreparedSecretVersion; + try { + prepared = + secret.managedMode === "external_reference" + ? await provider.linkExternalSecret({ + externalRef: input.externalRef ?? secret.externalRef ?? "", + providerVersionRef: input.providerVersionRef ?? null, + providerConfig, + context: providerWriteContext, + }) + : await provider.createVersion({ + value: input.value ?? "", + externalRef: secret.externalRef ?? null, + providerConfig, + context: providerWriteContext, + }); + } catch (error) { + throw remoteProviderWriteHttpError(error, { + companyId: secret.companyId, + provider: provider.id, + providerConfigId, + providerConfig, + operation: "secret.rotate", + }); + } try { await db.insert(companySecretVersions).values({