improvements: address feedback

This commit is contained in:
Scott Wilson
2025-04-02 14:11:59 -07:00
parent 766c1242fd
commit 577c81be65
44 changed files with 453 additions and 269 deletions
@@ -13,7 +13,8 @@ export const verifyHostInputValidity = async (host: string, isGateway = false) =
const reservedHosts = [appCfg.DB_HOST || getDbConnectionHost(appCfg.DB_CONNECTION_URI)].concat(
(appCfg.DB_READ_REPLICAS || []).map((el) => getDbConnectionHost(el.DB_CONNECTION_URI)),
getDbConnectionHost(appCfg.REDIS_URL)
getDbConnectionHost(appCfg.REDIS_URL),
getDbConnectionHost(appCfg.AUDIT_LOGS_DB_CONNECTION_URI)
);
// get host db ip
@@ -257,6 +257,11 @@ export const secretApprovalRequestSecretDALFactory = (db: TDbClient) => {
db.ref("id").withSchema("secVerTag")
)
.leftJoin(TableName.ResourceMetadata, `${TableName.SecretV2}.id`, `${TableName.ResourceMetadata}.secretId`)
.leftJoin(
TableName.SecretRotationV2SecretMapping,
`${TableName.SecretV2}.id`,
`${TableName.SecretRotationV2SecretMapping}.secretId`
)
.select(selectAllTableCols(TableName.SecretApprovalRequestSecretV2))
.select({
secVerTagId: "secVerTag.id",
@@ -285,7 +290,8 @@ export const secretApprovalRequestSecretDALFactory = (db: TDbClient) => {
db.ref("id").withSchema(TableName.ResourceMetadata).as("metadataId"),
db.ref("key").withSchema(TableName.ResourceMetadata).as("metadataKey"),
db.ref("value").withSchema(TableName.ResourceMetadata).as("metadataValue")
);
)
.select(db.ref("rotationId").withSchema(TableName.SecretRotationV2SecretMapping));
const formatedDoc = sqlNestRelationships({
data: doc,
key: "id",
@@ -304,14 +310,16 @@ export const secretApprovalRequestSecretDALFactory = (db: TDbClient) => {
{
key: "secretId",
label: "secret" as const,
mapper: ({ orgSecVersion, orgSecKey, orgSecValue, orgSecComment, secretId }) =>
mapper: ({ orgSecVersion, orgSecKey, orgSecValue, orgSecComment, secretId, rotationId }) =>
secretId
? {
id: secretId,
version: orgSecVersion,
key: orgSecKey,
encryptedValue: orgSecValue,
encryptedComment: orgSecComment
encryptedComment: orgSecComment,
isRotatedSecret: Boolean(rotationId),
rotationId
}
: undefined
},
@@ -262,7 +262,13 @@ export const secretApprovalRequestServiceFactory = ({
id: el.id,
version: el.version,
secretMetadata: el.secretMetadata as ResourceMetadataDTO,
secretValue: el.encryptedValue ? secretManagerDecryptor({ cipherTextBlob: el.encryptedValue }).toString() : "",
isRotatedSecret: el.secret.isRotatedSecret,
// eslint-disable-next-line no-nested-ternary
secretValue: el.secret.isRotatedSecret
? undefined
: el.encryptedValue
? secretManagerDecryptor({ cipherTextBlob: el.encryptedValue }).toString()
: "",
secretComment: el.encryptedComment
? secretManagerDecryptor({ cipherTextBlob: el.encryptedComment }).toString()
: "",
@@ -609,7 +615,7 @@ export const secretApprovalRequestServiceFactory = ({
tx,
inputSecrets: secretUpdationCommits.map((el) => {
const encryptedValue =
typeof el.encryptedValue !== "undefined"
!el.secret.isRotatedSecret && typeof el.encryptedValue !== "undefined"
? {
encryptedValue: el.encryptedValue as Buffer,
references: el.encryptedValue
@@ -189,9 +189,11 @@ export const secretRotationV2DALFactory = (
.countDistinct(`${TableName.SecretRotationV2}.name`);
if (search) {
void query
.whereILike(`${TableName.SecretV2}.key`, `%${search}%`)
.orWhereILike(`${TableName.SecretRotationV2}.name`, `%${search}%`);
void query.where((qb) => {
void qb
.whereILike(`${TableName.SecretV2}.key`, `%${search}%`)
.orWhereILike(`${TableName.SecretRotationV2}.name`, `%${search}%`);
});
}
const result = await query;
@@ -23,7 +23,7 @@ export const listSecretRotationOptions = () => {
return Object.values(SECRET_ROTATION_LIST_OPTIONS).sort((a, b) => a.name.localeCompare(b.name));
};
const getNextUTCMidnight = ({ hours, minutes }: TSecretRotationV2["rotateAtUtc"] = { hours: 0, minutes: 0 }) => {
const getNextUTCDayInterval = ({ hours, minutes }: TSecretRotationV2["rotateAtUtc"] = { hours: 0, minutes: 0 }) => {
const now = new Date();
return new Date(
@@ -39,7 +39,7 @@ const getNextUTCMidnight = ({ hours, minutes }: TSecretRotationV2["rotateAtUtc"]
);
};
const getNextUTCMinute = ({ minutes }: TSecretRotationV2["rotateAtUtc"] = { hours: 0, minutes: 0 }) => {
const getNextUTCMinuteInterval = ({ minutes }: TSecretRotationV2["rotateAtUtc"] = { hours: 0, minutes: 0 }) => {
const now = new Date();
return new Date(
Date.UTC(
@@ -58,10 +58,10 @@ export const getNextUtcRotationInterval = (rotateAtUtc?: TSecretRotationV2["rota
const appCfg = getConfig();
if (appCfg.isRotationDevelopmentMode) {
return getNextUTCMinute(rotateAtUtc);
return getNextUTCMinuteInterval(rotateAtUtc);
}
return getNextUTCMidnight(rotateAtUtc);
return getNextUTCDayInterval(rotateAtUtc);
};
export const encryptSecretRotationCredentials = async ({
@@ -80,7 +80,7 @@ export const secretRotationV2QueueServiceFactory = async ({
{
batchSize: 1,
workerCount: 1,
pollingIntervalSeconds: 0.5
pollingIntervalSeconds: appCfg.isRotationDevelopmentMode ? 0.5 : 30
}
);
@@ -122,7 +122,7 @@ export const secretRotationV2QueueServiceFactory = async ({
},
{
batchSize: 1,
workerCount: 30,
workerCount: 2,
pollingIntervalSeconds: 0.5
}
);
@@ -179,8 +179,8 @@ export const secretRotationV2QueueServiceFactory = async ({
},
{
batchSize: 1,
workerCount: 5,
pollingIntervalSeconds: 30
workerCount: 2,
pollingIntervalSeconds: 1
}
);
@@ -1,4 +1,5 @@
import { ForbiddenError, subject } from "@casl/ability";
import { Knex } from "knex";
import isEqual from "lodash.isequal";
import { ActionProjectType, SecretType, TableName } from "@app/db/schemas";
@@ -46,7 +47,7 @@ import {
} from "@app/ee/services/secret-rotation-v2/secret-rotation-v2-types";
import { sqlCredentialsRotationFactory } from "@app/ee/services/secret-rotation-v2/shared/sql-credentials";
import { TSecretSnapshotServiceFactory } from "@app/ee/services/secret-snapshot/secret-snapshot-service";
import { KeyStorePrefixes, TKeyStoreFactory } from "@app/keystore/keystore";
import { KeyStorePrefixes, PgSqlLock, TKeyStoreFactory } from "@app/keystore/keystore";
import { getConfig } from "@app/lib/config/env";
import { DatabaseErrorCode } from "@app/lib/error-codes";
import { BadRequestError, DatabaseError, InternalServerError, NotFoundError } from "@app/lib/errors";
@@ -141,6 +142,37 @@ export const secretRotationV2ServiceFactory = ({
);
};
const $throwOnConflictingSecrets = async ({
secretKeys,
folderId,
tx,
secretPath
}: {
secretKeys: string[];
folderId: string;
tx: Knex;
secretPath: string;
}) => {
const conflictingSecrets = await secretV2BridgeDAL.find(
{
$in: {
[`${TableName.SecretV2}.key` as "key"]: secretKeys
},
[`${TableName.SecretV2}.folderId` as "folderId"]: folderId,
[`${TableName.SecretV2}.type` as "type"]: SecretType.Shared
},
{ tx }
);
if (conflictingSecrets.length) {
throw new BadRequestError({
message: `The following secrets already exist at the path "${secretPath}": ${conflictingSecrets
.map(({ key }) => key)
.join(", ")}`
});
}
};
const listSecretRotationsByProjectId = async (
{ projectId, type }: TListSecretRotationsV2ByProjectId,
actor: OrgServiceActor
@@ -345,6 +377,7 @@ export const secretRotationV2ServiceFactory = ({
secretPath,
environment,
rotateAtUtc = { hours: 0, minutes: 0 },
secretsMapping,
...payload
}: TCreateSecretRotationV2DTO,
actor: OrgServiceActor
@@ -368,7 +401,10 @@ export const secretRotationV2ServiceFactory = ({
const { shouldUseSecretV2Bridge } = await projectBotService.getBotKey(projectId);
if (!shouldUseSecretV2Bridge)
throw new BadRequestError({ message: "Project version does not support Secret Rotation V2" });
throw new BadRequestError({
message:
"Project version does not support Secret Rotation V2. Please upgrade your project via the Infiscal Dashboard to gain access."
});
ForbiddenError.from(permission).throwUnlessCan(
ProjectPermissionSecretRotationActions.Create,
@@ -389,7 +425,7 @@ export const secretRotationV2ServiceFactory = ({
const rotationFactory = SECRET_ROTATION_FACTORY_MAP[payload.type]({
parameters: payload.parameters,
secretsMapping: payload.secretsMapping,
secretsMapping,
connection
} as TSecretRotationV2WithConnection);
@@ -405,9 +441,19 @@ export const secretRotationV2ServiceFactory = ({
});
return secretRotationV2DAL.transaction(async (tx) => {
await tx.raw("SELECT pg_advisory_xact_lock(?)", [PgSqlLock.SecretRotationV2Creation(folder.id)]);
await $throwOnConflictingSecrets({
secretPath,
secretKeys: Object.values(secretsMapping),
tx,
folderId: folder.id
});
const createdRotation = await secretRotationV2DAL.create(
{
folderId: folder.id,
secretsMapping,
...payload,
encryptedGeneratedCredentials,
rotateAtUtc,
@@ -483,12 +529,6 @@ export const secretRotationV2ServiceFactory = ({
throw new BadRequestError({
message: `A Secret Rotation with the name "${payload.name}" already exists at the secret path "${secretPath}"`
});
case TableName.SecretV2:
throw new BadRequestError({
message: `One or more of the following secrets already exists at the secret path "${secretPath}": ${Object.values(
payload.secretsMapping
).join(", ")}`
});
default:
throw err;
}
@@ -497,6 +537,8 @@ export const secretRotationV2ServiceFactory = ({
throw err;
}
if (err instanceof BadRequestError) throw err;
throw new BadRequestError({
message: parseRotationErrorMessage(err)
});
@@ -521,7 +563,8 @@ export const secretRotationV2ServiceFactory = ({
message: `Could not find ${SECRET_ROTATION_NAME_MAP[type]} Rotation with ID ${rotationId}`
});
const { folder, environment, projectId, folderId, connection, secretsMapping } = secretRotation;
const { folder, environment, projectId, folderId, connection } = secretRotation;
const secretsMapping = secretRotation.secretsMapping as TSecretRotationV2["secretsMapping"];
const { permission } = await permissionService.getProjectPermission({
actor: actor.type,
@@ -551,26 +594,36 @@ export const secretRotationV2ServiceFactory = ({
isManualRotation: false
});
let secretsMappingUpdated = false;
try {
const updatedSecretRotation = await secretRotationV2DAL.transaction(async (tx) => {
await tx.raw("SELECT pg_advisory_xact_lock(?)", [PgSqlLock.SecretRotationV2Creation(folder.id)]);
if (payload.secretsMapping && !isEqual(payload.secretsMapping, secretsMapping)) {
const currentMappingKeys = Object.values(secretsMapping);
await $throwOnConflictingSecrets({
secretPath: folder.path,
secretKeys: Object.values(payload.secretsMapping).filter((key) => !currentMappingKeys.includes(key)),
tx,
folderId: folder.id
});
// update mapped secrets names
await fnSecretBulkUpdate({
folderId,
orgId: connection.orgId,
tx,
inputSecrets: Object.entries(secretsMapping as TSecretRotationV2["secretsMapping"]).map(
([mappingKey, secretKey]) => ({
filter: {
key: secretKey,
folderId,
type: SecretType.Shared
},
data: {
key: payload.secretsMapping![mappingKey as keyof TSecretRotationV2["secretsMapping"]]
}
})
),
inputSecrets: Object.entries(secretsMapping).map(([mappingKey, secretKey]) => ({
filter: {
key: secretKey,
folderId,
type: SecretType.Shared
},
data: {
key: payload.secretsMapping![mappingKey as keyof TSecretRotationV2["secretsMapping"]]
}
})),
secretDAL: secretV2BridgeDAL,
secretVersionDAL: secretVersionV2BridgeDAL,
secretVersionTagDAL: secretVersionTagV2BridgeDAL,
@@ -578,14 +631,7 @@ export const secretRotationV2ServiceFactory = ({
resourceMetadataDAL
});
await snapshotService.performSnapshot(folder.id);
await secretQueueService.syncSecrets({
orgId: connection.orgId,
secretPath: folder.path,
projectId,
environmentSlug: environment.slug,
excludeReplication: true
});
secretsMappingUpdated = true;
}
return secretRotationV2DAL.updateById(
@@ -598,6 +644,17 @@ export const secretRotationV2ServiceFactory = ({
);
});
if (secretsMappingUpdated) {
await snapshotService.performSnapshot(folder.id);
await secretQueueService.syncSecrets({
orgId: connection.orgId,
secretPath: folder.path,
projectId,
environmentSlug: environment.slug,
excludeReplication: true
});
}
// queue for rotation if adjusted time falls before next cron
if (nextRotationAt && nextRotationAt.getTime() < getNextUtcRotationInterval().getTime()) {
await queueService.queuePg(
@@ -620,20 +677,14 @@ export const secretRotationV2ServiceFactory = ({
message: `A Secret Rotation with the name "${payload.name}" already exists at the secret path "${folder.path}"`
});
break;
case TableName.SecretV2:
if (payload.secretsMapping)
throw new BadRequestError({
message: `One or more of the following secrets already exists at the secret path "${
folder.path
}": ${Object.values(payload.secretsMapping).join(", ")}`
});
break;
default:
throw err;
}
}
}
if (err instanceof BadRequestError) throw err;
throw err;
}
};
@@ -695,15 +746,6 @@ export const secretRotationV2ServiceFactory = ({
actorId: actor.id, // not actually used since rotated secrets are shared
tx
});
await snapshotService.performSnapshot(folder.id);
await secretQueueService.syncSecrets({
orgId: connection.orgId,
secretPath: folder.path,
projectId,
environmentSlug: environment.slug,
excludeReplication: true
});
}
return secretRotationV2DAL.deleteById(rotationId, tx);
@@ -728,6 +770,17 @@ export const secretRotationV2ServiceFactory = ({
await deleteTransaction;
}
if (deleteSecrets) {
await snapshotService.performSnapshot(folder.id);
await secretQueueService.syncSecrets({
orgId: connection.orgId,
secretPath: folder.path,
projectId,
environmentSlug: environment.slug,
excludeReplication: true
});
}
return expandSecretRotation(secretRotation, kmsService);
};
@@ -398,8 +398,32 @@ export const secretSnapshotServiceFactory = ({
if (shouldUseBridge) {
const rollback = await snapshotDAL.transaction(async (tx) => {
const rollbackSnaps = await snapshotDAL.findRecursivelySnapshotsV2Bridge(snapshot.id, tx);
// this will remove all secrets in current folder
const deletedTopLevelSecs = await secretV2BridgeDAL.delete({ folderId: snapshot.folderId }, tx);
const secretRotationIds = rollbackSnaps
.flatMap((snap) => snap.secretVersions)
.filter((el) => el.isRotatedSecret)
.map((el) => el.secretId);
// this will remove all secrets in current folder except rotated secrets which we ignore
const deletedTopLevelSecs = await secretV2BridgeDAL.delete(
{
$complex: {
operator: "and",
value: [
{
operator: "eq",
field: "folderId",
value: snapshot.folderId
},
{
operator: "notIn",
field: "id",
value: secretRotationIds
}
]
}
},
tx
);
const deletedTopLevelSecsGroupById = groupBy(deletedTopLevelSecs, (item) => item.id);
// this will remove all secrets and folders on child
// due to sql foreign key and link list connection removing the folders removes everything below too
@@ -424,28 +448,31 @@ export const secretSnapshotServiceFactory = ({
);
const secrets = await secretV2BridgeDAL.insertMany(
rollbackSnaps.flatMap(({ secretVersions, folderId }) =>
secretVersions.map(
({
latestSecretVersion,
version,
updatedAt,
createdAt,
secretId,
envId,
id,
tags,
// exclude the bottom fields from the secret - they are for versioning only.
userActorId,
identityActorId,
actorType,
...el
}) => ({
...el,
id: secretId,
version: deletedTopLevelSecsGroupById[secretId] ? latestSecretVersion + 1 : latestSecretVersion,
folderId
})
)
secretVersions
.filter((v) => !v.isRotatedSecret)
.map(
({
latestSecretVersion,
version,
updatedAt,
createdAt,
secretId,
envId,
id,
tags,
// exclude the bottom fields from the secret - they are for versioning only.
userActorId,
identityActorId,
actorType,
isRotatedSecret,
...el
}) => ({
...el,
id: secretId,
version: deletedTopLevelSecsGroupById[secretId] ? latestSecretVersion + 1 : latestSecretVersion,
folderId
})
)
),
tx
);
@@ -181,6 +181,11 @@ export const snapshotDALFactory = (db: TDbClient) => {
`${TableName.SnapshotFolder}.folderVersionId`,
`${TableName.SecretFolderVersion}.id`
)
.leftJoin(
TableName.SecretRotationV2SecretMapping,
`${TableName.SecretRotationV2SecretMapping}.secretId`,
`${TableName.SecretVersionV2}.secretId`
)
.select(selectAllTableCols(TableName.SecretVersionV2))
.select(
db.ref("id").withSchema(TableName.Snapshot).as("snapshotId"),
@@ -195,7 +200,8 @@ export const snapshotDALFactory = (db: TDbClient) => {
db.ref("id").withSchema(TableName.SecretTag).as("tagId"),
db.ref("id").withSchema(TableName.SecretVersionV2Tag).as("tagVersionId"),
db.ref("color").withSchema(TableName.SecretTag).as("tagColor"),
db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug")
db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug"),
db.ref("rotationId").withSchema(TableName.SecretRotationV2SecretMapping)
);
return sqlNestRelationships({
data,
@@ -221,7 +227,11 @@ export const snapshotDALFactory = (db: TDbClient) => {
{
key: "id",
label: "secretVersions" as const,
mapper: (el) => SecretVersionsV2Schema.parse(el),
mapper: (el) => ({
...SecretVersionsV2Schema.parse(el),
isRotatedSecret: Boolean(el.rotationId),
rotationId: el.rotationId
}),
childrenMapper: [
{
key: "tagVersionId",
@@ -476,6 +486,11 @@ export const snapshotDALFactory = (db: TDbClient) => {
`${TableName.SecretVersionV2Tag}.${TableName.SecretTag}Id`,
`${TableName.SecretTag}.id`
)
.leftJoin(
TableName.SecretRotationV2SecretMapping,
`${TableName.SecretVersionV2}.secretId`,
`${TableName.SecretRotationV2SecretMapping}.secretId`
)
.leftJoin<{ latestSecretVersion: number }>(
(tx || db)(TableName.SecretVersionV2)
.groupBy("secretId")
@@ -506,7 +521,8 @@ export const snapshotDALFactory = (db: TDbClient) => {
db.ref("id").withSchema(TableName.SecretTag).as("tagId"),
db.ref("id").withSchema(TableName.SecretVersionV2Tag).as("tagVersionId"),
db.ref("color").withSchema(TableName.SecretTag).as("tagColor"),
db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug")
db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug"),
db.ref("rotationId").withSchema(TableName.SecretRotationV2SecretMapping)
);
const formated = sqlNestRelationships({
@@ -523,7 +539,8 @@ export const snapshotDALFactory = (db: TDbClient) => {
label: "secretVersions" as const,
mapper: (el) => ({
...SecretVersionsV2Schema.parse(el),
latestSecretVersion: el.latestSecretVersion as number
latestSecretVersion: el.latestSecretVersion as number,
isRotatedSecret: Boolean(el.rotationId)
}),
childrenMapper: [
{