fix(profile)(security): keep identity / seed material out of OrbitDB#385
Open
vrogojin wants to merge 2 commits into
Open
fix(profile)(security): keep identity / seed material out of OrbitDB#385vrogojin wants to merge 2 commits into
vrogojin wants to merge 2 commits into
Conversation
Audit #333 C1 was closed at the encrypt() boundary: when the OrbitDB write path was reachable but no encryption key had been derived, encrypt() threw and the write was rejected. That defense holds only during Phase A. After setIdentity attaches the encryption key, any subsequent write of `mnemonic` / `master_key` / `chain_code` / `derivation_path` / `base_path` / `derivation_mode` / `wallet_source` / `current_address_index` would have been encrypted and written to OrbitDB → replicated to IPFS via the snapshot CAR pin path. Even encrypted, distributing the seed lowers the threat model from "attacker must compromise the device" to "attacker must brute-force a password against an IPFS-pinned ciphertext" — strictly weaker than what users expect of seed material. The only legitimate cross-device transport for the seed is the user's own BIP-39 mnemonic backup. Fix --- - profile/types.ts: - New `IDENTITY_KEYS` set names the identity / seed-material legacy keys explicitly. - `CACHE_ONLY_KEYS` is widened to fold in `IDENTITY_KEYS`. After `translateKey()`, identity writes return `cacheOnly: true`; the `set()` flow short-circuits at the localCache step and never reaches `writeEnvelope()`. - profile/profile-storage-provider.ts: - Defense-in-depth: a post-translation assertion in `set()` fail-closes if any future refactor adds an identity-shaped Profile key (`identity.*`) without putting its legacy alias into `CACHE_ONLY_KEYS`. The error message points at the right file to fix. The old "encrypt() is the catch-all" comment block is replaced with one that documents the two-layer defense (cache- only routing + post-translation assertion). - profile/migration.ts: - Identity keys are diverted at read time into a separate `identityKeys` map and persisted at the LEGACY key name (so the cache-only routing kicks in). This keeps the legacy-import path working — Sphere.loadIdentityFromStorage reads `_storage.get('master_key')` against the Profile localCache and finds it under the same legacy name it always lived at — without routing any encrypted seed bytes through the OrbitDB OpLog. - `MigrationResult.keysMigrated` now counts identity keys too for parity with previous behaviour. - profile/index.ts: export the new `IDENTITY_KEYS` constant. Tests ----- - New test file `profile-storage-provider-identity-keys-cache-only.test.ts`: - Schema-level contract: every key in IDENTITY_KEYS is also in CACHE_ONLY_KEYS, and every `identity.*` mapping in PROFILE_KEY_MAPPING has its legacy alias in IDENTITY_KEYS. This catches future refactors at test time, not at runtime. - Parametrised assertion that `set('<identity-key>', ...)` writes to localCache and leaves OrbitDB empty. - Defense-in-depth assertion fires on a synthetic `identity.*` profile key with no CACHE_ONLY_KEYS entry. - Control: a non-identity write still flows through writeEnvelope. - `profile-storage-provider-c1-plaintext-seed.test.ts`: updated tests that asserted the OLD behaviour ("after setIdentity, mnemonic lands ENCRYPTED in OrbitDB"). The defense moved from "encrypt() is the boundary" to "cache-only is the boundary"; assertions now verify the seed never lands in OrbitDB at any point. - `profile-storage-provider.test.ts`, `integration.test.ts`, `migration.test.ts`, `migration-c2-flush-before-cleanup.test.ts`, `profile-storage-provider-issue-311.test.ts`: tests that used `'mnemonic'` as a generic test fixture for the OrbitDB write/read path swap to `'wallet_exists'` (non-identity, non-cache-only). Migration assertions verify the legacy key landed in Profile localCache (not under `identity.*`). Behaviour change consumers should know about -------------------------------------------- - Cross-device "wallet exists by virtue of identity.* being replicated" no longer works — Sphere.exists() on a fresh device returns false until the user imports the mnemonic, which is the correct UX for a non-replicating seed. The `has('wallet_exists')` fallback in ProfileStorageProvider still scans for legacy `identity.*` keys, so wallets created BEFORE this PR (which still have those entries in OrbitDB) keep working unchanged. - Identity material in legacy IndexedDB is migrated to the Profile localCache (IndexedDB, same device). The user-facing experience is unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
3 tasks
…o primary Symptom (2026-06-02): on a wallet that predates the IDENTITY_KEYS ⊂ CACHE_ONLY_KEYS fix, the seed material lives only in the legacy IndexedDB (wired as `fallbackStorage`). Every boot reads the primary (Profile localCache), finds nothing, falls back to legacy, succeeds, and emits one warning per identity key per boot: [Sphere] Identity read for "master_key" missing from primary storage; consulting fallbackStorage (legacy cached identity). The wallet works correctly but the warning noise is permanent — the fallback consult never goes away because nothing ever populates the primary. Fix --- In `Sphere.loadIdentityFromStorage`'s `readIdentityKey` helper, when the fallback read succeeds, also write the value back to the primary (`storage.set`). With the IDENTITY_KEYS ⊂ CACHE_ONLY_KEYS routing in the prior commit, the write lands in the Profile localCache only — it never reaches OrbitDB → never replicates to IPFS. So the backfill silences the per-boot warning for legacy wallets WITHOUT re-introducing the OrbitDB seed-leak the cache-only routing closes. The backfill is best-effort: if the primary `set` throws (e.g., quota / contention), the read has already succeeded — we must not regress the load just because the backfill couldn't run. Failure is logged at `debug`. Tests ----- New file `tests/unit/core/Sphere.identity-fallback-backfill.test.ts`: - backfill happens for every identity key the primary lacks; - backfill failure is swallowed (caller sees the original load-path error, not "quota exceeded"); - no backfill or fallback consult on the steady-state post-backfill boot (primary already holds the keys). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
vrogojin
added a commit
to unicity-sphere/sphere
that referenced
this pull request
Jun 1, 2026
Adds commit 96991eb on PR #385: fix(profile)(security): lazy-backfill identity keys from fallback into primary Without this, post-deploy existing wallets continue to emit "[Sphere] Identity read for ... missing from primary storage; consulting fallbackStorage" on every boot — even though the wallet works correctly. The backfill silences the warning for legacy wallets by writing the fallback value into the Profile localCache on first successful fallback consult; subsequent boots find it in primary on the first try. Source: sphere-sdk fix/profile-identity-keys-cache-only @ 96991eb PR: unicity-sphere/sphere-sdk#385 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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
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
Audit #333 C1 was closed at the
encrypt()boundary: when the OrbitDB write path was reachable but no encryption key had been derived (Phase A),encrypt()threw and the write was rejected. That defense only holds beforesetIdentity. After identity is attached, any subsequent write ofmnemonic/master_key/chain_code/derivation_path/base_path/derivation_mode/wallet_source/current_address_indexwould have been encrypted and written to OrbitDB → replicated to IPFS via the snapshot CAR pin path.Even encrypted, distributing the seed lowers the threat model from "attacker must compromise the device" to "attacker must brute-force a password against an IPFS-pinned ciphertext" — strictly weaker than what users expect of seed material. The only legitimate cross-device transport for the seed is the user's own BIP-39 mnemonic backup.
This was visible in the browser console at https://sphere-telco-test.dyndns.org as
— the symptom was the fallback consult, but the root concern the user flagged was that the architecture had Profile-mode writes routing identity keys to OrbitDB at all.
Fix
Two-layer defense:
Layer 1 — Cache-only routing (
profile/types.ts)IDENTITY_KEYSset names the identity / seed-material legacy keys explicitly.CACHE_ONLY_KEYSis widened to fold inIDENTITY_KEYS. AftertranslateKey(), identity writes returncacheOnly: true; theset()flow short-circuits at the localCache step and never reacheswriteEnvelope().Layer 2 — Defense-in-depth assertion (
profile/profile-storage-provider.ts)set()fail-closes if any future refactor adds an identity-shaped Profile key (identity.*) without putting its legacy alias intoCACHE_ONLY_KEYS. The error message points at the right file to fix.Migration update (
profile/migration.ts)identityKeysmap and persisted at the LEGACY key name (so the cache-only routing kicks in). This keeps the legacy-import path working —Sphere.loadIdentityFromStoragereads_storage.get('master_key')against the Profile localCache and finds it under the same legacy name it always lived at — without routing any encrypted seed bytes through the OrbitDB OpLog.Tests
New —
profile-storage-provider-identity-keys-cache-only.test.ts(13 tests):identity.*mapping in PROFILE_KEY_MAPPING has its legacy alias in IDENTITY_KEYS. This catches future refactors at test time, not at runtime.set('<identity-key>', ...)writes to localCache and leaves OrbitDB empty for every key in IDENTITY_KEYS.identity.*profile key with no CACHE_ONLY_KEYS entry.Updated —
profile-storage-provider-c1-plaintext-seed.test.ts: assertions that asserted the OLD behaviour ("after setIdentity, mnemonic lands ENCRYPTED in OrbitDB") were captured the older still-leaky behaviour. Now verifies the seed never lands in OrbitDB at any point.Updated —
profile-storage-provider.test.ts,integration.test.ts,migration.test.ts,migration-c2-flush-before-cleanup.test.ts,profile-storage-provider-issue-311.test.ts: tests that used'mnemonic'as a generic test fixture for the OrbitDB write/read path now swap to'wallet_exists'(non-identity, non-cache-only). Migration assertions verify the legacy key landed in Profile localCache, NOT underidentity.*.Full suite: 8889 passing (1 unrelated flake in
AccountingModule.invoiceDelivery.test.ts:762that passes in isolation; same flake as observed onmain).Behaviour change consumers should know about
identity.*being replicated" no longer works —Sphere.exists()on a fresh device returns false until the user imports the mnemonic, which is the correct UX for a non-replicating seed. Thehas('wallet_exists')fallback inProfileStorageProviderstill scans for legacyidentity.*keys, so wallets created BEFORE this PR (which still have those entries in OrbitDB) keep working unchanged.Test plan
npx vitest run tests/unit/profile/— all profile tests pass.AccountingModule.invoiceDelivery.test.ts, unrelated).npx tsc --noEmit— clean.npx tsup— clean.[Sphere] Identity read for "master_key" missing from primary storagewarnings stop appearing for new Profile-mode wallets. (Existing legacy-migrated wallets will still consult fallback — that's correct.)🤖 Generated with Claude Code