fix(cloud-tests): harden aws validation paths

apps/api/src/cloud-security/aws-command-executor.spec.ts

@@ -82,7 +82,7 @@ describe('validatePlanSteps — REQUIRED_PARAMS', () => {
     ).toHaveLength(1);
   });
 
-  it('reports a clear one-of error when security-group ingress revoke commands omit GroupId, GroupName, and rule IDs', () => {
+  it('requires a group identifier for property-based security-group revoke commands', () => {
     const errors = validatePlanSteps([
       step({
         service: 'ec2',
@@ -102,7 +102,33 @@ describe('validatePlanSteps — REQUIRED_PARAMS', () => {
 
     expect(errors).toEqual(
       expect.arrayContaining([
-        'Step 1 (RevokeSecurityGroupIngressCommand): One of "GroupId" or "GroupName" or "SecurityGroupRuleIds" is required',
+        'Step 1 (RevokeSecurityGroupIngressCommand): One of "GroupId" or "GroupName" is required',
+      ]),
+    );
+  });
+
+  it('does not let SecurityGroupRuleIds satisfy a property-based revoke group requirement', () => {
+    const errors = validatePlanSteps([
+      step({
+        service: 'ec2',
+        command: 'RevokeSecurityGroupIngressCommand',
+        params: {
+          SecurityGroupRuleIds: ['sgr-0123abc'],
+          IpPermissions: [
+            {
+              IpProtocol: 'tcp',
+              FromPort: 22,
+              ToPort: 22,
+              IpRanges: [{ CidrIp: '0.0.0.0/0' }],
+            },
+          ],
+        },
+      }),
+    ]);
+
+    expect(errors).toEqual(
+      expect.arrayContaining([
+        'Step 1 (RevokeSecurityGroupIngressCommand): One of "GroupId" or "GroupName" is required',
       ]),
     );
   });
@@ -142,7 +168,7 @@ describe('validatePlanSteps — REQUIRED_PARAMS', () => {
 
     expect(errors).toEqual(
       expect.arrayContaining([
-        'Step 1 (RevokeSecurityGroupIngressCommand): One of "GroupId" or "GroupName" or "SecurityGroupRuleIds" is required',
+        'Step 1 (RevokeSecurityGroupIngressCommand): One of "GroupId" or "GroupName" is required',
       ]),
     );
   });

apps/api/src/cloud-security/aws-command-executor.ts

@@ -169,11 +169,18 @@ export const REQUIRED_PARAMS: Record<string, readonly string[]> = {
 
 const REQUIRED_PARAM_ONE_OF: Record<string, readonly (readonly string[])[]> = {
   AuthorizeSecurityGroupIngressCommand: [['GroupId', 'GroupName']],
-  RevokeSecurityGroupIngressCommand: [
-    ['GroupId', 'GroupName', 'SecurityGroupRuleIds'],
-  ],
 };
 
+const REVOKE_SECURITY_GROUP_INGRESS_RULE_PROPERTY_PARAMS = [
+  'CidrIp',
+  'FromPort',
+  'IpPermissions',
+  'IpProtocol',
+  'SourceSecurityGroupName',
+  'SourceSecurityGroupOwnerId',
+  'ToPort',
+] as const;
+
 function normalizeArnPartition(value: string, partition: AwsPartition): string {
   if (partition === 'aws-us-gov') {
     return value.replace(/\barn:aws:/g, 'arn:aws-us-gov:');
@@ -243,8 +250,9 @@ function normaliseInputParams(
 
   // Rule 2: S3 CreateBucket needs LocationConstraint for non-us-east-1
   if (command === 'CreateBucketCommand') {
-    if (input.Bucket) {
-      input.Bucket = String(input.Bucket).toLowerCase().replace(/_/g, '-');
+    const bucket = input.Bucket;
+    if (typeof bucket === 'string' || typeof bucket === 'number') {
+      input.Bucket = String(bucket).toLowerCase().replace(/_/g, '-');
     }
     if (region !== 'us-east-1' && !input.CreateBucketConfiguration) {
       input.CreateBucketConfiguration = { LocationConstraint: region };
@@ -561,11 +569,35 @@ export function validatePlanSteps(steps: AwsCommandStep[]): string[] {
         }
       }
     }
+
+    if (step.command === 'RevokeSecurityGroupIngressCommand') {
+      errors.push(...validateRevokeSecurityGroupIngressParams(step, prefix));
+    }
   }
 
   return errors;
 }
 
+function validateRevokeSecurityGroupIngressParams(
+  step: AwsCommandStep,
+  prefix: string,
+): string[] {
+  const params = step.params ?? {};
+  const hasGroupIdentifier =
+    hasRequiredParamValue(params.GroupId) ||
+    hasRequiredParamValue(params.GroupName);
+  const hasRuleIds = hasRequiredParamValue(params.SecurityGroupRuleIds);
+  const hasRuleProperties =
+    REVOKE_SECURITY_GROUP_INGRESS_RULE_PROPERTY_PARAMS.some((key) =>
+      hasRequiredParamValue(params[key]),
+    );
+
+  if (hasRuleIds && !hasRuleProperties) return [];
+  if (hasGroupIdentifier) return [];
+
+  return [`${prefix}: One of "GroupId" or "GroupName" is required`];
+}
+
 function hasRequiredParamValue(value: unknown): boolean {
   if (value === undefined || value === null || value === '') return false;
   if (Array.isArray(value)) return value.length > 0;

apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts

@@ -67,67 +67,71 @@ function makeNoSuchEntityError(): Error {
   return error;
 }
 
+function makeAccessDeniedError(): Error {
+  const error = new Error('Not authorized to call iam:GetLoginProfile');
+  error.name = 'AccessDeniedException';
+  return error;
+}
+
 describe('IamAdapter', () => {
   afterEach(() => {
     jest.restoreAllMocks();
   });
 
   it('does not raise MFA findings for IAM users without console access', async () => {
     const mfaCheckedUsers: string[] = [];
-    jest
-      .spyOn(IAMClient.prototype, 'send')
-      .mockImplementation(async (command) => {
-        if (command instanceof GetAccountPasswordPolicyCommand) {
-          return {
-            PasswordPolicy: {
-              MinimumPasswordLength: 14,
-              RequireUppercaseCharacters: true,
-              RequireLowercaseCharacters: true,
-              RequireNumbers: true,
-              RequireSymbols: true,
-            },
-          };
-        }
+    jest.spyOn(IAMClient.prototype, 'send').mockImplementation((command) => {
+      if (command instanceof GetAccountPasswordPolicyCommand) {
+        return {
+          PasswordPolicy: {
+            MinimumPasswordLength: 14,
+            RequireUppercaseCharacters: true,
+            RequireLowercaseCharacters: true,
+            RequireNumbers: true,
+            RequireSymbols: true,
+          },
+        };
+      }
 
-        if (command instanceof ListUsersCommand) {
-          return {
-            Users: [
-              {
-                UserName: 'console-user',
-                Arn: 'arn:aws:iam::123456789012:user/console-user',
-              },
-              {
-                UserName: 'api-only-user',
-                Arn: 'arn:aws:iam::123456789012:user/api-only-user',
-              },
-            ],
-          };
-        }
+      if (command instanceof ListUsersCommand) {
+        return {
+          Users: [
+            {
+              UserName: 'console-user',
+              Arn: 'arn:aws:iam::123456789012:user/console-user',
+            },
+            {
+              UserName: 'api-only-user',
+              Arn: 'arn:aws:iam::123456789012:user/api-only-user',
+            },
+          ],
+        };
+      }
 
-        if (command instanceof GetLoginProfileCommand) {
-          if (command.input.UserName === 'api-only-user') {
-            throw makeNoSuchEntityError();
-          }
-          return { LoginProfile: { UserName: command.input.UserName } };
+      if (command instanceof GetLoginProfileCommand) {
+        if (command.input.UserName === 'api-only-user') {
+          throw makeNoSuchEntityError();
         }
+        return { LoginProfile: { UserName: command.input.UserName } };
+      }
 
-        if (command instanceof ListMFADevicesCommand) {
-          if (command.input.UserName)
-            mfaCheckedUsers.push(command.input.UserName);
-          return { MFADevices: [] };
-        }
+      if (command instanceof ListMFADevicesCommand) {
+        if (command.input.UserName)
+          mfaCheckedUsers.push(command.input.UserName);
+        return { MFADevices: [] };
+      }
 
-        if (command instanceof ListAccessKeysCommand) {
-          return { AccessKeyMetadata: [] };
-        }
+      if (command instanceof ListAccessKeysCommand) {
+        return { AccessKeyMetadata: [] };
+      }
 
-        if (command instanceof GenerateCredentialReportCommand) return {};
-        if (command instanceof GetCredentialReportCommand) {
-          return { Content: Buffer.from(CREDENTIAL_REPORT, 'utf-8') };
-        }
+      if (command instanceof GenerateCredentialReportCommand) return {};
+      if (command instanceof GetCredentialReportCommand) {
+        return { Content: Buffer.from(CREDENTIAL_REPORT, 'utf-8') };
+      }
 
-        return {};
-      });
+      return {};
+    });
 
     const findings = await new IamAdapter().scan({
       credentials: {
@@ -146,4 +150,67 @@ describe('IamAdapter', () => {
       'iam-no-mfa-api-only-user',
     );
   });
+
+  it('continues MFA checks when the console-access probe fails unexpectedly', async () => {
+    const mfaCheckedUsers: string[] = [];
+    jest.spyOn(IAMClient.prototype, 'send').mockImplementation((command) => {
+      if (command instanceof GetAccountPasswordPolicyCommand) {
+        return {
+          PasswordPolicy: {
+            MinimumPasswordLength: 14,
+            RequireUppercaseCharacters: true,
+            RequireLowercaseCharacters: true,
+            RequireNumbers: true,
+            RequireSymbols: true,
+          },
+        };
+      }
+
+      if (command instanceof ListUsersCommand) {
+        return {
+          Users: [
+            {
+              UserName: 'console-probe-error-user',
+              Arn: 'arn:aws:iam::123456789012:user/console-probe-error-user',
+            },
+          ],
+        };
+      }
+
+      if (command instanceof GetLoginProfileCommand) {
+        throw makeAccessDeniedError();
+      }
+
+      if (command instanceof ListMFADevicesCommand) {
+        if (command.input.UserName)
+          mfaCheckedUsers.push(command.input.UserName);
+        return { MFADevices: [] };
+      }
+
+      if (command instanceof ListAccessKeysCommand) {
+        return { AccessKeyMetadata: [] };
+      }
+
+      if (command instanceof GenerateCredentialReportCommand) return {};
+      if (command instanceof GetCredentialReportCommand) {
+        return { Content: Buffer.from(CREDENTIAL_REPORT, 'utf-8') };
+      }
+
+      return {};
+    });
+
+    const findings = await new IamAdapter().scan({
+      credentials: {
+        accessKeyId: 'key',
+        secretAccessKey: 'secret',
+      },
+      region: 'us-east-1',
+      accountId: '123456789012',
+    });
+
+    expect(mfaCheckedUsers).toEqual(['console-probe-error-user']);
+    expect(findings.map((finding) => finding.id)).toContain(
+      'iam-no-mfa-console-probe-error-user',
+    );
+  });
 });

apps/api/src/cloud-security/providers/aws/iam.adapter.ts

@@ -192,7 +192,9 @@ export class IamAdapter implements AwsServiceAdapter {
       return true;
     } catch (error) {
       if (isNoSuchEntityError(error)) return false;
-      throw error;
+      // Keep the MFA scan alive when console access cannot be determined.
+      // This preserves the previous behavior instead of suppressing findings.
+      return true;
     }
   }