diff --git a/backend/src/db/migrations/20250824173438_add-approval-secret-read-compat.ts b/backend/src/db/migrations/20250824173438_add-approval-secret-read-compat.ts new file mode 100644 index 000000000..7398920af --- /dev/null +++ b/backend/src/db/migrations/20250824173438_add-approval-secret-read-compat.ts @@ -0,0 +1,19 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + if (!(await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "shouldCheckSecretPermission"))) { + await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => { + t.boolean("shouldCheckSecretPermission").nullable(); + }); + } +} + +export async function down(knex: Knex): Promise { + if (await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "shouldCheckSecretPermission")) { + await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => { + t.dropColumn("shouldCheckSecretPermission"); + }); + } +} diff --git a/backend/src/db/migrations/20250824192801_backfill-secret-read-compat-flag.ts b/backend/src/db/migrations/20250824192801_backfill-secret-read-compat-flag.ts new file mode 100644 index 000000000..7a629ca52 --- /dev/null +++ b/backend/src/db/migrations/20250824192801_backfill-secret-read-compat-flag.ts @@ -0,0 +1,29 @@ +import { Knex } from "knex"; + +import { selectAllTableCols } from "@app/lib/knex"; + +import { TableName } from "../schemas"; + +const BATCH_SIZE = 100; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "shouldCheckSecretPermission")) { + // find all existing SecretApprovalPolicy rows to backfill shouldCheckSecretPermission flag + const rows = await knex(TableName.SecretApprovalPolicy).select(selectAllTableCols(TableName.SecretApprovalPolicy)); + + if (rows.length > 0) { + for (let i = 0; i < rows.length; i += BATCH_SIZE) { + const batch = rows.slice(i, i + BATCH_SIZE); + // eslint-disable-next-line no-await-in-loop + await knex(TableName.SecretApprovalPolicy) + .whereIn( + "id", + batch.map((row) => row.id) + ) + .update({ shouldCheckSecretPermission: true }); + } + } + } +} + +export async function down(): Promise {} diff --git a/backend/src/db/schemas/secret-approval-policies.ts b/backend/src/db/schemas/secret-approval-policies.ts index 0273e617c..dbb881db3 100644 --- a/backend/src/db/schemas/secret-approval-policies.ts +++ b/backend/src/db/schemas/secret-approval-policies.ts @@ -17,7 +17,8 @@ export const SecretApprovalPoliciesSchema = z.object({ updatedAt: z.date(), enforcementLevel: z.string().default("hard"), deletedAt: z.date().nullable().optional(), - allowedSelfApprovals: z.boolean().default(true) + allowedSelfApprovals: z.boolean().default(true), + shouldCheckSecretPermission: z.boolean().nullable().optional() }); export type TSecretApprovalPolicies = z.infer; diff --git a/backend/src/ee/routes/v1/secret-approval-request-router.ts b/backend/src/ee/routes/v1/secret-approval-request-router.ts index 104738140..bdc9c2dcd 100644 --- a/backend/src/ee/routes/v1/secret-approval-request-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-request-router.ts @@ -305,7 +305,8 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv secretPath: z.string().optional().nullable(), enforcementLevel: z.string(), deletedAt: z.date().nullish(), - allowedSelfApprovals: z.boolean() + allowedSelfApprovals: z.boolean(), + shouldCheckSecretPermission: z.boolean().nullable().optional() }), environment: z.string(), statusChangedByUser: approvalRequestUser.optional(), diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts index 49512768c..daa6ebc29 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts @@ -180,7 +180,11 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { tx.ref("enforcementLevel").withSchema(TableName.SecretApprovalPolicy).as("policyEnforcementLevel"), tx.ref("allowedSelfApprovals").withSchema(TableName.SecretApprovalPolicy).as("policyAllowedSelfApprovals"), tx.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals"), - tx.ref("deletedAt").withSchema(TableName.SecretApprovalPolicy).as("policyDeletedAt") + tx.ref("deletedAt").withSchema(TableName.SecretApprovalPolicy).as("policyDeletedAt"), + tx + .ref("shouldCheckSecretPermission") + .withSchema(TableName.SecretApprovalPolicy) + .as("policySecretReadAccessCompat") ); const findById = async (id: string, tx?: Knex) => { @@ -220,7 +224,8 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { enforcementLevel: el.policyEnforcementLevel, envId: el.policyEnvId, deletedAt: el.policyDeletedAt, - allowedSelfApprovals: el.policyAllowedSelfApprovals + allowedSelfApprovals: el.policyAllowedSelfApprovals, + shouldCheckSecretPermission: el.policySecretReadAccessCompat } }), childrenMapper: [ diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts index 9e32ad7de..d485f7ea0 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts @@ -281,13 +281,22 @@ export const secretApprovalRequestServiceFactory = ({ ) { throw new ForbiddenRequestError({ message: "User has insufficient privileges" }); } - const getHasSecretReadAccess = (environment: string, tags: { slug: string }[], secretPath?: string) => { - const canRead = hasSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, { - environment, - secretPath: secretPath || "/", - secretTags: tags.map((i) => i.slug) - }); - return canRead; + const getHasSecretReadAccess = ( + shouldCheckSecretPermission: boolean | null | undefined, + environment: string, + tags: { slug: string }[], + secretPath?: string + ) => { + if (shouldCheckSecretPermission) { + const canRead = hasSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: secretPath || "/", + secretTags: tags.map((i) => i.slug) + }); + return canRead; + } + + return true; }; let secrets; @@ -309,8 +318,18 @@ export const secretApprovalRequestServiceFactory = ({ version: el.version, secretMetadata: el.secretMetadata as ResourceMetadataDTO, isRotatedSecret: el.secret?.isRotatedSecret ?? false, - secretValueHidden: !getHasSecretReadAccess(secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path), - secretValue: !getHasSecretReadAccess(secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path) + secretValueHidden: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, + secretApprovalRequest.environment, + el.tags, + secretPath?.[0]?.path + ), + secretValue: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, + secretApprovalRequest.environment, + el.tags, + secretPath?.[0]?.path + ) ? INFISICAL_SECRET_VALUE_HIDDEN_MASK : el.secret && el.secret.isRotatedSecret ? undefined @@ -326,11 +345,17 @@ export const secretApprovalRequestServiceFactory = ({ id: el.secret.id, version: el.secret.version, secretValueHidden: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path ), - secretValue: !getHasSecretReadAccess(secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path) + secretValue: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, + secretApprovalRequest.environment, + el.tags, + secretPath?.[0]?.path + ) ? INFISICAL_SECRET_VALUE_HIDDEN_MASK : el.secret.encryptedValue ? secretManagerDecryptor({ cipherTextBlob: el.secret.encryptedValue }).toString() @@ -346,11 +371,17 @@ export const secretApprovalRequestServiceFactory = ({ id: el.secretVersion.id, version: el.secretVersion.version, secretValueHidden: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path ), - secretValue: !getHasSecretReadAccess(secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path) + secretValue: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, + secretApprovalRequest.environment, + el.tags, + secretPath?.[0]?.path + ) ? INFISICAL_SECRET_VALUE_HIDDEN_MASK : el.secretVersion.encryptedValue ? secretManagerDecryptor({ cipherTextBlob: el.secretVersion.encryptedValue }).toString() @@ -368,7 +399,12 @@ export const secretApprovalRequestServiceFactory = ({ const encryptedSecrets = await secretApprovalRequestSecretDAL.findByRequestId(secretApprovalRequest.id); secrets = encryptedSecrets.map((el) => ({ ...el, - secretValueHidden: !getHasSecretReadAccess(secretApprovalRequest.environment, el.tags, secretPath?.[0]?.path), + secretValueHidden: !getHasSecretReadAccess( + secretApprovalRequest.policy.shouldCheckSecretPermission, + secretApprovalRequest.environment, + el.tags, + secretPath?.[0]?.path + ), ...decryptSecretWithBot(el, botKey), secret: el.secret ? {