fix(provisioning): canonicalize CloudFront OAI principals in S3 BucketPolicy drift readback (#874)

Author: go-to-k

docs/_generated/integ-last-run.tsv

@@ -136,4 +136,4 @@ opensearch-domain-getatt	2026-06-14T10:54:56Z	PASS	2000	verify.sh	CC-API GetAtt
 s3-vectors	2026-06-14T13:39:25Z	PASS	60	verify.sh	Tags backfill + in-place Tags update() (TagResource/UntagResource) re-run after create dedup; 0 orphans
 dynamodb-ondemand	2026-06-14T14:54:16Z	PASS	120	verify.sh	BillingMode/ProvisionedThroughput in-place UPDATE + backfills; 0 orphans
 glue-update-hardening	2026-06-15T01:57:37Z	PASS		verify.sh	rc ok, orph clean (re-run after review fix; deploy+update+destroy clean)
-s3-cloudfront	2026-06-15T02:31:20Z	PASS		verify.sh	rc ok, orph clean (re-run after OriginGroups review fix; drift clean+detect, clean destroy)
+s3-cloudfront	2026-06-15T04:03:59Z	PASS		verify.sh	rc ok, orph clean (#872 re-run after review fixes; bucket policy + distribution clean; clean destroy)

docs/changelog-cdkd.md

@@ -16,6 +16,10 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
 
 ---
 
+**Recently Implemented** (2026-06-15):
+
+- ✅ **Fix: canonicalize CloudFront OAI grant principals in `S3BucketPolicyProvider.readCurrentState` (closes the phantom drift on an S3 BucketPolicy granting an OAI, issue [#872](https://github.com/go-to-k/cdkd/issues/872))** — `src/provisioning/providers/s3-bucket-policy-provider.ts`. **Bug:** a bucket policy statement that grants a CloudFront Origin Access Identity (OAI) read access stores its principal as the OAI's S3 canonical user id (`{ CanonicalUser: <64-hex> }`, what CDK's `Fn::GetAtt [<OAI>, S3CanonicalUserId]` resolves to). But `s3:GetBucketPolicy` returns that same principal in TWO other unstable forms over the policy's lifetime: a transient `{ AWS: <IAM-unique-id> }` (e.g. `AIDA…`) right after `PutBucketPolicy`, then the settled `{ AWS: arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity <id> }`. The drift comparator saw these three equivalent representations as different and fired a guaranteed false positive on every `cdkd drift` run for any stack with an OAI + S3 bucket policy (the deploy-time `observedProperties` capture caught the transient unique-id form; the later drift read got the ARN form — neither matched the template's canonical user id). **Fix:** `readCurrentState` now canonicalizes every recognizable OAI principal back to `{ CanonicalUser: <id> }`. For the settled `arn:…:cloudfront:user/…<oaiId>` form it maps the OAI id to its `S3CanonicalUserId`, **preferring the same-stack sibling OAI resource's already-read state attribute** (zero AWS call / no `cloudfront:GetCloudFrontOriginAccessIdentity` IAM grant — the OAI's `S3CanonicalUserId` is a readOnly attribute cdkd already resolved at deploy time, threaded into the drift read via the new `ReadCurrentStateContext.siblings[].attributes` + `.physicalId` fields) and falling back to `GetCloudFrontOriginAccessIdentity(<oaiId>)` only when the OAI is NOT a same-stack sibling (an imported / external OAI). The transient bare-IAM-unique-id form (which carries no recoverable link to the OAI) is canonicalized only by matching the corresponding template statement (same Effect / Action / Resource) carrying a `{ CanonicalUser }` principal — safe because a user cannot author a bare IAM unique id as a bucket-policy principal, so it only ever fires for AWS's transient rendering. Both the deploy-time capture and the drift read run through the same normalization, so both converge to the canonical user id and compare equal; a genuinely-different OAI resolves to a different canonical id so real drift is still detected (strict reconcile — never a blanket suppress). Non-OAI principals (`*`, service principals, normal role ARNs) are untouched. The `ReadCurrentStateContext.siblings` shape gained optional `physicalId` + `attributes` (populated by `buildReadCurrentStateContext` in `src/cli/commands/drift.ts`) so any provider can reconcile a sibling's computed identity from already-read state without an extra AWS call. 13 unit tests (ARN→CanonicalUser via sibling state attribute with NO CloudFront call; strict-reconcile: a DIFFERENT OAI resolves to a DIFFERENT canonical id so real drift is NOT suppressed; AWS-array principal + ambiguous-template-match left unchanged; ARN→CanonicalUser via `GetCloudFrontOriginAccessIdentity` fallback when not a sibling; bare-unique-id→template-match; no-template-match left unchanged; best-effort ARN-lookup-failure left unchanged; non-OAI principals untouched; one CloudFront call cached across statements). Validated against real AWS by extending the `s3-cloudfront` integ's `verify.sh` to assert the `AWS::S3::BucketPolicy` reports clean (in the `clean` drift bucket, not `drifted`) on a fresh deploy — the distribution-scoping caveat the CloudFront PR (#871) added is removed now that the bucket policy no longer phantom-drifts. (Approach mirrors the sibling cdk-real-drift project's fix per the #872 cross-note: read the OAI's `S3CanonicalUserId` from already-read state rather than re-fetching it.)
+
 **Recently Implemented** (2026-06-14):
 
 - ✅ **Fix: `AWS::DynamoDB::Table` in-place `BillingMode` / `ProvisionedThroughput` UPDATE (was a silent drop)** — `src/provisioning/providers/dynamodb-table-provider.ts`. **Bug:** `update()` handled OnDemandThroughput / WarmThroughput / PITR / TTL / ResourcePolicy / Kinesis / ContributorInsights / Tags but issued NO `UpdateTable` for `BillingMode` or `ProvisionedThroughput`, even though both are declared in `handledProperties` and both are mutable (CFn createOnly = only `TableName` + `ImportSourceSpecification`). So a pure capacity change (e.g. RCU 5→100, mode unchanged) or a pure billing-mode switch (PROVISIONED↔PAY_PER_REQUEST) was silently dropped — `update()` returned `{ wasReplaced: false }` with no AWS call while cdkd recorded the new value into state as if applied, so the next deploy saw no diff and the AWS-side capacity / mode stayed stale forever (the silent-drift failure mode documented in `feedback_tags_on_update_must_throw`). **Fix:** a new branch, ordered BEFORE the OnDemand/Warm throughput branches, fires a SINGLE `UpdateTable` whenever `BillingMode` OR `ProvisionedThroughput` changed. It forwards `BillingMode` when present and `ProvisionedThroughput` only when the (desired) mode is not `PAY_PER_REQUEST` (AWS rejects caps on on-demand; PROVISIONED requires them), coercing the string-typed capacity values to numbers via `Number()` (matching `create()`; CFn emits numerics as strings). A combined switch-to-PROVISIONED-with-caps now works because both fields ride one call before the OnDemand branch (closing the pre-existing fail-loud caveat the old comment documented). After the call it waits for ACTIVE via the existing `waitForTableActiveAfterUpdate` helper so later branches don't race a still-UPDATING table. Per-index `GlobalSecondaryIndexes` ProvisionedThroughput is explicitly NOT handled (a documented deferred gap, not a silent one — needs `GlobalSecondaryIndexUpdates`). 6 unit tests (pure capacity change PROVISIONED→PROVISIONED; string-numeric coercion; switch to PAY_PER_REQUEST drops caps; PAY_PER_REQUEST switch ignores stale template caps; switch to PROVISIONED sends both in one call; no-change makes no billing UpdateTable). Integ: the `dynamodb-ondemand` fixture gains a standalone PROVISIONED table whose capacity flips RCU 5→20 / WCU 5→10 under `CDKD_TEST_UPDATE=true`, and `verify.sh` adds a Phase-1.5 re-deploy + `describe-table` assertion that AWS reflects the new ProvisionedThroughput (plus a destroy-cleanup poll for the new table).

src/cli/commands/drift.ts

@@ -539,13 +539,14 @@ export function buildReadCurrentStateContext(
   state: StackState,
   excludedLogicalId: string
 ): ReadCurrentStateContext {
-  const siblings: Record<string, { resourceType: string; properties: Record<string, unknown> }> =
-    {};
+  const siblings: NonNullable<ReadCurrentStateContext['siblings']> = {};
   for (const [lid, res] of Object.entries(state.resources ?? {})) {
     if (lid === excludedLogicalId) continue;
     siblings[lid] = {
       resourceType: res.resourceType,
+      physicalId: res.physicalId,
       properties: res.properties ?? {},
+      attributes: res.attributes ?? {},
     };
   }
   return { siblings };

src/provisioning/providers/s3-bucket-policy-provider.ts

@@ -5,16 +5,105 @@ import {
   GetBucketPolicyCommand,
   NoSuchBucket,
 } from '@aws-sdk/client-s3';
+import { GetCloudFrontOriginAccessIdentityCommand } from '@aws-sdk/client-cloudfront';
 import { getLogger } from '../../utils/logger.js';
 import { getAwsClients } from '../../utils/aws-clients.js';
 import { ProvisioningError } from '../../utils/error-handler.js';
 import { assertRegionMatch, type DeleteContext } from '../region-check.js';
+
+/**
+ * Matches a CloudFront Origin Access Identity (OAI) principal ARN, e.g.
+ * `arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity E1UREC9EUJDVG5`.
+ * The capture group is the OAI id, which resolves to the OAI's S3 canonical
+ * user id via `GetCloudFrontOriginAccessIdentity`.
+ */
+const OAI_USER_ARN_RE =
+  /^arn:aws[a-z-]*:iam::cloudfront:user\/CloudFront Origin Access Identity ([A-Z0-9]+)$/;
+
+/**
+ * Matches a bare IAM principal unique id (e.g. `AIDAIBJOSOJSBZ753XCAW`). S3
+ * returns an OAI grant's principal in this transient form immediately after
+ * `PutBucketPolicy` (before it settles to the friendly cloudfront-user ARN).
+ * It carries no recoverable link back to the OAI, so it can only be
+ * canonicalized by matching against the template's `{ CanonicalUser }` form.
+ */
+const IAM_UNIQUE_ID_RE = /^A[A-Z0-9]{15,}$/;
+
+/**
+ * Process-lifetime cache of OAI id -> S3 canonical user id (or `null` when the
+ * lookup failed / the OAI is gone), so a `cdkd drift` run with many OAI-granting
+ * bucket policies issues at most one `GetCloudFrontOriginAccessIdentity` per OAI.
+ */
+const oaiCanonicalUserIdCache = new Map<string, string | null>();
+
+/** Test-only: reset the OAI canonical-user-id cache between unit tests. */
+export function clearOaiCanonicalUserIdCacheForTest(): void {
+  oaiCanonicalUserIdCache.clear();
+}
+
+/**
+ * Build an `oaiId -> S3CanonicalUserId` map from the same-stack sibling
+ * `AWS::CloudFront::CloudFrontOriginAccessIdentity` resources in the read
+ * context. The OAI's `S3CanonicalUserId` is a readOnly attribute cdkd already
+ * resolved at deploy time (the bucket-policy grant `Fn::GetAtt`s it), so this
+ * lets the bucket-policy drift read reconcile the OAI principal forms with zero
+ * extra AWS calls (and no `cloudfront:GetCloudFrontOriginAccessIdentity` IAM
+ * grant) whenever the OAI lives in the same stack. The OAI resource's
+ * `physicalId` IS the OAI id embedded in the cloudfront-user ARN.
+ */
+function buildSiblingOaiCanonicalMap(context?: ReadCurrentStateContext): Map<string, string> {
+  const map = new Map<string, string>();
+  for (const sib of Object.values(context?.siblings ?? {})) {
+    if (sib.resourceType !== 'AWS::CloudFront::CloudFrontOriginAccessIdentity') continue;
+    const canonical = sib.attributes?.['S3CanonicalUserId'];
+    if (sib.physicalId && typeof canonical === 'string') {
+      map.set(sib.physicalId, canonical);
+    }
+  }
+  return map;
+}
+
+/**
+ * Pull the `Statement` array out of a parsed policy document, tolerating both
+ * the single-object and array shapes (and a non-object input). Returns the live
+ * statement objects so callers can mutate their `Principal` in place.
+ */
+function extractStatements(policyDoc: unknown): Record<string, unknown>[] {
+  if (!policyDoc || typeof policyDoc !== 'object') return [];
+  const stmt = (policyDoc as Record<string, unknown>)['Statement'];
+  const arr = Array.isArray(stmt) ? stmt : stmt ? [stmt] : [];
+  return arr.filter(
+    (s): s is Record<string, unknown> => !!s && typeof s === 'object' && !Array.isArray(s)
+  );
+}
+
+/**
+ * Find the `CanonicalUser` principal of the template statement that matches a
+ * given AWS-side statement by Effect / Action / Resource, used to canonicalize
+ * the unresolvable transient `{ AWS: <IAM-unique-id> }` OAI form. Returns
+ * `undefined` when no single matching template statement carries one.
+ */
+function findTemplateCanonicalUser(
+  awsStmt: Record<string, unknown>,
+  templateStmts: Record<string, unknown>[]
+): string | undefined {
+  const key = (s: Record<string, unknown>): string =>
+    JSON.stringify([s['Effect'], s['Action'], s['Resource']]);
+  const want = key(awsStmt);
+  const matches = templateStmts.filter((t) => key(t) === want);
+  if (matches.length !== 1) return undefined;
+  const principal = matches[0]!['Principal'];
+  if (!principal || typeof principal !== 'object' || Array.isArray(principal)) return undefined;
+  const canonical = (principal as Record<string, unknown>)['CanonicalUser'];
+  return typeof canonical === 'string' ? canonical : undefined;
+}
 import type {
   ResourceProvider,
   ResourceCreateResult,
   ResourceUpdateResult,
   ResourceImportInput,
   ResourceImportResult,
+  ReadCurrentStateContext,
 } from '../../types/resource.js';
 
 /**
@@ -238,7 +327,9 @@ export class S3BucketPolicyProvider implements ResourceProvider {
   async readCurrentState(
     physicalId: string,
     _logicalId: string,
-    _resourceType: string
+    _resourceType: string,
+    properties?: Record<string, unknown>,
+    context?: ReadCurrentStateContext
   ): Promise<Record<string, unknown> | undefined> {
     let policyJson: string | undefined;
     try {
@@ -256,14 +347,118 @@ export class S3BucketPolicyProvider implements ResourceProvider {
     const result: Record<string, unknown> = {
       Bucket: physicalId,
     };
+    let policyDoc: unknown;
     try {
-      result['PolicyDocument'] = JSON.parse(policyJson) as unknown;
+      policyDoc = JSON.parse(policyJson) as unknown;
     } catch {
       result['PolicyDocument'] = policyJson;
+      return result;
     }
+
+    // Canonicalize CloudFront OAI grant principals (issue #872). AWS returns an
+    // OAI grant's principal in THREE unstable forms over a policy's lifetime —
+    // the template's `{ CanonicalUser: <64-hex> }`, a transient
+    // `{ AWS: <IAM-unique-id> }` right after PutBucketPolicy, and the settled
+    // `{ AWS: arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity <id> }`.
+    // The drift comparator sees these as different and fires a guaranteed false
+    // positive. Normalize every recognizable OAI principal back to the canonical
+    // user id form (matching what the template carries + what cdkd state holds).
+    await this.normalizeOaiPrincipals(policyDoc, properties, context);
+    result['PolicyDocument'] = policyDoc;
     return result;
   }
 
+  /**
+   * Normalize CloudFront OAI grant principals in a (parsed) bucket policy to
+   * the `{ CanonicalUser: <id> }` form, in place (issue #872):
+   *   - `{ AWS: <oai-user-arn> }` -> map the OAI id in the ARN to its
+   *     `S3CanonicalUserId`, preferring the sibling OAI resource's already-read
+   *     state attributes (zero AWS call — the OAI's `S3CanonicalUserId` is a
+   *     readOnly attribute cdkd already resolved at deploy time) and falling
+   *     back to `GetCloudFrontOriginAccessIdentity` only when the OAI is not a
+   *     same-stack sibling (e.g. an imported / external OAI). This is the SAFE
+   *     path: a genuinely-different OAI resolves to a different canonical id, so
+   *     real drift is still detected.
+   *   - `{ AWS: <bare-IAM-unique-id> }` -> the transient post-PutBucketPolicy
+   *     rendering, which carries no recoverable link to the OAI. Canonicalize
+   *     it only by matching the corresponding template statement (same Effect /
+   *     Action / Resource) carrying a `{ CanonicalUser }` principal. A user
+   *     cannot author a bare IAM unique id as a bucket-policy principal, so
+   *     adopting the template form here only fires for AWS's transient
+   *     rendering. NOTE: once the principal settles to the ARN form (seconds
+   *     to minutes), a genuine out-of-band OAI repoint IS detected via the ARN
+   *     path above; the only masking window is a repoint observed during the
+   *     transient bare-id phase, which the next drift run (ARN form) reports.
+   * Statements whose principal is not a single-string `AWS` OAI form (e.g. a
+   * wildcard, a service principal, a normal role ARN, or an `AWS` ARRAY of
+   * principals) are left untouched — skipping is safe (it can only leave a
+   * residual false positive, never hide real drift). CDK's single-OAI grant
+   * emits the single-string form.
+   */
+  private async normalizeOaiPrincipals(
+    policyDoc: unknown,
+    templateProps?: Record<string, unknown>,
+    context?: ReadCurrentStateContext
+  ): Promise<void> {
+    const stmts = extractStatements(policyDoc);
+    if (stmts.length === 0) return;
+    const templateStmts = extractStatements(templateProps?.['PolicyDocument']);
+    const siblingCanonicalById = buildSiblingOaiCanonicalMap(context);
+
+    for (const stmt of stmts) {
+      const principal = stmt['Principal'];
+      if (!principal || typeof principal !== 'object' || Array.isArray(principal)) continue;
+      const awsPrincipal = (principal as Record<string, unknown>)['AWS'];
+      if (typeof awsPrincipal !== 'string') continue;
+
+      const arnMatch = OAI_USER_ARN_RE.exec(awsPrincipal);
+      if (arnMatch) {
+        const oaiId = arnMatch[1]!;
+        // Prefer the sibling OAI's already-read state attribute; only call AWS
+        // when the OAI is not a same-stack sibling.
+        const canonical =
+          siblingCanonicalById.get(oaiId) ?? (await this.resolveOaiCanonicalUserId(oaiId));
+        if (canonical) stmt['Principal'] = { CanonicalUser: canonical };
+        continue;
+      }
+
+      if (IAM_UNIQUE_ID_RE.test(awsPrincipal)) {
+        const tmplCanonical = findTemplateCanonicalUser(stmt, templateStmts);
+        if (tmplCanonical) stmt['Principal'] = { CanonicalUser: tmplCanonical };
+      }
+    }
+  }
+
+  /**
+   * Resolve a CloudFront OAI id to its S3 canonical user id via
+   * `GetCloudFrontOriginAccessIdentity`, cached for the process lifetime. Used
+   * only as a FALLBACK when the OAI is not a same-stack sibling (its
+   * `S3CanonicalUserId` is otherwise read straight from sibling state — see
+   * {@link buildSiblingOaiCanonicalMap}). Best-effort: returns `null` (and
+   * caches it) on any failure so a missing OAI / missing permission leaves the
+   * principal unchanged rather than failing the drift read.
+   */
+  private async resolveOaiCanonicalUserId(oaiId: string): Promise<string | null> {
+    const cached = oaiCanonicalUserIdCache.get(oaiId);
+    if (cached !== undefined) return cached;
+    let canonical: string | null = null;
+    try {
+      const resp = await getAwsClients().cloudFront.send(
+        new GetCloudFrontOriginAccessIdentityCommand({ Id: oaiId })
+      );
+      canonical = resp.CloudFrontOriginAccessIdentity?.S3CanonicalUserId ?? null;
+    } catch (err) {
+      this.logger.debug(
+        `Could not resolve CloudFront OAI ${oaiId} canonical user id for bucket-policy drift normalization: ${
+          err instanceof Error ? err.message : String(err)
+        } — leaving the principal unchanged.`
+      );
+      canonical = null;
+    }
+    oaiCanonicalUserIdCache.set(oaiId, canonical);
+    return canonical;
+  }
+
   /**
    * Adopt an existing S3 bucket policy into cdkd state.
    *