From 76986706c2e6e1979156c9df84bfd9eb696a5b7e Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 23 Apr 2026 18:27:27 -0400 Subject: [PATCH] fix(cloud-security): recognize VPC-scope flow logs correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DescribeFlowLogs does not support a `resource-type` filter — the supported filters per the AWS SDK are resource-id, flow-log-id, log-group-name, log-destination-type, deliver-log-status, traffic-type, and tag. Passing `resource-type` was undefined behavior and caused legitimate VPC-level flow logs to be missed, so customers who had correctly enabled flow logs still saw the "no flow logs enabled" finding. Drop the invalid filter, fetch all flow logs with pagination, and determine VPC-scope client-side by the `vpc-` ResourceId prefix. Subnet-scope and ENI-scope flow logs continue to be excluded because they do not cover all VPC traffic. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../providers/aws/ec2-vpc.adapter.spec.ts | 164 ++++++++++++++++++ .../providers/aws/ec2-vpc.adapter.ts | 31 +++- 2 files changed, 186 insertions(+), 9 deletions(-) create mode 100644 apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.spec.ts diff --git a/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.spec.ts b/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.spec.ts new file mode 100644 index 0000000000..d82018e753 --- /dev/null +++ b/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.spec.ts @@ -0,0 +1,164 @@ +import { + DescribeFlowLogsCommand, + DescribeInstancesCommand, + DescribeVpcsCommand, + GetEbsEncryptionByDefaultCommand, + DescribeSecurityGroupsCommand, +} from '@aws-sdk/client-ec2'; +import { Ec2VpcAdapter } from './ec2-vpc.adapter'; + +type SendHandler = (command: unknown) => unknown; + +function buildClient(handler: SendHandler) { + return { + send: jest.fn((command: unknown) => Promise.resolve(handler(command))), + } as unknown as Parameters[0]['credentials'] extends infer _ + ? import('@aws-sdk/client-ec2').EC2Client + : never; +} + +const noopInstancesResponse = { Reservations: [] }; +const noopSgResponse = { SecurityGroups: [] }; +const encryptedByDefaultResponse = { EbsEncryptionByDefault: true }; + +describe('Ec2VpcAdapter — checkVpcFlowLogs', () => { + const adapter = new Ec2VpcAdapter(); + + // Call the private method through scan() with mocked client wiring. + // scan() constructs its own EC2Client; we override EC2Client by spying on send(). + // To keep this test focused on flow-log logic, we mock the EC2Client module + // and assert behavior via the returned findings. + + function runScanWithFlowLogs(args: { + vpcs: Array<{ VpcId: string; IsDefault?: boolean; Tags?: Array<{ Key: string; Value: string }> }>; + flowLogPages: Array<{ FlowLogs: Array<{ ResourceId: string }>; NextToken?: string }>; + hasRunningInstances?: boolean; + }) { + let flowLogPageIndex = 0; + const handler: SendHandler = (command) => { + if (command instanceof DescribeVpcsCommand) { + return { Vpcs: args.vpcs }; + } + if (command instanceof DescribeFlowLogsCommand) { + const page = args.flowLogPages[flowLogPageIndex] ?? { FlowLogs: [] }; + flowLogPageIndex += 1; + return page; + } + if (command instanceof DescribeInstancesCommand) { + return args.hasRunningInstances + ? { Reservations: [{ Instances: [{ InstanceId: 'i-abc' }] }] } + : noopInstancesResponse; + } + if (command instanceof GetEbsEncryptionByDefaultCommand) { + return encryptedByDefaultResponse; + } + if (command instanceof DescribeSecurityGroupsCommand) { + return noopSgResponse; + } + return {}; + }; + + const client = buildClient(handler); + // Access the private method for focused testing. + const fn = ( + adapter as unknown as { + checkVpcFlowLogs: ( + c: unknown, + region: string, + accountId?: string, + ) => Promise extends Promise ? U : never>; + } + ).checkVpcFlowLogs; + return fn.call(adapter, client, 'us-east-1', '123456789012'); + } + + it('passes a VPC when a VPC-scope flow log exists for it', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [{ VpcId: 'vpc-abc123', IsDefault: false }], + flowLogPages: [{ FlowLogs: [{ ResourceId: 'vpc-abc123' }] }], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('vpc-flow-logs-vpc-abc123'); + expect(findings[0].passed).toBe(true); + }); + + it('fails a VPC that only has subnet-scope flow logs', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [{ VpcId: 'vpc-abc123', IsDefault: false }], + flowLogPages: [ + { + FlowLogs: [ + { ResourceId: 'subnet-111' }, + { ResourceId: 'subnet-222' }, + ], + }, + ], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('vpc-no-flow-logs-vpc-abc123'); + expect(findings[0].passed).toBe(false); + }); + + it('fails a VPC that only has ENI-scope flow logs', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [{ VpcId: 'vpc-abc123', IsDefault: false }], + flowLogPages: [{ FlowLogs: [{ ResourceId: 'eni-deadbeef' }] }], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('vpc-no-flow-logs-vpc-abc123'); + expect(findings[0].passed).toBe(false); + }); + + it('fails a VPC when no flow logs exist at all', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [{ VpcId: 'vpc-abc123', IsDefault: false }], + flowLogPages: [{ FlowLogs: [] }], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('vpc-no-flow-logs-vpc-abc123'); + expect(findings[0].passed).toBe(false); + }); + + it('paginates DescribeFlowLogs and recognizes a VPC-scope flow log on a later page', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [{ VpcId: 'vpc-abc123', IsDefault: false }], + flowLogPages: [ + { + FlowLogs: [{ ResourceId: 'subnet-1' }, { ResourceId: 'eni-1' }], + NextToken: 'page-2', + }, + { FlowLogs: [{ ResourceId: 'vpc-abc123' }] }, + ], + }); + + expect(findings).toHaveLength(1); + expect(findings[0].id).toBe('vpc-flow-logs-vpc-abc123'); + expect(findings[0].passed).toBe(true); + }); + + it('handles multiple VPCs with mixed scopes correctly', async () => { + const findings = await runScanWithFlowLogs({ + vpcs: [ + { VpcId: 'vpc-aaa', IsDefault: false }, + { VpcId: 'vpc-bbb', IsDefault: false }, + ], + flowLogPages: [ + { + FlowLogs: [ + { ResourceId: 'vpc-aaa' }, // vpc-aaa: VPC-scope → pass + { ResourceId: 'subnet-xyz' }, // subnet for vpc-bbb doesn't count + ], + }, + ], + }); + + expect(findings).toHaveLength(2); + const byId = Object.fromEntries(findings.map((f) => [f.resourceId, f])); + expect(byId['vpc-aaa'].passed).toBe(true); + expect(byId['vpc-bbb'].passed).toBe(false); + }); +}); diff --git a/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.ts b/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.ts index 0f536f48dd..335b0892ab 100644 --- a/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.ts +++ b/apps/api/src/cloud-security/providers/aws/ec2-vpc.adapter.ts @@ -190,15 +190,28 @@ export class Ec2VpcAdapter implements AwsServiceAdapter { if (vpcs.length === 0) return findings; - const flowLogsResp = await client.send( - new DescribeFlowLogsCommand({ - Filter: [{ Name: 'resource-type', Values: ['VPC'] }], - }), - ); - - const vpcsWithFlowLogs = new Set( - (flowLogsResp.FlowLogs || []).map((fl) => fl.ResourceId), - ); + // DescribeFlowLogs does not support a `resource-type` filter — the only + // supported filters are documented in DescribeFlowLogsRequest (resource-id, + // flow-log-id, traffic-type, etc.). We fetch all flow logs with pagination + // and filter to VPC-scope by the `vpc-` ResourceId prefix client-side. + // Subnet-scope (subnet-*) and ENI-scope (eni-*) flow logs are ignored + // because they don't cover all VPC traffic. + const vpcsWithFlowLogs = new Set(); + let nextToken: string | undefined; + do { + const flowLogsResp = await client.send( + new DescribeFlowLogsCommand({ + MaxResults: 1000, + NextToken: nextToken, + }), + ); + for (const fl of flowLogsResp.FlowLogs || []) { + if (fl.ResourceId && fl.ResourceId.startsWith('vpc-')) { + vpcsWithFlowLogs.add(fl.ResourceId); + } + } + nextToken = flowLogsResp.NextToken; + } while (nextToken); for (const vpc of vpcs) { if (!vpc.VpcId) continue;