Skip to content

Fix Bun compatability: base64 DER key parsing and default to sha256 for verify#1561

Open
PowerSpy wants to merge 2 commits intosigstore:mainfrom
PowerSpy:fix/bun-compatability
Open

Fix Bun compatability: base64 DER key parsing and default to sha256 for verify#1561
PowerSpy wants to merge 2 commits intosigstore:mainfrom
PowerSpy:fix/bun-compatability

Conversation

@PowerSpy
Copy link
Copy Markdown

Added patches to ensure Bun/BoringSSL compatibility in sigstore-js:

  1. Base64 DER public key parsing:

    • Ensures createPublicKey handles base64 encoded DER keys correctly.
  2. Default to SHA256 when verifying:

    • verify() defaults to 'sha256' if no algorithm is provided.

Changes allow for better Bun compatibility for creating keys and verifying.

Summary

This PR adds patches to ensure sigstore-js works correctly in Bun/BoringSSL environments.

Specifically:

  1. Base64 DER parsing in createPublicKey.
  2. verify() defaults to SHA-256 if no algorithm is provided.

Release Note

Fix: Improved Bun/BoringSSL compatibility.

Documentation

No documentation changes required; this improves Bun compatibility.

…or verify

Added patches to ensure Bun/BoringSSL compatibility in sigstore-js:

1. Base64 DER public key parsing:
    - Ensures createPublicKey handles base64 encoded DER keys correctly.

2. Default to SHA256 when verifying:
    - verify() defaults to 'sha256' if no algorithm is provided.

Changes allow for better Bun compatibility for creating keys and verifying.
@PowerSpy PowerSpy requested a review from a team as a code owner January 14, 2026 07:01
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jan 14, 2026

⚠️ No Changeset found

Latest commit: e646e02

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

nklisch added a commit to nklisch/skilltap that referenced this pull request Mar 2, 2026
Two bugs found and fixed in verifyNpmProvenance:
- InTotoStatement digest was typed as sha256-only; npm SLSA attestations
  actually use sha512 — updated hash check to handle both
- sigstore.verify() always failed in Bun: @tufjs/models called
  crypto.verify(undefined, ...) which BoringSSL rejects with
  NO_DEFAULT_DIGEST. Patched via bun patch to derive the algorithm
  from the key's named curve (ECDSA P-256 → sha256, etc.)
  Upstream: sigstore/sigstore-js#1561

Adds trust.integration.test.ts (SKILLTAP_IT=1 for network tests):
- Full end-to-end Sigstore+TUF provenance verification for sigstore@4.1.0
- Validates DSSE payload structure, sha512 tarball hash, Rekor log entry
- Graceful-null for packages with no attestations
- Tap install → curated trust tier (3 variants, always run)
@bdehamer
Copy link
Copy Markdown
Collaborator

@PowerSpy can you give me some more context for:

verify() defaults to 'sha256' if no algorithm is provided.

There are definitely places in the code where we depend on verify to infer the correct algo based on the key. For example, here.

Sigstore is currently using an Ed25519 key for the transparency log. To verify these signatures with Node's crypto.verify() you must pass undefined -- passing any specific digest algorithm (like "sha256") causes it to throw "invalid digest" because Ed25519 handles its own hashing internally.

@PowerSpy
Copy link
Copy Markdown
Author

PowerSpy commented Apr 2, 2026

@bdehamer Thanks for the information, I was not aware of that Ed25519 behavior with the crypto.verify()
It makes sense to pass undefined here instead. I have updated the PR accordingly and tested with both Node and Bun confirming that it works on both ends.

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.

2 participants