Path-A Data Integrity invocation interop with @digitalbazaar/zcap (#117)#120
Conversation
…ations (Path A, #117) Adds an ADDITIVE digitalbazaar-compatible invocation mode and proves it live both directions against the real @digitalbazaar/zcap. The existing self-contained Invocation envelope (used in-stack and for revocation) is unchanged. - SigningService.SignCapabilityInvocationAsync(...): signs an application document with a capabilityInvocation proof whose proof object alone carries capability/capabilityAction/ invocationTarget (no envelope, no capabilityChain). The document carries @context=[zcap/v1, ed25519-2020/v1] + id + an absolute-IRI type (required so it expands to a non-empty RDF dataset under JSON-LD safe mode). Signed input = SHA-256(RDFC(proofOptions)) || SHA-256(RDFC(document)) via the existing LegacyProofCrypto path. - VerificationService.VerifyCapabilityInvocationAsync(JsonObject[, root]): verifies the signature over the application document, then reuses the chain/attenuation/caveat/controller/freshness/ replay checks. Root id-string vs embedded-delegated-zcap shapes handled per #51. - interop: CLI gen-invocation/verify-invocation; JS invoke-gen.mjs/invoke-verify.mjs + lib.mjs; run-interop.sh now runs 12 checks (6 delegation + 6 invocation: root + delegated, both directions, + 2 tamper negatives) — all green. - tests: DataIntegrityInvocationTests (root + delegated round-trip, tampered-action, action-not-allowed). - docs: README/AGENTS/CHANGELOG/interop README/roadmap updated. HTTP-Signature invocations (Path B) remain deferred (#119). 460 Core + 33 AspNetCore tests green; interop/run-interop.sh 12/12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fix incoming on this branch: (a) validate the application-document |
…ion verify (adversarial review) An adversarial red-team workflow against the new VerifyCapabilityInvocationCoreAsync (PR #120) found two execution-confirmed flaws; both are fixed here. - CRITICAL (forgery): the verifier did not validate the application document's @context. The proof's invocation terms (capability/capabilityAction/invocationTarget) are zcap/v1 terms and the proof options canonicalize under the document @context, so stripping zcap/v1 makes RDFC-1.0 drop them from the signed N-Quads — leaving them unauthenticated and rewritable after signing. FIX: require the application document @context to be an array beginning with https://w3id.org/zcap/v1 and including the signing suite context BEFORE trusting the signature (mirrors the chain R-CTX-2 check); reject MalformedCapability otherwise. Added AsArrayContextNode helper. - HIGH (confused-deputy): the verifier authorized whatever the proof claimed, with no relying-party expectation gate (unlike @digitalbazaar/zcap, which requires expectedAction/expectedTarget/ expectedRootCapability). FIX: VerifyCapabilityInvocation[Detailed]Async now REQUIRE expectedAction + expectedTargets (and accept optional expectedRootCapabilityIds), failing closed unless the proof matches (action exact, target in the set, resolved root pinned). Removed the no-expectation overloads (the footgun) and added the safe method to IVerificationService. - LOW (nonce/result not bound to the operation) — addressed by the expectation gate. Regression tests (DataIntegrityInvocationTests): stripped-@context forgery -> MalformedCapability; expected action/target/root mismatch -> rejected. Interop CLI/harness updated to pass expectations. 464 Core + 33 AspNetCore tests green; interop/run-interop.sh 12/12. Lessons recorded in tasks/lessons.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
✅ Both findings fixed in CRITICAL (forgery) — fixed. HIGH (confused-deputy) — fixed. LOW (nonce/result not bound to the operation) is subsumed by the expectation gate. The 6 attack classes that held in the first pass (signature binding, embedded-chain forgery, controller-auth, replay/freshness, etc.) still hold. Suite 464 Core + 33 AspNetCore green; Recorded the process lesson in |
moisesja
left a comment
There was a problem hiding this comment.
I found PR #120. Let me fetch the full diff and review it.
CI shows cross-stack-interop is failing. Let me read the diff and check the CI logs in parallel.
Let me continue reading the diff and check the failing CI job logs in parallel.
Interesting — the interop script itself reports "ALL 12 CHECKS PASSED" but then a dotnet test runner failure follows. Let me get the full CI log to understand what test actually failed.
Found it. The test asserts "ALL 6 CHECKS PASSED" but the PR changed the harness to print "ALL 12 CHECKS PASSED". The test assertion was never updated. Let me finish reading the diff to complete the review.
I've read the full diff. Let me check the exact interop test file that's failing to confirm the fix.
I now have a complete picture. Let me post the review.
The review API isn't accessible with this integration. Let me post it as a PR comment instead.
The GitHub integration doesn't have write permissions to post comments on this repo. Here's the full review I attempted to post — you can paste it yourself:
Review — PR #120
Verdict: one blocker, otherwise ready to merge.
❌ Blocker — Stale test assertion (cross-stack-interop CI is red)
tests/ZcapLd.Interop.Tests/InteropHarnessTests.cs:41
Assert.Contains("ALL 6 CHECKS PASSED", output); // ← staleThe PR changed run-interop.sh to now print "ALL 12 CHECKS PASSED — RDFC delegation + invocation interop…". The old string never appears, so Assert.Contains fails and the job exits 1 — even though the harness itself passed 12/12 (confirmed in the CI log: the success banner is printed, then the dotnet test runner fails on this assertion alone).
One-line fix: change "ALL 6 CHECKS PASSED" → "CHECKS PASSED" (or "ALL 12 CHECKS PASSED"). Using the substring avoids this breaking again when more checks are added.
Minor nits (not blocking)
- Stale method/class name.
RunInteropHarness_RdfcDelegation_RoundTripsWithDigitalBazaarand the class-level doc ("delegation interop harness") no longer reflect that invocation is also covered. Low-priority cosmetic. IVerificationServicemissing the single-string-target convenience overload.VerifyCapabilityInvocationDetailedAsync(..., expectedTarget: string, ...)is on the concrete class (used in 4 of 7 new tests) but not the interface — callers usingIVerificationServicehave to writenew[] { target }.- Bundled dependency bumps (DataProofsDotnet/NetDid 1.0.0→1.0.1 / 2.0.0→2.0.1) aren't called out in the CHANGELOG or PR description.
What's good
- Additive approach is correct — the self-contained
Invocationenvelope (revocation, in-stack) is untouched. - Both security gates are sound and well-tested:
@contextvalidation (forgery defense) and mandatoryexpectedAction/expectedTargets(confused-deputy defense). The adversarial regression tests lock both. - No no-expectation overload is the right design decision.
- Step ordering in
VerifyCapabilityInvocationCoreAsyncis correct: confused-deputy check before signature crypto (fast-fail first). - The
lessons.mdupdate properly captures the process failure and the rule going forward.
…erface overload, changelog - BLOCKER (cross-stack-interop CI red): InteropHarnessTests asserted "ALL 6 CHECKS PASSED" but the harness now prints "ALL 12 CHECKS PASSED". Assert the substring "CHECKS PASSED" so adding checks never re-breaks it. Renamed the test + class doc to reflect delegation AND invocation coverage. - Added the single-string-target VerifyCapabilityInvocationDetailedAsync convenience overload to IVerificationService (it was on the concrete class only). - CHANGELOG: note the DataProofsDotnet.* 1.0.0 -> 1.0.1 patch bump (from the "Minor version upgrade" commit on this branch). 464 Core + 33 AspNetCore green; interop wrapper test green (12/12). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in `a47924d`. ❌ Blocker — stale assertion (the `cross-stack-interop` CI failure) — fixed. `InteropHarnessTests` asserted `"ALL 6 CHECKS PASSED"` while the harness now prints `"ALL 12 CHECKS PASSED"` (the harness itself was 12/12; only the wrapper assertion failed). Now asserts the substring `"CHECKS PASSED"` so adding checks never re-breaks it. Wrapper test is green again. Nit 1 — stale name/doc — fixed. Renamed Nit 2 — interface overload — fixed. Added the single-string-target Nit 3 — dependency bump — documented. The Thanks for catching the two security gates earlier and this CI break. 464 Core + 33 AspNetCore green; interop wrapper 12/12. |
Closes the last interop gap: cross-stack invocation. zcap-dotnet can now produce and verify Data Integrity
capabilityInvocationproofs that round-trip with the real@digitalbazaar/zcap— root and delegated, both directions. Fixes #117.Approach: additive
A new digitalbazaar-compatible invocation mode alongside the existing self-contained
Invocationenvelope. The envelope (used in-stack and by revocation) is unchanged — no rip-out, no risk to the working control-plane flow.What's new
SigningService.SignCapabilityInvocationAsync(capability, action, target, signerDid, appDoc?)→ a secured application document with acapabilityInvocationproof. The proof object alone carriescapability/capabilityAction/invocationTarget(db's model); the signed document is the application payload (default carries@context=[zcap/v1, ed25519-2020/v1]+id+ an absolute-IRItype— both required so it expands to a non-empty RDF dataset under JSON-LD safe mode). Signing input =SHA-256(RDFC(proofOptions)) || SHA-256(RDFC(document))via the existingLegacyProofCryptopath.VerificationService.VerifyCapabilityInvocationAsync(JsonObject securedDocument[, root])→ verifies the signature over the application document, then reuses the chain / attenuation / caveat / controller-auth / freshness / replay checks. Root id-string vs embedded-delegated-zcap shapes per Support delegated DI invocation capability as full zcap object #51.Proven live (harness-first)
A subagent built the db-native DI invocation harness; the .NET side was matched to its exact wire shape.
interop/run-interop.shnow runs 12 checks — 6 delegation + 6 invocation:@digitalbazaar/zcapverifies@digitalbazaar/zcapsigns, dotnet verifiescapabilityAction, both directionsTests
DataIntegrityInvocationTests(root + delegated in-stack round-trip, tampered-action → InvalidSignature, action-not-in-allowedAction → ActionNotAllowed). Suite: 460 Core + 33 AspNetCore green.Not in scope
HTTP-Signature invocations (the deployed
ezcaptransport, "Path B") — tracked in #119.🤖 Generated with Claude Code