Commit Graph
2 Commits
Author SHA1 Message Date
thePR0M3TH3ANandClaude Opus 5 98b2967116 Ban Math.random package-wide and enforce it in CI
Neither remaining Math.random call was a secret. The backup-quiz picker
chooses which three recovery words to re-type -- an attacker who cannot see
the phrase gains nothing from predicting them -- and the relay subscription
id is a public wire identifier scoped to one socket, where collision is the
only failure mode.

They are changed anyway, because the point is the rule rather than these two
call sites. A Math.random ban that carries a standing exception is worth
much less than one that does not: the exception is what a future misuse
hides behind, and reviewers stop reading a warning they have already agreed
to ignore twice. Both now use the CSPRNG (randomUniformInt, and
bytesToHex(randomBytes(8)) for the subscription id, same 16-hex-character
shape as before), which costs nothing here.

With no exception left, the rule can be absolute. .semgrep.yml adds three
rules over packages/*/src/**, run in CI as a separate step after
--config auto:

- bitlogin-no-math-random: no Math.random anywhere, secret or not
- bitlogin-no-crypto-fallback: no `?? fallback`, `|| fallback`, or
  catch-and-substitute around getRandomValues -- the COLDCARD defect class
- bitlogin-no-time-or-id-seeded-secrets: no hashing Date.now() or
  performance.now() into a key

Verified the rules fire rather than merely passing, which is the failure
mode of a lint rule nobody tests: reintroducing the old
Math.floor(Math.random() * pool.length) produces 1 blocking finding and exit
1, and the tree is clean at 0 findings with it restored. Path patterns are
anchored (/packages/...) so semgrep's upcoming Semgrepignore v2 change does
not silently alter their scope.

Typecheck and the full suite pass (185 tests).

This closes the entropy audit for this repo. The only gate remaining is an
independent review of the RNG path by someone other than the author.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01HtNMsKzzFTPURemX7tT19u
2026-07-31 17:30:24 -04:00
thePR0M3TH3ANandClaude Fable 5 7d2209d706 Second-audit hardening: phrase-gated sudo key, deterministic migration, policy tests
Authored by ChatGPT/Codex in a separate review pass. Reviewed and verified
here before committing rather than taken on trust; findings below.

The substantive change is that the vault sudo key now lives ONLY in the
phrase-gated recovery capsule. A newly written password credential capsule
carries just connection_vault_root, so an ordinary login restores
connectable-tier records without putting personal-tier material behind a
password capsule that is downloadable and therefore offline-guessable forever.
Pre-hardening capsules stay readable, password login does not return their
sudo key, and the next rotation scrubs it. That closes the gate this project
had explicitly set on personal-tier UI.

Legacy vault-enable migration now derives roots deterministically from
phrase-protected material and account_id, so two concurrent attempts cannot
mint divergent roots -- the failure mode the docs previously mitigated with
"enable the vault once, from one device".

Manual (user-chosen) passwords are REMOVED, not merely discouraged, and a new
securityPolicy.test.ts pins that along with several things tests do not
usually reach: the deployment CSP (default-src 'none', frame-ancestors
'none') plus six baseline headers, demo pages free of inline scripts and
remote fonts, and README documenting same-origin self-hosting only rather
than a remote demo embed. llms.txt was updated to match, which I checked
because that test only reads README.

What I verified, since a passing suite proves less than it appears to:

- All 12 fixes from the 2026-07-30 audit survived the rewrite of recover.ts,
  changePassword.ts, validation.ts, relay.ts, session.ts and element.ts.
  Checked semantically where grep cannot help: changePassword still checks
  publish success BEFORE building the tombstone, and vaultSetBinding still
  unbinds on origin === null rather than silently rebinding to the worker
  global.
- No dependency changes; the package.json diff is scripts only.
- Argon2id and KDF parameters unchanged. The NIP-44 diff ADDS a base64 length
  cap before decoding, a memory-DoS guard.
- Validation increased rather than decreased: 44 throws added, 33 removed.
- No disabled or skipped tests, no secrets in the diff.

typecheck clean, 165/165 tests, build green, and the account manager exercised
in a browser: creation correctly refused when relay quorum could not be met,
and on retry failed at the recovery capsule with "No login credential was
published" -- the §18.1 fail-closed ordering doing its job against a flaky
public relay set.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMr9Yt8SYmZcfgrg3CV2G1
2026-07-31 13:54:36 -04:00