Skip to content

fix: serve org email assets from a private bucket via CloudFront (OAC) - #929

Open
diegomayorga-dept wants to merge 2 commits into
mainfrom
hotfix/org-email-assets-private-bucket
Open

fix: serve org email assets from a private bucket via CloudFront (OAC)#929
diegomayorga-dept wants to merge 2 commits into
mainfrom
hotfix/org-email-assets-private-bucket

Conversation

@diegomayorga-dept

@diegomayorga-dept diegomayorga-dept commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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*

  • New feature
  • Bug fix
  • Documentation update
  • Refactoring
  • Hotfix
  • Security patch
  • UI/UX improvement

Description

Target: v1.5.1. Branched from the v1.5 tag and targets main, not development.

The org-email-assets bucket introduced in v1.5 required a public-read bucket policy (AnyPrincipal s3: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 returned AccessDenied on AWS::S3::BucketPolicy and 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.com service principal scoped by AWS: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):

  • it geo-restricts to AR/AU/US, and Gmail fetches images through Google's image proxy whose egress country we don't control;
  • it maps 403/404 to /index.html with HTTP 200, so a missing asset returns SPA HTML with a success status;
  • its domain is a front-end stack value, and the front-end deploys after the back-end, so the back-end can't read it at deploy time.

A back-end-owned distribution with no custom domain and no certificate sidesteps all three — it always resolves at its own d….cloudfront.net address, so no environment needs a hosted zone or certificate for email assets to work.

Supporting changes:

  • The distribution's domain reaches every Lambda as ORG_EMAIL_ASSETS_CDN_DOMAIN (common env, not per-Lambda — default-email-branding.ts is reachable from four different senders, so a per-Lambda variable would silently render https://undefined/... in whichever one was missed).
  • Uploaded logo keys are now versioned ({orgId}/logo-{epochMillis}.{ext}) so a replaced logo can't render from CloudFront cache. A ?v= query param can't work — CACHING_OPTIMIZED excludes query strings from the cache key — and explicit invalidation isn't available, since the presigned PUT goes direct from the browser and the handler never learns it completed. Accepted cost: a re-upload orphans the previous object.
  • Legacy URL handling. v1.5 is a released tag and the failure is conditional on account-level BPA, so accounts without it deployed v1.5 successfully and may hold absolute https://<bucket>.s3.<region>.amazonaws.com/... values in EmailBrandingLogoUrl. Those 403 once the bucket is private, and SesService falls back to the default only on a falsy URL. resolveEmailBrandingLogoUrl rewrites 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.example and the local-dev doc gain the new variable.
  • Release notes and upgrade guidance. CHANGELOG.md gains 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.md gains a v1.5 → v1.5.1 Tier 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: a ROLLBACK_COMPLETE stack that CloudFormation cannot update, which on deletion orphans the {namePrefix}-* DynamoDB tables (RETAIN + deletionProtection: true, unconditional) and makes the next deploy fail with ResourceInUseException: Table already exists. Hit first-hand during deploy verification.

S3Url deliberately still points at the S3 endpoint — it's the presigned PUT — and the bucket's CORS rule is retained unchanged. No shared-lib schema change, no front-end change, no ROUTE_SCHEMAS change.

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/order warning 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 four PublicAccessBlockConfiguration controls are true and that no bucket-policy statement grants an Allow to a wildcard principal — with an Effect === 'Allow' filter, because enforceSSL legitimately emits a Deny to Principal: {AWS: '*'}. Further cases cover the distribution and OAC resources, the service-principal grant scoped by AWS: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, and AwsSolutions-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-audit still 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 throwaway env-name: bpatest with 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.5 tag (3444f40d) under account-level BPA fails exactly as reported:

AWS::S3::BucketPolicy  devbpatesteasygenomicsorgemailassetsbucketPolicy78F26541
is not authorized to perform: s3:PutBucketPolicy on resource:
"arn:aws:s3:::dev-bpatest-org-email-assets-bucket" because public policies are
prevented by the BlockPublicPolicy setting in S3 Block Public Access.
(Status Code: 403, HandlerErrorCode: AccessDenied)

