-
Notifications
You must be signed in to change notification settings - Fork 0
Add RDS database support and enhance security group configurations across infrastructure #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,48 @@ export class BastionConstruct extends Construct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add HTTP ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(80), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'HTTP' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add HTTPS ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(443), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'HTTPS' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add MySQL/Aurora ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(3306), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'MySQL/Aurora' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add PostgreSQL ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(5432), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'PostgreSQL' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add Redis ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(6379), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Redis' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add custom app port ingress | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.securityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.tcp(8080), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Custom app port' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+38
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocker: Bastion SG opens DB/cache/web ports to the internet. A bastion should expose SSH only (and preferably be SSM-only). Opening 80/443/3306/5432/6379/8080 to 0.0.0.0/0 is unsafe.
- // Add HTTP ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(80), 'HTTP');
-
- // Add HTTPS ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(443), 'HTTPS');
-
- // Add MySQL/Aurora ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(3306), 'MySQL/Aurora');
-
- // Add PostgreSQL ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(5432), 'PostgreSQL');
-
- // Add Redis ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(6379), 'Redis');
-
- // Add custom app port ingress
- this.securityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(8080), 'Custom app port');📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create IAM role for bastion with SSM support | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const bastionRole = new iam.Role(this, 'BastionRole', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -78,6 +120,10 @@ export class BastionConstruct extends Construct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requireImdsv2: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Enable instance metadata tags | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cfnInstance = this.instance.node.defaultChild as ec2.CfnInstance; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cfnInstance.addPropertyOverride('MetadataOptions.InstanceMetadataTags', 'enabled'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Allocate and associate Elastic IP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.elasticIp = new ec2.CfnEIP(this, 'BastionEIP', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| domain: 'vpc', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ export interface CloudFrontConstructProps { | |||||||||||||||||||||||||||||||||
| domainName?: string; | ||||||||||||||||||||||||||||||||||
| certificateArn?: string; | ||||||||||||||||||||||||||||||||||
| environment: string; | ||||||||||||||||||||||||||||||||||
| costCenter?: string; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export class CloudFrontConstruct extends Construct { | ||||||||||||||||||||||||||||||||||
|
|
@@ -182,6 +183,15 @@ export class CloudFrontConstruct extends Construct { | |||||||||||||||||||||||||||||||||
| errorResponses: errorResponses, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Add tags to CloudFront distribution | ||||||||||||||||||||||||||||||||||
| cdk.Tags.of(this.distribution).add('Name', `${props.environment}-cloudfront`); | ||||||||||||||||||||||||||||||||||
| cdk.Tags.of(this.distribution).add('Environment', props.environment); | ||||||||||||||||||||||||||||||||||
| cdk.Tags.of(this.distribution).add('ManagedBy', 'Terraform'); | ||||||||||||||||||||||||||||||||||
| cdk.Tags.of(this.distribution).add('Service', 'CDN'); | ||||||||||||||||||||||||||||||||||
| if (props.costCenter) { | ||||||||||||||||||||||||||||||||||
| cdk.Tags.of(this.distribution).add('CostCenter', props.costCenter); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
Comment on lines
+186
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tag inconsistency: ManagedBy should reflect CDK, not Terraform. Retain CostCenter tagging; change ManagedBy to “AWS CDK”. - cdk.Tags.of(this.distribution).add('ManagedBy', 'Terraform');
+ cdk.Tags.of(this.distribution).add('ManagedBy', 'AWS CDK');📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| // Outputs | ||||||||||||||||||||||||||||||||||
| new cdk.CfnOutput(this, 'DistributionId', { | ||||||||||||||||||||||||||||||||||
| value: this.distribution.distributionId, | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { Construct } from 'constructs'; | |
|
|
||
| export interface EksConstructProps { | ||
| vpc: ec2.IVpc; | ||
| vpcCidr: string; | ||
| clusterName: string; | ||
| kubernetesVersion: string; | ||
| nodeInstanceTypes: string[]; | ||
|
|
@@ -22,10 +23,41 @@ export interface EksConstructProps { | |
| export class EksConstruct extends Construct { | ||
| public readonly cluster: eks.Cluster; | ||
| public readonly nodeGroup: eks.Nodegroup; | ||
| public readonly nodeSecurityGroup: ec2.SecurityGroup; | ||
|
|
||
| constructor(scope: Construct, id: string, props: EksConstructProps) { | ||
| super(scope, id); | ||
|
|
||
| // Create node security group | ||
| this.nodeSecurityGroup = new ec2.SecurityGroup(this, 'NodeSecurityGroup', { | ||
| vpc: props.vpc, | ||
| description: 'Security group for EKS nodes', | ||
| allowAllOutbound: true, | ||
| }); | ||
|
|
||
| // Add Kubelet API ingress | ||
| this.nodeSecurityGroup.addIngressRule( | ||
| ec2.Peer.ipv4(props.vpcCidr), | ||
| ec2.Port.tcp(10251), | ||
| 'Kubelet API' | ||
| ); | ||
|
|
||
| // Add SSH access ingress | ||
| this.nodeSecurityGroup.addIngressRule( | ||
| ec2.Peer.anyIpv4(), | ||
| ec2.Port.tcp(22), | ||
| 'SSH access' | ||
| ); | ||
|
|
||
| // Add NodePort services ingress | ||
| this.nodeSecurityGroup.addIngressRule( | ||
| ec2.Peer.anyIpv4(), | ||
| ec2.Port.tcpRange(30000, 32767), | ||
| 'NodePort services' | ||
| ); | ||
|
|
||
| cdk.Tags.of(this.nodeSecurityGroup).add('Name', `${props.clusterName}-node-sg`); | ||
|
|
||
|
Comment on lines
+31
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainBlocker: EKS node SG opens SSH/NodePort to the world and targets wrong kubelet port.
Minimal safer change inside this construct: - // Add Kubelet API ingress
- this.nodeSecurityGroup.addIngressRule(ec2.Peer.ipv4(props.vpcCidr), ec2.Port.tcp(10251), 'Kubelet API');
+ // Kubelet API from within VPC
+ this.nodeSecurityGroup.addIngressRule(ec2.Peer.ipv4(props.vpcCidr), ec2.Port.tcp(10250), 'Kubelet API');
- // Add SSH access ingress
- this.nodeSecurityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(22), 'SSH access');
+ // Optional: restrict SSH to VPC; better: allow only from bastion SG in the stack wiring
+ this.nodeSecurityGroup.addIngressRule(ec2.Peer.ipv4(props.vpcCidr), ec2.Port.tcp(22), 'SSH from VPC');
- // Add NodePort services ingress
- this.nodeSecurityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcpRange(30000, 32767), 'NodePort services');
+ // NodePort only within VPC
+ this.nodeSecurityGroup.addIngressRule(ec2.Peer.ipv4(props.vpcCidr), ec2.Port.tcpRange(30000, 32767), 'NodePort within VPC');Follow-up (in the stack): add precise SG-to-SG rules from cluster SG and bastion SG as needed. I'll verify the technical accuracy of the review comment by checking AWS EKS best practices and inspecting the codebase. 🏁 Script executed: #!/bin/bash
# Locate and display the EKS construct file to verify the exact code
find . -type f -name "eks-construct.ts" | head -5Length of output: 33 🌐 Web query: 💡 Result: Short answer
Recommended security-group rules (minimal)
Notes and hardening tips
Sources
References Now I'll inspect the actual file and search for related security group definitions in the codebase. 🏁 Script executed: #!/bin/bash
# Display the full EKS construct file
cat ./lib/constructs/eks-construct.tsLength of output: 6130 🏁 Script executed: #!/bin/bash
# Search for other SecurityGroup or control-plane references in constructs
rg -n "SecurityGroup|controlPlane|Bastion|bastion" ./lib/constructs --type ts -C 2Length of output: 11929 🏁 Script executed: #!/bin/bash
# Search for props interface/type definition for EksConstruct
rg -n "interface.*Props|props.*vpcCidr|props.*vpc" ./lib/constructs/eks-construct.ts -A 3 -B 1Length of output: 764 Critical: EKS node security group exposes SSH/NodePort to 0.0.0.0/0 and targets wrong Kubelet port. Port 10250 is the Kubelet API port that the EKS control plane must reach; 10251 is the insecure kube-scheduler port that runs on the control plane only. The code at line 41 uses port 10251, which is incorrect. Additionally, lines 47 and 54 expose SSH and NodePort services to 0.0.0.0/0, creating critical security vulnerabilities. Fix required: // Add Kubelet API ingress
- this.nodeSecurityGroup.addIngressRule(
- ec2.Peer.ipv4(props.vpcCidr),
- ec2.Port.tcp(10251),
- 'Kubelet API'
- );
+ this.nodeSecurityGroup.addIngressRule(
+ ec2.Peer.ipv4(props.vpcCidr),
+ ec2.Port.tcp(10250),
+ 'Kubelet API'
+ );
// Add SSH access ingress
- this.nodeSecurityGroup.addIngressRule(
- ec2.Peer.anyIpv4(),
- ec2.Port.tcp(22),
- 'SSH access'
- );
+ this.nodeSecurityGroup.addIngressRule(
+ ec2.Peer.ipv4(props.vpcCidr),
+ ec2.Port.tcp(22),
+ 'SSH from VPC'
+ );
// Add NodePort services ingress
- this.nodeSecurityGroup.addIngressRule(
- ec2.Peer.anyIpv4(),
- ec2.Port.tcpRange(30000, 32767),
- 'NodePort services'
- );
+ this.nodeSecurityGroup.addIngressRule(
+ ec2.Peer.ipv4(props.vpcCidr),
+ ec2.Port.tcpRange(30000, 32767),
+ 'NodePort within VPC'
+ );For production hardening, add SG-to-SG rules in the consuming stack to allow kubelet/SSH access only from the cluster control-plane SG and bastion SG as needed. 🤖 Prompt for AI Agents |
||
| // Create CloudWatch log group | ||
| const logGroup = new logs.LogGroup(this, 'ClusterLogGroup', { | ||
| logGroupName: `/aws/eks/${props.clusterName}/cluster`, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import * as cdk from 'aws-cdk-lib'; | ||
| import * as ec2 from 'aws-cdk-lib/aws-ec2'; | ||
| import * as rds from 'aws-cdk-lib/aws-rds'; | ||
| import { Construct } from 'constructs'; | ||
|
|
||
| export interface RdsConstructProps { | ||
| vpc: ec2.IVpc; | ||
| instanceClass: string; | ||
| allocatedStorage: number; | ||
| maxAllocatedStorage: number; | ||
| databaseName: string; | ||
| masterUsername: string; | ||
| environment: string; | ||
| } | ||
|
|
||
| export class RdsConstruct extends Construct { | ||
| public readonly dbInstance: rds.DatabaseInstance; | ||
| public readonly securityGroup: ec2.SecurityGroup; | ||
|
|
||
| constructor(scope: Construct, id: string, props: RdsConstructProps) { | ||
| super(scope, id); | ||
|
|
||
| // Create security group for RDS | ||
| this.securityGroup = new ec2.SecurityGroup(this, 'RdsSecurityGroup', { | ||
| vpc: props.vpc, | ||
| description: 'Security group for RDS', | ||
| allowAllOutbound: true, | ||
| }); | ||
|
|
||
| // Add PostgreSQL ingress from anywhere (0.0.0.0/0) | ||
| this.securityGroup.addIngressRule( | ||
| ec2.Peer.anyIpv4(), | ||
| ec2.Port.tcp(5432), | ||
| 'PostgreSQL from anywhere' | ||
| ); | ||
|
|
||
|
Comment on lines
+31
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainBlocker: RDS instance is publicly exposed, unencrypted, with hardcoded credentials and no backups. Please address all of the following:
Suggested secure baseline: - // Add PostgreSQL ingress from anywhere (0.0.0.0/0)
- this.securityGroup.addIngressRule(
- ec2.Peer.anyIpv4(),
- ec2.Port.tcp(5432),
- 'PostgreSQL from anywhere'
- );
+ // No default ingress here. Grant SG-scoped access from callers (bastion/EKS) in the stack.
@@
- credentials: rds.Credentials.fromPassword(
- props.masterUsername,
- cdk.SecretValue.unsafePlainText('admin123') // WARNING: Hardcoded password from patch
- ),
+ credentials: rds.Credentials.fromGeneratedSecret(props.masterUsername, {
+ secretName: `${props.environment}/rds/${props.databaseName}/master`,
+ }),
@@
- storageEncrypted: false, // As per patch
+ storageEncrypted: true,
@@
- publiclyAccessible: true, // As per patch
+ publiclyAccessible: false,
@@
- backupRetention: cdk.Duration.days(0), // No backups as per patch
- deleteAutomatedBackups: true,
- removalPolicy: cdk.RemovalPolicy.DESTROY,
- deletionProtection: false,
+ backupRetention: props.environment === 'prod' ? cdk.Duration.days(7) : cdk.Duration.days(1),
+ deleteAutomatedBackups: false,
+ removalPolicy: props.environment === 'prod' ? cdk.RemovalPolicy.RETAIN : cdk.RemovalPolicy.DESTROY,
+ deletionProtection: props.environment === 'prod',Optionally also enable logs and Performance Insights: - enablePerformanceInsights: false,
- cloudwatchLogsExports: [],
+ enablePerformanceInsights: props.environment === 'prod',
+ cloudwatchLogsExports: ['postgresql'],Also applies to: 56-62, 63-71, 72-77 Length of output: 3594 Blocker: RDS instance is publicly exposed, unencrypted, with hardcoded credentials and no backups. Please address all of the following:
Suggested secure baseline: - // Add PostgreSQL ingress from anywhere (0.0.0.0/0)
- this.securityGroup.addIngressRule(
- ec2.Peer.anyIpv4(),
- ec2.Port.tcp(5432),
- 'PostgreSQL from anywhere'
- );
+ // No default ingress here. Grant SG-scoped access from callers (bastion/EKS) in the stack.
@@
- credentials: rds.Credentials.fromPassword(
- props.masterUsername,
- cdk.SecretValue.unsafePlainText('admin123') // WARNING: Hardcoded password from patch
- ),
+ credentials: rds.Credentials.fromGeneratedSecret(props.masterUsername, {
+ secretName: `${props.environment}/rds/${props.databaseName}/master`,
+ }),
@@
- storageEncrypted: false, // As per patch
+ storageEncrypted: true,
@@
- publiclyAccessible: true, // As per patch
+ publiclyAccessible: false,
@@
- backupRetention: cdk.Duration.days(0), // No backups as per patch
- deleteAutomatedBackups: true,
- removalPolicy: cdk.RemovalPolicy.DESTROY,
- deletionProtection: false,
+ backupRetention: props.environment === 'prod' ? cdk.Duration.days(7) : cdk.Duration.days(1),
+ deleteAutomatedBackups: false,
+ removalPolicy: props.environment === 'prod' ? cdk.RemovalPolicy.RETAIN : cdk.RemovalPolicy.DESTROY,
+ deletionProtection: props.environment === 'prod',Optionally also enable logs and Performance Insights: - enablePerformanceInsights: false,
- cloudwatchLogsExports: [],
+ enablePerformanceInsights: props.environment === 'prod',
+ cloudwatchLogsExports: ['postgresql'],
|
||
| // Create DB subnet group | ||
| const subnetGroup = new rds.SubnetGroup(this, 'DbSubnetGroup', { | ||
| vpc: props.vpc, | ||
| description: `${props.environment} DB subnet group`, | ||
| vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }, | ||
| subnetGroupName: `${props.environment}-db-subnet-group`, | ||
| }); | ||
|
|
||
| // Create PostgreSQL RDS instance | ||
| this.dbInstance = new rds.DatabaseInstance(this, 'PostgresInstance', { | ||
| engine: rds.DatabaseInstanceEngine.postgres({ | ||
| version: rds.PostgresEngineVersion.VER_14_7, | ||
| }), | ||
| instanceType: new ec2.InstanceType(props.instanceClass), | ||
| vpc: props.vpc, | ||
| vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }, | ||
| securityGroups: [this.securityGroup], | ||
| subnetGroup: subnetGroup, | ||
|
|
||
| // Database configuration | ||
| databaseName: props.databaseName, | ||
| credentials: rds.Credentials.fromPassword( | ||
| props.masterUsername, | ||
| cdk.SecretValue.unsafePlainText('admin123') // WARNING: Hardcoded password from patch | ||
| ), | ||
|
|
||
| // Storage configuration | ||
| allocatedStorage: props.allocatedStorage, | ||
| maxAllocatedStorage: props.maxAllocatedStorage, | ||
| storageType: rds.StorageType.GP3, | ||
| storageEncrypted: false, // As per patch | ||
|
|
||
| // Access configuration | ||
| publiclyAccessible: true, // As per patch | ||
|
|
||
| // Backup configuration | ||
| backupRetention: cdk.Duration.days(0), // No backups as per patch | ||
| deleteAutomatedBackups: true, | ||
| removalPolicy: cdk.RemovalPolicy.DESTROY, | ||
| deletionProtection: false, | ||
|
|
||
| // Monitoring | ||
| enablePerformanceInsights: false, | ||
| cloudwatchLogsExports: [], | ||
|
|
||
| instanceIdentifier: `${props.environment}-postgres`, | ||
| }); | ||
|
|
||
| // Outputs | ||
| new cdk.CfnOutput(this, 'DbInstanceId', { | ||
| value: this.dbInstance.instanceIdentifier, | ||
| description: 'RDS instance ID', | ||
| exportName: `${props.environment}-DbInstanceId`, | ||
| }); | ||
|
|
||
| new cdk.CfnOutput(this, 'DbInstanceEndpoint', { | ||
| value: this.dbInstance.dbInstanceEndpointAddress, | ||
| description: 'RDS instance endpoint', | ||
| exportName: `${props.environment}-DbInstanceEndpoint`, | ||
| }); | ||
|
|
||
| new cdk.CfnOutput(this, 'DbInstanceArn', { | ||
| value: this.dbInstance.instanceArn, | ||
| description: 'RDS instance ARN', | ||
| exportName: `${props.environment}-DbInstanceArn`, | ||
| }); | ||
|
|
||
| new cdk.CfnOutput(this, 'DbSecurityGroupId', { | ||
| value: this.securityGroup.securityGroupId, | ||
| description: 'RDS security group ID', | ||
| exportName: `${props.environment}-DbSecurityGroupId`, | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,8 +59,12 @@ export class S3Construct extends Construct { | |
| // Create static content bucket | ||
| this.staticBucket = new s3.Bucket(this, 'StaticBucket', { | ||
| bucketName: props.staticBucketName, | ||
| encryption: s3.BucketEncryption.S3_MANAGED, | ||
| blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, | ||
| blockPublicAccess: new s3.BlockPublicAccess({ | ||
| blockPublicAcls: false, | ||
| blockPublicPolicy: false, | ||
| ignorePublicAcls: false, | ||
| restrictPublicBuckets: false, | ||
| }), | ||
|
Comment on lines
+62
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocker: Public access protections disabled and encryption removed on S3.
Suggested fix: this.staticBucket = new s3.Bucket(this, 'StaticBucket', {
bucketName: props.staticBucketName,
- blockPublicAccess: new s3.BlockPublicAccess({
- blockPublicAcls: false,
- blockPublicPolicy: false,
- ignorePublicAcls: false,
- restrictPublicBuckets: false,
- }),
+ blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
+ encryption: s3.BucketEncryption.S3_MANAGED,
versioned: props.enableVersioning,
cors: staticBucketCors,
removalPolicy: cdk.RemovalPolicy.RETAIN,
autoDeleteObjects: false,
});
this.userBucket = new s3.Bucket(this, 'UserBucket', {
bucketName: props.userBucketName,
blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
+ encryption: s3.BucketEncryption.S3_MANAGED,
versioned: props.enableVersioning,
cors: userBucketCors,
removalPolicy: cdk.RemovalPolicy.RETAIN,
autoDeleteObjects: false,
lifecycleRules: props.enableLifecycleRules ? [ /* ... */ ] : undefined,
});Also applies to: 79-117 🤖 Prompt for AI Agents |
||
| versioned: props.enableVersioning, | ||
| cors: staticBucketCors, | ||
| removalPolicy: cdk.RemovalPolicy.RETAIN, | ||
|
|
@@ -73,7 +77,6 @@ export class S3Construct extends Construct { | |
| // Create user content bucket | ||
| this.userBucket = new s3.Bucket(this, 'UserBucket', { | ||
| bucketName: props.userBucketName, | ||
| encryption: s3.BucketEncryption.S3_MANAGED, | ||
| blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL, | ||
| versioned: props.enableVersioning, | ||
| cors: userBucketCors, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,23 @@ export class VpcConstruct extends Construct { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| this.publicSubnets = this.vpc.publicSubnets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.privateSubnets = this.vpc.privateSubnets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create default security group with allow all inbound | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const defaultSecurityGroup = new ec2.SecurityGroup(this, 'DefaultSecurityGroup', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| vpc: this.vpc, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: 'Default security group with allow all inbound', | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| allowAllOutbound: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| securityGroupName: `${props.environment}-default-sg`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Add allow all inbound rule | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| defaultSecurityGroup.addIngressRule( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Peer.anyIpv4(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ec2.Port.allTraffic(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Allow all inbound' | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| cdk.Tags.of(defaultSecurityGroup).add('Name', `${props.environment}-default-sg`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+56
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocker: Security group allows all inbound from the internet. Creating a “default” SG that permits all inbound (0.0.0.0/0, all ports) is a severe risk. If attached anywhere, it exposes workloads.
Suggested change: - // Add allow all inbound rule
- defaultSecurityGroup.addIngressRule(
- ec2.Peer.anyIpv4(),
- ec2.Port.allTraffic(),
- 'Allow all inbound'
- );📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Outputs | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| new cdk.CfnOutput(this, 'VpcId', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: this.vpc.vpcId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Security posture follow-up: restrict SSH CIDRs and plan DB secrets.
No code change here required; ensure the constructs use a generated/managed secret.
Also applies to: 83-95, 133-145
🏁 Script executed:
Length of output: 4256
🏁 Script executed:
Length of output: 567
🏁 Script executed:
Length of output: 5018
Critical: Remove hardcoded RDS password and restrict database access.
The security concerns are valid but more critical than noted:
Hardcoded credential in rds-construct.ts (line 60):
cdk.SecretValue.unsafePlainText('admin123')must be replaced with AWS Secrets Manager integration usingrds.Credentials.fromGeneratedSecret()orrds.Credentials.fromUsername()with a properly managed secret.Dev SSH unrestricted: Restrict
bastionAllowedCidrsin devConfig (line 73) from['0.0.0.0/0']to specific IP ranges (office/VPN).Additional RDS exposures (rds-construct.ts):
publiclyAccessible: falsestorageEncrypted: truebackupRetention(minimum 7 days for prod)deletionProtection: truefor prod🤖 Prompt for AI Agents