Merge pull request #31 from no10ds/fix/checkov-scans

Fix - Checkov Scanning

.pre-commit-config.yaml

@@ -38,7 +38,7 @@ repos:
   #     -   id: pylint
   #         exclude: (docs/|get_latest_release_changelog.py)
   - repo: https://github.com/antonbabenko/pre-commit-terraform
-    rev: v1.81.0
+    rev: v1.83.2
     hooks:
       - id: terraform_fmt
         exclude: '^(?!infrastructure/).*'
@@ -52,7 +52,7 @@ repos:
     rev: 2.3.261
     hooks:
       - id: checkov
-        args: [--quiet, --compact]
+        args: [--quiet, --compact, --skip-check, 'CKV2_GHA_1']
         exclude: '^(?!infrastructure/).*'
   - repo: local
     hooks:

api/Dockerfile

@@ -1,3 +1,6 @@
+#checkov:skip=CKV_DOCKER_9: Allow for use of apt
+#checkov:skip=CKV_DOCKER_2: No need for healthcheck in container
+#checkov:skip=CKV_DOCKER_3: No need for user in container
 FROM python:3.10-slim
 
 WORKDIR /app

docs/api/query.md

@@ -5,41 +5,36 @@ Data can be queried provided data has been uploaded at some point in the past.
 There are six values you can customise:
 
 - `select_columns`
-    - Which column(s) you want to select
-    - List of strings
-    - Can contain aggregation functions e.g.: `"avg(col1)"`, `"sum(col2)"`
-    - Can contain renaming of columns e.g.: `"col1 AS custom_name"`
+  - Which column(s) you want to select
+  - List of strings
+  - Can contain aggregation functions e.g.: `"avg(col1)"`, `"sum(col2)"`
+  - Can contain renaming of columns e.g.: `"col1 AS custom_name"`
 - `filter`
-    - How to filter the data
-    - This is provided as a raw SQL string
-    - Omit the `WHERE` keyword
+  - How to filter the data
+  - This is provided as a raw SQL string
+  - Omit the `WHERE` keyword
 - `group_by_columns`
-    - Which columns to group by
-    - List of column names as strings
+  - Which columns to group by
+  - List of column names as strings
 - `aggregation_conditions`
-    - What conditions you want to apply to aggregated values
-    - This is provided as a raw SQL string
-    - Omit the `HAVING` keyword
+  - What conditions you want to apply to aggregated values
+  - This is provided as a raw SQL string
+  - Omit the `HAVING` keyword
 - `order_by_columns`
-    - By which column(s) to order the data
-    - List of strings
-    - Defaults to ascending (`ASC`) if not provided
+  - By which column(s) to order the data
+  - List of strings
+  - Defaults to ascending (`ASC`) if not provided
 - `limit`
