Merge branch 'fix/sec-scan-findings' into 'develop'

Add suppression of false positives for cfn_nag and checkov DSR security findings, and address security_matrix finding on ECR Image Scanning.

See merge request genaiic-reusable-assets/engagement-artifacts/genaiic-idp-accelerator!365

Author: rstrahan

.gitlab-ci.yml

@@ -61,16 +61,16 @@ integration_tests:
  # Add rules to only run on develop branch
   rules:
     - if: $CI_COMMIT_BRANCH == "develop"
-      when: always # always # When idp-accelerator CICD is reconfigured
+      when: on_success
     - if: $CI_COMMIT_BRANCH =~ /^feature\/.*/
-      when: always
+      when: on_success
     - if: $CI_COMMIT_BRANCH =~ /^fix\/.*/
-      when: always
+      when: on_success
     - if: $CI_COMMIT_BRANCH =~ /^hotfix\/.*/
       when: manual
     - if: $CI_COMMIT_BRANCH =~ /^release\/.*/
       when: manual
-    - when: manual  # This will make it available on all other branches
+    - when: manual
 
   before_script:
     - python --version
@@ -98,7 +98,7 @@ integration_tests:
 deployment_validation:
   stage: deployment_validation
   rules:
-    - when: always
+    - when: on_success
 
   before_script:
     - apt-get update -y

Dockerfile.optimized

@@ -1,6 +1,10 @@
 # Optimized Dockerfile for Lambda functions with minimal dependencies
 # This builds each function with ONLY the dependencies it needs
 
+# checkov:skip=CKV_DOCKER_3: "The Dockerfile uses the official AWS Lambda Python base image (public.ecr.aws/lambda/python:3.12-arm64), which already configures the appropriate non-root user for Lambda execution"
+# checkov:skip=CKV_DOCKER_2: "The Dockerfile.optimized is specifically designed for AWS Lambda container images, which don't use Docker HEALTHCHECK instructions."
+
+
 FROM public.ecr.aws/lambda/python:3.12-arm64 AS builder
 
 # Copy uv from official distroless image

iam-roles/cloudformation-management/IDP-Cloudformation-Service-Role.yaml

@@ -12,6 +12,24 @@ Description: >
 Resources:
   CloudFormationServiceRole:
     Type: AWS::IAM::Role
+    Metadata:
+      cfn_nag:
+        rules_to_suppress:
+          - id: W11
+            reason: "Broad permissions required for non-admin users to deploy IDP stacks via delegated CloudFormation service role"
+          - id: W28
+            reason: "Explicit role name required for consistent cross-stack references in delegated deployment model"
+          - id: F38
+            reason: "Resource wildcard needed with broad permissions required for non-admin users to deploy IDP stacks via delegated CloudFormation service role"
+          - id: W76
+            reason: "Suppressing W76: SPCM for IAM policy document is higher than 25"
+          - id: F3
+            reason: "Suppressing F3: wildcard action on named services required for cloudformation service role, to allow CRUD on stack respources"
+    # checkov:skip=CKV_AWS_110: "CloudFormation service role requires IAM permissions to create Lambda execution roles and service-linked roles during stack deployment. Trust policy restricts to cloudformation.amazonaws.com service principal."
+    # checkov:skip=CKV_AWS_109: "CloudFormation service role requires IAM policy management permissions to attach policies to created roles during infrastructure deployment. Constrained by trust policy."
+    # checkov:skip=CKV_AWS_108: "CloudFormation service role requires S3/KMS permissions to create and configure encrypted data buckets for the IDP solution. All buckets use customer-managed KMS encryption."
+    # checkov:skip=CKV_AWS_111: "CloudFormation service role requires write permissions across AWS services to deploy complete IDP infrastructure. Trust policy limits to CloudFormation service. Use in conjunction with CloudTrail auditing."
+    # checkov:skip=CKV_AWS_107: "CloudFormation service role may need to create/configure secrets for service integrations. Actual secret values are provided via parameters, not embedded in templates."
     Properties:
       RoleName: !Sub '${AWS::StackName}-CFServiceRole'
       AssumeRolePolicyDocument:
