From ae50e1480640d49f4c06fb50c0c3337f68204166 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 9 May 2026 00:59:04 +0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(semgrep):=20go-aws-rds-no-storage-encry?= =?UTF-8?q?ption=20=E2=80=94=20accept=20lo.FromPtr=20runtime=20opt-in=20sh?= =?UTF-8?q?ape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `.Bool(lo.FromPtr())` 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()` 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. --- semgrep-scan/rules/go.yml | 22 ++++++++++----- semgrep-scan/tests/go_cases.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/semgrep-scan/rules/go.yml b/semgrep-scan/rules/go.yml index 92ef708..61a6e1d 100644 --- a/semgrep-scan/rules/go.yml +++ b/semgrep-scan/rules/go.yml @@ -204,16 +204,24 @@ rules: - pattern-either: - pattern: rds.NewInstance(...) - pattern: rds.NewCluster(...) - # Suppress only when `StorageEncrypted` is explicitly set to a - # truthy value. Two forms are accepted: + # Suppress when `StorageEncrypted` is set in a way that is NOT a + # static-disable. Three accepted shapes: # * `StorageEncrypted: pulumi.Bool(true)` / `sdk.Bool(true)` / - # anything that ends `.Bool(true)` (the canonical Pulumi shape) - # * bare `StorageEncrypted: true` (less idiomatic but valid) - # Important: setting the field to `false` (or `pulumi.Bool(false)`, - # or any non-true value) does NOT suppress — we want to flag those - # explicitly-disabled cases, not let them through. + # any `.Bool(true)` — the canonical Pulumi truthy shape. + # * bare `StorageEncrypted: true`. + # * `StorageEncrypted: .Bool(lo.FromPtr(<*bool expr>))` — + # a runtime-config opt-in: value is config-driven (nil → + # legacy default, explicit value otherwise) and 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. The pairing isn't verified in regex; the + # wrapper shape is the design signal. + # Setting the field to `false` (literal) or `Bool(false)` still + # fires — those are explicit disables. - pattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(true\)' - pattern-not-regex: 'StorageEncrypted:\s*true\b' + - pattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(lo\.FromPtr\([^()\n]+\)\)' paths: include: - "**/*.go" diff --git a/semgrep-scan/tests/go_cases.go b/semgrep-scan/tests/go_cases.go index 3c9337d..2291f88 100644 --- a/semgrep-scan/tests/go_cases.go +++ b/semgrep-scan/tests/go_cases.go @@ -219,6 +219,57 @@ func rdsExplicitlyDisabledBool(ctx *pulumiCtx) { }) } +// Runtime-config opt-in shape: `.Bool(lo.FromPtr(<*bool>))`. +// Used in simple-container-com/api for legacy-safe migration; paired +// with `IgnoreChanges([]string{"storageEncrypted"})` on the opts (not +// shown — pairing isn't regex-verified). Must NOT fire. +type pulumiSdk struct{} + +func (pulumiSdk) Bool(_ bool) bool { return false } + +var sdk = pulumiSdk{} +var pulumi = pulumiSdk{} + +type loPkg struct{} + +func (loPkg) FromPtr(_ *bool) bool { return false } + +var lo = loPkg{} + +type dbConfigShape struct{ StorageEncrypted *bool } + +func rdsRuntimeOptInSdk(ctx *pulumiCtx) { + cfg := dbConfigShape{} + // ok: go-aws-rds-no-storage-encryption + _ = rds.NewInstance(ctx, "db4", &instanceArgs{ + Engine: "postgres", + StorageEncrypted: sdk.Bool(lo.FromPtr(cfg.StorageEncrypted)), + }) +} + +// Same shape with `pulumi.Bool` prefix — must also NOT fire. +func rdsRuntimeOptInPulumi(ctx *pulumiCtx) { + cfg := dbConfigShape{} + // ok: go-aws-rds-no-storage-encryption + _ = rds.NewCluster(ctx, "cluster3", &clusterArgs{ + Engine: "aurora-postgresql", + StorageEncrypted: pulumi.Bool(lo.FromPtr(cfg.StorageEncrypted)), + }) +} + +// Negative guard: a `.Bool()` that ISN'T +// `lo.FromPtr(...)` must not bypass the rule (we only carve out the +// specific runtime-config helper, not any wrapper). Must FIRE. +func rdsArbitraryWrapperStillFires(ctx *pulumiCtx) { + // ruleid: go-aws-rds-no-storage-encryption + _ = rds.NewInstance(ctx, "db5", &instanceArgs{ + Engine: "postgres", + StorageEncrypted: sdk.Bool(someRuntimeBool()), + }) +} + +func someRuntimeBool() bool { return false } + // -------------------------------------------------------------------- // go-fmt-errorf-percent-v-for-error // -------------------------------------------------------------------- From 20f8d19f258896f607f4fc43ef37d2eb11d48e44 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 9 May 2026 12:17:47 +0400 Subject: [PATCH 2/2] fix(semgrep): tighten go-aws-rds-no-storage-encryption to require secure default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Earlier draft of this PR whitelisted any `.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) - `.Bool(true)` (canonical Pulumi truthy) - `.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 --- semgrep-scan/rules/go.yml | 27 ++++++++++-------- semgrep-scan/tests/go_cases.go | 51 +++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/semgrep-scan/rules/go.yml b/semgrep-scan/rules/go.yml index 61a6e1d..698af54 100644 --- a/semgrep-scan/rules/go.yml +++ b/semgrep-scan/rules/go.yml @@ -205,23 +205,26 @@ rules: - pattern: rds.NewInstance(...) - pattern: rds.NewCluster(...) # Suppress when `StorageEncrypted` is set in a way that is NOT a - # static-disable. Three accepted shapes: + # static-disable AND is secure-by-default. Three accepted shapes: # * `StorageEncrypted: pulumi.Bool(true)` / `sdk.Bool(true)` / # any `.Bool(true)` — the canonical Pulumi truthy shape. # * bare `StorageEncrypted: true`. - # * `StorageEncrypted: .Bool(lo.FromPtr(<*bool expr>))` — - # a runtime-config opt-in: value is config-driven (nil → - # legacy default, explicit value otherwise) and 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. The pairing isn't verified in regex; the - # wrapper shape is the design signal. - # Setting the field to `false` (literal) or `Bool(false)` still - # fires — those are explicit disables. + # * `StorageEncrypted: .Bool(lo.FromPtrOr(<*bool>, true))` + # — runtime-config with secure default. The literal `true` + # fallback is required: `lo.FromPtr(*ptr)` (nil → false → DB + # unencrypted-by-default) and `lo.FromPtrOr(*ptr, false)` + # both still fire. Pair this shape with + # `IgnoreChanges([]string{"storageEncrypted"})` on the + # resource opts so the default flip doesn't propose a + # destructive replacement on existing unencrypted instances + # (the pairing isn't verified in regex; the wrapper shape + + # literal-true fallback are the design signal). + # Setting the field to `false` (literal), `Bool(false)`, or + # `lo.FromPtr(*bool)` (= `lo.FromPtrOr(*bool, false)`) still + # fires — those default to unencrypted. - pattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(true\)' - pattern-not-regex: 'StorageEncrypted:\s*true\b' - - pattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(lo\.FromPtr\([^()\n]+\)\)' + - pattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(lo\.FromPtrOr\([^()\n]+,\s*true\s*\)\)' paths: include: - "**/*.go" diff --git a/semgrep-scan/tests/go_cases.go b/semgrep-scan/tests/go_cases.go index 2291f88..4361268 100644 --- a/semgrep-scan/tests/go_cases.go +++ b/semgrep-scan/tests/go_cases.go @@ -219,10 +219,12 @@ func rdsExplicitlyDisabledBool(ctx *pulumiCtx) { }) } -// Runtime-config opt-in shape: `.Bool(lo.FromPtr(<*bool>))`. -// Used in simple-container-com/api for legacy-safe migration; paired -// with `IgnoreChanges([]string{"storageEncrypted"})` on the opts (not -// shown — pairing isn't regex-verified). Must NOT fire. +// Runtime-config secure-by-default shape: +// `.Bool(lo.FromPtrOr(<*bool>, true))`. Used in +// simple-container-com/api: nil → true (encrypted), explicit value +// otherwise. Paired with `IgnoreChanges([]string{"storageEncrypted"})` +// on the resource opts (not shown — pairing isn't regex-verified). +// Must NOT fire. type pulumiSdk struct{} func (pulumiSdk) Bool(_ bool) bool { return false } @@ -232,37 +234,60 @@ var pulumi = pulumiSdk{} type loPkg struct{} -func (loPkg) FromPtr(_ *bool) bool { return false } +func (loPkg) FromPtr(_ *bool) bool { return false } +func (loPkg) FromPtrOr(_ *bool, _ bool) bool { return false } var lo = loPkg{} type dbConfigShape struct{ StorageEncrypted *bool } -func rdsRuntimeOptInSdk(ctx *pulumiCtx) { +func rdsRuntimeSecureSdk(ctx *pulumiCtx) { cfg := dbConfigShape{} // ok: go-aws-rds-no-storage-encryption _ = rds.NewInstance(ctx, "db4", &instanceArgs{ Engine: "postgres", - StorageEncrypted: sdk.Bool(lo.FromPtr(cfg.StorageEncrypted)), + StorageEncrypted: sdk.Bool(lo.FromPtrOr(cfg.StorageEncrypted, true)), }) } // Same shape with `pulumi.Bool` prefix — must also NOT fire. -func rdsRuntimeOptInPulumi(ctx *pulumiCtx) { +func rdsRuntimeSecurePulumi(ctx *pulumiCtx) { cfg := dbConfigShape{} // ok: go-aws-rds-no-storage-encryption _ = rds.NewCluster(ctx, "cluster3", &clusterArgs{ Engine: "aurora-postgresql", - StorageEncrypted: pulumi.Bool(lo.FromPtr(cfg.StorageEncrypted)), + StorageEncrypted: pulumi.Bool(lo.FromPtrOr(cfg.StorageEncrypted, true)), }) } -// Negative guard: a `.Bool()` that ISN'T -// `lo.FromPtr(...)` must not bypass the rule (we only carve out the -// specific runtime-config helper, not any wrapper). Must FIRE. -func rdsArbitraryWrapperStillFires(ctx *pulumiCtx) { +// Bare `lo.FromPtr(*ptr)` defaults to false when nil — DB ends up +// unencrypted-by-default. Must STILL FIRE. +func rdsBareFromPtrStillFires(ctx *pulumiCtx) { + cfg := dbConfigShape{} // ruleid: go-aws-rds-no-storage-encryption _ = rds.NewInstance(ctx, "db5", &instanceArgs{ + Engine: "postgres", + StorageEncrypted: sdk.Bool(lo.FromPtr(cfg.StorageEncrypted)), + }) +} + +// `lo.FromPtrOr(*ptr, false)` is just bare-FromPtr in disguise — the +// fallback is unencrypted, so the DB defaults to unencrypted. Must +// STILL FIRE. +func rdsFromPtrOrFalseStillFires(ctx *pulumiCtx) { + cfg := dbConfigShape{} + // ruleid: go-aws-rds-no-storage-encryption + _ = rds.NewInstance(ctx, "db6", &instanceArgs{ + Engine: "postgres", + StorageEncrypted: sdk.Bool(lo.FromPtrOr(cfg.StorageEncrypted, false)), + }) +} + +// Arbitrary wrapper that isn't the recognized helper — rule must +// FIRE (we don't trust unknown runtime expressions). +func rdsArbitraryWrapperStillFires(ctx *pulumiCtx) { + // ruleid: go-aws-rds-no-storage-encryption + _ = rds.NewInstance(ctx, "db7", &instanceArgs{ Engine: "postgres", StorageEncrypted: sdk.Bool(someRuntimeBool()), })