diff --git a/LESSONS.md b/LESSONS.md index 2764f4d..786105f 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -51,3 +51,15 @@ a lint/test) so the next agent never repeats it. Read after `AGENTS.md` + **Fix:** idempotency guard added to the npm job (the manual interactive-2FA ritual and the tag workflow can no longer race — v0.8.0's manual publish beat CI by 5 seconds); PyPI split onto `py-v*` tags (v0.8.0 tag failed against pyproject 0.1.0). Durable fix queued in READY-QUEUE: npm **Trusted Publisher** (OIDC) for `s402`, same pattern as `pinia-colada-plugin-normalizer` — no token, no rot, provenance attested. **For future agents:** on npm publish failures, treat E404-on-PUT as *authentication* first. And treat any long-lived registry token in CI as a rot liability — prefer OIDC trusted publishing; failing that, add a scheduled `npm whoami` canary so token death is discovered before release day. + +--- + +## [2026-08-16] — A guard that is red on a clean checkout is the guard getting deleted, not enforced + +**Mistake:** while fixing DAN-860 I added `scripts/check-vector-sync.sh` plus a CI job asserting that `spec/vectors/` and `typescript/test/conformance/vectors/` stay identical. It passed locally three ways — healthy `0`, planted drift `1`, missing-dir `2` — and then failed on the very first CI run with `published dir missing`. `typescript/test/conformance/vectors/` is **gitignored build output**: `typescript/scripts/prepare-publish.sh` copies `spec/vectors/*.json` into it during `prepublishOnly`. A clean checkout correctly does not have it, so the check would have been red on every commit forever. + +**Why it happened:** I inferred the relationship between the two directories from their *contents* — byte-identical across all 14 files — and wrote "kept in sync by discipline" into an ADR, a PR body, and a Linear comment before reading `.gitignore` or `package.json`'s `files` array. Identical bytes are equally consistent with "two hand-maintained copies" and "one is generated from the other," and only the second is true. DAN-860's own criterion 4 made the same inference in the other direction, calling it a "stale duplicate ... a trap with no upside" — it is neither stale nor a duplicate, and deleting it would strip the conformance vectors from the published npm package. + +**Fix:** the script and the CI job were removed before merge; `ci.yml` is byte-identical to `main`. The accurate account of both directories is in ADR-012's Consequences. The genuinely uncovered gap — `spec/vectors/` is itself generated by `test/conformance/generate-vectors.ts` from the live `encode`/`decode` functions, and nothing asserts the tracked vectors still match what the code produces — is recorded there too, and deliberately *not* built, because `tsx` is not an installed dependency and the check could not be watched green. + +**For future agents:** before adding any gate, run it against a **fresh clone**, not your working tree — your tree contains generated artifacts, local caches and untracked files that CI will not have, and those are exactly what a naive gate keys on. Two more rules earned here: identical file contents tell you *what* is true, never *why*, so read `.gitignore` and the packaging manifest before concluding two directories are peers; and ship no gate you have not watched go green in the environment that will run it — an unverified gate is worse than an absent one, because its red is indistinguishable from a real defect and the cheapest way to make it stop is to delete it. diff --git a/docs/adr/012-unlock-identity-lives-in-the-fulfillment.md b/docs/adr/012-unlock-identity-lives-in-the-fulfillment.md new file mode 100644 index 0000000..7fbca02 --- /dev/null +++ b/docs/adr/012-unlock-identity-lives-in-the-fulfillment.md @@ -0,0 +1,100 @@ +# ADR-012: The unlock Seal identity lives in the fulfillment, not in requirements + +**Status:** Accepted +**Implementation:** shipped +**Force:** invariant +**Date:** 2026-08-16 +**Related:** ADR-008 (Safety Invariants S9–S13, S11 unlock attestation), ADR-010 (S15–S16), DAN-860, DAN-598 + +--- + +## Context + +`main` was red from 2026-07-21 to 2026-08-16 — three and a half weeks — on two conformance +tests. The proximate symptom was `unlock payload requires encryptionId (string), got NoneType`. + +The underlying condition was worse than a missing field. The Python and TypeScript +implementations of the *same* scheme had **disjoint requirements field sets — the +intersection was empty**: + +| Surface | `unlock` requirements fields | +|---|---| +| `python/src/s402/http.py` | `encryptionId`, `encryptedContentId`, `encryptionServiceId` | +| `typescript/src/types.ts`, `typescript/src/http.ts`, `spec/vectors/` | `packageId`, `keyServers`, `threshold`, `contentDigest` | +| `docs/specification.md` | `encryptionId` et al., marked **Required: Yes** | + +A Seal-style threshold key-server model had replaced an `encryptionId` model on 2026-07-20 +(recorded in `docs/proposals/REVIEW-NOTES.md` §6b and `DESIGN-unlock-vs-escrow.md`), and it +propagated to the vectors and the TypeScript runtime but not to Python or the written spec. + +Three positions were on record, and picking whichever one made CI green would have made a +spec bug permanent. + +## Decision + +**The Seal threshold key-server model is canonical.** `unlock` requirements carry +`packageId`, `keyServers`, `threshold`, and optional `contentDigest`. The payload carries +`transaction` and `signature` only. + +**No identity field appears in requirements or payload.** The Seal identity is +`receiptId ‖ nonce`, where `receiptId` is the object ID of the `UnlockReceipt` minted by +`sweefi::unlock::pay_and_mint`. It travels in the unlock **fulfillment** +(`s402UnlockFulfillment`), returned in `PAYMENT-RESPONSE` after payment. + +### The test that settles it, and it is reusable + +> **Can the server know this value at the moment it must send it?** + +Requirements ship in the `402` response, **before any payment exists**. `packageId`, +`keyServers`, `threshold` and `contentDigest` are all properties of how the seller *already* +encrypted the content, so the server knows them. `receiptId` is the object ID of a receipt +minted by a transaction the **buyer** constructs *in response to* that very 402. + +So `encryptionId` in requirements was never merely stale — it was **unsatisfiable by the +party required to populate it**. That is why this is an invariant rather than a preference: +no amount of implementation effort makes the server able to answer. + +**State the reason precisely, because the next reader acts on it.** It is *not* that "the +escrow does not exist yet" — under `seal_policy.move`'s escrow-based flow the `escrow_id` is +deterministic from signed TX1 bytes and the seller genuinely can encrypt before release. +`unlock` was rebuilt on 2026-07-20 as a **single-transaction** scheme with no escrow at all; +its anchor is the receipt's own object ID. An agent who goes looking for `escrowId` in the +unlock flow will not find it. (`REVIEW-NOTES.md` contains both statements — §3 item C says +`escrowId‖nonce`, §6b says the receipt's own object ID — and §6b is the later and correct one.) + +## Alternatives Considered + +- **Add `encryptionId` to the conformance vectors so Python passes.** Rejected, and it is + explicitly out of scope in DAN-860. The vectors are the contract between implementations; + editing one to match a single implementation silently re-specifies the protocol for every + external implementer. +- **Keep `encryptionId` required and have the server mint the receipt first.** Rejected — it + inverts the payment flow. The buyer would have to pay before receiving requirements. +- **Move `encryptionId` out of requirements into a fulfillment object** (the "third position" + in `REVIEW-NOTES.md`). **Chosen — and it is not a third position.** It is the Seal model + with the identity placed where the timeline allows. `s402UnlockFulfillment` already existed + in `typescript/src/types.ts` when this was written; there were only ever two positions and + one is falsified. + +## Consequences + +- **Positive:** `main` goes green (154/154 Python, 1102/1102 TypeScript). All five surfaces + agree. The wire contract matches the deployed Move module. +- **Positive:** the "can the server know this?" test is reusable at every future scheme + boundary and is cheaper than re-litigating field placement. +- **Negative:** any external consumer that populated `unlock.encryptionId` in requirements + breaks. Acceptable — that shape never validated against the published vectors, so no + conforming implementation can have depended on it. +- **Risk to watch:** `spec/vectors/` is *generated* by + `typescript/test/conformance/generate-vectors.ts`, which writes vectors by running the + live `encode`/`decode` functions. Nothing asserts that the tracked vectors still match + what the current code produces. That check is worth building and is **not** built here — + `tsx` is not an installed dependency, so it could not be watched green, and an unverified + gate is worse than an absent one. + + Note what is *not* a risk, because it looks like one: `typescript/test/conformance/vectors/` + is gitignored build output, produced by `scripts/prepare-publish.sh` copying `spec/vectors/` + at publish time, and that script hard-fails if the source is missing. It cannot drift — `cp` + has no opinions — and it is correctly absent from a clean checkout. DAN-860's criterion 4 + reads it as a "stale duplicate ... a trap with no upside"; it is neither stale nor a + duplicate, and deleting it would strip the vectors from the published npm package. diff --git a/docs/specification.md b/docs/specification.md index 615f228..92dc0a1 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -218,9 +218,16 @@ Required when `accepts` includes `"unlock"`. | Field | Type | Required | Constraints | Description | |-------|------|----------|-------------|-------------| -| `encryptionId` | string | Yes | — | Encryption key identifier for key servers. | -| `encryptedContentId` | string | Yes | — | Identifier for the encrypted content (e.g., Walrus blob ID, IPFS CID). | -| `encryptionServiceId` | string | Yes | — | Identifier for the encryption service or module (e.g., Sui package ID, EVM contract address). | +| `packageId` | string | Yes | — | Move package implementing `pay_and_mint` and the `seal_approve` policy; also the Seal identity namespace. | +| `keyServers` | array | Yes | Non-empty | Key-server set the seller encrypted to. Each entry is an object with `objectId` (string, on-chain registered key server) and `weight` (number). | +| `threshold` | number | Yes | Integer >= 1 | Threshold `t` in the t-of-n threshold encryption. | +| `contentDigest` | string | No | `sha256-` | Commitment to the plaintext, for off-chain/reputational evidence. | + +> **No identity field appears in requirements, and this is structural rather than an +> omission.** The Seal identity is `receiptId || nonce`, where `receiptId` is the object ID +> of the `UnlockReceipt` minted by `pay_and_mint`. That receipt does not exist when the +> server writes the 402 — it is created by the *buyer's* transaction, in response to this +> very response. The identity therefore travels in the unlock **fulfillment**, not here. ### 4.10 Prepaid Sub-Object @@ -297,9 +304,8 @@ The Payment Payload is sent by the client in the `x-payment` header (or request | Field | Type | Required | Description | |-------|------|----------|-------------| -| `payload.transaction` | string | Yes | Base64-encoded escrow creation transaction (TX1 of two-stage flow). | +| `payload.transaction` | string | Yes | Base64-encoded signed `pay_and_mint` transaction. Single-transaction scheme — there is no TX1/TX2 split. | | `payload.signature` | string | Yes | Base64-encoded signature. | -| `payload.encryptionId` | string | Yes | Encryption key identifier. Must match requirements. | **Prepaid** (`scheme: "prepaid"`): @@ -429,7 +435,7 @@ Sub-object known keys: - **settlementOverrides**: `actualAmount` - **stream**: `ratePerSecond`, `budgetCap`, `minDeposit`, `streamSetupUrl` - **escrow**: `seller`, `arbiter`, `deadlineMs` -- **unlock**: `encryptionId`, `encryptedContentId`, `encryptionServiceId` +- **unlock**: `packageId`, `keyServers`, `threshold`, `contentDigest` - **prepaid**: `ratePerCall`, `maxCalls`, `minDeposit`, `withdrawalDelayMs`, `providerPubkey`, `disputeWindowMs` ### 10.2 Known Payload Keys @@ -440,7 +446,7 @@ Inner payload keys per scheme: - **exact, stream, escrow**: `transaction`, `signature` - **upto**: `transaction`, `signature`, `maxAmount`, `settlementCeiling` -- **unlock**: `transaction`, `signature`, `encryptionId` +- **unlock**: `transaction`, `signature` - **prepaid**: `transaction`, `signature`, `ratePerCall`, `maxCalls` ### 10.3 Known Settlement Response Keys diff --git a/python/src/s402/http.py b/python/src/s402/http.py index 57b87e7..2f64e9f 100644 --- a/python/src/s402/http.py +++ b/python/src/s402/http.py @@ -64,7 +64,7 @@ def is_valid_amount(s: str) -> bool: "settlementOverrides": ["actualAmount"], "stream": ["ratePerSecond", "budgetCap", "minDeposit", "streamSetupUrl"], "escrow": ["seller", "arbiter", "deadlineMs"], - "unlock": ["encryptionId", "encryptedContentId", "encryptionServiceId"], + "unlock": ["packageId", "keyServers", "threshold", "contentDigest"], "prepaid": ["ratePerCall", "maxCalls", "minDeposit", "withdrawalDelayMs", "providerPubkey", "disputeWindowMs"], } @@ -75,7 +75,7 @@ def is_valid_amount(s: str) -> bool: "upto": ["transaction", "signature", "maxAmount", "settlementCeiling"], "stream": ["transaction", "signature"], "escrow": ["transaction", "signature"], - "unlock": ["transaction", "signature", "encryptionId"], + "unlock": ["transaction", "signature"], "prepaid": ["transaction", "signature", "ratePerCall", "maxCalls"], } @@ -194,11 +194,29 @@ def validate_escrow_shape(value: Any) -> None: def validate_unlock_shape(value: Any) -> None: + """Validate the unlock sub-object (pay-to-decrypt, single-transaction). + + The Seal identity is `receiptId || nonce`, where `receiptId` is the object ID + of the `UnlockReceipt` minted by `pay_and_mint`. That receipt does not exist + when the 402 is written, so no identity field belongs here — it travels in the + fulfillment (see `s402UnlockFulfillment` in the TypeScript types). + """ if not isinstance(value, dict): raise S402Error("INVALID_PAYLOAD", f"unlock must be a plain object, got {type(value).__name__}") - _assert_string(value, "encryptionId", "unlock") - _assert_string(value, "encryptedContentId", "unlock") - _assert_string(value, "encryptionServiceId", "unlock") + _assert_string(value, "packageId", "unlock") + _assert_optional_string(value, "contentDigest", "unlock") + threshold = value.get("threshold") + if not isinstance(threshold, int) or isinstance(threshold, bool) or threshold < 1: + raise S402Error("INVALID_PAYLOAD", f"unlock.threshold must be a positive integer, got {type(threshold).__name__}") + key_servers = value.get("keyServers") + if not isinstance(key_servers, list) or len(key_servers) == 0: + raise S402Error("INVALID_PAYLOAD", f"unlock.keyServers must be a non-empty array, got {type(key_servers).__name__}") + for ks in key_servers: + if not isinstance(ks, dict): + raise S402Error("INVALID_PAYLOAD", f"unlock.keyServers[] must be a plain object, got {type(ks).__name__}") + _assert_string(ks, "objectId", "unlock.keyServers[]") + if not isinstance(ks.get("weight"), int) or isinstance(ks.get("weight"), bool): + raise S402Error("INVALID_PAYLOAD", f"unlock.keyServers[].weight must be a number, got {type(ks.get('weight')).__name__}") def validate_upto_shape(value: Any) -> None: @@ -411,8 +429,6 @@ def _validate_payload_shape(obj: Any) -> None: max_amt = int(inner["maxAmount"]) if ceiling > max_amt: raise S402Error("INVALID_PAYLOAD", f"upto payload settlementCeiling ({inner['settlementCeiling']}) must be <= maxAmount ({inner['maxAmount']})") - if obj["scheme"] == "unlock" and not isinstance(inner.get("encryptionId"), str): - raise S402Error("INVALID_PAYLOAD", f"unlock payload requires encryptionId (string), got {type(inner.get('encryptionId')).__name__}") if obj["scheme"] == "prepaid": if not isinstance(inner.get("ratePerCall"), str): raise S402Error("INVALID_PAYLOAD", f"prepaid payload requires ratePerCall (string), got {type(inner.get('ratePerCall')).__name__}")