From 2c0ce018d091687e68d784439c5f3f0fd899f0de Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Mon, 31 Aug 2026 16:39:04 +0200 Subject: [PATCH] fix: read the granted notice at the envelope shape the VTA sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Approving a task no longer auto-publishes on the requester's page. The operator approves on their device, returns to the site, and the banner still reads "this will publish automatically the moment you approve" — until they press the manual "Publish now" fallback. `parseTaskConsentGranted` was reading the pre-spec wire shape, in two ways that are each independently fatal: - It compared `message.type` against the *task* type. Over DIDComm that member is the envelope type, so the check never matched and the function returned `null` on its first line for every notice ever sent. - It read `message.body.payloadDigest`. The VTA sends a full Trust Task document as the body, so the digest is at `body.payload.payloadDigest`. The VTA moved this notice from a bare `{status, payloadDigest, taskType}` body to a `task-consent/granted/0.1` document inside a `TRUST_TASK_ENVELOPE_TYPE` envelope. `parseTaskConsentRequest`, three hundred lines up the same file, was migrated with it. This was not. Nothing failed loudly, which is why it survived. The requester's page listens for the `consentgranted` event to replay its pinned re-submit, and deliberately runs no timer poll for re-submitting — a blind retry loop would reopen the wallet's un-skippable confirm on every tick. So a dropped notice is indistinguishable from an approver who has not answered yet. The digests were never the problem: the VTA puts `wire_digest` in both the `consentRequired` refusal the page holds and the granted notice, so the match succeeds as soon as the event actually fires. The in-band `issuer` now gets the same treatment as the transport sender — checked when present, tolerated when absent. The notice is unsigned by design, so that is a cheap filter, not an authentication; the page's digest match remains the guard. Tests: the five that covered this asserted the pre-spec form, so they passed throughout while the feature was dead. They now build the fixture the way `push_granted` emits it (vta-service `trust_tasks/consent_request.rs`), and three of them fail against the old parser. Added: an envelope carrying a different task is ignored, an impostor in-band issuer is rejected, and the pre-spec bare body is refused outright — pinning one wire form so a later edit cannot restore the dual-shape tolerance that would hide this again. Signed-off-by: Glenn Gore --- packages/core/src/inbound/task-consent.ts | 56 +++++++++++---- packages/core/tests/inbound.task-consent.mjs | 71 +++++++++++++++++--- 2 files changed, 104 insertions(+), 23 deletions(-) diff --git a/packages/core/src/inbound/task-consent.ts b/packages/core/src/inbound/task-consent.ts index 43de18d..8f443da 100644 --- a/packages/core/src/inbound/task-consent.ts +++ b/packages/core/src/inbound/task-consent.ts @@ -206,27 +206,59 @@ export function parseTaskConsentOutcome( /** * Parse a VTA→requester `task-consent/granted` notice. * - * The VTA sends it as a plaintext DIDComm message whose `type` is the granted - * type and whose `body` carries the salted `payloadDigest` the requester already - * holds. It is a **non-load-bearing nudge**: it only tells the requester to - * re-submit now instead of polling, and the single-use grant check on that - * re-submit is the real gate — so this needs no Data-Integrity proof. We still - * accept it only from this device's enrolled VTA (the authcrypt sender), and the - * page re-checks the digest against its outstanding approval before acting. + * The VTA sends a **full Trust Task document inside a DIDComm envelope**, the + * same binding {@link parseTaskConsentRequest} reads: the DIDComm `type` is + * {@link TRUST_TASK_ENVELOPE_TYPE}, `body` is the document, and the salted + * `payloadDigest` the requester already holds sits in `body.payload`. + * + * It is a **non-load-bearing nudge**: it only tells the requester to re-submit + * now instead of polling, and the single-use grant check on that re-submit is + * the real gate — so this needs no Data-Integrity proof. We still accept it only + * from this device's enrolled VTA (the authcrypt sender), and the page re-checks + * the digest against its outstanding approval before acting. + * + * # It used to read the pre-spec shape + * + * This matched `message.type` against the *task* type and read + * `message.body.payloadDigest` — the bare `{status, payloadDigest, taskType}` + * body the VTA sent before the notice gained its envelope. Both are wrong + * against the current wire, and either alone is fatal: the DIDComm `type` is the + * envelope type, so the first check never matched and this returned `null` on + * every notice ever sent. + * + * Nothing failed loudly. The requester's page listens for the resulting + * `consentgranted` event to replay its pinned re-submit, and deliberately runs + * no timer poll for re-submitting (a blind retry loop would reopen the wallet's + * un-skippable confirm on every tick). So a dropped notice is indistinguishable + * from an approver who has not answered yet: the page sat on "this will publish + * automatically the moment you approve" until the operator pressed the manual + * fallback button. + * + * The sibling request parser was migrated to the envelope; this was not. Build + * fixtures at the shape the peer actually emits — the tests that covered this + * asserted the pre-spec form, so they passed throughout. */ export function parseTaskConsentGranted( message: Record, expectedVtaDid: string, ): { payloadDigest: string } | null { - if (message.type !== TASK_CONSENT_GRANTED_TYPE) return null; + if (message.type !== TRUST_TASK_ENVELOPE_TYPE) return null; const from = typeof message.from === "string" ? message.from : null; // If the transport surfaced a sender, it must be our VTA; a missing sender // is tolerated (the page-side digest match is the ultimate guard). if (from && from !== expectedVtaDid) return null; - const body = (message.body ?? {}) as { payloadDigest?: unknown }; - return typeof body.payloadDigest === "string" - ? { payloadDigest: body.payloadDigest } - : null; + const doc = (message.body ?? {}) as { + type?: unknown; + issuer?: unknown; + payload?: { payloadDigest?: unknown }; + }; + if (doc.type !== TASK_CONSENT_GRANTED_TYPE) return null; + // The in-band issuer gets the same treatment as the transport sender: checked + // when present, tolerated when absent. The notice is unsigned by design, so + // this is a cheap filter and not an authentication. + if (typeof doc.issuer === "string" && doc.issuer !== expectedVtaDid) return null; + const digest = doc.payload?.payloadDigest; + return typeof digest === "string" ? { payloadDigest: digest } : null; } /** SPEC §7.3 item 13 — the integrity effect of executing the task. */ diff --git a/packages/core/tests/inbound.task-consent.mjs b/packages/core/tests/inbound.task-consent.mjs index fc0b643..5c0cc8e 100644 --- a/packages/core/tests/inbound.task-consent.mjs +++ b/packages/core/tests/inbound.task-consent.mjs @@ -236,32 +236,81 @@ test("a denial is an explicit decision, not an absent one", async () => { }); // ── task-consent/granted (the pub/sub nudge) ────────────────────────────── +// +// Every fixture below is built the way `push_granted` on the VTA actually emits +// the notice (vta-service `trust_tasks/consent_request.rs`): a DIDComm envelope +// whose `body` is a full `task-consent/granted/0.1` Trust Task document, with +// the salted digest under `payload`. +// +// That shape is the point of these tests. They previously asserted the pre-spec +// form — the task type as the DIDComm `type`, and a bare +// `{status, payloadDigest, taskType}` body — which the VTA stopped sending when +// the notice gained its envelope. The parser matched the fixtures rather than +// the wire, so it returned `null` for every real notice while these passed, and +// the requester's page never auto-published. -test("parseTaskConsentGranted accepts a granted notice from our VTA", () => { - const msg = { - type: TASK_CONSENT_GRANTED_TYPE, - from: VTA.did, - body: { status: "granted", payloadDigest: "abc123", taskType: "t" }, +/** The notice exactly as the VTA puts it on the wire. */ +function grantedEnvelope({ digest = "zQmGrantedDigest", from = VTA.did, issuer = VTA.did } = {}) { + return { + ...(from ? { from } : {}), + type: TRUST_TASK_ENVELOPE_TYPE, + body: { + id: "urn:uuid:2b0f6a1e-64f2-4a1e-9a6e-1f0f4a0d7c31", + type: TASK_CONSENT_GRANTED_TYPE, + threadId: "urn:uuid:0d2f6b1a-1c3e-4f5a-8b7c-9e0d1a2b3c4d", + recipient: HOLDER, + ...(issuer ? { issuer } : {}), + issuedAt: "2026-08-31T14:00:00Z", + payload: { status: "granted", payloadDigest: digest, taskType: "t" }, + }, }; - assert.deepEqual(parseTaskConsentGranted(msg, VTA.did), { payloadDigest: "abc123" }); +} + +test("parseTaskConsentGranted accepts the enveloped notice the VTA sends", () => { + assert.deepEqual(parseTaskConsentGranted(grantedEnvelope(), VTA.did), { + payloadDigest: "zQmGrantedDigest", + }); }); -test("parseTaskConsentGranted ignores a non-granted message type", () => { +test("parseTaskConsentGranted ignores a non-envelope message type", () => { const msg = { type: "other", from: VTA.did, body: { payloadDigest: "x" } }; assert.equal(parseTaskConsentGranted(msg, VTA.did), null); }); +test("parseTaskConsentGranted ignores an envelope carrying another task", () => { + const msg = grantedEnvelope(); + msg.body.type = TASK_CONSENT_REQUEST_TYPE; + assert.equal(parseTaskConsentGranted(msg, VTA.did), null); +}); + test("parseTaskConsentGranted rejects a sender that is not our VTA", () => { - const msg = { type: TASK_CONSENT_GRANTED_TYPE, from: IMPOSTOR.did, body: { payloadDigest: "x" } }; + assert.equal(parseTaskConsentGranted(grantedEnvelope({ from: IMPOSTOR.did }), VTA.did), null); +}); + +test("parseTaskConsentGranted rejects an in-band issuer that is not our VTA", () => { + const msg = grantedEnvelope({ from: null, issuer: IMPOSTOR.did }); assert.equal(parseTaskConsentGranted(msg, VTA.did), null); }); test("parseTaskConsentGranted tolerates a missing sender (page re-checks the digest)", () => { - const msg = { type: TASK_CONSENT_GRANTED_TYPE, body: { payloadDigest: "x" } }; - assert.deepEqual(parseTaskConsentGranted(msg, VTA.did), { payloadDigest: "x" }); + const msg = grantedEnvelope({ from: null, issuer: null }); + assert.deepEqual(parseTaskConsentGranted(msg, VTA.did), { payloadDigest: "zQmGrantedDigest" }); }); test("parseTaskConsentGranted requires a string payloadDigest", () => { - const msg = { type: TASK_CONSENT_GRANTED_TYPE, from: VTA.did, body: {} }; + const msg = grantedEnvelope(); + delete msg.body.payload.payloadDigest; + assert.equal(parseTaskConsentGranted(msg, VTA.did), null); +}); + +// The regression, stated as the shape it was: a bare pre-spec body must not be +// accepted. Keeping it pins the parser to one wire form, so a future edit cannot +// quietly restore the dual-shape tolerance that hid the break. +test("parseTaskConsentGranted rejects the pre-spec bare body", () => { + const msg = { + type: TASK_CONSENT_GRANTED_TYPE, + from: VTA.did, + body: { status: "granted", payloadDigest: "abc123", taskType: "t" }, + }; assert.equal(parseTaskConsentGranted(msg, VTA.did), null); });