fix(ecs_patterns): openListener should be false when custom sg is provided

[pahud]

Issue # (if applicable)

Closes #35292.

Reason for this change

This change addresses a security vulnerability in the ECS patterns where providing a custom load balancer would still result in the creation of overly permissive internet access rules, potentially bypassing user-intended security controls.

The Security Issue:
The openListener property controls whether CDK automatically creates security group ingress rules allowing internet traffic to reach the load balancer:

• openListener: true (current default): CDK automatically creates an AWS::EC2::SecurityGroupIngress rule with CidrIp: '0.0.0.0/0', allowing anyone on the internet to access the load balancer on the listener port
• openListener: false: CDK does not create automatic ingress rules, leaving security group management entirely to the user

The Problem:
Currently, openListener always defaults to true, regardless of whether users have provided custom load balancers with their own security groups. This creates a security gap where:

1. Users carefully configure custom load balancers with restrictive security groups
2. CDK still automatically adds 0.0.0.0/0 ingress rules, potentially bypassing the user's intended access controls
3. The load balancer becomes accessible from the internet even when users intended to restrict access

Why This Matters:
When users provide custom load balancers, they typically want to manage security group rules themselves. The automatic creation of 0.0.0.0/0 rules can unintentionally expose services to the internet, creating a security vulnerability that users may not immediately notice.

Description of changes

This implementation adds a smart default mechanism for the openListener property in ApplicationLoadBalancedServiceBase that detects when users provide custom load balancers (which typically have custom security groups) and automatically defaults openListener to false for improved security.

Key changes made:

• Smart Default Logic: Added detection for custom load balancers in ApplicationLoadBalancedServiceBase.ts
• Feature Flag Protection: Implemented @aws-cdk/aws-ecs-patterns:smartDefaultOpenListener feature flag to ensure backward compatibility
• Secure Default Behavior: When feature flag is enabled and custom load balancer is detected, openListener defaults to false instead of true
• Override Capability: Users can still explicitly set openListener: true to override the smart default if needed
• Comprehensive Testing: Added 5 new test cases covering all scenarios including backward compatibility

Technical implementation details:

• The smart default logic checks if the feature flag is enabled and if a custom load balancer is provided
• When both conditions are met, openListener defaults to false, preventing automatic creation of AWS::EC2::SecurityGroupIngress rules with CidrIp: '0.0.0.0/0'
• When feature flag is disabled or no custom load balancer is provided, behavior remains unchanged (defaults to true)
• The implementation uses existing CDK patterns and requires no new dependencies

CloudFormation Impact:

• Before (or with openListener: true): CDK generates AWS::EC2::SecurityGroupIngress resources allowing internet access (CidrIp: '0.0.0.0/0')
• After (with smart default openListener: false): No automatic ingress rules are created, users maintain full control over security group configuration

Design decisions made:

• Feature Flag Approach: Chose feature flag implementation to ensure zero breaking changes for existing users
• Custom Load Balancer Detection: Used props.loadBalancer !== undefined as the detection mechanism since custom load balancers typically indicate custom security group management
• Explicit Override Support: Maintained ability for users to explicitly set openListener: true to override smart defaults when needed
• Conservative Approach: Only applies smart default when feature flag is explicitly enabled, ensuring opt-in behavior

Alternatives considered and rejected:

• Always-on behavior: Rejected due to potential breaking changes for existing users
• Security group inspection: Rejected due to complexity and potential for false positives
• Warning-only approach: Rejected as it doesn't actually fix the security vulnerability

Problem Description:

flowchart TD
    A[User Creates ECS Service] --> B[User Provides Custom Load Balancer<br/>with Custom Security Groups]
    B --> C[User Configures Restrictive Rules<br/>e.g., only VPC access]
    C --> D[CDK Always Defaults openListener = true]
    D --> E[CDK Creates 0.0.0.0/0 Ingress Rule<br/>⚠️ BYPASSES User's Security Intent]
    E --> F[🚨 Unintended Internet Exposure<br/>Security Vulnerability]
    
    style E fill:#ffebee
    style F fill:#ffcdd2

Loading

Smart Default Logic (Solution):

flowchart TD
    A[ECS Service Created] --> B{Feature Flag Enabled?}
    B -->|No| C[Legacy: openListener = true<br/>Creates 0.0.0.0/0 ingress rules]
    B -->|Yes| D{Custom Load Balancer Provided?}
    D -->|No| E[Default: openListener = true<br/>🌐 Creates 0.0.0.0/0 ingress rules]
    D -->|Yes| F[Smart Default: openListener = false<br/>🔒 No automatic ingress rules]
    
    style F fill:#e8f5e8
    style E fill:#fff3e0
    style C fill:#f5f5f5

Loading

Describe any new or updated permissions being added

N/A - This change does not introduce new IAM permissions or modify existing permission requirements. The change only affects the default behavior of security group rule creation, using existing ECS and ELB permissions.

Description of how you validated changes

Unit tests: Added comprehensive test coverage with 5 new test cases:

• Smart default with custom load balancer (feature flag enabled) - verifies openListener defaults to false
• Smart default without custom load balancer (feature flag enabled) - verifies openListener defaults to true
• Explicit override with openListener: true (feature flag enabled) - verifies explicit values override smart defaults
• Redirect listener behavior with smart defaults - verifies redirect functionality works with smart defaults
• Backward compatibility test (feature flag disabled) - verifies no behavior change when feature flag is disabled

Integration tests: Created comprehensive integration test (integ.alb-fargate-service-smart-defaults.ts) that:

• Deploys 4 different ECS service configurations to validate real AWS behavior
• Uses AWS SDK calls to verify actual security group configurations
• Confirms that custom security groups do not contain 0.0.0.0/0 ingress rules when smart defaults are applied
• Validates that the feature works correctly in real AWS environments
• Integration test results: Successfully deployed and all assertions passed

Manual validation:

• Verified CloudFormation template generation with and without feature flag enabled
• Confirmed that SecurityGroupIngress resources are not created when custom load balancers are used with smart defaults
• Tested explicit openListener: true override functionality
• Validated that existing behavior is unchanged when feature flag is disabled

Regression testing:

• All existing unit tests continue to pass (226/226 tests passed)
• Existing integration tests verified to ensure no functionality is broken
• Confirmed backward compatibility by testing with feature flag disabled

Performance testing:

• No performance impact - only adds a simple conditional check during construct creation
• Build times and synthesis times remain unchanged

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[Abogical]

Thanks!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.