Every other CREATE_FAILED in that stack was Resource creation cancelled — this single resource takes the whole deploy down.

This branch then deployed clean under identical settings, confirming the four checks below:

  • Deploy against an AWS account with account-level S3 Block Public Access fully enabled (all four controls)dev-bpatest-easy-genomics-api-stack reached CREATE_COMPLETE with all four controls on. dev-bpatest-main-back-end-stack went UPDATE_COMPLETE from the common Lambda env var.
  • Direct S3 URLs return 403 — anonymous GET of the raw https://dev-bpatest-org-email-assets-bucket.s3.us-west-2.amazonaws.com/defaults/easy-genomics.png returned 403, while the same object through the distribution returned 200 image/png 5699 bytes.
  • An environment with no custom domain configured works — bpatest ran with aws-hosted-zone-id and aws-certificate-arn both empty; the distribution served on its own d34l6xswsri39r.cloudfront.net.
  • Bucket is genuinely private and the policy is not a public policy — IsPublic: false, all four bucket-level controls true, and the only external grant is Allow s3:GetObject to Service: cloudfront.amazonaws.com scoped by AWS:SourceArn to this distribution. ORG_EMAIL_ASSETS_CDN_DOMAIN confirmed 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 with RETAIN + deletionProtection: true and explicit names, so the next deploy fails with ResourceInUseException: Table already exists. See the upgrade note under Additional Information; this affects every operator whose v1.5 deploy already failed.

⚠️ Still not verified — all three need SES, which bpatest cannot provide:

With aws-hosted-zone-id empty, ses-construct.ts:28 skips 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-era EmailBrandingLogoUrl rows to normalize. These need an environment with a hosted zone and existing v1.5 data (e.g. dev-demo):

  • Run-completion email received in Gmail renders both the platform default logo and an org-uploaded logo; verified in one other client.
  • A legacy v1.5 logo URL still renders (paste one into the Email branding field and use the test-email button — it takes the URL from the request body, so no persistence needed). This is the least-tested part of the change: the normalizer was added late in review, after the design's "no backfill needed" premise was found to be wrong.
  • A re-uploaded logo appears immediately rather than from cache.
  • Manual steps recorded in TESTING.md.

Impact

  • +2 CloudFormation resources in the easy-genomics nested stack (Distribution, OriginAccessControl); the BucketPolicy already existed and is rewritten under the same logical id. The stack goes 273 → 275 of 500, against a warn threshold of 400 — ample headroom.
  • One additional CloudFront distribution per environment. No fixed monthly charge; traffic is a handful of image GETs per email.
  • Operators no longer need to relax account-level Block Public Access, and any local workaround patch can be dropped. Net security improvement.
  • No new dependencies. No data migration. No API surface change.
  • First deploy of a new distribution takes several minutes to propagate; emails sent in that window will 404 the logo.

Additional Information

  • Branched off the v1.5 tag and targets main because this is a hotfix for a released version. It will need a back-merge into development after 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 distribution to the defaults BucketDeployment (so a changed default logo isn't served stale for up to 24h) grants its CDK-generated role cloudfront:CreateInvalidation. Whether that adds new IAM5 findings wasn't isolated against the 276 pre-existing ones; the front-end's www-hosting-construct.ts does the same thing unsuppressed. Two lines to revert if the wider IAM surface isn't wanted here.

  • There is no CfnOutput for 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_COMPLETE stack. Deleting it leaves the {namePrefix}-* DynamoDB tables behind, because dynamodb-construct.ts:74,84 sets RETAIN and deletionProtection: true unconditionally, regardless of env-type. Those tables carry explicit names, so the next deploy fails with ResourceInUseException: Table already exists until 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*

  • No new errors or warnings have been introduced.
  • All tests pass successfully and new tests added as necessary.
  • Documentation has been updated accordingly.
  • Code adheres to the coding and style guidelines of the project.
  • Code has been commented in particularly hard-to-understand areas.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants