fix(ecs): address #596 review — drop over-privileged artifacts grant + nits

B1 (least-privilege regression): remove props.artifactsBucket.grantReadWrite
from the ECS task role. coding/decompose-v1 delivers its plan via 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). The whole-bucket grant over-privileged the untrusted-code
role and broke cross-task isolation (a task could read/clobber other tasks'
artifacts/<other_id>/, traces/, attachments/). Task role keeps only the
ARTIFACTS_BUCKET_NAME env. Correct the inverted 'parity' comment and drop the
now-stale artifacts clause from the cdk-nag IAM5 reason.

Test: flip 'grants READ+WRITE on artifacts' → asserts the task role has NO S3
Put/Delete action at all (the read-only #502 payload GetObject*/List* grant is
not flagged).

N4: add lint_command to _KNOWN_ORCHESTRATOR_KEYS (sibling of build_command) so a
future 'wired build_command, forgot lint_command' contract gap WARNs instead of
dropping silently.

(N1/N2/N3 comment nits were already addressed on the branch head; B1 dead-const
ECS_PAYLOAD_OBJECT_KEY_PREFIX no longer present. B2 governance handled separately.)

Author: isadeks

agent/src/pipeline.py

@@ -1303,6 +1303,11 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
 _KNOWN_ORCHESTRATOR_KEYS = frozenset(
     {
         "build_command",
+        # ``lint_command``'s sibling: neither is a run_task param today (the build/
+        # lint commands are consumed via repo config, not passed through here), but
+        # listing both makes a future "wired build_command, forgot lint_command"
+        # contract gap WARN instead of drop silently (N4).
+        "lint_command",
         "merge_branches",
         "base_branch",
         "github_token_secret_arn",

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

@@ -233,13 +233,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).
+    // (no props.artifactsBucket grant — intentional; see comment)
 
     // F-2 (ABCA-488-class parity): grant the task role read+write on the
     // AgentCore Memory so the agent's cross-task learning writes
@@ -341,7 +344,7 @@ export class EcsAgentCluster extends Construct {
     NagSuppressions.addResourceSuppressions(this.taskDefinition, [
       {
         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

@@ -557,20 +557,27 @@ 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, so those
+          // are NOT flagged; what must be absent is any write to any S3 bucket.
+          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', () => {