Review fixes, Phase D and E, and the eight leftover findings - #210
Merged
Merged
Conversation
The paper vault prints a QR on every recovery strip, but nothing in the app could read one into the shares box. A strip scanned on the Decrypt tab was only told it was in the wrong box, and a whole printed backup scanned in one batch (container parts and strips together) failed as a single bad paste. The only way in from paper was retyping a ~140-character code per strip. - The recovery-shares box gains "Scan share QR images". - A scan now routes each decoded string by prefix: shares to the shares box (turning the shares path on), everything else to the container box. A share-only scan no longer switches File mode to Text, which would have dropped the selected .keym it was meant to open. - A strip scanned twice is entered once. combineShares refuses a repeated index (4.6), so a duplicate would fail an otherwise sufficient set with nothing to say which photo repeated. The QR decoder retries at several scales before reporting a miss. jsqr could not read the paper vault's own container symbol straight off its 300px canvas (it decodes at 0.75x, 1.25x, 1.5x and 2x), and a phone photo capped to 2000px can leave a version-40 part near 2.5 px per module. Choosing "Use recovery shares" after "Use a passkey" left usePasskey set behind a now-hidden control, and processData still took the passkey path: an heir with a sufficient share set was asked for a tap and told the container has no passkey. Turning shares on now turns the passkey off. Controls: removing the routing, the retry scales, the dedupe, or the passkey reset each fails its own test in shamir-ui.spec.ts.
Four defects, each of which could leave an heir without a way in. - The Recovery kit dialog offered keym.py alone, and keym.py reads KEYM v1 only. The app has written v2, then v3, for some time, so a kit saved exactly as offered could open none of its backups. It now lists keym2.py first, requirements.txt, RECOVERY.md, and keym.py labelled as v1-only. (The in-app docs and the inheritance plan already named keym2.py; only the dialog was wrong.) - The one-time shares dialog closed on Escape or a backdrop click, and closing it destroys the only copy of the shares. It now closes only from its X or an explicit "I have saved these shares" button, and it scrolls: with five or more shares it was taller than the viewport, and being fixed and centred, its lower half could not be reached. - The idle lock spared issued shares but wiped outputText, which on the encrypt side is the sealed container (ciphertext) and in Text mode the only copy of it. The dialog went on showing strips that opened nothing, and its Print paper vault button went dark. The container is now spared with the shares. - The encrypt-side inspector plan read the decrypt-side passkey toggle, so it never itemised the passkey slot the worker was about to write. Tests updated to close the shares dialog with its button rather than Escape. Controls: reverting each change fails its own test (kit links, Escape, backdrop, lock, inspector).
The previous commit stopped Escape from closing the one-time shares dialog. The palette and icon audits closed it with Escape, so both then timed out waiting for the Decrypt tab behind a dialog that was still open.
Audio: a carrier decoded through Web Audio came back with the low bit flipped on every sample above 16384. Browsers decode a 16-bit sample s as s / 32768, and the inverse scaled positives by 32767, so a stego WAV revealed through that path lost its payload and was reported as a wrong password. One scale, 32768, then clamp. The carrier path also chose the exact WAV parser by name or MIME type alone, so a stego WAV that lost its extension in a messenger went through Web Audio and was resampled; WAVs are now also recognised by their RIFF/WAVE bytes. New test:audio-decode-scale round-trips all 65,536 sample values; the old scale fails 16,383 of them. Service worker: install fetched the un-hashed shell with addAll's default cache mode, so a deploy inside GitHub Pages' max-age window could freeze the previous index.html (and crypto-worker.js, the recovery kit) into the new version's cache: a false tamper report beside the new SHA256SUMS, and a blank page offline. The shell is now fetched with cache: 'reload'. requirements.txt joins the precached kit, since the kit dialog offers it. New test:sw-precache runs public/sw.js in a stub worker scope. keym2.py: a text backup with a blank first line or a leading space was "unknown" to detect(), fell through to the binary parser, and printed "decryption failed". Section 7 says every reader strips ASCII whitespace; the text prefixes are now sniffed after it. A leading BOM is still refused, as the spec does not cover it (the app accepts one; that divergence is recorded as an open decision, not changed here). keym.py: an 8-14 byte v1 file escaped as a struct.error or IndexError traceback. It is now a KeymError, and the self-test truncates a real header at every length. RECOVERY.md told heirs to "use the recovery shares" and never said how. It now has a section with the keym2.py --shares-from command, and recovery_test.py runs it against a share set issued by the shipping enrolment: two of three strips (one lower-cased, hyphens typed as spaces) open the backup with nothing on stdin, and one strip does not. The "slots" paragraph no longer claims app containers have exactly one. Docs: README named a --outfile flag that does not exist (it is --out). SECURITY.md said length leaks "to within a 1 MiB chunk" (FORMAT-V2 section 8 says exactly) and called v2 the format the app writes (it writes v3). The in-app docs said 99 d6 rolls clear 256 bits (255.9; the tool itself requires 100). Controls: each new check was run against the reverted code and fails.
handleModeChange skipped the reset on the way into Tools, with a comment saying a peek at Tools should not wipe an in-progress form, but coming back out was an ordinary mode change and reset everything anyway: Encrypt, Tools, Encrypt lost the typed secret and the password. A return to the form Tools was opened from now keeps it; going to a different form still resets. Test in uat-polish.spec.ts; restoring the unconditional reset fails it. CHANGELOG gains an Unreleased section covering this branch.
A container larger than one symbol is split into full version-40 parts, 181 modules wide with their margin. The sheet drew each into a 300px QRCodeCanvas, and a canvas is a bitmap: at a devicePixelRatio of 1 each module got 1.66 pixels, and printing could only stretch that aliased bitmap to 46mm. A captured part from a 12-part sheet did not decode at any scale from 0.75x to 3x, nearest or cubic. The component's comment assumed 300px gave a 600dpi printer real modules, which holds for vector output only. The sheet now uses QRCodeSVG for parts and strips, so the printer draws each module at its own resolution. Print CSS follows the element change. The existing scan-back tests pasted the parts' text, never their pixels, which is why this went unnoticed. A new paper-vault test prints a multi-part sheet, rasterises each symbol the way paper would receive it (a canvas as the bitmap it is, an SVG at 800px over 46mm, about 440dpi) and feeds them all to the app's own scanner; the container must reassemble and decrypt. Restoring QRCodeCanvas fails it. capturePrintedSymbols in helpers.ts is shared with the strip-scanning tests.
…uilds
apply-csp-hashes.mjs, apply-build-id.mjs, build-manifest.mjs and
generate-wordlist.mjs resolved their output paths with
new URL('../out', import.meta.url).pathname. A URL pathname is
percent-encoded, so under a checkout path containing a space or a
non-ASCII character the scripts looked for `key%20maker%20%C3%BCn%C3%AF/out`
and `npm run build` failed with ENOENT right after `next build`.
fileURLToPath decodes it. These were the only `.pathname` uses on an
import.meta.url in scripts/.
Regression gate: the second reproducible-elsewhere leg in ci.yml now
checks out into "a checkout directory named nothing like the other one
ünï". It still varies the path, as before, and now also builds from a
path with a space and non-ASCII letters, and its manifest is compared
with the other legs', so the path must not leak into the bytes either.
No extra runner.
Shown locally, copying the repo into a directory named "key maker ünï":
- before this change `npm run build` exits 1 (ENOENT on the %20 path);
- after it exits 0, and its SHA256SUMS is byte-identical to a build of
the same commit from a plain path, with and without
KEYMAKER_BASE_PATH=/Keymaker-v2.
deploy.yml and release.yml sign the SHA256SUMS their own `build` job produced. No suite ever sees those bytes: the suites build their own copy of the commit. deploy.yml's comment asserted the two were byte-identical, but nothing compared them, so a nondeterminism that only appeared on the publishing runner would have been signed and published with every suite green. The sign job in both publishers now downloads the SHA256SUMS each ci.yml reproducible-elsewhere leg already uploads (sums-*, into reproduced/, outside out/) and runs scripts/check-reproduced-manifest.mjs before signing. It fails unless at least two leg manifests are present and every one is byte-identical to out/SHA256SUMS, naming the files that differ, are extra, or are missing. The called workflow runs inside the publisher's run, so the download needs no permission beyond what sign already holds; the token split and --ignore-scripts are unchanged, and the new download step reuses the repo's pinned download-artifact SHA. A release is labelled at build time (KEYMAKER_RELEASE_TAG), so it is not byte-identical to a development build. ci.yml gains a release-tag workflow_call input that the legs pass through env; release.yml sets it to github.ref_name, the same expression its build uses, and deploy.yml and PRs leave it empty, which builds the same bytes as leaving the variable unset (checked locally). Tests: - scripts/check-reproduced-manifest-test.mjs (npm run test:reproduced-manifest, now a ci.yml step) drives the checker on identical, differing, extra-file, missing-file, reordered, absent and too-few manifests. Controls: forcing the comparison to agree fails 7 checks; MIN_REPRODUCTIONS = 0 fails 3. - scripts/release-gate-test.mjs now also asserts the wiring: legs upload as sums-<leg> and honour the input, each sign job downloads and compares before sign-manifest.mjs and runs after verify-ci, and each publisher passes the channel it publishes. Seven controls (delete the compare step, delete the download, move the compare after signing, drop or add the release-tag, drop the leg env, rename the upload) each fail the gate.
docs/RECOVERY.md promises the recovery scripts run on "Python 3.10 or newer", but every job ran 3.12, so a 3.11-only construct in keym.py or keym2.py would have shipped green. The pinned cffi and argon2-cffi-bindings wheels also declare Requires-Python >=3.10, so a routine bump could drop the floor unnoticed. New conformance.yml job, reference-python-floor: setup-python 3.10 (the repo's existing pinned SHA), an assertion that python3 really is 3.10, the hash-pinned install, then `keym.py selftest` and `keym2.py selftest`. Finding: the pinned file does not install on 3.10 as written. `pip install --require-hashes -r reference/conformance-requirements.txt` fails there, because cryptography 50.0.1 declares typing-extensions>=4.13.2 for python_full_version < '3.11' and the closure is resolved for 3.12 only (PY_VERSION in scripts/pin-conformance-deps.py). Every pinned wheel does support 3.10. The job therefore installs with --no-deps: still hash-checked, exactly the pinned bytes, nothing unpinned. It cannot mask a missing module, since the self-tests would fail on import; cryptography uses typing-extensions only in hazmat.asn1, which neither script imports. Pins unchanged. Shown locally in a fresh 3.10 venv: the install and both self-tests pass (keym2: 550 checks). Control: with `import tomllib` (3.11+) added to copies of both scripts, both self-tests pass on 3.12 with the same pins and fail on 3.10 with ModuleNotFoundError. The interpreter assertion exits 1 under 3.11.
- conformance.yml said "KEYM v2 has no TypeScript yet, so there is nothing to cross-test against", two steps above the v2/v3 byte-for-byte cross-test, and counted four spec findings where §11 of FORMAT-V2-DESIGN.md now records eight. It now describes the self-test as what it is (the spec-first reference on its own) without a count that will drift again, and the step name covers v3. - conformance.yml's header named one reference written from FORMAT.md; there are two, keym.py (v1) and keym2.py (v2 and the v3 delta). - crypto-regression.yml said it runs "on every push and PR" directly above the note that it no longer runs on push, and "Node 22 across every workflow" although ci.yml deliberately builds one leg on Node 24. - ci.yml's header summarised it as "Build + typecheck + the KEYM v1 regression suite". - A stray blank line inside the conformance2 step is removed. Comments only, apart from the conformance step's display name.
Five review findings on the unlock and enrolment path, each verified
against the code before being fixed.
1. keym-v2.ts: buildKdfInput left the NFC password bytes and their
length-prefixed copy unerased, keyfileDigest's join left a complete
key-file copy, the key-file digest was never erased, and lp() made an
unerased length-prefixed copy of the share secret and the PRF output.
Once per slot attempted, every unlock and every enrolment. lp() is now
lpConcat(), which writes LP(x0) || LP(x1) || ... into one buffer (byte
identical, conformance2 unchanged), so the caller's erase reaches every
copy; buildKdfInput erases its intermediates in a finally, as v1's
buildBaseMaterial does; keyfileDigest erases its join in a finally.
2. keymaker-crypto.ts: decryptData returned result.data.buffer.slice(),
which always copies, and never erased the original: a second complete
plaintext per decrypt, 100 MB at the cap. The v1 ChaCha path had the
same slice. takePlaintextBuffer() hands over the buffer itself when the
view spans it, and otherwise copies and zeroes the source.
3. crypto-worker.ts: the erase of keyFileForSlots was a trailing call, so
an enrolment that threw (the catch turns it into a response) left the
worker's key-file copy in its heap. Now in a finally, matching the
crypto-client.ts fallback.
4. keym-v2-shamir.ts: combineShares decoded with a Promise.all that ran
before its try/finally, so one malformed share left every decoded share
value unerased, including ones still decoding when it rejected.
Decoding moved inside the try with allSettled; the first rejection is
rethrown unchanged.
6. keym-v2.ts: the Shamir module import in slotSecretFor was a bare
import() outside any typing, so on the main-thread fallback a chunk that
failed to load reached the user as "the password or key file may be
incorrect". It now goes through loadShamir(), which raises the same typed
dependency-unavailable error as loadHashWasm/loadNoble and does not cache
the rejection. dependencyUnavailable is exported for it.
Test: scripts/secret-erase-core-test.mjs (npm run test:secret-erase-core,
wired into CI). The copies are internal, so it takes a census: the global
Uint8Array constructor is wrapped in a Proxy, and Uint8Array.prototype.slice
and TextEncoder.prototype.encode are wrapped, recording every buffer the core
allocates during one call; afterwards each recorded buffer is searched for the
secret. The Shamir chunk failure is modelled with a real dynamic import of a
file that does not exist yet, then is written, and the same call retried.
Negative controls (each patched source typechecks; esbuild bundles it):
1a keyfileDigest join not erased:
FAIL encrypt/unlock: no copy of the key file survives [83]
FAIL worker: key-file copy erased when enrolment throws [83, 83]
FAIL worker: key-file copy erased on success too [83, 83]
1b buildKdfInput finally removed:
FAIL encrypt/unlock: no copy of the password bytes survives [49]
FAIL encrypt/unlock: no copy of the key-file digest survives [32]
1c per-field LP copies restored:
FAIL encrypt/unlock: password bytes [53], key-file digest [36]
FAIL unlock: no copy of the reconstructed share secret survives [36]
FAIL unlock: no copy of the PRF output survives [36]
2 decryptData back to buffer.slice at both sites:
FAIL v3-pbkdf2-aes256gcm: returned buffer is the only plaintext copy [44]
FAIL pbkdf2-chacha20poly1305: returned buffer is the only copy [47]
3 worker erase back after the try (success only):
FAIL worker: the key-file copy is erased when the enrolment throws [64]
4 combineShares back to Promise.all before the try:
FAIL the shares that did decode are erased when another is malformed
[32, 32]
6a bare import() restored:
FAIL a Shamir chunk that fails to load is a typed dependency-unavailable
error (got ERR_MODULE_NOT_FOUND)
6b loader caches its rejection:
FAIL once the chunk is reachable the same call opens the container
Not covered, and said so in the test: copies inside Web Crypto (imported
keys, digest inputs) have no JS handle, and JS strings are immutable. The
decoded share record that b32Decode returns also carries the share value and
is not erased; that belongs to decodeShare/b32Decode and is left to the
change already in progress there.
MAX_CONTAINER_SIZE was MAX_PLAINTEXT_SIZE + 4096, a v1-era allowance (71-byte
header, 32 bytes of tags). A v3 chained container with the 8 slots section 6
allows, holding a plaintext at the 100 MiB cap, is
57 (slot table offset) + 8 x 112 (chained slot) + 100 x 32 (chunk tags)
= 4153 bytes over the plaintext
so a legal backup at the cap was refused by decryptData as too large, with
the "open it with keym2.py" message, by the build that wrote it.
The ceiling is now computed from the format constants: the largest slot table
offset, slot length, slot count and per-chunk tag, with the chunk count taken
as max(1, ceil(n / chunk)), the rule the writer uses. keymaker-crypto.ts
restates the constants rather than importing keym-v2.ts (the dependency runs
one way), so keym2-dispatch.mts holds them to the format module's exports, as
it already does for KEYM2_HEADER_PEEK_BYTES:
- MAX_CONTAINER_SIZE >= the worst case over v2/v3 x all three ciphers,
computed from keym2SlotTableOffset, keym2SlotLen, KEYM2_MAX_SLOTS and
KEYM2_CHUNK_SIZE, with the per-chunk tag measured from real containers;
- the overhead stays under 64 KiB, so it cannot pass by being unbounded;
- the worst case is built for real (v3, chained, a MAX_PLAINTEXT_SIZE
plaintext, 1 passphrase + 7 passkey slots), its length matches the
arithmetic, and decryptData opens it byte-identically. About 3 s and
0.5 GiB RSS locally.
Negative control (typechecks): ceiling back to MAX_PLAINTEXT_SIZE + 4096
FAIL the container ceiling covers the largest container a capped
plaintext can become worst case is 104861753 bytes (v3 + chained,
8 slots, 100 chunks); the ceiling is 104861696.
FAIL decryptData opens the worst-case legal container at the plaintext
cap, byte-identical This backup is larger than the 100 MB this app
can open in a browser tab...
66 passed, 2 failed
Each of these was found in review, confirmed by reading or by a test, and
now has a test that fails without the fix.
- The idle lock fired in the middle of an operation: an unlock can take
minutes (the §6 ceiling is 315 s), and the lock cancelled it and wiped the
password. An operation in progress now counts as activity. (Test holds the
operation open with a passkey prompt that never answers; an earlier
derivation-length version raced the CPU.)
- A QR scan that finished after a tab switch or Wipe now wrote its result
into whatever form was showing. It now checks the op counter.
- The Decrypt password field was painted red by the encrypt policy, and the
Encrypt border ignored the generated-passphrase exemption the button uses.
- "Nothing was pasted" was appended to every container-box notice, including
the three that keep the paste (a share, a part, a damaged page).
- The scan toast said "type the password" after a scan that was not a
complete backup; the text handler now reports whether it accepted input.
- Names containing ".." ("Notes... draft.txt") were refused as invalid; a
browser File.name never carries a path.
- Stop pressed while a large file was still being read did not stop: the
disowned operation went on to start a worker and a full derivation.
- A text-form backup chosen as a file on Decrypt (a .txt of armor, a saved
self-extracting page, paper parts, a shares file) was read as a legacy
blob, ran 1M PBKDF2 iterations and blamed the password.
- Accessibility: the reveal, copy and QR buttons under the output and every
toast's close button had no accessible name; the lock warning and the
clipboard countdown were live regions re-announced every second. New axe
scans cover the encrypt and decrypt result states, which were never
scanned.
- The dice tool printed "Infinity" rolls for an invalid die.
- The inheritance plan opened on File mode, where its own paper-vault step
cannot be followed; step 4 now also says what to do with a sealed file.
- A rehearsal result survived into the next seal, so a new backup's dialog
and printed sheet claimed a rehearsal that never happened.
- The Recovery page said "Confirm in your downloads" for a container kept on
screen, and "No result shown" after a successful rehearsal.
- Service worker: offline lookups searched every cache on the (shared Pages)
origin; they now use this worker's own cache. /verify.html is precached, so
offline it no longer serves the home page under its URL.
…ed keys Spec first (FORMAT-V2-DESIGN), then keym2.py, then the TypeScript, then the parity gate, per CLAUDE.md. No bytes a writer emits change. §7 "Characters a reader ignores". The two readers inherited their language's idea of whitespace and disagreed in both directions: trim() removes U+FEFF and misses U+0085, str.strip() does the opposite and also removes U+001C..U+001F, and both upper() functions fold U+0131 and U+017F into ASCII. A Notepad-saved backup (leading BOM) opened in the app and failed in keym2.py. The set is now named (White_Space plus U+FEFF), applied at the ends and inside armor, share and part bodies, with ASCII-only case folding. keym2.py also refused a shares file saved with a BOM. §6 full set id. Both readers compared a KMSHARE2 share's 16-byte set id to the container over only its first four bytes. Each share is now compared over its own full length against the 16-byte derived id. §7.3 part checksum compared as text. keym2.py decoded it first and accepted sixteen spellings of each checksum where the TypeScript accepted one; it also accepted part indices in non-ASCII digits. §4.4 a master key is recovered only when it opens the payload. keym2.py returned the first slot that unwrapped, so behind a slot spliced into a v2 container from another container with the same password it refused a container that opens, and add-shares/add-passkey/rewrap wrapped new slots around the other container's key. The TypeScript decrypt already kept looking, but its enrolment path took the first unwrap too: every share it printed for such a container opened nothing. Gates: keym2.py selftest 553 -> 571 checks; crosstest2 265 -> 295, with a new bridge command (textverdicts) comparing verdicts and decoded bytes on the same inputs, a near-set share test and a spliced-slot enrolment test. dearmor-whitespace-test.mjs asserts the new rule (it asserted the old ASCII-only body rule, which only moved the disagreement). Controls: each Python and TypeScript change reverted fails its own named check, except the share-prefix dispatch in decodeShareAny, which the decoder behind it makes unobservable, and the paper-part strip, which only bites when both strip sites are reverted (the dispatcher and decoder each strip).
Spaced checkout paths build (fileURLToPath), deploy and release refuse to sign a manifest the independent builds did not reproduce, the reference self-tests run on Python 3.10 (the RECOVERY.md floor), and stale workflow comments are corrected.
Key material copies erased in buildKdfInput, keyfileDigest and the length-prefix helper; decryptData no longer keeps a second plaintext copy; the worker erases its key-file copy on every exit; combineShares erases shares decoded before a sibling failed; MAX_CONTAINER_SIZE is derived from the format constants and admits the legal worst case (v3, chained, eight slots, 100 MiB); a failed load of the Shamir module is a typed dependency error rather than a wrong password. Conflict in slotSecretFor resolved to keep the typed loader and the finally-erase from this branch and the full-width set id (shareSetIdV2) from the parity fix.
recovery_test.py ran WALKTHROUGH.md's bash blocks but only described RECOVERY.md's in hand-written argument lists, which is the copy nobody follows. It now extracts every command on the page and runs it against containers the shipping encryptor wrote: - Step 2's inspect lines and Step 4's decrypt lines, for v1, v2 and v3, with and without the key file the page says to add. Each line's trailing "# v3 or v2" comment is a checked claim: exactly one line per version, that line opens the file, the other refuses it. - The shares command, with two of three strips, one copied by hand. - A command the runner does not recognise fails the suite. - Step 3's two sample outputs are compared line by line with what inspect prints for a matching container. Step 2 and Step 4's CLI loops also cover v3 now. Drift the new checks found or that was carried over: - the v3 inspect sample lacked the container and table mac lines; - the primitives table described only v2's 8-byte core header; - RECOVERY.md and README said the fixtures open "under whatever version is installed", but CI installs exact pins only. Negative controls: the old v3 sample, a "# v2" comment, a renamed --shares-from, an unknown command, a wrong --key-file flag in the prose, swapped inspect comments and a changed v1 sample value each fail it.
One press fills both "Needed to open" and "Shares to print". The fields stay the source of truth: a preset only sets them, and shows as pressed only while both still match it, so an edited set is never labelled as a preset. The browser test checks both fields, the pressed state before and after an edit, and that the issued set really is 3 of 5. Negative controls: a click that sets nothing, and a pressed state that ignores the count, each fail it.
FORMAT-V2-DESIGN §4.6 gains "The set code": the first eight base32 characters of the v2 set id, written XXXX-XXXX. A KMSHARE2 strip's text is base32 of a record that starts with the set id, so every strip of a set already begins with those two groups; the section gives them a name and a vector, and says what they are not (a label for people, not a check, and not secret). A KMSHARE1 strip matches in six characters. - keym2.py: share_set_code, share_text_set_code, and a "set code" line in inspect for a share slot. Self-test covers the vector, every strip starting with the code, and the O/0 and I/L folding. - TypeScript: shareSetCode, shareTextSetCode, shamirSlotSaltsKeym2. - Parity: a new setcodes bridge command; crosstest2 compares the emitted strings for the vector, random salts and copied strip texts, including all-0 and all-1 codes so the folding is exercised. - Paper vault: the owner's sheet prints the code from the container's slot salt, so a sheet printed later without strips still has it; each strip prints the code from its own text. The footer names the version the container actually is. - RECOVERY.md says how to sort strips by set code; recovery_test checks that inspect's set code is what every strip begins with. Negative controls: a code from the wrong bytes, no O-to-0 folding, no I/L folding and KMSHARE1 accepted fail crosstest2; removing the Python folding fails the self-test; a sheet with no codes and a strip printing another code fail the browser tests; inspect without the line fails recovery_test.
A "Check a printout" control on the Recovery page. Each photo of a printed strip or container symbol is decoded on its own and reported in one line: whether the code read back intact, and whether it belongs to the backup this session wrote. No share is combined, no key derived and no password asked for. - A strip is matched on its full set id against the container's share slots (all 16 bytes for KMSHARE2, per §6). That needs only the header and slot table, so it works for a backup written straight to a file; shamirSlotSaltsKeym2 now reads only that far. - A KMPART2 symbol is matched on its fingerprint and length, which needs the whole container, so it is answered only when the backup is on screen. A KMPART1 part is said to carry no fingerprint. - A damaged strip or symbol, a stranger's QR and a photo with no code are each named. - A new seal or a wipe clears the results. src/lib/printout-check.ts holds the logic; test:printout-check drives it with two real containers (wired into CI), and a browser test checks real printed symbols before and after a second seal. Negative controls: a four-byte set-id compare, a skipped part checksum and a header treated as a whole container fail the unit test; results that survive a new seal, and a check with nothing to compare against, fail the browser test.
FORMAT-V2-DESIGN §7.3 gains "Symbol size" (writer guidance; readers accept any size). A full version-40 symbol printed 46 mm wide is 0.254 mm a module, which a phone must hold focus close to the page to resolve. A writer now sizes each part for version 25 at level M, 997 bytes: 0.38 mm a module at the same width, 702 container bytes a KMPART2 part instead of 1,704. - TypeScript: PAPER_QR_VERSION and PAPER_QR_MAX_BYTES (997), and encodePaperPartsForPrint, the one function the app and the bridge call. - keym2.py: paper_capacity_v2 and the same budget. `split` now writes KMPART2 by default, as §7.3 already required of a writer, sized to fit; --v1 writes KMPART1 and the old --v2 flag is still accepted. - Fixes `split --v2`, whose 1,734-byte default (the KMPART1 figure for version 40) made 2,359-character lines, over the 2,331 a version-40 level-M symbol holds. Checked against the app's QR library: 2,332 bytes at level M is "Data too long". - Parity: crosstest2 compares keym2.py split's default output with the paper vault's parts, string for string. - A browser test reads every printed symbol's module count from its SVG. - WALKTHROUGH.md said printed parts start KMPART1; the app prints KMPART2. Negative controls: the TypeScript budget back at 2,331, or the Python default back at 1,734, fail crosstest2; the former also fails the browser test.
A photo of a whole printed sheet now yields every code on it, on the Decrypt tab's scan and in the printout check. - BarcodeDetector where the browser has one, used alone when it finds anything. - Otherwise jsqr, which returns one code a call: each code read is painted out and the picture searched again, then overlapping square tiles are searched, because with several symbols in view jsqr can pair finder patterns from different ones and read none. The first test of a whole-page photo read the container symbol and missed both strips until the tiles were added. - Cost is kept where it was for a one-code image: one whole-frame read per retry size, as before, and a code filling a quarter of the picture or more ends the search. Measured on the sheet's own symbols at 800px: 513 ms an image before, 515 after; an earlier draft that tiled first took 905. - decodeAllQrInFrame reads one camera frame the same way, for the camera scanner that follows. - The printout check reports one line per code, named by photo and position. Negative controls: stopping at the first code read fails the one-photo test on the Decrypt tab, and the printout check reading one code per photo fails its whole-sheet test.
A camera dialog on the Decrypt tab, opened from "Use the camera" beside the container scan and "Scan strips with the camera" in the shares box. It reads every code in each frame (decodeAllQrInFrame: BarcodeDetector where present, jsqr otherwise), a few times a second, and says what is in and what is still needed until there is enough, then routes the codes exactly as scanned photos are routed. When enough has been read is src/lib/camera-progress.ts, judged from the codes alone: a strip's own threshold, a symbol's own total, each kind that has been started complete, and a whole backup in one code complete by itself. The same strip held twice counts once; a strip from a different set is named and not counted. The camera stops as soon as the dialog closes, however it closes. Tests: - test:camera-progress (in CI) with real strips, parts and armor. - camera-scan.spec.ts launches Chromium with a fake camera fed a Y4M file built from the real print sheet's strips (strip 1, blank, strip 3), checks the "next strip" prompt, that the dialog stops by itself, that the shares box holds exactly those strips, and that they open the backup with no password. It also shows the page's CSP does not block a camera stream. Chromium only; the flags are Chromium's. Negative controls: counting a repeated strip twice, counting a strip from another set, and taking strips alone as complete when symbols have been started fail the unit test; dropping the codes instead of routing them fails the browser test.
…ated When a backup fits one printed symbol (§7.3 "Symbol size"), the shares dialog offers "Put the whole backup on every strip". Each strip then prints the backup's one KMPART2 part beside its share, and any k strips open the backup with no sheet and no file from the owner. The dialog states the cost beside the switch: k holders who get together need nothing else. Off by default, and reset for every share set, since it changes who can open the backup without the owner. The sheet's cover, the strips page and each strip's note say which kind of strip it is. Also: - The dialog's warning said k holders "need nothing else from you"; they also need the backup. It says so. - Scanned container codes are de-duplicated. k strips that each carry the backup put the same symbol in k photos, and the reassembler refused the set as "Part 1 was supplied twice". The new test hit this before the fix. - RECOVERY.md explains a strip's second code and gives the join command, which recovery_test.py runs on the part the app prints and then opens the result with the strips. Browser tests: from a fresh page, the two codes of each of two strips alone open the backup; the switch is off by default and absent for a backup too big for one symbol. Negative control: printing without the part fails the first.
A new slot type, 0x03: the passphrase AND k of n shares, both needed.
Additive, as §9 now says a frozen format admits: the record's shape is
unchanged, and §4.4 has every older reader skip the type.
Spec (FORMAT-V2-DESIGN §4.8): passphrase_key is the slot's own KDF over
§4.1's input; slot_key = HKDF(LP("keymaker.v2.passphrase-and-shares") ||
LP(passphrase_key) || LP(share_secret), slot_salt, "keymaker.v2.slot-key").
The memory-hard cost is paid once, on the guessable half. Shares are
§4.6's with this slot's set id; a reader compares it before the KDF. A
reader MAY say strips need the password (the set id is public). A 0x03
slot may stand alone; the writer states the costs. A vector computed with
plain hashlib/hmac.
keym2.py, from the spec: the derivation, the walk, build_both_slot,
encrypt_both, add_both_slot, shares_need_password, inspect, and the CLI
asking for the password once the strips show they need it. Self-test:
the vector, domain separation from 0x00, each half refused, wrong-set
strips declined before stretching, a pre-§4.8 reader's view.
TypeScript: the same, one shared single-slot writer for the passphrase
and 0x03 slots, the worker and its fallback, the inspectors.
Parity: crosstest2 compares the container bytes and share strings of
both writers across v2/v3, three ciphers and two KDFs, cross-opens them,
and has each refuse either half alone. Three frozen fixtures
(v3-both-*); counts updated in crosstest2.py and keymaker-regression.mts.
UI: "The strips need the password too" under Recovery shares, with its
cost stated, and the passkey switch disabled. The shares dialog, the
rehearsal (which now takes the password), the paper vault and the
receipt say what the strips do. The Decrypt tab says the password is
needed too before any work. RECOVERY.md explains it; recovery_test runs
the page's shares command on such a backup.
Negative controls: a one-character domain string in the TypeScript
(25 parity failures); skipping the set-id check before stretching and
dropping the share secret from the derivation (self-test); the switch
not honoured (browser); the CLI never asking for the password
(recovery_test).
…deps reference/conformance-requirements.txt was resolved for 3.12 only. On 3.10 cryptography also requires typing-extensions (its marker is python_full_version < "3.11"), which the file did not pin, so the reference-python-floor job could only install it with --no-deps. scripts/pin-conformance-deps.py now resolves for both 3.12 and 3.10 and merges the closures, refusing if a package resolves to different versions on the two. Two defects in its resolution, found by running it: - pip's --python-version does not set python_full_version for marker evaluation, so typing-extensions never appeared. Each wheel's own Requires-Dist is now evaluated against the target interpreter and anything missing is downloaded in another round. - It resolved for manylinux_2_17 only. argon2-cffi-bindings 26.1.0, the version CI installs, publishes manylinux_2_26/2_28 wheels only, so a regenerate silently fell back to 21.2.0. It now accepts every manylinux tag down to 2014. Regenerated: the same versions as before, plus typing-extensions 4.16.0. The 3.10 job drops --no-deps. --check still passes, and a hash-checked dry-run install succeeds on 3.11; 3.10 itself is only available in CI.
… reason The page's decryptor (DECRYPTOR_JS): - A failed attempt hid #result but left the plaintext in the hidden #out. - A binary result left the previous text in #out. - The save link's blob URL was never revoked. Each submit now runs forget() first, which clears #out, revokes the URL and resets the link. The password input uses autocomplete="off" instead of current-password, so the heir's machine is not asked to keep the backup's password. SELF_EXTRACT_SCRIPT_SHA256 is recomputed. keym2.py emits only the sentinel block, so parity is unaffected, and the frozen fixture page keeps its own copy of the old decryptor. webcryptoProfileViolations: - Every non-AES cipher was called ChaCha20-Poly1305, chained mode included. Chained is now named as chained. - A container with no passphrase slot at all (shares only, passkey only) was told "the password slot uses Argon2id". It is now told it has no password slot, and what it opens with. Tests: - self-extract.spec.ts "nothing an earlier attempt recovered outlives the next one": builds a text and a binary page through the bridge, then checks #out after a failed attempt and after a binary result, the revoked URL, and the autocomplete value. Four controls, each with the hash recomputed so the patched page's script still runs, each fails on its own assertion: autocomplete back, forget() keeping #out, no revoke, and #out cleared only on failure (the binary case). - keymaker-regression.mts section 9: chained, ChaCha, Argon2id, passkey-only and share-only reasons. With the old function, the chained, passkey-only and share-only checks fail.
parseWavToPcm16 refused everything but 16-bit PCM ("Only 16-bit PCM WAV is
supported") while docs/FORMAT-AUDIO-STEGO.md promised Hide any WAV. Most
audio tools export 24-bit or float, so those files could not be carriers.
Web Audio is not the fallback: it resamples to the device rate, and the low
bit the payload lives in does not survive that. The exact parser now also
reads 8, 24 and 32-bit integer PCM and 32/64-bit float, plain or
WAVE_FORMAT_EXTENSIBLE (SubFormat GUID checked in full), at the file's own
sample rate, and scales each to 16 bits by a power of two. 16-bit PCM is
read sample for sample as before, so Reveal is unchanged for every file it
could open, and a stego WAV re-saved losslessly at 24-bit or float still
reveals. ADPCM, A-law, mu-law and other depths are refused with a message.
The format document now says exactly this.
Tests:
- New scripts/audio-wav-depths-test.mjs (npm run test:audio-wav-depths, CI
step added): every depth plain and extensible, rate kept, exact samples,
8-bit scaling, stereo order, float clamping, a payload surviving 24-bit and
float re-saves, and three EXTENSIBLE refusals. Controls: the old parser
fails 13 checks; dropping the GUID check reads a foreign header as PCM.
- audio-malformed-test.mjs: the float and 24-bit refusals became ADPCM,
12-bit PCM and 16-bit float refusals. Control: removing the encoding check
fails all three.
The sealed panel said "Forbidden to talk to any server" and "for every request to anywhere". default-src, connect-src and form-action set to 'none' block the scripted connection APIs, form posts and loads from other servers. They do not govern moving the tab to another address, WebRTC, requests for the site's own files, or the page's workers (docs/HOW-IT-WORKS.md, "What the CSP does not do"). The wording now lives in seal-verdict.ts as SEALED_CLAIM, names those limits, and sealed-status.tsx renders it. The unsealed text no longer says the export "forbids every request". Tests: - seal-verdict-test.mjs: the claim must not say "any server", "every request" or "anywhere", must name all four limits, and sealed-status.tsx must render SEALED_CLAIM and not carry the old phrases. Controls: the old component fails 4 checks, both old files fail 9, the old wording in the constant fails 7. - sealed-status.spec.ts: the panel's title and text equal SEALED_CLAIM. Controls, each built: the old component, and the new component with the old wording, both fail.
…isdescribed - The offsets comment and the v3 self-test header said v2 is what this file writes. VERSION is v3; v2 is written on request (--v2, version=VERSION_V2). - encrypt()'s docstring said "v2 by default, v3 on request" and that v3 "stays off by default". Reversed. - Four places said --outfile; the flag is --out (dest stays outfile). - unwrap_master_key_from_slot's comment called its try/except "defence in depth" because "this reader's §6 floor on memory_kib already rejects" mem=1 with p=8. It does not: ARGON2_MEM_MIN is 1, parse_slot accepts the pair, and the KDF raises. The try/except is the live guard. Checked by running it: parse_slot returned the slot and derive_slot_key raised argon2's HashingError. New self-test checks build that container (slot 0 rewritten to memory_kib=1, parallelism=8, slot 1 a valid PBKDF2 slot) and require it to open through slot 1. The call is caught broadly so a regression reports instead of crashing. Control: removing the try/except fails that check.
…cords Module loads: - keymaker-crypto.ts reached keym-v2.ts through a bare import() on decrypt, encrypt and encrypt-with-shares. On the main-thread fallback that is a separate chunk, and an unreachable one surfaced as a wrong password or "Encryption failed". loadKeym2() now types it as dependency-unavailable and does not cache the rejection, like loadHashWasm and loadNoble. - addShamirSlotKeym2 imported keym-v2-shamir directly instead of through loadShamir(). Share erasure: - decodeShare and decodeShareV2 now erase the b32 record, which holds the whole share, whether parseShare accepts it or not. - shareChecksum and shareChecksumV2 erase the concatenated input. - b32Decode erases a record it refuses for padding bits or length. - sharesNeedPasswordKeym2 and checkPrintoutCode decoded shares for the set id, index and threshold and dropped the value unerased. They erase it. Tests (secret-erase-core-test.mjs): - A missing keym-v2 chunk gives a typed error on decrypt, encrypt and encrypt with shares, and the same call succeeds once the chunk exists. A missing Shamir chunk on enrolment is typed, and retrying succeeds. Controls: the old keymaker-crypto.ts fails the three keym-v2 checks; the bare Shamir import fails the enrolment check. - Census checks for decode (KMSHARE1 and KMSHARE2), combine, a refused checksum, refused padding bits, sharesNeedPasswordKeym2 and the printout check. Five controls, one per erasure, each fails its own checks.
processData checks isStale() after the file read and before enrolPasskey. Nothing tested it. worker.spec.ts has the slow-read Stop test without a passkey, where the later check before the worker also catches it. The new test enables Passkey quick access with a virtual authenticator on localhost, slows the file read, presses Stop during it, and asserts that navigator.credentials.create was never called, the authenticator holds no credential, and nothing was downloaded. Control: removing that isStale() check, built, fails with one create call.
…nd share-erasure fixes
After the first batch (strip-2, symbol-1, blank) the list already holds three lines, so toHaveCount(3) after the strips-page photo was satisfied before its results landed. Under load in a full Chromium run the read then saw the old batch: ["2", undefined, undefined] where 1, 2, 3 were expected. It passed 4 of 4 alone. The wait now counts lines naming strips-page.png. Control, from a temporary copy that delays createImageBitmap for that photo by 3 s: the old wait fails with the same symptom every time, the new one passes.
…s not "the lock waits for an operation in progress" failed on WebKit in CI on both tries: Stop never appeared after Decrypt Text, so the unlock ended before the lock was tested at all. It passes on Chromium and Firefox. The CI artifact with the page snapshot could not be read, so what WebKit did is not known. The test held the unlock open by assigning a never-answering function to navigator.credentials.get, inside an `if (navigator.credentials)`. It now replaces get on CredentialsContainer.prototype and on the instance, and defines navigator.credentials if the engine has none. Each call is counted, and each toast is recorded as it appears, because toasts are dismissed long before a 15 s wait ends. - If Stop never appears, the failure names how many times the authenticator was asked and what the toasts said, instead of timing out. - Once Stop is up, the authenticator must have been asked exactly once, so the operation is known to be held by the prompt. The lock assertions are unchanged. Controls, each built: - The lock's isLoadingRef.current check removed: fails on "the idle lock cancelled an unlock the user was waiting on". - A prompt that rejects at once: fails with "the authenticator was asked 1 time(s), and the page said ["Processing ErrorUnlocking with the passkey was cancelled."]".
4 tasks
This was referenced Sep 25, 2026
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.
Summary
The end-to-end review's fixes, the Phase D and Phase E work, and the eight findings left over from the review. 34 commits. The controls for the eight leftover findings were re-run for this PR and are listed under Test plan.
Review fixes
keym2.py.Spec, Python and TypeScript parity
Phase D and E
recovery_test.pyruns RECOVERY.md's commands, and RECOVERY.md's v3 drift is fixed.keym2.py splitwriting KMPART2, every QR code read from one photo, live camera scanning, and self-contained strips.Leftover review findings (61f371c to 50960fa)
keym2.pyis corrected, and the slot-walk guard it misdescribed now has a self-test.WebKit test fix (f6b26d5)
The first CI run failed on WebKit in "the lock waits for an operation in progress". The unlock ended before Stop appeared, so the lock was never tested. The test now replaces the passkey prompt at every level an engine could look it up, reports how often it was asked and what the page said if the unlock ends early, and requires exactly one ask once Stop is up. The lock assertions are unchanged.
Test plan
Run locally on 50960fa unless noted.
npm run typecheckand all 37npm run test:*scripts excepttest:browserpython3 reference/keym.py selftestnpm run buildwith no base path, then the palette, icon and screenshot auditsnpx playwright test --project=chromium --workers=2. 304 passed, 5 skipped, exit 0.Owner actions
reproducible-elsewhereCI job is a required check in branch protection, update it.v2.0.0is a lightweight tag, and the live site has never been opened on a phone).