Harden YOLO: fix RDS ingress, remove dead EC2 SG, lock down S3 buckets#38
Merged
Conversation
The EC2 security group was an EC2/ASG-era leftover: Fargate tasks run in the task SG, so it held no workloads while still opening SSH/22 (to an ipify-derived IP) and HTTP/80. Its only live use was the RDS ingress rule, which authorised 3306 from the EC2 SG — so Fargate tasks were never actually granted DB access. RDS ingress now targets the task SG, applied additively: a tagged rule is ensured and nothing is ever revoked, so any out-of-band access is preserved. The RDS SG step moves to sync:compute (after the task SG it references). Also removes the orphaned launch-template/keyPair helpers in UsesEc2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The artefact bucket stores .env files but, unlike the asset bucket, had no Block Public Access. Enable all four BPA flags plus versioning (recovery and tamper-evidence for .env), reconciled on every sync. New app buckets get BPA on create only, so existing buckets that legitimately serve public objects aren't broken. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevethomas
commented
May 23, 2026
stevethomas
commented
May 23, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The name-based arn() lookup already throws ResourceDoesNotExistException if the task SG is missing — the defensive exists() guard was both redundant (sync:compute provisions it first) and wrong, silently swallowing a real failure instead of surfacing it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The tag only served to pre-check whether the rule already existed — a guarantee AWS already provides (it rejects duplicate permissions). That made it the same redundant pre-check as the exists() guard, and less robust: an untagged, manually added identical rule slips past the tag filter and then fails on the duplicate. Idempotency is now by content (protocol + port + source SG), which also spots out-of-band duplicates. The tag earns its place only for drift reconciliation (SyncLoadBalancerSecurityGroupStep), which this step doesn't do. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey, I made a thing! 🥳
A focused security pass over the Fargate deploy path.
What problems are you solving?
/32) and HTTP/80 but held no workloads — Fargate tasks run in the task SG. Removed the step, its enum cases, and the orphanedlaunchTemplate()/keyPair()/ec2SecurityGroup()helpers inUsesEc2.sync:compute(after the task SG it now references)..envfiles but, unlike the asset bucket, had no BPA. Added all four BPA flags + versioning, reconciled on every sync. New app buckets get BPA on create only.Is there anything the reviewer needs to know to deploy this?
yolo destroyif wanted). New environments never get an EC2 SG.public/CDN scoping (dotfiles + source maps excluded) already landed onmainas 169d94e, so it isn't in this diff.yolo sync:network --dry-runthensync:compute --dry-runagainst a real account to confirm the RDS rule lands additively before a real sync.Follow-ups (not in this PR, tracked in LPX-623): move secrets to runtime injection (SSM param versions, release-pinned in the task definition) so
.envstops being baked into the image; ECR tag immutability; extract the shared Block-Public-Access config; more dead EC2 code inUsesIam/EnsureIamRolesExistStep.🤖 Generated with Claude Code