Revise PR based on coderabbit, greptile review

backend/src/db/migrations/20250428173025_ssh-host-groups.ts

@@ -3,9 +3,6 @@ import { Knex } from "knex";
 import { TableName } from "../schemas";
 import { createOnUpdateTrigger, dropOnUpdateTrigger } from "../utils";
 
-// TODO: can attach default SSH login mappings to a host group
-// TODO: can attach default user SSH CA and host SSH CA (convert existing project level ones to a group)
-
 export async function up(knex: Knex): Promise<void> {
   if (!(await knex.schema.hasTable(TableName.SshHostGroup))) {
     await knex.schema.createTable(TableName.SshHostGroup, (t) => {
@@ -36,6 +33,7 @@ export async function up(knex: Knex): Promise<void> {
   if (!hasGroupColumn) {
     await knex.schema.alterTable(TableName.SshHostLoginUser, (t) => {
       t.uuid("sshHostGroupId").nullable();
+      t.foreign("sshHostGroupId").references("id").inTable(TableName.SshHostGroup).onDelete("CASCADE");
       t.uuid("sshHostId").nullable().alter();
     });
   }

backend/src/db/schemas/projects.ts

@@ -27,7 +27,7 @@ export const ProjectsSchema = z.object({
   description: z.string().nullable().optional(),
   type: z.string(),
   enforceCapitalization: z.boolean().default(false),
-  hasDeleteProtection: z.boolean().default(false).nullable().optional()
+  hasDeleteProtection: z.boolean().default(true).nullable().optional()
 });
 
 export type TProjects = z.infer<typeof ProjectsSchema>;

backend/src/ee/routes/v1/ssh-host-group-router.ts

@@ -63,7 +63,7 @@ export const registerSshHostGroupRouter = async (server: FastifyZodProvider) =>
       description: "Create SSH Host Group",
       body: z.object({
         projectId: z.string().describe(SSH_HOST_GROUPS.CREATE.projectId),
-        name: slugSchema({ min: 0, max: 64, field: "name" }).describe(SSH_HOST_GROUPS.CREATE.name),
+        name: slugSchema({ min: 1, max: 64, field: "name" }).describe(SSH_HOST_GROUPS.CREATE.name),
         loginMappings: z.array(loginMappingSchema).default([]).describe(SSH_HOST_GROUPS.CREATE.loginMappings)
       }),
       response: {
@@ -107,12 +107,12 @@ export const registerSshHostGroupRouter = async (server: FastifyZodProvider) =>
     },
     onRequest: verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN]),
     schema: {
-      description: "Update SSH Host",
+      description: "Update SSH Host Group",
       params: z.object({
         sshHostGroupId: z.string().trim().describe(SSH_HOST_GROUPS.UPDATE.sshHostGroupId)
       }),
       body: z.object({
-        name: slugSchema({ min: 0, max: 64, field: "name" }).describe(SSH_HOST_GROUPS.UPDATE.name).optional(),
+        name: slugSchema({ min: 1, max: 64, field: "name" }).describe(SSH_HOST_GROUPS.UPDATE.name).optional(),
         loginMappings: z.array(loginMappingSchema).optional().describe(SSH_HOST_GROUPS.UPDATE.loginMappings)
       }),
       response: {
@@ -239,7 +239,8 @@ export const registerSshHostGroupRouter = async (server: FastifyZodProvider) =>
         event: {
           type: EventType.GET_SSH_HOST_GROUP_HOSTS,
           metadata: {
-            sshHostGroupId: req.params.sshHostGroupId
+            sshHostGroupId: req.params.sshHostGroupId,
+            name: sshHostGroup.name
           }
         }
       });

backend/src/ee/services/audit-log/audit-log-types.ts

@@ -1603,6 +1603,7 @@ interface GetSshHostGroupHostsEvent {
   type: EventType.GET_SSH_HOST_GROUP_HOSTS;
   metadata: {
     sshHostGroupId: string;
+    name: string;
   };
 }
 

backend/src/ee/services/license/__mocks__/license-fns.ts

@@ -22,7 +22,7 @@ export const getDefaultOnPremFeatures = () => {
     samlSSO: false,
     scim: false,
     ldap: false,
-    groups: true,
+    groups: false,
     status: null,
     trial_end: null,
     has_used_trial: true,

backend/src/ee/services/license/license-fns.ts

@@ -35,7 +35,7 @@ export const getDefaultOnPremFeatures = (): TFeatureSet => ({
   oidcSSO: false,
   scim: false,
   ldap: false,
-  groups: true,
+  groups: false,
   status: null,
   trial_end: null,
   has_used_trial: true,

backend/src/ee/services/license/license-types.ts

@@ -52,7 +52,7 @@ export type TFeatureSet = {
   secretAccessInsights: false;
   scim: false;
   ldap: false;
-  groups: true;
+  groups: false;
   status: null;
   trial_end: null;
   has_used_trial: true;

backend/src/ee/services/ssh-host-group/ssh-host-group-dal.ts

@@ -2,7 +2,7 @@ import { Knex } from "knex";
 
 import { TDbClient } from "@app/db";
 import { TableName } from "@app/db/schemas";
-import { DatabaseError } from "@app/lib/errors";
+import { BadRequestError, DatabaseError } from "@app/lib/errors";
 import { groupBy, unique } from "@app/lib/fn";
 import { ormify } from "@app/lib/knex";
 
@@ -155,7 +155,9 @@ export const sshHostGroupDALFactory = (db: TDbClient) => {
         .first();
 
       if (!sshHostGroup) {
-        throw new Error(`SSH host group with ID ${sshHostGroupId} not found`);
+        throw new BadRequestError({
+          message: `SSH host group with ID ${sshHostGroupId} not found`
+        });
       }
 
       const query = db

backend/src/ee/services/ssh-host-group/ssh-host-group-service.ts

@@ -89,18 +89,24 @@ export const sshHostGroupServiceFactory = ({
     const newSshHostGroup = await sshHostGroupDAL.transaction(async (tx) => {
       // (dangtony98): room to optimize check to ensure that
       // the SSH host group name is unique across the whole org
-      const project = await projectDAL.findById(projectId);
+      const project = await projectDAL.findById(projectId, tx);
       if (!project) throw new NotFoundError({ message: `Project with ID '${projectId}' not found` });
-      const projects = await projectDAL.find({
+      const projects = await projectDAL.find(
-        orgId: project.orgId
+        {
-      });
+          orgId: project.orgId
+        },
+        { tx }
+      );
 
-      const existingSshHostGroup = await sshHostGroupDAL.find({
+      const existingSshHostGroup = await sshHostGroupDAL.find(
-        name,
+        {
-        $in: {
+          name,
-          projectId: projects.map((p) => p.id)
+          $in: {
-        }
+            projectId: projects.map((p) => p.id)
-      });
+          }
+        },
+        { tx }
+      );
 
       if (existingSshHostGroup.length) {
         throw new BadRequestError({
@@ -307,7 +313,7 @@ export const sshHostGroupServiceFactory = ({
     }
 
     if (sshHostGroup.projectId !== sshHost.projectId) {
-      throw new NotFoundError({
+      throw new BadRequestError({
         message: `SSH host with ID ${hostId} not found in project ${sshHostGroup.projectId}`
       });
     }
@@ -347,7 +353,7 @@ export const sshHostGroupServiceFactory = ({
     }
 
     if (sshHostGroup.projectId !== sshHost.projectId) {
-      throw new NotFoundError({
+      throw new BadRequestError({
         message: `SSH host with ID ${hostId} not found in project ${sshHostGroup.projectId}`
       });
     }

backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts

@@ -7,6 +7,7 @@ import { ProjectType, SecretsV2Schema, SecretType, TableName, TSecretsV2, TSecre
 import { TKeyStoreFactory } from "@app/keystore/keystore";
 import { getConfig } from "@app/lib/config/env";
 import { generateCacheKeyFromData } from "@app/lib/crypto/cache";
+import { applyJitter } from "@app/lib/dates";
 import { BadRequestError, DatabaseError, NotFoundError } from "@app/lib/errors";
 import {
   buildFindFilter,
@@ -22,7 +23,6 @@ import type {
   TFindSecretsByFolderIdsFilter,
   TGetSecretsDTO
 } from "@app/services/secret-v2-bridge/secret-v2-bridge-types";
-import { applyJitter } from "@app/lib/dates";
 
 export const SecretServiceCacheKeys = {
   get productKey() {

docs/documentation/platform/ssh/host-groups.mdx

@@ -9,7 +9,7 @@ description: "Learn how to organize SSH hosts into groups and manage access poli
 Infisical SSH lets you configure host groups to organize and manage multiple SSH hosts with shared access configuration.
 These host groups can be created based on environments (`development`, `staging`, `production`), geographical regions (`us-east`, `eu-west`, `ap-northeast`), or functions (`web-servers`, `database-servers`, `worker-nodes`) to streamline access management across your infrastructure.
 
-Using a host group, you can define login mappings at the group level and have them be applied to all hosts assigned to that group. For example, you can specify that `john@acme.com` can login as `ubuntu` on all hosts assigned to the `production` host group.
+Using a host group, you can define login mappings at the group level and have them be applied to all hosts assigned to that group. For example, you can specify that `john@example.com` can login as `ubuntu` on all hosts assigned to the `production` host group.
 
 ## Workflow
 

frontend/src/hooks/api/sshHost/types.ts

@@ -3,6 +3,14 @@ export enum LoginMappingSource {
   HOST_GROUP = "hostGroup"
 }
 
+export type TLoginMapping = {
+  loginUser: string;
+  allowedPrincipals: {
+    usernames: string[];
+  };
+  source: LoginMappingSource;
+};
+
 export type TSshHost = {
   id: string;
   projectId: string;
@@ -10,13 +18,7 @@ export type TSshHost = {
   alias: string | null;
   userCertTtl: string;
   hostCertTtl: string;
-  loginMappings: {
+  loginMappings: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-    source: LoginMappingSource;
-  }[];
 };
 
 export type TCreateSshHostDTO = {
@@ -25,12 +27,7 @@ export type TCreateSshHostDTO = {
   alias?: string;
   userCertTtl?: string;
   hostCertTtl?: string;
-  loginMappings: {
+  loginMappings: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-  }[];
 };
 
 export type TUpdateSshHostDTO = {
@@ -39,12 +36,7 @@ export type TUpdateSshHostDTO = {
   alias?: string;
   userCertTtl?: string;
   hostCertTtl?: string;
-  loginMappings?: {
+  loginMappings?: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-  }[];
 };
 
 export type TDeleteSshHostDTO = {

frontend/src/hooks/api/sshHostGroup/types.ts

@@ -1,37 +1,22 @@
-import { TSshHost } from "../sshHost/types";
+import { TLoginMapping, TSshHost } from "../sshHost/types";
 
 export type TSshHostGroup = {
   id: string;
   projectId: string;
   name: string;
-  loginMappings: {
+  loginMappings: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-  }[];
 };
 
 export type TCreateSshHostGroupDTO = {
   projectId: string;
   name: string;
-  loginMappings: {
+  loginMappings: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-  }[];
 };
 
 export type TUpdateSshHostGroupDTO = {
   sshHostGroupId: string;
   name?: string;
-  loginMappings?: {
+  loginMappings?: TLoginMapping[];
-    loginUser: string;
-    allowedPrincipals: {
-      usernames: string[];
-    };
-  }[];
 };
 
 export type TDeleteSshHostGroupDTO = {
@@ -39,7 +24,7 @@ export type TDeleteSshHostGroupDTO = {
 };
 
 export type TListSshHostGroupHostsResponse = {
-  hosts: TSshHost & { joinedGroupAt: string; isPartOfGroup: boolean }[];
+  hosts: (TSshHost & { joinedGroupAt: string; isPartOfGroup: boolean })[];
   totalCount: number;
 };
 

frontend/src/layouts/ProjectLayout/ProjectLayout.tsx

@@ -13,9 +13,9 @@ import {
   TBreadcrumbFormat
 } from "@app/components/v2";
 import {
-  useProjectPermission,
   ProjectPermissionActions,
   ProjectPermissionSub,
+  useProjectPermission,
   useSubscription,
   useWorkspace
 } from "@app/context";

frontend/src/pages/ssh/SshHostGroupDetailsByIDPage/components/AddHostGroupMemberModal.tsx

@@ -38,7 +38,8 @@ export const AddHostGroupMemberModal = ({ popUp, handlePopUpToggle }: Props) =>
     sshHostGroupId: popUpData?.sshHostGroupId,
     filter: EHostGroupMembershipFilter.NON_GROUP_MEMBERS
   });
-  const { mutateAsync: addHostToSshHostGroup } = useAddHostToSshHostGroup();
+  const { mutateAsync: addHostToSshHostGroup, isPending: isAddingHostToSshHostGroup } =
+    useAddHostToSshHostGroup();
 
   const handleAddHost = async (sshHostId: string) => {
     try {
@@ -99,11 +100,11 @@ export const AddHostGroupMemberModal = ({ popUp, handlePopUpToggle }: Props) =>
                         >
                           {(isAllowed) => (
                             <Button
-                              isLoading={isPending}
+                              isLoading={isAddingHostToSshHostGroup}
                               isDisabled={!isAllowed}
                               colorSchema="primary"
                               variant="outline_bg"
-                              type="submit"
+                              type="button"
                               onClick={() => handleAddHost(host.id)}
                             >
                               Add

frontend/src/pages/ssh/SshHostGroupDetailsByIDPage/components/SshHostGroupHostsTable.tsx

@@ -47,7 +47,7 @@ export const SshHostGroupHostsTable = ({ sshHostGroupId, handlePopUpOpen }: Prop
             </Tr>
           </THead>
           <TBody>
-            {isPending && <TableSkeleton columns={2} innerKey="ssh-host-group-hosts" />}
+            {isPending && <TableSkeleton columns={4} innerKey="ssh-host-group-hosts" />}
             {!isPending &&
               data?.hosts.map((host) => {
                 return (
@@ -61,10 +61,10 @@ export const SshHostGroupHostsTable = ({ sshHostGroupId, handlePopUpOpen }: Prop
                         a={ProjectPermissionSub.SshHostGroups}
                       >
                         {(isAllowed) => (
-                          <Tooltip content="Remove user from group">
+                          <Tooltip content="Remove host from group">
                             <IconButton
                               isDisabled={!isAllowed}
-                              ariaLabel="Remove user from group"
+                              ariaLabel="Remove host from group"
                               onClick={() =>
                                 handlePopUpOpen("removeHostFromSshHostGroup", {
                                   sshHostId: host.id,

frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupModal.tsx

@@ -34,7 +34,7 @@ type Props = {
 
 const schema = z
   .object({
-    name: z.string().trim(),
+    name: z.string().trim().min(1).max(64),
     loginMappings: z
       .object({
         loginUser: z.string().trim().min(1),

frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsSection.tsx

@@ -56,11 +56,14 @@ export const SshHostGroupsSection = () => {
     <div className="mb-6 rounded-lg border border-mineshaft-600 bg-mineshaft-900 p-4">
       <div className="mb-4 flex justify-between">
         <p className="text-xl font-semibold text-mineshaft-100">Host Groups</p>
-        <ProjectPermissionCan I={ProjectPermissionActions.Create} a={ProjectPermissionSub.SshHosts}>
+        <ProjectPermissionCan
+          I={ProjectPermissionActions.Create}
+          a={ProjectPermissionSub.SshHostGroups}
+        >
           {(isAllowed) => (
             <Button
               colorSchema="primary"
-              type="submit"
+              type="button"
               leftIcon={<FontAwesomeIcon icon={faPlus} />}
               onClick={() => handleAddSshHostGroupModal()}
               isDisabled={!isAllowed}

frontend/src/pages/ssh/SshHostsPage/components/SshHostGroupsTable.tsx

@@ -120,7 +120,7 @@ export const SshHostGroupsTable = ({ handlePopUpOpen }: Props) => {
                           </ProjectPermissionCan>
                           <ProjectPermissionCan
                             I={ProjectPermissionActions.Delete}
-                            a={ProjectPermissionSub.SshHosts}
+                            a={ProjectPermissionSub.SshHostGroups}
                           >
                             {(isAllowed) => (
                               <DropdownMenuItem