fix: the verifier could not verify any production certificate (cert.v2 + trust root) - #8
Merged
Merged
Conversation
Every certificate CertifiedData has issued is cert.v2. The published verifier
implemented only cert.v1 and rejected v2 as MALFORMED on a missing
certification_id — v2 names it certificate_id. Separately, the pinned keys URL
returned 404, so verification exited NETWORK before reaching a signature.
Signed bytes are Ed25519 over RFC 8785 JCS of the payload. Confirmed
empirically against the live production certificate now committed as
fixtures/valid-cert-v2.json: of JCS(payload), JSON.stringify(payload),
JCS(envelope minus signature) and JCS(payload minus the self-hash), only
JCS(payload) verifies. This is a schema-mapping fix, not a cryptography change.
Three things the original specification of this work got wrong about
production, each of which would have shipped a verifier that still failed:
1. `signature` is an OBJECT, not a base64 string —
/signed-payload serves {alg, key_id, value} and
/api/certificates/:id serves {alg, key_id, sig}.
Requiring a string reported MALFORMED on all 577 certificates. Both
spellings and a bare string are now accepted.
2. The keys document to trust is /.well-known/signing-keys.json, which is
live and DB-backed and is what every certificate's own public_key_url
references. /.well-known/certifieddata-keys.json returns 404 and was
never deployed. Its dialect differs (public_key_pem, "Ed25519",
revocation in a top-level revoked[] array, CRLF in PEM bodies), so
parseKeyDoc now normalizes both. Two of those differences were
security-relevant: a mis-read algorithm yielded UNKNOWN_KEY on a good
key, and an unmapped revoked[] would have let a revoked key keep
verifying.
3. Resolving a bare id must fetch /signed-payload. The plain
/api/certificates/:id response is a cert.v1-shaped display projection
carrying the real signature bytes, but the signature covers the v2
payload rather than the projection — so verifying it returned INVALID
on untampered certificates.
Key selection reads the signed payload only. The envelope is not covered by the
signature, so trusting its key_id would let whoever supplied the document choose
which key it is checked against; a disagreement between the two is now
MALFORMED rather than silently resolved.
cert.v1 and the receipt path are untouched and still verify. 93 tests pass
(was 71), typecheck clean, lint unchanged apart from one pre-existing warning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Fixture question resolved — no swap needed. Queried That is Drew's own account, and it holds 507 of the certificates. So For completeness, the other two ids checked: No branch rewrite is needed and this PR can merge as-is. |
dkitchell
added a commit
that referenced
this pull request
Sep 15, 2026
…enance it cannot have (#9) Two reasons a release-triggered publish would have failed. 1. Wrong secret name. The workflow read `secrets.NPM_TOKEN`. The secret that actually exists on this repo is `NPM_ACCESS_TOKEN` (set 2026-08-21), so NODE_AUTH_TOKEN resolved to an empty string and npm would have published as an anonymous client. I introduced this name in #8 without checking what was configured. 2. `--provenance` cannot work on a first publish. Trusted publishing is configured per package on npmjs.com, which requires the package to already exist — and @certifieddata/verify has never been published, so there is nothing to configure it against. On top of that, a classic automation token combined with --provenance is exactly the combination npm is restricting (https://gh.io/npm-gat-bypass2fa-deprecation); the local CLI now warns about it on every command. Dropped --provenance for 0.1.0 and pointed the token at the right secret. Once 0.1.0 is on the registry, trusted publishing can be enabled for the package and --provenance added back — at which point the token should be deleted rather than kept alongside it. The version-vs-tag guard and the lint/typecheck/test gates are unchanged, so a release still cannot publish a version nobody asked for. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes the certificate path. No production deploy is required — see "What this does not need" below.
What was broken
Every issued certificate is
cert.v2; the verifier implemented onlycert.v1and rejected v2 asMALFORMEDon a missingcertification_id(v2 names itcertificate_id). Separately the pinned keys URL 404'd, so verification exitedNETWORKbefore reaching a signature.The cryptography was fine
Signed bytes are
Ed25519(JCS(payload)). Confirmed empirically against the live production certificate now committed asfixtures/valid-cert-v2.json— of four candidate constructions, exactly one verifies:So this is schema mapping, not a cryptography change.
Three things the original spec of this work got wrong
Each would have shipped a verifier that still failed on every certificate. All three now have regression tests.
1.
signatureis an object, not a base64 string.The specified implementation required
typeof signature === "string"and would have returnedMALFORMEDon all 577 certificates — the same failure it set out to fix. Both spellings and a bare string are now accepted.2. The keys document to trust is
/.well-known/signing-keys.json.It is live, DB-backed, and is what every certificate's own
public_key_urlreferences./.well-known/certifieddata-keys.jsonreturns 404 and was never deployed. Its dialect differs, soparseKeyDocnow normalizes both:signing-keys.v1(published)public_keypublic_key_pem(with CRLF)"ed25519""Ed25519"revoked_atrevoked[]/retired[]Two of those are security-relevant, not cosmetic:
algorithmyieldedUNKNOWN_KEY— a security verdict — on a good key, for one capital letter. A verifier that cries wolf teaches people to disbelieve it.revoked[]would have let a revoked key keep verifying. A revocation entry that cannot be parsed is now an error rather than skipped, so an unreadable revocation record can never be mistaken for a good key.3. Resolving a bare id must fetch
/signed-payload.The plain
/api/certificates/:idresponse is acertifieddata.cert.v1-shaped display projection. It carries the real signature bytes (base.signature.sig === signed.signature.value), but the signature covers the v2 payload, not the projection — so verifying it returnsINVALIDon an untampered certificate.DEFAULT_CERT_APInow targets the envelope.Key selection is treated as a security boundary
Key selection reads the signed payload only (
payload.issuer.signing_key_id). The envelope is not covered by the signature, so trusting itskey_idwould let whoever supplied a document choose which key it is checked against. A disagreement between envelope and payload isMALFORMEDrather than silently resolved in either direction.Verified against production
Regressions both still pass:
93 tests pass (was 71), typecheck clean, lint unchanged apart from the one pre-existing unused-directive warning.
verify:distpasses.What this does not need
The plan for this work budgeted a keys-document deploy and a new
/api/v1/certificates/:idendpoint. Neither is required: pointing at the document the issuer already publishes, and at the envelope endpoint that already exists, closes the path with zero production changes. Worth noting/api/v1/would also have been a third URL convention — this repo's canonical prefix is/v1/<name>, not/api/v1/<name>.Also in here
.github/workflows/publish.ymlwould have failed on auth: it requested--provenancewith noNODE_AUTH_TOKEN, and Node 22 bundles npm 10.9.x where OIDC trusted publishing needs ≥ 11.5.1. Adds an npm upgrade step, a token fallback, and a tag-vs-version guard.cert.v1" claim, the offlinecurlexample (it fetched a 404), and added the v1/v2 table, the envelope shape, and the display-projection warning.hashes.certificate_payload_sha256is documented as non-normative. Its published value is not reproducible under JCS,JSON.stringify, sorted-key stringify, or pretty-printed JSON (claimedd611a011…, JCS gives1beed455…) — most likely computed over insertion-ordered JSON, which Postgresjsonbdoes not preserve. The signature is unaffected.Two things for a human to decide
fixtures/valid-cert-v2.jsonis a real production certificate in a public repo. It containssubject.user_idand a dataset filename. That data is already public — the endpoint it came from is unauthenticated — so this publishes nothing new. But if you'd rather not have a customer'suser_idin the repo, the fix is a dedicated canary certificate issued for this purpose, and I'd swap it.npm publishis a human action and irreversible for a given version.Found while doing this, not fixed here
/api/certificates/:id/downloadand/signed-payloadshare a helper that does not filter on certificate status, while/api/certificates/:iddoes (status = 'ISSUED'). So a draft or revoked certificate's signed payload is publicly retrievable and this verifier will report itVALID— correctly, since the signature is genuine. That is a platform-side issue, not a verifier one; filing separately.