Skip to content

Commit 3267363

Browse files
authored
fix(consent): read the executor's answer to a decision, don't drop it (#112)
Sending a decision is not the end of the ceremony. The executor replies on the same DIDComm thread — a `decision/0.1#response` on success, a `trust-task-error/{0.1,0.2}` on refusal — and nothing here recognised either shape. Both fell through `onInboundMessage`'s final "anything else is ignored" branch: no log, no surface, nothing. So a refused approval was indistinguishable, from this side, from one that worked. The human was shown a change, agreed to it, the VTA said no, and the wallet discarded the reason — while the requester re-submitted forever and the operator had no way to tell which end was at fault. That is strictly worse than a lost prompt, because the person believes they have acted. `parseTaskConsentOutcome` reads the answer. `buildTaskConsentDecision` now returns the decision's document id alongside the packed message, so the caller can match the reply to the decision it sent and say *which* approval was refused rather than that one was. A refusal raises a notification, not just a console line: it contradicts something the user was shown seconds earlier and agreed to, so it has to reach them where they are. Only the authcrypt sender is trusted, and only to decide whether to believe the reply. An unenrolled sender is dropped — an unauthenticated party must not be able to tell this device an approval failed (an invitation to approve a second time) or that one succeeded. Nothing in the outcome grants anything; the executor's grant remains the authority. The awaiting-decision map is in-memory, bounded and best-effort by design. It explains an outcome, never decides one, so an MV3 teardown losing it costs a good log line and not correctness — persisting it would add a write to the consent hot path and buy nothing. `not-a-task-consent-request` remains the only reason a caller may ignore an inbound message, and the outcome check runs before the request parser so a reply cannot be mistaken for one. A consent request still falls through to the parser that prompts a human. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
1 parent 1924646 commit 3267363

3 files changed

Lines changed: 471 additions & 7 deletions

File tree

packages/core/src/inbound/task-consent.ts

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@
3939

4040
import { packAuthcrypt, packAuthcryptJson, wrapForward, type Identity } from "../didcomm/index.js";
4141
import type { RemoteDidcommEndpoint } from "../vta/didcomm.js";
42-
import { TRUST_TASK_ENVELOPE_TYPE, type TrustTask } from "../vta/protocol.js";
42+
import {
43+
TRUST_TASK_ENVELOPE_TYPE,
44+
isTrustTaskErrorType,
45+
type TrustTask,
46+
type TrustTaskErrorPayload,
47+
} from "../vta/protocol.js";
4348
import { signTrustTask } from "../trust-tasks/sign.js";
4449
import { verifyTrustTaskProof } from "../trust-tasks/verify.js";
4550
import type { SigningIdentity } from "../siop/self-issued.js";
@@ -51,6 +56,118 @@ export const TASK_CONSENT_DECISION_TYPE =
5156
/** VTA → requester: an approval landed and a grant is ready — re-submit now. */
5257
export const TASK_CONSENT_GRANTED_TYPE =
5358
"https://trusttasks.org/spec/task-consent/granted/0.1";
59+
/** The executor's acknowledgement of a decision this device sent. */
60+
export const TASK_CONSENT_DECISION_RESPONSE_TYPE = `${TASK_CONSENT_DECISION_TYPE}#response`;
61+
62+
/**
63+
* What the executor did with a decision this device sent.
64+
*
65+
* `accepted: false` is the case that matters. A refusal means a human was
66+
* shown a change, agreed to it, and the agreement did not take — which is
67+
* strictly worse than a prompt that never arrived, because the person believes
68+
* they have acted. It has to reach them.
69+
*/
70+
export type TaskConsentOutcome =
71+
| {
72+
accepted: true;
73+
/** `granted` = threshold met, the requester can execute. `pending` =
74+
* recorded, more approvals needed. `denied` = the request was aborted,
75+
* which is a successful *outcome* of a `deny`, not a failure. */
76+
status: string;
77+
approvals?: number;
78+
needed?: number;
79+
payloadDigest?: string;
80+
/** The decision document id this answers, when the reply carried one. */
81+
thid?: string;
82+
}
83+
| {
84+
accepted: false;
85+
/** Framework status code — snake_case in error/0.1, lowerCamelCase in
86+
* 0.2. Opaque: log it, don't branch on a casing. */
87+
code: string;
88+
message?: string;
89+
retryable: boolean;
90+
details?: unknown;
91+
thid?: string;
92+
};
93+
94+
/**
95+
* Parse the executor's reply to a `task-consent/decision` this device sent.
96+
*
97+
* Returns `null` for anything that is not such a reply — that is the only case
98+
* a caller may ignore.
99+
*
100+
* ## Why this exists
101+
*
102+
* The executor answers a decision on the same DIDComm thread, as a Trust-Task
103+
* envelope: a `decision/0.1#response` document on success, a
104+
* `trust-task-error/{0.1,0.2}` on refusal. Nothing here recognised either, so
105+
* both fell through the inbound handler's final "anything else is ignored"
106+
* branch — no log, no surface, nothing.
107+
*
108+
* That is how an approval refused by the VTA looked identical, from this side,
109+
* to one that was delivered and worked: the human approved, the wallet sent,
110+
* the executor replied "no", and the wallet discarded the reply. The operator
111+
* then watched the requester re-submit forever with no clue which end was at
112+
* fault. Reading the answer is the difference between a two-minute diagnosis
113+
* and an afternoon of packet-staring.
114+
*
115+
* ## What is trusted
116+
*
117+
* Only the authcrypt sender, and only to decide whether to *believe* the
118+
* reply — it is diagnostic, and grants nothing. A reply whose sender is not an
119+
* enrolled executor is dropped: an unauthenticated party must not be able to
120+
* tell this device that its approval failed (a lie that invites the human to
121+
* approve a second time), nor that it succeeded.
122+
*/
123+
export function parseTaskConsentOutcome(
124+
message: Record<string, unknown>,
125+
opts: { enrolledExecutorDids: readonly string[] },
126+
): TaskConsentOutcome | null {
127+
if (message.type !== TRUST_TASK_ENVELOPE_TYPE) return null;
128+
129+
// A missing `from` means the transport could not authenticate the sender.
130+
// Unlike the `granted` nudge — which is cross-checked against a digest the
131+
// page already holds — nothing downstream re-verifies this, so an
132+
// unattributable reply is dropped rather than believed.
133+
const from = typeof message.from === "string" ? message.from : null;
134+
if (!from || !opts.enrolledExecutorDids.includes(from)) return null;
135+
136+
const doc = (message.body ?? {}) as Partial<TrustTask<Record<string, unknown>>>;
137+
const thid =
138+
(typeof message.thid === "string" ? message.thid : undefined) ??
139+
(typeof doc.threadId === "string" ? doc.threadId : undefined);
140+
141+
if (isTrustTaskErrorType(doc.type)) {
142+
const payload = (doc.payload ?? {}) as Partial<TrustTaskErrorPayload>;
143+
return {
144+
accepted: false,
145+
code: typeof payload.code === "string" ? payload.code : "unknown",
146+
...(typeof payload.message === "string" ? { message: payload.message } : {}),
147+
// The framework schema requires `retryable`; treat a missing one as
148+
// "don't retry" rather than inventing optimism about a refusal.
149+
retryable: payload.retryable === true,
150+
...(payload.details !== undefined ? { details: payload.details } : {}),
151+
...(thid ? { thid } : {}),
152+
};
153+
}
154+
155+
if (doc.type === TASK_CONSENT_DECISION_RESPONSE_TYPE) {
156+
const payload = (doc.payload ?? {}) as Record<string, unknown>;
157+
return {
158+
accepted: true,
159+
status: typeof payload.status === "string" ? payload.status : "unknown",
160+
...(typeof payload.approvals === "number" ? { approvals: payload.approvals } : {}),
161+
...(typeof payload.needed === "number" ? { needed: payload.needed } : {}),
162+
...(typeof payload.payloadDigest === "string"
163+
? { payloadDigest: payload.payloadDigest }
164+
: {}),
165+
...(thid ? { thid } : {}),
166+
};
167+
}
168+
169+
return null;
170+
}
54171

55172
/**
56173
* Parse a VTA→requester `task-consent/granted` notice.
@@ -362,10 +479,23 @@ export async function buildTaskConsentDecisionDocument(
362479
return document;
363480
}
364481

482+
/** A `task-consent/decision` ready to send, and the id to recognise its
483+
* answer by. */
484+
export interface BuiltTaskConsentDecision {
485+
/** The packed, mediator-routed wire message. */
486+
packed: string;
487+
/** The decision document's id. The executor answers on this thread
488+
* (`thid`), so a caller that keeps it can match the reply to the decision
489+
* it sent — and therefore tell the human *which* approval was refused.
490+
* Returned rather than left inside the opaque packed blob because the
491+
* alternative is not correlating at all, which is where this started. */
492+
id: string;
493+
}
494+
365495
/** Build the authcrypted, mediator-routed `task-consent/decision` wire message. */
366496
export async function buildTaskConsentDecision(
367497
args: BuildTaskConsentDecisionArgs,
368-
): Promise<string> {
498+
): Promise<BuiltTaskConsentDecision> {
369499
const document = await buildTaskConsentDecisionDocument(args);
370500

371501
const message = {
@@ -381,7 +511,8 @@ export async function buildTaskConsentDecision(
381511
{ kid: args.vta.keyAgreementKid, jwk: args.vta.keyAgreementPublicJwk },
382512
]);
383513
const forwardJson = wrapForward(args.vta.did, args.holder.did, args.mediator.did, inner);
384-
return packAuthcryptJson(forwardJson, args.holder, [
514+
const packed = await packAuthcryptJson(forwardJson, args.holder, [
385515
{ kid: args.mediator.keyAgreementKid, jwk: args.mediator.keyAgreementPublicJwk },
386516
]);
517+
return { packed, id: document.id };
387518
}
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
// The executor's answer to a decision this device sent.
2+
//
3+
// A refusal is the worst inbound event in the ceremony: a human was shown a
4+
// change, agreed to it, and the agreement did not take — so unlike a lost
5+
// prompt, the person believes they have acted. The wallet used to drop that
6+
// reply unread, because nothing recognised it and the inbound handler's final
7+
// branch ignores what it cannot name. An approval the VTA rejected then looked,
8+
// from this side, exactly like one that worked.
9+
//
10+
// These pin the two halves that matter: the answer is *read*, and it is read
11+
// only when it comes from an executor this device is enrolled with.
12+
13+
import { test } from "node:test";
14+
import assert from "node:assert/strict";
15+
16+
import {
17+
parseTaskConsentOutcome,
18+
TASK_CONSENT_DECISION_RESPONSE_TYPE,
19+
} from "../dist/inbound/task-consent.js";
20+
import {
21+
TRUST_TASK_ENVELOPE_TYPE,
22+
TRUST_TASK_ERROR_TYPE,
23+
TRUST_TASK_ERROR_TYPE_0_2,
24+
} from "../dist/vta/protocol.js";
25+
26+
const VTA = "did:webvh:zScid:vta.example:glenn-vta";
27+
const OPTS = { enrolledExecutorDids: [VTA] };
28+
const THID = "urn:uuid:decision-1";
29+
30+
function envelope(body, overrides = {}) {
31+
return {
32+
id: "urn:uuid:reply-1",
33+
type: TRUST_TASK_ENVELOPE_TYPE,
34+
from: VTA,
35+
to: ["did:key:zApprover"],
36+
thid: THID,
37+
body,
38+
...overrides,
39+
};
40+
}
41+
42+
function errorDoc(type, payload) {
43+
return { id: "urn:uuid:err-1", type, threadId: THID, payload };
44+
}
45+
46+
test("a permissionDenied refusal is read, not dropped", () => {
47+
// Precisely the reply that went unread in the field: the transport gate
48+
// refused the approver, and the wallet said nothing.
49+
const outcome = parseTaskConsentOutcome(
50+
envelope(
51+
errorDoc(TRUST_TASK_ERROR_TYPE_0_2, {
52+
code: "permissionDenied",
53+
message: "DID not in ACL: did:key:zApprover",
54+
retryable: false,
55+
}),
56+
),
57+
OPTS,
58+
);
59+
assert.ok(outcome, "the refusal must be recognised");
60+
assert.equal(outcome.accepted, false);
61+
assert.equal(outcome.code, "permissionDenied");
62+
assert.equal(outcome.retryable, false);
63+
assert.match(outcome.message, /not in ACL/);
64+
assert.equal(outcome.thid, THID, "correlates to the decision we sent");
65+
});
66+
67+
test("the 0.1 error type is read too, with its snake_case code left alone", () => {
68+
// `code` is opaque: 0.1 says permission_denied, 0.2 says permissionDenied.
69+
// Normalising here would invite a caller to branch on one casing.
70+
const outcome = parseTaskConsentOutcome(
71+
envelope(
72+
errorDoc(TRUST_TASK_ERROR_TYPE, { code: "permission_denied", retryable: false }),
73+
),
74+
OPTS,
75+
);
76+
assert.equal(outcome.accepted, false);
77+
assert.equal(outcome.code, "permission_denied");
78+
});
79+
80+
test("details ride through — that is where a task-specific reason lives", () => {
81+
const outcome = parseTaskConsentOutcome(
82+
envelope(
83+
errorDoc(TRUST_TASK_ERROR_TYPE_0_2, {
84+
code: "taskFailed",
85+
retryable: false,
86+
details: { payloadDigest: "abc123" },
87+
}),
88+
),
89+
OPTS,
90+
);
91+
assert.deepEqual(outcome.details, { payloadDigest: "abc123" });
92+
});
93+
94+
test("a missing retryable reads as not-retryable, never as optimism", () => {
95+
const outcome = parseTaskConsentOutcome(
96+
envelope(errorDoc(TRUST_TASK_ERROR_TYPE_0_2, { code: "internalError" })),
97+
OPTS,
98+
);
99+
assert.equal(outcome.retryable, false);
100+
});
101+
102+
test("an accepted decision reports the status and the tally", () => {
103+
const outcome = parseTaskConsentOutcome(
104+
envelope({
105+
id: "urn:uuid:ok-1",
106+
type: TASK_CONSENT_DECISION_RESPONSE_TYPE,
107+
threadId: THID,
108+
payload: { status: "granted", payloadDigest: "abc123", approvals: 1 },
109+
}),
110+
OPTS,
111+
);
112+
assert.equal(outcome.accepted, true);
113+
assert.equal(outcome.status, "granted");
114+
assert.equal(outcome.approvals, 1);
115+
assert.equal(outcome.payloadDigest, "abc123");
116+
});
117+
118+
test("a partial approval is accepted, and says how many more are needed", () => {
119+
const outcome = parseTaskConsentOutcome(
120+
envelope({
121+
id: "urn:uuid:ok-2",
122+
type: TASK_CONSENT_DECISION_RESPONSE_TYPE,
123+
threadId: THID,
124+
payload: { status: "pending", payloadDigest: "abc123", approvals: 1, needed: 2 },
125+
}),
126+
OPTS,
127+
);
128+
assert.equal(outcome.accepted, true);
129+
assert.equal(outcome.status, "pending");
130+
assert.equal(outcome.needed, 2);
131+
});
132+
133+
test("a reply from anyone but an enrolled executor is not believed", () => {
134+
// An unauthenticated party must not be able to tell this device its approval
135+
// failed — that is an invitation to approve a second time — nor that one
136+
// succeeded when it did not.
137+
const outcome = parseTaskConsentOutcome(
138+
envelope(errorDoc(TRUST_TASK_ERROR_TYPE_0_2, { code: "permissionDenied", retryable: false }), {
139+
from: "did:key:zSomeoneElse",
140+
}),
141+
OPTS,
142+
);
143+
assert.equal(outcome, null);
144+
});
145+
146+
test("an unattributable reply is dropped", () => {
147+
// No `from` means the transport could not authenticate the sender, and
148+
// nothing downstream re-verifies this document.
149+
const outcome = parseTaskConsentOutcome(
150+
envelope(errorDoc(TRUST_TASK_ERROR_TYPE_0_2, { code: "permissionDenied", retryable: false }), {
151+
from: undefined,
152+
}),
153+
OPTS,
154+
);
155+
assert.equal(outcome, null);
156+
});
157+
158+
test("anything that is not an answer returns null, so other handlers still see it", () => {
159+
// The one case a caller may ignore. A consent *request* must fall through to
160+
// the parser that prompts a human — returning an outcome here would swallow
161+
// the prompt, which is the failure this whole module exists to prevent.
162+
for (const body of [
163+
{ id: "x", type: "https://trusttasks.org/spec/task-consent/request/0.1", payload: {} },
164+
{ id: "x", type: "https://trusttasks.org/spec/vta/webvh/dids/update/1.0", payload: {} },
165+
{},
166+
]) {
167+
assert.equal(parseTaskConsentOutcome(envelope(body), OPTS), null);
168+
}
169+
assert.equal(
170+
parseTaskConsentOutcome(
171+
{ type: "https://didcomm.org/messagepickup/3.0/status", from: VTA },
172+
OPTS,
173+
),
174+
null,
175+
);
176+
});
177+
178+
test("the thid falls back to the document threadId when the envelope omits it", () => {
179+
const outcome = parseTaskConsentOutcome(
180+
envelope(errorDoc(TRUST_TASK_ERROR_TYPE_0_2, { code: "permissionDenied", retryable: false }), {
181+
thid: undefined,
182+
}),
183+
OPTS,
184+
);
185+
assert.equal(outcome.thid, THID);
186+
});
187+
188+
test("an answer with no correlation at all is still reported", () => {
189+
// Losing the thread costs detail, never the report: a refusal the wallet
190+
// cannot match to a specific decision is still a refusal the human needs.
191+
const outcome = parseTaskConsentOutcome(
192+
envelope({ id: "e", type: TRUST_TASK_ERROR_TYPE_0_2, payload: { code: "taskFailed", retryable: false } }, {
193+
thid: undefined,
194+
}),
195+
OPTS,
196+
);
197+
assert.equal(outcome.accepted, false);
198+
assert.equal(outcome.thid, undefined);
199+
});

0 commit comments

Comments
 (0)