Skip to content

Commit 1e9c410

Browse files
committed
feat: v0.3.3 — comprehensive code review, handler split, security hardening
Round 2 code review by 10 parallel Opus agents covering security, performance, code quality, dead code, documentation, and roadmap. 70 new findings documented in docs/code-review.md. Key changes: misc-handlers split into activity/profile/publish handlers, graph.ts private modifiers, NIP-07 input validation, permission cache test fixes, .js->.ts import standardization, nostr-tools/pure removal, IDB upgrade deduplication, centralized constants, async-lock extraction.
1 parent 755c5b7 commit 1e9c410

64 files changed

Lines changed: 2071 additions & 1210 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,32 @@
22

33
All notable changes to this project will be documented in this file.
44

5+
## [0.3.3] - 2026-03-13
6+
7+
### Changed
8+
- **Comprehensive code review (Round 2)** — 10 parallel Opus agents audited the full codebase covering security, performance, code quality, dead code, documentation accuracy, and future enhancements; 70 new findings documented in `docs/code-review.md`
9+
- **Misc-handlers split**`misc-handlers.ts` (507 lines) split into `activity-handlers.ts`, `profile-handlers.ts`, `publish-handlers.ts`; original file is now a re-export facade
10+
- **Graph module hardened** — underscore-prefixed methods replaced with `private` modifier; all 11 internal call sites updated
11+
- **NIP-07 input validation** — added `event.tags` validation (array of string arrays) and `event.created_at` validation (integer, not >1 hour in future)
12+
- **signEvent zeroing contract** — comprehensive JSDoc documenting caller responsibility for key zeroing
13+
- **LNbits HTTP warning** — console.warn added when admin key is sent over non-localhost HTTP
14+
15+
### Fixed
16+
- **Permission cache test failures** — added `storage.onChanged` support to browser mock so in-memory caches invalidate correctly between tests
17+
- **Wallet balance assertions** — fixed msats-to-sats conversion in NWC test mocks (values now in millisats, expected results in sats)
18+
- **Import extensions** — standardized all 17 test files from `.js` to `.ts` import extensions
19+
- **nostr-tools/pure removal** — replaced `generateSecretKey`/`getPublicKey` imports with own crypto; only `nostr-tools/nip46` remains
20+
- **IDB upgrade deduplication** — extracted shared `upgradeDatabase()` helper from duplicate `onupgradeneeded` callbacks
21+
- **NODE_ENV** — changed from `'development'` to `'production'` in vite.config.ts
22+
23+
### Added
24+
- `lib/constants.ts` — centralized magic numbers (timeouts, rate limits, crypto parameters)
25+
- `lib/utils/async-lock.ts` — shared async mutex (extracted from duplicated `withStorageLock` pattern)
26+
- `lib/bg/activity-handlers.ts` — activity log handlers with write buffering
27+
- `lib/bg/profile-handlers.ts` — profile metadata and mute list handlers
28+
- `lib/bg/publish-handlers.ts` — event signing, broadcasting, and NIP-46 session handlers
29+
- `docs/code-review.md` — comprehensive Round 2 audit with 70 findings and prioritized roadmap
30+
531
## [0.3.2] - 2026-03-10
632

733
### Changed

background.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ import type { ScoringConfig } from './lib/types.ts';
1313

1414
import {
1515
config, setOracle, resetLocalGraph,
16-
PRIVILEGED_METHODS, NIP07_SIGNING_METHODS,
16+
NIP07_SIGNING_METHODS,
1717
checkRateLimit, npubToHex,
18+
buildPrivilegedMethods, setPrivilegedMethods,
19+
PRIVILEGED_METHODS,
1820
type HandlerFn,
1921
} from './lib/bg/state.ts';
2022
import { handlers as wotHandlers } from './lib/bg/wot-handlers.ts';
@@ -32,27 +34,43 @@ import { handlers as onboardingHandlers } from './lib/bg/onboarding-handlers.ts'
3234
// ── Assemble handler map ──
3335

