fix(cli): asset publishing broken cross account (#18007)

In #17668, cross-account S3 asset publishing was broken.

The reason is that the `account()` function was always broken, using the
default account instead of the target account. However, previously this
function was only called in an irrecoverable situation anyway, and its
failure would be rare.

The recent change also calls this function for logging purposes in
a happy-case scenario, but then triggers an error during the logging.

Fix the invocation to use the right account.

Fixes #17988.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/aws-cdk/lib/util/asset-publishing.ts

@@ -50,7 +50,15 @@ class PublishingAws implements cdk_assets.IAws {
   }
 
   public async discoverCurrentAccount(): Promise<cdk_assets.Account> {
-    return (await this.sdk({})).currentAccount();
+    const account = await this.aws.defaultAccount();
+    return account ?? {
+      accountId: '<unknown account>',
+      partition: 'aws',
+    };
+  }
+
+  public async discoverTargetAccount(options: cdk_assets.ClientOptions): Promise<cdk_assets.Account> {
+    return (await this.sdk(options)).currentAccount();
   }
 
   public async s3Client(options: cdk_assets.ClientOptions): Promise<AWS.S3> {

packages/cdk-assets/lib/aws.ts

@@ -8,6 +8,7 @@ export interface IAws {
   discoverDefaultRegion(): Promise<string>;
   discoverCurrentAccount(): Promise<Account>;
 
+  discoverTargetAccount(options: ClientOptions): Promise<Account>;
   s3Client(options: ClientOptions): Promise<AWS.S3>;
   ecrClient(options: ClientOptions): Promise<AWS.ECR>;
   secretsManagerClient(options: ClientOptions): Promise<AWS.SecretsManager>;
@@ -94,6 +95,18 @@ export class DefaultAwsClient implements IAws {
     return this.account;
   }
 
+  public async discoverTargetAccount(options: ClientOptions): Promise<Account> {
+    const sts = new this.AWS.STS(await this.awsOptions(options));
+    const response = await sts.getCallerIdentity().promise();
+    if (!response.Account || !response.Arn) {
+      throw new Error(`Unrecognized reponse from STS: '${JSON.stringify(response)}'`);
+    }
+    return {
+      accountId: response.Account!,
+      partition: response.Arn!.split(':')[1],
+    };
+  }
+
   private async awsOptions(options: ClientOptions) {
     let credentials;
 

packages/cdk-assets/lib/private/handlers/files.ts

@@ -30,7 +30,7 @@ export class FileAssetHandler implements IAssetHandler {
 
     // A thunk for describing the current account. Used when we need to format an error
     // message, not in the success case.
-    const account = async () => (await this.host.aws.discoverCurrentAccount())?.accountId;
+    const account = async () => (await this.host.aws.discoverTargetAccount(destination))?.accountId;
     switch (await bucketInfo.bucketOwnership(s3, destination.bucketName)) {
       case BucketOwnership.MINE:
         break;

packages/cdk-assets/test/fake-listener.ts

@@ -0,0 +1,16 @@
+import { IPublishProgressListener, EventType, IPublishProgress } from '../lib/progress';
+
+export class FakeListener implements IPublishProgressListener {
+  public readonly messages = new Array<string>();
+
+  constructor(private readonly doAbort = false) {
+  }
+
+  public onPublishEvent(_type: EventType, event: IPublishProgress): void {
+    this.messages.push(event.message);
+
+    if (this.doAbort) {
+      event.abort();
+    }
+  }
+}

packages/cdk-assets/test/files.test.ts

@@ -3,6 +3,7 @@ jest.mock('child_process');
 import { Manifest } from '@aws-cdk/cloud-assembly-schema';
 import * as mockfs from 'mock-fs';
 import { AssetManifest, AssetPublishing } from '../lib';
+import { FakeListener } from './fake-listener';
 import { mockAws, mockedApiFailure, mockedApiResult, mockUpload } from './mock-aws';
 import { mockSpawn } from './mock-child_process';
 
@@ -226,7 +227,8 @@ test('will only read bucketEncryption once even for multiple assets', async () =
 });
 
 test('no server side encryption header if access denied for bucket encryption', async () => {
-  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws });
+  const progressListener = new FakeListener();
+  const pub = new AssetPublishing(AssetManifest.fromPath('/simple/cdk.out'), { aws, progressListener });
 
   aws.mockS3.getBucketEncryption = mockedApiFailure('AccessDenied', 'Access Denied');
 
@@ -243,7 +245,8 @@ test('no server side encryption header if access denied for bucket encryption',
     ServerSideEncryption: 'AES256',
   }));
 
-  // We'll just have to assume the contents are correct
+  // Error message references target_account, not current_account
+  expect(progressListener.messages).toContainEqual(expect.stringMatching(/ACCES_DENIED.*target_account/));
 });
 
 test('correctly looks up content type', async () => {
@@ -294,6 +297,7 @@ test('successful run does not need to query account ID', async () => {
   await pub.publish();
 
   expect(aws.discoverCurrentAccount).not.toHaveBeenCalled();
+  expect(aws.discoverTargetAccount).not.toHaveBeenCalled();
 });
 
 test('correctly identify asset path if path is absolute', async () => {

packages/cdk-assets/test/mock-aws.ts

@@ -24,6 +24,7 @@ export function mockAws() {
     discoverPartition: jest.fn(() => Promise.resolve('swa')),
     discoverCurrentAccount: jest.fn(() => Promise.resolve({ accountId: 'current_account', partition: 'swa' })),
     discoverDefaultRegion: jest.fn(() => Promise.resolve('current_region')),
+    discoverTargetAccount: jest.fn(() => Promise.resolve({ accountId: 'target_account', partition: 'swa' })),
     ecrClient: jest.fn(() => Promise.resolve(mockEcr)),
     s3Client: jest.fn(() => Promise.resolve(mockS3)),
     secretsManagerClient: jest.fn(() => Promise.resolve(mockSecretsManager)),
@@ -69,4 +70,4 @@ export function mockUpload(expectContent?: string) {
       });
     }),
   }));
-}
+}

packages/cdk-assets/test/progress.test.ts

@@ -1,6 +1,7 @@
 import { Manifest } from '@aws-cdk/cloud-assembly-schema';
 import * as mockfs from 'mock-fs';
-import { AssetManifest, AssetPublishing, EventType, IPublishProgress, IPublishProgressListener } from '../lib';
+import { AssetManifest, AssetPublishing } from '../lib';
+import { FakeListener } from './fake-listener';
 import { mockAws, mockedApiResult, mockUpload } from './mock-aws';
 
 let aws: ReturnType<typeof mockAws>;
@@ -68,19 +69,4 @@ test('test abort', async () => {
 
   // We never get to asset 2
   expect(allMessages).not.toContain('theAsset:theDestination2');
-});
-
-class FakeListener implements IPublishProgressListener {
-  public readonly messages = new Array<string>();
-
-  constructor(private readonly doAbort = false) {
-  }
-
-  public onPublishEvent(_type: EventType, event: IPublishProgress): void {
-    this.messages.push(event.message);
-
-    if (this.doAbort) {
-      event.abort();
-    }
-  }
-}
+});