Release v1.0.0 — VCDM 2.0 compliance hardening + API ergonomics + lock - #14
Conversation
PR-B of the v1.0.0-readiness series (compliance). Tightens the structural validator to enforce VCDM 2.0 rules the engine previously accepted; landed in the 1.0.0 major rather than a breaking 1.0.x because stricter validation is a behavioural breaking change. All changes internal/private -- no public API delta. - Identifier members must be URLs (absolute URI with a scheme; DIDs/URNs/URLs pass; scheme-less/whitespace/null/multi-valued reject): id, issuer (string and object .id), credentialStatus.id, credentialSchema.id, credentialSubject.id, refreshService.id, relatedResource.id. - refreshService entries require a type. - relatedResource: one or more objects, each with a unique URL id and at least one of digestSRI/digestMultibase (structural; digest hash-verify deferred to a focused follow-up per the agreed scope). - name/description language value objects (top-level + issuer object) are closed: only @value/@language/@direction permitted (§11.1). - Issuance rejects a Data Integrity proof whose proofPurpose != assertionMethod at the role boundary (DI 1.0 §3.2). W3C VCDM 2.0 suite 43 -> 57/59 (measured); the 2 remaining (unmapped-type JSON-LD term resolution, relatedResource digest hash-verify) are documented in docs/conformance.md, not hidden. PassingBaseline raised to 57. Core 172 (+25), DI 161 (+1); 0-warning build. Adversarial pass (false-positive + bypass): holds. README Status de-staled; lessons captured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
moisesja
left a comment
There was a problem hiding this comment.
Review — LGTM, ship it. Two non-blocking notes.
I read the full diff, traced the high-risk lines into the source, and checked CI. This is a clean, well-scoped, well-tested compliance PR. Verdict: approve, with two minor observations below (neither blocks merge).
What I verified (not just took on faith)
- The "no public API delta" claim holds — the
API compatibility (semver)check is green, so the stricter validation is genuinely internal. - The 57/59 number is CI-backed, not just a local measurement — the
W3C VCDM 2.0 suitejob passes against the newPassingBaseline = 57. The[SkippableFact]is not being skipped in CI, so the gate is real. - The proofPurpose guard is safe. This was the one line that could have been catastrophic — if
ProofPurposedidn't default toassertionMethod, the new throw would reject every normal DI issuance. ConfirmedIssuanceRequest.ProofPurposedefaults toProofPurpose.AssertionMethod(IssuanceRequest.cs:33), so the guard only fires when a caller explicitly sets a non-assertion purpose. TheIssuing_with_non_assertionMethod_proofPurpose_throwstest covers it. build + testgreen on both ubuntu and windows.
Strengths
- Each rule lands with both positive and negative tests, using the exact suite negatives (DIDs/URNs/URLs explicitly asserted to pass — the over-rejection risk is directly covered).
IsAbsoluteUricorrectly models "URL = any absolute URI with a scheme" rather than HTTP-only; DIDs and URNs pass, which is the right reading of VCDM identifier semantics.- The remaining 2 gaps are named honestly in
docs/conformance.mdrather than buried, and the "tightening belongs in the major" reasoning inlessons.mdis sound. - Commit message is clear and accurately describes the change.
Note 1 (minor, honesty/completeness) — undocumented scope expansion to termsOfUse.id / evidence.id
The identifier-URL check lives in the shared ValidateTypedEntry (StructuralValidator.cs:373), which also runs for termsOfUse and evidence. So a non-URL id on those members is now rejected too — but the CHANGELOG and PR body enumerate only id, issuer, credentialStatus.id, credentialSchema.id, credentialSubject.id, refreshService.id, relatedResource.id. The behavior is correct per spec (those ids should be URLs), but the docs undersell what actually changed. Suggest adding termsOfUse.id / evidence.id to the CHANGELOG list, or a one-liner noting "all typed-entry ids" — purely so the documented surface matches the enforced surface. No code change needed.
Note 2 (cosmetic) — redundant scheme check in IsAbsoluteUri
Uri.TryCreate(value, UriKind.Absolute, out var uri) already guarantees a non-empty scheme, so the trailing && !string.IsNullOrEmpty(uri.Scheme) is dead. Harmless; leave it or drop it. Not worth a round-trip on its own.
Neither note is a merge blocker. Nice work — the verification rigor (CI-backed baseline, semver gate, adversarial pass) is exactly what this kind of "newly rejects inputs" change needs.
Generated by Claude Code
…scheme check Address the two non-blocking review notes on PR #14: - The shared ValidateTypedEntry id-URL check also applies to termsOfUse.id and evidence.id; document that in the CHANGELOG (all typed-entry ids) so the documented surface matches the enforced surface. - Remove the redundant `&& !IsNullOrEmpty(uri.Scheme)` in IsAbsoluteUri -- Uri.TryCreate(UriKind.Absolute) already guarantees a scheme. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Both notes addressed in 168104d: Note 1 (termsOfUse/evidence scope). Right — the shared Note 2 (dead scheme check). Dropped the redundant Thanks for the CI-backed verification of the no-API-delta / 57-baseline / proofPurpose-default points — that's exactly the trace this "newly rejects inputs" change warranted. |
….Form -> SecuringForm Last-chance public-surface polish before the 1.0 API lock (from the ergonomics review): - Credential / VerifiablePresentation conversion methods unified to ToElement() / ToUtf8() / ToClaimsObject() / ToBytes() (was a mixed As*/To* split). All four return fresh copies, so To* (the .NET "materialize a copy" idiom) is consistent and matches the underlying CredentialDocument. - IssuedCredential.Form retyped SecuringState -> SecuringForm: an issued credential is always one of the secured forms (never Unsecured), and this removes the .Form name/type collision with SecuringSelector.Form (both now SecuringForm). Behaviour-preserving renames; PublicAPI.Unshipped updated. 0-warning build; full suite green (Core 172, DI 161). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…OTICE Finalize the 1.0.0 release on this branch: - Bump CredentialsVersion 0.1.0 -> 1.0.0. - Lock the public API: move all 793 PublicAPI.Unshipped entries to PublicAPI.Shipped (Core 749, DI 27, Rdfc 17); Unshipped reset to the header. - Add SECURITY.md (vuln-reporting policy + scope for a verification engine) and NOTICE (Apache attribution + the implemented W3C/IETF specs). 0-warning build; full suite green; all three libraries pack cleanly at 1.0.0 (.nupkg + .snupkg). ApiCompat skips (first release, no prior baseline). The v1.0.0 tag — which triggers the nuget.org publish via release.yml — is intentionally left to a human. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR-B of 4 in the v1.0.0-readiness series (A security ✅ → B compliance → C cleanup → D release). Closes the achievable VCDM 2.0 structural-conformance gaps and raises the W3C suite baseline 43 → 57 / 59. All changes are internal / private — no public API delta.
Why now (not 1.0.x)
This tightens validation — it newly rejects inputs the engine used to accept (non-URL identifiers, typeless
refreshService, malformedrelatedResource, badname/descriptionobjects). That's a behavioral breaking change, so it belongs inside the 1.0.0 major rather than a later minor.What changed (
StructuralValidator,JsonShape,DefaultIssuer)null/ multi-valued reject:id,issuer(string + object.id),credentialStatus.id,credentialSchema.id,credentialSubject.id,refreshService.id,relatedResource.id(§4.4/§4.7/§4.9/§4.11).refreshServiceentries require atype.relatedResource— one or more objects, each with a unique URLidand ≥1 ofdigestSRI/digestMultibase(§5.3 structural). Digest hash-verification is deliberately deferred to a focused follow-up (it needs an egress fetch+hash and a new 1.0 public hook).name/descriptionlanguage value objects (top-level +issuer) are closed — only@value/@language/@direction(§11.1).proofPurpose≠assertionMethodat the role boundary (DI 1.0 §3.2).Conformance — honest re-baseline
Measured against the real W3C suite (twice, deterministic): 57 / 59 passing. The 2 remaining are named in
docs/conformance.md:typemust be terms and/or absolute URL strings — detecting an unmapped-via-@contexttypeneeds JSON-LD term resolution; an STJ-only non-goal.relatedResourcedigest must match the retrieved resource — the deferred hash-verify.PassingBaselineraised 43 → 57.Verification
Also: README Status de-staled (was "M0 / early development"),
docs/conformance.mdrewritten, lessons captured. Tracked follow-up: relatedResource digest hash-verify (+ two pre-existing resolver edges from the PR #13 review).🤖 Generated with Claude Code