PIT: add commits to snapshots and improve old role hidding

2026-09-22 13:39:35 +00:00 · 2025-05-23 01:46:13 -03:00
parent 2b21c9d348
commit 4a391c7ac2
9 changed files with 197 additions and 60 deletions
--- a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts
+++ b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts
@@ -8,6 +8,7 @@ import { InternalServerError, NotFoundError } from "@app/lib/errors";
 import { groupBy } from "@app/lib/fn";
 import { logger } from "@app/lib/logger";
 import { ActorType } from "@app/services/auth/auth-type";
+import { TFolderCommitServiceFactory } from "@app/services/folder-commit/folder-commit-service";
 import { TKmsServiceFactory } from "@app/services/kms/kms-service";
 import { KmsDataKey } from "@app/services/kms/kms-types";
 import { TProjectBotServiceFactory } from "@app/services/project-bot/project-bot-service";
@@ -51,8 +52,8 @@ type TSecretSnapshotServiceFactoryDep = {
  snapshotSecretV2BridgeDAL: TSnapshotSecretV2DALFactory;
  snapshotFolderDAL: TSnapshotFolderDALFactory;
  secretVersionDAL: Pick<TSecretVersionDALFactory, "insertMany" | "findLatestVersionByFolderId">;
-  secretVersionV2BridgeDAL: Pick<TSecretVersionV2DALFactory, "insertMany" | "findLatestVersionByFolderId">;
-  folderVersionDAL: Pick<TSecretFolderVersionDALFactory, "findLatestVersionByFolderId" | "insertMany">;
+  secretVersionV2BridgeDAL: Pick<TSecretVersionV2DALFactory, "insertMany" | "findLatestVersionByFolderId" | "findOne">;
+  folderVersionDAL: Pick<TSecretFolderVersionDALFactory, "findLatestVersionByFolderId" | "insertMany" | "findOne">;
  secretDAL: Pick<TSecretDALFactory, "delete" | "insertMany">;
  secretV2BridgeDAL: Pick<TSecretV2BridgeDALFactory, "delete" | "insertMany">;
  secretTagDAL: Pick<TSecretTagDALFactory, "saveTagsToSecret" | "saveTagsToSecretV2">;
@@ -63,6 +64,7 @@ type TSecretSnapshotServiceFactoryDep = {
  licenseService: Pick<TLicenseServiceFactory, "isValidLicense">;
  kmsService: Pick<TKmsServiceFactory, "createCipherPairWithDataKey">;
  projectBotService: Pick<TProjectBotServiceFactory, "getBotKey">;
+  folderCommitService: Pick<TFolderCommitServiceFactory, "createCommit">;
 };

 export type TSecretSnapshotServiceFactory = ReturnType<typeof secretSnapshotServiceFactory>;
@@ -84,7 +86,8 @@ export const secretSnapshotServiceFactory = ({
  snapshotSecretV2BridgeDAL,
  secretVersionV2TagBridgeDAL,
  kmsService,
-  projectBotService
+  projectBotService,
+  folderCommitService
 }: TSecretSnapshotServiceFactoryDep) => {
  const projectSecretSnapshotCount = async ({
    environment,
@@ -403,6 +406,17 @@ export const secretSnapshotServiceFactory = ({
          .filter((el) => el.isRotatedSecret)
          .map((el) => el.secretId);

+        const deletedSecretsChanges = new Map(); // secretId -> version info
+        const deletedFoldersChanges = new Map(); // folderId -> version info
+        const addedSecretsChanges = new Map(); // secretId -> version info
+        const addedFoldersChanges = new Map(); // folderId -> version info
+        const commitChanges: {
+          type: string;
+          secretVersionId?: string;
+          folderVersionId?: string;
+          isUpdate?: boolean;
+        }[] = [];
+
        // this will remove all secrets in current folder except rotated secrets which we ignore
        const deletedTopLevelSecs = await secretV2BridgeDAL.delete(
          {
@@ -424,7 +438,35 @@ export const secretSnapshotServiceFactory = ({
          },
          tx
        );
+
+        await Promise.all(
+          deletedTopLevelSecs.map(async (sec) => {
+            const version = await secretVersionV2BridgeDAL.findOne({ secretId: sec.id, version: sec.version }, tx);
+            deletedSecretsChanges.set(sec.id, {
+              id: sec.id,
+              version: sec.version,
+              // Store the version ID if available from the snapshot
+              versionId: version?.id
+            });
+          })
+        );
+
        const deletedTopLevelSecsGroupById = groupBy(deletedTopLevelSecs, (item) => item.id);
+
+        const deletedFoldersData = await folderDAL.delete({ parentId: snapshot.folderId, isReserved: false }, tx);
+
+        await Promise.all(
+          deletedFoldersData.map(async (folder) => {
+            const version = await folderVersionDAL.findOne({ folderId: folder.id, version: folder.version }, tx);
+            deletedFoldersChanges.set(folder.id, {
+              id: folder.id,
+              version: folder.version,
+              // Store the version ID if available
+              versionId: version?.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
        const deletedFolders = await folderDAL.delete({ parentId: snapshot.folderId, isReserved: false }, tx);
@@ -497,6 +539,12 @@ export const secretSnapshotServiceFactory = ({
          })),
          tx
        );
+
+        // Track added folders
+        folderVersions.forEach((fv) => {
+          addedFoldersChanges.set(fv.folderId, fv);
+        });
+
        const userActorId = actor === ActorType.USER ? actorId : undefined;
        const identityActorId = actor !== ActorType.USER ? actorId : undefined;
        const actorType = actor || ActorType.PLATFORM;
@@ -511,6 +559,11 @@ export const secretSnapshotServiceFactory = ({
          })),
          tx
        );
+
+        secretVersions.forEach((sv) => {
+          addedSecretsChanges.set(sv.secretId, sv);
+        });
+
        await secretVersionV2TagBridgeDAL.insertMany(
          secretVersions.flatMap(({ secretId, id }) =>
            secretVerTagToBeInsert?.[secretId]?.length
@@ -522,6 +575,69 @@ export const secretSnapshotServiceFactory = ({
          ),
          tx
        );
+
+        // Compute commit changes
+        // Handle secrets
+        deletedSecretsChanges.forEach((deletedInfo, secretId) => {
+          const addedSecret = addedSecretsChanges.get(secretId);
+          if (addedSecret) {
+            // Secret was deleted and re-added - this is an update only if versions are different
+            if (deletedInfo.versionId !== addedSecret.id) {
+              commitChanges.push({
+                type: "add", // In the commit system, updates are tracked as "add" with isUpdate=true
+                secretVersionId: addedSecret.id,
+                isUpdate: true
+              });
+            }
+            // Remove from addedSecrets since we've handled it
+            addedSecretsChanges.delete(secretId);
+          } else if (deletedInfo.versionId) {
+            // Secret was only deleted
+            commitChanges.push({
+              type: "delete",
+              secretVersionId: deletedInfo.versionId
+            });
+          }
+        });
+        // Add remaining new secrets (not updates)
+        addedSecretsChanges.forEach((addedSecret) => {
+          commitChanges.push({
+            type: "add",
+            secretVersionId: addedSecret.id
+          });
+        });
+
+        // Handle folders
+        deletedFoldersChanges.forEach((deletedInfo, folderId) => {
+          const addedFolder = addedFoldersChanges.get(folderId);
+          if (addedFolder) {
+            // Folder was deleted and re-added - this is an update only if versions are different
+            if (deletedInfo.versionId !== addedFolder.id) {
+              commitChanges.push({
+                type: "add",
+                folderVersionId: addedFolder.id,
+                isUpdate: true
+              });
+            }
+            // Remove from addedFolders since we've handled it
+            addedFoldersChanges.delete(folderId);
+          } else if (deletedInfo.versionId) {
+            // Folder was only deleted
+            commitChanges.push({
+              type: "delete",
+              folderVersionId: deletedInfo.versionId
+            });
+          }
+        });
+
+        // Add remaining new folders (not updates)
+        addedFoldersChanges.forEach((addedFolder) => {
+          commitChanges.push({
+            type: "add",
+            folderVersionId: addedFolder.id
+          });
+        });
+
        const newSnapshot = await snapshotDAL.create(
          {
            folderId: snapshot.folderId,
@@ -550,6 +666,22 @@ export const secretSnapshotServiceFactory = ({
            })),
          tx
        );
+        if (commitChanges.length > 0) {
+          await folderCommitService.createCommit(
+            {
+              actor: {
+                type: actorType,
+                metadata: {
+                  id: userActorId || identityActorId
+                }
+              },
+              message: "Rollback to snapshot",
+              folderId: snapshot.folderId,
+              changes: commitChanges
+            },
+            tx
+          );
+        }

        return { ...newSnapshot, snapshotSecrets, snapshotFolders };
      });
--- a/backend/src/server/routes/index.ts
+++ b/backend/src/server/routes/index.ts
@@ -1172,6 +1172,7 @@ export const registerRoutes = async (
    snapshotDAL,
    snapshotFolderDAL,
    snapshotSecretDAL,
+    folderCommitService,
    secretVersionDAL,
    folderVersionDAL,
    secretTagDAL,
--- a/backend/src/services/folder-commit/folder-commit-types.ts
+++ b/backend/src/services/folder-commit/folder-commit-types.ts
@@ -1,21 +1,5 @@
 import { z } from "zod";

-const secretVersionSchema = z.object({
-  secretKey: z.string().optional().nullable(),
-  secretComment: z.string().optional().nullable(),
-  skipMultilineEncoding: z.boolean().optional().nullable(),
-  secretReminderRepeatDays: z.number().optional().nullable(),
-  secretReminderNote: z.string().optional().nullable(),
-  metadata: z.unknown().optional().nullable(),
-  tags: z.array(z.string()).optional().nullable(),
-  secretReminderRecipients: z.array(z.any()).optional().nullable(),
-  secretValue: z.string().optional().nullable()
-});
-
-const folderVersionSchema = z.object({
-  name: z.string().optional().nullable()
-});
-
 const baseChangeSchema = z.object({
  id: z.string(),
  folderCommitId: z.string(),
@@ -46,7 +30,22 @@ const commitChangeSchema = baseChangeSchema.extend({
  secretVersion: z.union([z.string(), z.number()]).optional().nullable(),
  secretId: z.string().optional().nullable(),
  folderVersion: z.union([z.string(), z.number()]).optional().nullable(),
-  versions: z.array(z.union([secretVersionSchema, folderVersionSchema])).optional()
+  versions: z
+    .array(
+      z.object({
+        secretKey: z.string().optional().nullable(),
+        secretComment: z.string().optional().nullable(),
+        skipMultilineEncoding: z.boolean().optional().nullable(),
+        secretReminderRepeatDays: z.number().optional().nullable(),
+        secretReminderNote: z.string().optional().nullable(),
+        metadata: z.unknown().optional().nullable(),
+        tags: z.array(z.string()).optional().nullable(),
+        secretReminderRecipients: z.array(z.any()).optional().nullable(),
+        secretValue: z.string().optional().nullable(),
+        name: z.string().optional().nullable()
+      })
+    )
+    .optional()
 });

 const commitSchema = z.object({
--- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/PolicySelectionModal.tsx
+++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/PolicySelectionModal.tsx
@@ -23,6 +23,7 @@ import { useGetProjectTypeFromRoute } from "@app/hooks";
 import { ProjectType } from "@app/hooks/api/workspace/types";

 import {
+  EXCLUDED_PERMISSION_SUBS,
  isConditionalSubjects,
  PROJECT_PERMISSION_OBJECT,
  ProjectTypePermissionSubjects,
@@ -66,6 +67,7 @@ const Content = ({ onClose }: ContentProps) => {
          subject as ProjectPermissionSub
        ] && (search ? title.toLowerCase().includes(search.toLowerCase()) : true)
    )
+    .filter(([subject]) => !EXCLUDED_PERMISSION_SUBS.includes(subject as ProjectPermissionSub))
    .sort((a, b) => a[1].title.localeCompare(b[1].title))
    .map(([subject]) => subject);

--- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx
+++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx
@@ -234,6 +234,7 @@ export const projectRoleFormSchema = z.object({
      })
        .array()
        .default([]),
+      [ProjectPermissionSub.Commits]: CommitPolicyActionSchema.array().default([]),
      [ProjectPermissionSub.Member]: MemberPolicyActionSchema.array().default([]),
      [ProjectPermissionSub.Groups]: GroupPolicyActionSchema.array().default([]),
      [ProjectPermissionSub.Role]: GeneralPolicyActionSchema.array().default([]),
@@ -280,8 +281,7 @@ export const projectRoleFormSchema = z.object({
      [ProjectPermissionSub.Kms]: GeneralPolicyActionSchema.array().default([]),
      [ProjectPermissionSub.Cmek]: CmekPolicyActionSchema.array().default([]),
      [ProjectPermissionSub.SecretSyncs]: SecretSyncPolicyActionSchema.array().default([]),
-      [ProjectPermissionSub.Kmip]: KmipPolicyActionSchema.array().default([]),
-      [ProjectPermissionSub.Commits]: CommitPolicyActionSchema.array().default([])
+      [ProjectPermissionSub.Kmip]: KmipPolicyActionSchema.array().default([])
    })
    .partial()
    .optional()
@@ -416,8 +416,7 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => {
        ProjectPermissionSub.SshCertificateTemplates,
        ProjectPermissionSub.SshCertificateAuthorities,
        ProjectPermissionSub.SshCertificates,
-        ProjectPermissionSub.SshHostGroups,
-        ProjectPermissionSub.Commits
+        ProjectPermissionSub.SshHostGroups
      ].includes(subject)
    ) {
      // from above statement we are sure it won't be undefined
@@ -742,6 +741,17 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => {
      });
    }

+    if (subject === ProjectPermissionSub.Commits) {
+      const canRead = action.includes(ProjectPermissionCommitsActions.Read);
+      const canPerformRollback = action.includes(ProjectPermissionCommitsActions.PerformRollback);
+
+      if (!formVal[subject]) formVal[subject] = [{}];
+      if (canRead) formVal[subject]![0][ProjectPermissionCommitsActions.Read] = true;
+      if (canPerformRollback)
+        formVal[subject]![0][ProjectPermissionCommitsActions.PerformRollback] = true;
+      return;
+    }
+
    if (subject === ProjectPermissionSub.PkiSubscribers) {
      if (!formVal[subject]) formVal[subject] = [];

@@ -861,6 +871,8 @@ export const formRolePermission2API = (formVal: TFormSchema["permissions"]) => {
  return permissions;
 };

+export const EXCLUDED_PERMISSION_SUBS = [ProjectPermissionSub.SecretRollback];
+
 export type TProjectPermissionObject = {
  [K in ProjectPermissionSub]: {
    title: string;
@@ -1291,7 +1303,7 @@ const SecretsManagerPermissionSubjects = (enabled = false) => ({
  [ProjectPermissionSub.Tags]: enabled,
  [ProjectPermissionSub.Webhooks]: enabled,
  [ProjectPermissionSub.IpAllowList]: enabled,
-  [ProjectPermissionSub.SecretRollback]: false,
+  [ProjectPermissionSub.SecretRollback]: enabled,
  [ProjectPermissionSub.SecretRotation]: enabled,
  [ProjectPermissionSub.ServiceTokens]: enabled,
  [ProjectPermissionSub.Commits]: enabled
--- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx
+++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx
@@ -24,6 +24,7 @@ import { IdentityManagementPermissionConditions } from "./IdentityManagementPerm
 import { PermissionEmptyState } from "./PermissionEmptyState";
 import { PkiSubscriberPermissionConditions } from "./PkiSubscriberPermissionConditions";
 import {
+  EXCLUDED_PERMISSION_SUBS,
  formRolePermission2API,
  isConditionalSubjects,
  PROJECT_PERMISSION_OBJECT,
@@ -171,6 +172,7 @@ export const RolePermissionsSection = ({ roleSlug, isDisabled }: Props) => {
          <div className="py-4">
            {!isPending && <PermissionEmptyState />}
            {(Object.keys(PROJECT_PERMISSION_OBJECT) as ProjectPermissionSub[])
+              .filter((subject) => !EXCLUDED_PERMISSION_SUBS.includes(subject))
              .filter((subject) => ProjectTypePermissionSubjects[currentWorkspace.type][subject])
              .map((subject) => (
                <GeneralPermissionPolicies
--- a/frontend/src/pages/secret-manager/CommitDetailsPage/components/RollbackPreviewTab/RollbackPreviewTab.tsx
+++ b/frontend/src/pages/secret-manager/CommitDetailsPage/components/RollbackPreviewTab/RollbackPreviewTab.tsx
@@ -328,39 +328,28 @@ export const RollbackPreviewTab = (): JSX.Element => {

            <div className="border-t border-mineshaft-600 py-4">
              <div className="flex w-full items-center justify-between gap-2">
-                <div className="flex-1">
-                  <Input
-                    placeholder="Restore Message"
-                    value={message}
-                    onChange={(e) => setMessage(e.target.value)}
-                    className="w-full border-mineshaft-500 bg-mineshaft-700 text-sm"
-                  />
-                </div>
-                <div className="flex gap-2">
-                  <Button
-                    onClick={() => {
-                      if (!message) {
-                        createNotification({
-                          type: "error",
-                          text: "Please enter a restore message"
-                        });
-                        return;
-                      }
-                      handlePopUpOpen("rollbackConfirm");
-                    }}
-                    colorSchema="primary"
-                    className="px-6 py-2"
-                    isDisabled={
-                      message.length === 0 ||
-                      !rollbackChangesNested ||
-                      !rollbackChangesNested?.some((folder) => {
-                        return folder.changes.length > 0;
-                      })
-                    }
-                  >
-                    Restore
-                  </Button>
-                </div>
+                <Input
+                  placeholder="Restore Message"
+                  value={message}
+                  onChange={(e) => setMessage(e.target.value)}
+                  className="w-full border-mineshaft-500 bg-mineshaft-700 py-2 text-sm"
+                />
+                <Button
+                  onClick={() => {
+                    handlePopUpOpen("rollbackConfirm");
+                  }}
+                  colorSchema="primary"
+                  className="px-6 py-2"
+                  isDisabled={
+                    message.length === 0 ||
+                    !rollbackChangesNested ||
+                    !rollbackChangesNested?.some((folder) => {
+                      return folder.changes.length > 0;
+                    })
+                  }
+                >
+                  Restore
+                </Button>
              </div>
            </div>
          </div>
--- a/frontend/src/pages/secret-manager/CommitDetailsPage/components/SecretVersionDiffView/SecretVersionDiffView.tsx
+++ b/frontend/src/pages/secret-manager/CommitDetailsPage/components/SecretVersionDiffView/SecretVersionDiffView.tsx
@@ -453,7 +453,7 @@ export const SecretVersionDiffView: React.FC<SecretVersionDiffViewProps> = ({
  const differences = getVersionDifferences(item.versions);

  if (differences.length === 0) {
-    return <div className="px-6 py-3 text-gray-400">No details available</div>;
+    return null;
  }

  const changedFields = new Set<string>();
--- a/frontend/src/pages/secret-manager/CommitsPage/components/CommitHistoryTab/CommitHistoryTab.tsx
+++ b/frontend/src/pages/secret-manager/CommitsPage/components/CommitHistoryTab/CommitHistoryTab.tsx
@@ -57,7 +57,7 @@ const CommitItem = ({
              </Button>
            </div>
            <p className="text-white-400 mt-2 flex flex-wrap items-center gap-4 text-sm">
-              <span className="flex items-center">
+              <span className="flex items-center text-mineshaft-300">
                {commit.actorMetadata?.email || commit.actorMetadata?.name || commit.actorType}
                <p className="ml-1 mr-1">committed</p>
                <time dateTime={commit.createdAt}>{formatTimeAgo(commit.createdAt)}</time>