Add R2 ECS CloudFormation template#268
Conversation
|
| Filename | Overview |
|---|---|
| cloudformation/retool-r2.ec2.yaml | New self-contained ECS-on-EC2 CloudFormation template for the Retool R2 agent sandbox stack; contains an overly-permissive SQS wildcard IAM policy, an RDS security group open to 0.0.0.0/0, JWT/encryption secrets injected as plaintext ECS env vars, a hardcoded expired license key placeholder, and a description typo on MinimumHealthyPercent. |
| cloudformation/gvisor-seccomp.json | New seccomp profile for gVisor (runsc --rootless) extending the Docker default allowlist with namespace, mount, pivot_root, and ptrace syscalls required by the systrap platform. |
| cloudformation/nsjail-seccomp.json | New seccomp profile for nsjail extending the Docker default allowlist with syscalls required for namespace isolation and serverless function sandboxing. |
| cloudformation/README.md | Documentation added for the new retool-r2.ec2.yaml template covering architecture, prerequisites, parameter reference, browser routing, and deploy/validate commands. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
ALB["ALB (HTTP :80)"] --> Retool["Retool Backend\n(awsvpc, RetoolSG)"]
Retool --> WFBackend["Workflows Backend\n(awsvpc)"]
Retool --> Controller["Agent Sandbox Controller\n(awsvpc :3018)"]
Retool --> Proxy["Agent Sandbox Proxy\n(awsvpc :3019)"]
Retool --> Temporal["Temporal Frontend\n(awsvpc :7233)"]
Retool --> JsExec["js-executor\n(awsvpc :3000, nsjail)"]
Controller -->|"ECS RunTask (bridge)"| SandboxPool["Sandbox EC2 Pool\n(retool.sandbox=true)"]
SandboxPool --> Sandbox["agent-executor Container\n(bridge :3017 dynamic hostPort\ngVisor runsc --rootless)"]
Proxy -->|"instanceIP:hostPort"| Sandbox
Retool --> RDS[("RDS PostgreSQL\n(hammerhead_production)")]
Controller --> RDS
Proxy --> RDS
Sandbox --> S3["RrSnapshotsBucket (S3)"]
Retool --> S3
Retool --> Git["RrGitBucket (S3)"]
EventBridge["EventBridge (optional)"] -->|"ECS task-state"| SQS["SQS SandboxTaskEventsQueue"]
SQS --> Controller
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
ALB["ALB (HTTP :80)"] --> Retool["Retool Backend\n(awsvpc, RetoolSG)"]
Retool --> WFBackend["Workflows Backend\n(awsvpc)"]
Retool --> Controller["Agent Sandbox Controller\n(awsvpc :3018)"]
Retool --> Proxy["Agent Sandbox Proxy\n(awsvpc :3019)"]
Retool --> Temporal["Temporal Frontend\n(awsvpc :7233)"]
Retool --> JsExec["js-executor\n(awsvpc :3000, nsjail)"]
Controller -->|"ECS RunTask (bridge)"| SandboxPool["Sandbox EC2 Pool\n(retool.sandbox=true)"]
SandboxPool --> Sandbox["agent-executor Container\n(bridge :3017 dynamic hostPort\ngVisor runsc --rootless)"]
Proxy -->|"instanceIP:hostPort"| Sandbox
Retool --> RDS[("RDS PostgreSQL\n(hammerhead_production)")]
Controller --> RDS
Proxy --> RDS
Sandbox --> S3["RrSnapshotsBucket (S3)"]
Retool --> S3
Retool --> Git["RrGitBucket (S3)"]
EventBridge["EventBridge (optional)"] -->|"ECS task-state"| SQS["SQS SandboxTaskEventsQueue"]
SQS --> Controller
Prompt To Fix All With AI
Fix the following 5 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 5
cloudformation/retool-r2.ec2.yaml:1843-1844
**SQS wildcard resource granted unconditionally**
The `sqs:ReceiveMessage`, `sqs:DeleteMessage`, and `sqs:GetQueueAttributes` permissions are always added to `AgentSandboxControllerTaskRole` with `Resource: '*'`, even when `EnableTaskEventWatcher=false` (the default). When the watcher is disabled, no `SandboxTaskEventsQueue` is created, yet the controller task can read from and delete messages in any SQS queue in the account. Even when the watcher is enabled, the resource should be scoped to `!GetAtt [SandboxTaskEventsQueue, Arn]` rather than `*`.
### Issue 2 of 5
cloudformation/retool-r2.ec2.yaml:390-397
**RDS security group open to 0.0.0.0/0 on port 5432**
`RetoolECSPostgresInbound` allows TCP 5432 from all IP addresses rather than from the Retool ECS task security group. Although the `SubnetId` parameter targets private subnets (no IGW route), this rule still permits any host that can reach the VPC (e.g. via VPN, peering, or any future public subnet placement) to attempt connections to the database. The existing pattern in the template already scopes other ingress rules by `SourceSecurityGroupId`; the same should be done here, sourced from `RetoolSecurityGroup`.
### Issue 3 of 5
cloudformation/retool-r2.ec2.yaml:470-475
**Sensitive secrets stored as plaintext ECS environment variables**
`AGENT_SANDBOX_JWT_PRIVATE_KEY`, `AGENT_SANDBOX_JWT_PUBLIC_KEY`, and `AGENT_SANDBOX_ENCRYPTION_KEY` are passed via `!Ref` directly into ECS task definition `Environment` entries. CloudFormation's `NoEcho: true` on the parameters only protects them during CF console/API operations; once resolved they are stored as plaintext in the ECS task definition and become readable by anyone with `ecs:DescribeTaskDefinition` on the account. The same pattern repeats in `AgentSandboxProxyTask` (lines 1573–1575), `AgentSandboxControllerTask` (line 1494), `R2AgentWorkerTask` (lines 1774–1779), and the sandbox task definition (line 1658). Using the ECS `secrets:` property backed by Secrets Manager would prevent the values from being stored in the task definition.
### Issue 4 of 5
cloudformation/retool-r2.ec2.yaml:39-42
The description says "Maximum percentage" but this parameter controls the **minimum** healthy task count during a deployment, not the maximum.
```suggestion
MinimumHealthyPercent:
Type: Number
Default: 50
Description: Minimum percentage of healthy tasks to keep running during a deployment
```
### Issue 5 of 5
cloudformation/retool-r2.ec2.yaml:453
**Placeholder `LICENSE_KEY` across all task definitions**
`LICENSE_KEY: "EXPIRED-LICENSE-KEY-TRIAL"` is hardcoded in every Retool task definition (`RetoolTask`, `RetoolJobsRunnerTask`, `RetoolWorkflowsBackendTask`, `RetoolWorkflowsWorkerTask`, `R2AgentWorkerTask`). Deploying without substituting a real license key will result in the backend refusing to start. This value should be a CloudFormation parameter so operators are prompted to supply it rather than relying on post-deploy manual edits.
Reviews (1): Last reviewed commit: "Add R2 ECS CloudFormation template" | Re-trigger Greptile
| Action: [sqs:ReceiveMessage, sqs:DeleteMessage, sqs:GetQueueAttributes] | ||
| Resource: '*' |
There was a problem hiding this comment.
SQS wildcard resource granted unconditionally
The sqs:ReceiveMessage, sqs:DeleteMessage, and sqs:GetQueueAttributes permissions are always added to AgentSandboxControllerTaskRole with Resource: '*', even when EnableTaskEventWatcher=false (the default). When the watcher is disabled, no SandboxTaskEventsQueue is created, yet the controller task can read from and delete messages in any SQS queue in the account. Even when the watcher is enabled, the resource should be scoped to !GetAtt [SandboxTaskEventsQueue, Arn] rather than *.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cloudformation/retool-r2.ec2.yaml
Line: 1843-1844
Comment:
**SQS wildcard resource granted unconditionally**
The `sqs:ReceiveMessage`, `sqs:DeleteMessage`, and `sqs:GetQueueAttributes` permissions are always added to `AgentSandboxControllerTaskRole` with `Resource: '*'`, even when `EnableTaskEventWatcher=false` (the default). When the watcher is disabled, no `SandboxTaskEventsQueue` is created, yet the controller task can read from and delete messages in any SQS queue in the account. Even when the watcher is enabled, the resource should be scoped to `!GetAtt [SandboxTaskEventsQueue, Arn]` rather than `*`.
How can I resolve this? If you propose a fix, please make it concise.| RetoolECSPostgresInbound: | ||
| Type: AWS::EC2::SecurityGroupIngress | ||
| Properties: | ||
| GroupId: !GetAtt [RDSSecurityGroup, GroupId] | ||
| IpProtocol: tcp | ||
| FromPort: '5432' | ||
| ToPort: '5432' | ||
| CidrIp: '0.0.0.0/0' |
There was a problem hiding this comment.
RDS security group open to 0.0.0.0/0 on port 5432
RetoolECSPostgresInbound allows TCP 5432 from all IP addresses rather than from the Retool ECS task security group. Although the SubnetId parameter targets private subnets (no IGW route), this rule still permits any host that can reach the VPC (e.g. via VPN, peering, or any future public subnet placement) to attempt connections to the database. The existing pattern in the template already scopes other ingress rules by SourceSecurityGroupId; the same should be done here, sourced from RetoolSecurityGroup.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cloudformation/retool-r2.ec2.yaml
Line: 390-397
Comment:
**RDS security group open to 0.0.0.0/0 on port 5432**
`RetoolECSPostgresInbound` allows TCP 5432 from all IP addresses rather than from the Retool ECS task security group. Although the `SubnetId` parameter targets private subnets (no IGW route), this rule still permits any host that can reach the VPC (e.g. via VPN, peering, or any future public subnet placement) to attempt connections to the database. The existing pattern in the template already scopes other ingress rules by `SourceSecurityGroupId`; the same should be done here, sourced from `RetoolSecurityGroup`.
How can I resolve this? If you propose a fix, please make it concise.| - Name: AGENT_SANDBOX_JWT_PRIVATE_KEY | ||
| Value: !Ref 'AgentSandboxJwtPrivateKey' | ||
| - Name: AGENT_SANDBOX_JWT_PUBLIC_KEY | ||
| Value: !Ref 'AgentSandboxJwtPublicKey' | ||
| - Name: AGENT_SANDBOX_ENCRYPTION_KEY | ||
| Value: !Ref 'AgentSandboxEncryptionKey' |
There was a problem hiding this comment.
Sensitive secrets stored as plaintext ECS environment variables
AGENT_SANDBOX_JWT_PRIVATE_KEY, AGENT_SANDBOX_JWT_PUBLIC_KEY, and AGENT_SANDBOX_ENCRYPTION_KEY are passed via !Ref directly into ECS task definition Environment entries. CloudFormation's NoEcho: true on the parameters only protects them during CF console/API operations; once resolved they are stored as plaintext in the ECS task definition and become readable by anyone with ecs:DescribeTaskDefinition on the account. The same pattern repeats in AgentSandboxProxyTask (lines 1573–1575), AgentSandboxControllerTask (line 1494), R2AgentWorkerTask (lines 1774–1779), and the sandbox task definition (line 1658). Using the ECS secrets: property backed by Secrets Manager would prevent the values from being stored in the task definition.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cloudformation/retool-r2.ec2.yaml
Line: 470-475
Comment:
**Sensitive secrets stored as plaintext ECS environment variables**
`AGENT_SANDBOX_JWT_PRIVATE_KEY`, `AGENT_SANDBOX_JWT_PUBLIC_KEY`, and `AGENT_SANDBOX_ENCRYPTION_KEY` are passed via `!Ref` directly into ECS task definition `Environment` entries. CloudFormation's `NoEcho: true` on the parameters only protects them during CF console/API operations; once resolved they are stored as plaintext in the ECS task definition and become readable by anyone with `ecs:DescribeTaskDefinition` on the account. The same pattern repeats in `AgentSandboxProxyTask` (lines 1573–1575), `AgentSandboxControllerTask` (line 1494), `R2AgentWorkerTask` (lines 1774–1779), and the sandbox task definition (line 1658). Using the ECS `secrets:` property backed by Secrets Manager would prevent the values from being stored in the task definition.
How can I resolve this? If you propose a fix, please make it concise.| MinimumHealthyPercent: | ||
| Type: Number | ||
| Default: 50 | ||
| Description: Maximum percentage of tasks to run during a deployment |
There was a problem hiding this comment.
The description says "Maximum percentage" but this parameter controls the minimum healthy task count during a deployment, not the maximum.
| MinimumHealthyPercent: | |
| Type: Number | |
| Default: 50 | |
| Description: Maximum percentage of tasks to run during a deployment | |
| MinimumHealthyPercent: | |
| Type: Number | |
| Default: 50 | |
| Description: Minimum percentage of healthy tasks to keep running during a deployment |
Prompt To Fix With AI
This is a comment left during a code review.
Path: cloudformation/retool-r2.ec2.yaml
Line: 39-42
Comment:
The description says "Maximum percentage" but this parameter controls the **minimum** healthy task count during a deployment, not the maximum.
```suggestion
MinimumHealthyPercent:
Type: Number
Default: 50
Description: Minimum percentage of healthy tasks to keep running during a deployment
```
How can I resolve this? If you propose a fix, please make it concise.| - Name: JS_EXECUTOR_INGRESS_DOMAIN | ||
| Value: http://js-executor.retoolsvc:3000 | ||
| - Name: LICENSE_KEY | ||
| Value: "EXPIRED-LICENSE-KEY-TRIAL" |
There was a problem hiding this comment.
Placeholder
LICENSE_KEY across all task definitions
LICENSE_KEY: "EXPIRED-LICENSE-KEY-TRIAL" is hardcoded in every Retool task definition (RetoolTask, RetoolJobsRunnerTask, RetoolWorkflowsBackendTask, RetoolWorkflowsWorkerTask, R2AgentWorkerTask). Deploying without substituting a real license key will result in the backend refusing to start. This value should be a CloudFormation parameter so operators are prompted to supply it rather than relying on post-deploy manual edits.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cloudformation/retool-r2.ec2.yaml
Line: 453
Comment:
**Placeholder `LICENSE_KEY` across all task definitions**
`LICENSE_KEY: "EXPIRED-LICENSE-KEY-TRIAL"` is hardcoded in every Retool task definition (`RetoolTask`, `RetoolJobsRunnerTask`, `RetoolWorkflowsBackendTask`, `RetoolWorkflowsWorkerTask`, `R2AgentWorkerTask`). Deploying without substituting a real license key will result in the backend refusing to start. This value should be a CloudFormation parameter so operators are prompted to supply it rather than relying on post-deploy manual edits.
How can I resolve this? If you propose a fix, please make it concise.
No description provided.