Code audit of integration/all-fixes vs main. This branch is a major rearchitecture — 949 commits, ~347k insertions across 813 files — introducing three large new subsystems that all sit on the money path:
| Subsystem |
Dir |
Role |
| UXF |
uxf/, types/uxf-* |
New token-bundle serialization/transfer format (IPLD/CAR, hashing, conservation) |
| Profile |
profile/ |
OrbitDB/Helia per-wallet storage; cross-device pointer, epoch reset, migration |
| Transfer pipeline |
modules/payments/transfer/ |
Worker-based send/receive (ingest pool, finalization workers, disposition engine) |
The code is, on the whole, defensively written (explicit signature/CID verification, DoS caps, key-wiping, content-addressing, documented "steelman" hardening passes). This issue collects the gaps found during audit. Intended to be sliced into subtasks later.
Confidence legend: ✅ = verified against the actual code during audit. Unmarked = from deep subsystem reads / analysis, not execution — several hinge on call-site/adapter details that should be confirmed with targeted tests before filing as fix-now.
CRITICAL — verified
✅ C1. Plaintext wallet seed can be written to OrbitDB→IPFS (fail-open encryption)
profile/profile-storage-provider.ts:2128 — encrypt() returns raw UTF-8 when profileEncryptionKey === null; set() calls it whenever dbConnected. The adapter-builder methods all fail closed (return null when the key is absent), but the generic set()/encrypt() path fails open. Migration's stepPersistToOrbitDb writes mnemonic and master_key through storage.set(). If any identity-class write lands in the window where OrbitDB is attached but setIdentity() hasn't derived the key, the seed is replicated to third-party IPFS gateways in cleartext — effectively permanent.
Fix: make encrypt() throw when encryptionEnabled && key === null; hard-block IDENTITY_CLASS_KEYS from the plaintext path unconditionally. Add a boot-order test proving no seed set() precedes key derivation.
✅ C2. Migration deletes legacy data before the new copy is durable
profile/migration.ts — stepPersistToOrbitDb (:585) accepts save()'s cid: 'debounced' return as success without forcing a flush; stepSanityCheck (:643) reads back from in-memory pendingData (passes even if nothing was pinned); stepCleanup (:779) then deletes legacy KV keys and unpins the legacy CID. No forceFlush/awaitNextFlush exists anywhere in migration.ts. A crash (or later flush failure) between cleanup and the debounced flush landing loses legacy KV state and the unpinned CID. Token-DB loss is partially mitigated by the #330 legacy-token-DB fallback if wired; legacy KV and the unpinned CID are not.
Fix: force a durable flush and verify the pin/bundle-ref exists in OrbitDB before stepCleanup.
✅ C3. Auto-db.drop() on a transient block-load error destroys un-bundled state
profile/orbitdb-adapter.ts:1382-1420 / profile/profile-token-storage/flush-scheduler.ts:1722-1847 — addBundleWithOplogAutoReset triggers resetCorruptedLog → db.drop() whenever extractLostHeadCid matches a "Failed to load block" error in the normal flush path. That wipes all OUTBOX/SENT/disposition/finalization entries not yet captured in a pinned bundle. The matcher cannot distinguish a permanently-corrupt head from a momentarily-unreachable one (gateway blip / propagation lag), so a transient fetch failure can permanently destroy operational state.
Fix: gate the destructive reset behind a retry/backoff that confirms the head is genuinely unrecoverable, and/or snapshot before drop.
HIGH
✅ H1. conservative-sender has no same-process source lock → double-spend window
modules/payments/transfer/conservative-sender.ts has zero locking primitives. instant-sender.ts declares const sourceLocks (:463) — module-local, not exported — added explicitly as "Wave 5 steelman fix #171" precisely to close a same-process double-spend race. Conservative sends (and instant-vs-conservative cross-pairs) therefore don't serialize on shared source tokens: two concurrent sends can both pass selection, both commit on-chain, and the aggregator only rejects the loser after a source is burned.
Caveat to confirm: whether PaymentsModule's PerTokenMutex/spend-queue (PaymentsModule.ts:2505) serializes the conservative path at a higher layer. If it does, severity drops. Trace this before fixing — but the asymmetry between the two senders is real.
H2. UXF manifest tokenId key is never bound to the referenced root's genesis tokenId
On import (uxf/json.ts:~420, uxf/ipld.ts:~549) and in uxf/verify.ts, the manifest key is only regex-shape-checked, never asserted equal to the genesis it points to. Element hashes still self-verify, so a bundle mapping tokenId=A → root that mints tokenId=B passes every existing gate — an identity-confusion primitive for any downstream logic that trusts the manifest key (dedup, ownership filtering).
H3. UxfPackage.mergePkg silently drops a token on any per-token resolver throw
uxf/UxfPackage.ts:~800 — a single malformed sibling element makes a legitimately-received token vanish from the merged manifest with only a logger.warn. Token loss from view on the receive path. Should be a hard failure or an explicit per-token error result.
H4. Inclusion proof verified against bundle-supplied requestId, not a re-derived one
modules/payments/transfer/disposition-engine.ts:~629 — nothing in the engine asserts requestId == RequestId(authenticator.publicKey, sourceState); correctness rests entirely on the un-audited hydrateChain adapter deriving it canonically. If it doesn't, a genuine anchored proof can be paired with an unrelated transaction. Assert the binding in the engine rather than assuming it.
H5. Source tokens stuck pending forever on failed-permanent
modules/payments/transfer/finalization-worker-sender.ts:~1183 — instant-sender marks sources pending; on hard-fail the worker never flips them back, and orphan auto-recovery is default-OFF. A failed instant send permanently locks that balance as unspendable pending.
H6. Equal-Lamport CRDT merges are non-convergent for lamport=0 surfaces
profile/profile-snapshot-merge.ts + profile/prefix-sync-writer.ts:256 — FinalizationQueue / RecipientContext / BundleRef / Disposition are all stamped at lamport=0. Two devices writing different content to the same key both sit at 0 → merge is a permanent no-op in both directions → replicas never converge. The "content-immutable" assumption is unenforced (defaultValidator accepts any object).
H7. ManifestCas compares a self-declared rootHash label, not recomputed content
profile/manifest-cas.ts:139-171 — the "compare-and-swap on content hash" is a string compare on an attacker-controllable field; real content-addressing happens elsewhere, so this is an integrity label check, not an integrity check.
H8. Stale-cursor fast-path publishes at localVersion+1 with no on-chain floor check
profile/aggregator-pointer/reconcile-algorithm.ts:330 — the idempotent-replay-vs-conflict disambiguation rests solely on a boolean hint (aggregator-submit.ts:436, which explicitly voids the CID comparison data). A wrong hint either drops a sibling's just-published state or forces needless rejoins. Genuine safety needs comparing the decoded on-chain CID to our intended CID.
MEDIUM
- UXF Wave-H null canonicalization collapses
null / '' / Uint8Array(0) to the same hash but different wire bytes (uxf/hash.ts:151), weakening the "content-hash == exact bytes" guarantee dedup/CID-equality rely on.
- Ingest worker drops a token with no disposition record if
processToken throws after the replay-LRU mark but before any durable write (modules/payments/transfer/ingest-worker-pool.ts:1328) — silent loss, no retry.
- Epoch-floor inspector doesn't enforce
EPOCH_MAX (profile/pointer-wiring.ts:765) while the canonical decoder does; a snapshot with epoch = MAX_SAFE_INTEGER gets persisted as the floor and permanently wedges the wallet (and siblings) above any future epoch.
- Per-writer JOIN errors swallowed while the pointer cursor still advances (
profile/profile-snapshot-dispatcher.ts:255) → persistent writer failure = silent cross-device partial data loss.
- Outbox/sent Lamport read-modify-write has no per-key mutex (
profile/outbox-writer.ts:288, profile/sent-ledger-writer.ts:236) → lost updates / duplicate Lamports under the explicitly-multi-writer worker pool, despite PerTokenMutex existing unused here.
PerTokenMutex 'cas' strategy is a no-op pass-through and 'bounded-hold' abandons a still-running fn on timeout (profile/per-token-mutex.ts:106, :127) — silent foot-guns for callers assuming exclusion.- UXF coin
amount values are fully opaque/unvalidated end-to-end through assemble/deconstruct (uxf/deconstruct.ts:418); a malformed amount string rides into balance computation undetected. UXF delegates conservation to STS, but a shape assertion at the ingest boundary is cheap insurance.
- UXF instance-chain
latest/fallback selection (uxf/instance-chain.ts:247) enables content substitution guarded only by version monotonicity, bypassing the per-reference hash check.
- Nostr replication ordering relies on 1-second
created_at with the disambiguating seq tag written but never read (profile/nostr-replication.ts:230) — non-deterministic re-ingestion order for same-second writes.
LOW / hygiene
uxf/header-validation.ts is committed as a binary blob because its doc comment embeds raw U+202E (RLO) and \x9f/\x1f control bytes as examples (lines 67, 71). The validation logic itself is correct and defensive (allowlist /^[A-Za-z0-9][A-Za-z0-9._-]*$/), but the raw bytes make git treat the file as binary — diffs and PR review go blind on it, and GitHub won't render it. Escape the examples as text.UxfPackage.estimatedSize returns a fixed pool.size * 500 (uxf/UxfPackage.ts:417) — orders of magnitude wrong for 64 KiB-blob elements; risky if any limit decision consumes it.Sphere.clear() may leave encrypted Helia blockstore on disk in Profile mode (core/Sphere.ts:1712, blockstore DB from profile/orbitdb-adapter.ts:430) — re-init with the same mnemonic can decrypt remnants. Add explicit blockstore-DB deletion to clear().
Verified clean (no action)
- No injection surface (no
eval/Function()/child_process/exec; all .exec() are regex). No path traversal (tokenIds hard-gated to hex). No SSRF (operator-configured gateways, http/https-only, CIDs encoded).
- No weak randomness in security paths; secrets use
crypto.getRandomValues. No secret logging; SecretKey redacts; key buffers wiped in finally.
- Nostr replication authenticates (NIP-01 id + schnorr) before decrypt, with replay cache + verify-time budget. IPFS fetches content-address-verified (
verifyCidMatchesBytes); Content-Encoding rejected.
- New dependencies are all well-known registry packages — no git/url deps, no pre/post-install scripts.
- Crypto: AES-256-GCM, random 12-byte IVs, HKDF-SHA256 with domain-separated salt/info, optional AAD against cross-record swaps. UXF element hashing has integer type-ID domain separation, fail-closed on unknown types, deterministic dag-cbor, re-verified CIDs.
Suggested release gate
Block on C1, C2, C3 (plaintext-seed window + two data-destruction paths). Next tier: H1, H2, H4, H5. Distributed-consistency items (H6, H8) are real but lower-probability — reproduce with the multi-device test matrix before fix-now.
Code audit of
integration/all-fixesvsmain. This branch is a major rearchitecture — 949 commits, ~347k insertions across 813 files — introducing three large new subsystems that all sit on the money path:uxf/,types/uxf-*profile/modules/payments/transfer/The code is, on the whole, defensively written (explicit signature/CID verification, DoS caps, key-wiping, content-addressing, documented "steelman" hardening passes). This issue collects the gaps found during audit. Intended to be sliced into subtasks later.
Confidence legend: ✅ = verified against the actual code during audit. Unmarked = from deep subsystem reads / analysis, not execution — several hinge on call-site/adapter details that should be confirmed with targeted tests before filing as fix-now.
CRITICAL — verified
✅ C1. Plaintext wallet seed can be written to OrbitDB→IPFS (fail-open encryption)
profile/profile-storage-provider.ts:2128—encrypt()returns raw UTF-8 whenprofileEncryptionKey === null;set()calls it wheneverdbConnected. The adapter-builder methods all fail closed (returnnullwhen the key is absent), but the genericset()/encrypt()path fails open. Migration'sstepPersistToOrbitDbwritesmnemonicandmaster_keythroughstorage.set(). If any identity-class write lands in the window where OrbitDB is attached butsetIdentity()hasn't derived the key, the seed is replicated to third-party IPFS gateways in cleartext — effectively permanent.Fix: make
encrypt()throw whenencryptionEnabled && key === null; hard-blockIDENTITY_CLASS_KEYSfrom the plaintext path unconditionally. Add a boot-order test proving no seedset()precedes key derivation.✅ C2. Migration deletes legacy data before the new copy is durable
profile/migration.ts—stepPersistToOrbitDb(:585) acceptssave()'scid: 'debounced'return as success without forcing a flush;stepSanityCheck(:643) reads back from in-memorypendingData(passes even if nothing was pinned);stepCleanup(:779) then deletes legacy KV keys and unpins the legacy CID. NoforceFlush/awaitNextFlushexists anywhere inmigration.ts. A crash (or later flush failure) between cleanup and the debounced flush landing loses legacy KV state and the unpinned CID. Token-DB loss is partially mitigated by the #330 legacy-token-DB fallback if wired; legacy KV and the unpinned CID are not.Fix: force a durable flush and verify the pin/bundle-ref exists in OrbitDB before
stepCleanup.✅ C3. Auto-
db.drop()on a transient block-load error destroys un-bundled stateprofile/orbitdb-adapter.ts:1382-1420/profile/profile-token-storage/flush-scheduler.ts:1722-1847—addBundleWithOplogAutoResettriggersresetCorruptedLog→db.drop()wheneverextractLostHeadCidmatches a "Failed to load block" error in the normal flush path. That wipes all OUTBOX/SENT/disposition/finalization entries not yet captured in a pinned bundle. The matcher cannot distinguish a permanently-corrupt head from a momentarily-unreachable one (gateway blip / propagation lag), so a transient fetch failure can permanently destroy operational state.Fix: gate the destructive reset behind a retry/backoff that confirms the head is genuinely unrecoverable, and/or snapshot before drop.
HIGH
✅ H1.
conservative-senderhas no same-process source lock → double-spend windowmodules/payments/transfer/conservative-sender.tshas zero locking primitives.instant-sender.tsdeclaresconst sourceLocks(:463) — module-local, not exported — added explicitly as "Wave 5 steelman fix #171" precisely to close a same-process double-spend race. Conservative sends (and instant-vs-conservative cross-pairs) therefore don't serialize on shared source tokens: two concurrent sends can both pass selection, both commit on-chain, and the aggregator only rejects the loser after a source is burned.Caveat to confirm: whether
PaymentsModule'sPerTokenMutex/spend-queue (PaymentsModule.ts:2505) serializes the conservative path at a higher layer. If it does, severity drops. Trace this before fixing — but the asymmetry between the two senders is real.H2. UXF manifest
tokenIdkey is never bound to the referenced root's genesistokenIdOn import (
uxf/json.ts:~420,uxf/ipld.ts:~549) and inuxf/verify.ts, the manifest key is only regex-shape-checked, never asserted equal to the genesis it points to. Element hashes still self-verify, so a bundle mappingtokenId=A → root that mints tokenId=Bpasses every existing gate — an identity-confusion primitive for any downstream logic that trusts the manifest key (dedup, ownership filtering).H3.
UxfPackage.mergePkgsilently drops a token on any per-token resolver throwuxf/UxfPackage.ts:~800— a single malformed sibling element makes a legitimately-received token vanish from the merged manifest with only alogger.warn. Token loss from view on the receive path. Should be a hard failure or an explicit per-token error result.H4. Inclusion proof verified against bundle-supplied
requestId, not a re-derived onemodules/payments/transfer/disposition-engine.ts:~629— nothing in the engine assertsrequestId == RequestId(authenticator.publicKey, sourceState); correctness rests entirely on the un-auditedhydrateChainadapter deriving it canonically. If it doesn't, a genuine anchored proof can be paired with an unrelated transaction. Assert the binding in the engine rather than assuming it.H5. Source tokens stuck
pendingforever onfailed-permanentmodules/payments/transfer/finalization-worker-sender.ts:~1183— instant-sender marks sourcespending; on hard-fail the worker never flips them back, and orphan auto-recovery is default-OFF. A failed instant send permanently locks that balance as unspendablepending.H6. Equal-Lamport CRDT merges are non-convergent for
lamport=0surfacesprofile/profile-snapshot-merge.ts+profile/prefix-sync-writer.ts:256— FinalizationQueue / RecipientContext / BundleRef / Disposition are all stamped atlamport=0. Two devices writing different content to the same key both sit at 0 → merge is a permanent no-op in both directions → replicas never converge. The "content-immutable" assumption is unenforced (defaultValidatoraccepts any object).H7.
ManifestCascompares a self-declaredrootHashlabel, not recomputed contentprofile/manifest-cas.ts:139-171— the "compare-and-swap on content hash" is a string compare on an attacker-controllable field; real content-addressing happens elsewhere, so this is an integrity label check, not an integrity check.H8. Stale-cursor fast-path publishes at
localVersion+1with no on-chain floor checkprofile/aggregator-pointer/reconcile-algorithm.ts:330— the idempotent-replay-vs-conflict disambiguation rests solely on a boolean hint (aggregator-submit.ts:436, which explicitlyvoids the CID comparison data). A wrong hint either drops a sibling's just-published state or forces needless rejoins. Genuine safety needs comparing the decoded on-chain CID to our intended CID.MEDIUM
null/''/Uint8Array(0)to the same hash but different wire bytes (uxf/hash.ts:151), weakening the "content-hash == exact bytes" guarantee dedup/CID-equality rely on.processTokenthrows after the replay-LRU mark but before any durable write (modules/payments/transfer/ingest-worker-pool.ts:1328) — silent loss, no retry.EPOCH_MAX(profile/pointer-wiring.ts:765) while the canonical decoder does; a snapshot withepoch = MAX_SAFE_INTEGERgets persisted as the floor and permanently wedges the wallet (and siblings) above any future epoch.profile/profile-snapshot-dispatcher.ts:255) → persistent writer failure = silent cross-device partial data loss.profile/outbox-writer.ts:288,profile/sent-ledger-writer.ts:236) → lost updates / duplicate Lamports under the explicitly-multi-writer worker pool, despitePerTokenMutexexisting unused here.PerTokenMutex'cas'strategy is a no-op pass-through and'bounded-hold'abandons a still-runningfnon timeout (profile/per-token-mutex.ts:106,:127) — silent foot-guns for callers assuming exclusion.amountvalues are fully opaque/unvalidated end-to-end through assemble/deconstruct (uxf/deconstruct.ts:418); a malformed amount string rides into balance computation undetected. UXF delegates conservation to STS, but a shape assertion at the ingest boundary is cheap insurance.latest/fallback selection (uxf/instance-chain.ts:247) enables content substitution guarded only by version monotonicity, bypassing the per-reference hash check.created_atwith the disambiguatingseqtag written but never read (profile/nostr-replication.ts:230) — non-deterministic re-ingestion order for same-second writes.LOW / hygiene
uxf/header-validation.tsis committed as a binary blob because its doc comment embeds rawU+202E(RLO) and\x9f/\x1fcontrol bytes as examples (lines 67, 71). The validation logic itself is correct and defensive (allowlist/^[A-Za-z0-9][A-Za-z0-9._-]*$/), but the raw bytes make git treat the file as binary — diffs and PR review go blind on it, and GitHub won't render it. Escape the examples astext.UxfPackage.estimatedSizereturns a fixedpool.size * 500(uxf/UxfPackage.ts:417) — orders of magnitude wrong for 64 KiB-blob elements; risky if any limit decision consumes it.Sphere.clear()may leave encrypted Helia blockstore on disk in Profile mode (core/Sphere.ts:1712, blockstore DB fromprofile/orbitdb-adapter.ts:430) — re-init with the same mnemonic can decrypt remnants. Add explicit blockstore-DB deletion toclear().Verified clean (no action)
eval/Function()/child_process/exec; all.exec()are regex). No path traversal (tokenIds hard-gated to hex). No SSRF (operator-configured gateways, http/https-only, CIDs encoded).crypto.getRandomValues. No secret logging;SecretKeyredacts; key buffers wiped infinally.verifyCidMatchesBytes);Content-Encodingrejected.Suggested release gate
Block on C1, C2, C3 (plaintext-seed window + two data-destruction paths). Next tier: H1, H2, H4, H5. Distributed-consistency items (H6, H8) are real but lower-probability — reproduce with the multi-device test matrix before fix-now.