feat(example-apps): harden WIF login against ambiguous identity resolution#109
Merged
Conversation
…solution Treat a WIF as a credential rather than an identity identifier. When a key resolves to multiple identities, refuse to list or guess among them: fall back to paginated `byNonUniquePublicKeyHash`, and require the user to supply the exact intended identity ID, which is fetched directly and verified. Match ECDSA keys by full public key or ECDSA HASH160 (hex or base64) while rejecting other HASH160 key types. Persist the identity ID and DPNS label only when the remember checkbox is enabled.
Bring TokenOps to parity with Dashnote's WIF sign-in resolver: paginate `byNonUniquePublicKeyHash`, match only full-ECDSA and ECDSA HASH160 key types, and require an explicit identity ID when a key maps to multiple identities — fetching it directly and verifying rather than selecting by result order. The resolver, secret-shape detector, key-manager adapter, and their core test suite are now byte-identical across both apps, enforced by scripts/check-shared-auth-parity.sh and wired into both CI workflows.
Pin prettier to an exact 3.8.4 in dashnote and token-ops so formatting is deterministic across contributors and CI.
…-ops Deep-link each app's GitHub footer to its own example-apps subdirectory instead of the repo root, and add the missing token-ops entry to the example-apps README (with section emoji).
Link the root README to the example-apps directory so the standalone React app projects are discoverable from the top level.
📝 WalkthroughWalkthroughDashnote and TokenOps now support explicit identity selection for ambiguous WIF credentials, stricter key matching, shared authentication parity checks, updated tests, CI workflows, and documentation. ChangesWIF authentication and shared parity
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
example-apps/dashmint-lab/src/components/AppShell.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. example-apps/dashnote/package.jsonESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. example-apps/dashnote/src/components/AppShell.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 (1)
example-apps/dashnote/src/hooks/useWifPreview.ts (1)
161-163: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
expectedIdentityIdin the cache key to prevent stale preview states.The
resolvedcache is currently keyed only bywif. When the user types anexpectedIdentityId, the hook incorrectly returns the staleresolved.state(e.g.,"ambiguous") during the 400ms debounce instead of yielding"checking". This hides the "Checking…" UI indicator and causes visual confusion.Include
expectedIdentityIdin the cache state and compare it before returning the cached result.🐛 Proposed fix
Update the
useStatedefinition (around line 75):const [resolved, setResolved] = useState<{ wif: string; + expectedIdentityId: string | undefined; state: WifPreviewState; } | null>(null);Update the
setResolvedcall (around line 152):if (cancelled) return; - setResolved({ wif: trimmed, state: next }); + setResolved({ wif: trimmed, expectedIdentityId, state: next }); }, DEBOUNCE_MS);Update the cache validation check:
if (!gateOk) return IDLE; - if (resolved && resolved.wif === trimmed) return resolved.state; + if (resolved && resolved.wif === trimmed && resolved.expectedIdentityId === expectedIdentityId) return resolved.state; return CHECKING;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/hooks/useWifPreview.ts` around lines 161 - 163, Update the resolved cache state in useWifPreview to store expectedIdentityId alongside wif and state, populate it in the setResolved call, and require both the trimmed WIF and expectedIdentityId to match before returning resolved.state; otherwise return CHECKING during debounce.
🧹 Nitpick comments (3)
example-apps/dashnote/src/dash/loginWithPrivateKey.ts (2)
391-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew
tryDecodeHexduplicates the inline hex-decode loop intryDecodeKeyData(lines 105-115).Both implement the identical fixed-width hex-to-bytes loop. Since
extractPubKeyByteswas already updated (line 430) to call the new sharedtryDecodeHex, consider also refactoringtryDecodeKeyData's hex branch to call it, eliminating the last copy of this logic.♻️ Proposed fix
function tryDecodeKeyData(data: string): Uint8Array | null { if (typeof data !== "string" || data.length === 0) return null; if (/^[0-9a-fA-F]+$/.test(data) && data.length % 2 === 0) { - try { - const bytes = new Uint8Array(data.length / 2); - for (let i = 0; i < bytes.length; i += 1) { - bytes[i] = parseInt(data.slice(i * 2, i * 2 + 2), 16); - } - return bytes; - } catch { - // fall through to base64 - } + const decoded = tryDecodeHex(data); + if (decoded) return decoded; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts` around lines 391 - 399, Update the hex branch of tryDecodeKeyData to reuse the shared tryDecodeHex helper instead of maintaining its inline hex-to-bytes loop. Preserve the existing invalid-input and decoded-byte behavior while removing the duplicated parsing logic.
333-340: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
matchIdentityKeyduplicates theidentityIdOfhelper.Lines 338-339 re-implement the same
typeof identity.id === "string" ? identity.id : identity.id.toString()logic thatidentityIdOf(defined later in this file) already encapsulates. CallidentityIdOf(identity)here instead to avoid two independent implementations of identity-ID stringification drifting apart.♻️ Proposed fix
function matchIdentityKey( identity: IdentityLike, ourPubKeyBytes: Uint8Array | null, ourPubKeyHashBytes: Uint8Array | null, ): MatchedIdentity | null { - const identityId = - typeof identity.id === "string" ? identity.id : identity.id.toString(); + const identityId = identityIdOf(identity); const publicKeys = identity.toJSON?.().publicKeys ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts` around lines 333 - 340, Update matchIdentityKey to obtain identityId by calling the existing identityIdOf(identity) helper instead of duplicating the identity.id stringification logic; leave the surrounding public-key matching behavior unchanged.example-apps/dashnote/src/components/LoginModal.tsx (1)
202-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse Tailwind v4 CSS variable syntax.
With Tailwind CSS v4, you can reference CSS variables directly using parentheses instead of the verbose arbitrary value syntax. For example,
text-[color:var(--color-danger)]can be simplified totext-(--color-danger).♻️ Proposed fix
{wifPreview.status === "ambiguous" && ( - <span className="text-[color:var(--color-danger)]"> + <span className="text-(--color-danger)"> This key is associated with multiple identities. Enter the full identity ID you intend to use. </span> )} {wifPreview.status === "identity-mismatch" && ( - <span className="text-[color:var(--color-danger)]"> + <span className="text-(--color-danger)"> This key could not be verified as an eligible authentication key for that identity ID. </span> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/components/LoginModal.tsx` around lines 202 - 213, Update the danger text classes on the ambiguous and identity-mismatch status messages in LoginModal to use Tailwind v4’s parenthesized CSS variable syntax, replacing the verbose arbitrary color syntax while preserving the existing styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example-apps/dashnote/src/components/LoginModal.tsx`:
- Around line 51-58: Update the needsIdentityId logic in LoginModal to latch the
preview’s ambiguousWif state, preserving the Identity ID input while the value
is cleared and the preview transitions through stale or checking states. Use the
latched ambiguousWif state alongside hasIdentityPrompt when determining whether
the input remains mounted.
In `@example-apps/token-ops/src/dash/loginWithPrivateKey.ts`:
- Around line 94-101: Correct the decode documentation above the public-key
data-field helper to state that hex is attempted first and base64 is used as the
fallback, matching the existing branch order. Apply the identical comment-only
change in the corresponding Dashnote shared-auth implementation to preserve
byte-identical parity.
---
Outside diff comments:
In `@example-apps/dashnote/src/hooks/useWifPreview.ts`:
- Around line 161-163: Update the resolved cache state in useWifPreview to store
expectedIdentityId alongside wif and state, populate it in the setResolved call,
and require both the trimmed WIF and expectedIdentityId to match before
returning resolved.state; otherwise return CHECKING during debounce.
---
Nitpick comments:
In `@example-apps/dashnote/src/components/LoginModal.tsx`:
- Around line 202-213: Update the danger text classes on the ambiguous and
identity-mismatch status messages in LoginModal to use Tailwind v4’s
parenthesized CSS variable syntax, replacing the verbose arbitrary color syntax
while preserving the existing styling.
In `@example-apps/dashnote/src/dash/loginWithPrivateKey.ts`:
- Around line 391-399: Update the hex branch of tryDecodeKeyData to reuse the
shared tryDecodeHex helper instead of maintaining its inline hex-to-bytes loop.
Preserve the existing invalid-input and decoded-byte behavior while removing the
duplicated parsing logic.
- Around line 333-340: Update matchIdentityKey to obtain identityId by calling
the existing identityIdOf(identity) helper instead of duplicating the
identity.id stringification logic; leave the surrounding public-key matching
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 451f5c08-d3c1-42bf-9a35-b3fa88566563
⛔ Files ignored due to path filters (2)
example-apps/dashnote/package-lock.jsonis excluded by!**/package-lock.jsonexample-apps/token-ops/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
.github/workflows/dashnote-e2e.yml.github/workflows/token-ops-ci.ymlREADME.mdexample-apps/README.mdexample-apps/dashmint-lab/src/components/AppShell.tsxexample-apps/dashnote/CLAUDE.mdexample-apps/dashnote/README.mdexample-apps/dashnote/package.jsonexample-apps/dashnote/src/components/AppShell.tsxexample-apps/dashnote/src/components/LoginModal.tsxexample-apps/dashnote/src/dash/loginWithPrivateKey.tsexample-apps/dashnote/src/dash/types.tsexample-apps/dashnote/src/hooks/useWifPreview.tsexample-apps/dashnote/src/session/SessionContext.tsxexample-apps/dashnote/test/LoginModal.test.tsxexample-apps/dashnote/test/SessionContext.test.tsxexample-apps/dashnote/test/loginWithPrivateKey.test.tsexample-apps/dashnote/test/useWifPreview.test.tsxexample-apps/dashproof-lab/src/components/AppShell.tsxexample-apps/dashrate/src/App.tsxexample-apps/token-ops/CLAUDE.mdexample-apps/token-ops/README.mdexample-apps/token-ops/package.jsonexample-apps/token-ops/src/components/LoginModal.tsxexample-apps/token-ops/src/dash/loginWithPrivateKey.tsexample-apps/token-ops/src/dash/types.tsexample-apps/token-ops/src/lib/detectSecretShape.tsexample-apps/token-ops/src/session/SessionContext.tsxexample-apps/token-ops/src/session/keyManagerFromKey.tsexample-apps/token-ops/test/LoginModal.test.tsxexample-apps/token-ops/test/SessionContext.test.tsxexample-apps/token-ops/test/loginWithPrivateKey.test.tsscripts/check-shared-auth-parity.sh
…tions The identity-ID prompt was tied to the live preview status, so it flickered away when a follow-up preview cycled through checking/idle. Latch the ambiguous result in an effect so the prompt persists once a key is known to map to multiple identities. Also correct the tryDecodeKeyData comment in the shared resolver to describe the actual hex-first, base64-fallback order (kept byte-identical across both apps).
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
1 participant
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
Treats a pasted WIF as a credential, not an identity identifier. When a private key maps to more than one identity, the login flow no longer guesses among them or exposes the candidate list — it requires the user to supply the exact identity ID, fetches it directly, and verifies the key against it.
This hardening lands in both Dashnote and TokenOps, and the shared auth code is now byte-identical across the two apps, enforced by CI.
What changed
Resolver (
loginWithPrivateKey.ts)identities.fetch(id)and verify the key against it — never select by result order.byNonUniquePublicKeyHashwhen the uniquebyPublicKeyHashlookup misses; throwAmbiguousIdentityErroras soon as a second match is found. Pagination guards against a non-advancing cursor.type 0) or ECDSA HASH160 (type 2), in hex or base64; reject other 20-byte key types (BIP13 script hashes, EDDSA HASH160).UI
useWifPreviewsurfacesambiguous/identity-mismatchstates before submit; prompts for an identity ID when needed.Shared-code parity
loginWithPrivateKey.ts,detectSecretShape.ts,keyManagerFromKey.ts, and the resolver test suite are byte-identical across Dashnote and TokenOps.scripts/check-shared-auth-parity.shenforces this and is wired into both CI workflows, with cross-app path triggers.Housekeeping
prettierto exact3.8.4in both apps for deterministic formatting.example-apps/subdirectory; add the missing token-ops entry to the example-apps README.Testing
npm test— 346 passing.npm test— 215 passing.tsc -b).scripts/check-shared-auth-parity.shpasses.Notes
looksLikeWifin TokenOps'detectSecretShape.tsis unused there — it exists only to keep the file byte-identical with Dashnote for the parity check.dash/contract.ts,dash/governance.ts, and untouched regions ofdash/types.ts/session/SessionContext.tsx). TokenOps CI does not gate onformat:check, so this is non-blocking; a follow-upnpm run formatwould reset the baseline.Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests