Merge pull request #4574 from Infisical/secret-update-request-fixes

fix(secret-change-requests): various UPDATE change request fixes

backend/src/db/migrations/20250922222724_remove-secret-approval-multiline-encoding-default.ts

@@ -0,0 +1,19 @@
+import { Knex } from "knex";
+
+import { TableName } from "@app/db/schemas";
+
+export async function up(knex: Knex): Promise<void> {
+  if (await knex.schema.hasTable(TableName.SecretApprovalRequestSecretV2)) {
+    await knex.schema.alterTable(TableName.SecretApprovalRequestSecretV2, (t) => {
+      t.boolean("skipMultilineEncoding").alter();
+    });
+  }
+}
+
+export async function down(knex: Knex): Promise<void> {
+  if (await knex.schema.hasTable(TableName.SecretApprovalRequestSecretV2)) {
+    await knex.schema.alterTable(TableName.SecretApprovalRequestSecretV2, (t) => {
+      t.boolean("skipMultilineEncoding").defaultTo(false).alter();
+    });
+  }
+}

backend/src/db/schemas/secret-approval-requests-secrets-v2.ts

@@ -17,7 +17,7 @@ export const SecretApprovalRequestsSecretsV2Schema = z.object({
   encryptedComment: zodBuffer.nullable().optional(),
   reminderNote: z.string().nullable().optional(),
   reminderRepeatDays: z.number().nullable().optional(),
-  skipMultilineEncoding: z.boolean().default(false).nullable().optional(),
+  skipMultilineEncoding: z.boolean().nullable().optional(),
   metadata: z.unknown().nullable().optional(),
   createdAt: z.date(),
   updatedAt: z.date(),

backend/src/ee/routes/v1/secret-approval-request-router.ts

@@ -320,10 +320,20 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
                 .array(),
               secretPath: z.string(),
               commits: secretRawSchema
-                .omit({ _id: true, environment: true, workspace: true, type: true, version: true, secretValue: true })
+                .omit({
+                  _id: true,
+                  environment: true,
+                  workspace: true,
+                  type: true,
+                  version: true,
+                  secretValue: true,
+                  secretComment: true
+                })
                 .extend({
                   secretValueHidden: z.boolean(),
                   secretValue: z.string().optional(),
+                  secretComment: z.string().optional(),
+                  skipMultilineEncoding: z.boolean().nullish(),
                   isRotatedSecret: z.boolean().optional(),
                   op: z.string(),
                   tags: SanitizedTagSchema.array().optional(),
@@ -348,7 +358,8 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
                       secretValueHidden: z.boolean(),
                       secretComment: z.string().optional(),
                       tags: SanitizedTagSchema.array().optional(),
-                      secretMetadata: ResourceMetadataSchema.nullish()
+                      secretMetadata: ResourceMetadataSchema.nullish(),
+                      skipMultilineEncoding: z.boolean().nullish()
                     })
                     .optional()
                 })

backend/src/ee/services/secret-approval-request/secret-approval-request-secret-dal.ts