3436
const allHandlers = new Map<string, HandlerFn>();
35-
for (const group of [wotHandlers, miscHandlers, domainHandlers, vaultHandlers, walletHandlers, nip07Handlers, onboardingHandlers]) {
37+
const handlerGroups = [wotHandlers, miscHandlers, domainHandlers, vaultHandlers, walletHandlers, nip07Handlers, onboardingHandlers];
38+
for (const group of handlerGroups) {
3639
for (const [method, fn] of group) {
40+
if (allHandlers.has(method)) {
41+
console.error(`[BG] Duplicate handler registration: "${method}" — later registration overwrites earlier one`);
42+
}
3743
allHandlers.set(method, fn);
3844
}
3945
}
4046

4147
// configUpdated stays here because it calls loadConfig which is local
48+
let _refreshDebounceTimer: ReturnType<typeof setTimeout> | null = null;
4249
allHandlers.set('configUpdated', async () => {
4350
await loadConfig();
44-
refreshBadgesOnAllTabs();
51+
// Debounce badge refresh — multiple rapid config updates only trigger one refresh
52+
if (_refreshDebounceTimer) clearTimeout(_refreshDebounceTimer);
53+
_refreshDebounceTimer = setTimeout(() => { refreshBadgesOnAllTabs(); _refreshDebounceTimer = null; }, 500);
4554
return { ok: true };
4655
});
4756

57+
// Auto-derive PRIVILEGED_METHODS from all handler maps (no manual allowlist needed)
58+
const privilegedHandlerGroups = [miscHandlers, domainHandlers, vaultHandlers, walletHandlers, nip07Handlers, onboardingHandlers];
59+
setPrivilegedMethods(buildPrivilegedMethods(...privilegedHandlerGroups));
60+
// Also add configUpdated and other locally-defined handlers
61+
PRIVILEGED_METHODS.add('configUpdated');
62+
PRIVILEGED_METHODS.add('syncGraph');
63+
PRIVILEGED_METHODS.add('stopSync');
64+
PRIVILEGED_METHODS.add('clearGraph');
65+
4866
// ── Config loading ──
4967

5068
async function loadConfig(): Promise<void> {
5169
const data = await browser.storage.sync.get([
5270
'mode', 'oracleUrl', 'myPubkey', 'relays', 'scoring'
5371
]) as Record<string, unknown>;
5472

55-
config.mode = (data.mode as string) || 'hybrid';
73+
config.mode = (data.mode as 'local' | 'remote' | 'hybrid') || 'hybrid';
5674
config.myPubkey = (data.myPubkey as string) || null;
5775
config.maxHops = 3;
5876
config.timeout = 5000;
@@ -290,9 +308,7 @@ browser.runtime.onConnect.addListener((port: chrome.runtime.Port) => {
290308
}
291309

292310
try {
293-
console.log('[PORT]', port.name, 'request:', method);
294311
const result = await handleRequest(request as { method: string; params: Record<string, unknown> });
295-
console.log('[PORT]', port.name, 'success:', method);
296312
try { port.postMessage({ result }); } catch {}
297313
} catch (error) {
298314
console.error('[PORT]', port.name, 'error:', method, (error as Error).message);

content.ts

Lines changed: 82 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -66,39 +66,99 @@ if (window.__nostrWotContentInjected) {
6666
return ++wotRequestCount <= WOT_RATE_LIMIT;
6767
}
6868

69-
function forwardViaPort(portName: string, responseType: string, id: string, method: string, params: unknown): void {
69+
// ── Persistent port management ──
70+
// One port per channel (nip07, webln). Requests are serialized per port because
71+
// the background handler does not echo request IDs in responses, so we process
72+
// one request at a time to match responses unambiguously.
73+
74+
interface PortRequest {
75+
id: string;
76+
responseType: string;
77+
method: string;
78+
params: unknown;
79+
}
80+
81+
interface PortState {
82+
port: ReturnType<typeof browser.runtime.connect> | null;
83+
queue: PortRequest[];
84+
inflight: PortRequest | null;
85+
}
86+
87+
const portStates: Record<string, PortState> = {
88+
nip07: { port: null, queue: [], inflight: null },
89+
webln: { port: null, queue: [], inflight: null },
90+
};
91+
92+
function postResponse(responseType: string, id: string, result: unknown, error: unknown): void {
93+
window.postMessage({ type: responseType, id, result, error }, window.location.origin);
94+
}
95+
96+
function processNextInQueue(portName: string): void {
97+
const state = portStates[portName];
98+
if (state.inflight || state.queue.length === 0) return;
99+
100+
const request = state.queue.shift()!;
101+
state.inflight = request;
102+
103+
const port = getOrCreatePort(portName);
104+
if (!port) {
105+
state.inflight = null;
106+
postResponse(request.responseType, request.id, null, 'Extension context invalidated — reload the page');
107+
processNextInQueue(portName);
108+
return;
109+
}
110+
70111
const origin = window.location.hostname;
112+
port.postMessage({
113+
method: portName + '_' + request.method,
114+
params: { ...(request.params as Record<string, unknown>), origin }
115+
});
116+
}
117+
118+
function getOrCreatePort(portName: string): ReturnType<typeof browser.runtime.connect> | null {
119+
const state = portStates[portName];
120+
if (state.port) return state.port;
121+
71122
try {
72123
const port = browser.runtime.connect({ name: portName });
73-
let responded = false;
74-
75-
const sendResult = (result: unknown, error: unknown) => {
76-
if (responded) return;
77-
responded = true;
78-
window.postMessage({ type: responseType, id, result, error }, window.location.origin);
79-
};
80124

81125
port.onMessage.addListener((response: Record<string, unknown>) => {
82-
sendResult(response.result, response.error);
83-
try { port.disconnect(); } catch {}
126+
const request = state.inflight;
127+
if (request) {
128+
state.inflight = null;
129+
postResponse(request.responseType, request.id, response.result, response.error);
130+
}
131+
processNextInQueue(portName);
84132
});
85133

86134
port.onDisconnect.addListener(() => {
87-
sendResult(null, 'Extension context invalidated — reload the page');
135+
state.port = null;
136+
// Reject the in-flight request if any
137+
if (state.inflight) {
138+
postResponse(state.inflight.responseType, state.inflight.id, null, 'Extension context invalidated — reload the page');
139+
state.inflight = null;
140+
}
141+
// Reject all queued requests
142+
for (const req of state.queue) {
143+
postResponse(req.responseType, req.id, null, 'Extension context invalidated — reload the page');
144+
}
145+
state.queue = [];
88146
});
89147

90-
port.postMessage({
91-
method: portName + '_' + method,
92-
params: { ...(params as Record<string, unknown>), origin }
93-
});
94-
} catch {
95-
window.postMessage({
96-
type: responseType, id, result: null,
97-
error: 'Extension context invalidated — reload the page'
98-
}, window.location.origin);
148+
state.port = port;
149+
return port;
150+
} catch (err) {
151+
console.debug(`[nostr-wot] Failed to create port "${portName}":`, err);
152+
return null;
99153
}
100154
}
101155

156+
function forwardViaPort(portName: string, responseType: string, id: string, method: string, params: unknown): void {
157+
const state = portStates[portName];
158+
state.queue.push({ id, responseType, method, params });
159+
processNextInQueue(portName);
160+
}
161+
102162
// Bridge between page and extension
103163
window.addEventListener('message', async (event: MessageEvent) => {
104164
if (event.source !== window) return;
@@ -130,7 +190,8 @@ if (window.__nostrWotContentInjected) {
130190
result: response.result,
131191
error: response.error
132192
}, window.location.origin);
133-
} catch {
193+
} catch (err) {
194+
console.debug('[nostr-wot] WoT request failed:', method, err);
134195
window.postMessage({
135196
type: 'WOT_RESPONSE', id, result: null,
136197
error: 'Extension context invalidated — reload the page'

0 commit comments

Comments
 (0)