Skip to content

Conversation

nzuresh
Copy link

@nzuresh nzuresh commented Oct 6, 2025

Fixes #

Summary

Changes

This PR adds deep infrastructure security analysis for ECS clusters by examining container instances and capacity providers. It extends the security analysis functionality introduced in PR #1 with comprehensive infrastructure-level checks.

New Security Checks Added:

🖥️ Container Instance Security (3 checks)

  • 🔴 Outdated ECS Agent Detection (High) - Identifies agents below v1.70.0 with security vulnerabilities
  • 🔴 Agent Connectivity Issues (High) - Detects disconnected agents or non-ACTIVE instances
  • 🟡 Legacy Instance Types (Medium) - Flags old generation instances (t2, m4, c4, r4) lacking modern security features

⚙️ Capacity Provider Security (3 checks)

  • 🟡 Managed Termination Protection (Medium) - Validates protection against premature termination
  • 🟢 Managed Scaling Status (Low) - Checks ECS-managed Auto Scaling configuration
  • 🟡 Target Capacity Configuration (Medium) - Validates capacity is within optimal 80-100% range

Technical Implementation:

  • Added collect_container_instances() and collect_capacity_providers() data collection methods
  • Implemented _analyze_enhanced_cluster_security() (~150 lines) for container instance analysis
  • Implemented _analyze_capacity_providers() (~100 lines) for capacity provider analysis
  • Added _is_agent_version_outdated() helper for semantic version comparison
  • Production Code: ~250 lines
  • Test Code: ~740 lines (3x ratio, following project patterns)
  • Test Coverage: 94% (exceeds 90% requirement)
  • Total Tests: 130 (60+ new tests added)

User experience

Before this change:
Users could only analyze cluster-level configuration settings (Container Insights, execute command logging). No visibility into infrastructure security posture.

After this change:
Users get comprehensive infrastructure security analysis including:

  • Identification of vulnerable ECS agents requiring updates
  • Detection of connectivity issues preventing task scheduling
  • Recommendations to migrate from legacy to modern instance types
  • Validation of capacity provider configurations for operational safety
  • Actionable CLI commands for remediation of each issue

Example Output:

{
  "recommendations": [
    {
      "title": "🔴 Outdated ECS Agent Version",
      "severity": "High",
      "category": "Container Instance",
      "resource": "abc123",
      "resource_type": "ContainerInstance",
      "issue": "Container instance abc123 is running ECS agent version 1.65.0, which is below the recommended minimum version 1.70.0. Outdated agents may have security vulnerabilities or lack important features.",
      "recommendation": "Update the ECS agent to the latest version to ensure security patches and feature improvements are applied",
      "remediation_steps": [
        "# For Amazon Linux 2 AMI:",
        "# SSH into the instance (EC2 instance ID: i-1234567890abcdef0)",
        "sudo yum update -y ecs-init",
        "sudo systemctl restart ecs"
      ],
      "documentation_links": [
        "https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-agent-update.html"
      ]
    }
  ]
}
Consistency with PR #1:

✅ Color-coded severity indicators (🔴 High, 🟡 Medium, 🟢 Low)
✅ Detailed recommendation structure with actionable CLI commands
✅ Resource type categorization
✅ AWS documentation links
✅ DRY principle with helper methods
Checklist
If your change doesn't seem to apply, please leave them unchecked.

[x] I have reviewed the contributing guidelines
[x] I have performed a self-review of this change
[x] Changes have been tested
[x] Changes are documented
Quality Checks Passed:

✅ Pattern checks passed (coding standards)
✅ Code formatting passed (ruff format)
✅ Linting passed (ruff check)
✅ Type checking passed (pyright)
✅ Pre-commit validation passed
✅ Tests passed: 130 tests, 94% coverage
Is this a breaking change? N

RFC issue number: N/A

Checklist:

[ ] Migration process documented
[ ] Implement warnings (if it can live side by side)
## Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the [project license](https://github.com/awslabs/mcp/blob/main/LICENSE).

Add comprehensive security analysis functionality for ECS clusters with
interactive user workflow and AWS Trusted Advisor-style recommendations.

Features:
- Interactive cluster selection workflow (prevents automatic analysis)
- Security checks for Container Insights, exec logging, CloudWatch encryption
- Color-coded recommendations (red/yellow/green) with severity levels
- Two-section output: Critical/High priority + Medium/Low priority
- Comprehensive error handling and user guidance

Implementation:
- Module layer: FastMCP tool registration with detailed documentation
- API layer: DataAdapter and SecurityAnalyzer classes
- Test suite: 75 tests with 97% coverage using parameterized tests

Files:
- awslabs/ecs_mcp_server/api/security_analysis.py (478 lines)
- awslabs/ecs_mcp_server/modules/security_analysis.py (198 lines)
- tests/unit/test_security_analysis.py (405 lines)
- awslabs/ecs_mcp_server/main.py (2 lines added)

All quality checks passed: formatting, linting, type checking, DRY principle
… instance and capacity provider checks

- Add container instance security checks (agent version, connectivity, instance types)
- Add capacity provider security checks (termination protection, scaling config)
- Implement data collection methods for container instances and capacity providers
- Add comprehensive test coverage (94%) with 60+ new tests
- Follow color-coded severity pattern (🔴 High, 🟡 Medium, 🟢 Low)
- Include actionable CLI remediation steps for all recommendations

Closes #<issue-number>
@nzuresh nzuresh force-pushed the feature/pr2-enhanced-cluster branch from a471941 to 20e77dc Compare October 6, 2025 21:59
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 91.18943% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.45%. Comparing base (c2d7bf8) to head (20e77dc).

Files with missing lines Patch % Lines
...er/awslabs/ecs_mcp_server/api/security_analysis.py 93.23% 7 Missing and 7 partials ⚠️
...wslabs/ecs_mcp_server/modules/security_analysis.py 68.42% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1458    +/-   ##
========================================
  Coverage   89.45%   89.45%            
========================================
  Files         726      728     +2     
  Lines       50305    50532   +227     
  Branches     7942     7980    +38     
========================================
+ Hits        44998    45205   +207     
- Misses       3452     3465    +13     
- Partials     1855     1862     +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

1 participant