fix: serve org email assets from a private bucket via CloudFront (OAC) - #929
Open
diegomayorga-dept wants to merge 2 commits into
Open
fix: serve org email assets from a private bucket via CloudFront (OAC)#929diegomayorga-dept wants to merge 2 commits into
diegomayorga-dept wants to merge 2 commits into
Conversation
The org-email-assets bucket required a public-read policy, so any AWS account with account-level S3 Block Public Access enabled could not deploy v1.5 at all: CloudFormation failed on AWS::S3::BucketPolicy and the nested stack rolled back. Keep the bucket fully private and serve its email-branding images through a new, dedicated CloudFront distribution using Origin Access Control, created alongside the bucket in the back-end stack. OAC's bucket policy names the cloudfront.amazonaws.com service principal scoped by AWS:SourceArn, which is not a public policy, so account-level Block Public Access permits it. The distribution has no custom domain and no certificate, so it always resolves at its own CloudFront domain; that domain reaches every Lambda as ORG_EMAIL_ASSETS_CDN_DOMAIN. The website's distribution cannot serve these assets: it geo-restricts to AR/AU/US, maps 404 to /index.html with HTTP 200, and its domain is a front-end stack value the back-end cannot read at deploy time. Uploaded logo keys are now versioned so a replaced logo cannot render from CloudFront cache. Legacy absolute S3 URLs persisted by v1.5 are rewritten host-only at read time, so existing org logos keep working without a backfill. Also corrects docs/deployment/production.md, which told operators to disable account-level Block Public Access or obtain an SCP exception. Plan: 2026-09-02-org-email-assets-private-bucket-plan v1.1
Resolve the stale [Unreleased] heading to [v1.5]: that section describes the back-end API stack split, which shipped in v1.5. Move the v1.5.1 entry above it so the changelog stays newest-first, and give it a Migration section recording that logo URLs persisted by v1.5 keep resolving with no backfill. Add upgrading.md section 7.1, Recovering from a failed deploy. Section 7 covered only rolling back a release that deployed successfully, so there was no guidance for the state this hotfix's own audience is in: a ROLLBACK_COMPLETE stack that CloudFormation cannot update, which on deletion orphans the DynamoDB tables (RETAIN and deletionProtection are set unconditionally, independent of env-type) and makes the next deploy fail with ResourceInUseException. Found first-hand while verifying this fix against an account with S3 Block Public Access enabled. Also correct the two "v1.4 -> next" placeholders to "v1.4 -> v1.5" and order the compatibility matrix chronologically. Plan: 2026-09-02-org-email-assets-private-bucket-plan v1.1
Kevin Loftus (kevin-loftus-dept)
approved these changes
Sep 4, 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.
Title*
Serve org email assets from a private bucket via CloudFront (OAC) — unblocks deploys into accounts with account-level S3 Block Public Access
Type of Change*
Description
Target: v1.5.1. Branched from the
v1.5tag and targetsmain, notdevelopment.The
org-email-assetsbucket introduced in v1.5 required a public-read bucket policy (AnyPrincipals3:GetObject) and opted out of two Block Public Access controls. Any AWS account with account-level S3 Block Public Access enabled — a standard security baseline and the default for newer accounts — could not deploy v1.5 at all: CloudFormation returnedAccessDeniedonAWS::S3::BucketPolicyand the nested stack rolled back. Reported by WSLH deploying v1.5 to their dev account.The bucket exists because email clients (Gmail included) strip base64
data:URI images, so branding logos must be fetchable over ordinary HTTPS. That requirement is legitimate; the public bucket was the wrong way to meet it.The fix: keep the bucket fully private and serve its images through a new, dedicated CloudFront distribution using Origin Access Control, created alongside the bucket in the back-end stack. OAC's bucket policy names the
cloudfront.amazonaws.comservice principal scoped byAWS:SourceArn— this is not a public policy, so account-level BPA permits it. That is the mechanism of the fix.Why not the website's existing distribution (the originally proposed approach):
AR/AU/US, and Gmail fetches images through Google's image proxy whose egress country we don't control;/index.htmlwith HTTP 200, so a missing asset returns SPA HTML with a success status;A back-end-owned distribution with no custom domain and no certificate sidesteps all three — it always resolves at its own
d….cloudfront.netaddress, so no environment needs a hosted zone or certificate for email assets to work.Supporting changes:
ORG_EMAIL_ASSETS_CDN_DOMAIN(common env, not per-Lambda —default-email-branding.tsis reachable from four different senders, so a per-Lambda variable would silently renderhttps://undefined/...in whichever one was missed).{orgId}/logo-{epochMillis}.{ext}) so a replaced logo can't render from CloudFront cache. A?v=query param can't work —CACHING_OPTIMIZEDexcludes query strings from the cache key — and explicit invalidation isn't available, since the presignedPUTgoes direct from the browser and the handler never learns it completed. Accepted cost: a re-upload orphans the previous object.v1.5is a released tag and the failure is conditional on account-level BPA, so accounts without it deployed v1.5 successfully and may hold absolutehttps://<bucket>.s3.<region>.amazonaws.com/...values inEmailBrandingLogoUrl. Those 403 once the bucket is private, andSesServicefalls back to the default only on a falsy URL.resolveEmailBrandingLogoUrlrewrites the host at read time (the object is still at its original key), so existing logos keep working with no backfill. Non-matching and external URLs pass through untouched — the logo URL field is free text an admin can paste anything into.docs/deployment/production.md§3.3 told operators to disable account-level BPA or obtain an SCP exception, and made a BPA pre-flight check a release gate. That advice is now wrong and actively harmful, so it's rewritten and the gate removed..env.local.exampleand the local-dev doc gain the new variable.CHANGELOG.mdgains a[v1.5.1]entry, and the stale[Unreleased]heading is resolved to[v1.5]— that section describes the back-end stack split, which shipped in v1.5.docs/deployment/upgrading.mdgains av1.5 → v1.5.1Tier 1 row (no DynamoDB, schema, or configuration change) and a new §7.1 Recovering from a failed deploy. §7 previously covered only rolling back a successful release, so there was no guidance for the state this hotfix's audience is actually in: aROLLBACK_COMPLETEstack that CloudFormation cannot update, which on deletion orphans the{namePrefix}-*DynamoDB tables (RETAIN+deletionProtection: true, unconditional) and makes the next deploy fail withResourceInUseException: Table already exists. Hit first-hand during deploy verification.S3Urldeliberately still points at the S3 endpoint — it's the presignedPUT— and the bucket's CORS rule is retained unchanged. No shared-lib schema change, no front-end change, noROUTE_SCHEMASchange.Testing*
Automated: back-end 116 suites / 877 tests, shared-lib 22/156, front-end 14/129 — all passing; lint clean across all three packages apart from one pre-existing
import/orderwarning unrelated to this branch.The CI regression guard is
test/infra/constructs/org-email-assets-bucket-construct.test.ts, whose first case previously asserted the public policy existed. It now asserts all fourPublicAccessBlockConfigurationcontrols aretrueand that no bucket-policy statement grants anAllowto a wildcard principal — with anEffect === 'Allow'filter, becauseenforceSSLlegitimately emits aDenytoPrincipal: {AWS: '*'}. Further cases cover the distribution and OAC resources, the service-principal grant scoped byAWS:SourceArn, absence of a custom domain/certificate, and absence of geo-restriction.New: 17 cases for the legacy-URL normalizer, covering pass-through for external hosts, a different bucket, a coincidentally-suffixed third-party bucket, unset env vars, and a malformed non-URL string.
cdk-nag: zero
AwsSolutions-CFR*findings, andAwsSolutions-S2(the public-bucket rule) no longer appears at all. CFR1/2/3/4 carry written suppressions; CFR7 needs none because the design uses OAC rather than OAI.pnpm cdk-auditstill exits 1 on ~700 pre-existing findings across unrelated resources, established as pre-existing by a before/after comparison against the pre-branch baseline.Deploy verification — done 2026-09-03
Run in DEPT's dev account (
851725267090) as a throwawayenv-name: bpatestwith account-level S3 Block Public Access fully enabled (all four controls). Both halves were run against the same account under the same settings, and the environment was torn down afterwards with the account restored to its pre-test state.v1.5 failure reproduced first. Deploying the
v1.5tag (3444f40d) under account-level BPA fails exactly as reported:Every other
CREATE_FAILEDin that stack wasResource creation cancelled— this single resource takes the whole deploy down.This branch then deployed clean under identical settings, confirming the four checks below:
dev-bpatest-easy-genomics-api-stackreachedCREATE_COMPLETEwith all four controls on.dev-bpatest-main-back-end-stackwentUPDATE_COMPLETEfrom the common Lambda env var.403— anonymousGETof the rawhttps://dev-bpatest-org-email-assets-bucket.s3.us-west-2.amazonaws.com/defaults/easy-genomics.pngreturned403, while the same object through the distribution returned200 image/png 5699 bytes.bpatestran withaws-hosted-zone-idandaws-certificate-arnboth empty; the distribution served on its ownd34l6xswsri39r.cloudfront.net.IsPublic: false, all four bucket-level controlstrue, and the only external grant isAllow s3:GetObjecttoService: cloudfront.amazonaws.comscoped byAWS:SourceArnto this distribution.ORG_EMAIL_ASSETS_CDN_DOMAINconfirmed present in the common Lambda environment.Correction to the deploy instruction this checklist originally carried. It suggested deploying "over the rolled-back stack". That is not possible: the failed stack lands in
ROLLBACK_COMPLETE, which CloudFormation can only delete, not update — and once deleted, the 13 DynamoDB tables survive withRETAIN+deletionProtection: trueand explicit names, so the next deploy fails withResourceInUseException: Table already exists. See the upgrade note under Additional Information; this affects every operator whose v1.5 deploy already failed.bpatestcannot provide:With
aws-hosted-zone-idempty,ses-construct.ts:28skips the entire SES block — no domain identity and no email templates — so there is no way to send a branded email from that environment. A fresh env also has no v1.5-eraEmailBrandingLogoUrlrows to normalize. These need an environment with a hosted zone and existing v1.5 data (e.g.dev-demo):TESTING.md.Impact
Distribution,OriginAccessControl); theBucketPolicyalready existed and is rewritten under the same logical id. The stack goes 273 → 275 of 500, against a warn threshold of 400 — ample headroom.Additional Information
Branched off the
v1.5tag and targetsmainbecause this is a hotfix for a released version. It will need a back-merge intodevelopmentafter merge, or the fix will be missing there.Every object in the bucket remains world-readable through the CDN (single default behaviour, no path restriction). Exposure is unchanged from v1.5's public bucket — "private" describes the bucket, and this is a deployability fix rather than an access-control tightening.
Orphaned logo objects accumulate from versioned keys; no lifecycle rule bounds them.
Passing
distributionto the defaultsBucketDeployment(so a changed default logo isn't served stale for up to 24h) grants its CDK-generated rolecloudfront:CreateInvalidation. Whether that adds newIAM5findings wasn't isolated against the 276 pre-existing ones; the front-end'swww-hosting-construct.tsdoes the same thing unsuppressed. Two lines to revert if the wider IAM surface isn't wanted here.There is no
CfnOutputfor the distribution domain, so it must be read from the CloudFront console when filling in.env.local. Worth adding as a follow-up.Upgrade path for operators whose v1.5 deploy already failed (the reporting customer included): the rolled-back stack must be deleted, not updated — CloudFormation cannot update a
ROLLBACK_COMPLETEstack. Deleting it leaves the{namePrefix}-*DynamoDB tables behind, becausedynamodb-construct.ts:74,84setsRETAINanddeletionProtection: trueunconditionally, regardless ofenv-type. Those tables carry explicit names, so the next deploy fails withResourceInUseException: Table already existsuntil they are removed (disable deletion protection, then delete). Confirmed on the dev-account reproduction — worth calling out in the v1.5.1 release notes.Checklist*