Skip to content

verifiable agreement signatures - #182

Draft
sanketshevkar wants to merge 2 commits into
accordproject:mainfrom
sanketshevkar:sanketshvkar/verifiable-signatures-agreements
Draft

verifiable agreement signatures#182
sanketshevkar wants to merge 2 commits into
accordproject:mainfrom
sanketshevkar:sanketshvkar/verifiable-signatures-agreements

Conversation

@sanketshevkar

Copy link
Copy Markdown
Member

No description provided.

@sanketshevkar
sanketshevkar marked this pull request as draft May 28, 2026 20:04
Comment on lines +246 to +261
crudRouter.post('/:id/verify-signatures', async function (req, res) {
try {
const agreement = await loadAgreementRow(res.locals.db, req.params.id);
const lite: AgreementLite = {
uri: agreement.uri,
data: agreement.data,
templateHash: agreement.templateHash ?? null,
agreementParties: agreement.agreementParties ?? [],
signatures: agreement.signatures ?? [],
};
const result = await verifyAllSignatures(lite);
res.json(result);
} catch (err: any) {
res.status(500).json({ error: 'Verification failed', details: err.message });
}
});
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open with no activity. Remove the stale label or comment to keep it active. Only items with maintainer engagement are auto-closed.

@github-actions github-actions Bot added the Stale label Jun 13, 2026
@niallroche

Copy link
Copy Markdown
Contributor

his is a good direction as discussed on a previous working group call @sanketshevkar — the JCS canonicalization and the SIGNNG → SIGNING fix are both solid.

A couple of shape suggestions while it's still in draft, aimed at keeping the door open for more signature types later:

I would suggesting typing the hashes with the new org.accordproject.crypto@1.0.0 model that I proposed. It landed recently with ContentHash / HashAlgorithm / Canonicalization primitives (enum-plus-CUSTOM). Typing agreementDataHash / templateHash as crypto.ContentHash rather than bare strings makes the algorithm and the canonicalization scheme explicit and self-describing — which is exactly the property you want on a hash that a verifier re-derives. It also means the canonicalization choice is declared in the data instead of implied by the code.

Add a method discriminator to the signature record. The record is already nicely method-agnostic — verifiableCredential is optional and signatureImage is there for non-VC signatures — so tagging each record with the method that produced it (vc-did, sign-on-glass, …) costs almost nothing now and makes any future signature type an additive change rather than a migration to retrofit a discriminator onto existing rows.

Minor: server/vendor/vc-signer-0.1.2.tgz — vendoring the signing lib as an in-repo tarball is a supply-chain/maintenance consideration. If it can be a published dependency that'd be easier to audit and update; if it has to stay vendored for now, a one-line note on why would help future maintainers.

None of these are blocking — they're about keeping the record shape open as more methods and the crypto/agentic-commerce model family come online.

@niallroche

Copy link
Copy Markdown
Contributor

@sanketshevkar

Copy link
Copy Markdown
Member Author

Thanks @niallroche for the review, I had raised this as a draft PR for reviewing for the discussion we had started a couple of months back.

The tarball package included was a part of the demo, I still don't have a concrete answer on owning a utility specific for signing and validating VCs and DIDs specific to AP templates. Or maybe we'd just want to have it as a reference implementation in APAP.

@github-actions github-actions Bot added the maintainer-engaged A maintainer has commented or reviewed this item label Aug 6, 2026
@JayDS22

JayDS22 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Not blocking, small heads-up from a scan today per @niallroche's Aug 5 endpoint-check ask:

The diff touches model/protocol.cto and adds server/handlers/signatures.ts + server/handlers/vcSigner.ts, but I did not see openapi.json or openapi-extra-paths.json in the changed-files list. If signatures.ts declares its own REST routes (or attaches new ones to the agreements router), the OpenAPI + generated TypeScript client will drift silently unless npm run gen && npm run merge runs at the repo root after the code changes settle.

Sequence I would follow once the current CONFLICTING state is rebased:

  1. cd apap && npm run gen (Concerto protocol.cto -> openapi.json)
  2. cd apap && npm run merge (folds openapi-extra-paths.json into openapi.json for non-CRUD verbs like sign / verify)
  3. commit the regenerated openapi.json + client/typescript/apap.ts in the same PR

Happy to help wire the extra-paths entries if the signing surface exposes new REST verbs beyond what CRUD generation covers. Flagging early so it does not surface as a post-merge follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-engaged A maintainer has commented or reviewed this item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants