Skip to content

Commit 68ceecc

Browse files
authored
fix(deploy): fail fast on AWS account mismatch (#1787)
* fix(deploy): fail fast on AWS account mismatch * fix(deploy): validate selected TUI target
1 parent 0053ba6 commit 68ceecc

8 files changed

Lines changed: 121 additions & 28 deletions

File tree

src/cli/aws/__tests__/account-extended.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { AwsCredentialsError } from '../../../lib/errors/types.js';
1+
import { AwsCredentialsError, ValidationError } from '../../../lib/errors/types.js';
22
import { detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
33
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
44

@@ -116,6 +116,22 @@ describe('validateAwsCredentials', () => {
116116
await expect(validateAwsCredentials()).resolves.toBeUndefined();
117117
});
118118

119+
it('does not throw when credentials match the deployment target', async () => {
120+
mockSend.mockResolvedValue({ Account: '123456789012' });
121+
122+
await expect(validateAwsCredentials({ name: 'prod', account: '123456789012' })).resolves.toBeUndefined();
123+
});
124+
125+
it('throws a clear error when credentials do not match the deployment target', async () => {
126+
mockSend.mockResolvedValue({ Account: '111111111111' });
127+
128+
const validation = validateAwsCredentials({ name: 'prod', account: '222222222222' });
129+
await expect(validation).rejects.toBeInstanceOf(ValidationError);
130+
await expect(validation).rejects.toThrow(
131+
'Your AWS credentials are for account 111111111111, but the target "prod" is configured for account 222222222222.'
132+
);
133+
});
134+
119135
it('throws AwsCredentialsError when detectAccount returns null', async () => {
120136
mockSend.mockRejectedValue(new Error('something'));
121137

src/cli/aws/account.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { AwsCredentialsError } from '../../lib/errors/types.js';
1+
import { AwsCredentialsError, ValidationError } from '../../lib/errors/types.js';
2+
import type { AwsDeploymentTarget } from '../../schema';
23
import { getAwsLoginGuidance } from '../external-requirements/checks';
34
import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
45
import { fromEnv, fromNodeProviderChain } from '@aws-sdk/credential-providers';
@@ -61,9 +62,10 @@ export async function detectAccount(): Promise<string | null> {
6162

6263
/**
6364
* Validate that AWS credentials are configured and working.
65+
* When a target is provided, also verify that the credentials belong to its account.
6466
* Throws AwsCredentialsError with a helpful message if not.
6567
*/
66-
export async function validateAwsCredentials(): Promise<void> {
68+
export async function validateAwsCredentials(target?: Pick<AwsDeploymentTarget, 'name' | 'account'>): Promise<void> {
6769
const account = await detectAccount();
6870
if (!account) {
6971
const guidance = await getAwsLoginGuidance();
@@ -75,4 +77,10 @@ export async function validateAwsCredentials(): Promise<void> {
7577
' 2. Or set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY environment variables'
7678
);
7779
}
80+
81+
if (target?.account && account !== target.account) {
82+
throw new ValidationError(
83+
`Your AWS credentials are for account ${account}, but the target "${target.name}" is configured for account ${target.account}.\nEnsure your credentials match the deployment target.`
84+
);
85+
}
7886
}

src/cli/commands/deploy/actions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
246246

247247
// Preflight: validate project
248248
startStep('Validate project');
249-
const context = await validateProject();
249+
const context = await validateProject(target);
250250
endStep('success');
251251

252252
// Warn about imperative-build orphan harnesses (preview→GA transition). These aren't
@@ -282,7 +282,7 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
282282
// Validate AWS credentials (deferred for teardown deploys until after confirmation)
283283
if (context.isTeardownDeploy) {
284284
startStep('Validate AWS credentials');
285-
await validateAwsCredentials();
285+
await validateAwsCredentials(target);
286286
endStep('success');
287287
}
288288

src/cli/operations/deploy/__tests__/preflight.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,43 @@ describe('validateProject', () => {
179179
expect(result.isTeardownDeploy).toBe(false);
180180
});
181181

182+
it('validates credentials against the selected deployment target', async () => {
183+
const selectedTarget = { name: 'prod', account: '222222222222', region: 'us-east-1' } as const;
184+
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
185+
mockValidate.mockReturnValue(undefined);
186+
mockReadProjectSpec.mockResolvedValue({
187+
name: 'test-project',
188+
runtimes: [{ name: 'test-agent' }],
189+
agentCoreGateways: [],
190+
});
191+
mockReadAWSDeploymentTargets.mockResolvedValue([
192+
{ name: 'default', account: '111111111111', region: 'us-west-2' },
193+
selectedTarget,
194+
]);
195+
mockValidateAwsCredentials.mockResolvedValue(undefined);
196+
197+
await validateProject(selectedTarget);
198+
199+
expect(mockValidateAwsCredentials).toHaveBeenCalledWith(selectedTarget);
200+
});
201+
202+
it('validates credentials against the first target when none is selected', async () => {
203+
const firstTarget = { name: 'default', account: '111111111111', region: 'us-west-2' } as const;
204+
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
205+
mockValidate.mockReturnValue(undefined);
206+
mockReadProjectSpec.mockResolvedValue({
207+
name: 'test-project',
208+
runtimes: [{ name: 'test-agent' }],
209+
agentCoreGateways: [],
210+
});
211+
mockReadAWSDeploymentTargets.mockResolvedValue([firstTarget]);
212+
mockValidateAwsCredentials.mockResolvedValue(undefined);
213+
214+
await validateProject();
215+
216+
expect(mockValidateAwsCredentials).toHaveBeenCalledWith(firstTarget);
217+
});
218+
182219
it('accepts gateway target name within 48 chars when prefixed with project name', async () => {
183220
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
184221
mockValidate.mockReturnValue(undefined);

src/cli/operations/deploy/preflight.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export function formatError(err: unknown): string {
7272
* Also validates AWS credentials are configured before proceeding.
7373
* Returns the project context needed for subsequent steps.
7474
*/
75-
export async function validateProject(): Promise<PreflightContext> {
75+
export async function validateProject(selectedTarget?: AwsDeploymentTarget): Promise<PreflightContext> {
7676
// Find the agentcore config directory, walking up from cwd if needed
7777
const configRoot = requireConfigRoot();
7878
// Project root is the parent of the agentcore directory
@@ -152,7 +152,7 @@ export async function validateProject(): Promise<PreflightContext> {
152152
// Validate AWS credentials before proceeding with build/synth.
153153
// Skip for teardown deploys — callers validate after teardown confirmation.
154154
if (!isTeardownDeploy) {
155-
await validateAwsCredentials();
155+
await validateAwsCredentials(selectedTarget ?? awsTargets[0]);
156156
}
157157

158158
return { projectSpec, awsTargets, cdkProject, isTeardownDeploy, isFirstDeploy: !hasExistingStack };

src/cli/tui/hooks/useCdkPreflight.ts

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ConfigIO, SecureCredentials, toError } from '../../../lib';
22
import type { DependencySyncResult } from '../../../lib/dependency-management';
33
import { AwsCredentialsError, DependencySyncError, UserCancellationError } from '../../../lib/errors/types';
4-
import type { DeployedState } from '../../../schema';
4+
import type { AwsDeploymentTarget, DeployedState } from '../../../schema';
55
import { applyTargetRegionToEnv } from '../../aws';
66
import { validateAwsCredentials } from '../../aws/account';
77
import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib';
@@ -40,7 +40,7 @@ const LABEL_PAYMENTS = 'Creating payment infrastructure';
4040

4141
interface RunPaymentSetupOptions {
4242
projectSpec: PreflightContext['projectSpec'];
43-
awsTargets: PreflightContext['awsTargets'];
43+
target: NonNullable<PreflightContext['awsTargets'][0]>;
4444
runtimeCredentials?: SecureCredentials;
4545
logger: ExecLogger;
4646
setSteps: React.Dispatch<React.SetStateAction<Step[]>>;
@@ -57,7 +57,7 @@ interface RunPaymentSetupOptions {
5757
async function runPaymentPreDeploy(opts: RunPaymentSetupOptions): Promise<boolean> {
5858
const {
5959
projectSpec,
60-
awsTargets,
60+
target,
6161
runtimeCredentials,
6262
logger,
6363
setSteps,
@@ -75,7 +75,6 @@ async function runPaymentPreDeploy(opts: RunPaymentSetupOptions): Promise<boolea
7575
});
7676
logger.startStep('Setting up payment credentials...');
7777

78-
const target = awsTargets[0]!;
7978
const paymentConfigIO = new ConfigIO();
8079

8180
const paymentResult = await setupPaymentCredentialProviders({
@@ -147,6 +146,8 @@ export interface PreflightOptions {
147146
isInteractive?: boolean;
148147
/** Skip identity provider check (for plan command which only synthesizes) */
149148
skipIdentityCheck?: boolean;
149+
/** Target selected by the TUI. Falls back to the first configured target when omitted. */
150+
selectedTarget?: AwsDeploymentTarget;
150151
/**
151152
* Preview mode (diff): the managed-dependency sync runs check-only, computing the plan and a
152153
* future-tense notice without writing package.json or running npm install. Previews must never
@@ -220,7 +221,13 @@ const IDENTITY_STEP: Step = { label: LABEL_API_KEY, status: 'pending' };
220221
const BOOTSTRAP_STEP: Step = { label: 'Bootstrap AWS environment', status: 'pending' };
221222

222223
export function useCdkPreflight(options: PreflightOptions): PreflightResult {
223-
const { logger, isInteractive = false, skipIdentityCheck = false, dependencySyncCheckOnly = false } = options;
224+
const {
225+
logger,
226+
isInteractive = false,
227+
skipIdentityCheck = false,
228+
selectedTarget,
229+
dependencySyncCheckOnly = false,
230+
} = options;
224231

225232
// Create switchable ioHost - starts silent, can be flipped to verbose for deploy
226233
const switchableIoHost = useMemo(() => createSwitchableIoHost(), []);
@@ -416,17 +423,18 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
416423
updateStep(STEP_VALIDATE, { status: 'running' });
417424
logger.startStep('Validate project');
418425
let preflightContext: PreflightContext;
426+
let target: AwsDeploymentTarget | undefined;
419427
try {
420-
preflightContext = await validateProject();
428+
preflightContext = await validateProject(selectedTarget);
429+
target = selectedTarget ?? preflightContext.awsTargets[0];
421430
setContext(preflightContext);
422431
// Make aws-targets.json region authoritative for downstream SDK / CDK
423432
// toolkit-lib clients that bypass explicit region options. Restored on
424433
// unmount, teardown rejection, or subsequent preflight start.
425434
// See https://github.com/aws/agentcore-cli/issues/924.
426-
const firstTarget = preflightContext.awsTargets[0];
427-
if (firstTarget) {
435+
if (target) {
428436
restoreRegionEnv();
429-
restoreRegionEnvRef.current = applyTargetRegionToEnv(firstTarget.region);
437+
restoreRegionEnvRef.current = applyTargetRegionToEnv(target.region);
430438
}
431439
logger.endStep('success');
432440
updateStep(STEP_VALIDATE, { status: 'success' });
@@ -460,7 +468,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
460468
// Validate AWS credentials (deferred for teardown deploys until after confirmation)
461469
if (preflightContext.isTeardownDeploy) {
462470
try {
463-
await validateAwsCredentials();
471+
await validateAwsCredentials(target);
464472
} catch (err) {
465473
const errorMsg = formatError(err);
466474
logger.endStep('error', errorMsg);
@@ -584,7 +592,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
584592
// Set up payment resources (no-identity-providers path)
585593
const paymentOk = await runPaymentPreDeploy({
586594
projectSpec: preflightContext.projectSpec,
587-
awsTargets: preflightContext.awsTargets,
595+
target: target!,
588596
logger,
589597
setSteps,
590598
updateStepByLabel,
@@ -625,7 +633,6 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
625633
}
626634

627635
// Step: Check stack status (ensure stacks are not in UPDATE_IN_PROGRESS etc.)
628-
const target = preflightContext.awsTargets[0];
629636
if (target && synthStackNames.length > 0) {
630637
updateStepByLabel(LABEL_STACK_STATUS, { status: 'running' });
631638
logger.startStep('Check stack status');
@@ -705,6 +712,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
705712
dependencySyncCheckOnly,
706713
teardownConfirmed,
707714
restoreRegionEnv,
715+
selectedTarget,
708716
]);
709717

710718
// Handle identity-setup phase (after user provides credentials)
@@ -722,7 +730,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
722730
// Set up payment resources even when identity is skipped
723731
const paymentOkSkip = await runPaymentPreDeploy({
724732
projectSpec: context.projectSpec,
725-
awsTargets: context.awsTargets,
733+
target: (selectedTarget ?? context.awsTargets[0])!,
726734
runtimeCredentials: runtimeCredentials ?? undefined,
727735
logger,
728736
setSteps,
@@ -760,7 +768,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
760768
}
761769

762770
// Check stack status
763-
const target = context.awsTargets[0];
771+
const target = selectedTarget ?? context.awsTargets[0];
764772
if (target && synthStackNames.length > 0) {
765773
updateStepByLabel(LABEL_STACK_STATUS, { status: 'running' });
766774
logger.startStep('Check stack status');
@@ -824,7 +832,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
824832
logger.startStep('Set up API key providers');
825833
}
826834

827-
const target = context.awsTargets[0];
835+
const target = selectedTarget ?? context.awsTargets[0];
828836
if (!target) {
829837
const errorMsg = 'No AWS target configured';
830838
if (hasApiKeys) {
@@ -953,7 +961,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
953961
if (Object.keys(deployedCredentials).length > 0) {
954962
setAllCredentials(deployedCredentials);
955963
const configIO = new ConfigIO();
956-
const target = context.awsTargets[0];
964+
const target = selectedTarget ?? context.awsTargets[0];
957965
const existingState = await configIO.readDeployedState().catch(() => ({ targets: {} }) as DeployedState);
958966
const targetState = existingState.targets?.[target!.name] ?? { resources: {} };
959967
targetState.resources ??= {};
@@ -968,7 +976,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
968976
// Set up payment resources (before CDK synth so ARNs are in deployed state)
969977
const paymentOkIdentity = await runPaymentPreDeploy({
970978
projectSpec: context.projectSpec,
971-
awsTargets: context.awsTargets,
979+
target: (selectedTarget ?? context.awsTargets[0])!,
972980
runtimeCredentials: runtimeCredentials ?? undefined,
973981
logger,
974982
setSteps,
@@ -1071,7 +1079,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
10711079
};
10721080

10731081
void runIdentitySetup();
1074-
}, [phase, context, skipIdentitySetup, runtimeCredentials, logger, switchableIoHost.ioHost]);
1082+
}, [phase, context, skipIdentitySetup, runtimeCredentials, logger, switchableIoHost.ioHost, selectedTarget]);
10751083

10761084
// Handle bootstrapping phase
10771085
useEffect(() => {

src/cli/tui/screens/deploy/__tests__/useDeployFlow.targets.test.tsx

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ const fakeIoHost = {
3737

3838
// Hoisted so the vi.mock factory below (hoisted above module init) can reference it, while the
3939
// test bodies keep a handle to assert the persist path polls the SELECTED target's stack/region.
40-
const { getStackOutputsSpy } = vi.hoisted(() => ({
40+
const { getStackOutputsSpy, useCdkPreflightSpy } = vi.hoisted(() => ({
4141
getStackOutputsSpy: vi.fn().mockRejectedValue(new Error('test: skip persist')),
42+
useCdkPreflightSpy: vi.fn(),
4243
}));
4344

4445
// preflightState is mutated per-test before render so the same mock can vary phase/context.
@@ -48,7 +49,10 @@ vi.mock('../../../hooks', async () => {
4849
const actual = await vi.importActual<any>('../../../hooks');
4950
return {
5051
...actual,
51-
useCdkPreflight: () => preflightState,
52+
useCdkPreflight: (options: unknown) => {
53+
useCdkPreflightSpy(options);
54+
return preflightState;
55+
},
5256
};
5357
});
5458

@@ -144,6 +148,7 @@ describe('useDeployFlow target scoping (issue #1267)', () => {
144148
fakeIoHost.setVerbose.mockClear();
145149
getStackOutputsSpy.mockClear();
146150
getStackOutputsSpy.mockRejectedValue(new Error('test: skip persist'));
151+
useCdkPreflightSpy.mockClear();
147152
});
148153
afterEach(() => {
149154
vi.clearAllTimers();
@@ -169,6 +174,20 @@ describe('useDeployFlow target scoping (issue #1267)', () => {
169174
expect(arg.stacks.patterns).not.toContain(toStackName(PROJECT_NAME, TARGET_A.name));
170175
});
171176

177+
it('passes the first selected target to preflight validation', async () => {
178+
preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] });
179+
180+
const { unmount } = render(<Harness selectedTargets={[TARGET_B]} />);
181+
await flush();
182+
unmount();
183+
184+
expect(useCdkPreflightSpy).toHaveBeenCalledWith(
185+
expect.objectContaining({
186+
selectedTarget: TARGET_B,
187+
})
188+
);
189+
});
190+
172191
it('selecting target A produces only A’s stack (no cross-leak to B)', async () => {
173192
preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] });
174193

src/cli/tui/screens/deploy/useDeployFlow.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,12 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState
162162
// Always call the hook (React rules), but we won't use it when preSynthesized is provided.
163163
// Diff mode is a preview: the managed-dependency sync runs check-only so the working tree
164164
// is never mutated by `agentcore deploy --diff`.
165-
const preflight = useCdkPreflight({ logger, isInteractive, dependencySyncCheckOnly: diffMode });
165+
const preflight = useCdkPreflight({
166+
logger,
167+
isInteractive,
168+
selectedTarget: selectedTargets?.[0],
169+
dependencySyncCheckOnly: diffMode,
170+
});
166171

167172
// Use pre-synthesized values when provided, otherwise use preflight values
168173
const cdkToolkitWrapper = preSynthesized?.cdkToolkitWrapper ?? preflight.cdkToolkitWrapper;

0 commit comments

Comments
 (0)