Skip to content

fix(iac/aws): grant kms:TagResource for KMS CreateKey tag-on-create - #1671

Merged
cristim merged 1 commit into
mainfrom
fix/kms-tag-on-create
Jul 28, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/kms-tag-on-create

Conversation

@cristim

@cristim cristim commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

Every AWS deploy that creates a new KMS CMK is currently failing:

Error: creating KMS Key: AccessDeniedException: User: arn:aws:sts::909626172446:assumed-role/cudly-terraform-deploy/GitHubActions
is not authorized to perform: kms:TagResource because no identity-based policy allows the kms:TagResource action
  with module.compute_lambda[0].aws_kms_key.signing,
  on ../../modules/compute/aws/lambda/signing-key.tf line 8

CreateKey's Tags parameter requires kms:TagResource (AWS KMS API reference, "Required permissions"), but the only existing grant (KMSMutateTaggedOnly in policy_compute.tf) is gated on aws:ResourceTag/Project, a condition key evaluated against tags already on an existing resource. At CreateKey time the key doesn't exist yet, so this condition can never be satisfied and the call is denied.

Verified against current AWS docs:

  • KMS CreateKey API reference: "Required permissions: kms:CreateKey ... To use the Tags parameter, kms:TagResource."
  • IAM "Controlling access during AWS requests" guide: aws:RequestTag gates what tags can be passed in a request (the correct key for tag-on-create); aws:ResourceTag gates access based on tags already on the resource.

Fix

Adds KMSTagOnCreate to terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf (not policy_compute.tf — see "Why policy_compute_b.tf" below):

{
  Sid      = "KMSTagOnCreate"
  Effect   = "Allow"
  Action   = ["kms:TagResource"]
  Resource = "*"
  Condition = {
    StringEqualsIgnoreCase = {
      "aws:RequestTag/Project" = "CUDly"
    }
    Null = {
      "aws:ResourceTag/Project" = "true"
    }
  }
},

The Null condition is a deliberate hardening beyond the minimal fix: aws:RequestTag alone says nothing about a resource's current tags, so without it a leaked deploy token could call kms:TagResource on any existing KMS key in the account (not just ones it creates), self-attach Project=CUDly, then use KMSMutateTaggedOnly to disable/delete/repolicy that now-mistagged key — i.e. hijack a key it never created. Null: {"aws:ResourceTag/Project": "true"} restricts the statement to resources that currently have no Project tag at all (true for a brand-new key mid-CreateKey, false for anything already tagged, ours or not), so it can only ever perform the initial tag-on-create.

Why policy_compute_b.tf

policy_compute.tf's compiled JSON is ~5923 of the AWS 6144-char managed-policy limit (per its own header comment, it's already the reason policy_compute_b.tf exists). Adding the new statement there would land at ~6083 chars — technically under the limit but with only ~60 chars of margin, which is too fragile. policy_compute_b.tf already houses the other KMS statements that were split out of the main policy for size reasons (KMSAliasMutate, KMSReadTaggedOnly), so it's the natural, low-risk home for this one too.

Completeness checks

  1. Every aws_kms_key the deploy role creates carries a Project tag: the only KMS key actually created by this deploy role's live apply graph is aws_kms_key.signing (modules/compute/aws/lambda/signing-key.tf), tagged via local.common_tags.Project = var.project_name (same lowercase-cudly path documented on KMSMutateTaggedOnly, matched case-insensitively). modules/monitoring/aws also defines an aws_kms_key.sns, but that module isn't wired into any terraform/environments/aws root today, so it isn't exercised by this deploy role.
  2. kms:TagResource is the only missing permission on this path: walked the AWS provider's create/read calls for aws_kms_key/aws_kms_alias (CreateKey, TagResource, DescribeKey, ListResourceTags, GetKeyPolicy, GetKeyRotationStatus, CreateAlias) against the granted statements. Tag-gated reads (GetKeyPolicy, the key-side of alias creation) are safe because CreateKey's Tags parameter tags the key atomically during creation, so the tag is already present by the time any subsequent read happens.
  3. Sibling clouds: GCP's ci-cd-permissions uses the broad predefined roles/cloudkms.admin (no ABAC tag-condition, so this bug shape can't occur); Azure's ci-cd-permissions/role.tf has zero tag/condition constructs. No equivalent gap found in either; no follow-up filed.
  4. test-iam-role.sh: this is a full live-AWS destroy+apply integration script (not run in CI), with a generic AccessDenied grep that already surfaces any missing action generically. No existing precedent in the script adds bespoke per-permission checks, so none was added here to avoid forcing a fit that doesn't match its structure.

