From e71b1368593fc3e84cf342997497c1644f2fa0d4 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Thu, 10 Jul 2025 16:14:40 +0400 Subject: [PATCH] requested changes --- ...5149_required-path-on-approval-policies.ts | 55 +++++++++++++++++++ .../v1/secret-approval-policy-router.ts | 11 ++-- .../secret-approval-policy-types.ts | 4 +- .../src/hooks/api/accessApproval/types.ts | 2 +- .../src/hooks/api/secretApproval/types.ts | 4 +- .../components/AccessPolicyModal.tsx | 11 +--- 6 files changed, 67 insertions(+), 20 deletions(-) create mode 100644 backend/src/db/migrations/20250710115149_required-path-on-approval-policies.ts diff --git a/backend/src/db/migrations/20250710115149_required-path-on-approval-policies.ts b/backend/src/db/migrations/20250710115149_required-path-on-approval-policies.ts new file mode 100644 index 000000000..b5f5abef7 --- /dev/null +++ b/backend/src/db/migrations/20250710115149_required-path-on-approval-policies.ts @@ -0,0 +1,55 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + const existingSecretApprovalPolicies = await knex(TableName.SecretApprovalPolicy) + .whereNull("secretPath") + .orWhere("secretPath", ""); + + const existingAccessApprovalPolicies = await knex(TableName.AccessApprovalPolicy) + .whereNull("secretPath") + .orWhere("secretPath", ""); + + // update all the secret approval policies secretPath to be "/**" + if (existingSecretApprovalPolicies.length) { + await knex(TableName.SecretApprovalPolicy) + .whereIn( + "id", + existingSecretApprovalPolicies.map((el) => el.id) + ) + .update({ + secretPath: "/**" + }); + } + + // update all the access approval policies secretPath to be "/**" + if (existingAccessApprovalPolicies.length) { + await knex(TableName.AccessApprovalPolicy) + .whereIn( + "id", + existingAccessApprovalPolicies.map((el) => el.id) + ) + .update({ + secretPath: "/**" + }); + } + + await knex.schema.alterTable(TableName.SecretApprovalPolicy, (table) => { + table.string("secretPath").notNullable().alter(); + }); + + await knex.schema.alterTable(TableName.AccessApprovalPolicy, (table) => { + table.string("secretPath").notNullable().alter(); + }); +} + +export async function down(knex: Knex): Promise { + await knex.schema.alterTable(TableName.SecretApprovalPolicy, (table) => { + table.string("secretPath").nullable().alter(); + }); + + await knex.schema.alterTable(TableName.AccessApprovalPolicy, (table) => { + table.string("secretPath").nullable().alter(); + }); +} diff --git a/backend/src/ee/routes/v1/secret-approval-policy-router.ts b/backend/src/ee/routes/v1/secret-approval-policy-router.ts index 2fe13e0d2..46b2544b2 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -23,10 +23,8 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi environment: z.string(), secretPath: z .string() - .optional() - .nullable() - .default("/") - .transform((val) => (val ? removeTrailingSlash(val) : val)), + .min(1, { message: "Secret path cannot be empty" }) + .transform((val) => removeTrailingSlash(val)), approvers: z .discriminatedUnion("type", [ z.object({ type: z.literal(ApproverType.Group), id: z.string() }), @@ -100,9 +98,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi approvals: z.number().min(1).default(1), secretPath: z .string() + .trim() + .min(1, { message: "Secret path cannot be empty" }) .optional() - .nullable() - .transform((val) => (val ? removeTrailingSlash(val) : val)), + .transform((val) => (val ? removeTrailingSlash(val) : undefined)), enforcementLevel: z.nativeEnum(EnforcementLevel).optional(), allowedSelfApprovals: z.boolean().default(true) }), diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts index ed074336c..ba5334e5c 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts @@ -4,7 +4,7 @@ import { ApproverType, BypasserType } from "../access-approval-policy/access-app export type TCreateSapDTO = { approvals: number; - secretPath?: string | null; + secretPath: string; environment: string; approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; username?: string })[]; bypassers?: ( @@ -20,7 +20,7 @@ export type TCreateSapDTO = { export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; - secretPath?: string | null; + secretPath?: string; approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; username?: string })[]; bypassers?: ( | { type: BypasserType.Group; id: string } diff --git a/frontend/src/hooks/api/accessApproval/types.ts b/frontend/src/hooks/api/accessApproval/types.ts index cde04ee61..70e9b883e 100644 --- a/frontend/src/hooks/api/accessApproval/types.ts +++ b/frontend/src/hooks/api/accessApproval/types.ts @@ -170,7 +170,7 @@ export type TCreateAccessPolicyDTO = { approvers?: Approver[]; bypassers?: Bypasser[]; approvals?: number; - secretPath?: string; + secretPath: string; enforcementLevel?: EnforcementLevel; allowedSelfApprovals: boolean; approvalsRequired?: { numberOfApprovals: number; stepNumber: number }[]; diff --git a/frontend/src/hooks/api/secretApproval/types.ts b/frontend/src/hooks/api/secretApproval/types.ts index 15afcf119..eeb734115 100644 --- a/frontend/src/hooks/api/secretApproval/types.ts +++ b/frontend/src/hooks/api/secretApproval/types.ts @@ -49,7 +49,7 @@ export type TCreateSecretPolicyDTO = { workspaceId: string; name?: string; environment: string; - secretPath?: string | null; + secretPath: string; approvers?: Approver[]; bypassers?: Bypasser[]; approvals?: number; @@ -62,7 +62,7 @@ export type TUpdateSecretPolicyDTO = { name?: string; approvers?: Approver[]; bypassers?: Bypasser[]; - secretPath?: string | null; + secretPath?: string; approvals?: number; allowedSelfApprovals?: boolean; enforcementLevel?: EnforcementLevel; diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx index e5b1825ef..40d116c5b 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/AccessPolicyModal.tsx @@ -55,7 +55,7 @@ const formSchema = z .object({ environment: z.object({ slug: z.string(), name: z.string() }), name: z.string().optional(), - secretPath: z.string().trim().optional(), + secretPath: z.string().trim().min(1), approvals: z.number().min(1).default(1), userApprovers: z .object({ type: z.literal(ApproverType.User), id: z.string() }) @@ -106,14 +106,6 @@ const formSchema = z message: "At least one approver should be provided" }); } - } else if (data.policyType === PolicyType.AccessPolicy) { - if (!data.secretPath) { - ctx.addIssue({ - path: ["secretPath"], - code: z.ZodIssueCode.custom, - message: "Secret path cannot be empty" - }); - } } }); @@ -477,6 +469,7 @@ const Form = ({