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
changed the title
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