From 552afcf9784f57ed09ebc3536d335b73e24cd2c5 Mon Sep 17 00:00:00 2001 From: bgagent Date: Mon, 10 Aug 2026 15:20:28 -0400 Subject: [PATCH 1/2] feat(observability): wire SNS notification action to DLQ CloudWatch alarms (#629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #117/#208: the DLQ-depth alarms shipped without an addAlarmAction. Add a reusable OperationalAlerts construct — a stack-wide, CMK-encrypted SNS topic — and wire all three DLQ-depth alarms (FanOut, ApprovalMetricsPublisher, GitHubScreenshot processor) to it via addAlarmAction(SnsAction). - Topic uses a customer-managed KMS key (not alias/aws/sns): CloudWatch cannot publish to an AWS-managed-key topic, so the key policy grants cloudwatch.amazonaws.com Decrypt/GenerateDataKey. Satisfies SNS2. - Delivery target is configurable via -c alertEmail=...; with none set the topic ships with no subscription and its ARN is exported for manual Slack/PagerDuty/email wiring. - Bootstrap deploy role: grant SNS topic + CMK create/lifecycle so a fresh cdk deploy can create these resources (bump bundle to 1.4.0, regenerate artifacts, update DEPLOYMENT_ROLES.md golden baseline). - Update CEDAR_HITL_GATES.md §11.5 + deferred note; regenerate Starlight. --- cdk/bootstrap/BOOTSTRAP_HASH | 2 +- cdk/bootstrap/BOOTSTRAP_VERSION | 2 +- cdk/bootstrap/bootstrap-template.yaml | 37 ++++- cdk/bootstrap/policies/application.json | 18 +++ cdk/bootstrap/policies/observability.json | 17 ++ cdk/src/bootstrap/policies/application.ts | 19 +++ cdk/src/bootstrap/policies/observability.ts | 23 +++ cdk/src/bootstrap/resource-action-map.ts | 4 + cdk/src/bootstrap/version.ts | 8 +- cdk/src/constructs/operational-alerts.ts | 153 ++++++++++++++++++ cdk/src/stacks/agent.ts | 30 +++- .../__snapshots__/version.test.ts.snap | 2 +- cdk/test/bootstrap/policies.test.ts | 3 + .../constructs/operational-alerts.test.ts | 140 ++++++++++++++++ cdk/test/stacks/agent.test.ts | 40 +++++ docs/design/CEDAR_HITL_GATES.md | 12 +- docs/design/DEPLOYMENT_ROLES.md | 37 ++++- .../docs/architecture/Cedar-hitl-gates.md | 12 +- .../docs/architecture/Deployment-roles.md | 37 ++++- 19 files changed, 574 insertions(+), 22 deletions(-) create mode 100644 cdk/src/constructs/operational-alerts.ts create mode 100644 cdk/test/constructs/operational-alerts.test.ts diff --git a/cdk/bootstrap/BOOTSTRAP_HASH b/cdk/bootstrap/BOOTSTRAP_HASH index 35b70e2bd..5dc0ef757 100644 --- a/cdk/bootstrap/BOOTSTRAP_HASH +++ b/cdk/bootstrap/BOOTSTRAP_HASH @@ -1 +1 @@ -40d0a8b2343663084f614423fc3e6210761377dedf0bc079956ba7d4cea84c5e +d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 diff --git a/cdk/bootstrap/BOOTSTRAP_VERSION b/cdk/bootstrap/BOOTSTRAP_VERSION index f0bb29e76..88c5fb891 100644 --- a/cdk/bootstrap/BOOTSTRAP_VERSION +++ b/cdk/bootstrap/BOOTSTRAP_VERSION @@ -1 +1 @@ -1.3.0 +1.4.0 diff --git a/cdk/bootstrap/bootstrap-template.yaml b/cdk/bootstrap/bootstrap-template.yaml index a2dcd08b4..e158ce5a5 100644 --- a/cdk/bootstrap/bootstrap-template.yaml +++ b/cdk/bootstrap/bootstrap-template.yaml @@ -1,7 +1,7 @@ # GENERATED FILE - DO NOT EDIT DIRECTLY # This template is generated by: npx tsx scripts/generate-bootstrap-template.ts -# ABCA Bootstrap Policy Version: 1.3.0 -# ABCA Bootstrap Policy Hash: 40d0a8b2343663084f614423fc3e6210761377dedf0bc079956ba7d4cea84c5e +# ABCA Bootstrap Policy Version: 1.4.0 +# ABCA Bootstrap Policy Hash: d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 # # Based on the default CDK bootstrap template with the following modifications: # - BootstrapVariant set to "ABCA: Least-Privilege Bootstrap" @@ -1119,6 +1119,21 @@ Resources: Effect: Allow Resource: arn:aws:sqs:*:*:backgroundagent-dev-* Sid: SQS + - Action: + - sns:CreateTopic + - sns:DeleteTopic + - sns:GetTopicAttributes + - sns:SetTopicAttributes + - sns:Subscribe + - sns:Unsubscribe + - sns:GetSubscriptionAttributes + - sns:ListSubscriptionsByTopic + - sns:TagResource + - sns:UntagResource + - sns:ListTagsForResource + Effect: Allow + Resource: arn:aws:sns:*:*:backgroundagent-dev-* + Sid: SNS - Action: - cloudfront:CreateDistribution - cloudfront:UpdateDistribution @@ -1269,6 +1284,20 @@ Resources: Effect: Allow Resource: '*' Sid: KMSForCDKAssets + - Action: + - kms:CreateKey + - kms:ScheduleKeyDeletion + - kms:PutKeyPolicy + - kms:GetKeyPolicy + - kms:GetKeyRotationStatus + - kms:EnableKeyRotation + - kms:DisableKeyRotation + - kms:TagResource + - kms:UntagResource + - kms:ListResourceTags + Effect: Allow + Resource: '*' + Sid: KMSCustomerManagedKeys - Action: - ecr:CreateRepository - ecr:DescribeRepositories @@ -1420,10 +1449,10 @@ Outputs: Value: '32' BootstrapPolicyVersion: Description: The version of the ABCA bootstrap policy bundle - Value: 1.3.0 + Value: 1.4.0 BootstrapPolicyHash: Description: SHA-256 hash of the ABCA bootstrap policy bundle for drift detection - Value: 40d0a8b2343663084f614423fc3e6210761377dedf0bc079956ba7d4cea84c5e + Value: d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 BootstrapPolicySet: Description: Comma-separated list of active ABCA bootstrap policy names Value: diff --git a/cdk/bootstrap/policies/application.json b/cdk/bootstrap/policies/application.json index 9f5932d89..20010171d 100644 --- a/cdk/bootstrap/policies/application.json +++ b/cdk/bootstrap/policies/application.json @@ -171,6 +171,24 @@ "Resource": "arn:aws:sqs:*:*:backgroundagent-dev-*", "Sid": "SQS" }, + { + "Action": [ + "sns:CreateTopic", + "sns:DeleteTopic", + "sns:GetTopicAttributes", + "sns:SetTopicAttributes", + "sns:Subscribe", + "sns:Unsubscribe", + "sns:GetSubscriptionAttributes", + "sns:ListSubscriptionsByTopic", + "sns:TagResource", + "sns:UntagResource", + "sns:ListTagsForResource" + ], + "Effect": "Allow", + "Resource": "arn:aws:sns:*:*:backgroundagent-dev-*", + "Sid": "SNS" + }, { "Action": [ "cloudfront:CreateDistribution", diff --git a/cdk/bootstrap/policies/observability.json b/cdk/bootstrap/policies/observability.json index 364a452ff..9c860f0f8 100644 --- a/cdk/bootstrap/policies/observability.json +++ b/cdk/bootstrap/policies/observability.json @@ -117,6 +117,23 @@ "Resource": "*", "Sid": "KMSForCDKAssets" }, + { + "Action": [ + "kms:CreateKey", + "kms:ScheduleKeyDeletion", + "kms:PutKeyPolicy", + "kms:GetKeyPolicy", + "kms:GetKeyRotationStatus", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:TagResource", + "kms:UntagResource", + "kms:ListResourceTags" + ], + "Effect": "Allow", + "Resource": "*", + "Sid": "KMSCustomerManagedKeys" + }, { "Action": [ "ecr:CreateRepository", diff --git a/cdk/src/bootstrap/policies/application.ts b/cdk/src/bootstrap/policies/application.ts index 88b668628..9171c06f7 100644 --- a/cdk/src/bootstrap/policies/application.ts +++ b/cdk/src/bootstrap/policies/application.ts @@ -214,6 +214,25 @@ export function applicationPolicy(): iam.PolicyDocument { resources: ['arn:aws:sqs:*:*:backgroundagent-dev-*'], }), + new iam.PolicyStatement({ + sid: 'SNS', + effect: iam.Effect.ALLOW, + actions: [ + 'sns:CreateTopic', + 'sns:DeleteTopic', + 'sns:GetTopicAttributes', + 'sns:SetTopicAttributes', + 'sns:Subscribe', + 'sns:Unsubscribe', + 'sns:GetSubscriptionAttributes', + 'sns:ListSubscriptionsByTopic', + 'sns:TagResource', + 'sns:UntagResource', + 'sns:ListTagsForResource', + ], + resources: ['arn:aws:sns:*:*:backgroundagent-dev-*'], + }), + new iam.PolicyStatement({ sid: 'CloudFront', effect: iam.Effect.ALLOW, diff --git a/cdk/src/bootstrap/policies/observability.ts b/cdk/src/bootstrap/policies/observability.ts index 70356bd89..c8fafdb76 100644 --- a/cdk/src/bootstrap/policies/observability.ts +++ b/cdk/src/bootstrap/policies/observability.ts @@ -154,6 +154,29 @@ export function observabilityPolicy(): iam.PolicyDocument { resources: ['*'], }), + new iam.PolicyStatement({ + // Customer-managed keys created BY the stack (e.g. the + // OperationalAlerts SNS topic key, issue #629), distinct from + // the CDK bootstrap asset key used above. kms:CreateKey cannot + // be resource-scoped (the key ARN does not exist yet) and CMK + // ARNs are UUIDs, so this statement is unavoidably `*`. + sid: 'KMSCustomerManagedKeys', + effect: iam.Effect.ALLOW, + actions: [ + 'kms:CreateKey', + 'kms:ScheduleKeyDeletion', + 'kms:PutKeyPolicy', + 'kms:GetKeyPolicy', + 'kms:GetKeyRotationStatus', + 'kms:EnableKeyRotation', + 'kms:DisableKeyRotation', + 'kms:TagResource', + 'kms:UntagResource', + 'kms:ListResourceTags', + ], + resources: ['*'], + }), + new iam.PolicyStatement({ sid: 'ECRForDockerAssets', effect: iam.Effect.ALLOW, diff --git a/cdk/src/bootstrap/resource-action-map.ts b/cdk/src/bootstrap/resource-action-map.ts index cb117290f..bda726eda 100644 --- a/cdk/src/bootstrap/resource-action-map.ts +++ b/cdk/src/bootstrap/resource-action-map.ts @@ -34,6 +34,7 @@ export const CFN_TYPES_WITHOUT_EXEC_ROLE_IAM = new Set([ 'AWS::ApiGateway::Stage', 'AWS::ApiGateway::Account', 'AWS::S3::BucketPolicy', + 'AWS::SNS::TopicPolicy', 'AWS::SQS::QueuePolicy', 'AWS::EC2::VPCGatewayAttachment', 'AWS::EC2::SubnetRouteTableAssociation', @@ -77,6 +78,7 @@ export const RESOURCE_ACTION_MAP: Record = { 'AWS::Events::Rule': ['events:PutRule'], 'AWS::IAM::Policy': ['iam:CreatePolicy', 'iam:PutRolePolicy'], 'AWS::IAM::Role': ['iam:CreateRole'], + 'AWS::KMS::Key': ['kms:CreateKey'], 'AWS::Lambda::EventInvokeConfig': ['lambda:PutFunctionEventInvokeConfig'], 'AWS::Lambda::EventSourceMapping': ['lambda:CreateEventSourceMapping'], 'AWS::Lambda::Function': ['lambda:CreateFunction'], @@ -98,6 +100,8 @@ export const RESOURCE_ACTION_MAP: Record = { 'AWS::Route53Resolver::ResolverQueryLoggingConfigAssociation': ['route53resolver:AssociateResolverQueryLogConfig'], 'AWS::S3::Bucket': ['s3:CreateBucket'], 'AWS::SecretsManager::Secret': ['secretsmanager:CreateSecret'], + 'AWS::SNS::Subscription': ['sns:Subscribe'], + 'AWS::SNS::Topic': ['sns:CreateTopic'], 'AWS::SQS::Queue': ['sqs:CreateQueue'], 'AWS::WAFv2::WebACL': ['wafv2:CreateWebACL'], 'AWS::WAFv2::WebACLAssociation': ['wafv2:AssociateWebACL'], diff --git a/cdk/src/bootstrap/version.ts b/cdk/src/bootstrap/version.ts index 17ad37ae7..153828a43 100644 --- a/cdk/src/bootstrap/version.ts +++ b/cdk/src/bootstrap/version.ts @@ -26,10 +26,12 @@ import { allPolicies } from './policies'; * * Bump history: 1.0.0 → 1.1.0 added the `compute-ecs` policy (#162), 1.1.0 → * 1.2.0 refreshed policies for a full deploy (#350), 1.2.0 → 1.3.0 adds the - * `compute-lambda-microvm` policy (#645 / ADR-021). Adding a policy to the - * bundle is a minor bump — that is the precedent `compute-ecs` set. + * `compute-lambda-microvm` policy (#645 / ADR-021), 1.3.0 → 1.4.0 grants SNS + * topic + customer-managed-KMS-key create/lifecycle for the OperationalAlerts + * notification channel (#629). Expanding the granted action set is a minor + * bump — that is the precedent the #350 policy refresh set. */ -export const BOOTSTRAP_VERSION = '1.3.0'; +export const BOOTSTRAP_VERSION = '1.4.0'; /** * Computes a SHA-256 hash over all bootstrap policies. diff --git a/cdk/src/constructs/operational-alerts.ts b/cdk/src/constructs/operational-alerts.ts new file mode 100644 index 000000000..ae45d677a --- /dev/null +++ b/cdk/src/constructs/operational-alerts.ts @@ -0,0 +1,153 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { RemovalPolicy } from 'aws-cdk-lib'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as cloudwatchActions from 'aws-cdk-lib/aws-cloudwatch-actions'; +import * as iam from 'aws-cdk-lib/aws-iam'; +import * as kms from 'aws-cdk-lib/aws-kms'; +import * as sns from 'aws-cdk-lib/aws-sns'; +import * as subscriptions from 'aws-cdk-lib/aws-sns-subscriptions'; +import { NagSuppressions } from 'cdk-nag'; +import { Construct } from 'constructs'; + +/** + * Properties for the ``OperationalAlerts`` construct. + */ +export interface OperationalAlertsProps { + /** + * Optional email address to subscribe to the alerts topic. When set, + * an SNS email subscription is created and AWS sends a confirmation + * link the operator must click before delivery starts. When omitted, + * the topic ships with no subscriptions — operators wire Slack / + * PagerDuty / email manually against {@link OperationalAlerts.topic} + * (exported as a stack output by the caller). + * + * Delivery target is configurable (stack context / Blueprint prop) + * rather than hard-coded per issue #629. + */ + readonly alertEmail?: string; + + /** + * Removal policy for the topic's customer-managed KMS key. + * @default RemovalPolicy.DESTROY + */ + readonly removalPolicy?: RemovalPolicy; +} + +/** + * Reusable operational notification channel: a single SNS topic that + * CloudWatch alarms publish to via ``addAlarmAction`` (issue #629, + * follow-up to the DLQ-depth alarms shipped in #117 / §11.5). + * + * **Encryption.** The topic is encrypted with a customer-managed KMS + * key rather than the AWS-managed ``alias/aws/sns`` key. This is + * load-bearing, not decorative: CloudWatch cannot publish to a topic + * encrypted with the AWS-managed key because that key's policy can't be + * edited to grant the ``cloudwatch.amazonaws.com`` service principal + * ``kms:GenerateDataKey*`` / ``kms:Decrypt``. The alarm→SNS action would + * fail silently at delivery time. The CMK below grants CloudWatch (and + * SNS) exactly those actions so delivery works while keeping + * encryption-at-rest and satisfying cdk-nag ``AwsSolutions-SNS2``. + * + * The topic is intentionally stack-wide (not per-consumer) so every + * DLQ-depth alarm shares one subscription surface — an operator + * confirms one email / wires one Slack endpoint and receives all + * operational alerts. + */ +export class OperationalAlerts extends Construct { + /** The shared alerts topic. Pass to ``addAlarmActions`` or attach + * additional subscriptions (Slack / PagerDuty) downstream. */ + public readonly topic: sns.Topic; + + /** Customer-managed key encrypting the topic. Exposed for tests and + * for any downstream resource that must publish to the topic. */ + public readonly key: kms.Key; + + constructor(scope: Construct, id: string, props: OperationalAlertsProps = {}) { + super(scope, id); + + const removalPolicy = props.removalPolicy ?? RemovalPolicy.DESTROY; + + // Customer-managed key so the CloudWatch service principal can be + // granted decrypt/data-key rights (see class doc). Rotation on by + // default — no reason not to for a low-throughput alerts key. + this.key = new kms.Key(this, 'Key', { + description: 'Encrypts the ABCA operational-alerts SNS topic (DLQ-depth alarms → operators)', + enableKeyRotation: true, + removalPolicy, + }); + + // Allow CloudWatch Alarms to publish through the encrypted topic. + // Without these grants the alarm action resolves at deploy time but + // every publish fails at runtime with a KMS AccessDenied the + // operator never sees. + this.key.addToResourcePolicy(new iam.PolicyStatement({ + sid: 'AllowCloudWatchAlarmsUseOfKey', + principals: [new iam.ServicePrincipal('cloudwatch.amazonaws.com')], + actions: ['kms:Decrypt', 'kms:GenerateDataKey*'], + resources: ['*'], + })); + + this.topic = new sns.Topic(this, 'Topic', { + displayName: 'ABCA operational alerts', + masterKey: this.key, + }); + + // SNS delivery is best-effort operational metadata; require callers + // to publish over TLS regardless. + this.topic.addToResourcePolicy(new iam.PolicyStatement({ + sid: 'DenyInsecureTransport', + effect: iam.Effect.DENY, + principals: [new iam.AnyPrincipal()], + actions: ['sns:Publish'], + resources: [this.topic.topicArn], + conditions: { Bool: { 'aws:SecureTransport': 'false' } }, + })); + + if (props.alertEmail) { + this.topic.addSubscription(new subscriptions.EmailSubscription(props.alertEmail)); + } + + NagSuppressions.addResourceSuppressions(this.topic, [ + { + id: 'AwsSolutions-SNS3', + reason: + 'Topic-wide SSL enforcement is applied via an explicit DenyInsecureTransport ' + + 'resource policy statement (aws:SecureTransport=false) rather than the L2 default.', + }, + ]); + } + + /** + * Wire one or more alarms to publish to the alerts topic on state + * change. Takes the concrete {@link cloudwatch.Alarm} because + * ``addAlarmAction`` is declared there, not on the ``IAlarm`` + * interface — which is precisely why the FanOut / + * ApprovalMetricsPublisher / screenshot DLQ-depth alarms are exposed + * as ``Alarm``. Those constructs stay decoupled from this one: the + * caller passes their alarms in. + */ + public addAlarmActions(...alarms: cloudwatch.Alarm[]): void { + const action = new cloudwatchActions.SnsAction(this.topic); + for (const alarm of alarms) { + alarm.addAlarmAction(action); + } + } +} diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 41c0351e0..594818298 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -53,6 +53,7 @@ import { type LambdaMicrovmImageInputs, } from '../constructs/lambda-microvm-compute'; import { LinearIntegration } from '../constructs/linear-integration'; +import { OperationalAlerts } from '../constructs/operational-alerts'; import { OrchestrationReconciler } from '../constructs/orchestration-reconciler'; import { OrchestrationTable } from '../constructs/orchestration-table'; import { PendingUploadCleanup } from '../constructs/pending-upload-cleanup'; @@ -960,7 +961,7 @@ export class AgentStack extends Stack { // 2-consumer architectural note in `task-events-table.ts` — // adding a third consumer here requires the Kinesis Data Streams // for DynamoDB migration. - new ApprovalMetricsPublisherConsumer(this, 'ApprovalMetricsPublisherConsumer', { + const approvalMetricsPublisher = new ApprovalMetricsPublisherConsumer(this, 'ApprovalMetricsPublisherConsumer', { taskEventsTable: taskEventsTable.table, }); @@ -1408,7 +1409,7 @@ export class AgentStack extends Stack { // ``bgagent/slack/*``; Linear dispatcher posts a single // deterministic final-status comment with cost/turns/duration. // Email remains a log-only stub until SES wires. - new FanOutConsumer(this, 'FanOutConsumer', { + const fanOutConsumer = new FanOutConsumer(this, 'FanOutConsumer', { taskEventsTable: taskEventsTable.table, taskTable: taskTable.table, repoTable: repoTable.table, @@ -1480,6 +1481,31 @@ export class AgentStack extends Stack { // (those are WAF-blocked by the API's managed rule set anyway), so there // is no RestackProcessor Lambda to wire here. + // --- Operational alerts channel (§11.5 follow-up, issue #629) --- + // A single stack-wide SNS topic that the DLQ-depth alarms publish to + // on state change, so poison-pill accumulation pushes a notification + // instead of sitting silently in the Alarms console. Delivery target + // is configurable: pass an email via `-c alertEmail=ops@example.com` + // (AWS sends a confirmation link that must be clicked), or leave it + // unset and subscribe Slack / PagerDuty manually against the exported + // topic ARN below. + const operationalAlerts = new OperationalAlerts(this, 'OperationalAlerts', { + alertEmail: this.node.tryGetContext('alertEmail') as string | undefined, + }); + // Wire the DLQ-depth alarms shipped in #117 (FanOut + approval-metrics + // publisher) plus the screenshot processor's async-invoke DLQ alarm — + // all three share the threshold-1 "records landed in a DLQ" shape. + operationalAlerts.addAlarmActions( + fanOutConsumer.dlqDepthAlarm, + approvalMetricsPublisher.dlqAlarm, + githubScreenshot.processorDlqDepthAlarm, + ); + + new CfnOutput(this, 'OperationalAlertsTopicArn', { + value: operationalAlerts.topic.topicArn, + description: 'SNS topic for DLQ-depth CloudWatch alarms — subscribe Slack / PagerDuty / email here (#629)', + }); + new CfnOutput(this, 'GitHubWebhookUrl', { value: `${taskApi.api.url}github/webhook`, description: 'URL to configure as the GitHub webhook target on demo repos (deployment_status events)', diff --git a/cdk/test/bootstrap/__snapshots__/version.test.ts.snap b/cdk/test/bootstrap/__snapshots__/version.test.ts.snap index f8b1a2e7e..e8e8bb8e2 100644 --- a/cdk/test/bootstrap/__snapshots__/version.test.ts.snap +++ b/cdk/test/bootstrap/__snapshots__/version.test.ts.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing -exports[`bootstrap version module hash is stable 1`] = `"40d0a8b2343663084f614423fc3e6210761377dedf0bc079956ba7d4cea84c5e"`; +exports[`bootstrap version module hash is stable 1`] = `"d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77"`; diff --git a/cdk/test/bootstrap/policies.test.ts b/cdk/test/bootstrap/policies.test.ts index e3cdae579..133e4f1f9 100644 --- a/cdk/test/bootstrap/policies.test.ts +++ b/cdk/test/bootstrap/policies.test.ts @@ -112,6 +112,7 @@ describe('IaCRole-ABCA-Application', () => { 'WAFv2', 'EventBridge', 'SQS', + 'SNS', 'CloudFront', 'SecretsManager', 'SecretsManagerAccountLevel', @@ -144,6 +145,7 @@ describe('IaCRole-ABCA-Application', () => { 'events', 'lambda', 'secretsmanager', + 'sns', 'sqs', 'wafv2', ]), @@ -239,6 +241,7 @@ describe('IaCRole-ABCA-Observability', () => { 'S3CDKAssets', 'S3ApplicationBuckets', 'KMSForCDKAssets', + 'KMSCustomerManagedKeys', 'ECRForDockerAssets', 'ECRAuthToken', 'XRay', diff --git a/cdk/test/constructs/operational-alerts.test.ts b/cdk/test/constructs/operational-alerts.test.ts new file mode 100644 index 000000000..986baad87 --- /dev/null +++ b/cdk/test/constructs/operational-alerts.test.ts @@ -0,0 +1,140 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { App, Duration, Stack } from 'aws-cdk-lib'; +import { Match, Template } from 'aws-cdk-lib/assertions'; +import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; +import * as sqs from 'aws-cdk-lib/aws-sqs'; +import { OperationalAlerts } from '../../src/constructs/operational-alerts'; + +/** Builds a throwaway DLQ-depth alarm to exercise ``addAlarmActions``. */ +function makeDlqAlarm(stack: Stack, id: string): cloudwatch.Alarm { + const queue = new sqs.Queue(stack, `${id}Queue`); + return new cloudwatch.Alarm(stack, `${id}Alarm`, { + metric: queue.metricApproximateNumberOfMessagesVisible({ + period: Duration.minutes(5), + statistic: 'Maximum', + }), + threshold: 1, + evaluationPeriods: 1, + comparisonOperator: cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD, + }); +} + +describe('OperationalAlerts', () => { + test('creates a KMS-encrypted SNS topic with key rotation', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts'); + const template = Template.fromStack(stack); + + template.resourceCountIs('AWS::SNS::Topic', 1); + template.resourceCountIs('AWS::KMS::Key', 1); + // The topic must reference the CMK (not the AWS-managed key) so + // CloudWatch can be granted publish/decrypt rights on it. + template.hasResourceProperties('AWS::SNS::Topic', { + KmsMasterKeyId: Match.anyValue(), + }); + template.hasResourceProperties('AWS::KMS::Key', { + EnableKeyRotation: true, + }); + }); + + test('CMK key policy grants CloudWatch decrypt + generate-data-key', () => { + // Load-bearing: CloudWatch cannot deliver to a topic on the + // AWS-managed key. Without this grant the alarm action deploys but + // every publish fails at runtime with KMS AccessDenied. + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts'); + const template = Template.fromStack(stack); + + template.hasResourceProperties('AWS::KMS::Key', { + KeyPolicy: { + Statement: Match.arrayWith([ + Match.objectLike({ + Principal: { Service: 'cloudwatch.amazonaws.com' }, + Action: Match.arrayWith(['kms:Decrypt', 'kms:GenerateDataKey*']), + }), + ]), + }, + }); + }); + + test('enforces TLS on publish via a DenyInsecureTransport topic policy', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts'); + const template = Template.fromStack(stack); + + template.hasResourceProperties('AWS::SNS::TopicPolicy', { + PolicyDocument: { + Statement: Match.arrayWith([ + Match.objectLike({ + Effect: 'Deny', + Action: 'sns:Publish', + Condition: { Bool: { 'aws:SecureTransport': 'false' } }, + }), + ]), + }, + }); + }); + + test('creates no subscription when alertEmail is omitted', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts'); + const template = Template.fromStack(stack); + + template.resourceCountIs('AWS::SNS::Subscription', 0); + }); + + test('creates an email subscription when alertEmail is provided', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts', { alertEmail: 'ops@example.com' }); + const template = Template.fromStack(stack); + + template.hasResourceProperties('AWS::SNS::Subscription', { + Protocol: 'email', + Endpoint: 'ops@example.com', + }); + }); + + test('addAlarmActions wires each alarm to publish to the topic', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + const alerts = new OperationalAlerts(stack, 'Alerts'); + const a1 = makeDlqAlarm(stack, 'One'); + const a2 = makeDlqAlarm(stack, 'Two'); + alerts.addAlarmActions(a1, a2); + const template = Template.fromStack(stack); + + // Both alarms carry an AlarmActions entry pointing at the topic. + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + const withActions = Object.values(alarms).filter( + (r: any) => Array.isArray(r.Properties?.AlarmActions) && r.Properties.AlarmActions.length > 0, + ); + expect(withActions.length).toBe(2); + for (const alarm of withActions) { + // The action references the SNS topic (Ref to the topic logical id). + expect(JSON.stringify((alarm as any).Properties.AlarmActions)).toContain('AlertsTopic'); + } + }); +}); diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 19fb077b5..c0c9b4701 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -670,6 +670,46 @@ describe('AgentStack', () => { ]), }); }); + + test('provisions a single OperationalAlerts SNS topic + CMK and exports its ARN (#629)', () => { + // One stack-wide topic, not per-consumer — every DLQ-depth alarm + // shares one subscription surface. + template.resourceCountIs('AWS::SNS::Topic', 1); + template.hasResourceProperties('AWS::SNS::Topic', { + KmsMasterKeyId: Match.anyValue(), + }); + template.hasOutput('OperationalAlertsTopicArn', { + Description: Match.stringLikeRegexp('#629'), + }); + }); + + test('wires all three DLQ-depth alarms to the alerts topic (#629)', () => { + // FanOut, ApprovalMetricsPublisher, and the screenshot processor + // DLQ alarms must each carry an AlarmActions entry — otherwise a + // poison-pill pile-up stays silent (the whole point of #629). + const alarms = template.findResources('AWS::CloudWatch::Alarm'); + const dlqAlarmsWithActions = Object.values(alarms).filter((r: any) => { + const dims: Array<{ Name: string }> = r.Properties?.Dimensions ?? []; + const isSqsDepth = + r.Properties?.Namespace === 'AWS/SQS' && + r.Properties?.MetricName === 'ApproximateNumberOfMessagesVisible' && + dims.some((d) => d.Name === 'QueueName'); + const hasActions = + Array.isArray(r.Properties?.AlarmActions) && r.Properties.AlarmActions.length > 0; + return isSqsDepth && hasActions; + }); + expect(dlqAlarmsWithActions).toHaveLength(3); + // Each action must reference the operational-alerts topic. + for (const alarm of dlqAlarmsWithActions) { + expect(JSON.stringify((alarm as any).Properties.AlarmActions)).toContain('OperationalAlerts'); + } + }); + + test('does NOT subscribe an email when no alertEmail context is set', () => { + // The default deploy ships the topic with no confirmed target; + // operators subscribe Slack / PagerDuty / email themselves. + template.resourceCountIs('AWS::SNS::Subscription', 0); + }); }); describe('AgentStack with the ECS substrate gate (--context compute_type=ecs)', () => { diff --git a/docs/design/CEDAR_HITL_GATES.md b/docs/design/CEDAR_HITL_GATES.md index 8b9ee88f8..bafbed104 100644 --- a/docs/design/CEDAR_HITL_GATES.md +++ b/docs/design/CEDAR_HITL_GATES.md @@ -1590,12 +1590,16 @@ Every `agent_milestone("approval_*")` event carries `trace_id` / `span_id`. A sp - **FanOutConsumer DLQ** — poison-pill DynamoDB Stream records that failed three consecutive Lambda invocations. - **ApprovalMetricsPublisher DLQ** — same failure mode for the metrics-publisher consumer. +- **GitHubScreenshotIntegration processor DLQ** — failed async invocations of the screenshot pipeline (same threshold-1 shape). -These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss. They ship **without an `addAlarmAction` / SNS notification target** — operators must check the CloudWatch Alarms console or configure a subscription manually. This is an intentional intermediate step: alarm state is durable and queryable even without push notifications, and prevents poison records from accumulating silently for the full 14-day DLQ retention window. +These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss, and prevent poison records from accumulating silently for the full 14-day DLQ retention window. -**Follow-up — notification channel wiring:** Once an operational notification channel (SNS topic → Slack / PagerDuty / email) is provisioned, add `alarm.addAlarmAction(new SnsAction(topic))` to both alarms. No metric or alarm restructuring is needed. +**Notification channel wiring (shipped, issue #629):** All three DLQ-depth alarms above are wired to a stack-wide SNS topic via `alarm.addAlarmAction(new SnsAction(topic))`. The topic is provisioned by the reusable `OperationalAlerts` construct (`cdk/src/constructs/operational-alerts.ts`) and its ARN is exported as the `OperationalAlertsTopicArn` stack output. -**Additional alarms (not yet shipped):** The following remain deferred until the notification channel exists (alarm-without-action provides limited value for rate/latency conditions that require human triage): +- **Encryption.** The topic is encrypted with a **customer-managed KMS key**, not the AWS-managed `alias/aws/sns` key. This is load-bearing: CloudWatch cannot publish to a topic on the AWS-managed key (its key policy can't be edited to grant the `cloudwatch.amazonaws.com` service principal `kms:Decrypt` / `kms:GenerateDataKey*`), so the alarm action would fail silently at delivery. The CMK grants CloudWatch exactly those actions. +- **Delivery target (configurable).** Pass `-c alertEmail=ops@example.com` at deploy to create an email subscription (AWS sends a confirmation link that must be clicked). With no context set, the topic ships with no subscription — operators subscribe Slack / PagerDuty / email manually against the exported topic ARN. Delivery is not hard-coded. + +**Additional alarms (not yet shipped):** The following remain deferred (each is a rate/latency condition that needs metric-math or composite-alarm design beyond the threshold-1 DLQ shape); now that the notification channel exists they can be wired to the same `OperationalAlerts` topic as separate follow-ups: - High approval-timeout rate (users not responding, notifications broken) - Tasks stuck in AWAITING_APPROVAL beyond `timeout_s + 60s` (reconciler failure) @@ -2108,7 +2112,7 @@ See §17.18 for the off-hours escalation future-work primitive, and §13.14 for **Future work — polish (tracked in §17):** - CLI inline streaming prompt (UX research first) - `approve --defer` / allowlist revocation (`bgagent revoke-approval`) -- CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned +- ~~CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned~~ — **shipped (issue #629):** all three DLQ-depth alarms are wired to the `OperationalAlerts` SNS topic (§11.5) - More soft-deny policies in the default set based on real usage - Persistent recent-decision cache (if container-restart telemetry justifies it) - Persistent per-minute rate limit (if restart amplification becomes significant) diff --git a/docs/design/DEPLOYMENT_ROLES.md b/docs/design/DEPLOYMENT_ROLES.md index 8a279edc8..212f964de 100644 --- a/docs/design/DEPLOYMENT_ROLES.md +++ b/docs/design/DEPLOYMENT_ROLES.md @@ -22,7 +22,7 @@ The policies are split into six IAM managed policies (each under the 6,144-chara | Policy Name | Scope | When applied | |-------------|-------|--------------| | `IaCRole-ABCA-Infrastructure` | CloudFormation, IAM, VPC networking, Route 53 Resolver DNS Firewall | Always | -| `IaCRole-ABCA-Application` | DynamoDB, Lambda, API Gateway, Cognito, WAFv2, EventBridge, SQS, CloudFront, Secrets Manager | Always | +| `IaCRole-ABCA-Application` | DynamoDB, Lambda, API Gateway, Cognito, WAFv2, EventBridge, SQS, SNS, CloudFront, Secrets Manager | Always | | `IaCRole-ABCA-Observability` | Bedrock Guardrails, CloudWatch, X-Ray, S3, ECR, KMS, SSM, STS | Always | | `IaCRole-ABCA-Compute-Agentcore` | Bedrock AgentCore (`bedrock-agentcore:*`) | Always (default compute backend) | | `IaCRole-ABCA-Compute-ECS` | ECS cluster + task-definition operations | Only when `ecs` is in `ComputeTypes` | @@ -450,6 +450,24 @@ DynamoDB tables, Lambda functions, API Gateway, Cognito, WAFv2, EventBridge, SQS ], "Resource": "arn:aws:sqs:*:*:backgroundagent-dev-*" }, + { + "Sid": "SNS", + "Effect": "Allow", + "Action": [ + "sns:CreateTopic", + "sns:DeleteTopic", + "sns:GetTopicAttributes", + "sns:SetTopicAttributes", + "sns:Subscribe", + "sns:Unsubscribe", + "sns:GetSubscriptionAttributes", + "sns:ListSubscriptionsByTopic", + "sns:TagResource", + "sns:UntagResource", + "sns:ListTagsForResource" + ], + "Resource": "arn:aws:sns:*:*:backgroundagent-dev-*" + }, { "Sid": "CloudFront", "Effect": "Allow", @@ -639,6 +657,23 @@ Bedrock Guardrails, CloudWatch Logs/Dashboards/Alarms, X-Ray, S3 (CDK assets), K ], "Resource": "*" }, + { + "Sid": "KMSCustomerManagedKeys", + "Effect": "Allow", + "Action": [ + "kms:CreateKey", + "kms:ScheduleKeyDeletion", + "kms:PutKeyPolicy", + "kms:GetKeyPolicy", + "kms:GetKeyRotationStatus", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:TagResource", + "kms:UntagResource", + "kms:ListResourceTags" + ], + "Resource": "*" + }, { "Sid": "ECRForDockerAssets", "Effect": "Allow", diff --git a/docs/src/content/docs/architecture/Cedar-hitl-gates.md b/docs/src/content/docs/architecture/Cedar-hitl-gates.md index 67ef505b7..b54af4b52 100644 --- a/docs/src/content/docs/architecture/Cedar-hitl-gates.md +++ b/docs/src/content/docs/architecture/Cedar-hitl-gates.md @@ -1594,12 +1594,16 @@ Every `agent_milestone("approval_*")` event carries `trace_id` / `span_id`. A sp - **FanOutConsumer DLQ** — poison-pill DynamoDB Stream records that failed three consecutive Lambda invocations. - **ApprovalMetricsPublisher DLQ** — same failure mode for the metrics-publisher consumer. +- **GitHubScreenshotIntegration processor DLQ** — failed async invocations of the screenshot pipeline (same threshold-1 shape). -These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss. They ship **without an `addAlarmAction` / SNS notification target** — operators must check the CloudWatch Alarms console or configure a subscription manually. This is an intentional intermediate step: alarm state is durable and queryable even without push notifications, and prevents poison records from accumulating silently for the full 14-day DLQ retention window. +These alarms transition to `ALARM` state in CloudWatch and appear in the console/dashboard, providing operator visibility into silent record loss, and prevent poison records from accumulating silently for the full 14-day DLQ retention window. -**Follow-up — notification channel wiring:** Once an operational notification channel (SNS topic → Slack / PagerDuty / email) is provisioned, add `alarm.addAlarmAction(new SnsAction(topic))` to both alarms. No metric or alarm restructuring is needed. +**Notification channel wiring (shipped, issue #629):** All three DLQ-depth alarms above are wired to a stack-wide SNS topic via `alarm.addAlarmAction(new SnsAction(topic))`. The topic is provisioned by the reusable `OperationalAlerts` construct (`cdk/src/constructs/operational-alerts.ts`) and its ARN is exported as the `OperationalAlertsTopicArn` stack output. -**Additional alarms (not yet shipped):** The following remain deferred until the notification channel exists (alarm-without-action provides limited value for rate/latency conditions that require human triage): +- **Encryption.** The topic is encrypted with a **customer-managed KMS key**, not the AWS-managed `alias/aws/sns` key. This is load-bearing: CloudWatch cannot publish to a topic on the AWS-managed key (its key policy can't be edited to grant the `cloudwatch.amazonaws.com` service principal `kms:Decrypt` / `kms:GenerateDataKey*`), so the alarm action would fail silently at delivery. The CMK grants CloudWatch exactly those actions. +- **Delivery target (configurable).** Pass `-c alertEmail=ops@example.com` at deploy to create an email subscription (AWS sends a confirmation link that must be clicked). With no context set, the topic ships with no subscription — operators subscribe Slack / PagerDuty / email manually against the exported topic ARN. Delivery is not hard-coded. + +**Additional alarms (not yet shipped):** The following remain deferred (each is a rate/latency condition that needs metric-math or composite-alarm design beyond the threshold-1 DLQ shape); now that the notification channel exists they can be wired to the same `OperationalAlerts` topic as separate follow-ups: - High approval-timeout rate (users not responding, notifications broken) - Tasks stuck in AWAITING_APPROVAL beyond `timeout_s + 60s` (reconciler failure) @@ -2112,7 +2116,7 @@ See §17.18 for the off-hours escalation future-work primitive, and §13.14 for **Future work — polish (tracked in §17):** - CLI inline streaming prompt (UX research first) - `approve --defer` / allowlist revocation (`bgagent revoke-approval`) -- CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned +- ~~CloudWatch alarm SNS notification wiring (§11.5) — DLQ-depth alarms ship without an action target; add `SnsAction` once a notification channel is provisioned~~ — **shipped (issue #629):** all three DLQ-depth alarms are wired to the `OperationalAlerts` SNS topic (§11.5) - More soft-deny policies in the default set based on real usage - Persistent recent-decision cache (if container-restart telemetry justifies it) - Persistent per-minute rate limit (if restart amplification becomes significant) diff --git a/docs/src/content/docs/architecture/Deployment-roles.md b/docs/src/content/docs/architecture/Deployment-roles.md index cdce53038..80c82304e 100644 --- a/docs/src/content/docs/architecture/Deployment-roles.md +++ b/docs/src/content/docs/architecture/Deployment-roles.md @@ -26,7 +26,7 @@ The policies are split into six IAM managed policies (each under the 6,144-chara | Policy Name | Scope | When applied | |-------------|-------|--------------| | `IaCRole-ABCA-Infrastructure` | CloudFormation, IAM, VPC networking, Route 53 Resolver DNS Firewall | Always | -| `IaCRole-ABCA-Application` | DynamoDB, Lambda, API Gateway, Cognito, WAFv2, EventBridge, SQS, CloudFront, Secrets Manager | Always | +| `IaCRole-ABCA-Application` | DynamoDB, Lambda, API Gateway, Cognito, WAFv2, EventBridge, SQS, SNS, CloudFront, Secrets Manager | Always | | `IaCRole-ABCA-Observability` | Bedrock Guardrails, CloudWatch, X-Ray, S3, ECR, KMS, SSM, STS | Always | | `IaCRole-ABCA-Compute-Agentcore` | Bedrock AgentCore (`bedrock-agentcore:*`) | Always (default compute backend) | | `IaCRole-ABCA-Compute-ECS` | ECS cluster + task-definition operations | Only when `ecs` is in `ComputeTypes` | @@ -454,6 +454,24 @@ DynamoDB tables, Lambda functions, API Gateway, Cognito, WAFv2, EventBridge, SQS ], "Resource": "arn:aws:sqs:*:*:backgroundagent-dev-*" }, + { + "Sid": "SNS", + "Effect": "Allow", + "Action": [ + "sns:CreateTopic", + "sns:DeleteTopic", + "sns:GetTopicAttributes", + "sns:SetTopicAttributes", + "sns:Subscribe", + "sns:Unsubscribe", + "sns:GetSubscriptionAttributes", + "sns:ListSubscriptionsByTopic", + "sns:TagResource", + "sns:UntagResource", + "sns:ListTagsForResource" + ], + "Resource": "arn:aws:sns:*:*:backgroundagent-dev-*" + }, { "Sid": "CloudFront", "Effect": "Allow", @@ -643,6 +661,23 @@ Bedrock Guardrails, CloudWatch Logs/Dashboards/Alarms, X-Ray, S3 (CDK assets), K ], "Resource": "*" }, + { + "Sid": "KMSCustomerManagedKeys", + "Effect": "Allow", + "Action": [ + "kms:CreateKey", + "kms:ScheduleKeyDeletion", + "kms:PutKeyPolicy", + "kms:GetKeyPolicy", + "kms:GetKeyRotationStatus", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:TagResource", + "kms:UntagResource", + "kms:ListResourceTags" + ], + "Resource": "*" + }, { "Sid": "ECRForDockerAssets", "Effect": "Allow", From 26224092de05b44c6d73f9dc33c1c94a7732fd34 Mon Sep 17 00:00:00 2001 From: bgagent Date: Mon, 10 Aug 2026 16:22:11 -0400 Subject: [PATCH 2/2] fix(observability): harden OperationalAlerts per PR #739 review (#629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review findings on the DLQ-alarm SNS wiring: - Security (confused-deputy): pin aws:SourceAccount on the CloudWatch→KMS key grant so an alarm in another account can't induce this key's use, matching the lambda-microvm-compute precedent. - Security (deploy-role blast radius): split KMSCustomerManagedKeys — CreateKey/read/tag stay on * (unscopable), but PutKeyPolicy/ ScheduleKeyDeletion/rotation are now gated on the ABCA=operational-alerts resource tag the construct stamps on its key, so the CFN deploy role can no longer take over or delete unrelated account CMKs. - removalPolicy now applies to the topic as well as the key (shared lifecycle); prop doc widened accordingly. - Validate alertEmail at synth — throw on malformed input instead of shipping a permanently-unconfirmed subscription. - Comments: drop the inert AwsSolutions-SNS3 suppression (the CMK already satisfies SNS3); correct SNS2-vs-SNS3 and AlarmBase-vs-Alarm claims. - Tests: assert topic KmsMasterKeyId is a GetAtt to the CMK (not anyValue), removalPolicy reaches both resources, malformed-email throws, and per-verb bootstrap guards for sns:CreateTopic/Subscribe + kms:CreateKey and the tag-scoped lifecycle statement. Tag the third stack test #629. Regenerated bootstrap artifacts + DEPLOYMENT_ROLES.md golden baseline and Starlight mirror. mise run build green (3844 cdk + 745 cli tests). --- cdk/bootstrap/BOOTSTRAP_HASH | 2 +- cdk/bootstrap/bootstrap-template.yaml | 21 ++++-- cdk/bootstrap/policies/observability.json | 22 ++++-- cdk/src/bootstrap/policies/observability.ts | 41 +++++++++--- cdk/src/constructs/operational-alerts.ts | 67 ++++++++++++------- .../__snapshots__/version.test.ts.snap | 2 +- cdk/test/bootstrap/policies.test.ts | 58 ++++++++++++++++ .../constructs/operational-alerts.test.ts | 33 +++++++-- cdk/test/stacks/agent.test.ts | 2 +- docs/design/DEPLOYMENT_ROLES.md | 22 ++++-- .../docs/architecture/Deployment-roles.md | 22 ++++-- 11 files changed, 230 insertions(+), 62 deletions(-) diff --git a/cdk/bootstrap/BOOTSTRAP_HASH b/cdk/bootstrap/BOOTSTRAP_HASH index 5dc0ef757..f4e0126c2 100644 --- a/cdk/bootstrap/BOOTSTRAP_HASH +++ b/cdk/bootstrap/BOOTSTRAP_HASH @@ -1 +1 @@ -d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 +4591304250a01e0d9e45d6890ab09a0b69aa6199b1149399fbb6418a91d1d77d diff --git a/cdk/bootstrap/bootstrap-template.yaml b/cdk/bootstrap/bootstrap-template.yaml index e158ce5a5..b6fd16205 100644 --- a/cdk/bootstrap/bootstrap-template.yaml +++ b/cdk/bootstrap/bootstrap-template.yaml @@ -1,7 +1,7 @@ # GENERATED FILE - DO NOT EDIT DIRECTLY # This template is generated by: npx tsx scripts/generate-bootstrap-template.ts # ABCA Bootstrap Policy Version: 1.4.0 -# ABCA Bootstrap Policy Hash: d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 +# ABCA Bootstrap Policy Hash: 4591304250a01e0d9e45d6890ab09a0b69aa6199b1149399fbb6418a91d1d77d # # Based on the default CDK bootstrap template with the following modifications: # - BootstrapVariant set to "ABCA: Least-Privilege Bootstrap" @@ -1286,18 +1286,25 @@ Resources: Sid: KMSForCDKAssets - Action: - kms:CreateKey - - kms:ScheduleKeyDeletion - - kms:PutKeyPolicy - kms:GetKeyPolicy - kms:GetKeyRotationStatus - - kms:EnableKeyRotation - - kms:DisableKeyRotation - kms:TagResource - - kms:UntagResource - kms:ListResourceTags Effect: Allow Resource: '*' Sid: KMSCustomerManagedKeys + - Action: + - kms:PutKeyPolicy + - kms:ScheduleKeyDeletion + - kms:EnableKeyRotation + - kms:DisableKeyRotation + - kms:UntagResource + Condition: + StringEquals: + aws:ResourceTag/ABCA: operational-alerts + Effect: Allow + Resource: '*' + Sid: KMSCustomerManagedKeysLifecycle - Action: - ecr:CreateRepository - ecr:DescribeRepositories @@ -1452,7 +1459,7 @@ Outputs: Value: 1.4.0 BootstrapPolicyHash: Description: SHA-256 hash of the ABCA bootstrap policy bundle for drift detection - Value: d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77 + Value: 4591304250a01e0d9e45d6890ab09a0b69aa6199b1149399fbb6418a91d1d77d BootstrapPolicySet: Description: Comma-separated list of active ABCA bootstrap policy names Value: diff --git a/cdk/bootstrap/policies/observability.json b/cdk/bootstrap/policies/observability.json index 9c860f0f8..0af8d7733 100644 --- a/cdk/bootstrap/policies/observability.json +++ b/cdk/bootstrap/policies/observability.json @@ -120,20 +120,32 @@ { "Action": [ "kms:CreateKey", - "kms:ScheduleKeyDeletion", - "kms:PutKeyPolicy", "kms:GetKeyPolicy", "kms:GetKeyRotationStatus", - "kms:EnableKeyRotation", - "kms:DisableKeyRotation", "kms:TagResource", - "kms:UntagResource", "kms:ListResourceTags" ], "Effect": "Allow", "Resource": "*", "Sid": "KMSCustomerManagedKeys" }, + { + "Action": [ + "kms:PutKeyPolicy", + "kms:ScheduleKeyDeletion", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:UntagResource" + ], + "Condition": { + "StringEquals": { + "aws:ResourceTag/ABCA": "operational-alerts" + } + }, + "Effect": "Allow", + "Resource": "*", + "Sid": "KMSCustomerManagedKeysLifecycle" + }, { "Action": [ "ecr:CreateRepository", diff --git a/cdk/src/bootstrap/policies/observability.ts b/cdk/src/bootstrap/policies/observability.ts index c8fafdb76..0c18a82db 100644 --- a/cdk/src/bootstrap/policies/observability.ts +++ b/cdk/src/bootstrap/policies/observability.ts @@ -155,26 +155,49 @@ export function observabilityPolicy(): iam.PolicyDocument { }), new iam.PolicyStatement({ - // Customer-managed keys created BY the stack (e.g. the - // OperationalAlerts SNS topic key, issue #629), distinct from - // the CDK bootstrap asset key used above. kms:CreateKey cannot - // be resource-scoped (the key ARN does not exist yet) and CMK - // ARNs are UUIDs, so this statement is unavoidably `*`. + // Create/read/tag for customer-managed keys the stack provisions + // (e.g. the OperationalAlerts SNS topic key, issue #629), distinct + // from the CDK bootstrap asset key above. kms:CreateKey cannot be + // resource-scoped (the key ARN does not exist yet) and CMK ARNs + // are UUIDs, so `*` is unavoidable here. This statement holds only + // create + read + tag actions — none can escalate privilege on an + // existing key. kms:TagResource must stay unconditioned because it + // applies the very `ABCA` tag that scopes the mutation statement + // below (a tag condition would deny the call that sets the tag). sid: 'KMSCustomerManagedKeys', effect: iam.Effect.ALLOW, actions: [ 'kms:CreateKey', - 'kms:ScheduleKeyDeletion', - 'kms:PutKeyPolicy', 'kms:GetKeyPolicy', 'kms:GetKeyRotationStatus', + 'kms:TagResource', + 'kms:ListResourceTags', + ], + resources: ['*'], + }), + + new iam.PolicyStatement({ + // Policy-mutation and deletion actions that COULD take over or + // destroy an unrelated account CMK if granted on `*`. CMK ARNs are + // UUIDs (still unscopable), so instead we gate them on the `ABCA` + // resource tag that the OperationalAlerts construct stamps on its + // key at creation (CloudFormation tags atomically in the CreateKey + // call, so an existing key always carries the tag by the time any + // of these run). This bounds the blast radius to keys this + // solution owns. Issue #629 review — MEDIUM 2. + sid: 'KMSCustomerManagedKeysLifecycle', + effect: iam.Effect.ALLOW, + actions: [ + 'kms:PutKeyPolicy', + 'kms:ScheduleKeyDeletion', 'kms:EnableKeyRotation', 'kms:DisableKeyRotation', - 'kms:TagResource', 'kms:UntagResource', - 'kms:ListResourceTags', ], resources: ['*'], + conditions: { + StringEquals: { 'aws:ResourceTag/ABCA': 'operational-alerts' }, + }, }), new iam.PolicyStatement({ diff --git a/cdk/src/constructs/operational-alerts.ts b/cdk/src/constructs/operational-alerts.ts index ae45d677a..c41576883 100644 --- a/cdk/src/constructs/operational-alerts.ts +++ b/cdk/src/constructs/operational-alerts.ts @@ -17,14 +17,13 @@ * SOFTWARE. */ -import { RemovalPolicy } from 'aws-cdk-lib'; +import { RemovalPolicy, Stack, Tags } from 'aws-cdk-lib'; import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; import * as cloudwatchActions from 'aws-cdk-lib/aws-cloudwatch-actions'; import * as iam from 'aws-cdk-lib/aws-iam'; import * as kms from 'aws-cdk-lib/aws-kms'; import * as sns from 'aws-cdk-lib/aws-sns'; import * as subscriptions from 'aws-cdk-lib/aws-sns-subscriptions'; -import { NagSuppressions } from 'cdk-nag'; import { Construct } from 'constructs'; /** @@ -45,7 +44,9 @@ export interface OperationalAlertsProps { readonly alertEmail?: string; /** - * Removal policy for the topic's customer-managed KMS key. + * Removal policy applied to both resources this construct owns — the + * SNS topic and its customer-managed KMS key — so they share one + * lifecycle on stack deletion. * @default RemovalPolicy.DESTROY */ readonly removalPolicy?: RemovalPolicy; @@ -64,7 +65,8 @@ export interface OperationalAlertsProps { * ``kms:GenerateDataKey*`` / ``kms:Decrypt``. The alarm→SNS action would * fail silently at delivery time. The CMK below grants CloudWatch (and * SNS) exactly those actions so delivery works while keeping - * encryption-at-rest and satisfying cdk-nag ``AwsSolutions-SNS2``. + * encryption-at-rest. Setting a CMK also satisfies cdk-nag + * ``AwsSolutions-SNS2`` (encryption at rest) with no suppression needed. * * The topic is intentionally stack-wide (not per-consumer) so every * DLQ-depth alarm shares one subscription surface — an operator @@ -86,32 +88,49 @@ export class OperationalAlerts extends Construct { const removalPolicy = props.removalPolicy ?? RemovalPolicy.DESTROY; // Customer-managed key so the CloudWatch service principal can be - // granted decrypt/data-key rights (see class doc). Rotation on by - // default — no reason not to for a low-throughput alerts key. + // granted decrypt/data-key rights (see class doc). We enable + // rotation explicitly (CDK defaults it off) — no reason not to for a + // low-throughput alerts key. this.key = new kms.Key(this, 'Key', { description: 'Encrypts the ABCA operational-alerts SNS topic (DLQ-depth alarms → operators)', enableKeyRotation: true, removalPolicy, }); + // Tag the key so the CFN deploy role can scope its unavoidably + // account-wide KMS-lifecycle grant (PutKeyPolicy / ScheduleKeyDeletion + // — CMK ARNs are UUIDs and cannot be ARN-scoped) to keys THIS solution + // creates. See the ``KMSCustomerManagedKeys`` statement in + // ``cdk/src/bootstrap/policies/observability.ts``. + Tags.of(this.key).add('ABCA', 'operational-alerts'); // Allow CloudWatch Alarms to publish through the encrypted topic. // Without these grants the alarm action resolves at deploy time but // every publish fails at runtime with a KMS AccessDenied the - // operator never sees. + // operator never sees. The ``aws:SourceAccount`` condition closes + // the service-principal confused-deputy hole — a CloudWatch alarm in + // another account cannot induce this key's use — matching the + // account-pinning precedent in ``lambda-microvm-compute.ts``. + // ``Resource: '*'`` here scopes to THIS key (it is the key's own + // resource policy), not to every key in the account. this.key.addToResourcePolicy(new iam.PolicyStatement({ sid: 'AllowCloudWatchAlarmsUseOfKey', principals: [new iam.ServicePrincipal('cloudwatch.amazonaws.com')], actions: ['kms:Decrypt', 'kms:GenerateDataKey*'], resources: ['*'], + conditions: { StringEquals: { 'aws:SourceAccount': Stack.of(this).account } }, })); this.topic = new sns.Topic(this, 'Topic', { displayName: 'ABCA operational alerts', masterKey: this.key, }); + // Key and topic share one lifecycle (see ``removalPolicy`` prop). + this.topic.applyRemovalPolicy(removalPolicy); - // SNS delivery is best-effort operational metadata; require callers - // to publish over TLS regardless. + // Defense-in-depth: deny any non-TLS publish. cdk-nag's SNS3 rule is + // already satisfied by the CMK above (it short-circuits to compliant + // once a topic key is set), so this statement is not there to clear a + // nag finding — it independently guarantees callers publish over TLS. this.topic.addToResourcePolicy(new iam.PolicyStatement({ sid: 'DenyInsecureTransport', effect: iam.Effect.DENY, @@ -122,27 +141,27 @@ export class OperationalAlerts extends Construct { })); if (props.alertEmail) { + // Fail at synth rather than ship a topic with a permanently + // unconfirmed junk subscription. A minimal shape check — full RFC + // 5322 validation is overkill for an operator alert address. + if (!/^[^@\s]+@[^@\s]+\.[^@\s]+$/.test(props.alertEmail)) { + throw new Error( + `OperationalAlerts: alertEmail "${props.alertEmail}" is not a valid email address`, + ); + } this.topic.addSubscription(new subscriptions.EmailSubscription(props.alertEmail)); } - - NagSuppressions.addResourceSuppressions(this.topic, [ - { - id: 'AwsSolutions-SNS3', - reason: - 'Topic-wide SSL enforcement is applied via an explicit DenyInsecureTransport ' + - 'resource policy statement (aws:SecureTransport=false) rather than the L2 default.', - }, - ]); } /** * Wire one or more alarms to publish to the alerts topic on state - * change. Takes the concrete {@link cloudwatch.Alarm} because - * ``addAlarmAction`` is declared there, not on the ``IAlarm`` - * interface — which is precisely why the FanOut / - * ApprovalMetricsPublisher / screenshot DLQ-depth alarms are exposed - * as ``Alarm``. Those constructs stay decoupled from this one: the - * caller passes their alarms in. + * change. ``addAlarmAction`` lives on the ``AlarmBase`` class (parent + * of both ``Alarm`` and ``CompositeAlarm``), not on the ``IAlarm`` + * interface — so the parameter is typed to the concrete + * {@link cloudwatch.Alarm} the DLQ-depth constructs (FanOut / + * ApprovalMetricsPublisher / screenshot) already expose. Those + * constructs stay decoupled from this one: the caller passes their + * alarms in. */ public addAlarmActions(...alarms: cloudwatch.Alarm[]): void { const action = new cloudwatchActions.SnsAction(this.topic); diff --git a/cdk/test/bootstrap/__snapshots__/version.test.ts.snap b/cdk/test/bootstrap/__snapshots__/version.test.ts.snap index e8e8bb8e2..fb7d2de22 100644 --- a/cdk/test/bootstrap/__snapshots__/version.test.ts.snap +++ b/cdk/test/bootstrap/__snapshots__/version.test.ts.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing -exports[`bootstrap version module hash is stable 1`] = `"d6515acf4dc102f582e05990b7086cd8413d9e558146eca2ba22c5f59bdc2e77"`; +exports[`bootstrap version module hash is stable 1`] = `"4591304250a01e0d9e45d6890ab09a0b69aa6199b1149399fbb6418a91d1d77d"`; diff --git a/cdk/test/bootstrap/policies.test.ts b/cdk/test/bootstrap/policies.test.ts index 133e4f1f9..6ee502713 100644 --- a/cdk/test/bootstrap/policies.test.ts +++ b/cdk/test/bootstrap/policies.test.ts @@ -213,6 +213,23 @@ describe('IaCRole-ABCA-Application', () => { expect(resources).toContain(`arn:aws:secretsmanager:*:*:secret:${prefix}*`); } }); + + it('SNS statement grants the create + subscribe verbs the OperationalAlerts topic needs (#629)', () => { + // Per-verb guard (mirrors the #409/#407 provisioned-concurrency and + // event-source-mapping-tagging guards): a missing verb is invisible to + // the service-prefix check above (still `sns:`) but rolls back a fresh + // deploy with AccessDenied. CreateTopic creates the topic; + // Subscribe/GetTopicAttributes are exercised when `-c alertEmail=...` + // adds an email subscription. + const resolvedDoc = stack.resolve(doc); + const statements = resolvedDoc.Statement as Array<{ Sid: string; Action?: string | string[] }>; + const sns = statements.find((s) => s.Sid === 'SNS'); + expect(sns).toBeDefined(); + const actions = Array.isArray(sns!.Action) ? sns!.Action : [sns!.Action]; + for (const action of ['sns:CreateTopic', 'sns:Subscribe', 'sns:GetTopicAttributes']) { + expect(actions).toContain(action); + } + }); }); describe('IaCRole-ABCA-Observability', () => { @@ -242,6 +259,7 @@ describe('IaCRole-ABCA-Observability', () => { 'S3ApplicationBuckets', 'KMSForCDKAssets', 'KMSCustomerManagedKeys', + 'KMSCustomerManagedKeysLifecycle', 'ECRForDockerAssets', 'ECRAuthToken', 'XRay', @@ -313,6 +331,46 @@ describe('IaCRole-ABCA-Observability', () => { expect(actions).toContain(action); } }); + + it('KMSCustomerManagedKeys grants CreateKey + tag on * (the unscopable create path) (#629)', () => { + // kms:CreateKey and kms:TagResource must be unconditioned on `*`: + // the key ARN does not exist at create time and TagResource applies + // the very tag that scopes the lifecycle statement below. + const resolvedDoc = stack.resolve(doc); + const statements = resolvedDoc.Statement as Array<{ + Sid: string; + Action?: string | string[]; + Condition?: unknown; + }>; + const create = statements.find((s) => s.Sid === 'KMSCustomerManagedKeys'); + expect(create).toBeDefined(); + const actions = Array.isArray(create!.Action) ? create!.Action : [create!.Action]; + expect(actions).toContain('kms:CreateKey'); + expect(actions).toContain('kms:TagResource'); + // The create/read statement must NOT carry policy-mutation actions — + // those belong to the tag-scoped lifecycle statement. + expect(actions).not.toContain('kms:PutKeyPolicy'); + expect(actions).not.toContain('kms:ScheduleKeyDeletion'); + expect(create!.Condition).toBeUndefined(); + }); + + it('KMSCustomerManagedKeysLifecycle gates key-takeover actions on the ABCA resource tag (#629)', () => { + // PutKeyPolicy / ScheduleKeyDeletion on `*` would let the deploy role + // take over or delete any CMK in the account. They are confined to + // keys carrying the `ABCA=operational-alerts` tag the construct stamps. + const resolvedDoc = stack.resolve(doc); + const statements = resolvedDoc.Statement as Array<{ + Sid: string; + Action?: string | string[]; + Condition?: { StringEquals?: Record }; + }>; + const lifecycle = statements.find((s) => s.Sid === 'KMSCustomerManagedKeysLifecycle'); + expect(lifecycle).toBeDefined(); + const actions = Array.isArray(lifecycle!.Action) ? lifecycle!.Action : [lifecycle!.Action]; + expect(actions).toContain('kms:PutKeyPolicy'); + expect(actions).toContain('kms:ScheduleKeyDeletion'); + expect(lifecycle!.Condition?.StringEquals?.['aws:ResourceTag/ABCA']).toBe('operational-alerts'); + }); }); describe('IaCRole-ABCA-Compute-AgentCore', () => { diff --git a/cdk/test/constructs/operational-alerts.test.ts b/cdk/test/constructs/operational-alerts.test.ts index 986baad87..8fc4ad8aa 100644 --- a/cdk/test/constructs/operational-alerts.test.ts +++ b/cdk/test/constructs/operational-alerts.test.ts @@ -17,7 +17,7 @@ * SOFTWARE. */ -import { App, Duration, Stack } from 'aws-cdk-lib'; +import { App, Duration, RemovalPolicy, Stack } from 'aws-cdk-lib'; import { Match, Template } from 'aws-cdk-lib/assertions'; import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch'; import * as sqs from 'aws-cdk-lib/aws-sqs'; @@ -46,10 +46,14 @@ describe('OperationalAlerts', () => { template.resourceCountIs('AWS::SNS::Topic', 1); template.resourceCountIs('AWS::KMS::Key', 1); - // The topic must reference the CMK (not the AWS-managed key) so - // CloudWatch can be granted publish/decrypt rights on it. + // The topic must reference THIS construct's CMK by GetAtt — asserting + // Match.anyValue() would pass even for the literal `alias/aws/sns`, + // the exact AWS-managed-key state the construct exists to prevent + // (CloudWatch can't publish through it). Pin the GetAtt to the Key. template.hasResourceProperties('AWS::SNS::Topic', { - KmsMasterKeyId: Match.anyValue(), + KmsMasterKeyId: { + 'Fn::GetAtt': [Match.stringLikeRegexp('AlertsKey'), 'Arn'], + }, }); template.hasResourceProperties('AWS::KMS::Key', { EnableKeyRotation: true, @@ -117,6 +121,27 @@ describe('OperationalAlerts', () => { }); }); + test('throws at synth on a malformed alertEmail rather than shipping a junk subscription', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + expect( + () => new OperationalAlerts(stack, 'Alerts', { alertEmail: 'not-an-email' }), + ).toThrow(/not a valid email/); + }); + + test('applies the removal policy to BOTH the topic and the key', () => { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + new OperationalAlerts(stack, 'Alerts', { removalPolicy: RemovalPolicy.RETAIN }); + const template = Template.fromStack(stack); + + // Regression guard: removalPolicy previously reached only the key, + // leaving the topic on CDK's implicit default — key and topic could + // diverge on stack deletion. + template.hasResource('AWS::SNS::Topic', { DeletionPolicy: 'Retain' }); + template.hasResource('AWS::KMS::Key', { DeletionPolicy: 'Retain' }); + }); + test('addAlarmActions wires each alarm to publish to the topic', () => { const app = new App(); const stack = new Stack(app, 'TestStack'); diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index c0c9b4701..8715ab407 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -705,7 +705,7 @@ describe('AgentStack', () => { } }); - test('does NOT subscribe an email when no alertEmail context is set', () => { + test('does NOT subscribe an email when no alertEmail context is set (#629)', () => { // The default deploy ships the topic with no confirmed target; // operators subscribe Slack / PagerDuty / email themselves. template.resourceCountIs('AWS::SNS::Subscription', 0); diff --git a/docs/design/DEPLOYMENT_ROLES.md b/docs/design/DEPLOYMENT_ROLES.md index 212f964de..7ec30db1a 100644 --- a/docs/design/DEPLOYMENT_ROLES.md +++ b/docs/design/DEPLOYMENT_ROLES.md @@ -662,18 +662,30 @@ Bedrock Guardrails, CloudWatch Logs/Dashboards/Alarms, X-Ray, S3 (CDK assets), K "Effect": "Allow", "Action": [ "kms:CreateKey", - "kms:ScheduleKeyDeletion", - "kms:PutKeyPolicy", "kms:GetKeyPolicy", "kms:GetKeyRotationStatus", - "kms:EnableKeyRotation", - "kms:DisableKeyRotation", "kms:TagResource", - "kms:UntagResource", "kms:ListResourceTags" ], "Resource": "*" }, + { + "Sid": "KMSCustomerManagedKeysLifecycle", + "Effect": "Allow", + "Action": [ + "kms:PutKeyPolicy", + "kms:ScheduleKeyDeletion", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:UntagResource" + ], + "Resource": "*", + "Condition": { + "StringEquals": { + "aws:ResourceTag/ABCA": "operational-alerts" + } + } + }, { "Sid": "ECRForDockerAssets", "Effect": "Allow", diff --git a/docs/src/content/docs/architecture/Deployment-roles.md b/docs/src/content/docs/architecture/Deployment-roles.md index 80c82304e..da61f26b5 100644 --- a/docs/src/content/docs/architecture/Deployment-roles.md +++ b/docs/src/content/docs/architecture/Deployment-roles.md @@ -666,18 +666,30 @@ Bedrock Guardrails, CloudWatch Logs/Dashboards/Alarms, X-Ray, S3 (CDK assets), K "Effect": "Allow", "Action": [ "kms:CreateKey", - "kms:ScheduleKeyDeletion", - "kms:PutKeyPolicy", "kms:GetKeyPolicy", "kms:GetKeyRotationStatus", - "kms:EnableKeyRotation", - "kms:DisableKeyRotation", "kms:TagResource", - "kms:UntagResource", "kms:ListResourceTags" ], "Resource": "*" }, + { + "Sid": "KMSCustomerManagedKeysLifecycle", + "Effect": "Allow", + "Action": [ + "kms:PutKeyPolicy", + "kms:ScheduleKeyDeletion", + "kms:EnableKeyRotation", + "kms:DisableKeyRotation", + "kms:UntagResource" + ], + "Resource": "*", + "Condition": { + "StringEquals": { + "aws:ResourceTag/ABCA": "operational-alerts" + } + } + }, { "Sid": "ECRForDockerAssets", "Effect": "Allow",