Skip to content

fix: replace unmaintained golang.org/x/crypto/openpgp with ProtonMail fork - #58

Open
jmrplens wants to merge 1 commit into
creativeprojects:mainfrom
jmrplens:fix/replace-unmaintained-openpgp
Open

fix: replace unmaintained golang.org/x/crypto/openpgp with ProtonMail fork#58
jmrplens wants to merge 1 commit into
creativeprojects:mainfrom
jmrplens:fix/replace-unmaintained-openpgp

Conversation

@jmrplens

Copy link
Copy Markdown

Fixes #57.

Problem

validate.go imports golang.org/x/crypto/openpgp at file scope for the PGPValidator.KeyRing field. That package is frozen and deprecated (golang/go#44226) and carries GO-2026-5932, which has no fixed version — its affected range is introduced: 0 with Fixed 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 configure ChecksumValidator or ECDSAValidator and never verify a PGP signature.

Change

Migrate to github.com/ProtonMail/go-crypto/openpgp, the maintained fork recommended both by the advisory and by the x/crypto/openpgp package documentation:

If you are required to interoperate with OpenPGP systems and need a maintained package, consider github.com/ProtonMail/go-crypto/openpgp, which is a maintained fork that aims to be a drop-in replacement for this package.

Three parts:

  1. validate.go — swap the import. The fork's CheckDetachedSignature / CheckArmoredDetachedSignature take an additional *packet.Config; nil selects the library defaults and matches the behaviour of the functions being replaced.
  2. go.mod — bump github.com/cloudflare/circl to v1.6.3. ProtonMail/go-crypto@v1.4.1 otherwise resolves circl v1.6.2, which is affected by GO-2026-4550 (incorrect secp384r1 CombinedMult calculation). Without this the change would trade one advisory for another.
  3. Tests — migrate the imports in validate_test.go / mockdata_test.go, and relax one assertion (see below).

Net diff is 19 insertions / 8 deletions.

The relaxed assertion

TestPGPValidator/Fail asserted the exact error string:

assert.EqualError(t, err, "openpgp: invalid signature: hash tag doesn't match")

Both libraries reject the signature correctly; they differ in the reason reported after the prefix (x/crypto fails at the hash-tag fast path, ProtonMail reports RSA verification failure after 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:

assert.ErrorContains(t, err, "openpgp: invalid signature")

Verification

  • go test ./... passes on all packages.
  • golangci-lint run ./... reports 0 issues.
  • Backward compatibility of existing signatures: I generated a key and an armored detached signature with golang.org/x/crypto/openpgp and verified them through the migrated PGPValidator — they validate correctly, and a tampered payload is still rejected. Release assets already signed with the previous implementation keep validating.
  • Consumer impact: in a project that previously reported GO-2026-5932 through this module, govulncheck now reports No vulnerabilities found.

Compatibility note

PGPValidator.KeyRing is typed openpgp.EntityList, and that type now comes from a different package. Code that builds a key ring itself with x/crypto and assigns it to KeyRing directly will not compile until it updates its import:

cannot use ring (variable of slice type "golang.org/x/crypto/openpgp".EntityList)
as "github.com/ProtonMail/go-crypto/openpgp".EntityList value in struct literal

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.

… 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.
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. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dependency on frozen golang.org/x/crypto/openpgp propagates GO-2026-5932 to all consumers

1 participant