ELI-597: Automates NHS Number salt creation and rotation #537

shweta-nhs · 2026-01-08T15:55:07Z

Description

Context

Type of changes

Refactoring (non-breaking change)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would change existing functionality)
Bug fix (non-breaking change which fixes an issue)

Checklist

I am familiar with the contributing guidelines
I have followed the code style of the project
I have added tests to cover my changes
I have updated the documentation accordingly
This PR is a result of pair or mob programming

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

eddalmond1

Looking good - a few minor checks around ensuring tags are definitely being managed by this process completely

infrastructure/stacks/api-layer/iam_policies.tf

infrastructure/stacks/api-layer/scripts/create_pending_secret.py

infrastructure/stacks/api-layer/scripts/promote_to_current.py

infrastructure/stacks/api-layer/step_functions.tf

infrastructure/stacks/api-layer/scripts/create_pending_secret.py

infrastructure/stacks/api-layer/step_functions.tf

eddalmond1 · 2026-01-09T16:32:55Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "create_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+
+  filename      = data.archive_file.create_zip.output_path
+  function_name = "${terraform.workspace}-CreatePendingSecretFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "create_pending_secret.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


this just needs a VPC config block e.g.

vpc_config {
subnet_ids = [for v in data.aws_subnet.private_subnets : v.id]
security_group_ids = [data.aws_security_group.main_sg.id]
}

I think we do need it within VPC, even if technically it'll just be making requests across the AWS control plane

infrastructure/stacks/api-layer/lambda.tf

eddalmond1 · 2026-01-09T16:43:29Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "create_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+
+  filename      = data.archive_file.create_zip.output_path
+  function_name = "${terraform.workspace}-CreatePendingSecretFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "create_pending_secret.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


this just needs

reserved_concurrent_executions = 1

adding in the config, and makes sense as we wouldn't want parallel executions

infrastructure/stacks/api-layer/lambda.tf

eddalmond1 · 2026-01-09T16:43:09Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "promote_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+  filename      = data.archive_file.promote_zip.output_path
+  function_name = "${terraform.workspace}-PromoteToCurrentFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "promote_to_current.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


this just needs

reserved_concurrent_executions = 1

adding in the config, and makes sense as we wouldn't want parallel executions

infrastructure/stacks/api-layer/lambda.tf

eddalmond1 · 2026-01-09T16:46:44Z

infrastructure/stacks/api-layer/sns.tf

+resource "aws_sns_topic" "cli_login_topic" {
+  #checkov:skip=CKV_AWS_26: Topic contains nothing sensitive so no encryption required
+  name = "cli-login-notifications"
+}


I think we'd want this encrypting so add a kms key

resource "aws_kms_key" "whatever you want to call it" {
description = "KMS key for SNS topic encryption"
deletion_window_in_days = 14
enable_key_rotation = true

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Sid = "Enable IAM User Permissions"
Effect = "Allow"
Principal = {
AWS = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"
}
Action = "kms:"
Resource = ""
},
{
Sid = "Allow SNS to use the key"
Effect = "Allow"
Principal = {
Service = "sns.amazonaws.com"
}
Action = [
"kms:Decrypt",
"kms:GenerateDataKey"
]
Resource = "*"
}
]
})
}

resource "aws_kms_alias" "sns_kms_alias" {
name = "alias/${terraform.workspace}-sns-topic-key" # change this as well
target_key_id = aws_kms_key.sns_kms_key.key_id
}

data "aws_caller_identity" "current" {}

and use above e.g.

name = "cli-login-notifications"
kms_master_key_id = aws_kms_key.sns_kms_key.id
}

infrastructure/stacks/api-layer/step_functions.tf

eddalmond1 · 2026-01-09T16:32:55Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "create_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+
+  filename      = data.archive_file.create_zip.output_path
+  function_name = "${terraform.workspace}-CreatePendingSecretFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "create_pending_secret.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


this just needs a VPC config block e.g.

vpc_config {
subnet_ids = [for v in data.aws_subnet.private_subnets : v.id]
security_group_ids = [data.aws_security_group.main_sg.id]
}

eddalmond1 · 2026-01-09T16:37:21Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "create_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+
+  filename      = data.archive_file.create_zip.output_path
+  function_name = "${terraform.workspace}-CreatePendingSecretFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "create_pending_secret.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


