fix: scope VPC security groups to minimum required ports (#960)

.github/workflows/deploy.yml

@@ -50,10 +50,30 @@ jobs:
 
           echo "✅ All required secrets are configured"
 
+  security-scan:
+    name: Checkov Security Scan
+    runs-on: ubuntu-latest
+    needs: validate-secrets
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Run Checkov
+        uses: bridgecrewio/checkov-action@v12
+        with:
+          directory: infrastructure/terraform
+          framework: terraform
+          # Fail on HIGH and CRITICAL severity findings only
+          soft_fail_on: LOW,MEDIUM
+          output_format: cli
+          # Skip checks that are intentional/acceptable in this architecture:
+          # CKV_AWS_91  - ALB access logs (cost tradeoff, tracked separately)
+          # CKV2_AWS_28 - WAF on ALB (tracked separately)
+          skip_check: CKV_AWS_91,CKV2_AWS_28
+
   plan:
     name: Terraform Plan
     runs-on: ubuntu-latest
-    needs: validate-secrets
+    needs: [validate-secrets, security-scan]
     strategy:
       matrix:
         environment: [dev, staging, prod]

infrastructure/terraform/main.tf

@@ -25,6 +25,53 @@ provider "aws" {
   }
 }
 
+# The ECS tasks SG is created at root level to break the circular dependency
+# between the ecs module (which needs the ALB SG to wire the ingress rule) and
+# rds/redis modules (which need this SG ID for their ingress rules).
+# Egress rules are added separately via aws_security_group_rule so that they
+# can reference the rds/redis module outputs without creating a cycle.
+resource "aws_security_group" "ecs_tasks" {
+  name   = "predictiq-${var.environment}-ecs-tasks-sg"
+  vpc_id = module.vpc.vpc_id
+
+  tags = {
+    Name        = "predictiq-${var.environment}-ecs-tasks-sg"
+    Project     = "predictiq"
+    Environment = var.environment
+    ManagedBy   = "terraform"
+  }
+}
+
+# Outbound to RDS (PostgreSQL)
+resource "aws_security_group_rule" "ecs_tasks_egress_rds" {
+  type                     = "egress"
+  from_port                = 5432
+  to_port                  = 5432
+  protocol                 = "tcp"
+  security_group_id        = aws_security_group.ecs_tasks.id
+  source_security_group_id = module.rds.sg_id
+}
+
+# Outbound to Redis
+resource "aws_security_group_rule" "ecs_tasks_egress_redis" {
+  type                     = "egress"
+  from_port                = 6379
+  to_port                  = 6379
+  protocol                 = "tcp"
+  security_group_id        = aws_security_group.ecs_tasks.id
+  source_security_group_id = module.redis.sg_id
+}
+
+# Outbound HTTPS for AWS API calls (Secrets Manager, ECR, CloudWatch)
+resource "aws_security_group_rule" "ecs_tasks_egress_https" {
+  type              = "egress"
+  from_port         = 443
+  to_port           = 443
+  protocol          = "tcp"
+  security_group_id = aws_security_group.ecs_tasks.id
+  cidr_blocks       = ["0.0.0.0/0"]
+}
+
 module "vpc" {
   source = "./modules/vpc"
 
@@ -44,6 +91,7 @@ module "rds" {
   db_instance_class    = var.db_instance_class
   allocated_storage    = var.allocated_storage
   backup_retention     = var.backup_retention_days
+  ecs_tasks_sg_id      = aws_security_group.ecs_tasks.id
 }
 
 module "redis" {
@@ -55,6 +103,7 @@ module "redis" {
   node_type          = var.redis_node_type
   num_cache_nodes    = var.redis_num_nodes
   engine_version     = var.redis_engine_version
+  ecs_tasks_sg_id    = aws_security_group.ecs_tasks.id
 }
 
 module "ecs" {
@@ -71,6 +120,7 @@ module "ecs" {
   api_memory            = var.api_memory
   database_url          = module.rds.database_url
   redis_url             = module.redis.redis_url
+  ecs_tasks_sg_id       = aws_security_group.ecs_tasks.id
 }
 
 module "monitoring" {

infrastructure/terraform/modules/ecs/main.tf

@@ -44,6 +44,11 @@ variable "redis_url" {
   sensitive = true
 }
 
+variable "ecs_tasks_sg_id" {
+  type        = string
+  description = "Security group ID of the ECS tasks (managed at root level)"
+}
+
 locals {
   common_tags = {
     Project   = "predictiq"
@@ -141,6 +146,7 @@ resource "aws_security_group" "alb" {
   name   = "predictiq-${var.environment}-alb-sg"
   vpc_id = var.vpc_id
 
+  # Allow inbound HTTP/HTTPS from the public internet
   ingress {
     from_port   = 80
     to_port     = 80
@@ -155,11 +161,12 @@ resource "aws_security_group" "alb" {
     cidr_blocks = ["0.0.0.0/0"]
   }
 
+  # Restrict egress to the container port on ECS tasks only
   egress {
-    from_port   = 0
-    to_port     = 0
-    protocol    = "-1"
-    cidr_blocks = ["0.0.0.0/0"]
+    from_port       = var.api_container_port
+    to_port         = var.api_container_port
+    protocol        = "tcp"
+    security_groups = [var.ecs_tasks_sg_id]
   }
 
   tags = merge(
@@ -170,6 +177,17 @@ resource "aws_security_group" "alb" {
   )
 }
 
+# Allow inbound from the ALB on the container port — added as a rule on the
+# externally-managed ecs_tasks SG to avoid a circular module dependency.
+resource "aws_security_group_rule" "ecs_tasks_ingress_alb" {
+  type                     = "ingress"
+  from_port                = var.api_container_port
+  to_port                  = var.api_container_port
+  protocol                 = "tcp"
+  security_group_id        = var.ecs_tasks_sg_id
+  source_security_group_id = aws_security_group.alb.id
+}
+
 resource "aws_lb" "main" {
   name               = "predictiq-${var.environment}-alb"
   internal           = false
@@ -220,32 +238,6 @@ resource "aws_lb_listener" "api" {
   }
 }
 
-resource "aws_security_group" "ecs_tasks" {
-  name   = "predictiq-${var.environment}-ecs-tasks-sg"
-  vpc_id = var.vpc_id
-
-  ingress {
-    from_port       = var.api_container_port
-    to_port         = var.api_container_port
-    protocol        = "tcp"
-    security_groups = [aws_security_group.alb.id]
-  }
-
-  egress {
-    from_port   = 0
-    to_port     = 0
-    protocol    = "-1"
-    cidr_blocks = ["0.0.0.0/0"]
-  }
-
-  tags = merge(
-    local.common_tags,
-    {
-      Name = "predictiq-${var.environment}-ecs-tasks-sg"
-    }
-  )
-}
-
 resource "aws_ecs_service" "api" {
   name            = "predictiq-${var.environment}-api"
   cluster         = aws_ecs_cluster.main.id
@@ -255,7 +247,7 @@ resource "aws_ecs_service" "api" {
 
   network_configuration {
     subnets          = var.private_subnet_ids
-    security_groups  = [aws_security_group.ecs_tasks.id]
+    security_groups  = [var.ecs_tasks_sg_id]
     assign_public_ip = false
   }
 

infrastructure/terraform/modules/rds/main.tf

@@ -36,6 +36,11 @@ variable "backup_retention" {
   type = number
 }
 
+variable "ecs_tasks_sg_id" {
+  type        = string
+  description = "Security group ID of the ECS tasks that are allowed to connect"
+}
+
 locals {
   common_tags = {
     Project   = "predictiq"
@@ -61,18 +66,12 @@ resource "aws_security_group" "rds" {
   name   = "predictiq-${var.environment}-rds-sg"
   vpc_id = var.vpc_id
 
+  # Inbound PostgreSQL from ECS tasks only
   ingress {
-    from_port   = 5432
-    to_port     = 5432
-    protocol    = "tcp"
-    cidr_blocks = ["10.0.0.0/8"]
-  }
-
-  egress {
-    from_port   = 0
-    to_port     = 0
-    protocol    = "-1"
-    cidr_blocks = ["0.0.0.0/0"]
+    from_port       = 5432
+    to_port         = 5432
+    protocol        = "tcp"
+    security_groups = [var.ecs_tasks_sg_id]
   }
 
   tags = merge(
@@ -113,6 +112,10 @@ resource "aws_db_instance" "main" {
   )
 }
 
+output "sg_id" {
+  value = aws_security_group.rds.id
+}
+
 output "endpoint" {
   value     = aws_db_instance.main.endpoint
   sensitive = true

infrastructure/terraform/modules/redis/main.tf

@@ -22,6 +22,11 @@ variable "engine_version" {
   type = string
 }
 
+variable "ecs_tasks_sg_id" {
+  type        = string
+  description = "Security group ID of the ECS tasks that are allowed to connect"
+}
+
 locals {
   common_tags = {
     Project   = "predictiq"
@@ -47,18 +52,12 @@ resource "aws_security_group" "redis" {
   name   = "predictiq-${var.environment}-redis-sg"
   vpc_id = var.vpc_id
 
+  # Inbound Redis from ECS tasks only
   ingress {
-    from_port   = 6379
-    to_port     = 6379
-    protocol    = "tcp"
-    cidr_blocks = ["10.0.0.0/8"]
-  }
-
-  egress {
-    from_port   = 0
-    to_port     = 0
-    protocol    = "-1"
-    cidr_blocks = ["0.0.0.0/0"]
+    from_port       = 6379
+    to_port         = 6379
+    protocol        = "tcp"
+    security_groups = [var.ecs_tasks_sg_id]
   }
 
   tags = merge(
@@ -95,6 +94,10 @@ resource "aws_elasticache_cluster" "main" {
   )
 }
 
+output "sg_id" {
+  value = aws_security_group.redis.id
+}
+
 output "endpoint" {
   value = aws_elasticache_cluster.main.cache_nodes[0].address
 }