feat(12.3.1): Pre-wipe identity cleanup#127
Conversation
Phase 12.3.1: Pre-Wipe Identity Cleanup - 3 plans in 2 waves - Wave 1: crypto vault IPNS derivation (01) + backend identity hashing (02) in parallel - Wave 2: backend vault schema + frontend vault init (03) - Ready for execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Plan 02: Hash googlePayload.sub (immutable) not email for Google auth - Plan 02: Split Task 1 into schema+hashing tasks (was 6 files in one task) - Plan 03: Remove ipns.service.ts from files_modified (no rootIpnsPublicKey refs) - Plan 03: Add cross-reference to Plan 01 test case 8 for recovery path verification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Plan 01: Add EncryptedVaultKeys, encryptVaultKeys, decryptVaultKeys updates + vault.test.ts - Plan 03: Fix SC-5 wording to passive compatibility, add grep verification - Plan 04: New plan for desktop Rust, E2E helpers, controller spec cleanup - ROADMAP: Update plan count to 4, fix SC-5 wording Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add hashIdentifier(value) to SiweService for non-wallet identifiers - Widen identifier_display from varchar(15) to varchar(255) for emails - Add composite index on (type, identifier_hash) for efficient lookups - Update entity and migration column comments for all auth method types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ltKeys cleanup - Create deriveVaultIpnsKeypair() using HKDF with info "cipherbox-vault-ipns-v1" - Update initializeVault() to require userPrivateKey and derive IPNS deterministically - Remove rootIpnsPublicKey from EncryptedVaultKeys type - Update encryptVaultKeys() to return only encrypted fields - Update decryptVaultKeys() to derive public key from private key via ed25519 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add 8 new tests: determinism, different keys, domain separation, error handling, IPNS format - Add 3 tests for initializeVault: deterministic IPNS, random folder key, keypair consistency - Update vault.test.ts: initializeVault calls pass privateKey - Remove rootIpnsPublicKey assertions (field removed from EncryptedVaultKeys) - Update uniqueness test to use different private keys (IPNS is now deterministic) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…to-linking - Google login hashes googlePayload.sub (immutable Google user ID) - Email login hashes normalized email for identifier lookup - Remove cross-method email auto-linking block entirely - Refactor findOrCreateUserByEmail to findOrCreateUserByIdentifier - Update auth.service login/linkJwtMethod/testLogin to use identifierHash - Return identifierDisplay (human-readable) from getLinkedMethods and refreshByToken Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - deriveVaultIpnsKeypair + updated vault init/encrypt/decrypt - Unit tests for vault IPNS derivation + updated vault tests SUMMARY: .planning/phases/12.3.1-pre-wipe-identity-cleanup/ 12.3.1-01-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…auth methods - Add hashIdentifier tests to siwe.service.spec (consistent hash, no normalization) - Update identity controller tests: Google hashes sub, email hashes normalized email - Add test: Google + email same email creates SEPARATE users (no auto-linking) - Update auth.service tests: identifierHash lookups, identifierDisplay in responses - Update linkJwtMethod tests: hash-based collision and duplicate detection - Update testLogin tests: hash-based lookup and creation with identifierHash - Update refreshByToken test: returns identifierDisplay not hashed identifier - Regenerate API client - All 460 tests pass across 24 suites Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 3/3 - Schema, entity, migration, and SiweService hash utility - Hash all auth identifiers and remove cross-method auto-linking - Update tests for hashed identifiers and independent auth methods SUMMARY: .planning/phases/12.3.1-pre-wipe-identity-cleanup/12.3.1-02-SUMMARY.md
… and DTOs - Remove rootIpnsPublicKey column from Vault entity (derivable from privateKey via HKDF) - Remove rootIpnsPublicKey from InitVaultDto and VaultResponseDto - Update VaultService.initializeVault and toVaultResponse mappings - Remove root_ipns_public_key from FullSchema migration - Update vault.service.spec.ts test fixtures and assertions - Remove rootIpnsPublicKey from desktop Rust types, commands, and state - Regenerate API client (openapi.json, orval models) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lt flow - Update initializeOrLoadVault to pass privateKey to initializeVault() - Remove rootIpnsPublicKey from decryptVaultKeys call (derived internally) - Remove rootIpnsPublicKey from vaultApi.initVault DTO - Remove rootIpnsPublicKey from VaultResponse and InitVaultDto types - Update vault.controller.spec.ts fixtures (remove rootIpnsPublicKey) - Update e2e web3auth-helpers (deterministic initializeVault, no rootIpnsPublicKey) - TEE compatibility verified: ipns.service.ts has no rootIpnsPublicKey references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 (both already committed by Plan 03) - Task 1: Remove rootIpnsPublicKey from desktop Rust app (07caf1b) - Task 2: Update E2E helpers and vault controller spec (da2bc83) Phase 12.3.1 complete (4/4 plans). SUMMARY: .planning/phases/12.3.1-pre-wipe-identity-cleanup/12.3.1-04-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove rootIpnsPublicKey column from Vault entity - Remove rootIpnsPublicKey from InitVaultDto and VaultResponseDto - Remove rootIpnsPublicKey from VaultService create/response mappings - Remove root_ipns_public_key from FullSchema migration - Update vault.service.spec.ts test fixtures - Regenerate openapi.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tasks completed: 2/2 - Task 1: Update backend vault schema, service, and DTOs - Task 2: Update frontend vault init, API client, and verify TEE compatibility SUMMARY: .planning/phases/12.3.1-pre-wipe-identity-cleanup/12.3.1-03-SUMMARY.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughPhase 12.3.1 (Pre-Wipe Identity Cleanup) marked complete (2026-02-14). Changes add deterministic IPNS keypair derivation from user private keys, switch auth identifiers to SHA‑256 per auth method (removing cross-method email auto‑linking), and remove the stored Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Web as "Web App"
participant Crypto as "@cipherbox/crypto"
participant API as "API Server"
participant DB as "Database"
User->>Web: Create account / Init vault
Web->>Crypto: initializeVault(userPrivateKey)
Crypto->>Crypto: deriveVaultIpnsKeypair(userPrivateKey)
Note over Crypto: HKDF-SHA256(salt="CipherBox-v1", info="cipherbox-vault-ipns-v1")<br/>→ Ed25519 seed → publicKey → IPNS name
Crypto->>Crypto: generate random rootFolderKey
Crypto-->>Web: VaultInit { rootIpnsKeypair, rootFolderKey, encryptedRootIpnsPrivateKey }
Web->>Crypto: encryptVaultKeys(vaultInit)
Crypto-->>Web: EncryptedVaultKeys { encryptedRootFolderKey, encryptedIpnsPrivateKey }
Web->>API: POST /vault (InitVaultDto without rootIpnsPublicKey)
API->>DB: Store vault (no root_ipns_public_key)
API-->>Web: VaultResponseDto { rootIpnsName, encryptedRootIpnsPrivateKey }
Web->>API: GET /vault
API->>DB: Fetch vault
API-->>Web: VaultResponseDto
Web->>Crypto: decryptVaultKeys(encryptedKeys)
Crypto->>Crypto: decrypt IPNS private key
Crypto->>Crypto: derive public key from private key
Crypto-->>Web: rootIpnsKeypair { publicKey, privateKey }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/auth/controllers/identity.controller.ts (1)
302-306:⚠️ Potential issue | 🟡 MinorStale JSDoc reference to removed method.
Line 305 says
"Same placeholder publicKey pattern as findOrCreateUserByEmail"but that method was renamed tofindOrCreateUserByIdentifier.Proposed fix
- * Same placeholder publicKey pattern as findOrCreateUserByEmail. + * Same placeholder publicKey pattern as findOrCreateUserByIdentifier.apps/api/src/auth/auth.service.ts (1)
80-105:⚠️ Potential issue | 🟠 MajorIdentifier hash in
login()is inconsistent with the identity controller's hash for Google users.The identity controller hashes
googlePayload.sub(the immutable Google user ID) for Google auth methods (line 97 ofidentity.controller.ts). However, here at line 83–84,identifierresolves topayload.email(since it's checked first), and the hash is computed from the email — not the Google sub. This means the primary lookup at line 85–86 will never match for Google users.The fallback at lines 90–92 (find any auth method for the user) masks this, but:
- It may update
lastUsedAton the wrong auth method if the user has multiple linked methods.- The safety-net type inference at line 97 doesn't account for
'google'(only'wallet'or'email').Consider aligning the hash computation with the identity controller by checking for a Google-specific indicator, or restructuring the lookup to use
userIdalone since the user is already resolved by publicKey.
🤖 Fix all issues with AI agents
In `@apps/api/src/auth/controllers/identity.controller.ts`:
- Around line 155-167: The JWT is being signed with the raw dto.email while the
identifier hash was computed from normalizedEmail, causing lookup mismatches;
update the call to jwtIssuerService.signIdentityJwt in identity.controller.ts to
pass the normalizedEmail (the same value used to produce identifierHash) instead
of dto.email so the payload.email used later will match the hashed identifier;
verify jwtIssuerService.signIdentityJwt accepts the normalized string (adjust
its parameter handling if necessary) and keep identifierHash computed from
normalizedEmail unchanged.
In `@tests/e2e/utils/web3auth-helpers.ts`:
- Line 10: The E2E tests import ed from '@noble/ed25519' (see import * as ed in
web3auth-helpers.ts) but that package is not declared in the tests/e2e
package.json, causing test-time resolution failures; add "@noble/ed25519":
"^2.2.3" (or compatible version) to the tests/e2e package.json devDependencies
so the test package can resolve the ed import at runtime and re-run the E2E
suite.
🧹 Nitpick comments (4)
packages/crypto/src/__tests__/vault-ipns.test.ts (1)
57-69: Minor: Consolidate the two-call error assertion into a single check.The try/catch block (lines 64–69) has no guard against the function unexpectedly resolving — if
rejects.toThrowon line 60 were removed, the test could silently pass. You can merge both assertions into one usingrejects.toMatchObjector a single try/catch with a fail sentinel.♻️ Suggested consolidation
it('throws CryptoError with INVALID_KEY_SIZE for 31-byte key', async () => { const shortKey = new Uint8Array(31); - await expect(deriveVaultIpnsKeypair(shortKey)).rejects.toThrow( - 'Invalid private key size for vault derivation' - ); - - try { - await deriveVaultIpnsKeypair(shortKey); - } catch (error) { - expect(error).toBeInstanceOf(CryptoError); - expect((error as CryptoError).code).toBe('INVALID_KEY_SIZE'); - } + await expect(deriveVaultIpnsKeypair(shortKey)).rejects.toSatisfy((error: unknown) => { + expect(error).toBeInstanceOf(CryptoError); + expect((error as CryptoError).code).toBe('INVALID_KEY_SIZE'); + expect((error as CryptoError).message).toContain('Invalid private key size for vault derivation'); + return true; + }); });packages/crypto/src/__tests__/vault.test.ts (1)
48-64: Nit: test title could be clearer."should produce unique folder keys but deterministic IPNS keys for different private keys" could be read as "same IPNS keys for different inputs." Consider something like "should produce unique keys per user: random folder keys and distinct deterministic IPNS keys."
apps/api/src/auth/controllers/identity.controller.ts (1)
260-300: TOCTOU race infindOrCreateUserByIdentifier— concurrent first-time logins can create duplicate users.The check at line 266 and the create at lines 282–297 are not wrapped in a transaction or protected by a unique constraint on
(type, identifier_hash). TheIDX_auth_methods_type_hashindex (per the migration plan) is a regular index, not aUNIQUEconstraint. Two concurrent first-login requests for the same identifier could both pass thefindOnecheck and create two separate users.Consider either:
- Making
IDX_auth_methods_type_hashaUNIQUEindex and catching the constraint violation, or- Wrapping the check-and-create in a serializable transaction.
apps/api/src/auth/auth.service.ts (1)
288-333:linkJwtMethodhashing is consistent and well-structured.Cross-account collision check, duplicate check, and creation all use
identifierHash. However, note the same DRY concern: this usescreateHash('sha256')directly (line 291) whileidentity.controller.tsusessiweService.hashIdentifier(). Both produce the same SHA-256 hex output, but centralizing onsiweService.hashIdentifier()would reduce the risk of divergence.
- Use normalizedEmail (not raw dto.email) in signIdentityJwt call - Add @noble/ed25519 as direct devDependency in tests/e2e - Add tests for missing SIWE nonce and duplicate wallet link branches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/controllers/identity.controller.ts (1)
302-306:⚠️ Potential issue | 🟡 MinorStale JSDoc reference.
The comment says "Same placeholder publicKey pattern as
findOrCreateUserByEmail" but that method was renamed tofindOrCreateUserByIdentifier.Proposed fix
/** * Find an existing user by wallet address hash, or create a new user. * - * Same placeholder publicKey pattern as findOrCreateUserByEmail. + * Same placeholder publicKey pattern as findOrCreateUserByIdentifier. */
🧹 Nitpick comments (2)
apps/api/src/auth/controllers/identity.controller.ts (2)
96-107: Google email used asidentifierDisplayis not normalized, unlike the email-OTP path.Line 156 normalizes email for the OTP flow (
toLowerCase().trim()), but the Google flow passesgooglePayload.emaildirectly asidentifierDisplay(Line 102) and into the JWT (Line 107). While the hash is correctly derived fromsub(not email), the display value could carry inconsistent casing across flows. Consider normalizing for consistency:Proposed fix
// 2. Hash Google sub (immutable user ID) -- NOT email (which can change) const identifierHash = this.siweService.hashIdentifier(googlePayload.sub); + const normalizedEmail = googlePayload.email?.toLowerCase().trim() ?? googlePayload.email; // 3. Find or create user by hashed identifier const { user, isNewUser } = await this.findOrCreateUserByIdentifier( identifierHash, - googlePayload.email, + normalizedEmail, 'google' ); // 4. Sign CipherBox identity JWT (include email for auth method identifier) - const idToken = await this.jwtIssuerService.signIdentityJwt(user.id, googlePayload.email); + const idToken = await this.jwtIssuerService.signIdentityJwt(user.id, normalizedEmail);
307-346:findOrCreateUserByWalletduplicates most offindOrCreateUserByIdentifier.The only differences are the hardcoded
'wallet'type and thetruncateWalletAddresscall for display. Consider whether this could delegate tofindOrCreateUserByIdentifierwith the truncated address asidentifierDisplay, removing ~30 lines of duplication.Sketch
private async findOrCreateUserByWallet( addressHash: string, walletAddress: string ): Promise<{ user: User; isNewUser: boolean }> { - // ... duplicated lookup + create logic ... + const truncated = this.siweService.truncateWalletAddress(walletAddress); + return this.findOrCreateUserByIdentifier(addressHash, truncated, 'wallet'); }This would require widening the
authMethodTypeunion to include'wallet'.
…ation - Normalize googlePayload.email for identifierDisplay and JWT signing - Widen findOrCreateUserByIdentifier to accept 'wallet' type - Remove duplicate findOrCreateUserByWallet method (~40 lines) - Fix stale JSDoc reference to findOrCreateUserByEmail Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add SiweService.truncateEmail() — first 3 + "..." + last 2 of local part + full domain (e.g. "michael@gmail.com" → "mic...el@gmail.com"). Short local parts (≤5 chars) shown in full. Applied to all email identifierDisplay storage paths: Google login, email OTP login, link method, test login, and safety net fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/auth.service.spec.ts (1)
324-362:⚠️ Potential issue | 🟠 Major
truncateWalletAddressmock is not configured — test assertion likely fails.In this test, the safety-net code path in
auth.service.tscallsthis.siweService.truncateWalletAddress(identifier)for wallet-type identifiers (Line 100). However,truncateWalletAddressis initialized as a barejest.fn()(Line 71) with no return value, so it returnsundefined. The assertion on Line 359 expectsidentifierDisplay: walletAddr, which won't match.Proposed fix — mock `truncateWalletAddress` for this test
userRepository.findOne.mockResolvedValue(mockUser); + siweService.truncateWalletAddress.mockReturnValue('0xd8dA...6045'); authMethodRepository.findOne .mockResolvedValueOnce(null) // identifierHash lookup .mockResolvedValueOnce(null); // userId fallback const savedMethod = { id: 'am-wallet', userId: 'user-id', type: 'wallet', identifier: identifierHash, identifierHash, - identifierDisplay: walletAddr, + identifierDisplay: '0xd8dA...6045', lastUsedAt: null, };And update the assertion accordingly:
expect(authMethodRepository.save).toHaveBeenCalledWith( expect.objectContaining({ userId: 'user-id', type: 'wallet', identifier: identifierHash, identifierHash, - identifierDisplay: walletAddr, + identifierDisplay: '0xd8dA...6045', }) );#!/bin/bash # Verify that truncateWalletAddress mock has no default return in the test setup rg -n 'truncateWalletAddress' apps/api/src/auth/auth.service.spec.ts
🤖 Fix all issues with AI agents
In `@apps/api/src/auth/auth.service.ts`:
- Around line 293-295: The code uses a fixed 'unknown' fallback for identifier
which can cause collisions; in the block that reads authMethodType =
linkDto.loginType and computes identifierHash via
createHash('sha256').update(identifier).digest('hex'), replace the 'unknown'
fallback with a guard that fails fast when neither payload.email nor payload.sub
is present (e.g., throw a BadRequestException or return an error) and only
compute identifierHash when a real identifier exists; this ensures
identifierHash is always derived from a real, unique identifier rather than the
constant 'unknown'.
- Line 83: The current fallback const identifier = payload.email || payload.sub
|| 'unknown' silently produces a deterministic collision; instead, validate
payload in the same scope (where identifier is computed) and throw a clear error
when neither payload.email nor payload.sub is present so you never hash a shared
'unknown' value; update the logic around identifier (the variable named
identifier in auth.service.ts) to reject/throw (e.g., throw new Error or custom
AuthError) when both are missing and ensure callers handle that exception.
- Around line 83-87: Replace the inline createHash usage with the centralized
siweService.hashIdentifier() to ensure a single hashing strategy: instead of
computing identifierHash via createHash('sha256') for the local variable
identifier (from payload.email || payload.sub || 'unknown') in auth.service.ts,
call this.siweService.hashIdentifier(identifier) and use that result in the
authMethod lookup (authMethodRepository.findOne where: { userId: user.id,
identifierHash }). Make the same change at the other two places in this file
where createHash is used (the occurrences you noted around the sections
currently at lines ~295 and ~479) so all identifier hashing goes through
siweService.hashIdentifier.
In `@apps/api/src/auth/controllers/identity.controller.ts`:
- Around line 96-105: normalizedEmail can be undefined and will cause TypeErrors
when passed to siweService.truncateEmail and later signIdentityJwt; update the
handler to first compute a safe normalizedEmail (e.g. const normalizedEmail =
googlePayload.email ? googlePayload.email.toLowerCase().trim() : ''), or
explicitly validate and return a 400/appropriate error when email is required,
then pass that safe value into findOrCreateUserByIdentifier and signIdentityJwt;
ensure references to siweService.truncateEmail, findOrCreateUserByIdentifier,
and signIdentityJwt handle the safe fallback or are skipped when email is
absent.
- Around line 287-293: The current two-step create in identity.controller.ts
uses a constant placeholder publicKey causing unique-constraint races; change
the initial save to use a unique temporary value (e.g.,
`pending-core-kit-${randomUUID()}`) so concurrent inserts won't collide: in the
block that calls this.userRepository.save({ publicKey:
`pending-core-kit-placeholder` }), generate a UUID (or use
crypto.randomUUID()/uuid.v4()) and save `{ publicKey:
\`pending-core-kit-${uuid}\` }`, then update newUser.publicKey =
`pending-core-kit-${newUser.id}` and save as before (reference:
userRepository.save and newUser.publicKey).
🧹 Nitpick comments (3)
apps/api/src/auth/auth.service.spec.ts (1)
16-19: Helpersha256Hexis duplicated across test files.The same helper appears in both
auth.service.spec.tsandidentity.controller.spec.ts. Consider extracting it into a shared test utility to keep things DRY.apps/api/src/auth/auth.service.ts (2)
325-333:truncateEmailcalled on potential non-email identifiers in link flow.Line 331 calls
truncateEmail(identifier)whereidentifierispayload.email || payload.sub. For Google linking,payload.subcould be an opaque ID like'user-123'. WhiletruncateEmailhandles no-@inputs gracefully (returns as-is per tests), the display value would be the raw sub string rather than a user-friendly identifier. Consider using the email specifically for display when available:Proposed fix
- identifierDisplay: this.siweService.truncateEmail(identifier), + identifierDisplay: payload.email + ? this.siweService.truncateEmail(payload.email) + : identifier,
354-356: Dynamicimport('viem/siwe')in a method body — inconsistent with the static import inidentity.controller.ts.The top-level
identity.controller.tsuses a static import forparseSiweMessage. Using a dynamic import here adds a minor first-call latency and is inconsistent. Consider a static import at the top of the file.
…ralization, race condition
- Replace 'unknown' fallback with throw when JWT has no email/sub (prevents hash collisions)
- Centralize hashing through siweService.hashIdentifier() instead of inline createHash
- Use unique tempId for placeholder publicKey to prevent concurrent insert race
- Fix normalizedEmail undefined fallback for Google login (use '' not undefined)
- Use payload.email specifically for identifierDisplay in link flow
- Convert dynamic import('viem/siwe') to static import
- Fix truncateWalletAddress mock and assertion in wallet safety-net test
- Add test for missing identifier exception
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/auth/auth.service.ts (1)
99-113:⚠️ Potential issue | 🟡 MinorSafety-net type inference can never produce
'google'.Line 102 infers only
'wallet'or'email', so a Google-authenticated user hitting this safety net would gettype: 'email'. The comment says this "shouldn't happen in practice," but if it does, subsequentidentifierHash-based lookups by type would fail to match because the auth method was stored with the wrong type.Consider adding a
'google'inference branch (e.g. checking if the identifier looks like a Googlesub), or passing the type hint from the JWT payload.
🧹 Nitpick comments (2)
apps/api/src/auth/auth.service.ts (1)
333-341:identifierDisplayfalls back to rawsubfor non-email Google links.When
payload.emailis falsy (e.g., a Google account without an email claim), Line 339 setsidentifierDisplayto the rawidentifier(which ispayload.sub). A Google sub like"104578923..."isn't a useful display value. Consider usingthis.siweService.truncateEmail(identifier)only when it's actually an email, and provide a more informative fallback otherwise (e.g.,'Google account').apps/api/src/auth/controllers/identity.controller.ts (1)
53-58: Redis connection created in constructor without error handling.The Redis client is instantiated eagerly in the constructor with
lazyConnect: true, but there's no connection error listener. If Redis becomes unavailable, thegetWalletNonceand nonce-consumption calls will throw unhandled errors. Consider adding anerrorlistener to log connection failures gracefully.Suggested improvement
this.redis = new Redis({ host: configService.get('REDIS_HOST', 'localhost'), port: configService.get<number>('REDIS_PORT', 6379), password: configService.get('REDIS_PASSWORD', undefined), lazyConnect: true, }); + this.redis.on('error', (err) => { + this.logger.error(`Redis connection error: ${err.message}`); + });
1 participant
Summary
deriveVaultIpnsKeypair(privateKey)via HKDF (context:cipherbox-vault-ipns-v1) replaces random keygen — enables self-sovereign vault recovery from recovery phrase aloneSHA-256(sub)), email (SHA-256(normalized_email)), wallet (SHA-256(checksummed_address)) — privacy-preserving lookup, no plaintext identifiers storedrootIpnsPublicKeyfully removed: From crypto types, vault entity/DTO/service, desktop Rust, E2E helpers, API client — derivable from privateKey, no longer storedChanges
Crypto (
packages/crypto)deriveVaultIpnsKeypair()using HKDF-SHA256 with domain separationinitializeVault(userPrivateKey)now deterministic;EncryptedVaultKeyssimplifieddecryptVaultKeysderives public key from IPNS private keyAPI (
apps/api)hashIdentifier()in siwe.service for all auth typesidentifier_hash+identifier_displaycolumns in auth_method entityfindOrCreateUserByIdentifiersearches by(type, identifierHash)only — no cross-method lookuprootIpnsPublicKeyremoved from vault entity, DTOs, service, migrationWeb (
apps/web)useAuthcallsinitializeVault(userKeypair.privateKey)for deterministic IPNSrootIpnsPublicKeyremoved from vault API types and init flowDesktop (
apps/desktop)root_ipns_public_keycargo checkpassesE2E (
tests/e2e)ed.getPublicKeyAsync()Test plan
cargo checkpasses for desktop app🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests / Docs