fix(ecs): drop over-privileged artifacts grant from ECS task role (mirror #596 B1)

Mirrors the #596 review B1 fix onto linear-vercel (this defect rode in via the
main→lv merge a9efaa6, which kept lv's version of the grant). coding/decompose-v1
delivers its plan via the assumed SessionRole (scoped to artifacts/${task_id}/*),
exactly like the AgentCore runtime whose task role has no direct artifacts grant.
The whole-bucket grantReadWrite over-privileged the untrusted-code role and broke
cross-task isolation. Task role keeps only the ARTIFACTS_BUCKET_NAME env; corrected
the inverted comment + the stale artifacts clause in the cdk-nag IAM5 reason. Test
flipped to assert the task role has NO s3:Put*/Delete* action. Full cdk build green.

Author: isadeks

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

@@ -353,13 +353,16 @@ export class EcsAgentCluster extends Construct {
       props.payloadBucket.grantRead(taskRole);
     }
 
-    // #299 ECS-parity: an artifact workflow (coding/decompose-v1) WRITES its plan
-    // to the artifacts bucket via deliver_artifact, so grant read+write (the
-    // AgentCore runtime's SessionRole/exec-role has the equivalent). Scoped to
-    // this bucket. Stays on the task role — delivery is a terminal step.
-    if (props.artifactsBucket) {
-      props.artifactsBucket.grantReadWrite(taskRole);
-    }
+    // #299 ECS-parity: coding/decompose-v1 delivers its plan to the artifacts
+    // bucket via deliver_artifact — but the write goes through the assumed
+    // SessionRole (deliverers.py -> tenant_client), scoped to
+    // artifacts/${task_id}/*, exactly like the AgentCore runtime (whose task
+    // role likewise has NO direct artifacts grant). So the task role needs only
+    // the ARTIFACTS_BUCKET_NAME env (set above), not a bucket grant. Granting
+    // whole-bucket read+write here would over-privilege the untrusted-code role
+    // and break cross-task isolation (a task could read/clobber other tasks'
+    // artifacts/<other_id>/, traces/, attachments/ on the same bucket). (#596 review B1)
+    // (no props.artifactsBucket grant — intentional; see comment)
 
     // F-2 ECS-parity: grant the task role read+write on the cross-task AgentCore
     // Memory. MEMORY_ID in the container env makes the agent ATTEMPT episodic +
@@ -467,7 +470,7 @@ export class EcsAgentCluster extends Construct {
     NagSuppressions.addResourceSuppressions(taskRole, [
       {
         id: 'AwsSolutions-IAM5',
-        reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead (GitHub token) and the bgagent-linear-oauth-*/bgagent-jira-oauth-* prefix grant (ABCA-488 — per-workspace channel OAuth tokens are created by the CLI at setup, name unknown at synth, GetSecretValue only); CloudWatch Logs wildcards from CDK grantWrite; S3 object/* wildcard from CDK grantRead on the ECS payload bucket (read-only, scoped to that bucket — #502) and from grantReadWrite on the artifacts bucket (scoped to that bucket — coding/decompose-v1 delivers its plan artifact there, #299). Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard resource). ec2:DescribeAvailabilityZones requires Resource:* (EC2 describe actions have no resource-level scoping) — read-only, no mutation/data access; needed so a CDK target repo\'s `cdk synth` build gate can resolve AZ context on a fresh clone (ECS-parity, no cdk.context.json cache in the container).',
+        reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead (GitHub token) and the bgagent-linear-oauth-*/bgagent-jira-oauth-* prefix grant (ABCA-488 — per-workspace channel OAuth tokens are created by the CLI at setup, name unknown at synth, GetSecretValue only); 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). ec2:DescribeAvailabilityZones requires Resource:* (EC2 describe actions have no resource-level scoping) — read-only, no mutation/data access; needed so a CDK target repo\'s `cdk synth` build gate can resolve AZ context on a fresh clone (ECS-parity, no cdk.context.json cache in the container).',
       },
       {
         id: 'AwsSolutions-ECS2',

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

@@ -634,20 +634,26 @@ describe('EcsAgentCluster artifacts bucket (#299 ECS-parity)', () => {
     });
   });
 
-  test('grants the task role READ + WRITE on the artifacts bucket (it delivers the plan artifact)', () => {
+  test('does NOT grant the task role write on the artifacts bucket (the scoped SessionRole owns delivery)', () => {
+    // #596 review B1: coding/decompose-v1 delivers via the assumed SessionRole
+    // (scoped to artifacts/${task_id}/*), exactly like the AgentCore runtime —
+    // whose task role likewise has no direct artifacts grant. A whole-bucket
+    // grantReadWrite here would over-privilege the untrusted-code role and break
+    // cross-task isolation. The task role gets only the ARTIFACTS_BUCKET_NAME env.
     const template = createWithArtifactsBucket();
     const policies = template.findResources('AWS::IAM::Policy');
-    const s3Actions = new Set<string>();
+    const s3WriteActions = new Set<string>();
     for (const policy of Object.values(policies)) {
       for (const stmt of policy.Properties.PolicyDocument.Statement) {
         const actions = Array.isArray(stmt.Action) ? stmt.Action : [stmt.Action];
         for (const a of actions) {
-          if (typeof a === 'string' && a.startsWith('s3:')) s3Actions.add(a);
+          // Only true S3 mutations — Put*/Delete*. The read-only payload bucket
+          // (#502) legitimately grants GetObject*/List* on the task role.
+          if (typeof a === 'string' && /^s3:(Put|Delete)/.test(a)) s3WriteActions.add(a);
         }
       }
     }
-    expect([...s3Actions].some(a => a === 's3:GetObject' || a === 's3:GetObject*')).toBe(true);
-    expect([...s3Actions].some(a => a === 's3:PutObject' || a === 's3:PutObject*')).toBe(true);
+    expect([...s3WriteActions]).toEqual([]);
   });
 
   test('omits ARTIFACTS_BUCKET_NAME when no artifacts bucket is provided', () => {