diff --git a/cdk/src/constructs/bedrock-models.ts b/cdk/src/constructs/bedrock-models.ts new file mode 100644 index 00000000..df3e127c --- /dev/null +++ b/cdk/src/constructs/bedrock-models.ts @@ -0,0 +1,86 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { Node } from 'constructs'; + +/** + * Single source of truth for the Bedrock **foundation-model IDs** the agent + * runtime may invoke. Both grant sites — the AgentCore runtime in + * `stacks/agent.ts` and the ECS task role in `constructs/ecs-agent-cluster.ts` + * — derive their `grantInvoke` / IAM ARNs from this one list, so the two + * backends can never drift (they were previously two hand-synced arrays; #433). + * + * Scoping is intentionally per-model (explicit foundation-model + + * cross-Region inference-profile ARNs), NOT a `Resource: '*'` wildcard — that + * hardening is preserved. Account-level Bedrock model access remains the outer + * gate; this list only controls the IAM grant. + */ +export const DEFAULT_BEDROCK_MODEL_IDS: readonly string[] = [ + 'anthropic.claude-sonnet-4-6', + 'anthropic.claude-opus-4-20250514-v1:0', + 'anthropic.claude-haiku-4-5-20251001-v1:0', +]; + +/** CDK context key whose value (a string array) overrides the model set. */ +export const BEDROCK_MODELS_CONTEXT_KEY = 'bedrockModels'; + +/** + * Resolves the invocable foundation-model IDs: CDK context `bedrockModels` + * (an array of **bare foundation-model IDs**) when provided, else + * {@link DEFAULT_BEDROCK_MODEL_IDS}. Set via `cdk.json` `context` or + * `-c bedrockModels='["anthropic.claude-opus-4-8", …]'`, then redeploy, to add + * a model the runtime may invoke — no construct edits needed. + * + * **Use the bare foundation-model ID (`anthropic.claude-…`), NOT the + * `us.`-prefixed inference-profile ID.** Both grant sites derive the US + * inference-profile ARN by prefixing `us.`, so passing `us.anthropic.…` here + * would produce an invalid `us.us.anthropic.…` ARN. The resolver rejects a + * `us.`/`eu.`/`apac.`-prefixed entry to catch that early. + * + * Throws on a malformed override (non-array, non-string / empty entries, or a + * region-prefixed ID) so a typo fails synth loudly instead of silently + * granting nothing or an invalid ARN. + */ +export function resolveBedrockModelIds(node: Node): readonly string[] { + const override = node.tryGetContext(BEDROCK_MODELS_CONTEXT_KEY); + if (override === undefined || override === null) { + return DEFAULT_BEDROCK_MODEL_IDS; + } + if (!Array.isArray(override) || override.length === 0) { + throw new Error( + `Context '${BEDROCK_MODELS_CONTEXT_KEY}' must be a non-empty array of foundation-model IDs ` + + `(e.g. ["anthropic.claude-sonnet-4-6"]); got ${JSON.stringify(override)}.`, + ); + } + for (const id of override) { + if (typeof id !== 'string' || id.trim().length === 0) { + throw new Error( + `Context '${BEDROCK_MODELS_CONTEXT_KEY}' entries must be non-empty strings; got ${JSON.stringify(id)}.`, + ); + } + if (/^(us|eu|apac)\./.test(id)) { + throw new Error( + `Context '${BEDROCK_MODELS_CONTEXT_KEY}' expects bare foundation-model IDs, not region-prefixed ` + + `inference-profile IDs — got '${id}'. Use '${id.replace(/^(us|eu|apac)\./, '')}'; ` + + 'the US inference-profile ARN is derived automatically.', + ); + } + } + return override as string[]; +} diff --git a/cdk/src/constructs/ecs-agent-cluster.ts b/cdk/src/constructs/ecs-agent-cluster.ts index 1ccbf7c4..4d6d16d1 100644 --- a/cdk/src/constructs/ecs-agent-cluster.ts +++ b/cdk/src/constructs/ecs-agent-cluster.ts @@ -28,6 +28,7 @@ import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; import { NagSuppressions } from 'cdk-nag'; import { Construct } from 'constructs'; import { AgentSessionRole } from './agent-session-role'; +import { resolveBedrockModelIds } from './bedrock-models'; export interface EcsAgentClusterProps { readonly vpc: ec2.IVpc; @@ -50,18 +51,6 @@ export interface EcsAgentClusterProps { readonly agentSessionRole?: AgentSessionRole; } -/** - * Bedrock model IDs the agent may invoke (kept in sync with the AgentCore - * runtime grants in agent.ts). Used to scope the ECS task role's Bedrock - * permissions to explicit foundation-model + inference-profile ARNs instead of - * a `Resource: '*'` wildcard. - */ -const BEDROCK_MODEL_IDS = [ - 'anthropic.claude-sonnet-4-6', - 'anthropic.claude-opus-4-20250514-v1:0', - 'anthropic.claude-haiku-4-5-20251001-v1:0', -]; - /** HTTPS port — the only egress allowed from the agent task ENIs. */ const HTTPS_PORT = 443; @@ -162,10 +151,12 @@ export class EcsAgentCluster extends Construct { // Bedrock model invocation — scoped to explicit foundation-model and // cross-region inference-profile ARNs (parity with the AgentCore runtime - // grants in agent.ts), replacing the prior Resource: '*' wildcard. + // grants in agent.ts), NOT a Resource: '*' wildcard. The model set is the + // shared, context-overridable list (constructs/bedrock-models.ts) so the + // ECS and AgentCore backends can't drift. const stack = Stack.of(this); const bedrockResources: string[] = []; - for (const modelId of BEDROCK_MODEL_IDS) { + for (const modelId of resolveBedrockModelIds(this.node)) { bedrockResources.push( stack.formatArn({ service: 'bedrock', diff --git a/cdk/src/handlers/shared/workflows.ts b/cdk/src/handlers/shared/workflows.ts index ca665531..470b81c0 100644 --- a/cdk/src/handlers/shared/workflows.ts +++ b/cdk/src/handlers/shared/workflows.ts @@ -68,12 +68,18 @@ export interface WorkflowDescriptor { /** * Platform allow-list of Bedrock model ids a workflow may pin via - * `agent_config.model.id` (WORKFLOWS.md rule 13 / §"Model selection"). Mirrors - * the foundation models the agent runtime is granted to invoke (`BEDROCK_MODEL_IDS` - * in `cdk/src/constructs/ecs-agent-cluster.ts`), accepting both the bare id and - * the `us.`-prefixed cross-region inference-profile form the runtime resolves. - * A future Phase 4 will source this from the repo Blueprint; until then it is a - * single platform-wide list checked at admission. + * `agent_config.model.id` (WORKFLOWS.md rule 13 / §"Model selection"). Should + * mirror the foundation models the agent runtime is granted to invoke — the + * shared list in `cdk/src/constructs/bedrock-models.ts` + * (`DEFAULT_BEDROCK_MODEL_IDS` / the `bedrockModels` context, #433) — accepting + * both the bare id and the `us.`-prefixed cross-region inference-profile form. + * + * NOTE (#433 follow-up): this is a SEPARATE, hand-maintained list from the IAM + * grant source. The `repo onboard --model` path is NOT gated by it (repo + * `model_id` isn't validated here), but a custom workflow pinning a model added + * via the `bedrockModels` context would still be rejected at create-task until + * it's added here too. A future Phase 4 will source this from the repo + * Blueprint; consolidating it with `bedrock-models.ts` is tracked separately. */ export const WORKFLOW_MODEL_ALLOWLIST: readonly string[] = [ 'anthropic.claude-sonnet-4-6', diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 192496d3..604f794d 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -35,6 +35,7 @@ import { AgentSessionRole } from '../constructs/agent-session-role'; import { AgentVpc } from '../constructs/agent-vpc'; import { ApprovalMetricsPublisherConsumer } from '../constructs/approval-metrics-publisher-consumer'; import { AttachmentsBucket } from '../constructs/attachments-bucket'; +import { resolveBedrockModelIds } from '../constructs/bedrock-models'; import { Blueprint } from '../constructs/blueprint'; import { CedarWasmLayer } from '../constructs/cedar-wasm-layer'; import { ConcurrencyReconciler } from '../constructs/concurrency-reconciler'; @@ -420,44 +421,24 @@ export class AgentStack extends Stack { applicationLogGroup.grantWrite(runtime); agentMemory.grantReadWrite(runtime); - const model = new bedrock.BedrockFoundationModel('anthropic.claude-sonnet-4-6', { - supportsAgents: true, - supportsCrossRegion: true, - }); - - // Create a cross-region inference profile for Claude Sonnet 4.6 - const inferenceProfile = bedrock.CrossRegionInferenceProfile.fromConfig({ - geoRegion: bedrock.CrossRegionInferenceProfileRegion.US, - model: model, - }); - - const model3 = new bedrock.BedrockFoundationModel('anthropic.claude-opus-4-20250514-v1:0', { - supportsAgents: true, - supportsCrossRegion: true, - }); - - const inferenceProfile3 = bedrock.CrossRegionInferenceProfile.fromConfig({ - geoRegion: bedrock.CrossRegionInferenceProfileRegion.US, - model: model3, - }); - - const model2 = new bedrock.BedrockFoundationModel('anthropic.claude-haiku-4-5-20251001-v1:0', { - supportsAgents: true, - supportsCrossRegion: true, - }); - - // Create a cross-region inference profile for Claude Haiku 4.5 - const inferenceProfile2 = bedrock.CrossRegionInferenceProfile.fromConfig({ - geoRegion: bedrock.CrossRegionInferenceProfileRegion.US, - model: model2, - }); - - model.grantInvoke(runtime); - inferenceProfile.grantInvoke(runtime); - model3.grantInvoke(runtime); - inferenceProfile3.grantInvoke(runtime); - model2.grantInvoke(runtime); - inferenceProfile2.grantInvoke(runtime); + // Grant the runtime invoke on each configured foundation model + its + // US cross-Region inference profile. The model set is a single source of + // truth (constructs/bedrock-models.ts), shared with the ECS task role, and + // overridable via the `bedrockModels` CDK context — add a model by config, + // no construct edits. Scoping stays per-model (no Resource:'*'); account- + // level Bedrock access remains the outer gate. + for (const modelId of resolveBedrockModelIds(this.node)) { + const foundationModel = new bedrock.BedrockFoundationModel(modelId, { + supportsAgents: true, + supportsCrossRegion: true, + }); + const crossRegionProfile = bedrock.CrossRegionInferenceProfile.fromConfig({ + geoRegion: bedrock.CrossRegionInferenceProfileRegion.US, + model: foundationModel, + }); + foundationModel.grantInvoke(runtime); + crossRegionProfile.grantInvoke(runtime); + } // --- Per-task SessionRole (#209) --- // Holds the tenant-data grants (the four task_id-partitioned tables, plus diff --git a/cdk/test/constructs/bedrock-models.test.ts b/cdk/test/constructs/bedrock-models.test.ts new file mode 100644 index 00000000..e39ff504 --- /dev/null +++ b/cdk/test/constructs/bedrock-models.test.ts @@ -0,0 +1,72 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { App, Stack } from 'aws-cdk-lib'; +import { + BEDROCK_MODELS_CONTEXT_KEY, + DEFAULT_BEDROCK_MODEL_IDS, + resolveBedrockModelIds, +} from '../../src/constructs/bedrock-models'; + +function nodeWithContext(context?: Record) { + const app = new App({ context }); + return new Stack(app, 'TestStack').node; +} + +describe('resolveBedrockModelIds', () => { + it('returns the default set when no context override is present', () => { + const ids = resolveBedrockModelIds(nodeWithContext()); + expect(ids).toEqual(DEFAULT_BEDROCK_MODEL_IDS); + }); + + it('returns the context override when provided', () => { + const override = ['anthropic.claude-opus-4-8', 'anthropic.claude-sonnet-4-6']; + const ids = resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: override })); + expect(ids).toEqual(override); + }); + + it('throws on a non-array override (typo guard)', () => { + expect(() => + resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: 'anthropic.claude-opus-4-8' })), + ).toThrow(/must be a non-empty array/); + }); + + it('throws on an empty-array override', () => { + expect(() => + resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: [] })), + ).toThrow(/must be a non-empty array/); + }); + + it('throws on a non-string / empty entry', () => { + expect(() => + resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: ['anthropic.claude-sonnet-4-6', ''] })), + ).toThrow(/non-empty strings/); + }); + + it('throws on a region-prefixed (us./eu./apac.) inference-profile ID', () => { + // Guards the us.us.… double-prefix footgun: both grant sites derive the + // inference-profile ARN by prefixing `us.`, so the context wants the bare id. + expect(() => + resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: ['us.anthropic.claude-opus-4-8'] })), + ).toThrow(/bare foundation-model IDs/); + expect(() => + resolveBedrockModelIds(nodeWithContext({ [BEDROCK_MODELS_CONTEXT_KEY]: ['eu.anthropic.claude-sonnet-4-6'] })), + ).toThrow(/bare foundation-model IDs/); + }); +}); diff --git a/cdk/test/constructs/ecs-agent-cluster.test.ts b/cdk/test/constructs/ecs-agent-cluster.test.ts index a6164a56..e190aa8f 100644 --- a/cdk/test/constructs/ecs-agent-cluster.test.ts +++ b/cdk/test/constructs/ecs-agent-cluster.test.ts @@ -29,8 +29,10 @@ import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; import { AgentSessionRole } from '../../src/constructs/agent-session-role'; import { EcsAgentCluster } from '../../src/constructs/ecs-agent-cluster'; -function createStack(overrides?: { memoryId?: string }): { stack: Stack; template: Template } { - const app = new App(); +function createStack(overrides?: { memoryId?: string; bedrockModels?: string[] }): { stack: Stack; template: Template } { + const app = new App({ + context: overrides?.bedrockModels ? { bedrockModels: overrides.bedrockModels } : undefined, + }); const stack = new Stack(app, 'TestStack'); const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 }); @@ -174,6 +176,29 @@ describe('EcsAgentCluster construct', () => { expect(serialized).toContain('anthropic.claude-haiku-4-5-20251001-v1:0'); }); + test('bedrockModels context override changes the granted model ARNs (#433)', () => { + const template = createStack({ bedrockModels: ['anthropic.claude-opus-4-8'] }).template; + const policies = template.findResources('AWS::IAM::Policy'); + let bedrockStatement: { Resource: unknown } | undefined; + for (const policy of Object.values(policies)) { + for (const s of policy.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + if (actions.includes('bedrock:InvokeModel')) { + bedrockStatement = s; + } + } + } + expect(bedrockStatement).toBeDefined(); + const serialized = JSON.stringify(bedrockStatement!.Resource); + // The override model is granted... + expect(serialized).toContain('foundation-model/anthropic.claude-opus-4-8'); + expect(serialized).toContain('inference-profile/us.anthropic.claude-opus-4-8'); + // ...and the defaults are NOT (the override replaces, not appends). + expect(serialized).not.toContain('claude-sonnet-4-6'); + // Still scoped, never a wildcard. + expect(bedrockStatement!.Resource).not.toEqual('*'); + }); + test('container has required environment variables', () => { baseTemplate.hasResourceProperties('AWS::ECS::TaskDefinition', { ContainerDefinitions: Match.arrayWith([ diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 22a295fd..486b4ad9 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -230,6 +230,50 @@ describe('AgentStack', () => { expect(serialized).toMatch(/"Fn::GetAtt":\["Runtime[0-9A-F]+","AgentRuntimeArn"\]/); }); + test('runtime is granted the default Bedrock model set (#433)', () => { + // Default (no bedrockModels context): the runtime execution role must hold + // bedrock:InvokeModel on the three default foundation models + their US + // inference profiles, scoped (never Resource: '*'). + const serialized = JSON.stringify(template.findResources('AWS::IAM::Policy')); + expect(serialized).toContain('foundation-model/anthropic.claude-sonnet-4-6'); + expect(serialized).toContain('inference-profile/us.anthropic.claude-sonnet-4-6'); + expect(serialized).toContain('anthropic.claude-opus-4-20250514-v1:0'); + expect(serialized).toContain('anthropic.claude-haiku-4-5-20251001-v1:0'); + }); + + test('bedrockModels context override propagates to the runtime execution role (#433)', () => { + // The other half of #433's acceptance criteria (the ECS side is covered in + // ecs-agent-cluster.test.ts): a context override must replace the runtime's + // granted models too — overridden model present, defaults absent, still scoped. + const app = new App({ context: { bedrockModels: ['anthropic.claude-opus-4-8'] } }); + const stack = new AgentStack(app, 'OverrideAgentStack', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + const overridden = Template.fromStack(stack); + + // Collect every bedrock:InvokeModel statement's Resource across IAM policies. + const policies = overridden.findResources('AWS::IAM::Policy'); + const bedrockResources: unknown[] = []; + for (const p of Object.values(policies)) { + for (const s of (p.Properties?.PolicyDocument?.Statement ?? []) as Array<{ Action?: string | string[]; Resource?: unknown }>) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + if (actions.some((a) => typeof a === 'string' && a.startsWith('bedrock:InvokeModel'))) { + bedrockResources.push(s.Resource); + } + } + } + const serialized = JSON.stringify(bedrockResources); + expect(bedrockResources.length).toBeGreaterThan(0); + // Overridden model is granted... + expect(serialized).toContain('foundation-model/anthropic.claude-opus-4-8'); + expect(serialized).toContain('inference-profile/us.anthropic.claude-opus-4-8'); + // ...defaults are NOT (override replaces, not appends)... + expect(serialized).not.toContain('claude-sonnet-4-6'); + expect(serialized).not.toContain('claude-haiku-4-5'); + // ...and the grant is never a bare wildcard. + expect(serialized).not.toContain('"*"'); + }); + test('outputs ApiUrl', () => { template.hasOutput('ApiUrl', { Description: 'URL of the Task API',