fix(semgrep): go-aws-rds-no-storage-encryption — require secure-by-default lo.FromPtrOr(_, true) - #9
Merged
Cre-eD merged 2 commits intoMay 10, 2026
Conversation
…ntime opt-in shape
PR #238 in simple-container-com/api flagged this rule as a false-
positive on two sites in pkg/clouds/pulumi/aws/rds_{mysql,postgres}.go,
both using a runtime-config opt-in pattern:
StorageEncrypted: sdk.Bool(lo.FromPtr(dbConfig.StorageEncrypted))
`lo.FromPtr(*bool)` returns false for nil (preserves legacy default)
or the explicit value otherwise. Each call site is paired with
`IgnoreChanges([]string{"storageEncrypted"})` on the resource opts so
flipping the bit later doesn't trigger destructive replacement of
pre-existing unencrypted instances — a deliberate migration-safe
design that the rule's strict CIS-RDS.3 reading didn't account for.
Fix: add a third `pattern-not-regex` that suppresses the specific
`<ptr>.Bool(lo.FromPtr(<expr>))` shape. The regex is narrow:
StorageEncrypted:\s*\w+\.Bool\(lo\.FromPtr\([^()\n]+\)\)
- Requires `lo.FromPtr(...)` literal, not arbitrary wrappers, so a
developer wrapping in some other helper still fires the rule.
- Single-line via `[^()\n]+` so multi-line shenanigans don't
accidentally bypass.
- Does NOT verify the IgnoreChanges pairing in regex; the wrapper
shape itself is the design signal.
The literal-`false` and `Bool(false)` cases continue to fire (covered
by existing fixtures + a new explicit guard test that uses
`sdk.Bool(<arbitrary expr>)` to confirm only the lo.FromPtr shape is
suppressed, not any wrapper).
Tests: 33/33 in go_cases.go (was 30) — added two `// ok:` cases for
the runtime-helper shape (sdk.Bool / pulumi.Bool variants) and one
`// ruleid:` guard for the arbitrary-wrapper case. `bash
semgrep-scan/run-tests.sh` is green (127/127 across all rule files).
Verified: 0 findings of `go-aws-rds-no-storage-encryption` against
simple-container-com/api/pkg/clouds/pulumi/aws/ after this change.
Security Scan ResultsRepository:
Scanned at 2026-05-09 08:18 UTC |
Semgrep Scan ResultsRepository:
Scanned at 2026-05-09 08:18 UTC |
5 tasks
…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>
Cre-eD
added a commit
to simple-container-com/api
that referenced
this pull request
May 10, 2026
## Summary PR #239 added the `StorageEncrypted *bool` field 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. | State | Before | After | |---|---|---| | `nil` (omitted from YAML) | false (unencrypted) | **true** (encrypted) | | `&true` (explicit) | true | true | | `&false` (explicit) | false | false | ## 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."* | Scenario | State | New spec | Diff | Outcome | |---|---|---|---|---| | Existing unencrypted, never set field | `false` | `true` (default flip) | IgnoreChanges silences | **No replacement, no data loss** | | Existing encrypted, set `true` previously | `true` | `true` | no change | unchanged | | Customer explicitly sets `false` in YAML | * | `false` | IgnoreChanges silences | honoured, no second-guessing | | Brand-new stack | none | `true` | initial create | encrypted ✓ | Customers 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.go` updated: - omit-case description: \"legacy default, encryption off\" → \"secure default, encryption on\" - resolver: \`lo.FromPtr(cfg.StorageEncrypted)\` → \`lo.FromPtrOr(cfg.StorageEncrypted, true)\` - expected: \`wantSet && wantVal\` → \`!wantSet || wantVal\` Locally: \`go build ./...\`, \`go vet ./...\`, \`TestReadRds{Mysql,Postgres}Config_StorageEncrypted\` all pass. ## Companion [simple-container-com/actions#9](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: - This api PR's Semgrep status passes (post-fix code uses the whitelisted shape). - Any regression to bare \`lo.FromPtr(*bool)\` on a security-critical default gets flagged at PR time across SC + every consumer repo using the ruleset. ## Test plan - [x] go build ./... + go vet ./... locally - [x] TestReadRds{Mysql,Postgres}Config_StorageEncrypted green - [ ] CI green - [ ] (after actions#9 merges) re-scan: 0 findings of `go-aws-rds-no-storage-encryption` - [ ] Validate via SC preview build against an existing PAY-SPACE staging stack — confirm `pulumi preview` shows no destructive replacement on the existing unencrypted RDS instances Signed-off-by: Dmitrii Creed <creeed22@gmail.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.
Context
Surfaced as a false positive in simple-container-com/api#238. Two sites:
pkg/clouds/pulumi/aws/rds_mysql.go:137—StorageEncrypted: sdk.Bool(lo.FromPtr(dbConfig.StorageEncrypted))pkg/clouds/pulumi/aws/rds_postgres.go:121—StorageEncrypted: sdk.Bool(lo.FromPtr(postgresCfg.StorageEncrypted))Both intentionally use a runtime-config opt-in pattern paired with
IgnoreChanges([]string{"storageEncrypted"})on the resource opts — a migration-safe design that the rule's previous strict-literal-only suppression (Bool(true)/true) didn't account for.Fix
Add a third
pattern-not-regexaccepting the specific<ptr>.Bool(lo.FromPtr(<expr>))shape:Narrow on purpose:
lo.FromPtr(...)literal — arbitrary wrappers (e.g.sdk.Bool(someFn())) still fire the rule.[^()\n]+— no multi-line bypass.IgnoreChangespairing in regex; the wrapper shape itself is the design signal. Documented in the inline comment.Tests
semgrep-scan/tests/go_cases.goadds 3 cases:// ok:—sdk.Bool(lo.FromPtr(cfg.X))— must NOT fire// ok:—pulumi.Bool(lo.FromPtr(cfg.X))— must NOT fire (different prefix)// ruleid:—sdk.Bool(someRuntimeBool())— must STILL fire (arbitrary wrapper, nolo.FromPtr)Existing fixtures (literal
true, literalfalse, omit, andBool(false)) all continue to behave as before.Real-world verification
After this change, scanning
simple-container-com/api/pkg/clouds/pulumi/aws/produces 0 findings forgo-aws-rds-no-storage-encryption— both former FPs cleared, no regression on the literal-disable / omit cases that exist elsewhere in the same package.Test plan
bash semgrep-scan/run-tests.shgreen locallysemgrep-self-testworkflow passes on this branch