ci: disable two over-broad Semgrep registry rules with replacement coverage#291
Merged
Merged
Conversation
…verage
Cuts the 10 WARNING findings the Semgrep job posts on every PR by
disabling two registry rules whose threat models don't apply to SC
code. Reachability-aware replacement coverage lives in the shared
`go-canon.yml` ruleset:
- `go.lang.security.audit.crypto.use_of_weak_crypto.use-of-md5`:
fires on every md5.Sum() regardless of context. All 7 SC call
sites are non-cryptographic fingerprints (S3-etag-style upload
checksums, K8s annotation change-hashes, dedup suffix for Pulumi
resource names on duplicate-name DNS records). Real MD5-in-crypto
use is now caught by the narrower `go-md5-in-crypto-context`
rule shipped in actions#23.
- `go.lang.security.deserialization.unsafe-deserialization-interface
.go-unsafe-deserialization-interface`: calibrated to Python
pickle/yaml.load (which executes code on unmarshal). Go's yaml.v3
and encoding/json do not execute on unmarshal; threat model
doesn't transfer. The 3 SC call sites are credential
obfuscators (commands_project.go, secure_file_reader.go) that
walk untyped YAML specifically to redact secrets BEFORE logging.
Bumps the SC actions reusable-workflow pin to the head commit of
chore/semgrep-disable-rules-and-md5-refinement (actions#23, which
adds the `disabled-rules` input). Pin must be bumped to the
main-merge SHA before this PR merges.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Semgrep Scan ResultsRepository:
Scanned at 2026-05-24 16:40 UTC |
Security Scan ResultsRepository:
Scanned at 2026-05-24 16:41 UTC |
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
smecsia
approved these changes
May 24, 2026
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
2 participants
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
Cuts the 10 Semgrep WARNING findings the job has been posting on every PR (and on #289) by disabling two registry-pack rules whose threat models don't apply to SC code. Reachability-aware replacement coverage ships in actions#23.
Findings being silenced
All 10 are pre-existing intentional patterns:
MD5-as-fingerprint (7) —
use-of-md5fires on everymd5.Sum()regardless of context. SC call sites are:yaml-into-interface{} (3) —
unsafe-deserialization-interfaceis calibrated to Python pickle/yaml.load (executes on unmarshal). Go'syaml.v3/encoding/jsondon't execute. SC call sites:The SC team already documented this exact reasoning in
go-canon.ymlround-6 triage, where they dropped their own equivalent yaml-unmarshal rule.What still catches the real threats
Disabling these two rules does NOT lose coverage; the genuine patterns are caught by narrower rules in
go-canon.yml:go-md5-in-crypto-contextrule (actions#23) fires onhmac.New(md5.New, ...)and on md5 result assigned to a variable namedmac/hmac/signature/tag/auth/token/password/digest.p/golangregistry pack remains enabled. Only the generic shape-only rule is disabled.Dependencies
⚠ Pinned to a branch SHA on actions. The reusable-workflow ref currently points at
2a7604ddac4fd9f912a1ab6896ac87c5b07bc537(the head of actions#23). Before merging this PR, bump the pin to the main-merge SHA that actions#23 produces.Test plan
go-md5-in-crypto-contextstill fires if someone writeshmac.New(md5.New, key)— verified in actions#23 fixtures (8 new cases)