Skip to content

Conversation

@mcintyre94
Copy link
Owner

  • Uses webauthn biometrics, eg touch ID
  • Optional, enable in settings
  • If enabled, locks after 30 mins and on browser startup

claude and others added 12 commits January 20, 2026 23:53
Detailed plan for adding Touch ID / Windows Hello authentication
to protect the extension UI. Includes:
- WebAuthn integration via @lo-fi/webauthn-local-client
- Lock screen UI component
- Settings route for enabling/disabling
- 30-minute inactivity timer
- Browser restart detection

Relates to #1
- Add @lo-fi/webauthn-local-client dependency for client-side WebAuthn
- Create biometricLock/schemas.ts with Zod schemas for settings and state
- Create biometricLock/storage.ts for persisting lock settings and state
- Create biometricLock/webauthn.ts wrapper for biometric registration/auth
- Add TypeScript declarations for webauthn-local-client
Simple branded lock screen with:
- Orbit logo
- "Orbit is locked" message
- Unlock button with fingerprint icon
- Error message display
- Create Settings route with biometric lock enable/disable functionality
- Add Settings menu item to Home page menu (with divider)
- Register Settings route in main.tsx router config
- Disabling lock requires biometric auth first
- Create LockGuard component that checks lock state on mount
- Wrap Layout with LockGuard to protect all routes
- Connect route (/accounts/connect) bypasses lock check
- Update activity timestamp on navigation when unlocked
- Create activity tracker that monitors user interactions
- Update last activity timestamp on click, keydown, scroll, focus
- Add browser startup handler to lock when browser restarts
- Add alarm that checks every 5 minutes for 30-minute inactivity
- Initialize activity tracking in main.tsx
Add "alarms" to manifest permissions to enable the periodic
inactivity check that locks the extension after 30 minutes.
@claude
Copy link

claude bot commented Jan 23, 2026

Code review

I found 4 issues during review:

1. Security Vulnerability: Connect Route Bypasses Lock 🔴

File: entrypoints/sidepanel/components/LockGuard.tsx (lines 86-89)
Severity: High

The LockGuard intentionally bypasses authentication for routes starting with /accounts/connect:

// Connect route bypasses lock
if (isConnectRoute) {
  return <>{children}</>;
}

Problem: The Connect route displays ALL wallet accounts with sensitive information (addresses, labels, notes, tags). Any website can trigger a connection request via the Wallet Standard protocol, causing the sidepanel to open at /accounts/connect and expose all wallet data without biometric authentication.

Attack vectors:

  1. Malicious dApp triggers wallet connection request → sidepanel opens at connect route → all accounts visible without authentication
  2. Physical attacker with browser access can programmatically trigger connection to view addresses

Why this matters: Users who enable biometric lock expect their wallet addresses to be protected. This bypass completely undermines that protection. Wallet addresses are sensitive financial information that allow tracking holdings and transaction history.

Recommendation: Require authentication before showing the connect dialog. Show lock screen first, then display accounts after successful biometric verification.


2. Bug: Lock Timeout Ineffective for Open Sidepanel 🔴

File: entrypoints/sidepanel/components/LockGuard.tsx (lines 90-92)
Severity: High

The LockGuard only checks lock state once on mount:

useEffect(() => {
  checkLockState();
}, []); // Empty dependency array = runs once on mount

Problem: If a user leaves the sidepanel open, the 30-minute inactivity timeout becomes completely ineffective. The background script will set isLocked: true in storage after 30 minutes, but the sidepanel never re-checks the lock state. The UI will remain accessible indefinitely until the sidepanel is closed and reopened.

Why this matters: This defeats a core security goal of the lock feature. The timeout is meant to protect against unauthorized access when the user steps away from their computer.

Recommendation: Add one of the following:

  1. Subscribe to Chrome storage changes: chrome.storage.onChanged.addListener() to detect when session:biometricLockState changes
  2. Implement periodic polling (every 30 seconds) to re-check lock state
  3. Have the background script send a message to the sidepanel when it locks

3. CLAUDE.md Violation: Import Order 🟡

File: entrypoints/background/index.ts (around lines 1-7)

CLAUDE.md specifies import order as: external packages → internal modules → relative imports (see rule)

Current order:

import type { InjectedEvent } from "../injected/events";  // relative
import { SidePanelEvent } from "../sidepanel/events";      // relative
import { makeConnectionSubmitForwardedEvent } from "./events"; // relative
import { isBiometricLockEnabled, setLockState, getLockState } from "~/biometricLock/storage"; // internal module

Expected order:

import { isBiometricLockEnabled, setLockState, getLockState } from "~/biometricLock/storage"; // internal module
import type { InjectedEvent } from "../injected/events";  // relative
import { SidePanelEvent } from "../sidepanel/events";      // relative
import { makeConnectionSubmitForwardedEvent } from "./events"; // relative

4. CLAUDE.md Violation: Undocumented Directory Structure 🟡

Files: All files in biometricLock/ directory

The new biometricLock/ directory follows the same pattern as other feature directories (accounts/, connections/, wallet/) but isn't documented in the Directory Structure section of CLAUDE.md.

Recommendation: Update CLAUDE.md to include:

├── biometricLock/               # Biometric lock feature
│   ├── schemas.ts               # Zod schemas for settings and state
│   ├── storage.ts               # CRUD operations for lock settings/state
│   ├── webauthn.ts              # WebAuthn integration
│   └── webauthn-local-client.d.ts  # Type definitions

Summary

  • 🔴 2 high-severity issues (security vulnerability, timeout bypass)
  • 🟡 2 CLAUDE.md compliance issues (import order, documentation)

The two security issues should be addressed before merging, as they significantly undermine the lock feature's security goals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants