fix(rds): flip StorageEncrypted default to true (CIS-AWS RDS.3)#240
Open
fix(rds): flip StorageEncrypted default to true (CIS-AWS RDS.3)#240
Conversation
PR #239 added the `StorageEncrypted *bool` field to MysqlConfig / PostgresConfig as opt-in (nil → false → unencrypted), preserving pre-2026.5 SC behaviour. That keeps every customer who hasn't read the new field's docs on unencrypted RDS, violating CIS-AWS Foundations RDS.3. This change flips the resolution helper from `lo.FromPtr(*bool)` to `lo.FromPtrOr(*bool, true)` at both call sites: pkg/clouds/pulumi/aws/rds_mysql.go:137 pkg/clouds/pulumi/aws/rds_postgres.go:121 Resolution table: before: after: nil → false (unencrypted) nil → true (encrypted, secure default) &true → true &true → true (unchanged) &false → false &false → false (unchanged, explicit opt-out) Why this is safe for existing instances ======================================= PR #239 already added `IgnoreChanges([]string{"storageEncrypted"})` on the resource opts for exactly this scenario. Pulumi's IgnoreChanges diffing rule: "treat the desired value as the recorded state value for the field." So: Existing unencrypted instance, customer never set the field in YAML: state.storageEncrypted = false new spec.storageEncrypted = true (from lo.FromPtrOr(nil, true)) diff: IgnoreChanges → no change registered → no operation ↳ existing DB stays as-is, NOT replaced. No data loss. Existing encrypted instance, customer set true previously: state.storageEncrypted = true new spec.storageEncrypted = true diff: trivially no change ↳ unchanged. Customer explicitly sets `storageEncrypted: false` in YAML: state.storageEncrypted = false (or whatever it was) new spec.storageEncrypted = false (lo.FromPtrOr(&false, true) = false) diff: IgnoreChanges → no change ↳ honoured; no second-guessing. Brand-new stack, customer never sets the field: no prior state — IgnoreChanges has no effect on initial create new spec.storageEncrypted = true ↳ AWS creates an encrypted instance. ✓ secure default. Out-of-band migration ===================== Customers who actually want to migrate an existing unencrypted RDS to encrypted still have to do it the AWS-immutable-field way: snapshot → encrypted-copy → restore → re-import. The IgnoreChanges silences SC's spec from triggering Pulumi to attempt that migration via destructive replacement. Documented on the struct field comments. Tests ===== `pkg/clouds/aws/rds_storage_encrypted_test.go` updated: - "omitted → nil (legacy default, encryption off)" → "omitted → nil (secure default, encryption on)" - resolution helper changed from `lo.FromPtr(cfg.StorageEncrypted)` to `lo.FromPtrOr(cfg.StorageEncrypted, true)` - expected value changed from `wantSet && wantVal` to `!wantSet || wantVal` - assertion message updated accordingly. Build, vet, and TestRead{Mysql,Postgres}Config_StorageEncrypted all pass locally. Companion: simple-container-com/actions PR #9 (open) tightens `go-aws-rds-no-storage-encryption` to whitelist only the `lo.FromPtrOr(_, true)` shape — bare `lo.FromPtr(*ptr)` and `lo.FromPtrOr(*ptr, false)` both still fire the rule, so this codebase will pass the rule going forward and any regression to the old shape gets flagged at PR time. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Semgrep Scan ResultsRepository:
Scanned at 2026-05-09 08:17 UTC |
Cre-eD
added a commit
to simple-container-com/actions
that referenced
this pull request
May 9, 2026
…ure default
Earlier draft of this PR whitelisted any `<ptr>.Bool(lo.FromPtr(*ptr))`
shape — but `lo.FromPtr(nil)` returns the type's zero value (false for
*bool), so that suppression accepted code that defaults to UNENCRYPTED
when a customer hasn't set the field. That permanently lowers the rule
below CIS-AWS RDS.3 and was the wrong call.
Replaced with a tighter whitelist that requires the literal-true
fallback:
StorageEncrypted:\s*\w+\.Bool\(lo\.FromPtrOr\([^()\n]+,\s*true\s*\)\)
What still fires:
- bare `StorageEncrypted: false` and `StorageEncrypted: Bool(false)`
- bare `lo.FromPtr(*ptr)` (defaults to false → unencrypted)
- `lo.FromPtrOr(*ptr, false)` (explicit insecure default)
- any other arbitrary wrapper around a runtime expression
What's suppressed:
- `StorageEncrypted: true` (literal)
- `<ptr>.Bool(true)` (canonical Pulumi truthy)
- `<ptr>.Bool(lo.FromPtrOr(*ptr, true))` (secure-by-default
runtime config)
The runtime-config form must be paired with
`IgnoreChanges([]string{"storageEncrypted"})` on the resource opts to
keep the default-flip safe for pre-existing unencrypted instances.
The pairing isn't regex-verified; the wrapper shape + literal-true
fallback are the design signal.
Tests
=====
`semgrep-scan/tests/go_cases.go` updated:
- Renamed positive cases to `rdsRuntimeSecure{Sdk,Pulumi}` (was
`rdsRuntimeOptIn{Sdk,Pulumi}`) and switched to the
`lo.FromPtrOr(_, true)` shape.
- Added `// ruleid:` for `lo.FromPtr(*ptr)` (bare-FromPtr-still-fires)
and `lo.FromPtrOr(*ptr, false)` (explicit-insecure-fallback).
- Kept arbitrary-wrapper guard.
Total go_cases.go: 35/35 passes; full ruleset 129/129 across all
fixtures.
Companion: simple-container-com/api#240 flips the call sites in
rds_{mysql,postgres}.go from `lo.FromPtr(*ptr)` to
`lo.FromPtrOr(*ptr, true)`, satisfying both this rule and CIS-AWS
RDS.3 going forward.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Security Scan ResultsRepository:
Scanned at 2026-05-09 08:18 UTC |
Contributor
Author
✅ Empirical validation passedRan Plan summary
The 2 replacements + 2 updates are unrelated cloudtrail-security-helpers `docker:index/remoteImage:RemoteImage` rebuilds (new SC version → new helper image digest). No RDS, RDS-cluster, EC2, VPC, or related resource appears in the diff. What this proves
Validation PR (closed, branch deleted): Integrail/devops#364 |
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.
Summary
PR #239 added the
StorageEncrypted *boolfield as opt-in (nil → false → unencrypted), preserving pre-2026.5 behaviour. That left every customer who hadn't read the new field's docs on unencrypted RDS, violating CIS-AWS Foundations RDS.3.This 1-line-each change flips the resolver from `lo.FromPtr(*ptr)` to `lo.FromPtrOr(*ptr, true)` at both call sites, making encryption the secure default while preserving every existing customer's data.
nil(omitted from YAML)&true(explicit)&false(explicit)Why existing DBs are safe — IgnoreChanges already in place
PR #239 added `sdk.IgnoreChanges([]string{"storageEncrypted"})` on the resource opts. Pulumi's IgnoreChanges semantics: "treat the desired value as the recorded state value for the diff."
falsetrue(default flip)truepreviouslytruetruefalsein YAMLfalsetrueCustomers who genuinely want to migrate an existing unencrypted RDS to encrypted still need the out-of-band path AWS forces (snapshot → encrypted-copy → restore → re-import). The IgnoreChanges deliberately blocks SC from attempting that migration via destructive replacement.
Tests
pkg/clouds/aws/rds_storage_encrypted_test.goupdated:Locally: `go build ./...`, `go vet ./...`, `TestReadRds{Mysql,Postgres}Config_StorageEncrypted` all pass.
Companion
simple-container-com/actions#9 tightens the `go-aws-rds-no-storage-encryption` Semgrep rule to whitelist only the `lo.FromPtrOr(_, true)` shape — bare `lo.FromPtr(*ptr)` and `lo.FromPtrOr(*ptr, false)` both still fire the rule.
After both land:
Test plan
go-aws-rds-no-storage-encryptionpulumi previewshows no destructive replacement on the existing unencrypted RDS instances