Repository navigation
fix: take a receipt's encryption claims from the manifest, not the request - #33
Merged
Merged
Conversation
…quest
Before: a receipt's `encrypted` and `suite` were whatever the uploader put in
the request body, signed as-is inside a statement stamped `"verified": true`.
A client could post `"suite": "totally-made-up-suite-v9"` for a document whose
manifest says `aes-256-gcm` and be handed a signed receipt saying so. The
honest direction was just as wrong: `issueReceipt` coerced a missing
`encrypted` to `false`, so a client that said nothing had its sealed document
receipted as plaintext, over a signature.
After: both fields come from the manifest header the gateway already fetches
and validates in `record()`, and `normalizeDocument()` strips them from the
request alongside `verified` — the same argument, one step removed. With no
verifier configured they are left undefined, `canonicalize()` drops them, and
the statement simply does not carry them: the honest answer is "this gateway
did not look", not `false` and `null`, which are assertions it cannot make.
The omission is additive on exactly the terms `verified` already established,
so RECEIPT_VERSION does not move and every receipt already in a user's hands
still verifies. The shipped browser client sends both fields and is unaffected:
it computes them from the same packing the manifest records, so the values it
sends and the values now signed agree.
How: `record()` keeps the `{manifest}` the verifier returns and sets
`encrypted` and `suite` from it; `issueReceipt` passes `undefined` through
instead of coercing. Four tests in test/verify.test.js cover the forged suite,
the plaintext-claimed-as-sealed direction, the omitted-field direction, and the
no-verifier case, where the receipt is round-tripped through JSON to confirm
the keys are absent and the signature still verifies. All four were confirmed
to fail against the unfixed source.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxTVcUSsapm2VLtKfSdskp
# Conflicts: # Readme.md # index.html
504 = 500 on master plus the four receipt-claim tests. Generated by `npm run test-count`, never hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxTVcUSsapm2VLtKfSdskp
Count-only, as expected: the only conflicts were the two generated test-count lines, taken from master and regenerated afterwards. server/proofs.mjs merged without one, and both sides survived — the keyring's import and its openKeyring() call, and this branch's manifest-sourced `encrypted` and `suite`. The only master lines the merge replaced are the four this change exists to replace. 509 = 505 on master plus the four receipt-claim tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxTVcUSsapm2VLtKfSdskp
Count lines only. The two conflicts were the generated totals in Readme.md and index.html, resolved per hunk and regenerated; every master line the merge drops in server/proofs.mjs and js/core/receipt.js is one of the six this change exists to replace.
nishchal-gond
added a commit
that referenced
this pull request
Sep 21, 2026
Count-only conflicts in Readme.md and index.html, resolved per hunk and regenerated. 549 = master's 537 plus this branch's 12. #33 changes what the gateway signs into a receipt, so the browser suite was re-run against it rather than trusted: 14/14.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by LPH · project thread
Before: a receipt's
encryptedandsuitewere whatever the uploader typed. They sit inside a statement the gateway stampsverified: true, so a reader takes them as checked — but nothing checked them. A client could post"suite": "totally-made-up-suite-v9"for a document whose manifest saysaes-256-gcmand get it signed over the gateway's own signature. The honest direction was just as wrong: a client that simply omittedencryptedhad a sealed document receipted as"encrypted": false, which is the more dangerous of the two, because it is the lie a careful client tells by accident.After: both come from the manifest header the gateway already fetches and validates before it signs anything, and
normalizeDocument()strips whatever the client sent alongsideverified, for the same reason. With no verifier configured —OREOCHAIN_VERIFY_MANIFESTS=false— neither field appears in the statement at all. That is the honest answer: this gateway did not look, wherefalseandnullwould be assertions it cannot make.How
issueReceipt()passesundefinedthrough rather than coercing, andcanonicalize()drops undefined keys, so an omitted field is genuinely absent rather than null. That is the same additive ruleverifiedalready relies on, which is why receipts issued before any of these fields existed still verify.RECEIPT_VERSIONis unchanged, deliberately: nothing in an existing receipt changes meaning.What this does and does not claim is worth stating plainly. The receipt is now self-consistent with the manifest its own CID commits to — it says "the manifest at this CID says
aes-256-gcm", not "this file is genuinelyaes-256-gcm". The manifest is still client-authored. That is a real gain, because the CID is in the signed statement and cannot be swapped afterwards, but it is not an authenticity guarantee about the ciphertext and should not be read as one.The shipped browser client sends both fields already, computed from the same packing the manifest records, so the values it sends and the values now signed agree; the endpoint's documentation keeps them in the sample body with a paragraph saying they are accepted and discarded.
Review notes
Reviewed from a second session, and checked by running rather than by reading.
Beyond the reproduction — post
{encrypted: false, suite: "totally-made-up-suite-v9"}for a manifest that saysaes-256-gcm, and get the manifest's values back in a statement that verifies against the published key — I checked the things that would make this change unsafe rather than merely correct:verifier.verify()already returned{manifest}before this change, so nothing new is being relied on.validateManifestHeader()requiresencryptedto be a boolean andsuiteto be a string wheneverencryptedis true, soBoolean(manifest.encrypted)andmanifest.encrypted ? manifest.suite || null : nullare well-typed. Forcingsuitetonullon an unencrypted manifest is right, because that field is unvalidated in that case.undefined.encrypted(js/chunked-app.js:675and:845) read the on-chain record, not the receipt statement, so nothing on the page depends on the field being present.The four new tests were mutation-checked: with
server/proofs.mjsandjs/core/receipt.jsreverted to master, all four fail (not ok 6..9) and the rest of the file still passes.Merging this
Count-only: the two generated totals in
Readme.mdandindex.htmlwere the only conflicts, resolved per hunk and regenerated to 537 — master's 533 plus this change's 4. Every master line the merge drops is one of the six this change exists to replace (await verifier.verify(anchorable), the three-line comment and destructure innormalizeDocument(), and the two coerced fields inissueReceipt()); the merged branch differs from master in exactly the four files the change touches.Tests
537 tests pass on Node 18, 20, 22 and 24, duplicate-declaration scan clean, every module passes
node --check.