From c903201dc41bed2757ec47eaf4005f6fe14db883 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 21 May 2026 20:27:18 -0400 Subject: [PATCH 1/2] fix(cloud-tests): harden aws validation paths --- .../aws-command-executor.spec.ts | 32 ++++++++- .../cloud-security/aws-command-executor.ts | 37 +++++++++- .../providers/aws/iam.adapter.spec.ts | 71 +++++++++++++++++++ .../providers/aws/iam.adapter.ts | 4 +- 4 files changed, 137 insertions(+), 7 deletions(-) diff --git a/apps/api/src/cloud-security/aws-command-executor.spec.ts b/apps/api/src/cloud-security/aws-command-executor.spec.ts index dbf0864b2..385891c99 100644 --- a/apps/api/src/cloud-security/aws-command-executor.spec.ts +++ b/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', ]), ); }); diff --git a/apps/api/src/cloud-security/aws-command-executor.ts b/apps/api/src/cloud-security/aws-command-executor.ts index 53fd16f81..1c1166c1a 100644 --- a/apps/api/src/cloud-security/aws-command-executor.ts +++ b/apps/api/src/cloud-security/aws-command-executor.ts @@ -169,11 +169,18 @@ export const REQUIRED_PARAMS: Record = { const REQUIRED_PARAM_ONE_OF: Record = { 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:'); @@ -561,11 +568,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; diff --git a/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts b/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts index b95310316..a8f7fc2fa 100644 --- a/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts +++ b/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts @@ -67,6 +67,12 @@ 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(); @@ -146,4 +152,69 @@ 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(async (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', + ); + }); }); diff --git a/apps/api/src/cloud-security/providers/aws/iam.adapter.ts b/apps/api/src/cloud-security/providers/aws/iam.adapter.ts index 69fc01383..9bc298a29 100644 --- a/apps/api/src/cloud-security/providers/aws/iam.adapter.ts +++ b/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; } } From 5a79bcbd2a7b1eaa35c5bcd95ca2cdc099fdbfee Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 21 May 2026 20:31:55 -0400 Subject: [PATCH 2/2] test(cloud-tests): lint remediation review coverage --- .../cloud-security/aws-command-executor.ts | 5 +- .../providers/aws/iam.adapter.spec.ts | 182 +++++++++--------- 2 files changed, 92 insertions(+), 95 deletions(-) diff --git a/apps/api/src/cloud-security/aws-command-executor.ts b/apps/api/src/cloud-security/aws-command-executor.ts index 1c1166c1a..37ee82cf2 100644 --- a/apps/api/src/cloud-security/aws-command-executor.ts +++ b/apps/api/src/cloud-security/aws-command-executor.ts @@ -250,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 }; diff --git a/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts b/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts index a8f7fc2fa..586ea2135 100644 --- a/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts +++ b/apps/api/src/cloud-security/providers/aws/iam.adapter.spec.ts @@ -80,60 +80,58 @@ describe('IamAdapter', () => { 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: { @@ -155,53 +153,51 @@ describe('IamAdapter', () => { it('continues MFA checks when the console-access probe fails unexpectedly', 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-probe-error-user', + Arn: 'arn:aws:iam::123456789012:user/console-probe-error-user', }, - }; - } - - 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 {}; - }); + ], + }; + } + + 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: {