fix: replace unmaintained golang.org/x/crypto/openpgp with ProtonMail fork - #58
Open
jmrplens wants to merge 1 commit into
Open
fix: replace unmaintained golang.org/x/crypto/openpgp with ProtonMail fork#58jmrplens wants to merge 1 commit into
jmrplens wants to merge 1 commit into
Conversation
… fork golang.org/x/crypto/openpgp is frozen and deprecated (golang/go#44226) and carries advisory GO-2026-5932, which has no fixed version: the advisory covers every release of the module (introduced: 0, Fixed in: N/A) because the finding is that the package is unmaintained and unsafe by design, not a specific bug. validate.go imports it unconditionally for PGPValidator, so the package is linked into every binary that imports go-selfupdate, and every downstream consumer inherits the advisory even when it configures a checksum or ECDSA validator and never verifies a PGP signature. Switch to github.com/ProtonMail/go-crypto/openpgp, the maintained fork that both the advisory and the x/crypto/openpgp package documentation recommend as a drop-in replacement. The fork's CheckDetachedSignature and CheckArmoredDetachedSignature take an additional *packet.Config; nil selects the library defaults, matching the behaviour of the functions they replace. Also bump github.com/cloudflare/circl to v1.6.3. ProtonMail/go-crypto v1.4.1 resolves circl v1.6.2, which is affected by GO-2026-4550 (incorrect secp384r1 CombinedMult calculation), so without this the change would trade one advisory for another. Verified: - Full test suite passes on all packages. - golangci-lint run ./... reports 0 issues. - Signatures produced by golang.org/x/crypto/openpgp still verify through the migrated PGPValidator, and tampered payloads are still rejected, so existing signed release assets keep validating. - A consumer that previously reported GO-2026-5932 through this module now reports no vulnerabilities. The TestPGPValidator/Fail assertion is relaxed from the exact string "openpgp: invalid signature: hash tag doesn't match" to the stable "openpgp: invalid signature" prefix. Both libraries reject the signature; they differ only in the reason reported after that prefix, which is an implementation detail of the openpgp library rather than part of this package's contract. BREAKING CHANGE: PGPValidator.KeyRing is typed openpgp.EntityList, and that type now comes from a different package. Code that builds a key ring with golang.org/x/crypto/openpgp and assigns it to KeyRing directly no longer compiles and must import the ProtonMail package instead. Code using WithArmoredKeyRing([]byte) is unaffected. This suggests a minor version bump.
21 tasks
jmrplens
added a commit
to jmrplens/gitlab-mcp-server
that referenced
this pull request
Jul 25, 2026
#245) ## Description Routine dependency refresh across the three ecosystems: Go modules, the pnpm docs site, and the Node/pnpm toolchain. GitHub Actions were audited and already sit at their latest releases, so no workflow action pins changed. It also corrects the recorded justification for the one accepted `govulncheck` advisory, which did not match what the code actually does. ## Related Issue N/A — routine maintenance. ## Type of Change - [x] Enhancement (improvement to existing functionality) - [x] Documentation update - [x] CI / build / tooling ## Changes Made **Go modules** — all direct dependencies and the Go toolchain (1.26.5) were already current. Two indirect bumps from `go get -u ./...` + `go mod tidy`: - `github.com/mattn/go-runewidth` v0.0.24 → v0.0.27 - `github.com/ulikunitz/xz` v0.5.15 → v0.5.16 **pnpm (`site/`)** - `@astrojs/starlight` ^0.41.3 → ^0.41.4 - `eslint` ^10.7.0 → ^10.8.0 - `prettier` ^3.9.5 → ^3.9.6 - `packageManager` pnpm@11.8.0 → pnpm@11.17.0 **TypeScript deliberately held at ^6.0.3.** 7.0.2 is available but not installable: `@astrojs/check` declares a peer of `typescript: "^5.0.0 || ^6.0.0"`, and `@typescript-eslint/*` declares `">=4.8.4 <6.1.0"`. **Node 22 → 24** - `engines.node` `>=22.22.0` → `>=24.18.0` - `node-version` in `ci.yml` (×2) and `pages.yml` `22.22.0` → `24.18.0` Verified compatible before bumping: Astro requires `>=22.12.0`, `sharp` requires `>=20.9.0`, and the strictest transitive constraint in the lockfile is `^22.22.0 || >=24.8.0`. **GitHub Actions** — no changes. All 19 actions are pinned to a major tag that already resolves to the newest release, and the two exact pins (`hadolint/hadolint-action@v3.3.0`, `sigstore/cosign-installer@v4.1.2`) match the latest published versions. **Corrected GO-2026-5932 justification** in `scripts/govulncheck.sh` and `docs/development/static-analysis.md`. The previous text claimed `x/crypto/openpgp` was reached by "GPG verification of our own signed releases". That is not accurate: - `internal/autoupdate` configures `selfupdate.ChecksumValidator`, never a PGP validator, so no openpgp function is ever called. - Releases are signed with cosign/sigstore (`checksums.txt.sigstore.json`). No `.asc` artifact is produced and we do not sign with GPG at all. - Every `govulncheck` trace is a package `init()` call. Reachability is linkage only, caused by `go-selfupdate`'s `validate.go` importing the package unconditionally for its `PGPValidator` type. The entry stays on the allowlist — the advisory covers every version of `x/crypto` (`introduced: 0`, `Fixed in: N/A`), so no bump can clear it — but the reasoning now matches reality, and the two real remedies are recorded. Upstream follow-up: [creativeprojects/go-selfupdate#57](creativeprojects/go-selfupdate#57) and [#58](creativeprojects/go-selfupdate#58) migrate the library to the maintained `ProtonMail/go-crypto` fork. Verified locally against a patched build that this makes our `govulncheck` report `No vulnerabilities found.`, so the allowlist entry can be retired once that lands and a release ships. ## How to Test 1. `go build ./... && go test ./internal/... -count=1` 2. `make govulncheck` — should pass with `only accepted advisories present: GO-2026-5932` 3. `cd site && pnpm install && pnpm run build && pnpm run lint` 4. `go run ./cmd/format_md_tables/ --check` ## Breaking Changes / Migration Notes Local development now requires **Node >= 24.18.0** for the docs site (`site/`). The Go server and its build are unaffected. ## Checklist ### Code Quality - [x] Code compiles: `go build ./...` - [x] Consolidated Go analysis passes on changed packages (or `make golangci-lint` / `make analyze`) - [x] Code is formatted: `make analyze-fix` - [x] Follows idiomatic Go patterns and the conventions documented in `CLAUDE.md` / `.github/instructions/` ### Testing - [x] All existing tests pass: `go test ./... -count=1` - [ ] New tests added for new functionality — N/A, no functional change - [ ] Coverage on modified packages is ≥ 90% — N/A, no Go source changed - [ ] Edge cases and error scenarios covered — N/A - [ ] If applicable, E2E tests updated/added under `test/e2e/suite/` — N/A ### Documentation - [x] Doc comments added for exported types/functions (godoc) — N/A, no Go source changed - [x] `docs/` updated if public API or behavior changed - [ ] `README.md` / Starlight site updated if user-facing behavior changed — N/A, no user-facing behavior change - [ ] If introducing/removing a tool — N/A - [x] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) ### Security - [x] No secrets, tokens, or credentials in code, tests, fixtures, or logs - [x] Input validation for user-provided parameters — N/A, no input handling changed - [x] Error messages do not leak sensitive information — N/A - [x] No new dependencies with known vulnerabilities (verify with `govulncheck`) ## Screenshots / Logs `make govulncheck`: ``` govulncheck: PASS — only accepted advisories present: GO-2026-5932 (see docs/development/static-analysis.md) ``` `pnpm run lint` (site): ``` astro check Result (4 files): 0 errors, 0 warnings, 0 hints i18n parity OK: 30 EN pages each have an ES translation. eslint . prettier --check . All matched files use Prettier code style! html-validate 'dist/**/*.html' htmlhint 'dist/**/*.html' Scanned 61 files, no errors found ``` `pnpm run build` (site): 61 pages built, all internal links valid. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/jmrplens/gitlab-mcp-server/pull/245?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
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
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.
Fixes #57.
Problem
validate.goimportsgolang.org/x/crypto/openpgpat file scope for thePGPValidator.KeyRingfield. That package is frozen and deprecated (golang/go#44226) and carries GO-2026-5932, which has no fixed version — its affected range isintroduced: 0withFixed in: N/A, because the finding is the package's unmaintained status rather than a specific defect.Since Go cannot drop a package-level import, openpgp is linked into every binary that imports
selfupdate, so all downstream consumers inherit the advisory even when they configureChecksumValidatororECDSAValidatorand never verify a PGP signature.Change
Migrate to
github.com/ProtonMail/go-crypto/openpgp, the maintained fork recommended both by the advisory and by thex/crypto/openpgppackage documentation:Three parts:
validate.go— swap the import. The fork'sCheckDetachedSignature/CheckArmoredDetachedSignaturetake an additional*packet.Config;nilselects the library defaults and matches the behaviour of the functions being replaced.go.mod— bumpgithub.com/cloudflare/circltov1.6.3.ProtonMail/go-crypto@v1.4.1otherwise resolves circlv1.6.2, which is affected by GO-2026-4550 (incorrect secp384r1CombinedMultcalculation). Without this the change would trade one advisory for another.validate_test.go/mockdata_test.go, and relax one assertion (see below).Net diff is 19 insertions / 8 deletions.
The relaxed assertion
TestPGPValidator/Failasserted the exact error string:Both libraries reject the signature correctly; they differ in the reason reported after the prefix (
x/cryptofails at the hash-tag fast path, ProtonMail reportsRSA verification failureafter the full check). The reason is an implementation detail of the openpgp library rather than part of this package's contract, so the assertion now pins the stable prefix:Verification
go test ./...passes on all packages.golangci-lint run ./...reports 0 issues.golang.org/x/crypto/openpgpand verified them through the migratedPGPValidator— they validate correctly, and a tampered payload is still rejected. Release assets already signed with the previous implementation keep validating.govulnchecknow reportsNo vulnerabilities found.Compatibility note
PGPValidator.KeyRingis typedopenpgp.EntityList, and that type now comes from a different package. Code that builds a key ring itself withx/cryptoand assigns it toKeyRingdirectly will not compile until it updates its import:Code using the
WithArmoredKeyRing([]byte)convenience method — which does not expose the type — is unaffected. I could not find a way to avoid this while still removing the dependency, since keeping the old type would require keeping the import. It probably warrants a minor version bump.If you would prefer a different approach — for example moving the PGP validators into a separate package so consumers that do not use them never link openpgp — I am happy to rework this.