Manual apply required

This fix does not take effect until a privileged human re-applies terraform/environments/aws/ci-cd-permissions/. The CI deploy role (cudly-terraform-deploy) cannot grant itself new IAM permissions — that would be self-privilege-escalation, and this bootstrap root is intentionally out of the CI deploy role's own write scope.

cd terraform/environments/aws/ci-cd-permissions
terraform init -reconfigure   # if backend.hcl differs from your last run
terraform plan
terraform apply

Run this with your own (human, non-CI) AWS credentials that have permission to modify the cudly-deploy-compute-b managed policy.

Closes #1670

CreateKey's Tags parameter requires kms:TagResource per the AWS API
reference, but the only existing grant (KMSMutateTaggedOnly) is gated
on aws:ResourceTag/Project, which can never be satisfied at create
time since the key doesn't exist yet. Every AWS deploy that creates
the OIDC signing KMS key was failing with AccessDeniedException.

Add a separate, aws:RequestTag-gated statement, restricted via a Null
condition on aws:ResourceTag/Project to resources with no Project tag
yet, so it only ever performs the initial tag-on-create and can't be
used to self-tag and hijack an existing, unrelated key through
KMSMutateTaggedOnly.

Closes #1670
@cristim cristim added triaged Item has been triaged priority/p0 Drop everything; same-day fix severity/critical Major harm when it happens urgency/now Drop other things impact/all-users Affects every user effort/s Hours type/bug Defect labels Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b165d4a-ec84-4b75-9bca-0d1715fe797f

📥 Commits

Reviewing files that changed from the base of the PR and between 3e0400b and 7b469aa.

📒 Files selected for processing (1)
  • terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/kms-tag-on-create

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Deferred completeness checks (non-blocking, per team-lead direction to ship the minimal fix first)

1. Sweep of other aws_kms_key resources: only aws_kms_key.signing (modules/compute/aws/lambda/signing-key.tf) is actually created by this deploy role's live apply graph today. modules/monitoring/aws also defines aws_kms_key.sns (behind enable_sns_encryption, default false), tagged via tags = var.tags, but that module isn't wired into any terraform/environments/aws root module currently — it's dead code from this role's perspective. If it's ever wired in, its tags flow the same local.common_tags.Project path, so it would already be covered by this fix; no action needed unless/until it's actually invoked.

2. Sibling clouds (GCP/Azure): checked terraform/environments/{gcp,azure}/ci-cd-permissions/ for the same bug shape (a create-time grant gated on a condition that can only be true for an existing resource). GCP's service_account.tf grants the broad predefined roles/cloudkms.admin (no ABAC tag-condition — this bug shape can't occur under GCP IAM). Azure's role.tf has zero tag/condition constructs at all. No equivalent gap found in either cloud; no follow-up issue filed.

3. test-iam-role.sh coverage: this script does full live-AWS destroy+apply cycles (requires real AWS creds via assume-role, not run in CI) with a generic AccessDenied grep (parse_access_denied) that already surfaces any missing action generically — it would have caught this exact gap the first time someone ran it against the lambda environment. There's no existing precedent in the script for bespoke per-permission checks (no other historical permission fix added one either), so no addition was forced in to avoid a fit that doesn't match its structure. If the team wants explicit regression coverage for this specific class of bug (tag-on-create vs tag-on-existing), that would be a separate, decidable addition — happy to take that as a follow-up if wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/all-users Affects every user priority/p0 Drop everything; same-day fix severity/critical Major harm when it happens triaged Item has been triaged type/bug Defect urgency/now Drop other things

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AWS deploy blocked: kms:TagResource unauthorizable at KMS CreateKey time

1 participant