From 723e96a7a82e5b3e6ec97a27a9f37a1cab8eb287 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 01:54:43 +0000 Subject: [PATCH 01/17] feat(schema): add optional networkConfig.vpcId for Container VPC builds --- .../schemas/__tests__/agent-env.test.ts | 89 +++++++++++++++++++ src/schema/schemas/agent-env.ts | 16 ++++ 2 files changed, 105 insertions(+) diff --git a/src/schema/schemas/__tests__/agent-env.test.ts b/src/schema/schemas/__tests__/agent-env.test.ts index b5b0e55a2..9879c2d77 100644 --- a/src/schema/schemas/__tests__/agent-env.test.ts +++ b/src/schema/schemas/__tests__/agent-env.test.ts @@ -624,3 +624,92 @@ describe('AgentEnvSpecSchema - endpoints', () => { } }); }); + +describe('NetworkConfigSchema - vpcId', () => { + it('accepts valid vpcId', () => { + const result = NetworkConfigSchema.safeParse({ + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + vpcId: 'vpc-12345678', + }); + expect(result.success).toBe(true); + }); + + it('rejects malformed vpcId', () => { + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + vpcId: 'invalid-vpc', + }).success + ).toBe(false); + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + vpcId: 'vpc-1234567', + }).success + ).toBe(false); + }); + + it('omits vpcId when not provided (optional)', () => { + const result = NetworkConfigSchema.safeParse({ + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.vpcId).toBeUndefined(); + } + }); +}); + +describe('AgentEnvSpecSchema - vpcId validation', () => { + const baseVpcAgent = { + name: 'VpcAgent', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/vpc', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + }, + }; + + it('requires vpcId for Container builds in VPC mode', () => { + const result = AgentEnvSpecSchema.safeParse(baseVpcAgent); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.path.includes('vpcId'))).toBe(true); + } + }); + + it('accepts Container+VPC agent with vpcId', () => { + const result = AgentEnvSpecSchema.safeParse({ + ...baseVpcAgent, + networkConfig: { + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + vpcId: 'vpc-12345678', + }, + }); + expect(result.success).toBe(true); + }); + + it('does not require vpcId for CodeZip+VPC', () => { + const result = AgentEnvSpecSchema.safeParse({ + name: 'CodeZipVpcAgent', + build: 'CodeZip', + entrypoint: 'main.py:handler', + codeLocation: './agents/codezipvpc', + runtimeVersion: 'PYTHON_3_12', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-12345678'], + securityGroups: ['sg-12345678'], + }, + }); + expect(result.success).toBe(true); + }); +}); diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index 91d258e19..fd3067543 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -121,6 +121,14 @@ export const NetworkConfigSchema = z.object({ .array(z.string().regex(/^sg-[0-9a-zA-Z]{8,17}$/)) .min(1) .max(16), + /** + * VPC ID. Required for Container builds in VPC mode because CodeBuild needs an explicit VPC ID; + * it cannot infer the VPC from subnets alone. Runtime/Lambda builds can omit this. + */ + vpcId: z + .string() + .regex(/^vpc-[0-9a-zA-Z]{8,17}$/) + .optional(), }); export type NetworkConfig = z.infer; @@ -371,6 +379,14 @@ export const AgentEnvSpecSchema = z path: ['networkConfig'], }); } + if (data.networkMode === 'VPC' && data.build === 'Container' && !data.networkConfig?.vpcId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'networkConfig.vpcId is required for Container builds in VPC mode (CodeBuild cannot infer the VPC from subnets)', + path: ['networkConfig', 'vpcId'], + }); + } if (data.authorizerType === 'CUSTOM_JWT' && !data.authorizerConfiguration?.customJwtAuthorizer) { ctx.addIssue({ code: z.ZodIssueCode.custom, From d7db7b9c9262f5b346352b9a2f845239c81a4dcb Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 01:58:25 +0000 Subject: [PATCH 02/17] feat(cli): add --vpc-id validation for Container VPC builds --- src/cli/commands/add/validate.ts | 2 +- src/cli/commands/create/validate.ts | 2 +- .../shared/__tests__/vpc-utils.test.ts | 46 ++++++++++++++++++- src/cli/commands/shared/vpc-utils.ts | 23 ++++++++-- 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/cli/commands/add/validate.ts b/src/cli/commands/add/validate.ts index 6246c2106..62e468020 100644 --- a/src/cli/commands/add/validate.ts +++ b/src/cli/commands/add/validate.ts @@ -310,7 +310,7 @@ export function validateAddAgentOptions(options: AddAgentOptions): ValidationRes } // Validate VPC options - const vpcResult = validateVpcOptions(options); + const vpcResult = validateVpcOptions(options, options.build); if (!vpcResult.valid) { return { valid: false, error: vpcResult.error }; } diff --git a/src/cli/commands/create/validate.ts b/src/cli/commands/create/validate.ts index ad6dac618..b4a1b7c09 100644 --- a/src/cli/commands/create/validate.ts +++ b/src/cli/commands/create/validate.ts @@ -231,7 +231,7 @@ export function validateCreateOptions(options: CreateOptions, cwd?: string): Val } // Validate VPC options - const vpcResult = validateVpcOptions(options); + const vpcResult = validateVpcOptions(options, options.build); if (!vpcResult.valid) { return { valid: false, error: vpcResult.error }; } diff --git a/src/cli/commands/shared/__tests__/vpc-utils.test.ts b/src/cli/commands/shared/__tests__/vpc-utils.test.ts index 916d6c425..d5b15b78f 100644 --- a/src/cli/commands/shared/__tests__/vpc-utils.test.ts +++ b/src/cli/commands/shared/__tests__/vpc-utils.test.ts @@ -1,4 +1,10 @@ -import { parseCommaSeparatedList, validateSecurityGroupIds, validateSubnetIds, validateVpcOptions } from '../vpc-utils'; +import { + parseCommaSeparatedList, + validateSecurityGroupIds, + validateSubnetIds, + validateVpcId, + validateVpcOptions, +} from '../vpc-utils'; import { describe, expect, it } from 'vitest'; describe('parseCommaSeparatedList', () => { @@ -80,6 +86,44 @@ describe('validateSecurityGroupIds', () => { }); }); +describe('validateVpcId', () => { + it('accepts a valid vpc id', () => { + expect(validateVpcId('vpc-0123456789abcdef0')).toBe(true); + }); + it('rejects a malformed vpc id', () => { + expect(typeof validateVpcId('vpc-xyz')).toBe('string'); + }); +}); + +describe('validateVpcOptions vpcId requirement', () => { + it('requires vpcId for Container + VPC', () => { + const r = validateVpcOptions( + { networkMode: 'VPC', subnets: 'subnet-0123456789abcdef0', securityGroups: 'sg-0123456789abcdef0' }, + 'Container' + ); + expect(r.valid).toBe(false); + }); + it('accepts Container + VPC with vpcId', () => { + const r = validateVpcOptions( + { + networkMode: 'VPC', + subnets: 'subnet-0123456789abcdef0', + securityGroups: 'sg-0123456789abcdef0', + vpcId: 'vpc-0123456789abcdef0', + }, + 'Container' + ); + expect(r.valid).toBe(true); + }); + it('does not require vpcId for CodeZip + VPC', () => { + const r = validateVpcOptions( + { networkMode: 'VPC', subnets: 'subnet-0123456789abcdef0', securityGroups: 'sg-0123456789abcdef0' }, + 'CodeZip' + ); + expect(r.valid).toBe(true); + }); +}); + describe('validateVpcOptions - format validation', () => { it('rejects VPC mode with invalid subnet format', () => { const result = validateVpcOptions({ diff --git a/src/cli/commands/shared/vpc-utils.ts b/src/cli/commands/shared/vpc-utils.ts index e38fbe6c9..1c0e54e74 100644 --- a/src/cli/commands/shared/vpc-utils.ts +++ b/src/cli/commands/shared/vpc-utils.ts @@ -2,6 +2,7 @@ export interface VpcOptions { networkMode?: string; subnets?: string; securityGroups?: string; + vpcId?: string; } export interface VpcValidationResult { @@ -18,6 +19,7 @@ export const VPC_ENDPOINT_WARNING = const SUBNET_PATTERN = /^subnet-[0-9a-zA-Z]{8,17}$/; const SECURITY_GROUP_PATTERN = /^sg-[0-9a-zA-Z]{8,17}$/; +const VPC_ID_PATTERN = /^vpc-[0-9a-zA-Z]{8,17}$/; export function parseCommaSeparatedList(value: string | undefined): string[] | undefined { if (!value) return undefined; @@ -57,7 +59,14 @@ export function validateSecurityGroupIds(value: string): true | string { return true; } -export function validateVpcOptions(options: VpcOptions): VpcValidationResult { +export function validateVpcId(value: string): true | string { + if (!VPC_ID_PATTERN.test(value.trim())) { + return `Invalid VPC ID format: ${value}. Expected vpc-xxxxxxxx`; + } + return true; +} + +export function validateVpcOptions(options: VpcOptions, buildType?: string): VpcValidationResult { if (options.networkMode && options.networkMode !== 'PUBLIC' && options.networkMode !== 'VPC') { return { valid: false, error: `Invalid network mode: ${options.networkMode}. Use PUBLIC or VPC` }; } @@ -74,10 +83,18 @@ export function validateVpcOptions(options: VpcOptions): VpcValidationResult { if (subnetResult !== true) return { valid: false, error: subnetResult }; const sgResult = validateSecurityGroupIds(options.securityGroups); if (sgResult !== true) return { valid: false, error: sgResult }; + + if (buildType === 'Container') { + if (!options.vpcId) { + return { valid: false, error: '--vpc-id is required for Container builds with --network-mode VPC' }; + } + const vpcResult = validateVpcId(options.vpcId); + if (vpcResult !== true) return { valid: false, error: vpcResult }; + } } - if (options.networkMode !== 'VPC' && (options.subnets || options.securityGroups)) { - return { valid: false, error: '--subnets and --security-groups are only valid with --network-mode VPC' }; + if (options.networkMode !== 'VPC' && (options.subnets || options.securityGroups || options.vpcId)) { + return { valid: false, error: '--subnets, --security-groups, and --vpc-id are only valid with --network-mode VPC' }; } return { valid: true }; From f9155f02aba6546c108faefcae64444d6d234821 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 02:08:30 +0000 Subject: [PATCH 03/17] feat(cli): collect and persist networkConfig.vpcId across create/import/export --- src/cli/commands/create/action.ts | 3 +++ src/cli/commands/create/command.tsx | 4 ++++ src/cli/commands/import/types.ts | 2 +- src/cli/commands/import/yaml-parser.ts | 3 +++ .../agent/generate/__tests__/schema-mapper.test.ts | 13 +++++++++++++ src/cli/operations/agent/generate/schema-mapper.ts | 1 + src/cli/primitives/AgentPrimitive.tsx | 9 ++++++++- src/cli/tui/screens/generate/types.ts | 1 + 8 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/create/action.ts b/src/cli/commands/create/action.ts index df3e52537..548d84d34 100644 --- a/src/cli/commands/create/action.ts +++ b/src/cli/commands/create/action.ts @@ -143,6 +143,7 @@ export interface CreateWithAgentOptions { networkMode?: NetworkMode; subnets?: string[]; securityGroups?: string[]; + vpcId?: string; requestHeaderAllowlist?: string[]; agentId?: string; agentAliasId?: string; @@ -174,6 +175,7 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P networkMode, subnets, securityGroups, + vpcId, requestHeaderAllowlist, idleTimeout, maxLifetime: maxLifetimeOpt, @@ -276,6 +278,7 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P networkMode, subnets, securityGroups, + vpcId, requestHeaderAllowlist, ...(idleTimeout !== undefined && { idleRuntimeSessionTimeout: idleTimeout }), ...(maxLifetimeOpt !== undefined && { maxLifetime: maxLifetimeOpt }), diff --git a/src/cli/commands/create/command.tsx b/src/cli/commands/create/command.tsx index 998405da3..bee7111f4 100644 --- a/src/cli/commands/create/command.tsx +++ b/src/cli/commands/create/command.tsx @@ -404,6 +404,7 @@ async function handleCreateCLI(options: CreateOptions): Promise { networkMode: options.networkMode as NetworkMode | undefined, subnets: parseCommaSeparatedList(options.subnets), securityGroups: parseCommaSeparatedList(options.securityGroups), + vpcId: options.vpcId, idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined, maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined, sessionStorageMountPath: options.sessionStorageMountPath, @@ -465,6 +466,7 @@ export const registerCreate = (program: Command) => { .option('--network-mode ', 'Network mode (PUBLIC, VPC) [non-interactive]') .option('--subnets ', 'Comma-separated subnet IDs (required for VPC mode) [non-interactive]') .option('--security-groups ', 'Comma-separated security group IDs (required for VPC mode) [non-interactive]') + .option('--vpc-id ', 'VPC ID (required for Container builds with VPC mode) [non-interactive]') .option( '--idle-timeout ', `Idle session timeout in seconds (${LIFECYCLE_TIMEOUT_MIN}-${LIFECYCLE_TIMEOUT_MAX}) [non-interactive]` @@ -547,6 +549,7 @@ export const registerCreate = (program: Command) => { networkMode?: string; subnets?: string; securityGroups?: string; + vpcId?: string; idleTimeout?: string; maxLifetime?: string; sessionStorageMountPath?: string; @@ -587,6 +590,7 @@ export const registerCreate = (program: Command) => { options.networkMode ?? options.subnets ?? options.securityGroups ?? + options.vpcId ?? options.idleTimeout ?? options.maxLifetime ?? options.outputDir ?? diff --git a/src/cli/commands/import/types.ts b/src/cli/commands/import/types.ts index ac0a061f2..0739eccbc 100644 --- a/src/cli/commands/import/types.ts +++ b/src/cli/commands/import/types.ts @@ -19,7 +19,7 @@ export interface ParsedStarterToolkitAgent { language: 'python' | 'typescript'; sourcePath?: string; networkMode: 'PUBLIC' | 'VPC'; - networkConfig?: { subnets: string[]; securityGroups: string[] }; + networkConfig?: { subnets: string[]; securityGroups: string[]; vpcId?: string }; protocol: 'HTTP' | 'MCP' | 'A2A' | 'AGUI'; enableOtel: boolean; /** Physical agent runtime ID from the starter toolkit deployment */ diff --git a/src/cli/commands/import/yaml-parser.ts b/src/cli/commands/import/yaml-parser.ts index 3002416d7..febc39d00 100644 --- a/src/cli/commands/import/yaml-parser.ts +++ b/src/cli/commands/import/yaml-parser.ts @@ -221,6 +221,9 @@ export function parseStarterToolkitYaml(filePath: string): ParsedStarterToolkitC securityGroups: Array.isArray(networkModeConfig.security_groups) ? (networkModeConfig.security_groups as string[]) : [], + ...(typeof networkModeConfig.vpc_id === 'string' && networkModeConfig.vpc_id + ? { vpcId: networkModeConfig.vpc_id } + : {}), } : undefined, protocol, diff --git a/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts b/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts index 579377afe..a47f032c5 100644 --- a/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts +++ b/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts @@ -245,6 +245,19 @@ describe('mapGenerateConfigToAgent - VPC support', () => { language: 'Python' as const, }; + it('persists vpcId into networkConfig for Container + VPC', () => { + const agent = mapGenerateConfigToAgent({ + ...vpcBaseConfig, + buildType: 'Container', + dockerfile: 'Dockerfile', + networkMode: 'VPC', + subnets: ['subnet-0123456789abcdef0'], + securityGroups: ['sg-0123456789abcdef0'], + vpcId: 'vpc-0123456789abcdef0', + }); + expect(agent.networkConfig?.vpcId).toBe('vpc-0123456789abcdef0'); + }); + it('defaults to PUBLIC network mode when networkMode is absent', () => { const result = mapGenerateConfigToAgent(vpcBaseConfig); expect(result.networkMode).toBe('PUBLIC'); diff --git a/src/cli/operations/agent/generate/schema-mapper.ts b/src/cli/operations/agent/generate/schema-mapper.ts index 6a3f68810..12094eb8d 100644 --- a/src/cli/operations/agent/generate/schema-mapper.ts +++ b/src/cli/operations/agent/generate/schema-mapper.ts @@ -144,6 +144,7 @@ export function mapGenerateConfigToAgent(config: GenerateConfig): AgentEnvSpec { networkConfig: { subnets: config.subnets, securityGroups: config.securityGroups, + ...(config.vpcId && { vpcId: config.vpcId }), }, }), ...(headerAllowlist.length > 0 && { diff --git a/src/cli/primitives/AgentPrimitive.tsx b/src/cli/primitives/AgentPrimitive.tsx index 80ffa0fbf..4d01c0294 100644 --- a/src/cli/primitives/AgentPrimitive.tsx +++ b/src/cli/primitives/AgentPrimitive.tsx @@ -274,6 +274,7 @@ export class AgentPrimitive extends BasePrimitive', 'Network mode (PUBLIC, VPC) [non-interactive]') .option('--subnets ', 'Comma-separated subnet IDs (required for VPC mode) [non-interactive]') .option('--security-groups ', 'Comma-separated security group IDs (required for VPC mode) [non-interactive]') + .option('--vpc-id ', 'VPC ID (required for Container builds with VPC mode) [non-interactive]') .option('--authorizer-type ', 'Inbound auth: AWS_IAM or CUSTOM_JWT [non-interactive]') .option('--discovery-url ', 'OIDC discovery URL (for CUSTOM_JWT) [non-interactive]') .option('--allowed-audience ', 'Comma-separated allowed audiences (for CUSTOM_JWT) [non-interactive]') @@ -421,6 +422,7 @@ export class AgentPrimitive extends BasePrimitive Date: Tue, 30 Jun 2026 03:07:25 +0000 Subject: [PATCH 04/17] fix(schema): require networkConfig.vpcId for container harness builds in VPC mode --- .../primitives/__tests__/harness.test.ts | 39 +++++++++++++++++++ src/schema/schemas/primitives/harness.ts | 8 ++++ 2 files changed, 47 insertions(+) diff --git a/src/schema/schemas/primitives/__tests__/harness.test.ts b/src/schema/schemas/primitives/__tests__/harness.test.ts index c4c89a390..3bada39b9 100644 --- a/src/schema/schemas/primitives/__tests__/harness.test.ts +++ b/src/schema/schemas/primitives/__tests__/harness.test.ts @@ -1256,3 +1256,42 @@ describe('HarnessSpecSchema skills field', () => { } }); }); + +describe('HarnessSpecSchema vpcId for container builds', () => { + const minimalHarnessForVpc = { + name: 'myHarness', + model: { + provider: 'bedrock' as const, + modelId: 'us.anthropic.claude-sonnet-4-5-20250514-v1:0', + }, + }; + const baseDockerfileHarness = { + ...minimalHarnessForVpc, + dockerfile: 'Dockerfile', + networkMode: 'VPC' as const, + networkConfig: { subnets: ['subnet-0123456789abcdef0'], securityGroups: ['sg-0123456789abcdef0'] }, + }; + + it('requires vpcId for a dockerfile build in VPC mode', () => { + const r = HarnessSpecSchema.safeParse(baseDockerfileHarness); + expect(r.success).toBe(false); + }); + + it('accepts a dockerfile build in VPC mode when vpcId is present', () => { + const r = HarnessSpecSchema.safeParse({ + ...baseDockerfileHarness, + networkConfig: { ...baseDockerfileHarness.networkConfig, vpcId: 'vpc-0123456789abcdef0' }, + }); + expect(r.success).toBe(true); + }); + + it('does NOT require vpcId for a containerUri harness in VPC mode (no build)', () => { + const r = HarnessSpecSchema.safeParse({ + ...minimalHarnessForVpc, + containerUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/repo:tag', + networkMode: 'VPC', + networkConfig: { subnets: ['subnet-0123456789abcdef0'], securityGroups: ['sg-0123456789abcdef0'] }, + }); + expect(r.success).toBe(true); + }); +}); diff --git a/src/schema/schemas/primitives/harness.ts b/src/schema/schemas/primitives/harness.ts index b70933e7e..6e2a829ba 100644 --- a/src/schema/schemas/primitives/harness.ts +++ b/src/schema/schemas/primitives/harness.ts @@ -633,6 +633,14 @@ export const HarnessSpecSchema = z path: ['networkConfig'], }); } + if (data.networkMode === 'VPC' && data.dockerfile && !data.networkConfig?.vpcId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'networkConfig.vpcId is required for container (dockerfile) builds in VPC mode (CodeBuild cannot infer the VPC from subnets)', + path: ['networkConfig', 'vpcId'], + }); + } if ((data.efsAccessPoints?.length || data.s3AccessPoints?.length) && data.networkMode !== 'VPC') { ctx.addIssue({ code: z.ZodIssueCode.custom, From 1369d693d3cab62075428aee39761b125301f9e7 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 03:22:09 +0000 Subject: [PATCH 05/17] feat(cli): collect vpcId in create/add-agent/add-harness TUI for Container VPC builds --- src/cli/primitives/HarnessPrimitive.ts | 3 + src/cli/tui/screens/agent/AddAgentScreen.tsx | 33 ++++++ .../agent/__tests__/computeByoSteps.test.ts | 40 +++++++ src/cli/tui/screens/agent/types.ts | 4 + src/cli/tui/screens/agent/useAddAgent.ts | 2 + .../tui/screens/generate/GenerateWizardUI.tsx | 23 +++- .../__tests__/useGenerateWizard.test.tsx | 80 +++++++++++++ src/cli/tui/screens/generate/types.ts | 2 + .../tui/screens/generate/useGenerateWizard.ts | 17 +++ .../tui/screens/harness/AddHarnessFlow.tsx | 1 + .../tui/screens/harness/AddHarnessScreen.tsx | 18 +++ .../useAddHarnessWizard.steps.test.tsx | 107 ++++++++++++++++++ src/cli/tui/screens/harness/types.ts | 4 + .../screens/harness/useAddHarnessWizard.ts | 18 +++ 14 files changed, 351 insertions(+), 1 deletion(-) create mode 100644 src/cli/tui/screens/harness/__tests__/useAddHarnessWizard.steps.test.tsx diff --git a/src/cli/primitives/HarnessPrimitive.ts b/src/cli/primitives/HarnessPrimitive.ts index 1ff48fa1b..c7597af69 100644 --- a/src/cli/primitives/HarnessPrimitive.ts +++ b/src/cli/primitives/HarnessPrimitive.ts @@ -139,6 +139,8 @@ export interface AddHarnessOptions { networkMode?: NetworkMode; subnets?: string[]; securityGroups?: string[]; + /** VPC ID for dockerfile (container) builds in VPC mode; CodeBuild cannot infer it from subnets. */ + vpcId?: string; idleTimeout?: number; maxLifetime?: number; sessionStoragePath?: string; @@ -349,6 +351,7 @@ export class HarnessPrimitive extends BasePrimitive 0 && { requestHeaderAllowlist }), ...(byoAuthorizerType !== 'AWS_IAM' && { authorizerType: byoAuthorizerType }), ...(byoAuthorizerType === 'CUSTOM_JWT' && byoJwtConfig && { jwtConfig: byoJwtConfig }), @@ -580,6 +594,7 @@ export function AddAgentScreen({ existingAgentNames, onComplete, onExit }: AddAg networkMode: 'PUBLIC' as NetworkMode, subnets: '', securityGroups: '', + vpcId: '', requestHeaderAllowlist: '', idleTimeout: '', maxLifetime: '', @@ -944,6 +959,7 @@ export function AddAgentScreen({ existingAgentNames, onComplete, onExit }: AddAg byoStep === 'apiKey' || byoStep === 'subnets' || byoStep === 'securityGroups' || + byoStep === 'vpcId' || byoStep === 'requestHeaderAllowlist' || byoStep === 'idleTimeout' || byoStep === 'maxLifetime' || @@ -1287,6 +1303,20 @@ export function AddAgentScreen({ existingAgentNames, onComplete, onExit }: AddAg /> )} + {byoStep === 'vpcId' && ( + { + setByoConfig(c => ({ ...c, vpcId: value.trim() })); + goToNextByoStep('vpcId'); + }} + onCancel={handleByoBack} + /> + )} + {byoStep === 'requestHeaderAllowlist' && ( { diff --git a/src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts b/src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts index 7bf419d12..bc8dee596 100644 --- a/src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts +++ b/src/cli/tui/screens/agent/__tests__/computeByoSteps.test.ts @@ -61,6 +61,46 @@ describe('computeByoSteps - dockerfile', () => { }); }); +describe('computeByoSteps - vpcId (Container + VPC)', () => { + it('Container + VPC includes vpcId immediately after securityGroups', () => { + const steps = computeByoSteps( + makeInput({ + buildType: 'Container', + networkMode: 'VPC', + advancedSettings: new Set(['network']), + }) + ); + const advIdx = steps.indexOf('advanced'); + expect(steps.slice(advIdx)).toEqual(['advanced', 'networkMode', 'subnets', 'securityGroups', 'vpcId', 'confirm']); + }); + + it('CodeZip + VPC does NOT include vpcId', () => { + const steps = computeByoSteps( + makeInput({ + buildType: 'CodeZip', + networkMode: 'VPC', + advancedSettings: new Set(['network']), + }) + ); + expect(steps).toContain('subnets'); + expect(steps).toContain('securityGroups'); + expect(steps).not.toContain('vpcId'); + }); + + it('Container + PUBLIC does NOT include vpcId (or subnets/securityGroups)', () => { + const steps = computeByoSteps( + makeInput({ + buildType: 'Container', + networkMode: 'PUBLIC', + advancedSettings: new Set(['network']), + }) + ); + expect(steps).not.toContain('vpcId'); + expect(steps).not.toContain('subnets'); + expect(steps).not.toContain('securityGroups'); + }); +}); + describe('computeByoSteps - filesystem', () => { it('filesystem without VPC: includes all filesystem steps (EFS/S3 shown with VPC warning)', () => { const steps = computeByoSteps( diff --git a/src/cli/tui/screens/agent/types.ts b/src/cli/tui/screens/agent/types.ts index 04961476c..7fb85b48c 100644 --- a/src/cli/tui/screens/agent/types.ts +++ b/src/cli/tui/screens/agent/types.ts @@ -52,6 +52,7 @@ export type AddAgentStep = | 'networkMode' | 'subnets' | 'securityGroups' + | 'vpcId' | 'requestHeaderAllowlist' | 'authorizerType' | 'jwtConfig' @@ -93,6 +94,8 @@ export interface AddAgentConfig { subnets?: string[]; /** Security group IDs for VPC mode */ securityGroups?: string[]; + /** VPC ID for Container builds in VPC mode (CodeBuild cannot infer it from subnets) */ + vpcId?: string; /** Allowed request headers for the runtime */ requestHeaderAllowlist?: string[]; /** Authorizer type for inbound requests */ @@ -138,6 +141,7 @@ export const ADD_AGENT_STEP_LABELS: Record = { networkMode: 'Network', subnets: 'Subnets', securityGroups: 'Security Groups', + vpcId: 'VPC ID', requestHeaderAllowlist: 'Headers', authorizerType: 'Auth', jwtConfig: 'JWT Config', diff --git a/src/cli/tui/screens/agent/useAddAgent.ts b/src/cli/tui/screens/agent/useAddAgent.ts index 6f1e18beb..4c3afde86 100644 --- a/src/cli/tui/screens/agent/useAddAgent.ts +++ b/src/cli/tui/screens/agent/useAddAgent.ts @@ -97,6 +97,7 @@ export function mapByoConfigToAgent(config: AddAgentConfig): AgentEnvSpec { networkConfig: { subnets: config.subnets, securityGroups: config.securityGroups, + ...(config.vpcId && { vpcId: config.vpcId }), }, }), ...(config.requestHeaderAllowlist?.length && { @@ -137,6 +138,7 @@ function mapAddAgentConfigToGenerateConfig(config: AddAgentConfig): GenerateConf networkMode: config.networkMode, subnets: config.subnets, securityGroups: config.securityGroups, + vpcId: config.vpcId, requestHeaderAllowlist: config.requestHeaderAllowlist, authorizerType: config.authorizerType, jwtConfig: config.jwtConfig, diff --git a/src/cli/tui/screens/generate/GenerateWizardUI.tsx b/src/cli/tui/screens/generate/GenerateWizardUI.tsx index 5c7217a37..c6b190343 100644 --- a/src/cli/tui/screens/generate/GenerateWizardUI.tsx +++ b/src/cli/tui/screens/generate/GenerateWizardUI.tsx @@ -14,7 +14,7 @@ import { validateS3FilesAccessPointArn, } from '../../../commands/shared/filesystem-utils'; import { parseAndNormalizeHeaders, validateHeaderAllowlist } from '../../../commands/shared/header-utils'; -import { validateSecurityGroupIds, validateSubnetIds } from '../../../commands/shared/vpc-utils'; +import { validateSecurityGroupIds, validateSubnetIds, validateVpcId } from '../../../commands/shared/vpc-utils'; import { computeDefaultCredentialEnvVarName } from '../../../primitives/credential-utils'; import { ApiKeySecretInput, @@ -115,6 +115,7 @@ export function GenerateWizardUI({ const isApiKeyStep = wizard.step === 'apiKey'; const isSubnetsStep = wizard.step === 'subnets'; const isSecurityGroupsStep = wizard.step === 'securityGroups'; + const isVpcIdStep = wizard.step === 'vpcId'; const isRequestHeaderAllowlistStep = wizard.step === 'requestHeaderAllowlist'; const isJwtConfigStep = wizard.step === 'jwtConfig'; const isIdleTimeoutStep = wizard.step === 'idleTimeout'; @@ -337,6 +338,19 @@ export function GenerateWizardUI({ /> )} + {isVpcIdStep && ( + { + wizard.setVpcId(value.trim()); + }} + onCancel={onBack} + /> + )} + {isRequestHeaderAllowlistStep && ( {config.securityGroups.join(', ')} )} + {config.networkMode === 'VPC' && config.vpcId && ( + + VPC ID: + {config.vpcId} + + )} {config.requestHeaderAllowlist && config.requestHeaderAllowlist.length > 0 && ( Headers: diff --git a/src/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx b/src/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx index c42e459b7..9cebb121f 100644 --- a/src/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx +++ b/src/cli/tui/screens/generate/__tests__/useGenerateWizard.test.tsx @@ -800,6 +800,86 @@ describe('useGenerateWizard — advanced config gate', () => { }); }); +describe('useGenerateWizard — vpcId step (Container + VPC)', () => { + function walkToAdvanced(ref: React.RefObject, build: 'CodeZip' | 'Container') { + act(() => { + ref.current!.wizard.setProjectName('Test'); + ref.current!.wizard.setLanguage('Python'); + ref.current!.wizard.setBuildType(build); + ref.current!.wizard.setProtocol('HTTP'); + ref.current!.wizard.setSdk('Strands'); + ref.current!.wizard.setModelProvider('Bedrock'); + ref.current!.wizard.setMemory('none'); + }); + } + + it('Container + VPC: steps include vpcId immediately after securityGroups', () => { + const { ref } = setup(); + walkToAdvanced(ref, 'Container'); + + act(() => ref.current!.wizard.setAdvanced(['network'])); + act(() => ref.current!.wizard.setNetworkMode('VPC')); + + const steps = ref.current!.wizard.steps; + expect(steps).toContain('vpcId'); + const sgIdx = steps.indexOf('securityGroups'); + expect(steps[sgIdx + 1]).toBe('vpcId'); + }); + + it('Container + VPC: setVpcId persists into config and advances to confirm', () => { + vi.useFakeTimers(); + const { ref } = setup(); + walkToAdvanced(ref, 'Container'); + + act(() => ref.current!.wizard.setAdvanced(['network'])); + act(() => ref.current!.wizard.setNetworkMode('VPC')); + act(() => ref.current!.wizard.setSubnets(['subnet-0123456789abcdef0'])); + act(() => { + vi.runAllTimers(); + }); + act(() => ref.current!.wizard.setSecurityGroups(['sg-0123456789abcdef0'])); + act(() => { + vi.runAllTimers(); + }); + expect(ref.current!.wizard.step).toBe('vpcId'); + + act(() => ref.current!.wizard.setVpcId('vpc-0123456789abcdef0')); + act(() => { + vi.runAllTimers(); + }); + + expect(ref.current!.wizard.config.vpcId).toBe('vpc-0123456789abcdef0'); + expect(ref.current!.wizard.step).toBe('confirm'); + vi.useRealTimers(); + }); + + it('CodeZip + VPC: steps do NOT include vpcId', () => { + const { ref } = setup(); + walkToAdvanced(ref, 'CodeZip'); + + act(() => ref.current!.wizard.setAdvanced(['network'])); + act(() => ref.current!.wizard.setNetworkMode('VPC')); + + const steps = ref.current!.wizard.steps; + expect(steps).toContain('subnets'); + expect(steps).toContain('securityGroups'); + expect(steps).not.toContain('vpcId'); + }); + + it('Container + PUBLIC: steps do NOT include vpcId (or subnets/securityGroups)', () => { + const { ref } = setup(); + walkToAdvanced(ref, 'Container'); + + act(() => ref.current!.wizard.setAdvanced(['network'])); + act(() => ref.current!.wizard.setNetworkMode('PUBLIC')); + + const steps = ref.current!.wizard.steps; + expect(steps).not.toContain('vpcId'); + expect(steps).not.toContain('subnets'); + expect(steps).not.toContain('securityGroups'); + }); +}); + describe('validateDockerfileInput', () => { it('accepts empty string (use default)', () => { expect(validateDockerfileInput('')).toBe(true); diff --git a/src/cli/tui/screens/generate/types.ts b/src/cli/tui/screens/generate/types.ts index 542afbf1f..fd28d119e 100644 --- a/src/cli/tui/screens/generate/types.ts +++ b/src/cli/tui/screens/generate/types.ts @@ -31,6 +31,7 @@ export type GenerateStep = | 'networkMode' | 'subnets' | 'securityGroups' + | 'vpcId' | 'requestHeaderAllowlist' | 'authorizerType' | 'jwtConfig' @@ -113,6 +114,7 @@ export const STEP_LABELS: Record = { networkMode: 'Network', subnets: 'Subnets', securityGroups: 'Security Groups', + vpcId: 'VPC ID', requestHeaderAllowlist: 'Headers', authorizerType: 'Auth', jwtConfig: 'JWT Config', diff --git a/src/cli/tui/screens/generate/useGenerateWizard.ts b/src/cli/tui/screens/generate/useGenerateWizard.ts index b94ebc76e..153ded775 100644 --- a/src/cli/tui/screens/generate/useGenerateWizard.ts +++ b/src/cli/tui/screens/generate/useGenerateWizard.ts @@ -72,6 +72,11 @@ export function useGenerateWizard(options?: UseGenerateWizardOptions) { subSteps.push('networkMode'); if (config.networkMode === 'VPC') { subSteps.push('subnets', 'securityGroups'); + // vpcId is required by the schema for Container builds in VPC mode (CodeBuild cannot + // infer the VPC from subnets). CodeZip builds no container, so it is not collected there. + if (config.buildType === 'Container') { + subSteps.push('vpcId'); + } } } // Headers @@ -261,6 +266,7 @@ export function useGenerateWizard(options?: UseGenerateWizardOptions) { networkMode: 'PUBLIC', subnets: undefined, securityGroups: undefined, + vpcId: undefined, requestHeaderAllowlist: undefined, authorizerType: undefined, jwtConfig: undefined, @@ -337,11 +343,21 @@ export function useGenerateWizard(options?: UseGenerateWizardOptions) { const setSecurityGroups = useCallback( (securityGroups: string[]) => { setConfig(c => ({ ...c, securityGroups })); + // When the vpcId step is active (Container build) goToNextStep advances to it; otherwise it + // advances past the network section — mirroring how subnets → securityGroups already works. setTimeout(() => goToNextStep('securityGroups'), 0); }, [goToNextStep] ); + const setVpcId = useCallback( + (vpcId: string) => { + setConfig(c => ({ ...c, vpcId })); + setTimeout(() => goToNextStep('vpcId'), 0); + }, + [goToNextStep] + ); + const setRequestHeaderAllowlist = useCallback( (requestHeaderAllowlist: string[]) => { setConfig(c => ({ ...c, requestHeaderAllowlist })); @@ -528,6 +544,7 @@ export function useGenerateWizard(options?: UseGenerateWizardOptions) { setNetworkMode, setSubnets, setSecurityGroups, + setVpcId, setRequestHeaderAllowlist, skipRequestHeaderAllowlist, setAuthorizerType, diff --git a/src/cli/tui/screens/harness/AddHarnessFlow.tsx b/src/cli/tui/screens/harness/AddHarnessFlow.tsx index 225f1131d..9c79842ed 100644 --- a/src/cli/tui/screens/harness/AddHarnessFlow.tsx +++ b/src/cli/tui/screens/harness/AddHarnessFlow.tsx @@ -81,6 +81,7 @@ export function AddHarnessFlow({ isInteractive = true, onExit, onBack, onDev, on networkMode: config.networkMode, subnets: config.subnets, securityGroups: config.securityGroups, + vpcId: config.vpcId, idleTimeout: config.idleTimeout, maxLifetime: config.maxLifetime, sessionStoragePath: config.sessionStoragePath, diff --git a/src/cli/tui/screens/harness/AddHarnessScreen.tsx b/src/cli/tui/screens/harness/AddHarnessScreen.tsx index 96d9954cd..d68bb571f 100644 --- a/src/cli/tui/screens/harness/AddHarnessScreen.tsx +++ b/src/cli/tui/screens/harness/AddHarnessScreen.tsx @@ -14,6 +14,7 @@ import { validateEfsAccessPointArn, validateS3FilesAccessPointArn, } from '../../../commands/shared/filesystem-utils'; +import { validateVpcId } from '../../../commands/shared/vpc-utils'; import { computeManagedOAuthCredentialName } from '../../../primitives/credential-utils'; import { ConfirmReview, @@ -183,6 +184,7 @@ export function AddHarnessScreen({ const isNetworkModeStep = wizard.step === 'network-mode'; const isSubnetsStep = wizard.step === 'subnets'; const isSecurityGroupsStep = wizard.step === 'security-groups'; + const isVpcIdStep = wizard.step === 'vpc-id'; const isIdleTimeoutStep = wizard.step === 'idle-timeout'; const isMaxLifetimeStep = wizard.step === 'max-lifetime'; const isMaxIterationsStep = wizard.step === 'max-iterations'; @@ -569,6 +571,9 @@ export function AddHarnessScreen({ if (wizard.config.securityGroups) { fields.push({ label: 'Security Groups', value: wizard.config.securityGroups.join(', ') }); } + if (wizard.config.vpcId) { + fields.push({ label: 'VPC ID', value: wizard.config.vpcId }); + } } } @@ -1154,6 +1159,19 @@ export function AddHarnessScreen({ /> )} + {isVpcIdStep && ( + wizard.goBack()} + customValidation={validateVpcId} + /> + )} + {isIdleTimeoutStep && ( ; + +interface HarnessHandle { + wizard: WizardReturn; +} + +const Harness = React.forwardRef((_props, ref) => { + const wizard = useAddHarnessWizard(); + useImperativeHandle(ref, () => ({ wizard })); + return step:{wizard.step}; +}); +Harness.displayName = 'Harness'; + +function setup() { + const ref = React.createRef(); + const result = render(); + return { ref, ...result }; +} + +/** + * Drive the bedrock harness wizard up to the network step with a given container mode and network mode. + * bedrock flow: name → model-provider → api-format → container → memory-mode → advanced → network-mode. + */ +function walkToNetwork( + ref: React.RefObject, + containerMode: ContainerMode, + networkMode: 'PUBLIC' | 'VPC' +) { + act(() => ref.current!.wizard.setName('my-harness')); + act(() => ref.current!.wizard.setModelProvider('bedrock')); + act(() => ref.current!.wizard.setApiFormat('converse_stream')); + act(() => ref.current!.wizard.setContainerMode(containerMode)); + if (containerMode === 'uri') { + act(() => ref.current!.wizard.setContainerUri('123.dkr.ecr.us-west-2.amazonaws.com/img:latest')); + } else if (containerMode === 'dockerfile') { + act(() => ref.current!.wizard.setDockerfilePath('Dockerfile')); + } + act(() => ref.current!.wizard.setMemoryMode('disabled')); + act(() => ref.current!.wizard.setAdvancedSettings(['network'])); + act(() => ref.current!.wizard.setNetworkMode(networkMode)); +} + +describe('useAddHarnessWizard — vpc-id step (dockerfile + VPC)', () => { + it('dockerfile + VPC: steps include vpc-id immediately after security-groups', () => { + const { ref } = setup(); + walkToNetwork(ref, 'dockerfile', 'VPC'); + + const steps = ref.current!.wizard.steps; + expect(steps).toContain('vpc-id'); + const sgIdx = steps.indexOf('security-groups'); + expect(steps[sgIdx + 1]).toBe('vpc-id'); + }); + + it('dockerfile + VPC: setVpcId persists into config and advances off the vpc-id step', () => { + const { ref } = setup(); + walkToNetwork(ref, 'dockerfile', 'VPC'); + + act(() => ref.current!.wizard.setSubnets('subnet-0123456789abcdef0')); + act(() => ref.current!.wizard.setSecurityGroups('sg-0123456789abcdef0')); + expect(ref.current!.wizard.step).toBe('vpc-id'); + + act(() => ref.current!.wizard.setVpcId('vpc-0123456789abcdef0')); + expect(ref.current!.wizard.config.vpcId).toBe('vpc-0123456789abcdef0'); + expect(ref.current!.wizard.step).not.toBe('vpc-id'); + }); + + it('prebuilt containerUri + VPC: steps do NOT include vpc-id', () => { + const { ref } = setup(); + walkToNetwork(ref, 'uri', 'VPC'); + + const steps = ref.current!.wizard.steps; + expect(steps).toContain('subnets'); + expect(steps).toContain('security-groups'); + expect(steps).not.toContain('vpc-id'); + }); + + it('default environment (none) + VPC: steps do NOT include vpc-id', () => { + const { ref } = setup(); + walkToNetwork(ref, 'none', 'VPC'); + + const steps = ref.current!.wizard.steps; + expect(steps).toContain('subnets'); + expect(steps).toContain('security-groups'); + expect(steps).not.toContain('vpc-id'); + }); + + it('dockerfile + PUBLIC: steps do NOT include vpc-id (or subnets/security-groups)', () => { + const { ref } = setup(); + walkToNetwork(ref, 'dockerfile', 'PUBLIC'); + + const steps = ref.current!.wizard.steps; + expect(steps).not.toContain('vpc-id'); + expect(steps).not.toContain('subnets'); + expect(steps).not.toContain('security-groups'); + }); +}); diff --git a/src/cli/tui/screens/harness/types.ts b/src/cli/tui/screens/harness/types.ts index 6c6906d25..306931f9f 100644 --- a/src/cli/tui/screens/harness/types.ts +++ b/src/cli/tui/screens/harness/types.ts @@ -40,6 +40,7 @@ export type AddHarnessStep = | 'network-mode' | 'subnets' | 'security-groups' + | 'vpc-id' | 'idle-timeout' | 'max-lifetime' | 'max-iterations' @@ -104,6 +105,8 @@ export interface AddHarnessConfig { networkMode?: NetworkMode; subnets?: string[]; securityGroups?: string[]; + /** VPC ID for dockerfile (container) builds in VPC mode (CodeBuild cannot infer it from subnets) */ + vpcId?: string; idleTimeout?: number; maxLifetime?: number; sessionStoragePath?: string; @@ -170,6 +173,7 @@ export const HARNESS_STEP_LABELS: Record = { 'network-mode': 'Network mode', subnets: 'Subnets', 'security-groups': 'Security groups', + 'vpc-id': 'VPC ID', 'idle-timeout': 'Idle timeout', 'max-lifetime': 'Max lifetime', 'max-iterations': 'Max iterations', diff --git a/src/cli/tui/screens/harness/useAddHarnessWizard.ts b/src/cli/tui/screens/harness/useAddHarnessWizard.ts index 5d5e2fb5a..828608228 100644 --- a/src/cli/tui/screens/harness/useAddHarnessWizard.ts +++ b/src/cli/tui/screens/harness/useAddHarnessWizard.ts @@ -137,6 +137,12 @@ export function useAddHarnessWizard() { steps.push('network-mode'); if (config.networkMode === 'VPC') { steps.push('subnets', 'security-groups'); + // vpcId is required by the schema for dockerfile (container) builds in VPC mode — CodeBuild + // cannot infer the VPC from subnets. A prebuilt containerUri ('uri') or the default env + // ('none') runs no build, so vpcId is not collected for those. + if (config.containerMode === 'dockerfile') { + steps.push('vpc-id'); + } } } @@ -633,12 +639,23 @@ export function useAddHarnessWizard() { .map(s => s.trim()) .filter(Boolean); setConfig(c => ({ ...c, securityGroups })); + // When the vpc-id step is active (dockerfile build) nextStep advances to it; otherwise it + // advances past the network section — mirroring how subnets → security-groups already works. const next = nextStep('security-groups'); if (next) setStep(next); }, [nextStep] ); + const setVpcId = useCallback( + (vpcId: string) => { + setConfig(c => ({ ...c, vpcId: vpcId.trim() })); + const next = nextStep('vpc-id'); + if (next) setStep(next); + }, + [nextStep] + ); + const setIdleTimeout = useCallback( (idleTimeoutStr: string) => { const idleTimeout = parseInt(idleTimeoutStr, 10); @@ -932,6 +949,7 @@ export function useAddHarnessWizard() { setNetworkMode, setSubnets, setSecurityGroups, + setVpcId, setIdleTimeout, setMaxLifetime, setMaxIterations, From ec83dd360178b8383c41c5191ea031e255566d90 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 03:42:09 +0000 Subject: [PATCH 06/17] feat(cli): add --vpc-id to add harness and cap container-build security groups at 5 Register --vpc-id on `agentcore add harness` so the non-interactive flag path can supply a VPC ID for dockerfile+VPC harnesses (the TUI already collects it; the flag path could not). Thread the flag through AddHarnessCliOptions and the command handler into AddHarnessOptions.vpcId, which add() already spreads into networkConfig. The flag is distinct from --private-endpoint-vpc-id (a separate PrivateLink feature). Also mirror the SG<=5 guard into the CLI's copies of agent-env.ts and harness.ts: Container+VPC and dockerfile+VPC configs with more than 5 security groups now fail validation with a clear error instead of a CodeBuild InvalidInputException at deploy. Non-build runtimes (CodeZip/containerUri) are unaffected and still allow up to 16. validateVpcOptions buildType for the harness path is deferred to the schema: the validateAddHarnessOptions call site does not have the dockerfile signal without restructuring (the container flag is parsed later via parseContainerFlag), so the Task 14 schema rule catches a missing vpcId and the new SG<=5 rule catches an oversized security-group list, both at write time. --- src/cli/commands/add/types.ts | 1 + src/cli/primitives/HarnessPrimitive.ts | 3 + .../HarnessPrimitive.add.vpc.test.ts | 98 +++++++++++++++++++ .../schemas/__tests__/agent-env.test.ts | 63 ++++++++++++ src/schema/schemas/agent-env.ts | 12 +++ .../primitives/__tests__/harness.test.ts | 63 ++++++++++++ src/schema/schemas/primitives/harness.ts | 12 +++ 7 files changed, 252 insertions(+) create mode 100644 src/cli/primitives/__tests__/HarnessPrimitive.add.vpc.test.ts diff --git a/src/cli/commands/add/types.ts b/src/cli/commands/add/types.ts index 465c4d717..49ae70798 100644 --- a/src/cli/commands/add/types.ts +++ b/src/cli/commands/add/types.ts @@ -130,6 +130,7 @@ export interface AddHarnessCliOptions { networkMode?: string; subnets?: string; securityGroups?: string; + vpcId?: string; idleTimeout?: number; maxLifetime?: number; sessionStorage?: string; diff --git a/src/cli/primitives/HarnessPrimitive.ts b/src/cli/primitives/HarnessPrimitive.ts index c7597af69..149c96502 100644 --- a/src/cli/primitives/HarnessPrimitive.ts +++ b/src/cli/primitives/HarnessPrimitive.ts @@ -675,6 +675,7 @@ export class HarnessPrimitive extends BasePrimitive', 'Network mode: PUBLIC or VPC') .option('--subnets ', 'Comma-separated subnet IDs (for VPC mode)') .option('--security-groups ', 'Comma-separated security group IDs (for VPC mode)') + .option('--vpc-id ', 'VPC ID for dockerfile (container) builds in VPC mode') .option('--idle-timeout ', 'Idle timeout in seconds', strictInt('--idle-timeout')) .option('--max-lifetime ', 'Max lifetime in seconds', strictInt('--max-lifetime')) .option('--session-storage ', 'Mount path for persistent session storage (e.g., /mnt/data/)') @@ -784,6 +785,7 @@ export class HarnessPrimitive extends BasePrimitive s.trim()), securityGroups: cliOptions.securityGroups?.split(',').map(s => s.trim()), + vpcId: cliOptions.vpcId, idleTimeout: cliOptions.idleTimeout, maxLifetime: cliOptions.maxLifetime, sessionStoragePath: cliOptions.sessionStorage, diff --git a/src/cli/primitives/__tests__/HarnessPrimitive.add.vpc.test.ts b/src/cli/primitives/__tests__/HarnessPrimitive.add.vpc.test.ts new file mode 100644 index 000000000..e62fbd44e --- /dev/null +++ b/src/cli/primitives/__tests__/HarnessPrimitive.add.vpc.test.ts @@ -0,0 +1,98 @@ +import { HarnessPrimitive } from '../HarnessPrimitive'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// Part B (Task 16): `agentcore add harness --vpc-id` parity for dockerfile+VPC harnesses. The TUI +// already collects a VPC ID; the non-interactive flag path could not. These tests pin the flag +// registration (--vpc-id, distinct from --private-endpoint-vpc-id) and that add() threads vpcId +// into the written harness spec's networkConfig. + +const mockReadProjectSpec = vi.fn(); +const mockWriteProjectSpec = vi.fn(); +const mockWriteHarnessSpec = vi.fn(); + +vi.mock('../../../lib', () => ({ + APP_DIR: 'app', + ConfigIO: class { + readProjectSpec = mockReadProjectSpec; + writeProjectSpec = mockWriteProjectSpec; + writeHarnessSpec = mockWriteHarnessSpec; + getPathResolver = () => ({ getHarnessDir: (name: string) => `/fake/root/app/${name}` }); + }, + findConfigRoot: () => '/fake/root', +})); + +vi.mock('fs/promises', () => ({ + access: vi.fn(), + copyFile: vi.fn(), + mkdir: vi.fn(), + readFile: vi.fn(), + writeFile: vi.fn(), +})); + +function baseProject() { + return { name: 'proj', harnesses: [], memories: [], runtimes: [] }; +} + +/** Runs add() with the given options and returns the networkConfig written to harness.json. */ +async function networkConfigWrittenFor(options: Record) { + mockReadProjectSpec.mockResolvedValue(baseProject()); + const primitive = new HarnessPrimitive(); + const result = await primitive.add({ + name: 'support', + modelProvider: 'bedrock', + modelId: 'anthropic.claude-3', + configBaseDir: '/fake/root', + ...options, + } as never); + expect(result.success).toBe(true); + const [, spec] = mockWriteHarnessSpec.mock.calls.at(-1)!; + return (spec as { networkConfig?: { subnets: string[]; securityGroups: string[]; vpcId?: string } }).networkConfig; +} + +describe('HarnessPrimitive.add — --vpc-id threading', () => { + afterEach(() => vi.clearAllMocks()); + + it('threads vpcId into networkConfig for a dockerfile+VPC harness', async () => { + const networkConfig = await networkConfigWrittenFor({ + dockerfilePath: 'Dockerfile', + networkMode: 'VPC', + subnets: ['subnet-0123456789abcdef0'], + securityGroups: ['sg-0123456789abcdef0'], + vpcId: 'vpc-0123456789abcdef0', + }); + expect(networkConfig).toEqual({ + subnets: ['subnet-0123456789abcdef0'], + securityGroups: ['sg-0123456789abcdef0'], + vpcId: 'vpc-0123456789abcdef0', + }); + }); + + it('omits vpcId from networkConfig when not provided', async () => { + const networkConfig = await networkConfigWrittenFor({ + networkMode: 'VPC', + subnets: ['subnet-0123456789abcdef0'], + securityGroups: ['sg-0123456789abcdef0'], + }); + expect(networkConfig).toEqual({ + subnets: ['subnet-0123456789abcdef0'], + securityGroups: ['sg-0123456789abcdef0'], + }); + }); +}); + +describe('HarnessPrimitive add command — --vpc-id flag registration', () => { + it('registers --vpc-id, distinct from --private-endpoint-vpc-id', () => { + const primitive = new HarnessPrimitive(); + const addCmd = new Command('add'); + const removeCmd = new Command('remove'); + primitive.registerCommands(addCmd, removeCmd); + + const harnessCmd = addCmd.commands.find(c => c.name() === 'harness'); + expect(harnessCmd).toBeDefined(); + + const flags = harnessCmd!.options.map(o => o.long).filter((f): f is string => Boolean(f)); + expect(flags).toContain('--vpc-id'); + expect(flags).toContain('--private-endpoint-vpc-id'); + }); +}); diff --git a/src/schema/schemas/__tests__/agent-env.test.ts b/src/schema/schemas/__tests__/agent-env.test.ts index 9879c2d77..4c1695fcf 100644 --- a/src/schema/schemas/__tests__/agent-env.test.ts +++ b/src/schema/schemas/__tests__/agent-env.test.ts @@ -713,3 +713,66 @@ describe('AgentEnvSpecSchema - vpcId validation', () => { expect(result.success).toBe(true); }); }); + +describe('AgentEnvSpecSchema — SG≤5 for Container builds in VPC mode', () => { + const sixSgs = [ + 'sg-00000000000000001', + 'sg-00000000000000002', + 'sg-00000000000000003', + 'sg-00000000000000004', + 'sg-00000000000000005', + 'sg-00000000000000006', + ]; + const fiveSgs = sixSgs.slice(0, 5); + + it('rejects Container+VPC with 6 security groups', () => { + const result = AgentEnvSpecSchema.safeParse({ + name: 'ContainerVpcAgent', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/container', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: sixSgs, + vpcId: 'vpc-0123456789abcdef0', + }, + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some(i => i.message.includes('5 security groups'))).toBe(true); + } + }); + + it('accepts Container+VPC with exactly 5 security groups', () => { + const result = AgentEnvSpecSchema.safeParse({ + name: 'ContainerVpcAgent', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/container', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: fiveSgs, + vpcId: 'vpc-0123456789abcdef0', + }, + }); + expect(result.success).toBe(true); + }); + + it('accepts CodeZip+VPC with 6 security groups (no CodeBuild, no cap)', () => { + const result = AgentEnvSpecSchema.safeParse({ + name: 'CodeZipVpcAgent', + build: 'CodeZip', + entrypoint: 'main.py:handler', + codeLocation: './agents/codezipvpc', + runtimeVersion: 'PYTHON_3_12', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: sixSgs, + }, + }); + expect(result.success).toBe(true); + }); +}); diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index fd3067543..b1a5a81c8 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -387,6 +387,18 @@ export const AgentEnvSpecSchema = z path: ['networkConfig', 'vpcId'], }); } + if ( + data.networkMode === 'VPC' && + data.build === 'Container' && + data.networkConfig && + data.networkConfig.securityGroups.length > 5 + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Container builds in VPC mode allow at most 5 security groups (CodeBuild limit)', + path: ['networkConfig', 'securityGroups'], + }); + } if (data.authorizerType === 'CUSTOM_JWT' && !data.authorizerConfiguration?.customJwtAuthorizer) { ctx.addIssue({ code: z.ZodIssueCode.custom, diff --git a/src/schema/schemas/primitives/__tests__/harness.test.ts b/src/schema/schemas/primitives/__tests__/harness.test.ts index 3bada39b9..247f1113b 100644 --- a/src/schema/schemas/primitives/__tests__/harness.test.ts +++ b/src/schema/schemas/primitives/__tests__/harness.test.ts @@ -1295,3 +1295,66 @@ describe('HarnessSpecSchema vpcId for container builds', () => { expect(r.success).toBe(true); }); }); + +describe('HarnessSpecSchema — SG≤5 for dockerfile builds in VPC mode', () => { + const minimalHarnessForSg = { + name: 'myHarness', + model: { + provider: 'bedrock' as const, + modelId: 'us.anthropic.claude-sonnet-4-5-20250514-v1:0', + }, + }; + const sixSgs = [ + 'sg-00000000000000001', + 'sg-00000000000000002', + 'sg-00000000000000003', + 'sg-00000000000000004', + 'sg-00000000000000005', + 'sg-00000000000000006', + ]; + const fiveSgs = sixSgs.slice(0, 5); + + it('rejects dockerfile+VPC with 6 security groups', () => { + const r = HarnessSpecSchema.safeParse({ + ...minimalHarnessForSg, + dockerfile: 'Dockerfile', + networkMode: 'VPC' as const, + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: sixSgs, + vpcId: 'vpc-0123456789abcdef0', + }, + }); + expect(r.success).toBe(false); + if (!r.success) { + expect(r.error.issues.some(i => i.message.includes('5 security groups'))).toBe(true); + } + }); + + it('accepts dockerfile+VPC with exactly 5 security groups', () => { + const r = HarnessSpecSchema.safeParse({ + ...minimalHarnessForSg, + dockerfile: 'Dockerfile', + networkMode: 'VPC' as const, + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: fiveSgs, + vpcId: 'vpc-0123456789abcdef0', + }, + }); + expect(r.success).toBe(true); + }); + + it('accepts containerUri+VPC with 6 security groups (no build, no cap)', () => { + const r = HarnessSpecSchema.safeParse({ + ...minimalHarnessForSg, + containerUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/repo:tag', + networkMode: 'VPC', + networkConfig: { + subnets: ['subnet-0123456789abcdef0'], + securityGroups: sixSgs, + }, + }); + expect(r.success).toBe(true); + }); +}); diff --git a/src/schema/schemas/primitives/harness.ts b/src/schema/schemas/primitives/harness.ts index 6e2a829ba..8cd23dce5 100644 --- a/src/schema/schemas/primitives/harness.ts +++ b/src/schema/schemas/primitives/harness.ts @@ -641,6 +641,18 @@ export const HarnessSpecSchema = z path: ['networkConfig', 'vpcId'], }); } + if ( + data.networkMode === 'VPC' && + data.dockerfile && + data.networkConfig && + data.networkConfig.securityGroups.length > 5 + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Container (dockerfile) builds in VPC mode allow at most 5 security groups (CodeBuild limit)', + path: ['networkConfig', 'securityGroups'], + }); + } if ((data.efsAccessPoints?.length || data.s3AccessPoints?.length) && data.networkMode !== 'VPC') { ctx.addIssue({ code: z.ZodIssueCode.custom, From 0e91fe0206b3a6c0920ad617ee0030b8ed902b83 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 05:56:27 +0000 Subject: [PATCH 07/17] fix(cli): persist vpcId from the interactive create wizard for Container VPC builds --- .../generate/__tests__/schema-mapper.test.ts | 47 ++++++++++++++++++- .../__tests__/write-agent-to-project.test.ts | 40 ++++++++++++++++ src/cli/tui/screens/agent/index.ts | 2 +- src/cli/tui/screens/agent/useAddAgent.ts | 6 ++- src/cli/tui/screens/create/useCreateFlow.ts | 27 ++--------- 5 files changed, 97 insertions(+), 25 deletions(-) diff --git a/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts b/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts index a47f032c5..6d5604df6 100644 --- a/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts +++ b/src/cli/operations/agent/generate/__tests__/schema-mapper.test.ts @@ -1,5 +1,6 @@ import { computeManagedOAuthCredentialName } from '../../../../primitives/credential-utils.js'; -import { mapByoConfigToAgent } from '../../../../tui/screens/agent/useAddAgent.js'; +import type { AddAgentConfig } from '../../../../tui/screens/agent/types.js'; +import { mapAddAgentConfigToGenerateConfig, mapByoConfigToAgent } from '../../../../tui/screens/agent/useAddAgent.js'; import type { GenerateConfig } from '../../../../tui/screens/generate/types.js'; import { mapGenerateConfigToAgent, @@ -631,3 +632,47 @@ describe('mapGenerateConfigToRenderConfig - needsOs', () => { expect(result.hasMemory).toBe(false); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// mapAddAgentConfigToGenerateConfig - create-wizard persistence path (vpcId) +// +// Regression for the interactive `agentcore create` wizard dropping vpcId for +// Container + VPC builds: the AddAgentConfig produced by the wizard MUST round-trip +// vpcId through GenerateConfig so mapGenerateConfigToAgent persists +// networkConfig.vpcId (required by the schema for Container builds in VPC mode). +// ───────────────────────────────────────────────────────────────────────────── + +describe('mapAddAgentConfigToGenerateConfig - Container + VPC vpcId', () => { + const containerVpcAddAgentConfig: AddAgentConfig = { + name: 'VpcAgent', + agentType: 'create', + codeLocation: 'VpcAgent/', + entrypoint: 'main.py', + language: 'Python', + buildType: 'Container', + protocol: 'HTTP', + framework: 'Strands', + modelProvider: 'Bedrock', + pythonVersion: 'PYTHON_3_12', + memory: 'none', + networkMode: 'VPC', + subnets: ['subnet-05169b775866f2440'], + securityGroups: ['sg-0390682a9d9f7dd8d'], + vpcId: 'vpc-07086549ccf106a5d', + }; + + it('carries vpcId from AddAgentConfig into GenerateConfig', () => { + const generateConfig = mapAddAgentConfigToGenerateConfig(containerVpcAddAgentConfig); + expect(generateConfig.vpcId).toBe('vpc-07086549ccf106a5d'); + }); + + it('persists networkConfig.vpcId end-to-end for the create path', () => { + const generateConfig = mapAddAgentConfigToGenerateConfig(containerVpcAddAgentConfig); + const agent = mapGenerateConfigToAgent(generateConfig); + expect(agent.networkConfig).toEqual({ + subnets: ['subnet-05169b775866f2440'], + securityGroups: ['sg-0390682a9d9f7dd8d'], + vpcId: 'vpc-07086549ccf106a5d', + }); + }); +}); diff --git a/src/cli/operations/agent/generate/__tests__/write-agent-to-project.test.ts b/src/cli/operations/agent/generate/__tests__/write-agent-to-project.test.ts index c5690360e..dceb0bb9b 100644 --- a/src/cli/operations/agent/generate/__tests__/write-agent-to-project.test.ts +++ b/src/cli/operations/agent/generate/__tests__/write-agent-to-project.test.ts @@ -1,4 +1,6 @@ import type { CredentialStrategy } from '../../../../primitives/CredentialPrimitive.js'; +import type { AddAgentConfig } from '../../../../tui/screens/agent/types.js'; +import { mapAddAgentConfigToGenerateConfig } from '../../../../tui/screens/agent/useAddAgent.js'; import type { GenerateConfig } from '../../../../tui/screens/generate/types.js'; import { randomUUID } from 'node:crypto'; import { mkdir, rm, writeFile } from 'node:fs/promises'; @@ -138,6 +140,44 @@ describe('writeAgentToProject with credentialStrategy', () => { }); }); + describe('Container + VPC create path persists vpcId', () => { + // Mirrors the interactive `agentcore create` wizard: AddAgentConfig → GenerateConfig + // (via the shared mapper used by useCreateFlow) → writeAgentToProject. Regression for the + // wizard dropping vpcId and failing schema validation with empty runtimes. + const containerVpcAddAgentConfig: AddAgentConfig = { + name: 'VpcAgent', + agentType: 'create', + codeLocation: 'VpcAgent/', + entrypoint: 'main.py', + language: 'Python', + buildType: 'Container', + protocol: 'HTTP', + framework: 'Strands', + modelProvider: 'Bedrock', + pythonVersion: 'PYTHON_3_12', + memory: 'none', + networkMode: 'VPC', + subnets: ['subnet-05169b775866f2440'], + securityGroups: ['sg-0390682a9d9f7dd8d'], + vpcId: 'vpc-07086549ccf106a5d', + }; + + it('writes networkConfig.vpcId into the runtime spec', async () => { + const writeAgentToProject = await getWriteAgentToProject(); + const generateConfig = mapAddAgentConfigToGenerateConfig(containerVpcAddAgentConfig); + + await writeAgentToProject(generateConfig, { configBaseDir }); + + const project = await readProject(); + expect(project.runtimes).toHaveLength(1); + expect(project.runtimes[0].networkConfig).toEqual({ + subnets: ['subnet-05169b775866f2440'], + securityGroups: ['sg-0390682a9d9f7dd8d'], + vpcId: 'vpc-07086549ccf106a5d', + }); + }); + }); + describe('without credentialStrategy (backward compatibility)', () => { it('uses mapModelProviderToCredentials behavior', async () => { const writeAgentToProject = await getWriteAgentToProject(); diff --git a/src/cli/tui/screens/agent/index.ts b/src/cli/tui/screens/agent/index.ts index 76f70c340..b4c39a5c7 100644 --- a/src/cli/tui/screens/agent/index.ts +++ b/src/cli/tui/screens/agent/index.ts @@ -1,4 +1,4 @@ export { AddAgentFlow } from './AddAgentFlow'; export { AddAgentScreen } from './AddAgentScreen'; export type { AddAgentConfig, AddAgentStep } from './types'; -export { mapByoConfigToAgent, useAddAgent } from './useAddAgent'; +export { mapAddAgentConfigToGenerateConfig, mapByoConfigToAgent, useAddAgent } from './useAddAgent'; diff --git a/src/cli/tui/screens/agent/useAddAgent.ts b/src/cli/tui/screens/agent/useAddAgent.ts index 4c3afde86..d63aec2cb 100644 --- a/src/cli/tui/screens/agent/useAddAgent.ts +++ b/src/cli/tui/screens/agent/useAddAgent.ts @@ -124,8 +124,12 @@ export function mapByoConfigToAgent(config: AddAgentConfig): AgentEnvSpec { /** * Maps AddAgentConfig to GenerateConfig for the create path. + * + * Shared by the add-agent flow (useAddAgent) and the interactive create wizard + * (useCreateFlow) so both threads carry the same fields — notably `vpcId`, which + * is required by the schema for Container builds in VPC mode. */ -function mapAddAgentConfigToGenerateConfig(config: AddAgentConfig): GenerateConfig { +export function mapAddAgentConfigToGenerateConfig(config: AddAgentConfig): GenerateConfig { return { projectName: config.name, // In create context, this is the agent name buildType: config.buildType, diff --git a/src/cli/tui/screens/create/useCreateFlow.ts b/src/cli/tui/screens/create/useCreateFlow.ts index c1b909531..df9123e2e 100644 --- a/src/cli/tui/screens/create/useCreateFlow.ts +++ b/src/cli/tui/screens/create/useCreateFlow.ts @@ -41,7 +41,7 @@ import { import { CDKRenderer, createRenderer } from '../../../templates'; import { type Step, areStepsComplete, hasStepError } from '../../components'; import { withMinDuration } from '../../utils'; -import { mapByoConfigToAgent } from '../agent'; +import { mapAddAgentConfigToGenerateConfig, mapByoConfigToAgent } from '../agent'; import type { AddAgentConfig } from '../agent/types'; import type { GenerateConfig } from '../generate/types'; import { toMemoryAddOptions } from '../harness/memory-options'; @@ -380,29 +380,12 @@ export function useCreateFlow(cwd: string): CreateFlowState { if (addAgentConfig.agentType === 'create') { await validateFilesystemMounts(); - // Create path: generate agent from template + // Create path: generate agent from template. Reuse the shared mapper so this path + // carries every field the add-agent path does — notably vpcId, required by the + // schema for Container builds in VPC mode. const generateConfig: GenerateConfig = { - projectName: addAgentConfig.name, - buildType: addAgentConfig.buildType, - ...(addAgentConfig.dockerfile && { dockerfile: addAgentConfig.dockerfile }), - protocol: addAgentConfig.protocol, - sdk: addAgentConfig.framework, - modelProvider: addAgentConfig.modelProvider, - memory: addAgentConfig.memory, - language: addAgentConfig.language, + ...mapAddAgentConfigToGenerateConfig(addAgentConfig), apiKey: addAgentConfig.apiKey, - networkMode: addAgentConfig.networkMode, - subnets: addAgentConfig.subnets, - securityGroups: addAgentConfig.securityGroups, - requestHeaderAllowlist: addAgentConfig.requestHeaderAllowlist, - authorizerType: addAgentConfig.authorizerType, - jwtConfig: addAgentConfig.jwtConfig, - idleRuntimeSessionTimeout: addAgentConfig.idleRuntimeSessionTimeout, - maxLifetime: addAgentConfig.maxLifetime, - sessionStorageMountPath: addAgentConfig.sessionStorageMountPath, - efsAccessPoints: addAgentConfig.efsAccessPoints, - s3AccessPoints: addAgentConfig.s3AccessPoints, - withConfigBundle: addAgentConfig.withConfigBundle, }; logger.logSubStep(`Framework: ${generateConfig.sdk}`); From 735ae1c248d98eba8646b04f85ec8abc14b38c66 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 14:18:47 +0000 Subject: [PATCH 08/17] fix(cli): tighten vpc/subnet/sg ID validation to real AWS format Tighten subnet, security-group, and vpcId regexes from the lax `[0-9a-zA-Z]{8,17}` range to the strict alternation `(?:[0-9a-f]{8}|[0-9a-f]{17})` in both the Zod schema and the vpc-utils runtime validators. Adds TDD tests covering accept (8/17-char hex) and reject (uppercase, non-hex, 9-char) cases. Fixes two 16-char test fixtures (`subnet-12345678abcdef12`, `sg-12345678abcdef12`) that are invalid under the new strict pattern. --- .../shared/__tests__/vpc-utils.test.ts | 46 +++++++++- src/cli/commands/shared/vpc-utils.ts | 6 +- .../schemas/__tests__/agent-env.test.ts | 91 +++++++++++++++++++ src/schema/schemas/agent-env.ts | 6 +- 4 files changed, 141 insertions(+), 8 deletions(-) diff --git a/src/cli/commands/shared/__tests__/vpc-utils.test.ts b/src/cli/commands/shared/__tests__/vpc-utils.test.ts index d5b15b78f..5b9f778a8 100644 --- a/src/cli/commands/shared/__tests__/vpc-utils.test.ts +++ b/src/cli/commands/shared/__tests__/vpc-utils.test.ts @@ -33,7 +33,7 @@ describe('validateSubnetIds', () => { it('accepts valid subnet IDs', () => { expect(validateSubnetIds('subnet-12345678')).toBe(true); expect(validateSubnetIds('subnet-12345678, subnet-abcdef12')).toBe(true); - expect(validateSubnetIds('subnet-12345678abcdef12')).toBe(true); + expect(validateSubnetIds('subnet-0123456789abcdef0')).toBe(true); }); it('rejects empty input', () => { @@ -64,7 +64,7 @@ describe('validateSecurityGroupIds', () => { it('accepts valid security group IDs', () => { expect(validateSecurityGroupIds('sg-12345678')).toBe(true); expect(validateSecurityGroupIds('sg-12345678, sg-abcdef12')).toBe(true); - expect(validateSecurityGroupIds('sg-12345678abcdef12')).toBe(true); + expect(validateSecurityGroupIds('sg-0123456789abcdef0')).toBe(true); }); it('rejects empty input', () => { @@ -93,6 +93,48 @@ describe('validateVpcId', () => { it('rejects a malformed vpc id', () => { expect(typeof validateVpcId('vpc-xyz')).toBe('string'); }); + it('accepts 8-char lowercase-hex vpc id', () => { + expect(validateVpcId('vpc-0a1b2c3d')).toBe(true); + }); + it('rejects uppercase vpc id', () => { + expect(validateVpcId('vpc-ABCDEFGH')).toBeTypeOf('string'); + }); + it('rejects non-hex vpc id', () => { + expect(validateVpcId('vpc-zzzzzzzz')).toBeTypeOf('string'); + }); + it('rejects 9-char vpc id', () => { + expect(validateVpcId('vpc-123456789')).toBeTypeOf('string'); + }); +}); + +describe('validateSubnetIds — strict format', () => { + it('accepts 8-char lowercase-hex subnet id', () => { + expect(validateSubnetIds('subnet-0a1b2c3d')).toBe(true); + }); + it('rejects uppercase subnet id', () => { + expect(validateSubnetIds('subnet-ABCDEFGH')).toBeTypeOf('string'); + }); + it('rejects non-hex subnet id', () => { + expect(validateSubnetIds('subnet-zzzzzzzz')).toBeTypeOf('string'); + }); + it('rejects 9-char subnet id', () => { + expect(validateSubnetIds('subnet-0a1b2c3d4')).toBeTypeOf('string'); + }); +}); + +describe('validateSecurityGroupIds — strict format', () => { + it('accepts 8-char lowercase-hex sg id', () => { + expect(validateSecurityGroupIds('sg-0a1b2c3d')).toBe(true); + }); + it('rejects uppercase sg id', () => { + expect(validateSecurityGroupIds('sg-ZZZZZZZZ')).toBeTypeOf('string'); + }); + it('rejects non-hex sg id', () => { + expect(validateSecurityGroupIds('sg-zzzzzzzz')).toBeTypeOf('string'); + }); + it('rejects 9-char sg id', () => { + expect(validateSecurityGroupIds('sg-0a1b2c3d4')).toBeTypeOf('string'); + }); }); describe('validateVpcOptions vpcId requirement', () => { diff --git a/src/cli/commands/shared/vpc-utils.ts b/src/cli/commands/shared/vpc-utils.ts index 1c0e54e74..43b19e754 100644 --- a/src/cli/commands/shared/vpc-utils.ts +++ b/src/cli/commands/shared/vpc-utils.ts @@ -17,9 +17,9 @@ export interface VpcValidationResult { export const VPC_ENDPOINT_WARNING = 'VPC mode may require VPC endpoints for CloudWatch, X-Ray, ECR, and Bedrock depending on your agent configuration. If your agent calls public APIs or uses an API-key-based provider, a NAT gateway or additional endpoints may also be needed.'; -const SUBNET_PATTERN = /^subnet-[0-9a-zA-Z]{8,17}$/; -const SECURITY_GROUP_PATTERN = /^sg-[0-9a-zA-Z]{8,17}$/; -const VPC_ID_PATTERN = /^vpc-[0-9a-zA-Z]{8,17}$/; +const SUBNET_PATTERN = /^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; +const SECURITY_GROUP_PATTERN = /^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; +const VPC_ID_PATTERN = /^vpc-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; export function parseCommaSeparatedList(value: string | undefined): string[] | undefined { if (!value) return undefined; diff --git a/src/schema/schemas/__tests__/agent-env.test.ts b/src/schema/schemas/__tests__/agent-env.test.ts index 4c1695fcf..c2e5d721f 100644 --- a/src/schema/schemas/__tests__/agent-env.test.ts +++ b/src/schema/schemas/__tests__/agent-env.test.ts @@ -274,6 +274,97 @@ describe('AgentEnvSpecSchema', () => { }); }); +describe('NetworkConfigSchema — strict AWS ID format', () => { + it('accepts 8-char lowercase-hex IDs', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-0a1b2c3d'], securityGroups: ['sg-0a1b2c3d'] }).success + ).toBe(true); + }); + + it('accepts 17-char lowercase-hex IDs', () => { + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-0a1b2c3d4e5f60718'], + securityGroups: ['sg-0a1b2c3d4e5f60718'], + vpcId: 'vpc-0a1b2c3d4e5f60718', + }).success + ).toBe(true); + }); + + it('rejects uppercase vpcId', () => { + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-0a1b2c3d'], + securityGroups: ['sg-0a1b2c3d'], + vpcId: 'vpc-ABCDEFGH', + }).success + ).toBe(false); + }); + + it('rejects non-hex vpcId', () => { + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-0a1b2c3d'], + securityGroups: ['sg-0a1b2c3d'], + vpcId: 'vpc-zzzzzzzz', + }).success + ).toBe(false); + }); + + it('rejects 9-char vpcId', () => { + expect( + NetworkConfigSchema.safeParse({ + subnets: ['subnet-0a1b2c3d'], + securityGroups: ['sg-0a1b2c3d'], + vpcId: 'vpc-123456789', + }).success + ).toBe(false); + }); + + it('rejects too-short vpcId', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-0a1b2c3d'], securityGroups: ['sg-0a1b2c3d'], vpcId: 'vpc-xyz' }) + .success + ).toBe(false); + }); + + it('rejects uppercase subnet ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-ABCDEFGH'], securityGroups: ['sg-0a1b2c3d'] }).success + ).toBe(false); + }); + + it('rejects non-hex subnet ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-zzzzzzzz'], securityGroups: ['sg-0a1b2c3d'] }).success + ).toBe(false); + }); + + it('rejects 9-char subnet ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-123456789'], securityGroups: ['sg-0a1b2c3d'] }).success + ).toBe(false); + }); + + it('rejects uppercase security group ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-0a1b2c3d'], securityGroups: ['sg-ABCDEFGH'] }).success + ).toBe(false); + }); + + it('rejects non-hex security group ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-0a1b2c3d'], securityGroups: ['sg-zzzzzzzz'] }).success + ).toBe(false); + }); + + it('rejects 9-char security group ID', () => { + expect( + NetworkConfigSchema.safeParse({ subnets: ['subnet-0a1b2c3d'], securityGroups: ['sg-123456789'] }).success + ).toBe(false); + }); +}); + describe('NetworkConfigSchema', () => { it('accepts valid subnets and security groups', () => { const result = NetworkConfigSchema.safeParse({ diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index b1a5a81c8..022614b8d 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -114,11 +114,11 @@ export type Instrumentation = z.infer; */ export const NetworkConfigSchema = z.object({ subnets: z - .array(z.string().regex(/^subnet-[0-9a-zA-Z]{8,17}$/)) + .array(z.string().regex(/^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/)) .min(1) .max(16), securityGroups: z - .array(z.string().regex(/^sg-[0-9a-zA-Z]{8,17}$/)) + .array(z.string().regex(/^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/)) .min(1) .max(16), /** @@ -127,7 +127,7 @@ export const NetworkConfigSchema = z.object({ */ vpcId: z .string() - .regex(/^vpc-[0-9a-zA-Z]{8,17}$/) + .regex(/^vpc-(?:[0-9a-f]{8}|[0-9a-f]{17})$/) .optional(), }); export type NetworkConfig = z.infer; From b0db7350d1d64ea7591306b9a85a800be3f5b27b Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 15:35:53 +0000 Subject: [PATCH 09/17] refactor(schema): centralize AWS resource-ID patterns as single source of truth in constants.ts VPC_ID_PATTERN, SUBNET_ID_PATTERN, and SECURITY_GROUP_ID_PATTERN were defined in 3 places (schemas/auth.ts, schemas/agent-env.ts inline literals, and cli/commands/shared/vpc-utils.ts) and had diverged: auth.ts used a lax subnet pattern ([0-9a-zA-Z]{8,17}), agent-env.ts had correct but duplicated inline literals, and vpc-utils.ts had its own copies. All three patterns now live exclusively in schema/constants.ts (lowercase hex, exactly 8 or 17 chars). auth.ts imports and re-exports them so existing TUI importers (JwtConfigInput, AddHarnessScreen) keep working. agent-env.ts and vpc-utils.ts import from constants directly. This also tightens auth/PrivateLink subnet validation from the lax form to the canonical hex 8/17 form, which is intentional. Tests: 18 new pattern tests added to constants.test.ts; full suite green (5760 tests, 395 files). --- src/cli/commands/shared/vpc-utils.ts | 10 ++-- src/schema/__tests__/constants.test.ts | 70 ++++++++++++++++++++++++++ src/schema/constants.ts | 16 ++++++ src/schema/schemas/agent-env.ts | 15 +++--- src/schema/schemas/auth.ts | 7 +-- 5 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/cli/commands/shared/vpc-utils.ts b/src/cli/commands/shared/vpc-utils.ts index 43b19e754..ea3a15c26 100644 --- a/src/cli/commands/shared/vpc-utils.ts +++ b/src/cli/commands/shared/vpc-utils.ts @@ -1,3 +1,5 @@ +import { SECURITY_GROUP_ID_PATTERN, SUBNET_ID_PATTERN, VPC_ID_PATTERN } from '../../../schema/constants'; + export interface VpcOptions { networkMode?: string; subnets?: string; @@ -17,10 +19,6 @@ export interface VpcValidationResult { export const VPC_ENDPOINT_WARNING = 'VPC mode may require VPC endpoints for CloudWatch, X-Ray, ECR, and Bedrock depending on your agent configuration. If your agent calls public APIs or uses an API-key-based provider, a NAT gateway or additional endpoints may also be needed.'; -const SUBNET_PATTERN = /^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; -const SECURITY_GROUP_PATTERN = /^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; -const VPC_ID_PATTERN = /^vpc-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; - export function parseCommaSeparatedList(value: string | undefined): string[] | undefined { if (!value) return undefined; return value @@ -39,7 +37,7 @@ export function validateSubnetIds(value: string): true | string { .map(s => s.trim()) .filter(Boolean); if (ids.length === 0) return 'At least one subnet ID is required'; - const invalid = ids.filter(id => !SUBNET_PATTERN.test(id)); + const invalid = ids.filter(id => !SUBNET_ID_PATTERN.test(id)); if (invalid.length > 0) return `Invalid subnet ID format: ${invalid[0]}. Expected subnet-xxxxxxxx`; return true; } @@ -54,7 +52,7 @@ export function validateSecurityGroupIds(value: string): true | string { .map(s => s.trim()) .filter(Boolean); if (ids.length === 0) return 'At least one security group ID is required'; - const invalid = ids.filter(id => !SECURITY_GROUP_PATTERN.test(id)); + const invalid = ids.filter(id => !SECURITY_GROUP_ID_PATTERN.test(id)); if (invalid.length > 0) return `Invalid security group ID format: ${invalid[0]}. Expected sg-xxxxxxxx`; return true; } diff --git a/src/schema/__tests__/constants.test.ts b/src/schema/__tests__/constants.test.ts index 45d3381e4..d92e9fa68 100644 --- a/src/schema/__tests__/constants.test.ts +++ b/src/schema/__tests__/constants.test.ts @@ -8,7 +8,10 @@ import { RESERVED_PROJECT_NAMES, RuntimeVersionSchema, SDKFrameworkSchema, + SECURITY_GROUP_ID_PATTERN, + SUBNET_ID_PATTERN, TargetLanguageSchema, + VPC_ID_PATTERN, getFrameworksForLanguage, getSupportedFrameworksForProtocol, getSupportedModelProviders, @@ -244,3 +247,70 @@ describe('isFrameworkSupportedForProtocol', () => { expect(isFrameworkSupportedForProtocol('MCP', 'OpenAIAgents')).toBe(false); }); }); + +// ============================================================================ +// AWS Network Resource ID Patterns — canonical hex 8/17 form +// ============================================================================ + +describe('VPC_ID_PATTERN', () => { + it('accepts 8-char lowercase-hex vpc id', () => { + expect(VPC_ID_PATTERN.test('vpc-0a1b2c3d')).toBe(true); + }); + it('accepts 17-char lowercase-hex vpc id', () => { + expect(VPC_ID_PATTERN.test('vpc-0123456789abcdef0')).toBe(true); + }); + it('rejects uppercase hex vpc id', () => { + expect(VPC_ID_PATTERN.test('vpc-ABCDEFGH')).toBe(false); + }); + it('rejects non-hex vpc id', () => { + expect(VPC_ID_PATTERN.test('vpc-zzzzzzzz')).toBe(false); + }); + it('rejects 9-char vpc id', () => { + expect(VPC_ID_PATTERN.test('vpc-123456789')).toBe(false); + }); + it('rejects wrong prefix', () => { + expect(VPC_ID_PATTERN.test('subnet-0a1b2c3d')).toBe(false); + }); +}); + +describe('SUBNET_ID_PATTERN', () => { + it('accepts 8-char lowercase-hex subnet id', () => { + expect(SUBNET_ID_PATTERN.test('subnet-0a1b2c3d')).toBe(true); + }); + it('accepts 17-char lowercase-hex subnet id', () => { + expect(SUBNET_ID_PATTERN.test('subnet-0123456789abcdef0')).toBe(true); + }); + it('rejects uppercase hex subnet id', () => { + expect(SUBNET_ID_PATTERN.test('subnet-ABCDEFGH')).toBe(false); + }); + it('rejects non-hex subnet id', () => { + expect(SUBNET_ID_PATTERN.test('subnet-zzzzzzzz')).toBe(false); + }); + it('rejects 9-char subnet id', () => { + expect(SUBNET_ID_PATTERN.test('subnet-123456789')).toBe(false); + }); + it('rejects wrong prefix', () => { + expect(SUBNET_ID_PATTERN.test('vpc-0a1b2c3d')).toBe(false); + }); +}); + +describe('SECURITY_GROUP_ID_PATTERN', () => { + it('accepts 8-char lowercase-hex sg id', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('sg-0a1b2c3d')).toBe(true); + }); + it('accepts 17-char lowercase-hex sg id', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('sg-0123456789abcdef0')).toBe(true); + }); + it('rejects uppercase hex sg id', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('sg-ABCDEFGH')).toBe(false); + }); + it('rejects non-hex sg id', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('sg-zzzzzzzz')).toBe(false); + }); + it('rejects 9-char sg id', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('sg-123456789')).toBe(false); + }); + it('rejects wrong prefix', () => { + expect(SECURITY_GROUP_ID_PATTERN.test('subnet-0a1b2c3d')).toBe(false); + }); +}); diff --git a/src/schema/constants.ts b/src/schema/constants.ts index bba816241..ca8c15404 100644 --- a/src/schema/constants.ts +++ b/src/schema/constants.ts @@ -180,6 +180,22 @@ export const DEFAULT_RUNTIME_BY_LANGUAGE: Record<'Python' | 'TypeScript', Runtim export const NetworkModeSchema = z.enum(['PUBLIC', 'VPC']); export type NetworkMode = z.infer; +// ============================================================================ +// AWS Network Resource ID Patterns (single source of truth) +// +// Canonical AWS resource ID format: lowercase hex, exactly 8 chars (legacy) or +// 17 chars (current). These are shared by NetworkConfigSchema (agent VPC config) +// and the auth/PrivateLink schema (ManagedVpcResource). All consumers must import +// from here rather than defining their own copies. +// ============================================================================ + +/** Matches a VPC ID: vpc- followed by 8 or 17 lowercase hex digits. */ +export const VPC_ID_PATTERN = /^vpc-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; +/** Matches a subnet ID: subnet- followed by 8 or 17 lowercase hex digits. */ +export const SUBNET_ID_PATTERN = /^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; +/** Matches a security group ID: sg- followed by 8 or 17 lowercase hex digits. */ +export const SECURITY_GROUP_ID_PATTERN = /^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; + // ============================================================================ // Protocol Mode // ============================================================================ diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index 022614b8d..afd0f9d85 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -7,6 +7,9 @@ import { NetworkModeSchema, ProtocolModeSchema, RuntimeVersionSchema as RuntimeVersionSchemaFromConstants, + SECURITY_GROUP_ID_PATTERN, + SUBNET_ID_PATTERN, + VPC_ID_PATTERN, } from '../constants'; import type { DirectoryPath, FilePath } from '../types'; import { AuthorizerConfigSchema, RuntimeAuthorizerTypeSchema } from './auth'; @@ -113,22 +116,16 @@ export type Instrumentation = z.infer; * Required when networkMode is 'VPC'. */ export const NetworkConfigSchema = z.object({ - subnets: z - .array(z.string().regex(/^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/)) - .min(1) - .max(16), + subnets: z.array(z.string().regex(SUBNET_ID_PATTERN, 'Must be a subnet id (subnet-...)')).min(1).max(16), securityGroups: z - .array(z.string().regex(/^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/)) + .array(z.string().regex(SECURITY_GROUP_ID_PATTERN, 'Must be a security group id (sg-...)')) .min(1) .max(16), /** * VPC ID. Required for Container builds in VPC mode because CodeBuild needs an explicit VPC ID; * it cannot infer the VPC from subnets alone. Runtime/Lambda builds can omit this. */ - vpcId: z - .string() - .regex(/^vpc-(?:[0-9a-f]{8}|[0-9a-f]{17})$/) - .optional(), + vpcId: z.string().regex(VPC_ID_PATTERN, 'Must be a VPC id (vpc-...)').optional(), }); export type NetworkConfig = z.infer; diff --git a/src/schema/schemas/auth.ts b/src/schema/schemas/auth.ts index 8310a5231..16860576d 100644 --- a/src/schema/schemas/auth.ts +++ b/src/schema/schemas/auth.ts @@ -1,6 +1,10 @@ +import { SECURITY_GROUP_ID_PATTERN, SUBNET_ID_PATTERN, VPC_ID_PATTERN } from '../constants'; import { TagsSchema } from './primitives/tags'; import { z } from 'zod'; +// Re-exported so TUI components that import *_ID_PATTERN from schemas/auth keep working. +export { SECURITY_GROUP_ID_PATTERN, SUBNET_ID_PATTERN, VPC_ID_PATTERN }; + // ============================================================================ // Shared Authorization Schemas // ============================================================================ @@ -98,9 +102,6 @@ export type CustomClaimValidation = z.infer; // schema regex (single source of truth) instead of hand-rolled checks that drift from it. export const LATTICE_RESOURCE_CONFIG_PATTERN = /^((rcfg-[0-9a-z]{17})|(arn:[a-z0-9-]+:vpc-lattice:[a-zA-Z0-9-]+:\d{12}:resourceconfiguration\/rcfg-[0-9a-z]{17}))$/; -export const VPC_ID_PATTERN = /^vpc-(([0-9a-z]{8})|([0-9a-z]{17}))$/; -export const SUBNET_ID_PATTERN = /^subnet-[0-9a-zA-Z]{8,17}$/; -export const SECURITY_GROUP_ID_PATTERN = /^sg-(([0-9a-z]{8})|([0-9a-z]{17}))$/; export const EndpointIpAddressTypeSchema = z.enum(['IPV4', 'IPV6']); export type EndpointIpAddressType = z.infer; From 39b52312d28dddf970470eebe2c4fe712dc71c26 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 15:42:41 +0000 Subject: [PATCH 10/17] fix(cli): validate --vpc-id at CLI layer for container harness builds in VPC mode validateAddHarnessOptions was calling validateVpcOptions without a build type, so 'add harness --container ./Dockerfile --network-mode VPC' without --vpc-id fell through to a Zod schema error instead of the friendly CLI message. Derive the build type from the --container flag (same logic as HarnessPrimitive.parseContainerFlag) and thread it into validateVpcOptions, mirroring the agent path at validate.ts:313. Adds harness-vpc-guard.test.ts to cover the full VPC + container-flag matrix at the CLI-validation layer. --- .../add/__tests__/harness-vpc-guard.test.ts | 94 +++++++++++++++++++ src/cli/commands/add/validate.ts | 11 ++- 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/cli/commands/add/__tests__/harness-vpc-guard.test.ts diff --git a/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts b/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts new file mode 100644 index 000000000..8af4e4988 --- /dev/null +++ b/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts @@ -0,0 +1,94 @@ +import type { AddHarnessCliOptions } from '../types'; +import { validateAddHarnessOptions } from '../validate'; +import { describe, expect, it } from 'vitest'; + +const base: AddHarnessCliOptions = { + name: 'h1', + modelProvider: 'bedrock', + modelId: 'us.anthropic.claude-haiku-4-5-20251001-v1:0', +}; + +describe('validateAddHarnessOptions — VPC network-mode guard', () => { + it('rejects VPC mode without subnets', () => { + const result = validateAddHarnessOptions({ + ...base, + networkMode: 'VPC', + securityGroups: 'sg-0a1b2c3d', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--subnets is required'); + }); + + it('rejects VPC mode without security groups', () => { + const result = validateAddHarnessOptions({ + ...base, + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--security-groups is required'); + }); + + it('accepts valid VPC options for a non-container harness', () => { + const result = validateAddHarnessOptions({ + ...base, + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + securityGroups: 'sg-0a1b2c3d', + }); + expect(result.valid).toBe(true); + }); + + it('rejects a container (dockerfile) harness with --network-mode VPC and no --vpc-id', () => { + const result = validateAddHarnessOptions({ + ...base, + container: './Dockerfile', + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + securityGroups: 'sg-0a1b2c3d', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--vpc-id is required'); + }); + + it('accepts a container (dockerfile) harness with --network-mode VPC and a valid --vpc-id', () => { + const result = validateAddHarnessOptions({ + ...base, + container: './Dockerfile', + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + securityGroups: 'sg-0a1b2c3d', + vpcId: 'vpc-0a1b2c3d', + }); + expect(result.valid).toBe(true); + }); + + it('does not require --vpc-id for a container URI (non-dockerfile) harness in VPC mode', () => { + const result = validateAddHarnessOptions({ + ...base, + container: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-image:latest', + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + securityGroups: 'sg-0a1b2c3d', + }); + expect(result.valid).toBe(true); + }); + + it('rejects subnets provided without --network-mode VPC', () => { + const result = validateAddHarnessOptions({ + ...base, + subnets: 'subnet-0a1b2c3d', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('only valid with --network-mode VPC'); + }); + + it('rejects --vpc-id provided without --network-mode VPC', () => { + const result = validateAddHarnessOptions({ + ...base, + vpcId: 'vpc-0a1b2c3d', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('only valid with --network-mode VPC'); + }); +}); diff --git a/src/cli/commands/add/validate.ts b/src/cli/commands/add/validate.ts index 62e468020..42534f759 100644 --- a/src/cli/commands/add/validate.ts +++ b/src/cli/commands/add/validate.ts @@ -1131,7 +1131,16 @@ export function validateAddHarnessOptions(options: AddHarnessCliOptions): Valida // VPC network-mode coupling: reject --subnets/--security-groups when network mode isn't VPC // (and require them when it is), instead of silently dropping them. Mirrors the agent path. - const vpcResult = validateVpcOptions(options); + // Derive build type from --container: a dockerfile path means Container, absent/URI means CodeZip. + const containerValue = options.container; + const looksLikeDockerfile = + containerValue !== undefined && + (containerValue.endsWith('Dockerfile') || + containerValue.endsWith('.dockerfile') || + containerValue.startsWith('./') || + containerValue.startsWith('../')); + const harnessBuildType = looksLikeDockerfile ? 'Container' : undefined; + const vpcResult = validateVpcOptions(options, harnessBuildType); if (!vpcResult.valid) { return vpcResult; } From 1bd733f20dc89a256af4a05aff4fa21d6724807d Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 15:51:35 +0000 Subject: [PATCH 11/17] fix(cli): resolve networkConfig.vpcId via DescribeSubnets on import/export of Container VPC agents --- .../aws/__tests__/agentcore-control.test.ts | 88 +++++++++++++++ src/cli/aws/agentcore-control.ts | 20 ++-- .../__tests__/fetch-harness-spec.test.ts | 100 +++++++++++++++++- src/cli/commands/export/fetch-harness-spec.ts | 8 +- src/cli/commands/shared/vpc-utils.ts | 25 +++++ 5 files changed, 230 insertions(+), 11 deletions(-) diff --git a/src/cli/aws/__tests__/agentcore-control.test.ts b/src/cli/aws/__tests__/agentcore-control.test.ts index 34351c3cc..31a444678 100644 --- a/src/cli/aws/__tests__/agentcore-control.test.ts +++ b/src/cli/aws/__tests__/agentcore-control.test.ts @@ -48,6 +48,14 @@ vi.mock('../account', () => ({ getCredentialProvider: vi.fn().mockReturnValue({}), })); +const { mockResolveVpcId } = vi.hoisted(() => ({ + mockResolveVpcId: vi.fn(), +})); + +vi.mock('../../commands/shared/vpc-utils', () => ({ + resolveVpcIdFromSubnets: (...args: unknown[]) => mockResolveVpcId(...args), +})); + describe('getAgentRuntimeStatus', () => { beforeEach(() => { vi.clearAllMocks(); @@ -502,6 +510,86 @@ describe('getAgentRuntimeDetail — new fields', () => { }); }); +describe('getAgentRuntimeDetail — VPC vpcId resolution', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const vpcBaseResponse = { + agentRuntimeId: 'rt-vpc', + agentRuntimeArn: 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/rt-vpc', + agentRuntimeName: 'vpc-runtime', + status: 'READY', + roleArn: 'arn:aws:iam::123:role/test', + networkConfiguration: { + networkMode: 'VPC', + networkModeConfig: { + subnets: ['subnet-0a1b2c3d4e5f6a7b8'], + securityGroups: ['sg-0a1b2c3d4e5f6a7b8'], + }, + }, + protocolConfiguration: { serverProtocol: 'HTTP' }, + agentRuntimeArtifact: { containerConfiguration: { containerUri: 'ecr.io/repo:tag' } }, + }; + + it('resolves vpcId from subnets for a VPC runtime and includes it in networkConfig', async () => { + mockSend.mockResolvedValueOnce(vpcBaseResponse).mockResolvedValueOnce({ tags: {} }); + mockResolveVpcId.mockResolvedValue('vpc-0abc1234567890def'); + + const result = await getAgentRuntimeDetail({ region: 'us-east-1', runtimeId: 'rt-vpc' }); + + expect(result.networkMode).toBe('VPC'); + expect(result.networkConfig).toEqual({ + subnets: ['subnet-0a1b2c3d4e5f6a7b8'], + securityGroups: ['sg-0a1b2c3d4e5f6a7b8'], + vpcId: 'vpc-0abc1234567890def', + }); + expect(mockResolveVpcId).toHaveBeenCalledWith(['subnet-0a1b2c3d4e5f6a7b8'], 'us-east-1'); + }); + + it('propagates DescribeSubnets error with actionable message', async () => { + mockSend.mockResolvedValue(vpcBaseResponse); + mockResolveVpcId.mockRejectedValue( + new Error( + 'Failed to resolve VPC ID from subnet subnet-0a1b2c3d4e5f6a7b8: ec2:DescribeSubnets permission is required.' + ) + ); + + await expect(getAgentRuntimeDetail({ region: 'us-east-1', runtimeId: 'rt-vpc' })).rejects.toThrow( + 'ec2:DescribeSubnets' + ); + }); + + it('sets networkConfig without vpcId when subnets list is empty', async () => { + mockSend + .mockResolvedValueOnce({ + ...vpcBaseResponse, + networkConfiguration: { + networkMode: 'VPC', + networkModeConfig: { subnets: [], securityGroups: ['sg-0a1b2c3d4e5f6a7b8'] }, + }, + }) + .mockResolvedValueOnce({ tags: {} }); + + const result = await getAgentRuntimeDetail({ region: 'us-east-1', runtimeId: 'rt-vpc' }); + expect(result.networkConfig?.vpcId).toBeUndefined(); + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); + + it('returns undefined networkConfig for PUBLIC mode', async () => { + mockSend + .mockResolvedValueOnce({ + ...vpcBaseResponse, + networkConfiguration: { networkMode: 'PUBLIC' }, + }) + .mockResolvedValueOnce({ tags: {} }); + + const result = await getAgentRuntimeDetail({ region: 'us-east-1', runtimeId: 'rt-vpc' }); + expect(result.networkConfig).toBeUndefined(); + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); +}); + describe('listEvaluators', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/cli/aws/agentcore-control.ts b/src/cli/aws/agentcore-control.ts index fd8f80052..cabf55bcf 100644 --- a/src/cli/aws/agentcore-control.ts +++ b/src/cli/aws/agentcore-control.ts @@ -1,4 +1,5 @@ import type { EvaluationLevel } from '../../schema/schemas/primitives/evaluator'; +import { resolveVpcIdFromSubnets } from '../commands/shared/vpc-utils'; import { getCredentialProvider } from './account'; import { controlPlaneEndpoint } from './stage-endpoint'; import { @@ -190,7 +191,7 @@ export interface AgentRuntimeDetail { description?: string; roleArn: string; networkMode: string; - networkConfig?: { subnets: string[]; securityGroups: string[] }; + networkConfig?: { subnets: string[]; securityGroups: string[]; vpcId?: string }; protocol: string; runtimeVersion?: string; entryPoint?: string[]; @@ -223,13 +224,16 @@ export async function getAgentRuntimeDetail(options: GetAgentRuntimeOptions): Pr const response = await client.send(command); const networkMode = response.networkConfiguration?.networkMode ?? 'PUBLIC'; - const networkConfig = - networkMode === 'VPC' && response.networkConfiguration?.networkModeConfig - ? { - subnets: response.networkConfiguration.networkModeConfig.subnets ?? [], - securityGroups: response.networkConfiguration.networkModeConfig.securityGroups ?? [], - } - : undefined; + let networkConfig: AgentRuntimeDetail['networkConfig']; + if (networkMode === 'VPC' && response.networkConfiguration?.networkModeConfig) { + const subnets = response.networkConfiguration.networkModeConfig.subnets ?? []; + const securityGroups = response.networkConfiguration.networkModeConfig.securityGroups ?? []; + let vpcId: string | undefined; + if (subnets.length > 0) { + vpcId = await resolveVpcIdFromSubnets(subnets, options.region); + } + networkConfig = { subnets, securityGroups, vpcId }; + } const isContainer = !!response.agentRuntimeArtifact?.containerConfiguration; const codeConfig = response.agentRuntimeArtifact?.codeConfiguration; diff --git a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts index 643362183..d9a6080b8 100644 --- a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts +++ b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts @@ -1,6 +1,21 @@ import type { Harness } from '../../../aws/agentcore-harness'; -import { harnessIdFromArn, mapApiHarnessToSpec } from '../fetch-harness-spec'; -import { describe, expect, it } from 'vitest'; +import { fetchHarnessSpecByArn, harnessIdFromArn, mapApiHarnessToSpec } from '../fetch-harness-spec'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockGetHarness } = vi.hoisted(() => ({ mockGetHarness: vi.fn() })); +const { mockResolveVpcId } = vi.hoisted(() => ({ mockResolveVpcId: vi.fn() })); + +vi.mock('../../../aws/agentcore-harness', async () => { + const actual = await vi.importActual( + '../../../aws/agentcore-harness' + ); + return { ...actual, getHarness: (...args: unknown[]) => mockGetHarness(...args) }; +}); + +vi.mock('../../shared/vpc-utils', async () => { + const actual = await vi.importActual('../../shared/vpc-utils'); + return { ...actual, resolveVpcIdFromSubnets: (...args: unknown[]) => mockResolveVpcId(...args) }; +}); function makeApiHarness(overrides: Partial = {}): Harness { return { @@ -360,3 +375,84 @@ describe('mapApiHarnessToSpec', () => { expect('containerUri' in spec).toBe(false); }); }); + +describe('fetchHarnessSpecByArn — VPC vpcId resolution', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + function makeVpcHarness(): Harness { + return makeApiHarness({ + environment: { + agentCoreRuntimeEnvironment: { + networkConfiguration: { + networkMode: 'VPC', + networkModeConfig: { + subnets: ['subnet-0a1b2c3d4e5f6a7b8'], + securityGroups: ['sg-0a1b2c3d4e5f6a7b8'], + }, + }, + }, + }, + }); + } + + it('resolves vpcId from subnets for a VPC harness and includes it in networkConfig', async () => { + mockGetHarness.mockResolvedValue({ harness: makeVpcHarness() }); + mockResolveVpcId.mockResolvedValue('vpc-0abc1234567890def'); + + const { spec } = await fetchHarnessSpecByArn( + 'arn:aws:bedrock-agentcore:us-east-1:111122223333:harness/h-123', + 'us-east-1' + ); + + expect(spec.networkMode).toBe('VPC'); + expect(spec.networkConfig?.vpcId).toBe('vpc-0abc1234567890def'); + expect(spec.networkConfig?.subnets).toEqual(['subnet-0a1b2c3d4e5f6a7b8']); + expect(spec.networkConfig?.securityGroups).toEqual(['sg-0a1b2c3d4e5f6a7b8']); + expect(mockResolveVpcId).toHaveBeenCalledWith(['subnet-0a1b2c3d4e5f6a7b8'], 'us-east-1'); + }); + + it('propagates DescribeSubnets error with actionable message naming ec2:DescribeSubnets', async () => { + mockGetHarness.mockResolvedValue({ harness: makeVpcHarness() }); + mockResolveVpcId.mockRejectedValue( + new Error( + 'Failed to resolve VPC ID from subnet subnet-0a1b2c3d4e5f6a7b8: ec2:DescribeSubnets permission is required.' + ) + ); + + await expect( + fetchHarnessSpecByArn('arn:aws:bedrock-agentcore:us-east-1:111122223333:harness/h-123', 'us-east-1') + ).rejects.toThrow('ec2:DescribeSubnets'); + }); + + it('does not call resolveVpcIdFromSubnets for PUBLIC mode harnesses', async () => { + mockGetHarness.mockResolvedValue({ + harness: makeApiHarness({ + environment: { + agentCoreRuntimeEnvironment: { networkConfiguration: { networkMode: 'PUBLIC' } }, + }, + }), + }); + + const { spec } = await fetchHarnessSpecByArn( + 'arn:aws:bedrock-agentcore:us-east-1:111122223333:harness/h-123', + 'us-east-1' + ); + + expect('networkMode' in spec).toBe(false); + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); + + it('does not call resolveVpcIdFromSubnets when harness has no environment block', async () => { + mockGetHarness.mockResolvedValue({ harness: makeApiHarness() }); + + const { spec } = await fetchHarnessSpecByArn( + 'arn:aws:bedrock-agentcore:us-east-1:111122223333:harness/h-123', + 'us-east-1' + ); + + expect('networkMode' in spec).toBe(false); + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/export/fetch-harness-spec.ts b/src/cli/commands/export/fetch-harness-spec.ts index 7235ecf5e..ad6d69a6d 100644 --- a/src/cli/commands/export/fetch-harness-spec.ts +++ b/src/cli/commands/export/fetch-harness-spec.ts @@ -8,6 +8,7 @@ import type { HarnessModelConfiguration, } from '../../aws/agentcore-harness'; import { getHarness } from '../../aws/agentcore-harness'; +import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; /** * Fetch a harness by ARN from the control plane and map it to a local HarnessSpec — the same @@ -21,7 +22,12 @@ export async function fetchHarnessSpecByArn( ): Promise<{ spec: HarnessSpec; systemPrompt?: string }> { const harnessId = harnessIdFromArn(arn); const { harness } = await getHarness({ region, harnessId }); - return mapApiHarnessToSpec(harness); + const result = mapApiHarnessToSpec(harness); + const nc = result.spec.networkConfig; + if (result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { + nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); + } + return result; } /** Extract the harness id from a harness ARN (`.../harness/` -> ``). */ diff --git a/src/cli/commands/shared/vpc-utils.ts b/src/cli/commands/shared/vpc-utils.ts index ea3a15c26..1485a8f86 100644 --- a/src/cli/commands/shared/vpc-utils.ts +++ b/src/cli/commands/shared/vpc-utils.ts @@ -1,4 +1,6 @@ import { SECURITY_GROUP_ID_PATTERN, SUBNET_ID_PATTERN, VPC_ID_PATTERN } from '../../../schema/constants'; +import { getCredentialProvider } from '../../aws/account'; +import { DescribeSubnetsCommand, EC2Client } from '@aws-sdk/client-ec2'; export interface VpcOptions { networkMode?: string; @@ -64,6 +66,29 @@ export function validateVpcId(value: string): true | string { return true; } +/** + * Resolve the VPC ID from the first subnet via ec2:DescribeSubnets. + * Throws a clear error naming the required permission if the call fails. + */ +export async function resolveVpcIdFromSubnets(subnetIds: string[], region: string): Promise { + const ec2 = new EC2Client({ region, credentials: getCredentialProvider() }); + let vpcId: string | undefined; + try { + const resp = await ec2.send(new DescribeSubnetsCommand({ SubnetIds: subnetIds })); + vpcId = resp.Subnets?.[0]?.VpcId; + } catch (err) { + throw new Error( + `Failed to resolve VPC ID from subnet ${subnetIds[0]}: ec2:DescribeSubnets permission is required. Cause: ${(err as Error).message ?? String(err)}` + ); + } + if (!vpcId) { + throw new Error( + `ec2:DescribeSubnets returned no VPC ID for subnet ${subnetIds[0]}. Verify the subnet exists and ec2:DescribeSubnets permission is granted.` + ); + } + return vpcId; +} + export function validateVpcOptions(options: VpcOptions, buildType?: string): VpcValidationResult { if (options.networkMode && options.networkMode !== 'PUBLIC' && options.networkMode !== 'VPC') { return { valid: false, error: `Invalid network mode: ${options.networkMode}. Use PUBLIC or VPC` }; From dc8e8caed3a16c20a0d9c48562a5f17bd0cf0c2a Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 16:18:05 +0000 Subject: [PATCH 12/17] fix(cli): resolve vpcId via DescribeSubnets on starter-toolkit YAML import of Container VPC agents The starter-toolkit YAML never emits vpc_id, only subnets + security_groups. Importing a Container+VPC agent from YAML produced a networkConfig with no vpcId, which the strict AgentEnvSpecSchema rejects. Reuse the shared resolveVpcIdFromSubnets helper (already used by the ARN-import and export paths) in the async handleImport flow, after agents are merged into the projectSpec but before writeProjectSpec. On DescribeSubnets failure, the error propagates with the ec2:DescribeSubnets permission name intact. --- .../__tests__/vpc-yaml-import-vpcid.test.ts | 329 ++++++++++++++++++ src/cli/commands/import/actions.ts | 26 ++ 2 files changed, 355 insertions(+) create mode 100644 src/cli/commands/import/__tests__/vpc-yaml-import-vpcid.test.ts diff --git a/src/cli/commands/import/__tests__/vpc-yaml-import-vpcid.test.ts b/src/cli/commands/import/__tests__/vpc-yaml-import-vpcid.test.ts new file mode 100644 index 000000000..9191d7233 --- /dev/null +++ b/src/cli/commands/import/__tests__/vpc-yaml-import-vpcid.test.ts @@ -0,0 +1,329 @@ +/** + * Tests that the YAML import flow resolves vpcId via DescribeSubnets for Container+VPC agents + * whose starter-toolkit YAML omits vpc_id (the normal case — the toolkit never emits it). + * + * Mirrors the mocking style used in import-no-deploy.test.ts and agentcore-control.test.ts. + */ +import { AgentEnvSpecSchema } from '../../../../schema/schemas/agent-env.js'; +import assert from 'node:assert'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// ---- Hoisted mocks ---- + +const { mockResolveVpcId } = vi.hoisted(() => ({ + mockResolveVpcId: vi.fn(), +})); + +vi.mock('../../../commands/shared/vpc-utils', () => ({ + resolveVpcIdFromSubnets: (...args: unknown[]) => mockResolveVpcId(...args), +})); + +const mockReadProjectSpec = vi.fn(); +const mockWriteProjectSpec = vi.fn(); +const mockReadAWSDeploymentTargets = vi.fn(); +const mockWriteAWSDeploymentTargets = vi.fn(); +const mockReadDeployedState = vi.fn(); +const mockWriteDeployedState = vi.fn(); +const mockFindConfigRoot = vi.fn(); + +vi.mock('../../../../lib', () => ({ + APP_DIR: 'app', + ConfigIO: class MockConfigIO { + readProjectSpec = mockReadProjectSpec; + writeProjectSpec = mockWriteProjectSpec; + readAWSDeploymentTargets = mockReadAWSDeploymentTargets; + writeAWSDeploymentTargets = mockWriteAWSDeploymentTargets; + readDeployedState = mockReadDeployedState; + writeDeployedState = mockWriteDeployedState; + }, + NoProjectError: class NoProjectError extends Error { + constructor(msg?: string) { + super(msg ?? 'No agentcore project found'); + this.name = 'NoProjectError'; + } + }, + ValidationError: class ValidationError extends Error { + constructor(msg?: string) { + super(msg ?? 'Validation error'); + this.name = 'ValidationError'; + } + }, + findConfigRoot: (...args: unknown[]) => mockFindConfigRoot(...args), + toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), +})); + +vi.mock('../../../aws/account', () => ({ + validateAwsCredentials: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../../../aws/partition', () => ({ + arnPrefix: vi.fn().mockReturnValue('arn:aws'), +})); + +vi.mock('../../../cdk/local-cdk-project', () => ({ + LocalCdkProject: vi.fn(), +})); + +vi.mock('../../../cdk/toolkit-lib', () => ({ + silentIoHost: {}, +})); + +vi.mock('../../../logging', () => ({ + ExecLogger: class MockExecLogger { + startStep = vi.fn(); + endStep = vi.fn(); + log = vi.fn(); + finalize = vi.fn(); + getRelativeLogPath = vi.fn().mockReturnValue('agentcore/.cli/logs/import/import-mock.log'); + logFilePath = 'agentcore/.cli/logs/import/import-mock.log'; + }, +})); + +vi.mock('../../../operations/deploy', () => ({ + buildCdkProject: vi.fn(), + synthesizeCdk: vi.fn(), +})); + +vi.mock('../../../operations/python/setup', () => ({ + setupPythonProject: vi.fn().mockResolvedValue({ status: 'success' }), +})); + +vi.mock('../phase1-update', () => ({ + executePhase1: vi.fn(), + getDeployedTemplate: vi.fn(), +})); + +vi.mock('../phase2-import', () => ({ + executePhase2: vi.fn(), + publishCdkAssets: vi.fn(), +})); + +// ============================================================================ +// Helpers +// ============================================================================ + +function makeProjectDir(tmpDir: string): string { + const configDir = path.join(tmpDir, 'myproject', 'agentcore'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'agentcore.json'), + JSON.stringify({ name: 'myproject', version: 1, runtimes: [], memories: [], knowledgeBases: [], credentials: [] }) + ); + return configDir; +} + +function writeYaml(dir: string, content: string): string { + const p = path.join(dir, '.bedrock_agentcore.yaml'); + fs.writeFileSync(p, content, 'utf-8'); + return p; +} + +const CONTAINER_VPC_YAML = (agentId: string) => ` +default_agent: vpc_agent +agents: + vpc_agent: + name: vpc_agent + entrypoint: main.py + deployment_type: container + runtime_type: PYTHON_3_12 + aws: + account: '123456789012' + region: us-west-2 + network_configuration: + network_mode: VPC + network_mode_config: + subnets: + - subnet-0a1b2c3d4e5f6a7b8 + - subnet-0b2c3d4e5f6a7b8c9 + security_groups: + - sg-0abcdef1234567890 + protocol_configuration: + server_protocol: HTTP + observability: + enabled: true + bedrock_agentcore: + agent_id: ${agentId} +`; + +const EMPTY_SPEC = { + name: 'myproject', + version: 1, + runtimes: [], + memories: [], + knowledgeBases: [], + credentials: [], +}; + +// ============================================================================ +// 1. vpcId resolved via DescribeSubnets on YAML import (no-deploy path) +// ============================================================================ + +describe('handleImport: Container+VPC agent — vpcId resolved via DescribeSubnets', () => { + let tmpDir: string; + + beforeEach(() => { + vi.clearAllMocks(); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vpc-yaml-import-')); + const configDir = makeProjectDir(tmpDir); + mockFindConfigRoot.mockReturnValue(configDir); + mockResolveVpcId.mockResolvedValue('vpc-0a1b2c3d4e5f60718'); + mockReadProjectSpec.mockResolvedValue(structuredClone(EMPTY_SPEC)); + mockWriteProjectSpec.mockResolvedValue(undefined); + mockReadAWSDeploymentTargets.mockResolvedValue([{ name: 'default', account: '123456789012', region: 'us-west-2' }]); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('populates networkConfig.vpcId from DescribeSubnets when YAML has no vpc_id', async () => { + const yamlPath = writeYaml(tmpDir, CONTAINER_VPC_YAML('null')); + const { handleImport } = await import('../actions.js'); + + const result = await handleImport({ source: yamlPath }); + + assert(result.success); + expect(mockResolveVpcId).toHaveBeenCalledWith( + ['subnet-0a1b2c3d4e5f6a7b8', 'subnet-0b2c3d4e5f6a7b8c9'], + 'us-west-2' + ); + const writtenSpec = mockWriteProjectSpec.mock.calls[0]![0]; + const agent = writtenSpec.runtimes[0]; + expect(agent.networkConfig.vpcId).toBe('vpc-0a1b2c3d4e5f60718'); + }); + + it('written spec passes AgentEnvSpecSchema validation after vpcId is resolved', async () => { + const yamlPath = writeYaml(tmpDir, CONTAINER_VPC_YAML('null')); + const { handleImport } = await import('../actions.js'); + + await handleImport({ source: yamlPath }); + + const writtenSpec = mockWriteProjectSpec.mock.calls[0]![0]; + const agentSpec = writtenSpec.runtimes[0]; + const parseResult = AgentEnvSpecSchema.safeParse(agentSpec); + expect(parseResult.success).toBe(true); + }); + + it('does NOT call DescribeSubnets when YAML already carries vpc_id', async () => { + const yamlWithVpcId = ` +default_agent: vpc_agent +agents: + vpc_agent: + name: vpc_agent + entrypoint: main.py + deployment_type: container + runtime_type: PYTHON_3_12 + aws: + account: '123456789012' + region: us-west-2 + network_configuration: + network_mode: VPC + network_mode_config: + subnets: + - subnet-0a1b2c3d4e5f6a7b8 + security_groups: + - sg-0abcdef1234567890 + vpc_id: vpc-0existingvpcid0 + protocol_configuration: + server_protocol: HTTP + observability: + enabled: true + bedrock_agentcore: + agent_id: null +`; + const yamlPath = writeYaml(tmpDir, yamlWithVpcId); + const { handleImport } = await import('../actions.js'); + + await handleImport({ source: yamlPath }); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + const writtenSpec = mockWriteProjectSpec.mock.calls[0]![0]; + expect(writtenSpec.runtimes[0].networkConfig.vpcId).toBe('vpc-0existingvpcid0'); + }); + + it('does NOT call DescribeSubnets for PUBLIC agents', async () => { + const publicYaml = ` +default_agent: public_agent +agents: + public_agent: + name: public_agent + entrypoint: main.py + deployment_type: container + aws: + account: '123456789012' + region: us-west-2 + network_configuration: + network_mode: PUBLIC + observability: + enabled: true + bedrock_agentcore: + agent_id: null +`; + const yamlPath = writeYaml(tmpDir, publicYaml); + const { handleImport } = await import('../actions.js'); + + await handleImport({ source: yamlPath }); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); + + it('does NOT call DescribeSubnets for CodeZip VPC agents (only Container requires vpcId)', async () => { + const codezipVpcYaml = ` +default_agent: codzip_agent +agents: + codzip_agent: + name: codzip_agent + entrypoint: main.py + deployment_type: direct_code_deploy + aws: + account: '123456789012' + region: us-west-2 + network_configuration: + network_mode: VPC + network_mode_config: + subnets: + - subnet-0a1b2c3d4e5f6a7b8 + security_groups: + - sg-0abcdef1234567890 + observability: + enabled: true + bedrock_agentcore: + agent_id: null +`; + const yamlPath = writeYaml(tmpDir, codezipVpcYaml); + const { handleImport } = await import('../actions.js'); + + await handleImport({ source: yamlPath }); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); + + it('throws (propagates) ec2:DescribeSubnets error when DescribeSubnets fails', async () => { + mockResolveVpcId.mockRejectedValue( + new Error( + 'Failed to resolve VPC ID from subnet subnet-0a1b2c3d4e5f6a7b8: ec2:DescribeSubnets permission is required. Cause: Access Denied' + ) + ); + const yamlPath = writeYaml(tmpDir, CONTAINER_VPC_YAML('null')); + const { handleImport } = await import('../actions.js'); + + const result = await handleImport({ source: yamlPath }); + + expect(result.success).toBe(false); + assert(!result.success); + expect(result.error.message).toContain('ec2:DescribeSubnets'); + }); + + it('uses region from YAML aws.region when no deployment target exists', async () => { + mockReadAWSDeploymentTargets.mockResolvedValue([]); + const yamlPath = writeYaml(tmpDir, CONTAINER_VPC_YAML('null')); + const { handleImport } = await import('../actions.js'); + + await handleImport({ source: yamlPath }); + + expect(mockResolveVpcId).toHaveBeenCalledWith(expect.any(Array), 'us-west-2'); + }); +}); diff --git a/src/cli/commands/import/actions.ts b/src/cli/commands/import/actions.ts index b1bbdc821..30700e616 100644 --- a/src/cli/commands/import/actions.ts +++ b/src/cli/commands/import/actions.ts @@ -12,6 +12,7 @@ import { arnPrefix } from '../../aws/partition'; import { ANSI, PYTHON_BASE_IMAGE } from '../../constants'; import { ExecLogger } from '../../logging'; import { setupPythonProject } from '../../operations/python/setup'; +import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; import { executeCdkImportPipeline } from './import-pipeline'; import { copyDirRecursive, fixPyprojectForSetuptools, toStackName } from './import-utils'; import { findLogicalIdByProperty, findLogicalIdsByType } from './template-utils'; @@ -335,6 +336,31 @@ export async function handleImport(options: ImportOptions): Promise 0 && + !agentSpec.networkConfig.vpcId + ) { + if (!importRegion) { + const error = + `Cannot resolve VPC ID for agent "${agentSpec.name}": no AWS region available. ` + + 'Add an aws.region to the YAML or set up a deployment target first.'; + logger.endStep('error', error); + logger.finalize(false); + return { success: false, error: new ValidationError(error), logPath: logger.getRelativeLogPath() }; + } + agentSpec.networkConfig.vpcId = await resolveVpcIdFromSubnets(agentSpec.networkConfig.subnets, importRegion); + logger.log(`Resolved vpcId for agent "${agentSpec.name}": ${agentSpec.networkConfig.vpcId}`); + onProgress?.(`Resolved vpcId for agent "${agentSpec.name}": ${agentSpec.networkConfig.vpcId}`); + } + } + // Write updated project config await configIO.writeProjectSpec(projectSpec); configWritten = true; From 0d875609ba056af392d1e160a2dd961f1052f3a6 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 16:37:41 +0000 Subject: [PATCH 13/17] fix(cli): only resolve vpcId for container builds on import/export, not all VPC-mode --- src/cli/aws/agentcore-control.ts | 11 ++++--- .../__tests__/fetch-harness-spec.test.ts | 33 +++++++++++++++++++ src/cli/commands/export/fetch-harness-spec.ts | 7 +++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/cli/aws/agentcore-control.ts b/src/cli/aws/agentcore-control.ts index cabf55bcf..276a46ef6 100644 --- a/src/cli/aws/agentcore-control.ts +++ b/src/cli/aws/agentcore-control.ts @@ -223,21 +223,24 @@ export async function getAgentRuntimeDetail(options: GetAgentRuntimeOptions): Pr const response = await client.send(command); + const isContainer = !!response.agentRuntimeArtifact?.containerConfiguration; + const codeConfig = response.agentRuntimeArtifact?.codeConfiguration; + const networkMode = response.networkConfiguration?.networkMode ?? 'PUBLIC'; let networkConfig: AgentRuntimeDetail['networkConfig']; if (networkMode === 'VPC' && response.networkConfiguration?.networkModeConfig) { const subnets = response.networkConfiguration.networkModeConfig.subnets ?? []; const securityGroups = response.networkConfiguration.networkModeConfig.securityGroups ?? []; + // Only resolve the VPC ID for Container builds: that's the only case the schema requires it + // (CodeBuild can't infer the VPC). CodeZip+VPC runtimes don't need it, so we avoid the extra + // ec2:DescribeSubnets call (and the IAM it would demand) for them. let vpcId: string | undefined; - if (subnets.length > 0) { + if (isContainer && subnets.length > 0) { vpcId = await resolveVpcIdFromSubnets(subnets, options.region); } networkConfig = { subnets, securityGroups, vpcId }; } - const isContainer = !!response.agentRuntimeArtifact?.containerConfiguration; - const codeConfig = response.agentRuntimeArtifact?.codeConfiguration; - let authorizerType: string | undefined; let authorizerConfiguration: AgentRuntimeDetail['authorizerConfiguration']; if (response.authorizerConfiguration?.customJWTAuthorizer) { diff --git a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts index d9a6080b8..be5668c55 100644 --- a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts +++ b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts @@ -444,6 +444,39 @@ describe('fetchHarnessSpecByArn — VPC vpcId resolution', () => { expect(mockResolveVpcId).not.toHaveBeenCalled(); }); + it('does not call resolveVpcIdFromSubnets for a prebuilt-containerUri VPC harness (no build → no vpcId needed)', async () => { + mockGetHarness.mockResolvedValue({ + harness: makeApiHarness({ + environmentArtifact: { + containerConfiguration: { + containerUri: '111122223333.dkr.ecr.us-east-1.amazonaws.com/my-harness:latest', + }, + }, + environment: { + agentCoreRuntimeEnvironment: { + networkConfiguration: { + networkMode: 'VPC', + networkModeConfig: { + subnets: ['subnet-0a1b2c3d4e5f6a7b8'], + securityGroups: ['sg-0a1b2c3d4e5f6a7b8'], + }, + }, + }, + }, + }), + }); + + const { spec } = await fetchHarnessSpecByArn( + 'arn:aws:bedrock-agentcore:us-east-1:111122223333:harness/h-123', + 'us-east-1' + ); + + // Prebuilt image → containerUri present, no vpcId resolution, no DescribeSubnets call. + expect(spec.containerUri).toBeDefined(); + expect(spec.networkConfig?.vpcId).toBeUndefined(); + expect(mockResolveVpcId).not.toHaveBeenCalled(); + }); + it('does not call resolveVpcIdFromSubnets when harness has no environment block', async () => { mockGetHarness.mockResolvedValue({ harness: makeApiHarness() }); diff --git a/src/cli/commands/export/fetch-harness-spec.ts b/src/cli/commands/export/fetch-harness-spec.ts index ad6d69a6d..6708a6cf2 100644 --- a/src/cli/commands/export/fetch-harness-spec.ts +++ b/src/cli/commands/export/fetch-harness-spec.ts @@ -24,7 +24,12 @@ export async function fetchHarnessSpecByArn( const { harness } = await getHarness({ region, harnessId }); const result = mapApiHarnessToSpec(harness); const nc = result.spec.networkConfig; - if (result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { + // Resolve the VPC ID unless the harness is a prebuilt-containerUri export — that case runs no + // CodeBuild and so never needs vpcId, while a from-source export becomes a Container build that + // does. Skipping the prebuilt case avoids a needless ec2:DescribeSubnets call (and the IAM it + // would otherwise demand). + const isPrebuiltImage = !!result.spec.containerUri; + if (!isPrebuiltImage && result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); } return result; From 709e9ef23ff69db9e57630ea4f1d5865ce7a682a Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 30 Jun 2026 17:19:21 +0000 Subject: [PATCH 14/17] fix(cli): resolve vpcId for ALL container exports incl prebuilt containerUri (a containerUri harness still runs CodeBuild) --- .../__tests__/fetch-harness-spec.test.ts | 19 +++++++++++-------- src/cli/commands/export/fetch-harness-spec.ts | 13 +++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts index be5668c55..07443c9b7 100644 --- a/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts +++ b/src/cli/commands/export/__tests__/fetch-harness-spec.test.ts @@ -381,8 +381,15 @@ describe('fetchHarnessSpecByArn — VPC vpcId resolution', () => { vi.clearAllMocks(); }); + // A Container (containerUri) harness in VPC mode — exercises the vpcId-resolution path, since the + // schema requires vpcId for Container builds and a containerUri export still runs CodeBuild. function makeVpcHarness(): Harness { return makeApiHarness({ + environmentArtifact: { + containerConfiguration: { + containerUri: '111122223333.dkr.ecr.us-east-1.amazonaws.com/my-harness:latest', + }, + }, environment: { agentCoreRuntimeEnvironment: { networkConfiguration: { @@ -444,14 +451,11 @@ describe('fetchHarnessSpecByArn — VPC vpcId resolution', () => { expect(mockResolveVpcId).not.toHaveBeenCalled(); }); - it('does not call resolveVpcIdFromSubnets for a prebuilt-containerUri VPC harness (no build → no vpcId needed)', async () => { + it('does NOT resolve vpcId for a CodeZip VPC harness (no containerUri/dockerfile → no CodeBuild → vpcId not required)', async () => { + // Neither containerUri nor dockerfile → CodeZip build. The schema does not require vpcId for it, + // so we must not make a needless DescribeSubnets call (and must not demand that IAM permission). mockGetHarness.mockResolvedValue({ harness: makeApiHarness({ - environmentArtifact: { - containerConfiguration: { - containerUri: '111122223333.dkr.ecr.us-east-1.amazonaws.com/my-harness:latest', - }, - }, environment: { agentCoreRuntimeEnvironment: { networkConfiguration: { @@ -471,8 +475,7 @@ describe('fetchHarnessSpecByArn — VPC vpcId resolution', () => { 'us-east-1' ); - // Prebuilt image → containerUri present, no vpcId resolution, no DescribeSubnets call. - expect(spec.containerUri).toBeDefined(); + expect(spec.containerUri).toBeUndefined(); expect(spec.networkConfig?.vpcId).toBeUndefined(); expect(mockResolveVpcId).not.toHaveBeenCalled(); }); diff --git a/src/cli/commands/export/fetch-harness-spec.ts b/src/cli/commands/export/fetch-harness-spec.ts index 6708a6cf2..a52e1c735 100644 --- a/src/cli/commands/export/fetch-harness-spec.ts +++ b/src/cli/commands/export/fetch-harness-spec.ts @@ -24,12 +24,13 @@ export async function fetchHarnessSpecByArn( const { harness } = await getHarness({ region, harnessId }); const result = mapApiHarnessToSpec(harness); const nc = result.spec.networkConfig; - // Resolve the VPC ID unless the harness is a prebuilt-containerUri export — that case runs no - // CodeBuild and so never needs vpcId, while a from-source export becomes a Container build that - // does. Skipping the prebuilt case avoids a needless ec2:DescribeSubnets call (and the IAM it - // would otherwise demand). - const isPrebuiltImage = !!result.spec.containerUri; - if (!isPrebuiltImage && result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { + // Resolve the VPC ID for any Container build in VPC mode — that's exactly the case the schema + // requires it for (CodeBuild can't infer the VPC). A harness is a Container build whenever it + // carries a containerUri OR a dockerfile (matching export's resolveBuildType): even a prebuilt + // containerUri export emits a `FROM ` Dockerfile stub that CodeBuild builds, so it needs + // vpcId too. Only a CodeZip harness (neither) genuinely skips resolution. + const isContainerBuild = !!result.spec.containerUri || !!result.spec.dockerfile; + if (isContainerBuild && result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); } return result; From 4485c61d037e0c46b012595034027f35583c8eff Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 2 Jul 2026 17:02:29 +0000 Subject: [PATCH 15/17] =?UTF-8?q?fix(vpc):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20unify=20container-build=20detection,=20backfill=20vpcId=20on?= =?UTF-8?q?=20deploy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses @tejaskash's review on #1671: - C1: a prebuilt-containerUri harness in VPC mode is a container build (export emits a `FROM ` Dockerfile that CodeBuild builds), so it now requires a vpcId and honors the SG<=5 cap just like a dockerfile harness. Previously a `--container --network-mode VPC` harness (no --vpc-id) persisted and then dead-ended at deploy/export. - C4/C5: "is this a container build?" was decided ~5 different ways that had drifted apart. Consolidated into one shared isContainerBuild(spec) predicate in schema/constants.ts, used by both schema refinements, the CLI harness validator (replacing the duplicated looksLikeDockerfile heuristic), and the import/export vpcId-resolution guards. - C2: vpcId is no longer required at the Zod schema level. It validates at READ time, so requiring it there hard-failed pre-existing agentcore.json files written before the vpcId field existed (breaking status/remove/validate/deploy on upgrade). Instead: CLI validators require --vpc-id for fresh creates, deploy backfills a missing vpcId from the subnets (ec2:DescribeSubnets) and persists it to disk so the CDK synth process reads it, and the CDK construct fails fast at synth. The SG<=5 cap stays in the schema (a hard limit backfill can't satisfy). - C6: reuse the exported NetworkConfig type instead of re-declaring its shape inline in agentcore-control.ts and import/types.ts. - C7: resolveVpcIdFromSubnets no longer blindly trusts Subnets[0] — it asserts all returned subnets share one VpcId (DescribeSubnets does not guarantee order) and names all requested subnets in the error, not just the first. - C3: document the new ec2:DescribeSubnets requirement for import/export/deploy of Container+VPC builds in PERMISSIONS.md + container-builds.md, incl. the upgrade (backfill) note. Tests: new backfill-vpc-id suite, multi-VPC rejection in vpc-utils, containerUri dead-end + SG-cap coverage, and updated schema/validator tests to the new enforce-at-deploy contract. Full suite green (5796). --- docs/PERMISSIONS.md | 20 ++- docs/container-builds.md | 32 ++++ src/cli/aws/agentcore-control.ts | 14 +- .../add/__tests__/harness-vpc-guard.test.ts | 18 ++- src/cli/commands/add/validate.ts | 14 +- src/cli/commands/deploy/actions.ts | 10 ++ src/cli/commands/export/fetch-harness-spec.ts | 12 +- src/cli/commands/import/actions.ts | 3 +- src/cli/commands/import/types.ts | 3 +- .../shared/__tests__/vpc-utils.test.ts | 69 +++++++- src/cli/commands/shared/vpc-utils.ts | 28 +++- .../deploy/__tests__/backfill-vpc-id.test.ts | 150 ++++++++++++++++++ src/cli/operations/deploy/backfill-vpc-id.ts | 70 ++++++++ src/cli/operations/deploy/index.ts | 3 + src/schema/constants.ts | 20 +++ .../schemas/__tests__/agent-env.test.ts | 11 +- src/schema/schemas/agent-env.ts | 22 +-- .../primitives/__tests__/harness.test.ts | 19 ++- src/schema/schemas/primitives/harness.ts | 22 ++- 19 files changed, 465 insertions(+), 75 deletions(-) create mode 100644 src/cli/operations/deploy/__tests__/backfill-vpc-id.test.ts create mode 100644 src/cli/operations/deploy/backfill-vpc-id.ts diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index 858584456..a5f7f38c3 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -323,12 +323,20 @@ These EC2 and EFS `Describe*` actions do not support resource-level scoping, so | Action | CLI Commands | Purpose | | ----------------------------------------------------- | ------------------ | ------------------------------------------------------------------ | -| `ec2:DescribeSecurityGroups` | `create`, `deploy` | Validate agent/mount-target security groups allow NFS (port 2049) | -| `ec2:DescribeSubnets` | `create`, `deploy` | Validate mount-target subnets are in the agent's VPC and AZs | -| `elasticfilesystem:DescribeAccessPoints` | `create`, `deploy` | Resolve the EFS access point and its file system | -| `elasticfilesystem:DescribeMountTargets` | `create`, `deploy` | Find the EFS mount targets to validate against the agent's subnets | -| `elasticfilesystem:DescribeMountTargetSecurityGroups` | `create`, `deploy` | Check mount-target security groups allow NFS from the agent | -| `s3files:ListMountTargets` | `create`, `deploy` | List the S3 Files access point's mount targets | +| `ec2:DescribeSecurityGroups` | `create`, `deploy` | Validate agent/mount-target security groups allow NFS (port 2049) | +| `ec2:DescribeSubnets` | `create`, `deploy`, `import` | Validate mount-target subnets are in the agent's VPC and AZs; resolve the VPC ID for Container+VPC builds (see note below) | +| `elasticfilesystem:DescribeAccessPoints` | `create`, `deploy` | Resolve the EFS access point and its file system | +| `elasticfilesystem:DescribeMountTargets` | `create`, `deploy` | Find the EFS mount targets to validate against the agent's subnets | +| `elasticfilesystem:DescribeMountTargetSecurityGroups` | `create`, `deploy` | Check mount-target security groups allow NFS from the agent | +| `s3files:ListMountTargets` | `create`, `deploy` | List the S3 Files access point's mount targets | + +> **Container builds in VPC mode also require `ec2:DescribeSubnets`.** A Container build (agent or +> dockerfile/prebuilt-image harness) that deploys in VPC mode needs an explicit VPC ID — CodeBuild's +> `CreateProject` cannot infer the VPC from subnets. When the VPC ID is not already in the config +> (e.g. `import` of a deployed Container+VPC runtime, `export` of a container harness, or `deploy` of +> a project created before the VPC ID field existed), the CLI resolves it from the first subnet via +> `ec2:DescribeSubnets`. Grant this action for `import`, `export`, and `deploy` if you use Container +> builds in VPC mode, even without filesystem mounts. | `s3files:GetMountTarget` | `create`, `deploy` | Inspect an S3 Files mount target's subnet/network configuration | ### Agent invocation diff --git a/docs/container-builds.md b/docs/container-builds.md index 5d82d2f03..bd6817edd 100644 --- a/docs/container-builds.md +++ b/docs/container-builds.md @@ -115,6 +115,36 @@ agentcore deploy -y # Build via CodeBuild, push to ECR Local packaging validates the image size (1 GB limit). If no local runtime is available, packaging is skipped and deployment handles the build remotely. +## VPC network mode + +A container agent (or dockerfile/prebuilt-image harness) can build and run inside a VPC. The build infrastructure — the +orchestrator Lambda and the shared CodeBuild project — is placed in the same VPC as the runtime: + +```bash +agentcore create --project-name MyProject --name myagent \ + --build Container --network-mode VPC \ + --subnets subnet-0123456789abcdef0 --security-groups sg-0123456789abcdef0 \ + --vpc-id vpc-0123456789abcdef0 \ + --language Python --framework Strands --model-provider Bedrock +``` + +Key points: + +- **`--vpc-id` is required for Container builds in VPC mode.** CodeBuild's `CreateProject` cannot infer the VPC from + subnets alone (unlike Lambda, which does). CodeZip builds and any PUBLIC build neither need nor accept a VPC ID. +- **At most 5 security groups** for a container build in VPC mode (a CodeBuild limit; the runtime itself allows 16). +- **A NAT-routed subnet or VPC endpoints are required** so the in-VPC CodeBuild/Lambda can reach ECR, S3, CloudWatch + Logs, STS, and CodeBuild. An isolated subnet with no egress will hang the build. +- **The build needs `ec2:DescribeSubnets`** on `import`/`export`/`deploy` to resolve the VPC ID — see + [PERMISSIONS.md](./PERMISSIONS.md#filesystem-network-validation). + +### Upgrading a project created before the VPC ID field existed + +Earlier CLI versions let a Container+VPC agent be configured with only subnets and security groups. If you upgrade and +your `agentcore.json` has a Container+VPC agent with no `networkConfig.vpcId`, `deploy` resolves it automatically from +the first subnet (via `ec2:DescribeSubnets`) and writes it back to the config — no manual edit needed. Grant +`ec2:DescribeSubnets` before deploying, or add the `vpcId` to the config by hand. + ## Troubleshooting | Error | Fix | @@ -123,4 +153,6 @@ deployment handles the build remotely. | Runtime not ready | Docker: start Docker Desktop / `sudo systemctl start docker`. Podman: `podman machine start`. Finch: `finch vm init && finch vm start` | | Dockerfile not found | Ensure `Dockerfile` exists in the agent's `codeLocation` directory | | Image exceeds 2 GB | Use multi-stage builds, minimize packages, review `.dockerignore` | +| `vpcId is required` at deploy/synth | Container+VPC build with no VPC ID. Grant `ec2:DescribeSubnets` so deploy can resolve it, or add `networkConfig.vpcId` to the config manually | +| Build hangs in VPC mode | The subnet has no egress. Use a NAT-routed subnet or add VPC endpoints (ECR api+dkr, S3, CloudWatch Logs, STS, CodeBuild) | | Build fails | Check `pyproject.toml` is valid; verify network access for dependency installation | diff --git a/src/cli/aws/agentcore-control.ts b/src/cli/aws/agentcore-control.ts index 276a46ef6..1c9278894 100644 --- a/src/cli/aws/agentcore-control.ts +++ b/src/cli/aws/agentcore-control.ts @@ -1,4 +1,5 @@ import type { EvaluationLevel } from '../../schema/schemas/primitives/evaluator'; +import type { NetworkConfig } from '../../schema'; import { resolveVpcIdFromSubnets } from '../commands/shared/vpc-utils'; import { getCredentialProvider } from './account'; import { controlPlaneEndpoint } from './stage-endpoint'; @@ -191,7 +192,7 @@ export interface AgentRuntimeDetail { description?: string; roleArn: string; networkMode: string; - networkConfig?: { subnets: string[]; securityGroups: string[]; vpcId?: string }; + networkConfig?: NetworkConfig; protocol: string; runtimeVersion?: string; entryPoint?: string[]; @@ -231,13 +232,10 @@ export async function getAgentRuntimeDetail(options: GetAgentRuntimeOptions): Pr if (networkMode === 'VPC' && response.networkConfiguration?.networkModeConfig) { const subnets = response.networkConfiguration.networkModeConfig.subnets ?? []; const securityGroups = response.networkConfiguration.networkModeConfig.securityGroups ?? []; - // Only resolve the VPC ID for Container builds: that's the only case the schema requires it - // (CodeBuild can't infer the VPC). CodeZip+VPC runtimes don't need it, so we avoid the extra - // ec2:DescribeSubnets call (and the IAM it would demand) for them. - let vpcId: string | undefined; - if (isContainer && subnets.length > 0) { - vpcId = await resolveVpcIdFromSubnets(subnets, options.region); - } + // Resolve the VPC ID only for Container builds (the case that requires it — CodeBuild can't infer + // the VPC). CodeZip+VPC runtimes don't need it, so this avoids the extra ec2:DescribeSubnets call + // (and its IAM) for them. `isContainer` here comes from the runtime's containerConfiguration. + const vpcId = isContainer && subnets.length > 0 ? await resolveVpcIdFromSubnets(subnets, options.region) : undefined; networkConfig = { subnets, securityGroups, vpcId }; } diff --git a/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts b/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts index 8af4e4988..271398f62 100644 --- a/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts +++ b/src/cli/commands/add/__tests__/harness-vpc-guard.test.ts @@ -63,7 +63,10 @@ describe('validateAddHarnessOptions — VPC network-mode guard', () => { expect(result.valid).toBe(true); }); - it('does not require --vpc-id for a container URI (non-dockerfile) harness in VPC mode', () => { + it('requires --vpc-id for a container URI harness in VPC mode (a prebuilt image still runs CodeBuild)', () => { + // A --container harness is a container build: export emits a `FROM ` Dockerfile + // that CodeBuild builds, so it needs a vpcId just like a dockerfile harness. Previously this + // slipped through and dead-ended at deploy/export. const result = validateAddHarnessOptions({ ...base, container: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-image:latest', @@ -71,6 +74,19 @@ describe('validateAddHarnessOptions — VPC network-mode guard', () => { subnets: 'subnet-0a1b2c3d', securityGroups: 'sg-0a1b2c3d', }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--vpc-id is required'); + }); + + it('accepts a container URI harness in VPC mode when --vpc-id is provided', () => { + const result = validateAddHarnessOptions({ + ...base, + container: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-image:latest', + networkMode: 'VPC', + subnets: 'subnet-0a1b2c3d', + securityGroups: 'sg-0a1b2c3d', + vpcId: 'vpc-0a1b2c3d', + }); expect(result.valid).toBe(true); }); diff --git a/src/cli/commands/add/validate.ts b/src/cli/commands/add/validate.ts index 42534f759..ca5910293 100644 --- a/src/cli/commands/add/validate.ts +++ b/src/cli/commands/add/validate.ts @@ -1131,15 +1131,11 @@ export function validateAddHarnessOptions(options: AddHarnessCliOptions): Valida // VPC network-mode coupling: reject --subnets/--security-groups when network mode isn't VPC // (and require them when it is), instead of silently dropping them. Mirrors the agent path. - // Derive build type from --container: a dockerfile path means Container, absent/URI means CodeZip. - const containerValue = options.container; - const looksLikeDockerfile = - containerValue !== undefined && - (containerValue.endsWith('Dockerfile') || - containerValue.endsWith('.dockerfile') || - containerValue.startsWith('./') || - containerValue.startsWith('../')); - const harnessBuildType = looksLikeDockerfile ? 'Container' : undefined; + // Any --container value is a container build — a dockerfile path AND a prebuilt image URI both run + // CodeBuild (a URI export emits a `FROM ` Dockerfile stub), so both require --vpc-id in VPC + // mode. Gating on a dockerfile-only heuristic let a `--container ` VPC harness persist + // without a vpcId and then dead-end at deploy/export. + const harnessBuildType = options.container ? 'Container' : undefined; const vpcResult = validateVpcOptions(options, harnessBuildType); if (!vpcResult.valid) { return vpcResult; diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index ae4e146f5..3f0d02ac8 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -27,6 +27,7 @@ import { ExecLogger } from '../../logging'; import { MANAGED_MEMORY_DEPLOY_NOTICE, assertEnvFileExists, + backfillContainerVpcIds, bootstrapEnvironment, buildCdkProject, checkBootstrapNeeded, @@ -409,6 +410,15 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise 0) { + logger.log(`Resolved networkConfig.vpcId for Container+VPC build(s): ${backfill.backfilled.join(', ')}`, 'info'); + } + // Synthesize CloudFormation templates startStep('Synthesize CloudFormation'); const switchableIoHost = options.verbose || options.onDeployMessage ? createSwitchableIoHost() : undefined; diff --git a/src/cli/commands/export/fetch-harness-spec.ts b/src/cli/commands/export/fetch-harness-spec.ts index a52e1c735..3887436d1 100644 --- a/src/cli/commands/export/fetch-harness-spec.ts +++ b/src/cli/commands/export/fetch-harness-spec.ts @@ -8,6 +8,7 @@ import type { HarnessModelConfiguration, } from '../../aws/agentcore-harness'; import { getHarness } from '../../aws/agentcore-harness'; +import { isContainerBuild } from '../../../schema/constants'; import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; /** @@ -23,14 +24,11 @@ export async function fetchHarnessSpecByArn( const harnessId = harnessIdFromArn(arn); const { harness } = await getHarness({ region, harnessId }); const result = mapApiHarnessToSpec(harness); + // Resolve vpcId for a container build in VPC mode (the case that requires it — CodeBuild can't infer + // the VPC). A prebuilt-containerUri harness still exports a `FROM ` Dockerfile stub that + // CodeBuild builds, so isContainerBuild covers it too; only a CodeZip harness skips resolution. const nc = result.spec.networkConfig; - // Resolve the VPC ID for any Container build in VPC mode — that's exactly the case the schema - // requires it for (CodeBuild can't infer the VPC). A harness is a Container build whenever it - // carries a containerUri OR a dockerfile (matching export's resolveBuildType): even a prebuilt - // containerUri export emits a `FROM ` Dockerfile stub that CodeBuild builds, so it needs - // vpcId too. Only a CodeZip harness (neither) genuinely skips resolution. - const isContainerBuild = !!result.spec.containerUri || !!result.spec.dockerfile; - if (isContainerBuild && result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { + if (isContainerBuild(result.spec) && result.spec.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); } return result; diff --git a/src/cli/commands/import/actions.ts b/src/cli/commands/import/actions.ts index 30700e616..02d331a43 100644 --- a/src/cli/commands/import/actions.ts +++ b/src/cli/commands/import/actions.ts @@ -12,6 +12,7 @@ import { arnPrefix } from '../../aws/partition'; import { ANSI, PYTHON_BASE_IMAGE } from '../../constants'; import { ExecLogger } from '../../logging'; import { setupPythonProject } from '../../operations/python/setup'; +import { isContainerBuild } from '../../../schema/constants'; import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; import { executeCdkImportPipeline } from './import-pipeline'; import { copyDirRecursive, fixPyprojectForSetuptools, toStackName } from './import-utils'; @@ -341,7 +342,7 @@ export async function handleImport(options: ImportOptions): Promise 0 && diff --git a/src/cli/commands/import/types.ts b/src/cli/commands/import/types.ts index 0739eccbc..f0bfa965f 100644 --- a/src/cli/commands/import/types.ts +++ b/src/cli/commands/import/types.ts @@ -3,6 +3,7 @@ import type { AgentCoreProjectSpec, AuthorizerConfig, AwsDeploymentTarget, + NetworkConfig, RuntimeAuthorizerType, } from '../../../schema'; import type { ExecLogger } from '../../logging'; @@ -19,7 +20,7 @@ export interface ParsedStarterToolkitAgent { language: 'python' | 'typescript'; sourcePath?: string; networkMode: 'PUBLIC' | 'VPC'; - networkConfig?: { subnets: string[]; securityGroups: string[]; vpcId?: string }; + networkConfig?: NetworkConfig; protocol: 'HTTP' | 'MCP' | 'A2A' | 'AGUI'; enableOtel: boolean; /** Physical agent runtime ID from the starter toolkit deployment */ diff --git a/src/cli/commands/shared/__tests__/vpc-utils.test.ts b/src/cli/commands/shared/__tests__/vpc-utils.test.ts index 5b9f778a8..ff53fdf5a 100644 --- a/src/cli/commands/shared/__tests__/vpc-utils.test.ts +++ b/src/cli/commands/shared/__tests__/vpc-utils.test.ts @@ -1,11 +1,26 @@ import { parseCommaSeparatedList, + resolveVpcIdFromSubnets, validateSecurityGroupIds, validateSubnetIds, validateVpcId, validateVpcOptions, } from '../vpc-utils'; -import { describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockSend } = vi.hoisted(() => ({ mockSend: vi.fn() })); + +vi.mock('@aws-sdk/client-ec2', async () => { + const actual = await vi.importActual('@aws-sdk/client-ec2'); + return { + ...actual, + EC2Client: class { + send = mockSend; + }, + }; +}); + +vi.mock('../../../aws/account', () => ({ getCredentialProvider: () => undefined })); describe('parseCommaSeparatedList', () => { it('returns undefined for undefined input', () => { @@ -166,6 +181,58 @@ describe('validateVpcOptions vpcId requirement', () => { }); }); +describe('resolveVpcIdFromSubnets', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns the VPC ID when all subnets resolve to one VPC', async () => { + mockSend.mockResolvedValue({ + Subnets: [ + { SubnetId: 'subnet-0000000000000000a', VpcId: 'vpc-0123456789abcdef0' }, + { SubnetId: 'subnet-0000000000000000b', VpcId: 'vpc-0123456789abcdef0' }, + ], + }); + const vpcId = await resolveVpcIdFromSubnets(['subnet-0000000000000000a', 'subnet-0000000000000000b'], 'us-east-1'); + expect(vpcId).toBe('vpc-0123456789abcdef0'); + }); + + it('does NOT blindly trust Subnets[0]: rejects when subnets span multiple VPCs', async () => { + // DescribeSubnets does not guarantee response order, and cross-VPC subnets would misconfigure the + // build. The result must not silently be whichever VpcId happened to come back first. + mockSend.mockResolvedValue({ + Subnets: [ + { SubnetId: 'subnet-0000000000000000a', VpcId: 'vpc-0000000000000000a' }, + { SubnetId: 'subnet-0000000000000000b', VpcId: 'vpc-0000000000000000b' }, + ], + }); + await expect( + resolveVpcIdFromSubnets(['subnet-0000000000000000a', 'subnet-0000000000000000b'], 'us-east-1') + ).rejects.toThrow(/span multiple VPCs/); + }); + + it('throws naming ec2:DescribeSubnets when the API call fails', async () => { + mockSend.mockRejectedValue(new Error('AccessDenied')); + await expect(resolveVpcIdFromSubnets(['subnet-0000000000000000a'], 'us-east-1')).rejects.toThrow( + /ec2:DescribeSubnets permission is required/ + ); + }); + + it('throws when DescribeSubnets returns no VPC ID', async () => { + mockSend.mockResolvedValue({ Subnets: [] }); + await expect(resolveVpcIdFromSubnets(['subnet-0000000000000000a'], 'us-east-1')).rejects.toThrow( + /returned no VPC ID/ + ); + }); + + it('lists all requested subnets in the error (not just the first) on failure', async () => { + mockSend.mockRejectedValue(new Error('boom')); + await expect( + resolveVpcIdFromSubnets(['subnet-0000000000000000a', 'subnet-0000000000000000b'], 'us-east-1') + ).rejects.toThrow(/subnet-0000000000000000a, subnet-0000000000000000b/); + }); +}); + describe('validateVpcOptions - format validation', () => { it('rejects VPC mode with invalid subnet format', () => { const result = validateVpcOptions({ diff --git a/src/cli/commands/shared/vpc-utils.ts b/src/cli/commands/shared/vpc-utils.ts index 1485a8f86..db855ffe4 100644 --- a/src/cli/commands/shared/vpc-utils.ts +++ b/src/cli/commands/shared/vpc-utils.ts @@ -67,26 +67,38 @@ export function validateVpcId(value: string): true | string { } /** - * Resolve the VPC ID from the first subnet via ec2:DescribeSubnets. - * Throws a clear error naming the required permission if the call fails. + * Resolve the VPC ID that a set of subnets belongs to, via ec2:DescribeSubnets. + * + * All subnets in a build's network config must live in one VPC (CodeBuild and Lambda both require + * it), so this asserts they resolve to a single VpcId rather than blindly trusting the first result + * — DescribeSubnets does not guarantee response order matches the request, and a wrong vpcId would + * silently misconfigure the build. Throws a clear error naming ec2:DescribeSubnets on failure. */ export async function resolveVpcIdFromSubnets(subnetIds: string[], region: string): Promise { const ec2 = new EC2Client({ region, credentials: getCredentialProvider() }); - let vpcId: string | undefined; + let subnets: { SubnetId?: string; VpcId?: string }[]; try { const resp = await ec2.send(new DescribeSubnetsCommand({ SubnetIds: subnetIds })); - vpcId = resp.Subnets?.[0]?.VpcId; + subnets = resp.Subnets ?? []; } catch (err) { throw new Error( - `Failed to resolve VPC ID from subnet ${subnetIds[0]}: ec2:DescribeSubnets permission is required. Cause: ${(err as Error).message ?? String(err)}` + `Failed to resolve VPC ID from subnets [${subnetIds.join(', ')}]: ec2:DescribeSubnets permission is required. Cause: ${(err as Error).message ?? String(err)}` ); } - if (!vpcId) { + + const vpcIds = new Set(subnets.map(s => s.VpcId).filter((v): v is string => !!v)); + if (vpcIds.size === 0) { + throw new Error( + `ec2:DescribeSubnets returned no VPC ID for subnets [${subnetIds.join(', ')}]. Verify the subnets exist and ec2:DescribeSubnets permission is granted.` + ); + } + if (vpcIds.size > 1) { throw new Error( - `ec2:DescribeSubnets returned no VPC ID for subnet ${subnetIds[0]}. Verify the subnet exists and ec2:DescribeSubnets permission is granted.` + `Subnets [${subnetIds.join(', ')}] span multiple VPCs (${[...vpcIds].join(', ')}). ` + + `All subnets for a container build must be in the same VPC.` ); } - return vpcId; + return [...vpcIds][0]!; } export function validateVpcOptions(options: VpcOptions, buildType?: string): VpcValidationResult { diff --git a/src/cli/operations/deploy/__tests__/backfill-vpc-id.test.ts b/src/cli/operations/deploy/__tests__/backfill-vpc-id.test.ts new file mode 100644 index 000000000..0b068bd14 --- /dev/null +++ b/src/cli/operations/deploy/__tests__/backfill-vpc-id.test.ts @@ -0,0 +1,150 @@ +import type { ConfigIO } from '../../../../lib'; +import type { AgentCoreProjectSpec, HarnessSpec } from '../../../../schema'; +import { backfillContainerVpcIds } from '../backfill-vpc-id'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockResolveVpcId } = vi.hoisted(() => ({ mockResolveVpcId: vi.fn() })); + +vi.mock('../../../commands/shared/vpc-utils', async () => { + const actual = await vi.importActual( + '../../../commands/shared/vpc-utils' + ); + return { ...actual, resolveVpcIdFromSubnets: (...args: unknown[]) => mockResolveVpcId(...args) }; +}); + +function makeConfigIO(overrides: Partial = {}): { + configIO: ConfigIO; + writeProjectSpec: ReturnType; + writeHarnessSpec: ReturnType; + harnessSpecs: Record; +} { + const harnessSpecs: Record = {}; + const writeProjectSpec = vi.fn().mockResolvedValue(undefined); + const writeHarnessSpec = vi.fn().mockImplementation((name: string, data: HarnessSpec) => { + harnessSpecs[name] = data; + return Promise.resolve(); + }); + const configIO = { + writeProjectSpec, + writeHarnessSpec, + readHarnessSpec: (name: string) => Promise.resolve(harnessSpecs[name]), + ...overrides, + } as unknown as ConfigIO; + return { configIO, writeProjectSpec, writeHarnessSpec, harnessSpecs }; +} + +function containerVpcRuntime(overrides: Record = {}) { + return { + name: 'agent1', + build: 'Container', + entrypoint: 'main.py', + codeLocation: './agents/agent1', + networkMode: 'VPC', + networkConfig: { subnets: ['subnet-0000000000000000a'], securityGroups: ['sg-0000000000000000a'] }, + ...overrides, + }; +} + +describe('backfillContainerVpcIds', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockResolveVpcId.mockResolvedValue('vpc-0123456789abcdef0'); + }); + + it('backfills + persists vpcId for a Container+VPC runtime missing it', async () => { + const runtime = containerVpcRuntime(); + const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec; + const { configIO, writeProjectSpec } = makeConfigIO(); + + const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1'); + + expect(mockResolveVpcId).toHaveBeenCalledWith(['subnet-0000000000000000a'], 'us-east-1'); + expect((runtime.networkConfig as { vpcId?: string }).vpcId).toBe('vpc-0123456789abcdef0'); + expect(writeProjectSpec).toHaveBeenCalledOnce(); + expect(result.backfilled).toEqual(['agent1']); + }); + + it('is a no-op when vpcId is already present (fresh create)', async () => { + const runtime = containerVpcRuntime({ + networkConfig: { + subnets: ['subnet-0000000000000000a'], + securityGroups: ['sg-0000000000000000a'], + vpcId: 'vpc-aaaaaaaa', + }, + }); + const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec; + const { configIO, writeProjectSpec } = makeConfigIO(); + + const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1'); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + expect(writeProjectSpec).not.toHaveBeenCalled(); + expect(result.backfilled).toEqual([]); + }); + + it('does not touch a CodeZip+VPC runtime (no CodeBuild → no vpcId needed)', async () => { + const runtime = containerVpcRuntime({ build: 'CodeZip' }); + const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec; + const { configIO, writeProjectSpec } = makeConfigIO(); + + const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1'); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + expect(writeProjectSpec).not.toHaveBeenCalled(); + expect(result.backfilled).toEqual([]); + }); + + it('does not touch a PUBLIC-mode Container runtime', async () => { + const runtime = containerVpcRuntime({ networkMode: 'PUBLIC', networkConfig: undefined }); + const spec = { name: 'proj', runtimes: [runtime], harnesses: [] } as unknown as AgentCoreProjectSpec; + const { configIO, writeProjectSpec } = makeConfigIO(); + + const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1'); + + expect(mockResolveVpcId).not.toHaveBeenCalled(); + expect(writeProjectSpec).not.toHaveBeenCalled(); + expect(result.backfilled).toEqual([]); + }); + + it('backfills + persists a dockerfile harness missing vpcId (via harness.json)', async () => { + const spec = { + name: 'proj', + runtimes: [], + harnesses: [{ name: 'h1' }], + } as unknown as AgentCoreProjectSpec; + const { configIO, writeHarnessSpec, harnessSpecs } = makeConfigIO(); + harnessSpecs.h1 = { + name: 'h1', + dockerfile: 'Dockerfile', + networkMode: 'VPC', + networkConfig: { subnets: ['subnet-0000000000000000b'], securityGroups: ['sg-0000000000000000b'] }, + } as unknown as HarnessSpec; + + const result = await backfillContainerVpcIds(configIO, spec, 'us-west-2'); + + expect(mockResolveVpcId).toHaveBeenCalledWith(['subnet-0000000000000000b'], 'us-west-2'); + expect(writeHarnessSpec).toHaveBeenCalledOnce(); + expect(harnessSpecs.h1.networkConfig?.vpcId).toBe('vpc-0123456789abcdef0'); + expect(result.backfilled).toEqual(['h1']); + }); + + it('backfills a prebuilt-containerUri harness too (it still runs CodeBuild)', async () => { + const spec = { + name: 'proj', + runtimes: [], + harnesses: [{ name: 'h2' }], + } as unknown as AgentCoreProjectSpec; + const { configIO, harnessSpecs } = makeConfigIO(); + harnessSpecs.h2 = { + name: 'h2', + containerUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/repo:tag', + networkMode: 'VPC', + networkConfig: { subnets: ['subnet-0000000000000000c'], securityGroups: ['sg-0000000000000000c'] }, + } as unknown as HarnessSpec; + + const result = await backfillContainerVpcIds(configIO, spec, 'us-east-1'); + + expect(harnessSpecs.h2.networkConfig?.vpcId).toBe('vpc-0123456789abcdef0'); + expect(result.backfilled).toEqual(['h2']); + }); +}); diff --git a/src/cli/operations/deploy/backfill-vpc-id.ts b/src/cli/operations/deploy/backfill-vpc-id.ts new file mode 100644 index 000000000..3dcb520de --- /dev/null +++ b/src/cli/operations/deploy/backfill-vpc-id.ts @@ -0,0 +1,70 @@ +import type { ConfigIO } from '../../../lib'; +import { isContainerBuild } from '../../../schema/constants'; +import type { AgentCoreProjectSpec } from '../../../schema'; +import { resolveVpcIdFromSubnets } from '../../commands/shared/vpc-utils'; + +export interface BackfillVpcIdResult { + /** Names of runtimes/harnesses whose vpcId was resolved and persisted this run. */ + backfilled: string[]; +} + +/** + * Backfill `networkConfig.vpcId` for any Container+VPC runtime or harness that is missing it, then + * persist the change to disk so CDK synth (a separate process that re-reads agentcore.json / + * harness.json) sees the resolved value. + * + * vpcId became required for Container+VPC builds after this feature shipped, but agentcore.json + * files written before then have only subnets + security groups. Rather than hard-fail those + * configs on read, deploy resolves the VPC from the first subnet via ec2:DescribeSubnets — a subnet + * uniquely determines its VPC — and writes it back once. Fresh creates already collect --vpc-id at + * the CLI layer, so this is a no-op for them. + * + * @returns the names that were backfilled (empty if none needed it). + */ +export async function backfillContainerVpcIds( + configIO: ConfigIO, + projectSpec: AgentCoreProjectSpec, + region: string +): Promise { + const backfilled: string[] = []; + + // Runtimes live inline in agentcore.json; mutate the spec and write it back once at the end if any + // changed. + let runtimesChanged = false; + for (const runtime of projectSpec.runtimes ?? []) { + const nc = runtime.networkConfig; + if ( + isContainerBuild(runtime) && + runtime.networkMode === 'VPC' && + nc && + nc.subnets.length > 0 && + !nc.vpcId + ) { + nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); + runtimesChanged = true; + backfilled.push(runtime.name); + } + } + if (runtimesChanged) { + await configIO.writeProjectSpec(projectSpec); + } + + // Harnesses live in their own harness.json files; resolve + persist each independently. + for (const entry of projectSpec.harnesses ?? []) { + const harness = await configIO.readHarnessSpec(entry.name); + const nc = harness.networkConfig; + if ( + isContainerBuild(harness) && + harness.networkMode === 'VPC' && + nc && + nc.subnets.length > 0 && + !nc.vpcId + ) { + nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); + await configIO.writeHarnessSpec(entry.name, harness); + backfilled.push(entry.name); + } + } + + return { backfilled }; +} diff --git a/src/cli/operations/deploy/index.ts b/src/cli/operations/deploy/index.ts index 29f0fd7e7..d4eb1e5df 100644 --- a/src/cli/operations/deploy/index.ts +++ b/src/cli/operations/deploy/index.ts @@ -65,6 +65,9 @@ export { export { ensureDefaultDeploymentTarget } from './ensure-target'; +// Pre-synth backfill of vpcId for pre-existing Container+VPC configs written before vpcId was added +export { backfillContainerVpcIds, type BackfillVpcIdResult } from './backfill-vpc-id'; + // Managed-memory heads-up (shared by the CLI command + TUI deploy flow + add harness) export { MANAGED_MEMORY_DEPLOY_NOTICE, diff --git a/src/schema/constants.ts b/src/schema/constants.ts index ca8c15404..25e12a28b 100644 --- a/src/schema/constants.ts +++ b/src/schema/constants.ts @@ -196,6 +196,26 @@ export const SUBNET_ID_PATTERN = /^subnet-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; /** Matches a security group ID: sg- followed by 8 or 17 lowercase hex digits. */ export const SECURITY_GROUP_ID_PATTERN = /^sg-(?:[0-9a-f]{8}|[0-9a-f]{17})$/; +/** CodeBuild caps a project's VPC config at 5 security groups (vs 16 for the runtime). */ +export const MAX_CONTAINER_BUILD_SECURITY_GROUPS = 5; + +/** + * Whether a build runs the container-image pipeline (CodeBuild) and therefore requires an explicit + * `networkConfig.vpcId` in VPC mode — CodeBuild's CreateProject cannot infer the VPC from subnets + * alone. This is the single source of truth for "is this a container build?", shared by the schema + * refinements (agent-env + harness), the CLI validators, and the import/export vpcId-resolution + * guards so the decision cannot drift between them. + * + * A build is a container build when ANY of these hold: + * - `build === 'Container'` (agent specs, and harnesses whose build type resolved to Container), + * - it carries a `dockerfile` (from-source container harness), or + * - it carries a `containerUri` (prebuilt image harness — export still emits a `FROM ` + * Dockerfile stub that CodeBuild builds, so it needs a vpcId too). + */ +export function isContainerBuild(spec: { build?: string; containerUri?: string; dockerfile?: string }): boolean { + return spec.build === 'Container' || !!spec.containerUri || !!spec.dockerfile; +} + // ============================================================================ // Protocol Mode // ============================================================================ diff --git a/src/schema/schemas/__tests__/agent-env.test.ts b/src/schema/schemas/__tests__/agent-env.test.ts index c2e5d721f..57fc01657 100644 --- a/src/schema/schemas/__tests__/agent-env.test.ts +++ b/src/schema/schemas/__tests__/agent-env.test.ts @@ -768,12 +768,13 @@ describe('AgentEnvSpecSchema - vpcId validation', () => { }, }; - it('requires vpcId for Container builds in VPC mode', () => { + it('accepts a Container+VPC agent WITHOUT vpcId at the schema level (backfilled at deploy)', () => { + // vpcId is required to build, but NOT enforced on read/write: pre-existing agentcore.json files + // written before the vpcId field existed must still load. The CLI validators require --vpc-id for + // fresh creates, deploy backfills a missing vpcId from the subnets, and the CDK construct fails + // fast at synth. Keeping the schema lenient is what lets status/remove/validate work on old configs. const result = AgentEnvSpecSchema.safeParse(baseVpcAgent); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.issues.some(i => i.path.includes('vpcId'))).toBe(true); - } + expect(result.success).toBe(true); }); it('accepts Container+VPC agent with vpcId', () => { diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index afd0f9d85..11c2e51b5 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -4,6 +4,8 @@ * @module agent-env */ import { + isContainerBuild, + MAX_CONTAINER_BUILD_SECURITY_GROUPS, NetworkModeSchema, ProtocolModeSchema, RuntimeVersionSchema as RuntimeVersionSchemaFromConstants, @@ -376,23 +378,21 @@ export const AgentEnvSpecSchema = z path: ['networkConfig'], }); } - if (data.networkMode === 'VPC' && data.build === 'Container' && !data.networkConfig?.vpcId) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - 'networkConfig.vpcId is required for Container builds in VPC mode (CodeBuild cannot infer the VPC from subnets)', - path: ['networkConfig', 'vpcId'], - }); - } + // NOTE: vpcId is NOT required here at the schema (read/write) level. A Container+VPC agent needs + // a vpcId to build (CodeBuild can't infer it), but enforcing that on read would hard-fail + // pre-existing agentcore.json files written before vpcId existed. Instead: the CLI input + // validators require --vpc-id for fresh Container+VPC creates, deploy backfills a missing vpcId + // from the subnets (ec2:DescribeSubnets) and persists it, and the CDK construct fails fast if it + // is still missing at synth. This keeps `status`/`remove`/`validate` working on old configs. if ( data.networkMode === 'VPC' && - data.build === 'Container' && + isContainerBuild(data) && data.networkConfig && - data.networkConfig.securityGroups.length > 5 + data.networkConfig.securityGroups.length > MAX_CONTAINER_BUILD_SECURITY_GROUPS ) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: 'Container builds in VPC mode allow at most 5 security groups (CodeBuild limit)', + message: `Container builds in VPC mode allow at most ${MAX_CONTAINER_BUILD_SECURITY_GROUPS} security groups (CodeBuild limit)`, path: ['networkConfig', 'securityGroups'], }); } diff --git a/src/schema/schemas/primitives/__tests__/harness.test.ts b/src/schema/schemas/primitives/__tests__/harness.test.ts index 247f1113b..87c4d600e 100644 --- a/src/schema/schemas/primitives/__tests__/harness.test.ts +++ b/src/schema/schemas/primitives/__tests__/harness.test.ts @@ -1272,9 +1272,11 @@ describe('HarnessSpecSchema vpcId for container builds', () => { networkConfig: { subnets: ['subnet-0123456789abcdef0'], securityGroups: ['sg-0123456789abcdef0'] }, }; - it('requires vpcId for a dockerfile build in VPC mode', () => { + it('accepts a dockerfile build in VPC mode WITHOUT vpcId at the schema level (backfilled at deploy)', () => { + // vpcId is required to build but not enforced on read/write — old configs must still load. See the + // matching note in agent-env.ts; deploy backfills it and the CDK construct fails fast if missing. const r = HarnessSpecSchema.safeParse(baseDockerfileHarness); - expect(r.success).toBe(false); + expect(r.success).toBe(true); }); it('accepts a dockerfile build in VPC mode when vpcId is present', () => { @@ -1285,7 +1287,10 @@ describe('HarnessSpecSchema vpcId for container builds', () => { expect(r.success).toBe(true); }); - it('does NOT require vpcId for a containerUri harness in VPC mode (no build)', () => { + it('accepts a containerUri harness in VPC mode without vpcId (backfilled at deploy, like dockerfile)', () => { + // A containerUri harness IS a container build (export emits a `FROM ` Dockerfile that + // CodeBuild builds), so it is treated the same as a dockerfile harness — lenient on read, vpcId + // backfilled at deploy. const r = HarnessSpecSchema.safeParse({ ...minimalHarnessForVpc, containerUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/repo:tag', @@ -1345,7 +1350,7 @@ describe('HarnessSpecSchema — SG≤5 for dockerfile builds in VPC mode', () => expect(r.success).toBe(true); }); - it('accepts containerUri+VPC with 6 security groups (no build, no cap)', () => { + it('rejects containerUri+VPC with 6 security groups (a containerUri harness is a container build, so the cap applies)', () => { const r = HarnessSpecSchema.safeParse({ ...minimalHarnessForSg, containerUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/repo:tag', @@ -1353,8 +1358,12 @@ describe('HarnessSpecSchema — SG≤5 for dockerfile builds in VPC mode', () => networkConfig: { subnets: ['subnet-0123456789abcdef0'], securityGroups: sixSgs, + vpcId: 'vpc-0123456789abcdef0', }, }); - expect(r.success).toBe(true); + expect(r.success).toBe(false); + if (!r.success) { + expect(r.error.issues.some(i => i.message.includes('5 security groups'))).toBe(true); + } }); }); diff --git a/src/schema/schemas/primitives/harness.ts b/src/schema/schemas/primitives/harness.ts index 8cd23dce5..7fa34f72f 100644 --- a/src/schema/schemas/primitives/harness.ts +++ b/src/schema/schemas/primitives/harness.ts @@ -1,4 +1,4 @@ -import { NetworkModeSchema } from '../../constants'; +import { isContainerBuild, MAX_CONTAINER_BUILD_SECURITY_GROUPS, NetworkModeSchema } from '../../constants'; import { EfsAccessPointConfigSchema, LifecycleConfigurationSchema, @@ -633,23 +633,21 @@ export const HarnessSpecSchema = z path: ['networkConfig'], }); } - if (data.networkMode === 'VPC' && data.dockerfile && !data.networkConfig?.vpcId) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - 'networkConfig.vpcId is required for container (dockerfile) builds in VPC mode (CodeBuild cannot infer the VPC from subnets)', - path: ['networkConfig', 'vpcId'], - }); - } + // vpcId is NOT required here at the schema level (see the matching note in agent-env.ts): a + // container harness build in VPC mode needs one, but requiring it on read would hard-fail + // pre-existing configs. The CLI validators enforce it for fresh creates, deploy backfills + + // persists a missing vpcId, and the CDK construct fails fast at synth. The SG cap stays here — it + // is a hard CodeBuild limit that backfill can't satisfy. Gating on the shared isContainerBuild + // predicate keeps this in sync (a dockerfile-only gate previously missed containerUri harnesses). if ( data.networkMode === 'VPC' && - data.dockerfile && + isContainerBuild(data) && data.networkConfig && - data.networkConfig.securityGroups.length > 5 + data.networkConfig.securityGroups.length > MAX_CONTAINER_BUILD_SECURITY_GROUPS ) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: 'Container (dockerfile) builds in VPC mode allow at most 5 security groups (CodeBuild limit)', + message: `Container builds in VPC mode allow at most ${MAX_CONTAINER_BUILD_SECURITY_GROUPS} security groups (CodeBuild limit)`, path: ['networkConfig', 'securityGroups'], }); } From e8f71d500b335ce84c06d51af09397c5b6b0e937 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 2 Jul 2026 17:05:18 +0000 Subject: [PATCH 16/17] style: prettier formatting for review-fix changes --- docs/PERMISSIONS.md | 27 +++++++++---------- docs/container-builds.md | 16 +++++------ src/cli/aws/agentcore-control.ts | 5 ++-- src/cli/commands/export/fetch-harness-spec.ts | 2 +- src/cli/commands/import/actions.ts | 2 +- src/cli/operations/deploy/backfill-vpc-id.ts | 18 +++---------- src/schema/schemas/agent-env.ts | 2 +- src/schema/schemas/primitives/harness.ts | 2 +- 8 files changed, 31 insertions(+), 43 deletions(-) diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md index a5f7f38c3..74b4d0bf7 100644 --- a/docs/PERMISSIONS.md +++ b/docs/PERMISSIONS.md @@ -321,23 +321,22 @@ Required only when deploying an agent with EFS or S3 filesystem mounts and a VPC preflight check that the agent's subnets and security groups are correctly set up for NFS (port 2049) before deploying. These EC2 and EFS `Describe*` actions do not support resource-level scoping, so `Resource` must be `*`. -| Action | CLI Commands | Purpose | -| ----------------------------------------------------- | ------------------ | ------------------------------------------------------------------ | -| `ec2:DescribeSecurityGroups` | `create`, `deploy` | Validate agent/mount-target security groups allow NFS (port 2049) | +| Action | CLI Commands | Purpose | +| ----------------------------------------------------- | ---------------------------- | -------------------------------------------------------------------------------------------------------------------------- | +| `ec2:DescribeSecurityGroups` | `create`, `deploy` | Validate agent/mount-target security groups allow NFS (port 2049) | | `ec2:DescribeSubnets` | `create`, `deploy`, `import` | Validate mount-target subnets are in the agent's VPC and AZs; resolve the VPC ID for Container+VPC builds (see note below) | -| `elasticfilesystem:DescribeAccessPoints` | `create`, `deploy` | Resolve the EFS access point and its file system | -| `elasticfilesystem:DescribeMountTargets` | `create`, `deploy` | Find the EFS mount targets to validate against the agent's subnets | -| `elasticfilesystem:DescribeMountTargetSecurityGroups` | `create`, `deploy` | Check mount-target security groups allow NFS from the agent | -| `s3files:ListMountTargets` | `create`, `deploy` | List the S3 Files access point's mount targets | +| `elasticfilesystem:DescribeAccessPoints` | `create`, `deploy` | Resolve the EFS access point and its file system | +| `elasticfilesystem:DescribeMountTargets` | `create`, `deploy` | Find the EFS mount targets to validate against the agent's subnets | +| `elasticfilesystem:DescribeMountTargetSecurityGroups` | `create`, `deploy` | Check mount-target security groups allow NFS from the agent | +| `s3files:ListMountTargets` | `create`, `deploy` | List the S3 Files access point's mount targets | > **Container builds in VPC mode also require `ec2:DescribeSubnets`.** A Container build (agent or -> dockerfile/prebuilt-image harness) that deploys in VPC mode needs an explicit VPC ID — CodeBuild's -> `CreateProject` cannot infer the VPC from subnets. When the VPC ID is not already in the config -> (e.g. `import` of a deployed Container+VPC runtime, `export` of a container harness, or `deploy` of -> a project created before the VPC ID field existed), the CLI resolves it from the first subnet via -> `ec2:DescribeSubnets`. Grant this action for `import`, `export`, and `deploy` if you use Container -> builds in VPC mode, even without filesystem mounts. -| `s3files:GetMountTarget` | `create`, `deploy` | Inspect an S3 Files mount target's subnet/network configuration | +> dockerfile/prebuilt-image harness) that deploys in VPC mode needs an explicit VPC ID — CodeBuild's `CreateProject` +> cannot infer the VPC from subnets. When the VPC ID is not already in the config (e.g. `import` of a deployed +> Container+VPC runtime, `export` of a container harness, or `deploy` of a project created before the VPC ID field +> existed), the CLI resolves it from the first subnet via `ec2:DescribeSubnets`. Grant this action for `import`, +> `export`, and `deploy` if you use Container builds in VPC mode, even without filesystem mounts. | +> `s3files:GetMountTarget` | `create`, `deploy` | Inspect an S3 Files mount target's subnet/network configuration | ### Agent invocation diff --git a/docs/container-builds.md b/docs/container-builds.md index bd6817edd..1f34ea64e 100644 --- a/docs/container-builds.md +++ b/docs/container-builds.md @@ -147,12 +147,12 @@ the first subnet (via `ec2:DescribeSubnets`) and writes it back to the config ## Troubleshooting -| Error | Fix | -| -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | -| No container runtime found | Install Docker, Podman, or Finch | -| Runtime not ready | Docker: start Docker Desktop / `sudo systemctl start docker`. Podman: `podman machine start`. Finch: `finch vm init && finch vm start` | -| Dockerfile not found | Ensure `Dockerfile` exists in the agent's `codeLocation` directory | -| Image exceeds 2 GB | Use multi-stage builds, minimize packages, review `.dockerignore` | +| Error | Fix | +| ----------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| No container runtime found | Install Docker, Podman, or Finch | +| Runtime not ready | Docker: start Docker Desktop / `sudo systemctl start docker`. Podman: `podman machine start`. Finch: `finch vm init && finch vm start` | +| Dockerfile not found | Ensure `Dockerfile` exists in the agent's `codeLocation` directory | +| Image exceeds 2 GB | Use multi-stage builds, minimize packages, review `.dockerignore` | | `vpcId is required` at deploy/synth | Container+VPC build with no VPC ID. Grant `ec2:DescribeSubnets` so deploy can resolve it, or add `networkConfig.vpcId` to the config manually | -| Build hangs in VPC mode | The subnet has no egress. Use a NAT-routed subnet or add VPC endpoints (ECR api+dkr, S3, CloudWatch Logs, STS, CodeBuild) | -| Build fails | Check `pyproject.toml` is valid; verify network access for dependency installation | +| Build hangs in VPC mode | The subnet has no egress. Use a NAT-routed subnet or add VPC endpoints (ECR api+dkr, S3, CloudWatch Logs, STS, CodeBuild) | +| Build fails | Check `pyproject.toml` is valid; verify network access for dependency installation | diff --git a/src/cli/aws/agentcore-control.ts b/src/cli/aws/agentcore-control.ts index 1c9278894..09e11af2b 100644 --- a/src/cli/aws/agentcore-control.ts +++ b/src/cli/aws/agentcore-control.ts @@ -1,5 +1,5 @@ -import type { EvaluationLevel } from '../../schema/schemas/primitives/evaluator'; import type { NetworkConfig } from '../../schema'; +import type { EvaluationLevel } from '../../schema/schemas/primitives/evaluator'; import { resolveVpcIdFromSubnets } from '../commands/shared/vpc-utils'; import { getCredentialProvider } from './account'; import { controlPlaneEndpoint } from './stage-endpoint'; @@ -235,7 +235,8 @@ export async function getAgentRuntimeDetail(options: GetAgentRuntimeOptions): Pr // Resolve the VPC ID only for Container builds (the case that requires it — CodeBuild can't infer // the VPC). CodeZip+VPC runtimes don't need it, so this avoids the extra ec2:DescribeSubnets call // (and its IAM) for them. `isContainer` here comes from the runtime's containerConfiguration. - const vpcId = isContainer && subnets.length > 0 ? await resolveVpcIdFromSubnets(subnets, options.region) : undefined; + const vpcId = + isContainer && subnets.length > 0 ? await resolveVpcIdFromSubnets(subnets, options.region) : undefined; networkConfig = { subnets, securityGroups, vpcId }; } diff --git a/src/cli/commands/export/fetch-harness-spec.ts b/src/cli/commands/export/fetch-harness-spec.ts index 3887436d1..fce025241 100644 --- a/src/cli/commands/export/fetch-harness-spec.ts +++ b/src/cli/commands/export/fetch-harness-spec.ts @@ -1,5 +1,6 @@ import { ValidationError } from '../../../lib/errors/types'; import type { HarnessSpec } from '../../../schema'; +import { isContainerBuild } from '../../../schema/constants'; import type { HarnessSkill as ApiHarnessSkill, HarnessTool as ApiHarnessTool, @@ -8,7 +9,6 @@ import type { HarnessModelConfiguration, } from '../../aws/agentcore-harness'; import { getHarness } from '../../aws/agentcore-harness'; -import { isContainerBuild } from '../../../schema/constants'; import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; /** diff --git a/src/cli/commands/import/actions.ts b/src/cli/commands/import/actions.ts index 02d331a43..1c3f08116 100644 --- a/src/cli/commands/import/actions.ts +++ b/src/cli/commands/import/actions.ts @@ -7,12 +7,12 @@ import type { Credential, Memory, } from '../../../schema'; +import { isContainerBuild } from '../../../schema/constants'; import { validateAwsCredentials } from '../../aws/account'; import { arnPrefix } from '../../aws/partition'; import { ANSI, PYTHON_BASE_IMAGE } from '../../constants'; import { ExecLogger } from '../../logging'; import { setupPythonProject } from '../../operations/python/setup'; -import { isContainerBuild } from '../../../schema/constants'; import { resolveVpcIdFromSubnets } from '../shared/vpc-utils'; import { executeCdkImportPipeline } from './import-pipeline'; import { copyDirRecursive, fixPyprojectForSetuptools, toStackName } from './import-utils'; diff --git a/src/cli/operations/deploy/backfill-vpc-id.ts b/src/cli/operations/deploy/backfill-vpc-id.ts index 3dcb520de..a85195237 100644 --- a/src/cli/operations/deploy/backfill-vpc-id.ts +++ b/src/cli/operations/deploy/backfill-vpc-id.ts @@ -1,6 +1,6 @@ import type { ConfigIO } from '../../../lib'; -import { isContainerBuild } from '../../../schema/constants'; import type { AgentCoreProjectSpec } from '../../../schema'; +import { isContainerBuild } from '../../../schema/constants'; import { resolveVpcIdFromSubnets } from '../../commands/shared/vpc-utils'; export interface BackfillVpcIdResult { @@ -33,13 +33,7 @@ export async function backfillContainerVpcIds( let runtimesChanged = false; for (const runtime of projectSpec.runtimes ?? []) { const nc = runtime.networkConfig; - if ( - isContainerBuild(runtime) && - runtime.networkMode === 'VPC' && - nc && - nc.subnets.length > 0 && - !nc.vpcId - ) { + if (isContainerBuild(runtime) && runtime.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); runtimesChanged = true; backfilled.push(runtime.name); @@ -53,13 +47,7 @@ export async function backfillContainerVpcIds( for (const entry of projectSpec.harnesses ?? []) { const harness = await configIO.readHarnessSpec(entry.name); const nc = harness.networkConfig; - if ( - isContainerBuild(harness) && - harness.networkMode === 'VPC' && - nc && - nc.subnets.length > 0 && - !nc.vpcId - ) { + if (isContainerBuild(harness) && harness.networkMode === 'VPC' && nc && nc.subnets.length > 0 && !nc.vpcId) { nc.vpcId = await resolveVpcIdFromSubnets(nc.subnets, region); await configIO.writeHarnessSpec(entry.name, harness); backfilled.push(entry.name); diff --git a/src/schema/schemas/agent-env.ts b/src/schema/schemas/agent-env.ts index 11c2e51b5..e5cd74395 100644 --- a/src/schema/schemas/agent-env.ts +++ b/src/schema/schemas/agent-env.ts @@ -4,7 +4,6 @@ * @module agent-env */ import { - isContainerBuild, MAX_CONTAINER_BUILD_SECURITY_GROUPS, NetworkModeSchema, ProtocolModeSchema, @@ -12,6 +11,7 @@ import { SECURITY_GROUP_ID_PATTERN, SUBNET_ID_PATTERN, VPC_ID_PATTERN, + isContainerBuild, } from '../constants'; import type { DirectoryPath, FilePath } from '../types'; import { AuthorizerConfigSchema, RuntimeAuthorizerTypeSchema } from './auth'; diff --git a/src/schema/schemas/primitives/harness.ts b/src/schema/schemas/primitives/harness.ts index 7fa34f72f..203de4d1f 100644 --- a/src/schema/schemas/primitives/harness.ts +++ b/src/schema/schemas/primitives/harness.ts @@ -1,4 +1,4 @@ -import { isContainerBuild, MAX_CONTAINER_BUILD_SECURITY_GROUPS, NetworkModeSchema } from '../../constants'; +import { MAX_CONTAINER_BUILD_SECURITY_GROUPS, NetworkModeSchema, isContainerBuild } from '../../constants'; import { EfsAccessPointConfigSchema, LifecycleConfigurationSchema, From 59bbde0a291c46da1346f3e562b2b25ffe817329 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 2 Jul 2026 17:15:45 +0000 Subject: [PATCH 17/17] fix(create): thread vpcId + VPC validation through the create-harness path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review follow-up (@tejaskash / automation, command.tsx harness path): `agentcore create --container --network-mode VPC --vpc-id ...` silently dropped --vpc-id. The agent create path forwarded it, but the harness path did not: CreateHarnessProjectOptions had no vpcId field, so the value never reached harnessPrimitive.add (which does accept it), and validateCreateHarnessOptions never ran validateVpcOptions. - Add vpcId to CreateHarnessProjectOptions and forward it into harnessPrimitive.add. - Forward options.vpcId from command.tsx's create-harness dispatch. - validateCreateHarnessOptions now runs validateVpcOptions with a Container build type derived from --container (any value ⇒ container build), so a missing or malformed --vpc-id surfaces the friendly CLI error instead of being dropped and masked by the deploy-time backfill. Tests: create-harness VPC cases (requires --vpc-id for dockerfile + containerUri, accepts with valid vpcId, rejects malformed, rejects vpcId without VPC mode). --- .../create/__tests__/harness-validate.test.ts | 56 +++++++++++++++++++ src/cli/commands/create/command.tsx | 1 + src/cli/commands/create/harness-action.ts | 2 + src/cli/commands/create/harness-validate.ts | 19 +++++++ 4 files changed, 78 insertions(+) diff --git a/src/cli/commands/create/__tests__/harness-validate.test.ts b/src/cli/commands/create/__tests__/harness-validate.test.ts index a7f82c608..113423d25 100644 --- a/src/cli/commands/create/__tests__/harness-validate.test.ts +++ b/src/cli/commands/create/__tests__/harness-validate.test.ts @@ -287,3 +287,59 @@ describe('validateCreateHarnessOptions - S3 Files access points', () => { expect(result.error).toContain('matching pairs'); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// VPC / vpcId validation on the create-harness path (parity with add harness) +// ───────────────────────────────────────────────────────────────────────────── + +describe('validateCreateHarnessOptions - VPC networking', () => { + const VALID_VPC = 'vpc-07086549ccf106a5d'; + + it('requires --vpc-id for a dockerfile container build in VPC mode', () => { + const result = validateCreateHarnessOptions( + { ...baseOptions, ...vpcOptions, container: './Dockerfile' }, + makeCwd() + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('--vpc-id is required'); + }); + + it('requires --vpc-id for a prebuilt containerUri build in VPC mode (still runs CodeBuild)', () => { + const result = validateCreateHarnessOptions( + { + ...baseOptions, + ...vpcOptions, + container: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-image:latest', + }, + makeCwd() + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('--vpc-id is required'); + }); + + it('accepts a container build in VPC mode when --vpc-id is provided', () => { + const result = validateCreateHarnessOptions( + { ...baseOptions, ...vpcOptions, container: './Dockerfile', vpcId: VALID_VPC }, + makeCwd() + ); + expect(result.valid).toBe(true); + }); + + it('rejects a malformed --vpc-id', () => { + const result = validateCreateHarnessOptions( + { ...baseOptions, ...vpcOptions, container: './Dockerfile', vpcId: 'vpc-XYZ' }, + makeCwd() + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('Invalid VPC ID format'); + }); + + it('rejects --vpc-id supplied without --network-mode VPC', () => { + const result = validateCreateHarnessOptions( + { ...baseOptions, container: './Dockerfile', vpcId: VALID_VPC }, + makeCwd() + ); + expect(result.valid).toBe(false); + expect(result.error).toContain('only valid with --network-mode VPC'); + }); +}); diff --git a/src/cli/commands/create/command.tsx b/src/cli/commands/create/command.tsx index bee7111f4..67998b5a1 100644 --- a/src/cli/commands/create/command.tsx +++ b/src/cli/commands/create/command.tsx @@ -282,6 +282,7 @@ async function handleCreateHarnessCLI(options: CreateOptions): Promise { networkMode: options.networkMode as NetworkMode | undefined, subnets: parseCommaSeparatedList(options.subnets), securityGroups: parseCommaSeparatedList(options.securityGroups), + vpcId: options.vpcId, idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined, maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined, sessionStoragePath: options.sessionStorageMountPath, diff --git a/src/cli/commands/create/harness-action.ts b/src/cli/commands/create/harness-action.ts index a6e9db258..b153cc560 100644 --- a/src/cli/commands/create/harness-action.ts +++ b/src/cli/commands/create/harness-action.ts @@ -26,6 +26,7 @@ export interface CreateHarnessProjectOptions { networkMode?: NetworkMode; subnets?: string[]; securityGroups?: string[]; + vpcId?: string; idleTimeout?: number; maxLifetime?: number; sessionStoragePath?: string; @@ -76,6 +77,7 @@ export async function createProjectWithHarness(options: CreateHarnessProjectOpti networkMode: options.networkMode, subnets: options.subnets, securityGroups: options.securityGroups, + vpcId: options.vpcId, idleTimeout: options.idleTimeout, maxLifetime: options.maxLifetime, sessionStoragePath: options.sessionStoragePath, diff --git a/src/cli/commands/create/harness-validate.ts b/src/cli/commands/create/harness-validate.ts index 926cda032..67ceb2ae6 100644 --- a/src/cli/commands/create/harness-validate.ts +++ b/src/cli/commands/create/harness-validate.ts @@ -11,6 +11,7 @@ import { validateS3FilesAccessPointArn, zipAccessPointPairs, } from '../shared/filesystem-utils'; +import { validateVpcOptions } from '../shared/vpc-utils'; import { validateFolderNotExists } from './validate'; export interface CreateHarnessCliOptions { @@ -29,6 +30,7 @@ export interface CreateHarnessCliOptions { networkMode?: string; subnets?: string; securityGroups?: string; + vpcId?: string; sessionStorageMountPath?: string; efsAccessPointArn?: string[]; efsMountPath?: string[]; @@ -160,5 +162,22 @@ export function validateCreateHarnessOptions(options: CreateHarnessCliOptions, c } } + // VPC coupling for the create-harness path (mirrors validateAddHarnessOptions). Any --container + // value is a container build (dockerfile OR prebuilt image URI — both run CodeBuild), so --vpc-id + // is required in VPC mode; validate + surface the friendly error at CLI time instead of dropping it. + const harnessBuildType = options.container ? 'Container' : undefined; + const vpcResult = validateVpcOptions( + { + networkMode: options.networkMode, + subnets: options.subnets, + securityGroups: options.securityGroups, + vpcId: options.vpcId, + }, + harnessBuildType + ); + if (!vpcResult.valid) { + return vpcResult; + } + return { valid: true }; }