Skip to content

Name the concrete removal trigger for the openpgp exclusion - #6286

Merged
samuv merged 2 commits into
mainfrom
docs/openpgp-exclusion-trigger
Aug 19, 2026
Merged

Name the concrete removal trigger for the openpgp exclusion#6286
samuv merged 2 commits into
mainfrom
docs/openpgp-exclusion-trigger

Conversation

@samuv

@samuv samuv commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

security-scan.yml excludes GO-2026-5932 (deprecated golang.org/x/crypto/openpgp) with the note "Remove when sigstore drops the openpgp dependency." That is not a condition anyone can check, so the entry would outlive its cause — the usual fate of a suppression with a vague expiry.

It turns out the condition is already half met, and is now nameable: rekor migrated to ProtonMail/go-crypto/openpgp in sigstore/rekor#2883, merged 2026-07-15. That landed after v1.5.3 (2026-07-02), so it is not in any release yet. The trigger becomes "bump rekor past v1.5.3 once a release contains it", which someone can actually evaluate.

While documenting it I traced where openpgp enters, which the old note left vague:

pkg/skills/signer → sigstore-go/pkg/sign → rekor/pkg/pki → rekor/pkg/pki/pgp → x/crypto/openpgp

rekor/pkg/pki is a pluggable signature-format registry, so importing it links every format, PGP included. Worth recording because it explains why this cannot be fixed locally — there is no openpgp call in ToolHive to rewrite.

Also recorded: every govulncheck trace is package-init reachability (signer.init calls sign.init, which eventually calls armor.init), not a call. The code is linked, never invoked; nothing on any ToolHive path parses PGP data. That distinction is what makes the exclusion defensible rather than merely convenient, and it was missing.

Comment-only change — no behaviour, no logic.

Type of change

  • Documentation

Test plan

  • Manual testing (describe below)

YAML re-parsed after editing (yaml.safe_load) to confirm the block comment did not break the workflow. No executable lines changed — IGNORED_VULNS is byte-identical.

Special notes for reviewers

Prompted by a reviewer question on a related PR: should we use ProtonMail's gopenpgp instead of accepting the vuln? Worth recording the answer, since it will come up again:

  • We cannot. Nothing here imports openpgp; the import is four levels up in rekor.
  • gopenpgp would be the wrong library anyway. ProtonMail/go-crypto/openpgp is the API-compatible fork and the official migration path (what rekor chose). ProtonMail/gopenpgp/v3 is a higher-level wrapper with a different API, built on top of go-crypto.
  • A replace directive does not work either. It would have to be golang.org/x/crypto => ProtonMail/go-crypto, which swaps the entire x/crypto module while go-crypto only carries the openpgp subtree — chacha20, ssh, and the rest vanish and the build breaks.

The same justification is applied to stacklok/toolhive-core in stacklok/toolhive-core#230, which consumes the same sigstore packages.

Generated with Claude Code

@samuv
samuv requested a review from JAORMX as a code owner August 12, 2026 10:39
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Aug 12, 2026
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.03%. Comparing base (7af27d3) to head (dff06fa).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6286      +/-   ##
==========================================
- Coverage   73.08%   73.03%   -0.05%     
==========================================
  Files         745      745              
  Lines       78804    78804              
==========================================
- Hits        57597    57558      -39     
- Misses      17177    17232      +55     
+ Partials     4030     4014      -16     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 12, 2026
@samuv samuv self-assigned this Aug 12, 2026
samuv added 2 commits August 19, 2026 15:48
"Remove when sigstore drops the dependency" is not something anyone can
check, so the entry would outlive its cause. Rekor already made the
change — sigstore/rekor#2883, merged after v1.5.3 — so the trigger is a
release containing it.

Also record the import chain and that every govulncheck trace is
package-init reachability rather than a call, which is the part that
makes the exclusion defensible.
The trailing `|| true` covered the whole pipeline, so a jq parse error, an
empty output file, or a missing one all produced an empty finding list —
indistinguishable from a clean scan, and the gate reported success.

Verified against malformed, empty, and missing output: all three passed
before, all three now fail. A genuinely clean scan and a real unexcluded
finding both keep their previous verdicts.

Found in review of stacklok/toolhive-core#230, whose equivalent step was
modelled on this one.
@samuv
samuv force-pushed the docs/openpgp-exclusion-trigger branch from 5ecb6a9 to dff06fa Compare August 19, 2026 13:48
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 19, 2026
@samuv
samuv merged commit 0b8acf2 into main Aug 19, 2026
54 of 55 checks passed
@samuv
samuv deleted the docs/openpgp-exclusion-trigger branch August 19, 2026 15:39
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants