test: known-answer coverage for HD derivation and the vault (closes #159) #209

Merged
clawbot merged 1 commits from test/issue-159-crypto-core-tests into next 2026-08-11 15:06:36 +02:00
Collaborator

Closes #159. Tests only — no
source file is modified.

What is covered

tests/wallet.test.js (extended around the six existing DEBUG-flag tests,
which are untouched) and tests/vault.test.js (new).

wallet.js, pinned to published vectors rather than to what the code returns
today:

  • hdWalletFromMnemonic — first address for two independent phrases; the
    account-level xpub is watch-only; the account path is m/44'/60'/0'/0; an
    invalid phrase throws.
  • deriveAddressFromXpub — children 0, 1, 2 against published addresses, and
    agreement with hdWalletFromMnemonic at index 0; garbage throws.
  • hdWalletFromXprv — master xprv for the same phrase reaches the same
    published address, and toEqual against hdWalletFromMnemonic's whole
    result; its xpub generates the same three children; a watch-only xpub and
    garbage are rejected.
  • isValidMnemonic / isValidXprv — valid, bad checksum, 11 words, off-wordlist
    word, empty, garbage, bare private key, truncated and over-long keys.
  • addressFromPrivateKey — the three published key/address pairs; wrong length
    and empty string throw.
  • getSignerForAddress — all three wallet types at indices 0, 1, 2 (address and
    private key), and a signature that recovers to the expected address.
  • Seed passphrase — an assertion that a non-empty BIP-39 passphrase would yield
    a different address, so the empty string in hdWalletFromMnemonic is pinned
    as load-bearing.

vault.js:

  • Round trip, including a non-ASCII plaintext and an empty password (which must
    not act as a skeleton key in either direction).
  • Wrong password rejects as a rejected promise, with no partial plaintext in the
    error.
  • Tampering rejected: flipped ciphertext bit, flipped bit in the Poly1305 tag,
    flipped nonce bit, flipped salt bit, truncated ciphertext, ciphertext shorter
    than the tag, truncated nonce, a ciphertext spliced in from another vault, and
    each field missing.
  • Argon2id cost pinned (describe("key derivation cost")) — see below.
  • Fresh salt and nonce: two encryptions of the same plaintext under the same
    password differ in all three fields and both still decrypt.
  • Shape: exactly { salt, nonce, ciphertext }, all base64, salt 16 bytes, nonce
    24 bytes, ciphertext = plaintext length + 16-byte tag, survives
    JSON.parse(JSON.stringify(...)).
  • No leakage: neither the plaintext (nor any single word of it) nor the password
    appears in the serialized blob, and the ciphertext bytes do not contain the
    plaintext bytes.

Production Argon2id parameters are not weakened or overridden. The tamper cases
share one encrypted fixture instead of re-encrypting per test; the whole suite
runs in ~5s against script/test's 30-second budget.

Argon2id cost is now pinned

The KDF cost is the whole of the vault's resistance to offline brute force on a
stolen blob, and lowering it breaks nothing else the suite can observe — it just
runs faster. Three tests pin it from independent angles, so no single edit slips
through:

  1. the interactive constants still mean 2 passes over 64 MiB — pins
    crypto_pwhash_OPSLIMIT_INTERACTIVE / MEMLIMIT_INTERACTIVE to their
    absolute values (2 and 67108864), so a libsodium upgrade that redefined the
    constants downward would also be caught, and asserts the _MIN floor is
    strictly below both.
  2. a key derived at the interactive parameters opens the vault — spy-free and
    independent of the module's code path: derives a key in the test from the
    vault's own published salt at the interactive cost and opens the vault's
    ciphertext with crypto_secretbox_open_easy directly. A vault built at any
    other opslimit, memlimit or Argon2id variant yields a different key and
    cannot be opened this way.
  3. encrypt / decrypt derives exactly one key at the interactive cost
    observes the actual crypto_pwhash call from each direction and asserts key
    length, salt length, opslimit, memlimit and ALG_ARGON2ID13.

Mutant evidence

Mutation applied with the editor (no scripted substitution), all four
occurrences in src/shared/vault.js:24-25,51-52 changed from the _INTERACTIVE
constants to the _MIN constants — opslimit 2 to 1, memlimit 64 MiB to 8 KiB,
an 8192x memory reduction.

Before, on the previous head (16a3d3c), the mutant was invisible — all 217
tests passed, the suite merely ran faster (1.77s vs 4.23s).

After, on this head:

● key derivation cost › a key derived at the interactive parameters opens the vault
● key derivation cost › encrypt derives exactly one key at the interactive cost
● key derivation cost › decrypt derives exactly one key at the interactive cost
Test Suites: 1 failed, 8 passed, 9 total
Tests:       3 failed, 1 skipped, 218 passed, 222 total
Time:        1.009 s
● key derivation cost › encrypt derives exactly one key at the interactive cost

    expect(received).toBe(expected) // Object.is equality

    Expected: 2
    Received: 1

    > 232 |             expect(opslimit).toBe(sodium.crypto_pwhash_OPSLIMIT_INTERACTIVE);

The mutation was then reverted with the editor and git diff src/ confirmed
empty before committing; this PR remains tests-only.

Test vector sources

  • test test test test test test test test test test test junk and its first
    three accounts at m/44'/60'/0'/0/n with an empty seed passphrase
    (0xf39Fd6e5..., 0x70997970..., 0x3C44CdDd...) plus their private keys —
    the standard development phrase published in the Hardhat and Ganache
    documentation. Publicly known; never funded.
  • abandon abandon ... about (BIP-39 all-zero-entropy vector, first entry of the
    official vector set) with 0x9858EfFD232B4033E47d90003D41EC34EcaEda94 at
    m/44'/60'/0'/0/0.
  • BIP-32 test vector 1 master key (xprv9s21ZrQH143K3QTDL..., seed
    000102030405060708090a0b0c0d0e0f).

The two Hardhat facts cross-check each other: the published private key for
account n must yield the same address the HD path reaches, so a wrong path and a
wrong key-to-address step cannot cancel out.

The m/ prefix asymmetry: proven harmless

hdWalletFromMnemonic:25 derives the absolute m/44'/60'/0'/0 while
hdWalletFromXprv:36 derives the relative 44'/60'/0'/0. For a depth-0 master
key these are the same derivation. Pinned by test: for the same phrase, the xprv
path and the mnemonic path produce the identical xpub, the identical first
address, and the identical children 0-2, all equal to the published vectors. The
only behavioural difference is that the relative form would also accept a
non-master extended key and derive beneath it; the absolute form would not.

Defect found — reported, not fixed

isValidXprv accepts an extended private key containing a one-character typo,
and the import flow (src/popup/views/addWallet.js:215 gates on it) then
silently creates a different wallet. ethers' HDNodeWallet.fromExtendedKey
skips base58 checksum verification whenever the decoded payload is the usual 82
bytes, which is exactly what that checksum exists to catch.

Measured on the BIP-32 vector 1 key: changing any one of the last 14 characters
passes isValidXprv, and for 9 of those 14 positions the derived wallet
differs, e.g.

good                    0x022b971dFF0C43305e691DEd7a14367AF19D6407
typo at position -6     0x3F334f0a356d6B46B1d70B590E7437D77100d28D
typo at position -8     0xb2F65a69C455e86be8ed5e2fd170E47fb1cD0CCe
typo at position -14    0x95E7E0Ba8CD1a76F70A3fDD747AF47051Df39774

No error is shown; the user sees an empty wallet and no indication that their
key was mistyped. Out of scope for this PR, which is tests only. It is now
tracked as #210, which the
skipped test in tests/wallet.test.js cites; that test asserts the correct
behaviour and is to be unskipped when the validation is fixed. Everything else
passes.

Mutation evidence (derivation and vault)

Each mutation was applied to the source, the suite run, then the source
restored, with git diff verified clean before commit.

wallet.js, hdWalletFromMnemonic child index 0 -> 1 — 4 failures:

● hdWalletFromMnemonic › first address matches the published vector for m/44'/60'/0'/0/0
● hdWalletFromMnemonic › second published phrase derives its published address
● deriveAddressFromXpub › agrees with hdWalletFromMnemonic at index 0
● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address

wallet.js, seed passphrase "" -> "x" — 6 failures:

● hdWalletFromMnemonic › first address matches the published vector for m/44'/60'/0'/0/0
● hdWalletFromMnemonic › second published phrase derives its published address
● deriveAddressFromXpub › child 0 matches the published vector address
● deriveAddressFromXpub › child 1 matches the published vector address
● deriveAddressFromXpub › child 2 matches the published vector address
● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address

wallet.js, xprv path 44'/60'/0'/0 -> 44'/60'/1'/0 — 3 failures:

● hdWalletFromXprv › master xprv for the vector phrase yields the vector address
● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address
● hdWalletFromXprv › derived xpub generates the same child addresses

vault.js, fixed all-zero nonce — 1 failure:

● fresh salt and nonce › two encryptions of the same plaintext differ in all three fields

vault.js, plaintext copied into the stored blob as an extra field — 2
failures:

● stored blob shape › is exactly the documented { salt, nonce, ciphertext }
● no plaintext leakage › the secret does not appear in the serialized vault

On the auth-tag layer: a realistic skipped-verification mutant (true XSalsa20
keystream XOR with no MAC check, confirmed to act as a decryption oracle) trips
5 tests, including a flipped ciphertext bit is rejected by the auth tag and
a flipped bit in the authentication tag itself is rejected. The tamper tests
do discriminate the tag layer; no dedicated auth-tag test is needed.

Verification

Rebased onto current next (cf5f582); the TODO.md Completed Steps conflict
was resolved keeping all sides' entries. One commit, tests only, base next,
git merge-tree clean against next.

make check on the rebased branch:

Test Suites: 9 passed, 9 total
Tests:       1 skipped, 221 passed, 222 total
Time:        5.041 s
All matched files use Prettier code style!   (lint)
All matched files use Prettier code style!   (fmt-check)

And in the container on this exact head, docker build --no-cache on this
image only, so the step demonstrably executed rather than reporting CACHED:

#11 [7/8] RUN make check
#11 7.942 Test Suites: 9 passed, 9 total
#11 7.942 Tests:       1 skipped, 221 passed, 222 total
#11 14.86 All matched files use Prettier code style!
#11 20.37 All matched files use Prettier code style!
#11 DONE 21.4s

The two README testing checkboxes for mnemonic/address derivation and xpub child
generation are ticked, and TODO.md gains one line, in the same commit.

Closes [#159](https://git.eeqj.de/sneak/AutistMask/issues/159). Tests only — no source file is modified. ## What is covered `tests/wallet.test.js` (extended around the six existing `DEBUG`-flag tests, which are untouched) and `tests/vault.test.js` (new). `wallet.js`, pinned to published vectors rather than to what the code returns today: - `hdWalletFromMnemonic` — first address for two independent phrases; the account-level xpub is watch-only; the account path is `m/44'/60'/0'/0`; an invalid phrase throws. - `deriveAddressFromXpub` — children 0, 1, 2 against published addresses, and agreement with `hdWalletFromMnemonic` at index 0; garbage throws. - `hdWalletFromXprv` — master xprv for the same phrase reaches the same published address, and `toEqual` against `hdWalletFromMnemonic`'s whole result; its xpub generates the same three children; a watch-only xpub and garbage are rejected. - `isValidMnemonic` / `isValidXprv` — valid, bad checksum, 11 words, off-wordlist word, empty, garbage, bare private key, truncated and over-long keys. - `addressFromPrivateKey` — the three published key/address pairs; wrong length and empty string throw. - `getSignerForAddress` — all three wallet types at indices 0, 1, 2 (address and private key), and a signature that recovers to the expected address. - Seed passphrase — an assertion that a non-empty BIP-39 passphrase would yield a different address, so the empty string in `hdWalletFromMnemonic` is pinned as load-bearing. `vault.js`: - Round trip, including a non-ASCII plaintext and an empty password (which must not act as a skeleton key in either direction). - Wrong password rejects as a rejected promise, with no partial plaintext in the error. - Tampering rejected: flipped ciphertext bit, flipped bit in the Poly1305 tag, flipped nonce bit, flipped salt bit, truncated ciphertext, ciphertext shorter than the tag, truncated nonce, a ciphertext spliced in from another vault, and each field missing. - **Argon2id cost pinned** (`describe("key derivation cost")`) — see below. - Fresh salt and nonce: two encryptions of the same plaintext under the same password differ in all three fields and both still decrypt. - Shape: exactly `{ salt, nonce, ciphertext }`, all base64, salt 16 bytes, nonce 24 bytes, ciphertext = plaintext length + 16-byte tag, survives `JSON.parse(JSON.stringify(...))`. - No leakage: neither the plaintext (nor any single word of it) nor the password appears in the serialized blob, and the ciphertext bytes do not contain the plaintext bytes. Production Argon2id parameters are not weakened or overridden. The tamper cases share one encrypted fixture instead of re-encrypting per test; the whole suite runs in ~5s against `script/test`'s 30-second budget. ## Argon2id cost is now pinned The KDF cost is the whole of the vault's resistance to offline brute force on a stolen blob, and lowering it breaks nothing else the suite can observe — it just runs faster. Three tests pin it from independent angles, so no single edit slips through: 1. `the interactive constants still mean 2 passes over 64 MiB` — pins `crypto_pwhash_OPSLIMIT_INTERACTIVE` / `MEMLIMIT_INTERACTIVE` to their absolute values (2 and 67108864), so a libsodium upgrade that redefined the constants downward would also be caught, and asserts the `_MIN` floor is strictly below both. 2. `a key derived at the interactive parameters opens the vault` — spy-free and independent of the module's code path: derives a key in the test from the vault's own published salt at the interactive cost and opens the vault's ciphertext with `crypto_secretbox_open_easy` directly. A vault built at any other opslimit, memlimit or Argon2id variant yields a different key and cannot be opened this way. 3. `encrypt` / `decrypt` `derives exactly one key at the interactive cost` — observes the actual `crypto_pwhash` call from each direction and asserts key length, salt length, opslimit, memlimit and `ALG_ARGON2ID13`. ### Mutant evidence Mutation applied with the editor (no scripted substitution), all four occurrences in `src/shared/vault.js:24-25,51-52` changed from the `_INTERACTIVE` constants to the `_MIN` constants — opslimit 2 to 1, memlimit 64 MiB to 8 KiB, an 8192x memory reduction. Before, on the previous head (`16a3d3c`), the mutant was invisible — all 217 tests passed, the suite merely ran faster (1.77s vs 4.23s). After, on this head: ``` ● key derivation cost › a key derived at the interactive parameters opens the vault ● key derivation cost › encrypt derives exactly one key at the interactive cost ● key derivation cost › decrypt derives exactly one key at the interactive cost Test Suites: 1 failed, 8 passed, 9 total Tests: 3 failed, 1 skipped, 218 passed, 222 total Time: 1.009 s ``` ``` ● key derivation cost › encrypt derives exactly one key at the interactive cost expect(received).toBe(expected) // Object.is equality Expected: 2 Received: 1 > 232 | expect(opslimit).toBe(sodium.crypto_pwhash_OPSLIMIT_INTERACTIVE); ``` The mutation was then reverted with the editor and `git diff src/` confirmed empty before committing; this PR remains tests-only. ## Test vector sources - `test test test test test test test test test test test junk` and its first three accounts at `m/44'/60'/0'/0/n` with an empty seed passphrase (`0xf39Fd6e5...`, `0x70997970...`, `0x3C44CdDd...`) plus their private keys — the standard development phrase published in the Hardhat and Ganache documentation. Publicly known; never funded. - `abandon abandon ... about` (BIP-39 all-zero-entropy vector, first entry of the official vector set) with `0x9858EfFD232B4033E47d90003D41EC34EcaEda94` at `m/44'/60'/0'/0/0`. - BIP-32 test vector 1 master key (`xprv9s21ZrQH143K3QTDL...`, seed `000102030405060708090a0b0c0d0e0f`). The two Hardhat facts cross-check each other: the published private key for account n must yield the same address the HD path reaches, so a wrong path and a wrong key-to-address step cannot cancel out. ## The `m/` prefix asymmetry: proven harmless `hdWalletFromMnemonic:25` derives the absolute `m/44'/60'/0'/0` while `hdWalletFromXprv:36` derives the relative `44'/60'/0'/0`. For a depth-0 master key these are the same derivation. Pinned by test: for the same phrase, the xprv path and the mnemonic path produce the identical xpub, the identical first address, and the identical children 0-2, all equal to the published vectors. The only behavioural difference is that the relative form would also accept a non-master extended key and derive beneath it; the absolute form would not. ## Defect found — reported, not fixed `isValidXprv` accepts an extended private key containing a one-character typo, and the import flow (`src/popup/views/addWallet.js:215` gates on it) then silently creates a **different wallet**. ethers' `HDNodeWallet.fromExtendedKey` skips base58 checksum verification whenever the decoded payload is the usual 82 bytes, which is exactly what that checksum exists to catch. Measured on the BIP-32 vector 1 key: changing any one of the last 14 characters passes `isValidXprv`, and for 9 of those 14 positions the derived wallet differs, e.g. ``` good 0x022b971dFF0C43305e691DEd7a14367AF19D6407 typo at position -6 0x3F334f0a356d6B46B1d70B590E7437D77100d28D typo at position -8 0xb2F65a69C455e86be8ed5e2fd170E47fb1cD0CCe typo at position -14 0x95E7E0Ba8CD1a76F70A3fDD747AF47051Df39774 ``` No error is shown; the user sees an empty wallet and no indication that their key was mistyped. Out of scope for this PR, which is tests only. It is now tracked as [#210](https://git.eeqj.de/sneak/AutistMask/issues/210), which the skipped test in `tests/wallet.test.js` cites; that test asserts the correct behaviour and is to be unskipped when the validation is fixed. Everything else passes. ## Mutation evidence (derivation and vault) Each mutation was applied to the source, the suite run, then the source restored, with `git diff` verified clean before commit. `wallet.js`, `hdWalletFromMnemonic` child index `0` -> `1` — 4 failures: ``` ● hdWalletFromMnemonic › first address matches the published vector for m/44'/60'/0'/0/0 ● hdWalletFromMnemonic › second published phrase derives its published address ● deriveAddressFromXpub › agrees with hdWalletFromMnemonic at index 0 ● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address ``` `wallet.js`, seed passphrase `""` -> `"x"` — 6 failures: ``` ● hdWalletFromMnemonic › first address matches the published vector for m/44'/60'/0'/0/0 ● hdWalletFromMnemonic › second published phrase derives its published address ● deriveAddressFromXpub › child 0 matches the published vector address ● deriveAddressFromXpub › child 1 matches the published vector address ● deriveAddressFromXpub › child 2 matches the published vector address ● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address ``` `wallet.js`, xprv path `44'/60'/0'/0` -> `44'/60'/1'/0` — 3 failures: ``` ● hdWalletFromXprv › master xprv for the vector phrase yields the vector address ● hdWalletFromXprv › agrees with hdWalletFromMnemonic on xpub and address ● hdWalletFromXprv › derived xpub generates the same child addresses ``` `vault.js`, fixed all-zero nonce — 1 failure: ``` ● fresh salt and nonce › two encryptions of the same plaintext differ in all three fields ``` `vault.js`, plaintext copied into the stored blob as an extra field — 2 failures: ``` ● stored blob shape › is exactly the documented { salt, nonce, ciphertext } ● no plaintext leakage › the secret does not appear in the serialized vault ``` On the auth-tag layer: a realistic skipped-verification mutant (true XSalsa20 keystream XOR with no MAC check, confirmed to act as a decryption oracle) trips 5 tests, including `a flipped ciphertext bit is rejected by the auth tag` and `a flipped bit in the authentication tag itself is rejected`. The tamper tests do discriminate the tag layer; no dedicated auth-tag test is needed. ## Verification Rebased onto current `next` (`cf5f582`); the `TODO.md` Completed Steps conflict was resolved keeping all sides' entries. One commit, tests only, base `next`, `git merge-tree` clean against `next`. `make check` on the rebased branch: ``` Test Suites: 9 passed, 9 total Tests: 1 skipped, 221 passed, 222 total Time: 5.041 s All matched files use Prettier code style! (lint) All matched files use Prettier code style! (fmt-check) ``` And in the container on this exact head, `docker build --no-cache` on this image only, so the step demonstrably executed rather than reporting `CACHED`: ``` #11 [7/8] RUN make check #11 7.942 Test Suites: 9 passed, 9 total #11 7.942 Tests: 1 skipped, 221 passed, 222 total #11 14.86 All matched files use Prettier code style! #11 20.37 All matched files use Prettier code style! #11 DONE 21.4s ``` The two README testing checkboxes for mnemonic/address derivation and xpub child generation are ticked, and `TODO.md` gains one line, in the same commit.
clawbot added the needs-review label 2026-08-11 14:29:44 +02:00
clawbot added 1 commit 2026-08-11 14:29:44 +02:00
test: known-answer coverage for HD derivation and the vault (closes #159)
All checks were successful
check / check (push) Successful in 32s
16a3d3cefc
wallet.js and vault.js — the two modules that hold user funds — had no
derivation or encryption tests. Add them, pinned to published vectors
rather than to whatever the implementation returns today.

wallet.js: hdWalletFromMnemonic, hdWalletFromXprv, deriveAddressFromXpub
and getSignerForAddress are pinned to the standard development recovery
phrase's first three accounts at m/44'/60'/0'/0/n and to the BIP-39
all-zero-entropy phrase's first address; addressFromPrivateKey is pinned
to the published key/address pairs, so the HD path and the bare-key path
must meet at the same address from two directions. isValidMnemonic and
isValidXprv cover bad checksum, wrong word count, wrong key type and
empty/garbage input. The absolute-vs-relative path asymmetry between
hdWalletFromMnemonic and hdWalletFromXprv is proven harmless: for the
same master key both reach the same xpub and the same addresses.

vault.js: round trip (including non-ASCII and an empty password), wrong
password rejected as a rejected promise with no partial plaintext,
tampered ciphertext / auth tag / nonce / salt rejected, truncated and
spliced blobs rejected, missing fields rejected, fresh salt and nonce per
encryption, the documented { salt, nonce, ciphertext } shape, and no
trace of the plaintext or password anywhere in the serialized blob. The
production Argon2id parameters are not weakened; the tamper cases share
one encrypted fixture to stay inside script/test's 30-second budget.

One test is skipped: isValidXprv accepts an extended private key with a
one-character typo, because ethers skips base58 checksum verification for
the usual 82-byte payload. That is a defect to be filed separately, not
fixed here; the skipped test asserts the correct behaviour and names the
reason.

Suite: 211 passed, 1 skipped, 7.2s inside the container build.
clawbot self-assigned this 2026-08-11 14:30:00 +02:00
Author
Collaborator

FAIL — needs-rebase.

Blocking

1. Not mergeable. Branch is based on 19cb1ca; next is now b9bc226.
git merge-tree gives CONFLICT (content): Merge conflict in TODO.md — both
sides added a "Completed Steps" entry at the same position. Rebase onto current
next and re-push.

2. CI never ran on the head commit. check / check (push) on 16a3d3c has
been pending / "Waiting to run" since 14:28:50. Nothing is red, but nothing is
green either. I ran docker build --no-cache on the head myself and step
RUN make check executed in 18.2s uncached — 9 suites, 217 passed, 1 skipped,
prettier clean — so this is a queue problem, not a code problem. It still needs
a green run before merge.

Finding — the Argon2id cost parameters are unpinned

tests/vault.test.js pins every vault property except the one that decides
whether a stolen vault survives an offline attack.

Changing src/shared/vault.js:24-25,51-52 from
crypto_pwhash_OPSLIMIT_INTERACTIVE / MEMLIMIT_INTERACTIVE to the _MIN
constants — opslimit 2 to 1, memlimit 64 MiB to 8 KiB, an 8192x memory
reduction — leaves all 217 tests passing. The suite even gets faster
(1.77s vs 4.23s), which is the only signal it produces.

Why it matters: the KDF cost is the whole of the vault's resistance to offline
brute force on a stolen blob. A future edit that guts it is precisely the
silent-failure class this file's own header says it exists to catch, and it is
the one case that walks straight through. The PR's statement that production
parameters are not weakened is accurate about what the tests do; the gap is
the absence of any test that would notice a later weakening.

Acceptable: assert the derived key against a published Argon2id known-answer
vector, or at minimum assert the module derives with the INTERACTIVE
opslimit/memlimit constants rather than whatever is currently passed.

On the disclosed auth-tag bound — no action needed

The disclosure understates the coverage. The mutant used (returning the raw
unverified ciphertext body) is not a shape a real bug takes, and its non-UTF-8
output is the only reason just 2 tests tripped.

Substituting a realistic skipped-verification mutant — true XSalsa20 keystream
XOR with no MAC check, confirmed to be a genuine decryption oracle (a flipped
ciphertext bit returns "test!test test test..." successfully, no error) —
trips 5 tests, including both a flipped ciphertext bit is rejected by the auth tag and a flipped bit in the authentication tag itself is rejected. The
tamper tests do discriminate the tag layer. A dedicated auth-tag test is not
required before this lands.

Non-blocking

The skipped test's comment names no issue number for the isValidXprv defect
("to be filed as its own issue"). Once that issue exists the comment should cite
it, or the skip has no tracking anchor.

Verified

Vector provenance is genuine known-answer, not self-consistency: recomputed from
first principles with a hand-rolled BIP-39 PBKDF2 + BIP-32 CKDpriv over noble
secp256k1/keccak, touching no ethers HD code — all three development-phrase
addresses and private keys, the zero-entropy address, and the BIP-32 vector-1
master xprv rebuilt from seed 000102030405060708090a0b0c0d0e0f — and
cross-checked against the published Hardhat reference and the BIP-32 spec text.

Mutants caught: coin type 12, account index 12, change index 12, xprv relative
path 3, non-empty seed passphrase 6, xpub child off-by-one 5, dropped EIP-55
checksum casing 3, fixed nonce 1.

The isValidXprv typo defect reproduces exactly as characterised: 14 of 14
single-character edits to the last 14 characters pass validation, 9 of 14 yield
a different wallet, and the quoted addresses match mine.

Six pre-existing DEBUG tests byte-identical (only the file header comment
changed); README checkboxes accurate; one TODO.md entry; single commit ending
(closes #159); base next; tests only, no source change; no attribution
trailers; make check green on host and in-container.

FAIL — `needs-rebase`. ## Blocking **1. Not mergeable.** Branch is based on `19cb1ca`; `next` is now `b9bc226`. `git merge-tree` gives `CONFLICT (content): Merge conflict in TODO.md` — both sides added a "Completed Steps" entry at the same position. Rebase onto current `next` and re-push. **2. CI never ran on the head commit.** `check / check (push)` on `16a3d3c` has been `pending` / "Waiting to run" since 14:28:50. Nothing is red, but nothing is green either. I ran `docker build --no-cache` on the head myself and step `RUN make check` executed in 18.2s uncached — 9 suites, 217 passed, 1 skipped, prettier clean — so this is a queue problem, not a code problem. It still needs a green run before merge. ## Finding — the Argon2id cost parameters are unpinned `tests/vault.test.js` pins every vault property except the one that decides whether a stolen vault survives an offline attack. Changing `src/shared/vault.js:24-25,51-52` from `crypto_pwhash_OPSLIMIT_INTERACTIVE` / `MEMLIMIT_INTERACTIVE` to the `_MIN` constants — opslimit 2 to 1, memlimit 64 MiB to 8 KiB, an 8192x memory reduction — leaves **all 217 tests passing**. The suite even gets faster (1.77s vs 4.23s), which is the only signal it produces. Why it matters: the KDF cost is the whole of the vault's resistance to offline brute force on a stolen blob. A future edit that guts it is precisely the silent-failure class this file's own header says it exists to catch, and it is the one case that walks straight through. The PR's statement that production parameters are not weakened is accurate about what the tests *do*; the gap is the absence of any test that would notice a later weakening. Acceptable: assert the derived key against a published Argon2id known-answer vector, or at minimum assert the module derives with the INTERACTIVE opslimit/memlimit constants rather than whatever is currently passed. ## On the disclosed auth-tag bound — no action needed The disclosure understates the coverage. The mutant used (returning the raw unverified ciphertext body) is not a shape a real bug takes, and its non-UTF-8 output is the only reason just 2 tests tripped. Substituting a realistic skipped-verification mutant — true XSalsa20 keystream XOR with no MAC check, confirmed to be a genuine decryption oracle (a flipped ciphertext bit returns `"test!test test test..."` successfully, no error) — trips 5 tests, including both `a flipped ciphertext bit is rejected by the auth tag` and `a flipped bit in the authentication tag itself is rejected`. The tamper tests do discriminate the tag layer. A dedicated auth-tag test is not required before this lands. ## Non-blocking The skipped test's comment names no issue number for the `isValidXprv` defect ("to be filed as its own issue"). Once that issue exists the comment should cite it, or the skip has no tracking anchor. ## Verified Vector provenance is genuine known-answer, not self-consistency: recomputed from first principles with a hand-rolled BIP-39 PBKDF2 + BIP-32 CKDpriv over noble secp256k1/keccak, touching no ethers HD code — all three development-phrase addresses and private keys, the zero-entropy address, and the BIP-32 vector-1 master xprv rebuilt from seed `000102030405060708090a0b0c0d0e0f` — and cross-checked against the published Hardhat reference and the BIP-32 spec text. Mutants caught: coin type 12, account index 12, change index 12, xprv relative path 3, non-empty seed passphrase 6, xpub child off-by-one 5, dropped EIP-55 checksum casing 3, fixed nonce 1. The `isValidXprv` typo defect reproduces exactly as characterised: 14 of 14 single-character edits to the last 14 characters pass validation, 9 of 14 yield a different wallet, and the quoted addresses match mine. Six pre-existing DEBUG tests byte-identical (only the file header comment changed); README checkboxes accurate; one `TODO.md` entry; single commit ending ` (closes #159)`; base `next`; tests only, no source change; no attribution trailers; `make check` green on host and in-container.
clawbot added needs-rebase and removed needs-review labels 2026-08-11 14:44:19 +02:00
clawbot force-pushed test/issue-159-crypto-core-tests from 16a3d3cefc to 2dcea6c306 2026-08-11 14:49:07 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-08-11 14:56:50 +02:00
Author
Collaborator

FAIL — needs-rebase. The substance passes; mergeability does not.

Blocking: conflicts with current next (f271bcd). Branch is 1 ahead / 3 behind; git merge origin/next gives a content conflict in TODO.md Completed Steps, where the #159 entry sits adjacent to the #166 and #180 entries that next gained after the rebase onto cf5f582. No entries are lost on either side — interleave and force-push. Tracker agrees (mergeable: false).

Argon2id pin independently verified — no partial mutant survives. Each mutation applied to src/shared/vault.js with the editor, suite run via make test, then reverted with the tree re-confirmed clean: all four _INTERACTIVE to _MIN gives 3 failures (matches the claim); opslimit alone, 3; memlimit alone, 3; ALG_ARGON2ID13 to ALG_ARGON2I13 at opslimit 3, i.e. a valid non-crashing Argon2i vault, 3; salt 16 to 8 bytes, 28 (libsodium rejects the length outright, so this one is a crash-level catch rather than a discriminating one); asymmetric encrypt-at-interactive / decrypt-at-_MIN, 6.

The three angles are not three spellings of one assertion. The spy-free test hardcodes the literals 2 and 67108864 rather than reading sodium.crypto_pwhash_*_INTERACTIVE, so it genuinely fails for a vault built at any other cost or variant rather than re-deriving with whatever the source currently uses — confirmed by the Argon2i mutant, which produces a perfectly functional vault at a higher opslimit and is still caught. It correctly stays green on the decrypt-only mutant, which the spy test catches instead; the two cover opposite directions.

make check on 2dcea6c: 9 suites, 221 passed, 1 skipped, prettier clean on both lint and fmt-check. Tests-only confirmed — git diff --name-only cf5f582..HEAD -- src/ is empty and no trace of the author's evidence mutations survives. Single commit, title ends (closes #159), base next, authored clawbot, no attribution trailers, no inclusive-terminology hits; the skipped xprv test cites #210.

Disclosure: the pre-rebase head 16a3d3c is no longer fetchable, so I could not byte-compare the HD vectors against their pre-rebase form; instead I confirmed they are the published values and that TODO.md lost nothing relative to its base cf5f582. This repo's script/lint runs prettier on the host by design, not in a container; I invoked it only through make check.

FAIL — `needs-rebase`. The substance passes; mergeability does not. **Blocking:** conflicts with current `next` (`f271bcd`). Branch is 1 ahead / 3 behind; `git merge origin/next` gives a content conflict in `TODO.md` Completed Steps, where the [#159](https://git.eeqj.de/sneak/AutistMask/issues/159) entry sits adjacent to the [#166](https://git.eeqj.de/sneak/AutistMask/issues/166) and [#180](https://git.eeqj.de/sneak/AutistMask/issues/180) entries that `next` gained after the rebase onto `cf5f582`. No entries are lost on either side — interleave and force-push. Tracker agrees (`mergeable: false`). **Argon2id pin independently verified — no partial mutant survives.** Each mutation applied to `src/shared/vault.js` with the editor, suite run via `make test`, then reverted with the tree re-confirmed clean: all four `_INTERACTIVE` to `_MIN` gives 3 failures (matches the claim); opslimit alone, 3; memlimit alone, 3; `ALG_ARGON2ID13` to `ALG_ARGON2I13` at opslimit 3, i.e. a valid non-crashing Argon2i vault, 3; salt 16 to 8 bytes, 28 (libsodium rejects the length outright, so this one is a crash-level catch rather than a discriminating one); asymmetric encrypt-at-interactive / decrypt-at-`_MIN`, 6. The three angles are not three spellings of one assertion. The spy-free test hardcodes the literals `2` and `67108864` rather than reading `sodium.crypto_pwhash_*_INTERACTIVE`, so it genuinely fails for a vault built at any other cost or variant rather than re-deriving with whatever the source currently uses — confirmed by the Argon2i mutant, which produces a perfectly functional vault at a *higher* opslimit and is still caught. It correctly stays green on the decrypt-only mutant, which the spy test catches instead; the two cover opposite directions. `make check` on `2dcea6c`: 9 suites, 221 passed, 1 skipped, prettier clean on both lint and fmt-check. Tests-only confirmed — `git diff --name-only cf5f582..HEAD -- src/` is empty and no trace of the author's evidence mutations survives. Single commit, title ends ` (closes #159)`, base `next`, authored `clawbot`, no attribution trailers, no inclusive-terminology hits; the skipped xprv test cites [#210](https://git.eeqj.de/sneak/AutistMask/issues/210). Disclosure: the pre-rebase head `16a3d3c` is no longer fetchable, so I could not byte-compare the HD vectors against their pre-rebase form; instead I confirmed they are the published values and that `TODO.md` lost nothing relative to its base `cf5f582`. This repo's `script/lint` runs prettier on the host by design, not in a container; I invoked it only through `make check`.
clawbot added needs-rebase and removed needs-review labels 2026-08-11 15:04:25 +02:00
clawbot force-pushed test/issue-159-crypto-core-tests from 2dcea6c306 to 22cf2f43f5 2026-08-11 15:05:59 +02:00 Compare
clawbot merged commit f455b0ae7f into next 2026-08-11 15:06:36 +02:00
clawbot deleted branch test/issue-159-crypto-core-tests 2026-08-11 15:06:36 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#209