-    - How many rows to limit the results to
-    - String of an integer
+  - How many rows to limit the results to
+  - String of an integer
 
 For example:
 
 ```json
 {
-  "select_columns": [
-    "col1",
-    "avg(col2)"
-  ],
+  "select_columns": ["col1", "avg(col2)"],
   "filter": "col2 >= 10",
-  "group_by_columns": [
-    "col1"
-  ],
+  "group_by_columns": ["col1"],
   "aggregation_conditions": "avg(col2) <= 15",
   "order_by_columns": [
     {

infrastructure/Dockerfile

@@ -1,3 +1,6 @@
+#checkov:skip=CKV_DOCKER_9: Allow for use of apt
+#checkov:skip=CKV_DOCKER_2: No need for healthcheck in container
+#checkov:skip=CKV_DOCKER_3: No need for user in container
 FROM python:3.10-slim
 
 WORKDIR /app

infrastructure/blocks/app-cluster/main.tf

@@ -15,6 +15,7 @@ module "app_cluster" {
   cognito_user_login_app_credentials_secrets_name = data.terraform_remote_state.auth-state.outputs.cognito_user_app_secret_manager_name
   permissions_table_arn                           = data.terraform_remote_state.auth-state.outputs.user_permission_table_arn
   schema_table_arn                                = data.terraform_remote_state.data-workflow-state.outputs.schema_table_arn
+  catalogue_db_arn                                = data.terraform_remote_state.data-workflow-state.outputs.catalogue_db_arn
 
   application_version                  = var.application_version
   domain_name                          = var.domain_name

infrastructure/blocks/data-workflow/output.tf

@@ -13,6 +13,11 @@ output "schema_table_arn" {
   description = "The ARN of the DynamoDB schema table"
 }
 
+output "catalogue_db_arn" {
+  value       = module.data_workflow.catalogue_db_arn
+  description = "The ARN of the Glue Catalogue database"
+}
+
 output "tags" {
   value       = module.data_workflow.tags
   description = "The tags used in the project"

infrastructure/blocks/pipeline/iam.tf

@@ -1,4 +1,5 @@
 resource "aws_iam_policy" "pipeline_ecr_access" {
+  # checkov:skip=CKV_AWS_355: GetAuthorizationToken has no resource constraint
   name        = "pipeline_ecr_access"
   description = "Allow pipeline to access ECR"
   tags        = var.tags
@@ -53,6 +54,7 @@ resource "aws_iam_policy" "pipeline_ecr_access" {
 }
 
 resource "aws_iam_policy" "pipeline_ecr_public_access" {
+  # checkov:skip=CKV_AWS_355: GetAuthorizationToken has no resource constraint
   name        = "pipeline_ecr_public_access"
   description = "Allow pipeline to access the public ECR"
   tags        = var.tags
@@ -128,6 +130,8 @@ resource "aws_iam_policy" "pipeline_s3_access" {
 }
 
 resource "aws_iam_policy" "pipeline_secrets_manager_access" {
+  # checkov:skip=CKV_AWS_355: Allow secrets manager access
+  # checkov:skip=CKV_AWS_288: Likewise
   name        = "pipeline_secrets_manager_access"
   description = "Allow pipeline to access AWS Secrets Manager"
   tags        = var.tags
@@ -152,6 +156,8 @@ resource "aws_iam_policy" "pipeline_secrets_manager_access" {
 }
 
 resource "aws_iam_policy" "pipeline_dynamodb_access" {
+  # checkov:skip=CKV_AWS_290: Allow dynamodb access
+  # checkov:skip=CKV_AWS_355: Likewise
   name        = "pipeline_dynamodb_access"
   description = "Allow pipeline to access DynamoDB"
   tags        = var.tags
@@ -172,6 +178,8 @@ resource "aws_iam_policy" "pipeline_dynamodb_access" {
 }
 
 resource "aws_iam_policy" "pipeline_glue_access" {
+  # checkov:skip=CKV_AWS_290: Allow glue access
+  # checkov:skip=CKV_AWS_355: Likewise
   name        = "pipeline_glue_access"
   description = "Allow pipeline to access Glue"
   tags        = var.tags

infrastructure/blocks/s3/main.tf

@@ -7,7 +7,8 @@ terraform {
 resource "aws_s3_bucket" "rapid_data_storage" {
   #checkov:skip=CKV_AWS_144:No need for cross region replication
   #checkov:skip=CKV_AWS_145:No need for non default key
-
+  #checkov:skip=CKV2_AWS_62:No need for event notifications
+  #checkov:skip=CKV2_AWS_61:No need for lifecycle configuration
   bucket        = var.data_bucket_name
   acl           = "private"
   force_destroy = false
@@ -44,6 +45,8 @@ resource "aws_s3_bucket" "logs" {
   #checkov:skip=CKV_AWS_145:No need for non default key
   #checkov:skip=CKV_AWS_18:Log bucket shouldn't be logging
   #checkov:skip=CKV_AWS_21:No need to version log bucket
+  #checkov:skip=CKV2_AWS_62:No need for event notifications
+  #checkov:skip=CKV2_AWS_61:No need for lifecycle configuration
   bucket        = var.log_bucket_name
   acl           = "private"
   force_destroy = false

infrastructure/modules/app-cluster/cloudtrail.tf

@@ -90,6 +90,7 @@ resource "aws_kms_key" "access_logs_key" {
 }
 
 resource "aws_cloudwatch_log_group" "access_logs_log_group" {
+  # checkov:skip=CKV_AWS_338: No need for 1 year log retention
   count             = var.enable_cloudtrail ? 1 : 0
   depends_on        = [aws_kms_key.access_logs_key]
   name              = "${var.resource-name-prefix}_access_logs"
@@ -165,6 +166,9 @@ resource "aws_s3_bucket" "access_logs" {
   #checkov:skip=CKV_AWS_144:No need for cross region replication
   #checkov:skip=CKV_AWS_145:No need for non default key
   #checkov:skip=CKV_AWS_19:No need for securely encrypted at rest
+  #checkov:skip=CKV2_AWS_6:Public blocking applied via resource
+  #checkov:skip=CKV2_AWS_62:No need for event notifications
+  #checkov:skip=CKV2_AWS_61:No need for lifecycle configuration
   count         = var.enable_cloudtrail ? 1 : 0
   bucket        = "${var.resource-name-prefix}-access-logs"
   force_destroy = true
@@ -242,6 +246,7 @@ POLICY
 }
 
 resource "aws_s3_bucket_lifecycle_configuration" "access_logs_lifecycle" {
+  # checkov:skip=CKV_AWS_300: No need for aborting failed rule
   count  = var.enable_cloudtrail ? 1 : 0
   bucket = aws_s3_bucket.access_logs[0].id
 
@@ -270,6 +275,7 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "access_logs_s3_en
 
 resource "aws_cloudtrail" "access_logs_trail" {
   # checkov:skip=CKV_AWS_252:No need for an SNS topic
+  # checkov:skip=CKV2_AWS_10:Cloudwatch logs are enabled
   count = var.enable_cloudtrail ? 1 : 0
   depends_on = [
     aws_s3_bucket_policy.access_logs_bucket_policy, # Policy for S3 that cloudtrail dumps too
@@ -302,7 +308,7 @@ resource "aws_cloudtrail" "access_logs_trail" {
   }
 
   # CloudTrail requires the Log Stream wildcard
-  cloud_watch_logs_group_arn = "${aws_cloudwatch_log_group.access_logs_log_group[0].arn}:*" # Needs to be created
+  cloud_watch_logs_group_arn = "${aws_cloudwatch_log_group.access_logs_log_group[0].arn}:*"
   cloud_watch_logs_role_arn  = aws_iam_role.cloud_trail_role[0].arn
 
   tags = var.tags

infrastructure/modules/app-cluster/load_balancer.tf

@@ -65,6 +65,7 @@ data "aws_ec2_managed_prefix_list" "cloudwatch" {
   name = "com.amazonaws.global.cloudfront.origin-facing"
 }
 resource "aws_security_group" "load_balancer_security_group" {
+  # checkov:skip=CKV_AWS_260: Limits by prefix list ID's
   vpc_id      = var.vpc_id
   description = "ALB Security Group"
   ingress {

infrastructure/modules/app-cluster/main.tf

@@ -141,7 +141,7 @@ resource "aws_iam_policy" "app_glue_access" {
           "glue:CreateTable"
         ],
         "Resource" : [
-          "*"
+          var.catalogue_db_arn
         ]
       },
     ]
@@ -210,6 +210,8 @@ resource "aws_iam_policy" "app_dynamodb_access" {
 }
 
 resource "aws_iam_policy" "app_secrets_manager_access" {
+  # checkov:skip=CKV_AWS_355: Allow secrets manager access
+  # checkov:skip=CKV_AWS_288: Likewise
   name        = "${var.resource-name-prefix}-app_secrets_manager_access"
   description = "Allow application instance to access AWS Secrets Manager"
   tags        = var.tags
@@ -233,24 +235,6 @@ resource "aws_iam_policy" "app_secrets_manager_access" {
   })
 }
 
-resource "aws_iam_policy" "app_tags_access" {
-  name        = "${var.resource-name-prefix}-app_tags_access"
-  description = "Allow application instance to get resources by tags"
-  tags        = var.tags
-
-  policy = jsonencode({
-    "Version" : "2012-10-17",
-    "Statement" : [
-      {
-        Effect : "Allow",
-        Action : ["tag:GetResources"],
-        Resource : "*"
-      }
-    ]
-  })
-}
-
-
 resource "aws_iam_role_policy_attachment" "role_s3_access_policy_attachment" {
   role       = aws_iam_role.ecsTaskExecutionRole.name
   policy_arn = aws_iam_policy.app_s3_access.arn
@@ -276,11 +260,6 @@ resource "aws_iam_role_policy_attachment" "role_glue_access_policy_attachment" {
   policy_arn = aws_iam_policy.app_glue_access.arn
 }
 
-resource "aws_iam_role_policy_attachment" "role_tags_access_policy_attachment" {
-  role       = aws_iam_role.ecsTaskExecutionRole.name
-  policy_arn = aws_iam_policy.app_tags_access.arn
-}
-
 resource "aws_iam_role_policy_attachment" "role_dynamodb_access_policy_attachment" {
   role       = aws_iam_role.ecsTaskExecutionRole.name
   policy_arn = aws_iam_policy.app_dynamodb_access.arn
@@ -298,6 +277,7 @@ resource "aws_ecs_cluster" "aws-ecs-cluster" {
 
 resource "aws_cloudwatch_log_group" "log-group" {
   #checkov:skip=CKV_AWS_158:No need for KMS
+  #checkov:skip=CKV_AWS_338:No need for year long retention
   name              = "${var.resource-name-prefix}-logs"
   tags              = var.tags
   retention_in_days = 90

infrastructure/modules/app-cluster/routing.tf

@@ -1,4 +1,6 @@
 resource "aws_route53_zone" "primary-hosted-zone" {
+  # checkov:skip=CKV2_AWS_39: No need for DNS query logging
+  # checkov:skip=CKV2_AWS_38: No need for DNSSEC signing
   count = var.hosted_zone_id == "" ? 1 : 0
   name  = var.domain_name
   tags  = var.tags

infrastructure/modules/app-cluster/variables.tf

@@ -133,6 +133,11 @@ variable "schema_table_arn" {
   description = "The ARN of the schema table in dynamoDB"
 }
 
+variable "catalogue_db_arn" {
+  type        = string
+  description = "The ARN of the catalogue db in dynamoDB"
+}
+
 variable "cognito_user_pool_id" {
   type        = string
   description = "User pool id for cognito"

infrastructure/modules/auth/main.tf

@@ -80,11 +80,13 @@ resource "aws_cognito_user_pool_domain" "rapid_cognito_domain" {
 
 resource "aws_secretsmanager_secret" "client_secrets_cognito" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name = "${var.resource-name-prefix}_client_secrets_cognito"
 }
 
 resource "aws_secretsmanager_secret" "user_secrets_cognito" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name = "${var.resource-name-prefix}_user_secrets_cognito"
 }
 

infrastructure/modules/auth/users.tf

@@ -14,6 +14,7 @@ resource "aws_cognito_user_pool_client" "e2e_test_client_user_admin" {
 
 resource "aws_secretsmanager_secret" "e2e_test_client_user_admin" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name                    = "${var.resource-name-prefix}_E2E_TEST_CLIENT_USER_ADMIN"
   recovery_window_in_days = 0
 }
@@ -43,6 +44,7 @@ resource "aws_cognito_user_pool_client" "e2e_test_client_data_admin" {
 
 resource "aws_secretsmanager_secret" "e2e_test_client_data_admin" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name                    = "${var.resource-name-prefix}_E2E_TEST_CLIENT_DATA_ADMIN"
   recovery_window_in_days = 0
 }
@@ -72,6 +74,7 @@ resource "aws_cognito_user_pool_client" "e2e_test_client_read_and_write" {
 
 resource "aws_secretsmanager_secret" "e2e_test_client_read_and_write" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name                    = "${var.resource-name-prefix}_E2E_TEST_CLIENT_READ_ALL_WRITE_ALL"
   recovery_window_in_days = 0
 }
@@ -101,6 +104,7 @@ resource "aws_cognito_user_pool_client" "e2e_test_client_write_all" {
 
 resource "aws_secretsmanager_secret" "e2e_test_client_write_all" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name                    = "${var.resource-name-prefix}_E2E_TEST_CLIENT_WRITE_ALL"
   recovery_window_in_days = 0
 }
@@ -132,6 +136,7 @@ resource "aws_cognito_user" "ui_test_user" {
 
 resource "aws_secretsmanager_secret" "ui_test_user" {
   # checkov:skip=CKV_AWS_149:AWS Managed Key is sufficient
+  # checkov:skip=CKV2_AWS_57:Disable automatic rotation enabled
   name                    = "${var.resource-name-prefix}_UI_TEST_USER"
   recovery_window_in_days = 0
 }

infrastructure/modules/aws-core/config/main.tf

@@ -65,9 +65,11 @@ resource "aws_iam_role_policy_attachment" "allow_s3_access_for_aws_config_attach
 
 # S3 buckets
 resource "aws_s3_bucket" "config_with_lifecycle" {
-  # checkov:skip=CKV_AWS_144:No need for cross region replication
-  # checkov:skip=CKV_AWS_18:No need for logging
-
+  #checkov:skip=CKV_AWS_144:No need for cross region replication
+  #checkov:skip=CKV_AWS_18:No need for logging
+  #checkov:skip=CKV2_AWS_62:No need for event notifications
+  #checkov:skip=CKV2_AWS_6:Public blocking applied via resource
+  #checkov:skip=CKV2_AWS_61:No need for lifecycle configuration
   count         = var.enable_lifecycle_management_for_s3 ? 1 : 0
   bucket_prefix = var.bucket_prefix
   acl           = "private"
@@ -115,9 +117,10 @@ resource "aws_s3_bucket_public_access_block" "config_with_lifecycle" {
 
 
 resource "aws_s3_bucket" "config_without_lifecycle" {
-  # checkov:skip=CKV_AWS_144:No need for cross region replication
-  # checkov:skip=CKV_AWS_18:No need for logging
-
+  #checkov:skip=CKV_AWS_144:No need for cross region replication
+  #checkov:skip=CKV_AWS_18:No need for logging
+  #checkov:skip=CKV2_AWS_61:No need for lifecycle configuration
+  #checkov:skip=CKV2_AWS_62:No need for event notifications
   count         = var.enable_lifecycle_management_for_s3 ? 0 : 1
   bucket_prefix = var.bucket_prefix
   acl           = "private"

infrastructure/modules/aws-core/iam-resources/iam.tf

@@ -173,6 +173,7 @@ resource "aws_iam_policy" "user_no_vpc_access_policy" {
 
 # Policy attachments for roles
 resource "aws_iam_policy_attachment" "admin_access_policy_attachment" {
+  # checkov:skip=CKV_AWS_274: Allow for AWS Administator Access
   name       = "admin_access_policy_attachment"
   roles      = [aws_iam_role.admin_access_role.name]
   policy_arn = local.administrator_access_policy_arn