Uh oh!
There was an error while loading. Please reload this page.
fix(aws): add validateAwsRegion to all AWS route schemas to prevent SSRF - #4250
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Updates Reviewed by Cursor Bugbot for commit 1b97c15. Configure here. |
Greptile SummaryThis PR correctly applies Confidence Score: 4/5Safe to merge for the 61 covered routes, but the SSRF vulnerability persists in 31 other AWS routes — follow-up is required before the fix can be considered complete. The regex fix and all 61 route updates are correct and well-tested. A concrete, confirmed P1 security gap remains: S3, RDS, Secrets Manager, SQS, Textract, Athena, and CloudFormation routes still have the original unvalidated region field. The PR title and description claim completeness so the omission is unexpected rather than a deliberate deferral. apps/sim/app/api/tools/s3/, apps/sim/app/api/tools/rds/, apps/sim/app/api/tools/secrets_manager/, apps/sim/app/api/tools/sqs/, apps/sim/app/api/tools/textract/, apps/sim/app/api/tools/athena/, apps/sim/app/api/tools/cloudformation/*
|
| Filename | Overview |
|---|---|
| apps/sim/lib/core/security/input-validation.ts | Regex updated to add eu-isoe and mx prefixes, and alternation reordered (longer prefixes first) — logic is correct and covers all 41 known AWS regions |
| apps/sim/lib/core/security/input-validation.test.ts | New test cases added for us-iso-west-1, mx-central-1, and eu-isoe-west-1 — all are correct and cover the newly added regex branches |
| apps/sim/app/api/tools/cloudwatch/describe-alarms/route.ts | Representative of all 61 routes updated in this PR — validateAwsRegion.refine() applied correctly with a static message object |
| apps/sim/app/api/tools/s3/put-object/route.ts | Still uses bare region: z.string() — region is passed to S3Client and also string-interpolated into the response URL, leaving the same SSRF vector unpatched |
| apps/sim/app/api/tools/rds/query/route.ts | Still uses bare region: z.string() passed directly to createRdsClient — same SSRF vector as the routes fixed in this PR |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming POST request] --> B[checkInternalAuth]
B -->|fail| C[401 Unauthorized]
B -->|pass| D[Zod schema parse]
D --> E{region field}
E -->|61 patched routes: validateAwsRegion refine| F[Regex check passes]
E -->|31 unpatched routes: bare z.string| G[Any string accepted]
F --> H[AWS SDK Client constructed with validated region]
G --> I[AWS SDK Client constructed with arbitrary region]
I --> J[SSRF: SDK resolves attacker-controlled endpoint]
H --> K[AWS API call succeeds safely]
Comments Outside Diff (1)
apps/sim/app/api/tools/s3/put-object/route.ts, line 19 (link)SSRF fix incomplete — S3, RDS, Secrets Manager, SQS, Textract, Athena, and CloudFormation routes still unprotected
The PR title states "add
validateAwsRegionto all AWS route schemas" but 31 AWS routes were omitted. All of them accept a bareregion: z.string()that is passed directly to the AWS SDK (and in S3's case, also interpolated into the response URL on line 121:`https://${bucketName}.s3.${region}.amazonaws.com/...`). Affected services and representative files:- S3 —
s3/copy-object,s3/delete-object,s3/list-objects,s3/put-object - RDS —
rds/delete,rds/execute,rds/insert,rds/introspect,rds/query,rds/update - Secrets Manager —
secrets_manager/create-secret,secrets_manager/delete-secret,secrets_manager/get-secret,secrets_manager/list-secrets,secrets_manager/update-secret - SQS —
sqs/send - Textract —
textract/parse - Athena — 7 routes
- CloudFormation — 7 routes
The same
.refine((v) => validateAwsRegion(v).isValid, { message: ... })pattern used in this PR should be applied to all of these.- S3 —
Reviews (2): Last reviewed commit: "fix(aws): eliminate double validateAwsRe..." | Re-trigger Greptile
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…nation order - Replace double-call .refine() pattern with single-call + static message across all 61 AWS routes - Reorder regex alternation to put longer prefixes first (eu-isoe before eu, us-isob/us-iso/us-gov before us) for engine-agnostic correctness
waleedlatif1
commented
Apr 22, 2026
waleedlatif1
commented
Apr 22, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1b97c15. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Type of Change
Testing
Checklist