⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Enhance Trivy IaC scanning #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
neil-sproston merged 1 commit into from
Feb 10, 2026
+70 −2
Merged

Enhance Trivy IaC scanning #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/preview-env.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ jobs:
with:
mtls-secret-name: ${{ vars.PREVIEW_ENV_MTLS_SECRET_NAME}}
target-url: ${{ steps.tf-output.outputs.preview_url }}
proxy-base-path: 'clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}'
proxy-base-path: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}"
proxygen-key-secret: ${{ env._cds_gateway_dev_proxygen_proxygen_key_secret }}
proxygen-key-id: ${{ vars.PREVIEW_ENV_PROXYGEN_KEY_ID }}
proxygen-api-name: ${{ vars.PROXYGEN_API_NAME }}
Expand All @@ -178,7 +178,7 @@ jobs:
if: github.event.action == 'closed'
uses: ./.github/actions/proxy/tear-down-proxy
with:
proxy-base-path: 'clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}'
proxy-base-path: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}"
proxygen-key-secret: ${{ env._cds_gateway_dev_proxygen_proxygen_key_secret }}
proxygen-key-id: ${{ vars.PREVIEW_ENV_PROXYGEN_KEY_ID }}
proxygen-api-name: ${{ vars.PROXYGEN_API_NAME }}
Expand Down Expand Up @@ -348,6 +348,13 @@ jobs:
});

# ---------- Security scanning ----------
- name: Trivy IaC scan
if: github.event.action != 'closed'
uses: nhs-england-tools/trivy-action/iac-scan@3456c1657a37d500027fd782e6b08911725392da
Copy link
Contributor

@Vox-Ben Vox-Ben Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse my ignorance, what's the magic number for?

Copy link
Contributor Author

@neil-sproston neil-sproston Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number is the commit hash of a specific release of the action.
so instead of pinning it ti V4.2 we are pining it to s specific hash (considered best practice so as to prevent malicious re-releases...)

with:
scan-ref: infrastructure/environments/preview
artifact-name: trivy-iac-scan-${{ steps.meta.outputs.branch_name }}

- name: Trivy filesystem scan
if: github.event.action != 'closed'
uses: nhs-england-tools/trivy-action/image-scan@3456c1657a37d500027fd782e6b08911725392da
Expand Down
55 changes: 55 additions & 0 deletions infrastructure/environments/preview/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

data "aws_region" "current" {}

data "aws_caller_identity" "current" {}

data "terraform_remote_state" "core" {
backend = "s3"
config = {
Expand Down Expand Up @@ -38,6 +40,8 @@ locals {
alb_listener_arn = data.terraform_remote_state.core.outputs.alb_listener_arn
ecs_cluster_name = data.terraform_remote_state.core.outputs.ecs_cluster_name
ecr_repository_url = data.terraform_remote_state.core.outputs.ecr_repository_url

log_kms_key_id = var.log_kms_key_id != null ? var.log_kms_key_id : aws_kms_key.log_group[0].arn
}

############################
Expand Down Expand Up @@ -141,6 +145,57 @@ resource "aws_iam_role_policy" "task_exec_command" {
resource "aws_cloudwatch_log_group" "branch" {
name = local.log_group_name
retention_in_days = var.log_retention_days
kms_key_id = local.log_kms_key_id
}

resource "aws_kms_key" "log_group" {
count = var.log_kms_key_id == null ? 1 : 0

description = "KMS key for preview CloudWatch Logs"
deletion_window_in_days = 30
Copy link
Contributor

@Vox-Ben Vox-Ben Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to keep logs for 30 days? Seems like a long time, and if we ever have a lot going on it might be a lot of logs.

Copy link
Contributor Author

@neil-sproston neil-sproston Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each environment gets its own logs so should always be easy to focus down.
We can certainly look to reduce the retention as we move on

enable_key_rotation = true

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Sid = "EnableRootPermissions"
Effect = "Allow"
Principal = {
AWS = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"
}
Action = "kms:*"
Resource = "*"
},
{
Sid = "AllowCloudWatchLogs"
Effect = "Allow"
Principal = {
Service = "logs.${data.aws_region.current.name}.amazonaws.com"
}
Action = [
"kms:Encrypt",
"kms:Decrypt",
"kms:ReEncrypt*",
"kms:GenerateDataKey*",
"kms:DescribeKey"
]
Resource = "*"
Condition = {
ArnLike = {
"kms:EncryptionContext:aws:logs:arn" = "arn:aws:logs:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:log-group:${local.log_group_name}*"
}
}
}
]
})
}

resource "aws_kms_alias" "log_group" {
count = var.log_kms_key_id == null ? 1 : 0

name = "alias/preview-logs-${local.branch_role_suffix}"
target_key_id = aws_kms_key.log_group[0].key_id
}

############################
Expand Down
6 changes: 6 additions & 0 deletions infrastructure/environments/preview/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,9 @@ variable "log_retention_days" {
type = number
default = 14
}

variable "log_kms_key_id" {
description = "KMS CMK ARN or ID used to encrypt the CloudWatch log group."
type = string
default = null
}