Fix cosign + gh-attestation rules — line-anchor + shell-continuation aware#11
Merged
Merged
Conversation
…tinuation The line-scoped `pattern-not-regex` design (shipped in #10) misfired on real consumer workflows in two ways: 1. **Shell line-continuation invisibility.** Real cosign commands span multiple lines via trailing backslash: cosign verify "$img" \ --certificate-identity-regexp "$IDENTITY_REGEX" \ --certificate-oidc-issuer "$OIDC_ISSUER" The line-scoped not-regex only checked the `cosign verify` line, not the continuation, so every multi-line invocation tripped both `gha-cosign-verify-without-identity-flag` and `gha-cosign-verify-without-oidc-issuer-flag`. 2. **Comment / error-string false matches.** Patterns like echo "::warning title=cosign verify failed::$img" # `cosign verify-attestation --type cyclonedx` contain the literal `cosign verify` substring but are not actual commands. The bare regex `\bcosign\s+verify\b` matched them. Rewrite as a single multi-line regex per rule with: - command-start anchor: `(?:^[ \t]*|run:[ \t]+)cosign\s+verify…` so the match must sit at the beginning of an (indented) line or directly after `run:` (single-line step form). - shell-continuation-aware negative lookahead: `(?!(?:[^\n]*\\\s*\n)*?[^\n]*<required-flag>)` Greedily walks any trailing-backslash continuation lines and checks the entire shell command body for the flag. Empirical validation on the api repo's 4 attestation-bearing workflows (verify-attestations.yml, push.yaml, build-staging.yml, branch-preview.yaml): 30 → 0 false positives. The api repo CI gate that went red after #10 merged will go green on the next pull_request / workflow_dispatch. Fixtures already cover the command-start anchor path (`- run: cosign verify …`) — semgrep-scan/run-tests.sh still 0 findings full-repo, 9/9 on sigstore-cases.yml. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Security Scan ResultsRepository:
Scanned at 2026-05-16 17:07 UTC |
Semgrep Scan ResultsRepository:
Scanned at 2026-05-16 17:08 UTC |
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.
Problem
After #10 merged, the api repo's Semgrep CI gate went red with 40 ERROR-severity findings — all false positives from three of the new rules:
gha-cosign-verify-without-identity-flag× 15gha-cosign-verify-without-oidc-issuer-flag× 15gha-gh-attestation-missing-scope× 10Two failure modes, both rooted in
pattern-not-regexbeing line-scoped:Shell line-continuation invisibility. Real cosign commands span multiple lines via trailing
\:The line-scoped not-regex only checked the
cosign verifyline itself, missing the required flags on continuation lines.Comment / error-string false matches. Patterns like
echo "::warning title=cosign verify failed::$img"and comment text mentioningcosign verify-attestationcontain the literal substring but are not real commands.Fix
Rewrite each affected rule as a single multi-line regex with:
(?:^[ \t]*|run:[ \t]+)run:(single-line step form). Filters out error strings, comments, and inline backtick mentions.(?!(?:[^\n]*\\\s*\n)*?[^\n]*<flag>)Validation
verify-attestations.ymlpush.yamlbuild-staging.ymlbranch-preview.yamlsemgrep-scan/run-tests.shfull-reposigstore-cases.ymlfixtureTest plan
bash semgrep-scan/run-tests.sh— all fixtures pass, full-repo scan 0 findingsmain(it went red after Add sigstore + gha-extras + pulumi-iac + go-canon Semgrep rule packs #10 merged) — expect green