Conversation
There was a problem hiding this comment.
Pull request overview
Adds AWS Lambda code signing to improve deployment security by signing the built Lambda zip and enforcing signed-only deployments in higher environments.
Changes:
- Adds AWS Signer signing profile + Lambda Code Signing Config in the Lambda Terraform module and conditionally attaches it in test/preprod/prod.
- Introduces a “sign artifact” job in the TEST deployment workflow to sign the Lambda zip via AWS Signer before Terraform apply.
- Extends IAM policies/boundaries to allow GitHub Actions (and developers) to manage code signing resources.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| infrastructure/stacks/iams-developer-roles/iams_permissions_boundary.tf | Expands developer permissions boundary to include Signer + Lambda code signing actions |
| infrastructure/stacks/iams-developer-roles/github_actions_policies.tf | Adds a GitHub Actions IAM policy for managing code signing and Signer operations |
| infrastructure/modules/lambda/signing.tf | Creates AWS Signer signing profile and Lambda code signing config; exports profile name |
| infrastructure/modules/lambda/locals.tf | Adds env-gated toggle for enabling Lambda code signing attachment |
| infrastructure/modules/lambda/lambda.tf | Attaches code signing config ARN to the Lambda function when enabled |
| .github/workflows/cicd-3-test-deploy.yaml | Adds signing job in TEST pipeline; deploy job consumes signed artifact |
infrastructure/stacks/iams-developer-roles/github_actions_policies.tf
Outdated
Show resolved
Hide resolved
| - name: "Extract S3 bucket name from Terraform output" | ||
| id: tf_output | ||
| run: | | ||
| BUCKET=$(terraform output -raw lambda_artifact_bucket) | ||
| PROFILE=$(terraform output -raw lambda_signing_profile_name) | ||
| echo "bucket_name=$BUCKET" >> $GITHUB_OUTPUT | ||
| echo "signing_profile_name=$PROFILE" >> $GITHUB_OUTPUT | ||
| working-directory: ./infrastructure/stacks/api-layer |
There was a problem hiding this comment.
This step runs terraform output -raw lambda_signing_profile_name, but the api-layer stack currently doesn’t define an output "lambda_signing_profile_name" (only lambda_artifact_bucket exists in infrastructure/stacks/api-layer/s3_buckets.tf). As a result, this workflow will fail here. Expose the module output from the stack (or avoid relying on a Terraform output by deriving/standardising the profile name).
| run: | | ||
| echo "Running: make terraform env=$ENVIRONMENT workspace=$WORKSPACE stack=api-layer tf-command=init" | ||
| make terraform env=$ENVIRONMENT stack=api-layer tf-command=init workspace=$WORKSPACE | ||
| working-directory: ./infrastructure |
There was a problem hiding this comment.
The signing job depends on a signing profile that must already exist in the target AWS account/state before this workflow runs (it only does init + output). On the first deployment after introducing these Terraform resources, the profile/config won’t exist yet, so signing will fail before the workflow reaches the later terraform apply that would create them. Consider a bootstrap path (e.g. create signing profile/config in a prior step/stack, or handle missing outputs by applying the signing resources first).
| working-directory: ./infrastructure | |
| working-directory: ./infrastructure/stacks/api-layer |
infrastructure/stacks/iams-developer-roles/github_actions_policies.tf
Dismissed
Show dismissed
Hide dismissed
infrastructure/stacks/iams-developer-roles/github_actions_policies.tf
Dismissed
Show dismissed
Hide dismissed
|
After we agreed to split this up into two tickets, I have disabled the signing requirement on the lambda simply by un-linking the signing config arn from the lambda definition using a variable and left comments for the next step. I have also removed the workflow changes and put them in another workflow that we can activate once we have the required resources. In effect this pr will make changes to our infrastructure but no functional changes to the api or the ci/cd pipeline, just sets us up to do that on a subsequent pr. |
| description = "Only allow Lambda bundles signed by our trusted signer profile" | ||
| } | ||
|
|
||
| output "lambda_signing_profile_name" { |
There was a problem hiding this comment.
this might need to be added to outputs.tf in the module, then potentially re-output in the stack (in infrastructure/stacks/api-layer/lambda.tf ?)
| echo "Running: make terraform env=$ENVIRONMENT workspace=$WORKSPACE stack=api-layer tf-command=init" | ||
| make terraform env=$ENVIRONMENT stack=api-layer tf-command=init workspace=$WORKSPACE | ||
| working-directory: ./infrastructure | ||
|
|
There was a problem hiding this comment.
I'm not clear on intention here - no apply step, which would presumably block, unless the intention is to run the other workflow, to deploy the codesigning infra, then ths to test it out / turn it on?
There was a problem hiding this comment.
its to get the outputs from the init so that we can use them in the signing step below (120)
There was a problem hiding this comment.
then once we have a signed lambda we can do the next job in this workflow - deploy which does the tf apply
| with: | ||
| terraform_version: ${{ needs.metadata.outputs.terraform_version }} | ||
|
|
||
| - name: "Configure AWS Credentials" |
There was a problem hiding this comment.
Likewise (to below comment), this should still have the iams roles deployment
| "signer:CancelSigningProfile", | ||
| "signer:RevokeSignature" | ||
| ], | ||
| Resource = "arn:aws:signer:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:/signing-profiles/eligibility-signposting-api-*" |
There was a problem hiding this comment.
The resource gets a name like 'EligibilityApiLambdaSigningProfile'
e.g. line 2 of signing.tf:
"${terraform.workspace}"}EligibilityApiLambdaSigningProfile"
There was a problem hiding this comment.
yeah oversight there, changed it in one place and not the other - will fix
| echo "signing_profile_name=$PROFILE" >> $GITHUB_OUTPUT | ||
| working-directory: ./infrastructure/stacks/api-layer | ||
|
|
||
| - name: "Upload unsigned lambda artifact to S3" |
There was a problem hiding this comment.
I'm wondering now whether each environment will need to sign, as we're using separate accounts (so Prod has no reason to trust PreProd etc....)
Description
Implements code signing for test env and beyond - we only need to sign once and then that signed object is passed through
Context
increases security
Type of changes
Checklist
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.