fix(cdn-log-delivery): address remaining ABHA61 review findings

- Strict 12-digit accountId validation (reject non-digit chars outright)
- toSafeAwsName guard: throw on IMS org IDs that produce invalid AWS names
- Remove dead ResourceAlreadyExistsException from CreateDelivery catch
- Add 50-page guard to findExistingDelivery paginator
- Audit log on ResourceNotFoundException path in enableCdnLogDelivery
- CDN_LOG_RESCAN_MAX_DISTRIBUTIONS cap in rescanCdnLogDelivery (truncated+totalFound)
- Assert AWS command inputs (put/create) in cdn-log-delivery tests
- Partial-state recovery test (source exists, no delivery -> created:true)
- empty-distributions rescan test + max-distributions cap test

Introduced by: #2680

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: cboldis

src/controllers/llmo/llmo-cloudfront.js

@@ -201,12 +201,13 @@ function LlmoCloudFrontController(ctx) {
   // `{ accountId, distributionId, error }` where `error` is a badRequest Response when validation
   // fails (undefined otherwise).
   const validateCloudfrontCredentials = (context, { requireDistribution = false } = {}) => {
-    const accountId = String(context.data?.accountId || '').replace(/\D/g, '');
+    const rawAccountId = String(context.data?.accountId || '');
     const distributionId = String(context.data?.distributionId || '').trim();
 
-    if (accountId.length !== 12) {
+    if (!/^\d{12}$/.test(rawAccountId)) {
       return { error: badRequest('accountId must be a 12-digit AWS account ID') };
     }
+    const accountId = rawAccountId;
     if (requireDistribution && !hasText(distributionId)) {
       return { error: badRequest('distributionId is required') };
     }
@@ -1228,6 +1229,7 @@ function LlmoCloudFrontController(ctx) {
       } catch (deliveryError) {
         // Adobe destination not provisioned yet → clear 4xx instead of a raw AWS error + retry.
         if (deliveryError?.name === 'ResourceNotFoundException') {
+          log.warn(`[cdn-onboard-cloudfront] Adobe log destination not provisioned for site ${siteId} (imsOrgId=${imsOrgId})`);
           return badRequest('Adobe log destination is not provisioned for this organization yet — run cdn-logs provisioning first');
         }
         throw deliveryError;
@@ -1283,7 +1285,10 @@ function LlmoCloudFrontController(ctx) {
         accountId, externalId, roleName,
       });
 
-      const distributions = await cloudFrontClient.listDistributions();
+      const allDistributions = await cloudFrontClient.listDistributions();
+      const maxDists = Number(env?.CDN_LOG_RESCAN_MAX_DISTRIBUTIONS) || 0;
+      const truncated = maxDists > 0 && allDistributions.length > maxDists;
+      const distributions = truncated ? allDistributions.slice(0, maxDists) : allDistributions;
 
       // Run in bounded batches (CDN_LOG_RESCAN_CONCURRENCY) so a large account doesn't trip
       // CloudWatch Logs per-account throttling. slice()/push() preserve order, so the per-index
@@ -1332,6 +1337,7 @@ function LlmoCloudFrontController(ctx) {
         created: createdCount,
         alreadyExisted,
         failed,
+        ...(truncated && { truncated: true, totalFound: allDistributions.length }),
         distributions: summary,
       });
     } catch (error) {

src/support/cdn-log-delivery.js

@@ -88,7 +88,11 @@ export function buildDeliverySourceName({
   resourceId,
 }) {
   const { sourceNamePrefix } = getProviderConfig(provider);
-  const name = `${sourceNamePrefix}-${toSafeAwsName(imsOrgId)}-${resourceId}`;
+  const safeOrg = toSafeAwsName(imsOrgId);
+  if (!safeOrg || !/^[a-z0-9]/.test(safeOrg)) {
+    throw new Error(`IMS org ID '${imsOrgId}' produces an invalid AWS resource name segment`);
+  }
+  const name = `${sourceNamePrefix}-${safeOrg}-${resourceId}`;
   if (name.length <= MAX_DELIVERY_SOURCE_NAME_LENGTH) {
     return name;
   }
@@ -147,9 +151,12 @@ export async function createCdnLogDelivery(credentials, {
 
   // Find the existing delivery for this source pointing at the expected destination (paginated).
   // The DescribeDeliveries API does not accept a server-side filter; pagination is client-side.
+  const MAX_DESCRIBE_PAGES = 50;
   const findExistingDelivery = async () => {
     let nextToken;
+    let pages = 0;
     do {
+      pages += 1;
       // eslint-disable-next-line no-await-in-loop
       const page = await client.send(new DescribeDeliveriesCommand({
         ...(nextToken && { nextToken }),
@@ -164,7 +171,7 @@ export async function createCdnLogDelivery(credentials, {
         return match;
       }
       nextToken = page.nextToken;
-    } while (nextToken);
+    } while (nextToken && pages < MAX_DESCRIBE_PAGES);
     return undefined;
   };
 
@@ -220,7 +227,7 @@ export async function createCdnLogDelivery(credentials, {
     // TOCTOU: a concurrent enable/rescan for the same distribution can both pass the existence
     // check above and race here; the losing CreateDelivery conflicts. Treat as already-enabled to
     // preserve the idempotency contract instead of surfacing a 500.
-    if (err?.name === 'ConflictException' || err?.name === 'ResourceAlreadyExistsException') {
+    if (err?.name === 'ConflictException') {
       const existing = await findExistingDelivery();
       // deliveryId may be undefined here if DescribeDeliveries hasn't caught up to the winning
       // CreateDelivery yet (eventual consistency). That is acceptable: delivery IS enabled — the

test/controllers/llmo/llmo-cloudfront.test.js

@@ -2295,6 +2295,11 @@ describe('LlmoCloudFrontController', () => {
       const callArgs = createCdnLogDeliveryStub.firstCall.args[1];
       expect(callArgs.provider).to.equal('cloudfront');
       expect(callArgs.resourceId).to.equal('E2EXAMPLE123');
+      expect(callArgs.accountId).to.equal('120569600543');
+      expect(callArgs.imsOrgId).to.equal('ABC123@AdobeOrg');
+      expect(callArgs.deliveryDestinationArn).to.equal(
+        'arn:aws:logs:us-east-1:111122223333:delivery-destination:cdn-logs-org',
+      );
     });
 
     it('is a no-op when forwarding is already enabled', async () => {
@@ -2584,5 +2589,36 @@ describe('LlmoCloudFrontController', () => {
       expect(result.status).to.equal(500);
       expect((await result.json()).message).to.equal('An unexpected error occurred');
     });
+
+    it('returns an empty distribution list when the account has no distributions', async () => {
+      listDistributionsStub.resolves([]);
+
+      const result = await controller.rescanCdnLogDelivery(rescanContext);
+
+      expect(result.status).to.equal(200);
+      const body = await result.json();
+      expect(body.scanned).to.equal(0);
+      expect(body.distributions).to.deep.equal([]);
+      expect(createCdnLogDeliveryStub.called).to.equal(false);
+    });
+
+    it('caps distributions at CDN_LOG_RESCAN_MAX_DISTRIBUTIONS and sets truncated flag', async () => {
+      listDistributionsStub.resolves([
+        { id: 'E2DIST000001' }, { id: 'E2DIST000002' }, { id: 'E2DIST000003' },
+      ]);
+      createCdnLogDeliveryStub.resolves({ created: true, alreadyExisted: false });
+
+      const result = await controller.rescanCdnLogDelivery({
+        ...rescanContext,
+        env: { ...rescanContext.env, CDN_LOG_RESCAN_MAX_DISTRIBUTIONS: '2' },
+      });
+
+      expect(result.status).to.equal(200);
+      const body = await result.json();
+      expect(body.scanned).to.equal(2);
+      expect(body.truncated).to.equal(true);
+      expect(body.totalFound).to.equal(3);
+      expect(createCdnLogDeliveryStub.callCount).to.equal(2);
+    });
   });
 });

test/support/cdn-log-delivery.test.js

@@ -113,11 +113,18 @@ describe('cdn-log-delivery support', () => {
     const rnf = () => Object.assign(new Error('not found'), { name: 'ResourceNotFoundException' });
 
     it('creates source + delivery when none exists', async () => {
+      let putCmd;
+      let createCmd;
       sendStub.callsFake((cmd) => {
         if (cmd.type === 'get') {
           return Promise.reject(rnf());
         }
+        if (cmd.type === 'put') {
+          putCmd = cmd;
+          return Promise.resolve({});
+        }
         if (cmd.type === 'create') {
+          createCmd = cmd;
           return Promise.resolve({ delivery: { id: 'del-new' } });
         }
         return Promise.resolve({});
@@ -129,6 +136,17 @@ describe('cdn-log-delivery support', () => {
       expect(result.alreadyExisted).to.equal(false);
       expect(result.deliveryId).to.equal('del-new');
       expect(result.deliverySourceName).to.equal('llmo-cf-abc123-E2EXAMPLE123');
+
+      // Assert the exact inputs sent to each AWS command.
+      expect(putCmd.input.name).to.equal('llmo-cf-abc123-E2EXAMPLE123');
+      expect(putCmd.input.resourceArn).to.equal(
+        `arn:aws:cloudfront::${baseParams.accountId}:distribution/${baseParams.resourceId}`,
+      );
+      expect(putCmd.input.logType).to.equal('ACCESS_LOGS');
+      expect(createCmd.input.deliverySourceName).to.equal('llmo-cf-abc123-E2EXAMPLE123');
+      expect(createCmd.input.deliveryDestinationArn).to.equal(baseParams.deliveryDestinationArn);
+      expect(createCmd.input.s3DeliveryConfiguration).to.deep.equal({ suffixPath: '/{yyyy}/{MM}/{dd}/{HH}' });
+      expect(createCmd.input.recordFields).to.include('cs-uri-stem');
     });
 
     it('is idempotent when a delivery already exists (paginated)', async () => {
@@ -189,13 +207,35 @@ describe('cdn-log-delivery support', () => {
       expect(describeCalls).to.equal(1); // looked up the winner's delivery id after the conflict
     });
 
+    it('recovers when the source already exists but no delivery is found (partial state)', async () => {
+      sendStub.callsFake((cmd) => {
+        if (cmd.type === 'get') {
+          return Promise.resolve({}); // source already exists → skip PutDeliverySource
+        }
+        if (cmd.type === 'describe') {
+          // No existing delivery found for this source+destination pair.
+          return Promise.resolve({ deliveries: [] });
+        }
+        if (cmd.type === 'create') {
+          return Promise.resolve({ delivery: { id: 'del-recovered' } });
+        }
+        return Promise.resolve({});
+      });
+
+      const result = await mod.createCdnLogDelivery(creds, baseParams);
+
+      expect(result.created).to.equal(true);
+      expect(result.alreadyExisted).to.equal(false);
+      expect(result.deliveryId).to.equal('del-recovered');
+    });
+
     it('on a conflict, returns alreadyExisted with undefined id when the raced delivery is not found', async () => {
       sendStub.callsFake((cmd) => {
         if (cmd.type === 'get') {
           return Promise.reject(rnf()); // source absent → reach PutDeliverySource + CreateDelivery
         }
         if (cmd.type === 'create') {
-          return Promise.reject(Object.assign(new Error('exists'), { name: 'ResourceAlreadyExistsException' }));
+          return Promise.reject(Object.assign(new Error('exists'), { name: 'ConflictException' }));
         }
         if (cmd.type === 'describe') {
           // Paginate through pages with no matching deliverySourceName, then exhaust nextToken