@@ -284,7 +284,8 @@ export const secretApprovalRequestSecretDALFactory = (db: TDbClient) => {
           db.ref("version").withSchema(TableName.SecretVersionV2).as("secVerVersion"),
           db.ref("key").withSchema(TableName.SecretVersionV2).as("secVerKey"),
           db.ref("encryptedValue").withSchema(TableName.SecretVersionV2).as("secVerValue"),
-          db.ref("encryptedComment").withSchema(TableName.SecretVersionV2).as("secVerComment")
+          db.ref("encryptedComment").withSchema(TableName.SecretVersionV2).as("secVerComment"),
+          db.ref("skipMultilineEncoding").withSchema(TableName.SecretVersionV2).as("secVerSkipMultilineEncoding")
         )
         .select(
           db.ref("id").withSchema(TableName.ResourceMetadata).as("metadataId"),
@@ -326,14 +327,22 @@ export const secretApprovalRequestSecretDALFactory = (db: TDbClient) => {
           {
             key: "secretVersion",
             label: "secretVersion" as const,
-            mapper: ({ secretVersion, secVerVersion, secVerKey, secVerValue, secVerComment }) =>
+            mapper: ({
+              secretVersion,
+              secVerVersion,
+              secVerKey,
+              secVerValue,
+              secVerComment,
+              secVerSkipMultilineEncoding
+            }) =>
               secretVersion
                 ? {
                     version: secVerVersion,
                     id: secretVersion,
                     key: secVerKey,
                     encryptedValue: secVerValue,
-                    encryptedComment: secVerComment
+                    encryptedComment: secVerComment,
+                    skipMultilineEncoding: secVerSkipMultilineEncoding
                   }
                 : undefined,
             childrenMapper: [

backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts

@@ -337,12 +337,17 @@ export const secretApprovalRequestServiceFactory = ({
           ? INFISICAL_SECRET_VALUE_HIDDEN_MASK
           : el.secret && el.secret.isRotatedSecret
             ? undefined
-            : el.encryptedValue
+            : el.encryptedValue !== undefined && el.encryptedValue !== null
               ? secretManagerDecryptor({ cipherTextBlob: el.encryptedValue }).toString()
-              : "",
-        secretComment: el.encryptedComment
-          ? secretManagerDecryptor({ cipherTextBlob: el.encryptedComment }).toString()
-          : "",
+              : undefined,
+        secretComment:
+          el.encryptedComment !== undefined && el.encryptedComment !== null
+            ? secretManagerDecryptor({ cipherTextBlob: el.encryptedComment }).toString()
+            : undefined,
+        skipMultilineEncoding:
+          el.skipMultilineEncoding !== undefined && el.skipMultilineEncoding !== null
+            ? el.skipMultilineEncoding
+            : undefined,
         secret: el.secret
           ? {
               secretKey: el.secret.key,
@@ -394,7 +399,8 @@ export const secretApprovalRequestServiceFactory = ({
                 ? secretManagerDecryptor({ cipherTextBlob: el.secretVersion.encryptedComment }).toString()
                 : "",
               tags: el.secretVersion.tags,
-              secretMetadata: el.oldSecretMetadata as ResourceMetadataDTO
+              secretMetadata: el.oldSecretMetadata as ResourceMetadataDTO,
+              skipMultilineEncoding: el.secretVersion.skipMultilineEncoding
             }
           : undefined
       }));
@@ -733,9 +739,9 @@ export const secretApprovalRequestServiceFactory = ({
               tx,
               inputSecrets: secretUpdationCommits.map((el) => {
                 const encryptedValue =
-                  !el.secret?.isRotatedSecret && typeof el.encryptedValue !== "undefined"
+                  !el.secret?.isRotatedSecret && el.encryptedValue !== null && el.encryptedValue !== undefined
                     ? {
-                        encryptedValue: el.encryptedValue as Buffer,
+                        encryptedValue: el.encryptedValue,
                         references: el.encryptedValue
                           ? getAllSecretReferencesV2Bridge(
                               secretManagerDecryptor({
@@ -749,9 +755,9 @@ export const secretApprovalRequestServiceFactory = ({
                   filter: { id: el.secretId as string, type: SecretType.Shared },
                   data: {
                     reminderRepeatDays: el.reminderRepeatDays,
-                    encryptedComment: el.encryptedComment,
+                    encryptedComment: el.encryptedComment !== null ? el.encryptedComment : undefined,
                     reminderNote: el.reminderNote,
-                    skipMultilineEncoding: el.skipMultilineEncoding,
+                    skipMultilineEncoding: el.skipMultilineEncoding !== null ? el.skipMultilineEncoding : undefined,
                     key: el.key,
                     tags: el?.tags.map(({ id }) => id),
                     secretMetadata: el.secretMetadata as ResourceMetadataDTO,
@@ -1633,11 +1639,13 @@ export const secretApprovalRequestServiceFactory = ({
               key: newSecretName || secretKey,
               encryptedComment: setKnexStringValue(
                 secretComment,
-                (value) => secretManagerEncryptor({ plainText: Buffer.from(value) }).cipherTextBlob
+                (value) => secretManagerEncryptor({ plainText: Buffer.from(value) }).cipherTextBlob,
+                true // scott: we need to encrypt empty string on update to differentiate not updating comment vs clearing comment
               ),
               encryptedValue: setKnexStringValue(
                 secretValue,
-                (value) => secretManagerEncryptor({ plainText: Buffer.from(value) }).cipherTextBlob
+                (value) => secretManagerEncryptor({ plainText: Buffer.from(value) }).cipherTextBlob,
+                true // scott: we need to encrypt empty string on update to differentiate not updating value vs clearing value
               ),
               reminderRepeatDays,
               reminderNote,

backend/src/lib/knex/select.ts

@@ -16,8 +16,12 @@ export const stripUndefinedInWhere = <T extends object>(val: T): Exclude<T, unde
 // if its undefined its skipped in knex
 // if its empty string its set as null
 // else pass to the required one
-export const setKnexStringValue = <T>(value: string | null | undefined, cb: (arg: string) => T) => {
+export const setKnexStringValue = <T>(
+  value: string | null | undefined,
+  cb: (arg: string) => T,
+  allowEmptyString?: boolean
+) => {
   if (typeof value === "undefined") return;
-  if (value === "" || value === null) return null;
+  if ((value === "" && !allowEmptyString) || value === null) return null;
   return cb(value);
 };