Skip to content

Commit c3f2b6f

Browse files
author
bgagent
committed
fix(ecs): write task payload to S3, not inline overrides (#502)
The ECS compute strategy inlined the full orchestrator payload (incl. the large hydrated_context) into the AGENT_PAYLOAD container-override env var. ECS RunTask caps the TOTAL containerOverrides blob at 8192 bytes, so any real task was rejected before the container started: InvalidParameterException: Container Overrides length must be at most 8192 AgentCore is unaffected — it passes the payload in the InvokeAgentRuntime request body, which has no comparable limit. The bug only surfaces with a realistic hydrated payload, which is why the prior ECS smoke test (a small Rust cargo-check, #494) didn't catch it. Fix — stash the payload out-of-band and pass only a pointer: - New EcsPayloadBucket construct (mirrors TraceArtifactsBucket): BLOCK_ALL, enforceSSL, S3_MANAGED encryption, 1-day lifecycle TTL (payloads are ephemeral — read once at boot). Dedicated bucket so the ECS task role's S3 read is scoped to payloads only and can't touch attachments/traces. - ecs-strategy: when ECS_PAYLOAD_BUCKET is set, PutObject the payload to <task_id>/payload.json and pass AGENT_PAYLOAD_S3_URI in the override; the boot command fetches+parses it via boto3. Inline AGENT_PAYLOAD remains as a fallback (small payloads / no bucket), so nothing regresses. deleteEcsPayload helper removes the object. - orchestrate-task finalize: best-effort deleteEcsPayload for ECS tasks once terminal (the container has long since read it); lifecycle rule is the crash backstop. - EcsAgentCluster: accept payloadBucket, inject ECS_PAYLOAD_BUCKET env, grant the task role READ ONLY (untrusted repo code must not write/delete payloads; the trusted orchestrator owns write+delete). Session-role-aware. - task-orchestrator: ecsPayloadBucket prop → grantPut + grantDelete to the orchestrator; @aws-sdk/client-s3 added to bundling externals. - agent.ts: updated the commented uncomment-to-enable ECS scaffolding to wire the payload bucket. Tests: new bucket construct (TTL/SSL/block-public/autoDelete); strategy S3-write + URI-pointer + inline fallback + deleteEcsPayload (incl. best-effort swallow + no-op without bucket); cluster read-grant + env var + read-only (no put/delete). Full build green. Closes #502
1 parent 338615f commit c3f2b6f

9 files changed

Lines changed: 617 additions & 9 deletions

File tree

cdk/src/constructs/ecs-agent-cluster.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets';
2424
import * as ecs from 'aws-cdk-lib/aws-ecs';
2525
import * as iam from 'aws-cdk-lib/aws-iam';
2626
import * as logs from 'aws-cdk-lib/aws-logs';
27+
import * as s3 from 'aws-cdk-lib/aws-s3';
2728
import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager';
2829
import { NagSuppressions } from 'cdk-nag';
2930
import { Construct } from 'constructs';
@@ -38,6 +39,18 @@ export interface EcsAgentClusterProps {
3839
readonly githubTokenSecret: secretsmanager.ISecret;
3940
readonly memoryId?: string;
4041

42+
/**
43+
* S3 bucket holding per-task ECS payloads (#502). The orchestrator writes the
44+
* payload (incl. the large hydrated_context, which can't fit in the 8 KB
45+
* RunTask containerOverrides limit) here and passes only an
46+
* `AGENT_PAYLOAD_S3_URI` pointer; the container fetches it on boot. The task
47+
* role gets **read-only** on this bucket — the container runs untrusted repo
48+
* code, so it must not be able to delete payloads (the trusted orchestrator
49+
* owns write + delete). When omitted (isolated construct tests / deployments
50+
* that still pass the payload inline), no grant or env var is added.
51+
*/
52+
readonly payloadBucket?: s3.IBucket;
53+
4154
/**
4255
* Per-task SessionRole (#209). When provided, tenant-data DynamoDB access
4356
* (task/events tables) is NOT granted to the Fargate task role; instead the
@@ -132,6 +145,10 @@ export class EcsAgentCluster extends Construct {
132145
LOG_GROUP_NAME: logGroup.logGroupName,
133146
GITHUB_TOKEN_SECRET_ARN: props.githubTokenSecret.secretArn,
134147
...(props.memoryId && { MEMORY_ID: props.memoryId }),
148+
// #502: the payload bucket name so the orchestrator-issued
149+
// AGENT_PAYLOAD_S3_URI can be fetched. (The orchestrator sets the URI
150+
// per-task via container override; this is informational parity.)
151+
...(props.payloadBucket && { ECS_PAYLOAD_BUCKET: props.payloadBucket.bucketName }),
135152
// Per-session IAM scoping (#209): when a SessionRole is wired, the
136153
// agent assumes it for tenant-data access (see aws_session.py).
137154
...(props.agentSessionRole && {
@@ -160,6 +177,15 @@ export class EcsAgentCluster extends Construct {
160177
// agent assumes the SessionRole — stays on the task role).
161178
props.githubTokenSecret.grantRead(taskRole);
162179

180+
// #502: read-only on the ECS payload bucket so the container can fetch its
181+
// payload (AGENT_PAYLOAD_S3_URI) at boot. READ only — the container runs
182+
// untrusted repo code, so it must not be able to write or delete payloads
183+
// (the trusted orchestrator owns write + delete). Stays on the task role
184+
// (read once at startup, before the agent assumes any SessionRole).
185+
if (props.payloadBucket) {
186+
props.payloadBucket.grantRead(taskRole);
187+
}
188+
163189
// Bedrock model invocation — scoped to explicit foundation-model and
164190
// cross-region inference-profile ARNs (parity with the AgentCore runtime
165191
// grants in agent.ts), replacing the prior Resource: '*' wildcard.
@@ -201,7 +227,7 @@ export class EcsAgentCluster extends Construct {
201227
NagSuppressions.addResourceSuppressions(this.taskDefinition, [
202228
{
203229
id: 'AwsSolutions-IAM5',
204-
reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead; CloudWatch Logs wildcards from CDK grantWrite. Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard resource).',
230+
reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead; CloudWatch Logs wildcards from CDK grantWrite; S3 object/* wildcard from CDK grantRead on the ECS payload bucket (read-only, scoped to that bucket — #502). Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard resource).',
205231
},
206232
{
207233
id: 'AwsSolutions-ECS2',
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* MIT No Attribution
3+
*
4+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
7+
* the Software without restriction, including without limitation the rights to
8+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
9+
* the Software, and to permit persons to whom the Software is furnished to do so.
10+
*
11+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
12+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
13+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
14+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
15+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
16+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
17+
* SOFTWARE.
18+
*/
19+
20+
import { Duration, RemovalPolicy } from 'aws-cdk-lib';
21+
import * as s3 from 'aws-cdk-lib/aws-s3';
22+
import { Construct } from 'constructs';
23+
24+
/**
25+
* Lifecycle expiry for ECS task payloads. The payload is consumed once, at
26+
* container boot, and the orchestrator deletes it promptly in the ``finalize``
27+
* step. This 1-day rule is only a crash backstop: if the orchestrator dies
28+
* before finalize (rare — it runs under durable execution), the object is still
29+
* reaped within a day instead of lingering. Payloads carry the hydrated prompt
30+
* context, so a tight TTL also keeps the blast radius of an accidental
31+
* permission leak small.
32+
*/
33+
export const ECS_PAYLOAD_TTL_DAYS = 1;
34+
35+
/**
36+
* Object-key prefix for ECS task payloads. Key layout:
37+
* ``<task_id>/payload.json``. Each task writes a single object under its own
38+
* task-id prefix; the orchestrator deletes it on terminal.
39+
*/
40+
export const ECS_PAYLOAD_OBJECT_KEY_PREFIX = '';
41+
42+
/**
43+
* Properties for the EcsPayloadBucket construct.
44+
*/
45+
export interface EcsPayloadBucketProps {
46+
/**
47+
* Removal policy for the bucket.
48+
* @default RemovalPolicy.DESTROY
49+
*/
50+
readonly removalPolicy?: RemovalPolicy;
51+
52+
/**
53+
* Whether to auto-delete objects when the bucket is removed (so ``cdk
54+
* destroy`` does not need a manual bucket-empty first). Mirrors
55+
* ``TraceArtifactsBucket`` / ``AttachmentsBucket``. Deploys CDK's
56+
* ``Custom::S3AutoDeleteObjects`` Lambda with delete permissions on this
57+
* bucket — acceptable here because the contents are ephemeral throwaway
58+
* payloads.
59+
* @default true
60+
*/
61+
readonly autoDeleteObjects?: boolean;
62+
}
63+
64+
/**
65+
* S3 bucket for ECS task payloads (#502).
66+
*
67+
* The ECS compute strategy cannot pass the orchestrator payload (repo URL,
68+
* prompt, and the large ``hydrated_context``) inline: a Fargate ``RunTask``
69+
* caps the entire ``containerOverrides`` blob at 8192 bytes, and the hydrated
70+
* context routinely exceeds that, so the call is rejected with
71+
* ``InvalidParameterException``. (AgentCore is unaffected — it passes the
72+
* payload in the ``InvokeAgentRuntime`` request body, which has no comparable
73+
* limit.) Instead, the orchestrator writes the payload to
74+
* ``s3://<bucket>/<task_id>/payload.json`` and passes only a small
75+
* ``AGENT_PAYLOAD_S3_URI`` pointer in the override; the container fetches and
76+
* parses it on boot.
77+
*
78+
* Dedicated (not co-tenant with attachments/traces) so the boundary is
79+
* structural: the ECS task role gets S3 **read** here and nowhere else, the
80+
* attachments feature can never collide with payload keys, and the tight
81+
* 1-day TTL is whole-bucket rather than a prefix-scoped rule grafted onto a
82+
* shared bucket.
83+
*
84+
* Security / hygiene (parity with TraceArtifactsBucket):
85+
* - ``blockPublicAccess: BLOCK_ALL`` + ``enforceSSL: true`` — no public read,
86+
* TLS-only transport.
87+
* - ``encryption: S3_MANAGED`` — server-side encryption at rest.
88+
* - 1-day lifecycle expiry — payloads are ephemeral (read once at boot,
89+
* deleted by the orchestrator at finalize); this is the crash backstop.
90+
*/
91+
export class EcsPayloadBucket extends Construct {
92+
/** The underlying S3 bucket. */
93+
public readonly bucket: s3.Bucket;
94+
95+
constructor(scope: Construct, id: string, props: EcsPayloadBucketProps = {}) {
96+
super(scope, id);
97+
98+
this.bucket = new s3.Bucket(this, 'Bucket', {
99+
blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
100+
encryption: s3.BucketEncryption.S3_MANAGED,
101+
enforceSSL: true,
102+
lifecycleRules: [
103+
{
104+
id: 'ecs-payload-ttl',
105+
enabled: true,
106+
expiration: Duration.days(ECS_PAYLOAD_TTL_DAYS),
107+
// Reap incomplete multipart uploads after 1 day. Object expiration
108+
// does not apply to in-flight MPUs (they are not objects yet), so a
109+
// separate reaper keeps stale upload parts from lingering.
110+
abortIncompleteMultipartUploadAfter: Duration.days(1),
111+
},
112+
],
113+
removalPolicy: props.removalPolicy ?? RemovalPolicy.DESTROY,
114+
autoDeleteObjects: props.autoDeleteObjects ?? true,
115+
});
116+
}
117+
}

cdk/src/constructs/task-orchestrator.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,16 @@ export interface TaskOrchestratorProps {
159159
readonly executionRoleArn: string;
160160
};
161161

162+
/**
163+
* S3 bucket for per-task ECS payloads (#502). When provided (alongside
164+
* ``ecsConfig``), the orchestrator writes the payload here and passes only an
165+
* ``AGENT_PAYLOAD_S3_URI`` pointer in the RunTask override (the full payload
166+
* exceeds the 8 KB containerOverrides limit), then deletes the object in the
167+
* finalize step. The orchestrator gets write + delete; the ECS task role gets
168+
* read-only (granted on the bucket by ``EcsAgentCluster``).
169+
*/
170+
readonly ecsPayloadBucket?: s3.IBucket;
171+
162172
/**
163173
* S3 bucket for task attachments. When provided, the orchestrator gets
164174
* ReadWrite grants for URL fetch/screen/upload during hydration.
@@ -220,6 +230,7 @@ export class TaskOrchestrator extends Construct {
220230
'@aws-sdk/client-ecs',
221231
'@aws-sdk/client-lambda',
222232
'@aws-sdk/client-bedrock-runtime',
233+
'@aws-sdk/client-s3',
223234
'@aws-sdk/client-secrets-manager',
224235
'@aws-sdk/lib-dynamodb',
225236
'@aws-sdk/util-dynamodb',
@@ -262,6 +273,9 @@ export class TaskOrchestrator extends Construct {
262273
ECS_SECURITY_GROUP: props.ecsConfig.securityGroup,
263274
ECS_CONTAINER_NAME: props.ecsConfig.containerName,
264275
}),
276+
// #502: bucket the orchestrator writes the ECS payload to (and deletes
277+
// from at finalize); the ECS strategy reads this to build the S3 URI.
278+
...(props.ecsPayloadBucket && { ECS_PAYLOAD_BUCKET: props.ecsPayloadBucket.bucketName }),
265279
...(props.attachmentsBucket && { ATTACHMENTS_BUCKET_NAME: props.attachmentsBucket.bucketName }),
266280
},
267281
bundling: orchestratorBundling,
@@ -280,6 +294,15 @@ export class TaskOrchestrator extends Construct {
280294
props.attachmentsBucket.grantReadWrite(this.fn);
281295
}
282296

297+
// #502: ECS payload bucket — the orchestrator writes the payload before
298+
// RunTask and deletes it at finalize. Write + delete only (it never reads
299+
// its own payload back; the ECS container is the reader, with its own
300+
// read-only grant from EcsAgentCluster).
301+
if (props.ecsPayloadBucket) {
302+
props.ecsPayloadBucket.grantPut(this.fn);
303+
props.ecsPayloadBucket.grantDelete(this.fn);
304+
}
305+
283306
// Durable execution managed policy
284307
this.fn.role!.addManagedPolicy(
285308
iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicDurableExecutionRolePolicy'),

cdk/src/handlers/orchestrate-task.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
type PollState,
3737
} from './shared/orchestrator';
3838
import { runPreflightChecks } from './shared/preflight';
39+
import { deleteEcsPayload } from './shared/strategies/ecs-strategy';
3940
import type { TaskRecord } from './shared/types';
4041
import { workflowIsReadOnly, workflowRequiresRepo } from './shared/workflows';
4142

@@ -297,6 +298,14 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
297298
// Step 6: Finalize — update terminal status, emit events, release concurrency
298299
await context.step('finalize', async () => {
299300
await finalizeTask(taskId, finalPollState, task.user_id);
301+
// #502: the task is terminal — the container has long since read its
302+
// payload, so delete the ephemeral S3 payload object now. Best-effort
303+
// (deleteEcsPayload swallows errors) and a no-op for AgentCore tasks /
304+
// deployments without a payload bucket; the bucket's 1-day lifecycle rule
305+
// is the backstop if this delete or the whole step never runs.
306+
if (blueprintConfig.compute_type === 'ecs') {
307+
await deleteEcsPayload(taskId);
308+
}
300309
});
301310
};
302311

0 commit comments

Comments
 (0)