From 698ed5539c120a7092ca44da818471f1daf71426 Mon Sep 17 00:00:00 2001 From: Reza Maghoul Date: Tue, 9 Jun 2026 16:18:23 +0200 Subject: [PATCH 1/2] fix: witness list changes must be witnessed by the prior list (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per spec §3.7.5 a replacement `witness` list only becomes active after the entry that introduces it is published, so that entry must be witnessed by the list in effect before it. WizardWitnessProofs and WitnessValidator were using the new (merged) list instead, so reducing a list (e.g. two witnesses down to one) produced and required only a single proof. Both now use the prior active list whenever witnessing was already active, with the new list applying only on first activation, matching the Rust reference output for vectors/witness-update. Add WitnessValidator regression tests for the witness-update reduction scenario, update the CHANGELOG [Unreleased] section, and document the changelog/commit workflow in docs/AGENTS.md. --- CHANGELOG.md | 14 +++++ .../core/validate/WitnessValidator.java | 27 ++++----- .../core/validate/WitnessValidatorTest.java | 59 +++++++++++++++++++ .../didwebvh/wizard/WizardWitnessProofs.java | 28 +++++---- docs/AGENTS.md | 16 +++++ 5 files changed, 119 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa7c6f..42368d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Witness-list updates witnessed by the wrong list** (spec §3.7.5, + issue #6): per the spec a replacement `witness` list only becomes + active *after* the entry that introduces it is published, so the + entry making the change must be witnessed by the list in effect + *before* it. Both the proof generator (`WizardWitnessProofs`) and + the resolver (`WitnessValidator`) were instead using the new + (merged) list, so reducing a list (e.g. two witnesses down to one) + produced and required only a single proof. Now an entry is + witnessed by the prior active list whenever witnessing was already + active — covering list changes and turning witnessing off — with + the new list applying only on first activation. This matches the + Rust reference output for `vectors/witness-update`. + ## [0.3.0] - 2026-05-29 This release closes the failures reported by the diff --git a/didwebvh-core/src/main/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidator.java b/didwebvh-core/src/main/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidator.java index b460712..e2d3106 100644 --- a/didwebvh-core/src/main/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidator.java +++ b/didwebvh-core/src/main/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidator.java @@ -66,24 +66,23 @@ public WitnessValidationResult validate(List entries, activeParams = activeParams.merge(entryParams); WitnessConfig afterConfig = activeParams.getWitness(); - // Spec §3.7.5 (lines 884-889): when the witness parameter is set to - // {} while witnesses were previously active, that log entry MUST be - // witnessed by the prior witnesses. Otherwise an attacker could - // legitimately disable witnessing in a forged entry and slip the - // change past the resolver. Skip only when no witnessing was - // active either side of this entry. + // Spec §3.7.5 (lines 884-889): a replacement witness list "becomes + // active AFTER the new DID log has been published". So while + // witnesses are already active, every entry — including one that + // changes the list (e.g. two witnesses down to one) or sets it to + // {} — MUST be witnessed by the list in effect BEFORE this entry; + // the replacement only governs subsequent entries. This also stops + // an attacker from shrinking or disabling oversight in a single + // forged entry and slipping it past the resolver. The sole + // exception is the first activation, where there is no prior list. boolean priorActive = priorConfig != null && priorConfig.isActive(); boolean afterActive = afterConfig != null && afterConfig.isActive(); - boolean witnessChanged = entryParams.getWitness() != null; WitnessConfig witnessConfig; - if (afterActive) { - // Per spec, "from {} to active" means the new config is - // immediately active and approves this entry. - witnessConfig = afterConfig; - } else if (priorActive && witnessChanged) { - // Transition from active to {}: the prior witnesses must - // approve this transition. + if (priorActive) { witnessConfig = priorConfig; + } else if (afterActive) { + // First activation ({} -> active): the new list approves this entry. + witnessConfig = afterConfig; } else { continue; } diff --git a/didwebvh-core/src/test/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidatorTest.java b/didwebvh-core/src/test/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidatorTest.java index 438106c..4b57530 100644 --- a/didwebvh-core/src/test/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidatorTest.java +++ b/didwebvh-core/src/test/java/io/github/decentralizedidentity/didwebvh/core/validate/WitnessValidatorTest.java @@ -208,6 +208,65 @@ void crossDidWitnessReplayRejected() { assertThat(result.getFailureReason()).contains("insufficient witness proofs"); } + private WitnessProofCollection makeWitnessProofs(WitnessProofEntry... entries) { + return new WitnessProofCollection(Arrays.asList(entries)); + } + + private WitnessProofEntry makeProofEntry(String versionId, Signer... signers) { + JsonObject doc = new JsonObject(); + doc.addProperty("versionId", versionId); + DataIntegrityProof[] proofs = new DataIntegrityProof[signers.length]; + for (int i = 0; i < signers.length; i++) { + proofs[i] = LogChainValidatorTest.signDocument(signers[i], doc); + } + return new WitnessProofEntry(versionId, Arrays.asList(proofs)); + } + + @Test + void witnessListReductionStillRequiresPriorList() { + // Spec §3.7.5: when entry 2 reduces the witness list from {w1,w2}@2 to + // {w1}@1, that change takes effect only AFTER entry 2 is published, so + // entry 2 must still be witnessed by the PRIOR list {w1,w2}@2. A pruned + // did-witness.json with both proofs at versionId 2 satisfies both + // entries (issue #6). See vectors/witness-update/rust. + CreateDidResult create = createWithWitness(2, + new WitnessEntry(witnessDid1), new WitnessEntry(witnessDid2)); + LogEntry e1 = create.getLogEntry(); + LogEntry e2 = LogChainValidatorTest.buildUpdateEntry( + e1, create.getDid(), authorSigner, + new Parameters().setWitness(new WitnessConfig(1, + Collections.singletonList(new WitnessEntry(witnessDid1)))), + null); + + WitnessProofCollection proofs = makeWitnessProofs( + makeProofEntry(e2.getVersionId(), witnessSigner1, witnessSigner2)); + + WitnessValidationResult result = validator.validate(Arrays.asList(e1, e2), proofs, 0); + assertThat(result.isValid()).isTrue(); + } + + @Test + void witnessListReductionWithOnlyNewListFails() { + // The buggy behaviour from issue #6: entry 2 reduces to {w1}@1 and only + // w1 provides a proof. Because entry 2 must be witnessed by the prior + // {w1,w2}@2 list, a single proof is insufficient and resolution fails. + CreateDidResult create = createWithWitness(2, + new WitnessEntry(witnessDid1), new WitnessEntry(witnessDid2)); + LogEntry e1 = create.getLogEntry(); + LogEntry e2 = LogChainValidatorTest.buildUpdateEntry( + e1, create.getDid(), authorSigner, + new Parameters().setWitness(new WitnessConfig(1, + Collections.singletonList(new WitnessEntry(witnessDid1)))), + null); + + WitnessProofCollection proofs = makeWitnessProofs( + makeProofEntry(e2.getVersionId(), witnessSigner1)); + + WitnessValidationResult result = validator.validate(Arrays.asList(e1, e2), proofs, 0); + assertThat(result.isValid()).isFalse(); + assertThat(result.getFailureReason()).contains("insufficient"); + } + @Test void fromEntryIndexRespected() { // Two entries; only second requires witnessing diff --git a/didwebvh-wizard/src/main/java/io/github/decentralizedidentity/didwebvh/wizard/WizardWitnessProofs.java b/didwebvh-wizard/src/main/java/io/github/decentralizedidentity/didwebvh/wizard/WizardWitnessProofs.java index 94d5636..c2fe80c 100644 --- a/didwebvh-wizard/src/main/java/io/github/decentralizedidentity/didwebvh/wizard/WizardWitnessProofs.java +++ b/didwebvh-wizard/src/main/java/io/github/decentralizedidentity/didwebvh/wizard/WizardWitnessProofs.java @@ -79,22 +79,28 @@ void collectForNewEntries(Parameters priorParams, List newEntries, Pat } /** - * Per spec: witness proofs are required when either the merged config AFTER this - * entry's delta is active (normal case) OR when this entry sets witness back to - * {@code {}} while it was previously active (transition-off case). In both cases - * the authorized witness set for signing is the previously active set when turning - * witnesses off, otherwise the newly merged set. + * Determine which witness set must approve {@code entryDelta}. + * + *

Per spec §3.7.5, a new {@code witness} list "becomes active AFTER the new + * DID log has been published". So while witnesses are already active, every + * entry — including one that changes the list (e.g. from two witnesses + * to one) or one that turns witnessing off ({@code {}}) — MUST be witnessed by + * the list that was in effect before this entry. The replacement list + * only governs subsequent entries. Returning the prior list here also closes the + * loophole where a controller could unilaterally shrink or remove oversight in a + * single unapproved entry. + * + *

The only exception is the first activation: when no witnesses were active + * before, there is no prior list to defer to, so the newly merged list applies + * immediately. */ private WitnessConfig authorizedWitnesses(Parameters priorParams, Parameters entryDelta) { WitnessConfig priorActive = priorParams.getWitness(); - Parameters merged = priorParams.merge(entryDelta); - WitnessConfig mergedActive = merged.getWitness(); - boolean turningOff = priorActive != null && priorActive.isActive() - && (mergedActive == null || !mergedActive.isActive()); - if (turningOff) { + if (priorActive != null && priorActive.isActive()) { return priorActive; } - return mergedActive; + // First activation: no prior witnesses, so the new list applies to this entry. + return priorParams.merge(entryDelta).getWitness(); } private WitnessProofEntry buildProofEntry(String versionId, WitnessConfig authorized, diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 572289d..80e40a8 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -161,6 +161,20 @@ Use [Conventional Commits](https://www.conventionalcommits.org/): - `docs: update README with resolve example` - `chore: configure JaCoCo for coverage reporting` +## Changelog and Commit Workflow + +After completing any change to the codebase, the agent MUST: + +1. **Update the changelog.** Add a summary of the change under the `## [Unreleased]` + section of `CHANGELOG.md`, in the appropriate `### Added` / `### Changed` / + `### Fixed` / `### Removed` subsection (create the subsection if it does not yet + exist). Follow the existing style: reference the spec section and/or issue number + for non-obvious behaviour. +2. **Propose a commit message.** End the response with a suggested + [Conventional Commits](https://www.conventionalcommits.org/) commit message for the + change, so the developer can review and commit it. Do not commit on the developer's + behalf unless explicitly asked. + ## How to Work on This Project 1. Read `docs/ARCHITECTURE.md` for the full technical design. @@ -169,3 +183,5 @@ Use [Conventional Commits](https://www.conventionalcommits.org/): 4. Run `./mvnw clean verify` with JDK 21 after every change to ensure tests, Checkstyle, SpotBugs, and coverage run locally. 5. Keep dependencies minimal. Don't add a library for something that can be done in 20 lines. +6. After every change, update the `[Unreleased]` section of `CHANGELOG.md` and propose a + commit message (see [Changelog and Commit Workflow](#changelog-and-commit-workflow)). From 54babc6d63748be6a6992a86270bd68a1e632558 Mon Sep 17 00:00:00 2001 From: Reza Maghoul Date: Tue, 9 Jun 2026 16:27:42 +0200 Subject: [PATCH 2/2] chore(release): prepare v0.3.1 Bump parent + module poms from 0.3.0-SNAPSHOT to 0.3.1-SNAPSHOT and update README Maven/Gradle coordinates to 0.3.1. Promote the witness-list-timing fix (issue #6) from [Unreleased] to a new "## [0.3.1] - 2026-06-09" CHANGELOG entry and add the compare links. --- CHANGELOG.md | 5 ++++- README.md | 4 ++-- didwebvh-core/pom.xml | 2 +- didwebvh-signing-local/pom.xml | 2 +- didwebvh-wizard/pom.xml | 2 +- pom.xml | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42368d0..8a15608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.3.1] - 2026-06-09 + ### Fixed - **Witness-list updates witnessed by the wrong list** (spec §3.7.5, issue #6): per the spec a replacement `witness` list only becomes @@ -217,7 +219,8 @@ Initial public release of `didwebvh-java`, a Java 11+ implementation of the `.github/workflows/release.yml` that publishes to Sonatype Central on tag push (`v*`) and attaches JARs to the GitHub Release. -[Unreleased]: https://github.com/decentralized-identity/didwebvh-java/compare/v0.3.0...HEAD +[Unreleased]: https://github.com/decentralized-identity/didwebvh-java/compare/v0.3.1...HEAD +[0.3.1]: https://github.com/decentralized-identity/didwebvh-java/compare/v0.3.0...v0.3.1 [0.3.0]: https://github.com/decentralized-identity/didwebvh-java/compare/v0.2.0...v0.3.0 [0.2.0]: https://github.com/decentralized-identity/didwebvh-java/compare/v0.1.0...v0.2.0 [0.1.0]: https://github.com/decentralized-identity/didwebvh-java/releases/tag/v0.1.0 diff --git a/README.md b/README.md index 95726c0..46c80fd 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ A Java 11+ library for the [did:webvh](https://didwebvh.info/) (DID Web + Verifi io.github.decentralized-identity didwebvh-java - 0.3.0 + 0.3.1 ``` @@ -44,7 +44,7 @@ need the local-key adapter. ### Gradle ```groovy -implementation 'io.github.decentralized-identity:didwebvh-java:0.3.0' +implementation 'io.github.decentralized-identity:didwebvh-java:0.3.1' ``` ## Library Usage diff --git a/didwebvh-core/pom.xml b/didwebvh-core/pom.xml index b627854..183f88e 100644 --- a/didwebvh-core/pom.xml +++ b/didwebvh-core/pom.xml @@ -7,7 +7,7 @@ io.github.decentralized-identity didwebvh-java - 0.3.0-SNAPSHOT + 0.3.1-SNAPSHOT didwebvh-core diff --git a/didwebvh-signing-local/pom.xml b/didwebvh-signing-local/pom.xml index 393d43b..2d03a6c 100644 --- a/didwebvh-signing-local/pom.xml +++ b/didwebvh-signing-local/pom.xml @@ -7,7 +7,7 @@ io.github.decentralized-identity didwebvh-java - 0.3.0-SNAPSHOT + 0.3.1-SNAPSHOT didwebvh-signing-local diff --git a/didwebvh-wizard/pom.xml b/didwebvh-wizard/pom.xml index 93ae45e..fc9106b 100644 --- a/didwebvh-wizard/pom.xml +++ b/didwebvh-wizard/pom.xml @@ -7,7 +7,7 @@ io.github.decentralized-identity didwebvh-java - 0.3.0-SNAPSHOT + 0.3.1-SNAPSHOT didwebvh-wizard diff --git a/pom.xml b/pom.xml index afc6dd1..67dd9a9 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ io.github.decentralized-identity didwebvh-java - 0.3.0-SNAPSHOT + 0.3.1-SNAPSHOT pom didwebvh-java