Implement platform product improvement backlog#12
Conversation
There was a problem hiding this comment.
Summary
This PR implements a platform product improvement by introducing a reusable ApiLambdaDynamoService CDK construct, which consolidates Lambda, API Gateway, DynamoDB, and supporting infrastructure into a golden-path pattern. The refactoring improves code reusability and maintainability.
Critical Security Issues (Must Fix Before Merge)
-
CORS Configuration: The construct allows
ALL_ORIGINSin CORS settings, enabling any domain to make cross-origin requests. This creates CSRF vulnerability risks. -
VPC Default Security Group: Setting
restrictDefaultSecurityGroup: falseviolates AWS security best practices by allowing unrestricted traffic through the default security group. -
DLQ Configuration: The Lambda configuration sets both
deadLetterQueueEnabled: trueand provides adeadLetterQueue, causing unnecessary resource allocation.
Positive Changes
- Clean separation of platform constructs into reusable packages
- Comprehensive test coverage for the new construct
- Proper validation logic for service naming conventions
- CI/CD workflow properly updated to trigger on new package paths
- Maintains security defaults (KMS encryption, IAM auth, VPC isolation)
The architecture refactoring is solid, but the security vulnerabilities must be addressed before merging.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| authorizationType: apigateway.AuthorizationType.IAM, | ||
| }, | ||
| defaultCorsPreflightOptions: { | ||
| allowOrigins: apigateway.Cors.ALL_ORIGINS, |
There was a problem hiding this comment.
🛑 Security Vulnerability: Allowing ALL_ORIGINS in CORS configuration permits any domain to make requests to this API. This enables CSRF attacks and unauthorized cross-origin access.1
| allowOrigins: apigateway.Cors.ALL_ORIGINS, | |
| allowOrigins: [''], // Replace with actual allowed origins |
Footnotes
-
CWE-346: Origin Validation Error - https://cwe.mitre.org/data/definitions/346.html ↩
| deadLetterQueueEnabled: true, | ||
| deadLetterQueue: this.retryQueue, |
There was a problem hiding this comment.
Set deadLetterQueueEnabled to false since you're already providing deadLetterQueue. Setting both causes the Lambda service to create an additional DLQ resource that won't be used, wasting resources.
| deadLetterQueueEnabled: true, | |
| deadLetterQueue: this.retryQueue, | |
| deadLetterQueueEnabled: false, | |
| deadLetterQueue: this.retryQueue, |
| new ec2.Vpc(this, 'ServiceVpc', { | ||
| maxAzs: 2, | ||
| natGateways: 1, | ||
| restrictDefaultSecurityGroup: false, |
There was a problem hiding this comment.
🛑 Security Vulnerability: Setting restrictDefaultSecurityGroup to false disables the default security group restrictions, allowing all traffic by default. This violates AWS security best practices and creates an open security group.1
| restrictDefaultSecurityGroup: false, | |
| restrictDefaultSecurityGroup: true, |
Footnotes
-
CWE-284: Improper Access Control - https://cwe.mitre.org/data/definitions/284.html ↩
…stom resource Lambda Apply Aspect at stack level to reach the singleton provider Lambda, set reservedConcurrentExecutions and DLQ, and skip VPC placement check.
No description provided.