Merged
Conversation
883ba76 to
9fc8c15
Compare
sidnhs
approved these changes
Mar 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the integration test harness away from the S3 “debug log bucket” approach to using CloudWatch Logs for callback/assertion data, and adds end-to-end signature verification coverage.
Changes:
- Replace S3 debug-log-bucket-based integration assertions with CloudWatch Logs polling helpers and update integration tests accordingly.
- Add callback signature verification (mock webhook verifies inbound signature; integration tests compute expected signature and assert it).
- Remove S3 debug logging support from the shared logger and retire the debug log bucket Terraform resources.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/package.json | Adds AWS Lambda SDK client dependency used by integration helpers. |
| tests/integration/metrics.test.ts | New integration test validating EMF metrics via CloudWatch Logs. |
| tests/integration/jest.setup.ts | Removes log flush in teardown after logger refactor. |
| tests/integration/jest.global-setup.ts | Removes debug-bucket cleanup logic. |
| tests/integration/jest.config.ts | Forces Jest to exit after integration run completes. |
| tests/integration/helpers/status-events.ts | Switches status-event processing helper to read callbacks from CloudWatch logs. |
| tests/integration/helpers/signature.ts | Adds expected signature computation for integration assertions. |
| tests/integration/helpers/lambda.ts | Adds helper to read Lambda environment variables. |
| tests/integration/helpers/index.ts | Re-exports new CloudWatch/Lambda/signature helpers; drops S3 callbacks export. |
| tests/integration/helpers/deployment.ts | Replaces debug-bucket naming with Lambda function/log group naming helpers. |
| tests/integration/helpers/cloudwatch.ts | Adds CloudWatch Logs polling helpers for signed callbacks + EMF metrics. |
| tests/integration/helpers/clients.ts | Adds CloudWatch Logs and Lambda client factories. |
| tests/integration/helpers/callbacks.ts | Deleted: removes S3 debug-bucket callback extraction helpers. |
| tests/integration/event-bus-to-webhook.test.ts | Updates integration flow assertions to use CloudWatch logs + signature checks. |
| tests/integration/dlq-redrive.test.ts | Updates DLQ/redrive assertions to use CloudWatch logs + signature checks. |
| src/logger/src/index.ts | Removes S3 debug-bucket write path and flushLogs. |
| src/logger/src/tests/index.test.ts | Removes S3 debug-bucket write tests and flushLogs usage. |
| package-lock.json | Locks new Lambda SDK client and related dependency updates. |
| lambdas/mock-webhook-lambda/src/index.ts | Adds HMAC signature verification and extra structured logging fields. |
| lambdas/mock-webhook-lambda/src/tests/index.test.ts | Adds coverage for signature verification and non-Error throws. |
| lambdas/client-transform-filter-lambda/src/services/observability.ts | Adds recordCallbackSigned to emit “Callback signed” logs. |
| lambdas/client-transform-filter-lambda/src/services/callback-logger.ts | Adds logCallbackSigned structured logger. |
| lambdas/client-transform-filter-lambda/src/index.ts | Removes flushLogs usage from handler wiring. |
| lambdas/client-transform-filter-lambda/src/handler.ts | Logs callback signature after signing and threads observability into signing. |
| lambdas/client-transform-filter-lambda/src/tests/services/callback-logger.test.ts | Adds tests for logCallbackSigned. |
| lambdas/client-transform-filter-lambda/src/tests/index.test.ts | Removes flushLogs mocking. |
| infrastructure/terraform/components/callbacks/variables.tf | Removes enable_debug_log_bucket variable. |
| infrastructure/terraform/components/callbacks/s3_bucket_debug_log.tf | Deleted: removes debug log bucket module/policy. |
| infrastructure/terraform/components/callbacks/outputs.tf | Removes debug bucket output. |
| infrastructure/terraform/components/callbacks/module_transform_filter_lambda.tf | Removes DEBUG_BUCKET_NAME env var and conditional S3 PutObject policy. |
| infrastructure/terraform/components/callbacks/module_mock_webhook_lambda.tf | Removes debug bucket usage; adds HMAC secret env var. |
| infrastructure/terraform/components/callbacks/README.md | Updates generated docs to remove debug bucket references. |
| .github/workflows/pr_destroy_dynamic_env.yml | Removes branch-name override (but leaves a trailing line continuation). |
Comments suppressed due to low confidence (1)
.github/workflows/pr_destroy_dynamic_env.yml:38
- The bash command ends with a trailing line-continuation (
\) after--overrideRoleName ..., but no subsequent argument is provided. If this workflow is enabled, this will cause a shell syntax error and the destroy dispatch won't run. Remove the trailing\or add the missing final argument back.
--targetAccountGroup "nhs-notify-client-callbacks-dev" \
--terraformAction "destroy" \
--overrideProjectName "nhs" \
--overrideRoleName "nhs-main-acct-client-callbacks-github-deploy" \
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cgitim
requested changes
Mar 24, 2026
cgitim
reviewed
Mar 24, 2026
cgitim
requested changes
Mar 24, 2026
cgitim
approved these changes
Mar 24, 2026
mjewildnhs
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Context
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.