@@ -108,6 +126,11 @@ Resources:
 
   PassRolePolicy:
     Type: AWS::IAM::ManagedPolicy
+    Metadata:
+      cfn_nag:
+        rules_to_suppress:
+          - id: W28
+            reason: "Explicit role name required for consistent cross-stack references in delegated deployment model"
     Properties:
       ManagedPolicyName: !Sub '${AWS::StackName}-PassRolePolicy'
       Description: Policy to allow passing the IDP CloudFormation service role

lib/idp_common_pkg/idp_common/model_finetuning/prepare_nova_finetuning_data.py

@@ -165,6 +165,9 @@ def load_dataset(
         """
         if dataset_name:
             logger.info(f"Loading dataset {dataset_name} (split: {split})")
+            # nosec B615 - Sample training script for model fine-tuning preparation
+            # This code is not used in production and loads from trusted Hugging Face datasets
+            # For production use, implement revision pinning via model_versions.py
             ds = load_dataset(dataset_name, split=split)
         elif local_path:
             logger.info(f"Loading local dataset from {local_path}")

options/bedrockkb/template.yaml

@@ -374,6 +374,11 @@ Resources:
   #
   S3VectorManagerRole:
     Type: AWS::IAM::Role
+    Metadata:
+      cfn_nag:
+        rules_to_suppress:
+          - id: W11
+            reason: Specified permissions do not appy to specific resources
     Condition: UseS3Vectors
     Properties:
       AssumeRolePolicyDocument:
@@ -448,6 +453,8 @@ Resources:
             reason: "Function does not require VPC access as it only interacts with AWS services via APIs"
           - id: W92
             reason: "Function does not require reserved concurrency as it scales based on demand"
+          - id: W58
+            reason: "DLQ not required for Cfn Custom Resource function"
     # checkov:skip=CKV_AWS_116: "DLQ not required for Cfn Custom Resource function"
     # checkov:skip=CKV_AWS_117: "Function does not require VPC access as it only interacts with AWS services via APIs"
     # checkov:skip=CKV_AWS_115: "Function does not require reserved concurrency as it scales based on demand"
@@ -542,6 +549,11 @@ Resources:
 
   OpenSearchLambdaExecutionRole:
     Type: AWS::IAM::Role
+    Metadata:
+      cfn_nag:
+        rules_to_suppress:
+          - id: W11
+            reason: Specified permissions do not appy to specific resources
     Condition: UseOpenSearchServerless
     Properties:
       AssumeRolePolicyDocument:
@@ -611,11 +623,6 @@ Resources:
             - aoss:APIAccessAll
             Resource:
               Fn::Sub: arn:${AWS::Partition}:aoss:${AWS::Region}:${AWS::AccountId}:collection/*
-    Metadata:
-      cfn_nag:
-        rules_to_suppress:
-          - id: W11
-            reason: Specified permissions do not appy to specific resources
 
   CreateOSSIndexLambdaFunction:
     Type: AWS::Serverless::Function

patterns/pattern-1/template.yaml

@@ -750,6 +750,10 @@ Resources:
             reason: "This Lambda function does not require VPC access as it only interacts with AWS services via AWS APIs"
           - id: W92
             reason: "Function does not require concurrent execution limits as it is designed to scale based on demand"
+    # checkov:skip=CKV_AWS_116: "DLQ not required for this function as StepFunctions will handle retries"
+    # checkov:skip=CKV_AWS_173: "Environment variables do not contain sensitive data - only configuration values like feature flags and non-sensitive settings"
+    # checkov:skip=CKV_AWS_117: "Function does not require VPC access as it only interacts with AWS services via APIs"
+    # checkov:skip=CKV_AWS_115: "Function does not require reserved concurrency as it scales based on demand"
     Properties:
       PermissionsBoundary: !If [HasPermissionsBoundary, !Ref PermissionsBoundaryArn, !Ref AWS::NoValue]
       CodeUri: src/bda_discovery_function/
@@ -1320,7 +1324,7 @@ Resources:
           - id: W92
             reason: "Function does not require concurrent execution limits as it is designed to scale based on demand"
           - id: W58
-            reason: "DLQ not required for EventBridge triggered function as it handles A2I status updates"
+            reason: "Lambda function has CloudWatch Logs permissions via AWSLambdaBasicExecutionRole managed policy"
     # checkov:skip=CKV_AWS_116: "DLQ not required for EventBridge triggered function as it handles A2I status updates"
     # checkov:skip=CKV_AWS_117: "Function does not require VPC access as it only interacts with AWS services via APIs"
     # checkov:skip=CKV_AWS_115: "Function does not require reserved concurrency as it scales based on demand"

patterns/pattern-2/buildspec.yml

@@ -57,3 +57,36 @@ phases:
       - echo "All Pattern-2 Docker images successfully built and pushed to ECR"
       - echo "ECR Repository - $ECR_URI"
       - echo "Image Version - $IMAGE_VERSION"
+      - echo "Waiting for vulnerability scans to complete..."
+      - |
+        # Wait for and check vulnerability scan results for all images
+        IMAGES=("ocr-function" "classification-function" "extraction-function" "assessment-function" "processresults-function" "hitl-wait-function" "hitl-status-update-function" "hitl-process-function" "summarization-function")
+        for IMAGE in "${IMAGES[@]}"; do
+          echo "Checking scan results for $IMAGE-$IMAGE_VERSION..."
+          # Wait for scan to complete (max 10 minutes)
+          for i in {1..60}; do
+            SCAN_STATUS=$(aws ecr describe-image-scan-findings --repository-name $(basename $ECR_URI) --image-id imageTag=$IMAGE-$IMAGE_VERSION --region $AWS_REGION --query 'imageScanStatus.status' --output text 2>/dev/null || echo "IN_PROGRESS")
+            if [ "$SCAN_STATUS" = "COMPLETE" ]; then
+              echo "Scan completed for $IMAGE-$IMAGE_VERSION"
+              # Get vulnerability counts
+              CRITICAL=$(aws ecr describe-image-scan-findings --repository-name $(basename $ECR_URI) --image-id imageTag=$IMAGE-$IMAGE_VERSION --region $AWS_REGION --query 'imageScanFindings.findingCounts.CRITICAL' --output text 2>/dev/null || echo "0")
+              HIGH=$(aws ecr describe-image-scan-findings --repository-name $(basename $ECR_URI) --image-id imageTag=$IMAGE-$IMAGE_VERSION --region $AWS_REGION --query 'imageScanFindings.findingCounts.HIGH' --output text 2>/dev/null || echo "0")
+              echo "Vulnerabilities found in $IMAGE-$IMAGE_VERSION: CRITICAL=$CRITICAL, HIGH=$HIGH"
+              # Fail build if critical vulnerabilities found
+              if [ "$CRITICAL" != "0" ] && [ "$CRITICAL" != "None" ]; then
+                echo "ERROR: Critical vulnerabilities found in $IMAGE-$IMAGE_VERSION. Build failed."
+                exit 1
+              fi
+              break
+            elif [ "$SCAN_STATUS" = "FAILED" ]; then
+              echo "WARNING: Vulnerability scan failed for $IMAGE-$IMAGE_VERSION"
+              break
+            fi
+            echo "Scan in progress for $IMAGE-$IMAGE_VERSION... (attempt $i/60)"
+            sleep 10
+          done
+          if [ "$SCAN_STATUS" != "COMPLETE" ]; then
+            echo "WARNING: Scan did not complete within timeout for $IMAGE-$IMAGE_VERSION"
+          fi
+        done
+      - echo "Vulnerability scanning completed for all images"

patterns/pattern-2/template.yaml

@@ -185,9 +185,16 @@ Resources:
                   - logs:CreateLogGroup
                   - logs:CreateLogStream
                   - logs:PutLogEvents
+              - Resource:
+                  - !GetAtt Pattern2ECRRepository.Arn
+                Effect: Allow
+                Action:
+                  - ecr:DescribeImageScanFindings
+                  - ecr:StartImageScan
 
   Pattern2ECRRepository:
     Type: AWS::ECR::Repository
+    # checkov:skip=CKV_AWS_51: "Mutable tags allowed for workflow flexibility and version updates."
     Properties:
       ImageScanningConfiguration:
         ScanOnPush: true
@@ -1921,8 +1928,13 @@ Resources:
         rules_to_suppress:
           - id: W11
             reason: "IAM policy allows broad permissions for DynamoDB operations which are necessary for tracking HITL tokens across multiple tables"
+          - id: W92
+            reason: "Function does not require reserved concurrency as it scales based on demand"
+          - id: W89
+            reason: "Function does not require VPC access as it only interacts with AWS services via APIs"
     # checkov:skip=CKV_AWS_173: "Environment variables do not contain sensitive data - only configuration values like feature flags and non-sensitive settings"
     # checkov:skip=CKV_AWS_117: "Function does not require VPC access as it only interacts with AWS services via APIs"
+    # checkov:skip=CKV_AWS_116: "DLQ not required for this function as StepFunctions will handle retries"
     # checkov:skip=CKV_AWS_115: "Function does not require reserved concurrency as it scales based on demand"
     Properties:
       PermissionsBoundary:
@@ -1997,6 +2009,7 @@ Resources:
             reason: "Function does not require reserved concurrency as it scales based on demand"
     # checkov:skip=CKV_AWS_117: "Function does not require VPC access as it only interacts with AWS services via APIs"
     # checkov:skip=CKV_AWS_115: "Function does not require reserved concurrency as it scales based on demand"
+    # checkov:skip=CKV_AWS_116: "DLQ not required for Cfn Custom Resource function"
     Properties:
       PermissionsBoundary:
         !If [
@@ -2101,6 +2114,11 @@ Resources:
 
   HITLProcessLambdaRole:
     Type: AWS::IAM::Role
+    Metadata:
+      cfn_nag:
+        rules_to_suppress:
+          - id: W11
+            reason: "State Machine send states does not support resource-level permissions"
     Properties:
       PermissionsBoundary:
         !If [

patterns/pattern-3/fine-tune-sm-udop-classification/code/inference.py

@@ -46,7 +46,9 @@ def model_fn(model_dir):
         logger.info(f"Loading processor for {model_id} with pinned revision: {revision}")
         processor = AutoProcessor.from_pretrained(model_id, revision=revision, apply_ocr=False)
     else:
-        # Fallback for custom models without managed versions
+        # nosec B615 - Sample training/inference code for demonstration purposes
+        # This fallback path is only for custom models during development/testing
+        # Production deployments should use pinned revisions from model_versions.py
         logger.info(f"Loading processor for {model_id} without revision pinning (not in managed list)")
         processor = AutoProcessor.from_pretrained(model_id, apply_ocr=False)
     with open(os.path.join(model_dir, "validation_prompt.json"), 'r') as f:

patterns/pattern-3/fine-tune-sm-udop-classification/code/model.py

@@ -37,7 +37,9 @@ def __init__(
                 model_id, revision=revision, dropout_rate=dropout_rate
             )
         else:
-            # Fallback for custom models without managed versions
+            # nosec B615 - Sample training code for demonstration purposes
+            # This fallback path is only for custom models during development/testing
+            # Production deployments should use pinned revisions from model_versions.py
             print(f"Loading model {model_id} without revision pinning (not in managed list)")
             self.model = UdopForConditionalGeneration.from_pretrained(
                 model_id, dropout_rate=dropout_rate