aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

feat(security): scope payload-bucket reads to the task's own key (ecs + lambda-microvm)

Open
#700 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

### Context

Both compute payload buckets grant the compute role bucket-wide read via CDK's `grantRead`, which renders `s3:GetObject*`, `s3:GetBucket*` and `s3:List*` across the whole bucket:

- ECS payload bucket (`cdk/src/constructs/ecs-agent-cluster.ts`) — `grantRead(taskRole)` (introduced with the S3-pointer payload path, #502)
- `cdk/src/constructs/lambda-microvm-compute.ts` — `payloadBucket.grantRead(executionRole)` (ADR-021 P1, same shape by design/precedent)

Raised in review of PR #689 (ADR-021 P1). Pre-existing on the ECS side, so not a MicroVM regression — but the 4 KB `runHookPayload` cap makes the MicroVM payload bucket a hot path (virtually every task delivers its payload through it), which raises the value of tightening it.

### Why it matters

The compute role runs **untrusted repository code**. Payload objects are keyed `/payload.json`, and a hydrated payload contains the prompt, issue thread and repo context. Bucket-wide read means task A's container can read task B's hydrated payload, and `s3:List*` lets it enumerate every recent task id — a cross-tenant read plus an inventory primitive, on the least-trusted role in the platform. Write and delete are correctly withheld; read is the remaining gap.

### Proposal

1. **Drop `s3:List*`.** Nothing in either agent lists the bucket — both are handed an exact URI. No-behavior-change tightening; removes the enumeration primitive on its own.
2. **Scope `s3:GetObject` to the task's own prefix.** Preference order:
- (a) `aws:PrincipalTag/task_id` condition on the object ARN, reusing the per-task SessionRole tagging `AgentSessionRole.admitComputeRole` already establishes (#209) — the artifacts bucket already uses this exact pattern (`artifacts/${aws:PrincipalTag/task_id}/*`);
- (b) if the payload read happens before SessionRole assumption (it does today: read once at boot on the task/execution role), either move the read after assumption or hand the container a presigned URL minted by the orchestrator (which already has write access).
3. **Apply to BOTH backends in one change** and extend the existing "read-only, nothing mutating" assertions in both construct test suites to also assert the absence of `s3:List*` and the presence of the task-scoping condition.

### Notes / open questions

- Option (b) changes the boot sequence and is the larger change; (a) needs confirmation the payload read can move behind SessionRole assumption without breaking the "read payload first" boot ordering both paths rely on.
- The existing cdk-nag `AwsSolutions-IAM5` suppressions on both constructs cite "CDK grantRead on the dedicated payload bucket" — those reasons must be narrowed as part of this work.
- Not a P1 blocker for ADR-021: exposure equals the shipped ECS backend, and the payload TTL is 1 day.

Refs #645, #502, PR #689 (review suggestion 3)

Contributor guide

Open the contributing guide

Research direction

Start with cdk/src/constructs/ecs-agent-cluster.ts and cdk/src/constructs/lambda-microvm-compute.ts, then inspect the existing construct test suites and the artifacts bucket task-scoping pattern. Confirm whether payload reads can occur after SessionRole assumption without breaking boot ordering, and compare the two implementation options. Done means both backends omit s3:List*, scope reads to the task key, retain read-only permissions, and have narrowed cdk-nag suppressions with updated tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.