I think we do need it within VPC, even if technically it'll just be making requests across the AWS control plane

infrastructure/stacks/api-layer/lambda.tf

eddalmond1 · 2026-01-09T16:43:29Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "create_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+
+  filename      = data.archive_file.create_zip.output_path
+  function_name = "${terraform.workspace}-CreatePendingSecretFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "create_pending_secret.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


this just needs

reserved_concurrent_executions = 1

adding in the config, and makes sense as we wouldn't want parallel executions

eddalmond1 · 2026-01-09T16:43:41Z

infrastructure/stacks/api-layer/lambda.tf

+resource "aws_lambda_function" "promote_secret_lambda" {
+  #checkov:skip=CKV_AWS_116: No deadletter queue is required for this Lambda function
+  #checkov:skip=CKV_AWS_115: Not applicable as it will not be possible to trigger concurrently
+  #checkov:skip=CKV_AWS_272: Skipping code signing but flagged to create ticket to investigate on ELI-238
+  #checkov:skip=CKV_AWS_50: No x-ray needed for this function
+  #checkov:skip=CKV_AWS_173: No encryption needed for the secret name
+  #checkov:skip=CKV_AWS_117: Does not need to be in a VPC
+  filename      = data.archive_file.promote_zip.output_path
+  function_name = "${terraform.workspace}-PromoteToCurrentFunction"
+  role          = aws_iam_role.rotation_lambda_role.arn
+  handler       = "promote_to_current.lambda_handler"
+  runtime       = "python3.13"
+  timeout       = 30
+
+  environment {
+    variables = { SECRET_NAME = module.secrets_manager.aws_hashing_secret_name }
+  }
+}


eddalmond1 · 2026-01-09T16:46:44Z

infrastructure/stacks/api-layer/sns.tf

+resource "aws_sns_topic" "cli_login_topic" {
+  #checkov:skip=CKV_AWS_26: Topic contains nothing sensitive so no encryption required
+  name = "cli-login-notifications"
+}


I think we'd want this encrypting so add a kms key

resource "aws_kms_key" "whatever you want to call it" {
description = "KMS key for SNS topic encryption"
deletion_window_in_days = 14
enable_key_rotation = true

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Sid = "Enable IAM User Permissions"
Effect = "Allow"
Principal = {
AWS = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"
}
Action = "kms:"
Resource = ""
},
{
Sid = "Allow SNS to use the key"
Effect = "Allow"
Principal = {
Service = "sns.amazonaws.com"
}
Action = [
"kms:Decrypt",
"kms:GenerateDataKey"
]
Resource = "*"
}
]
})
}

resource "aws_kms_alias" "sns_kms_alias" {
name = "alias/${terraform.workspace}-sns-topic-key" # change this as well
target_key_id = aws_kms_key.sns_kms_key.key_id
}

data "aws_caller_identity" "current" {}

and use above e.g.

name = "cli-login-notifications"
kms_master_key_id = aws_kms_key.sns_kms_key.id
}

infrastructure/stacks/api-layer/step_functions.tf

shweta-nhs added 2 commits January 8, 2026 14:38

ELI-597: Initial setup for automation of secret rotation

236769b

ELI-597: Use github env variable for sns subscription email

91d631b

eddalmond1 reviewed Jan 8, 2026

View reviewed changes

Karthikeyannhs reviewed Jan 9, 2026

View reviewed changes

infrastructure/stacks/api-layer/step_functions.tf Show resolved Hide resolved

(ELI-597) solving checkov issues and addressing comments

75b207e

github-advanced-security bot found potential problems Jan 9, 2026

View reviewed changes

TOEL2 and others added 5 commits January 9, 2026 15:15

(ELI-597) linting

671c1b8

Merge branch 'main' into ELI-597

e44b7dd

(ELI-597) linting

99919c8

ELI-597: Adds FBT001 ignore back

9d7ca86

Merge branch 'main' into ELI-597

b78df45

eddalmond1 requested changes Jan 9, 2026

View reviewed changes

ELI-597: Automates NHS Number salt creation and rotation #537

Are you sure you want to change the base?

ELI-597: Automates NHS Number salt creation and rotation #537

Uh oh!

Conversation

shweta-nhs commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Type of changes

Checklist

Sensitive Information Declaration

Uh oh!

eddalmond1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shweta-nhs commented Jan 8, 2026 •

edited

Loading