Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/cli/aws/__tests__/account-extended.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AwsCredentialsError } from '../../../lib/errors/types.js';
import { AwsCredentialsError, ValidationError } from '../../../lib/errors/types.js';
import { detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

Expand Down Expand Up @@ -116,6 +116,22 @@ describe('validateAwsCredentials', () => {
await expect(validateAwsCredentials()).resolves.toBeUndefined();
});

it('does not throw when credentials match the deployment target', async () => {
mockSend.mockResolvedValue({ Account: '123456789012' });

await expect(validateAwsCredentials({ name: 'prod', account: '123456789012' })).resolves.toBeUndefined();
});

it('throws a clear error when credentials do not match the deployment target', async () => {
mockSend.mockResolvedValue({ Account: '111111111111' });

const validation = validateAwsCredentials({ name: 'prod', account: '222222222222' });
await expect(validation).rejects.toBeInstanceOf(ValidationError);
await expect(validation).rejects.toThrow(
'Your AWS credentials are for account 111111111111, but the target "prod" is configured for account 222222222222.'
);
});

it('throws AwsCredentialsError when detectAccount returns null', async () => {
mockSend.mockRejectedValue(new Error('something'));

Expand Down
12 changes: 10 additions & 2 deletions src/cli/aws/account.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AwsCredentialsError } from '../../lib/errors/types.js';
import { AwsCredentialsError, ValidationError } from '../../lib/errors/types.js';
import type { AwsDeploymentTarget } from '../../schema';
import { getAwsLoginGuidance } from '../external-requirements/checks';
import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
import { fromEnv, fromNodeProviderChain } from '@aws-sdk/credential-providers';
Expand Down Expand Up @@ -61,9 +62,10 @@ export async function detectAccount(): Promise<string | null> {

/**
* Validate that AWS credentials are configured and working.
* When a target is provided, also verify that the credentials belong to its account.
* Throws AwsCredentialsError with a helpful message if not.
*/
export async function validateAwsCredentials(): Promise<void> {
export async function validateAwsCredentials(target?: Pick<AwsDeploymentTarget, 'name' | 'account'>): Promise<void> {
const account = await detectAccount();
if (!account) {
const guidance = await getAwsLoginGuidance();
Expand All @@ -75,4 +77,10 @@ export async function validateAwsCredentials(): Promise<void> {
' 2. Or set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY environment variables'
);
}

if (target?.account && account !== target.account) {
throw new ValidationError(
`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.`
);
}
}
4 changes: 2 additions & 2 deletions src/cli/commands/deploy/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep

// Preflight: validate project
startStep('Validate project');
const context = await validateProject();
const context = await validateProject(target);
endStep('success');

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

Expand Down
37 changes: 37 additions & 0 deletions src/cli/operations/deploy/__tests__/preflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,43 @@ describe('validateProject', () => {
expect(result.isTeardownDeploy).toBe(false);
});

it('validates credentials against the selected deployment target', async () => {
const selectedTarget = { name: 'prod', account: '222222222222', region: 'us-east-1' } as const;
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
mockValidate.mockReturnValue(undefined);
mockReadProjectSpec.mockResolvedValue({
name: 'test-project',
runtimes: [{ name: 'test-agent' }],
agentCoreGateways: [],
});
mockReadAWSDeploymentTargets.mockResolvedValue([
{ name: 'default', account: '111111111111', region: 'us-west-2' },
selectedTarget,
]);
mockValidateAwsCredentials.mockResolvedValue(undefined);

await validateProject(selectedTarget);

expect(mockValidateAwsCredentials).toHaveBeenCalledWith(selectedTarget);
});

it('validates credentials against the first target when none is selected', async () => {
const firstTarget = { name: 'default', account: '111111111111', region: 'us-west-2' } as const;
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
mockValidate.mockReturnValue(undefined);
mockReadProjectSpec.mockResolvedValue({
name: 'test-project',
runtimes: [{ name: 'test-agent' }],
agentCoreGateways: [],
});
mockReadAWSDeploymentTargets.mockResolvedValue([firstTarget]);
mockValidateAwsCredentials.mockResolvedValue(undefined);

await validateProject();

expect(mockValidateAwsCredentials).toHaveBeenCalledWith(firstTarget);
});

it('accepts gateway target name within 48 chars when prefixed with project name', async () => {
mockRequireConfigRoot.mockReturnValue('/project/agentcore');
mockValidate.mockReturnValue(undefined);
Expand Down
4 changes: 2 additions & 2 deletions src/cli/operations/deploy/preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export function formatError(err: unknown): string {
* Also validates AWS credentials are configured before proceeding.
* Returns the project context needed for subsequent steps.
*/
export async function validateProject(): Promise<PreflightContext> {
export async function validateProject(selectedTarget?: AwsDeploymentTarget): Promise<PreflightContext> {
// Find the agentcore config directory, walking up from cwd if needed
const configRoot = requireConfigRoot();
// Project root is the parent of the agentcore directory
Expand Down Expand Up @@ -152,7 +152,7 @@ export async function validateProject(): Promise<PreflightContext> {
// Validate AWS credentials before proceeding with build/synth.
// Skip for teardown deploys — callers validate after teardown confirmation.
if (!isTeardownDeploy) {
await validateAwsCredentials();
await validateAwsCredentials(selectedTarget ?? awsTargets[0]);
}

return { projectSpec, awsTargets, cdkProject, isTeardownDeploy, isFirstDeploy: !hasExistingStack };
Expand Down
44 changes: 26 additions & 18 deletions src/cli/tui/hooks/useCdkPreflight.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ConfigIO, SecureCredentials, toError } from '../../../lib';
import type { DependencySyncResult } from '../../../lib/dependency-management';
import { AwsCredentialsError, DependencySyncError, UserCancellationError } from '../../../lib/errors/types';
import type { DeployedState } from '../../../schema';
import type { AwsDeploymentTarget, DeployedState } from '../../../schema';
import { applyTargetRegionToEnv } from '../../aws';
import { validateAwsCredentials } from '../../aws/account';
import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib';
Expand Down Expand Up @@ -40,7 +40,7 @@ const LABEL_PAYMENTS = 'Creating payment infrastructure';

interface RunPaymentSetupOptions {
projectSpec: PreflightContext['projectSpec'];
awsTargets: PreflightContext['awsTargets'];
target: NonNullable<PreflightContext['awsTargets'][0]>;
runtimeCredentials?: SecureCredentials;
logger: ExecLogger;
setSteps: React.Dispatch<React.SetStateAction<Step[]>>;
Expand All @@ -57,7 +57,7 @@ interface RunPaymentSetupOptions {
async function runPaymentPreDeploy(opts: RunPaymentSetupOptions): Promise<boolean> {
const {
projectSpec,
awsTargets,
target,
runtimeCredentials,
logger,
setSteps,
Expand All @@ -75,7 +75,6 @@ async function runPaymentPreDeploy(opts: RunPaymentSetupOptions): Promise<boolea
});
logger.startStep('Setting up payment credentials...');

const target = awsTargets[0]!;
const paymentConfigIO = new ConfigIO();

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

export function useCdkPreflight(options: PreflightOptions): PreflightResult {
const { logger, isInteractive = false, skipIdentityCheck = false, dependencySyncCheckOnly = false } = options;
const {
logger,
isInteractive = false,
skipIdentityCheck = false,
selectedTarget,
dependencySyncCheckOnly = false,
} = options;

// Create switchable ioHost - starts silent, can be flipped to verbose for deploy
const switchableIoHost = useMemo(() => createSwitchableIoHost(), []);
Expand Down Expand Up @@ -416,17 +423,18 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
updateStep(STEP_VALIDATE, { status: 'running' });
logger.startStep('Validate project');
let preflightContext: PreflightContext;
let target: AwsDeploymentTarget | undefined;
try {
preflightContext = await validateProject();
preflightContext = await validateProject(selectedTarget);
target = selectedTarget ?? preflightContext.awsTargets[0];
setContext(preflightContext);
// Make aws-targets.json region authoritative for downstream SDK / CDK
// toolkit-lib clients that bypass explicit region options. Restored on
// unmount, teardown rejection, or subsequent preflight start.
// See https://github.com/aws/agentcore-cli/issues/924.
const firstTarget = preflightContext.awsTargets[0];
if (firstTarget) {
if (target) {
restoreRegionEnv();
restoreRegionEnvRef.current = applyTargetRegionToEnv(firstTarget.region);
restoreRegionEnvRef.current = applyTargetRegionToEnv(target.region);
}
logger.endStep('success');
updateStep(STEP_VALIDATE, { status: 'success' });
Expand Down Expand Up @@ -460,7 +468,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
// Validate AWS credentials (deferred for teardown deploys until after confirmation)
if (preflightContext.isTeardownDeploy) {
try {
await validateAwsCredentials();
await validateAwsCredentials(target);
} catch (err) {
const errorMsg = formatError(err);
logger.endStep('error', errorMsg);
Expand Down Expand Up @@ -584,7 +592,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
// Set up payment resources (no-identity-providers path)
const paymentOk = await runPaymentPreDeploy({
projectSpec: preflightContext.projectSpec,
awsTargets: preflightContext.awsTargets,
target: target!,
logger,
setSteps,
updateStepByLabel,
Expand Down Expand Up @@ -625,7 +633,6 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
}

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

// Handle identity-setup phase (after user provides credentials)
Expand All @@ -722,7 +730,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
// Set up payment resources even when identity is skipped
const paymentOkSkip = await runPaymentPreDeploy({
projectSpec: context.projectSpec,
awsTargets: context.awsTargets,
target: (selectedTarget ?? context.awsTargets[0])!,
runtimeCredentials: runtimeCredentials ?? undefined,
logger,
setSteps,
Expand Down Expand Up @@ -760,7 +768,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
}

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

const target = context.awsTargets[0];
const target = selectedTarget ?? context.awsTargets[0];
if (!target) {
const errorMsg = 'No AWS target configured';
if (hasApiKeys) {
Expand Down Expand Up @@ -953,7 +961,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
if (Object.keys(deployedCredentials).length > 0) {
setAllCredentials(deployedCredentials);
const configIO = new ConfigIO();
const target = context.awsTargets[0];
const target = selectedTarget ?? context.awsTargets[0];
const existingState = await configIO.readDeployedState().catch(() => ({ targets: {} }) as DeployedState);
const targetState = existingState.targets?.[target!.name] ?? { resources: {} };
targetState.resources ??= {};
Expand All @@ -968,7 +976,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
// Set up payment resources (before CDK synth so ARNs are in deployed state)
const paymentOkIdentity = await runPaymentPreDeploy({
projectSpec: context.projectSpec,
awsTargets: context.awsTargets,
target: (selectedTarget ?? context.awsTargets[0])!,
runtimeCredentials: runtimeCredentials ?? undefined,
logger,
setSteps,
Expand Down Expand Up @@ -1071,7 +1079,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
};

void runIdentitySetup();
}, [phase, context, skipIdentitySetup, runtimeCredentials, logger, switchableIoHost.ioHost]);
}, [phase, context, skipIdentitySetup, runtimeCredentials, logger, switchableIoHost.ioHost, selectedTarget]);

// Handle bootstrapping phase
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ const fakeIoHost = {

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

// preflightState is mutated per-test before render so the same mock can vary phase/context.
Expand All @@ -48,7 +49,10 @@ vi.mock('../../../hooks', async () => {
const actual = await vi.importActual<any>('../../../hooks');
return {
...actual,
useCdkPreflight: () => preflightState,
useCdkPreflight: (options: unknown) => {
useCdkPreflightSpy(options);
return preflightState;
},
};
});

Expand Down Expand Up @@ -144,6 +148,7 @@ describe('useDeployFlow target scoping (issue #1267)', () => {
fakeIoHost.setVerbose.mockClear();
getStackOutputsSpy.mockClear();
getStackOutputsSpy.mockRejectedValue(new Error('test: skip persist'));
useCdkPreflightSpy.mockClear();
});
afterEach(() => {
vi.clearAllTimers();
Expand All @@ -169,6 +174,20 @@ describe('useDeployFlow target scoping (issue #1267)', () => {
expect(arg.stacks.patterns).not.toContain(toStackName(PROJECT_NAME, TARGET_A.name));
});

it('passes the first selected target to preflight validation', async () => {
preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] });

const { unmount } = render(<Harness selectedTargets={[TARGET_B]} />);
await flush();
unmount();

expect(useCdkPreflightSpy).toHaveBeenCalledWith(
expect.objectContaining({
selectedTarget: TARGET_B,
})
);
});

it('selecting target A produces only A’s stack (no cross-leak to B)', async () => {
preflightState = makePreflight({ awsTargets: [TARGET_A, TARGET_B] });

Expand Down
7 changes: 6 additions & 1 deletion src/cli/tui/screens/deploy/useDeployFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ export function useDeployFlow(options: DeployFlowOptions = {}): DeployFlowState
// Always call the hook (React rules), but we won't use it when preSynthesized is provided.
// Diff mode is a preview: the managed-dependency sync runs check-only so the working tree
// is never mutated by `agentcore deploy --diff`.
const preflight = useCdkPreflight({ logger, isInteractive, dependencySyncCheckOnly: diffMode });
const preflight = useCdkPreflight({
logger,
isInteractive,
selectedTarget: selectedTargets?.[0],
dependencySyncCheckOnly: diffMode,
});

// Use pre-synthesized values when provided, otherwise use preflight values
const cdkToolkitWrapper = preSynthesized?.cdkToolkitWrapper ?? preflight.cdkToolkitWrapper;
Expand Down
Loading