mirror of
https://github.com/PR0M3TH3AN/bitlogin.git
synced 2026-10-27 08:31:21 +00:00
Tier 1 hardening: no more silent hangs, no writes before consent
Reliability, which is where most of these reach users: - The worker RPC had no message validation, no error/messageerror listener, no timeout, and terminate() left every pending promise unsettled. A malformed frame threw inside the listener (posting no response at all) and a dead worker stranded its callers -- the documented innerHTML-remount hazard manifested as a permanent freeze rather than an error. Both sides now validate the envelope, calls time out at 60s, and worker death rejects everything outstanding. - requestNwcConnection fired no pending event, so a host using the RECOMMENDED hidden-mount pattern navigated a hidden element to a consent screen nobody could see and waited forever. Now symmetric with bitlogin-offer-pending. Consent integrity: - vaultOfferCheck published an origin rebinding to relays -- a durable mutation of the user's account -- from a plain host API call before any consent screen existed. It is read-only now, and a wallet stored under a DIFFERENT origin no longer reports as already-saved, so the binding can only change through the consented path. - offerNwcConnection's guard sat above three awaits; two concurrent calls both passed it and the second dropped the first's promise. The slot is claimed synchronously. Vault reads fail closed: - listConnections threw rather than returning a relay-shaped subset when quorum is not met -- a short listing is indistinguishable from "you have no connections", and one relay should not get to say that. - The 500-record page limit is reported (truncated) instead of silently deciding which connections exist. Supply chain: - All eight crypto dependencies pinned to exact versions (they held users' keys behind caret ranges), and Bitcoin Connect with them. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01CMr9Yt8SYmZcfgrg3CV2G1
This commit is contained in:
1 parent
b414cc53d2
commit
d040c2f5af
7 files changed
+141
-36
No files matched your search
@@ -21,14 +21,14 @@
|
||||
"typecheck": "tsc -b --noEmit"
|
||||
},
|
||||
"dependencies": {
|
||||
"@noble/ciphers": "^2.2.0",
|
||||
"@noble/curves": "^1.9.1",
|
||||
"@noble/hashes": "^1.7.1",
|
||||
"@scure/base": "^1.2.6",
|
||||
"@scure/bip39": "^1.5.4",
|
||||
"canonicalize": "^2.0.0",
|
||||
"diceware-wordlist-en-eff": "^1.0.1",
|
||||
"hash-wasm": "^4.12.0"
|
||||
"@noble/ciphers": "2.2.0",
|
||||
"@noble/curves": "1.9.7",
|
||||
"@noble/hashes": "1.8.0",
|
||||
"@scure/base": "1.2.6",
|
||||
"@scure/bip39": "1.6.0",
|
||||
"canonicalize": "2.1.0",
|
||||
"diceware-wordlist-en-eff": "1.0.1",
|
||||
"hash-wasm": "4.12.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/node": "^22.10.0",
|
||||
|
||||
@@ -226,14 +226,28 @@ export class VaultSession {
|
||||
relayUrls: string[];
|
||||
store: KeyValueStore;
|
||||
timeoutMs?: number;
|
||||
/** Accept a below-quorum answer anyway. Off by default: a partial view
|
||||
* of a credential store reads as authoritative and isn't. */
|
||||
acknowledgeIncompleteQuorum?: boolean;
|
||||
}): Promise<{
|
||||
connections: DecryptedConnectionRecord[];
|
||||
rollbackWarnings: string[];
|
||||
/** Event ids that failed to decrypt or validate. Surfaced, never silent. */
|
||||
unreadable: string[];
|
||||
/** True when the relay page filled, so records may be missing. */
|
||||
truncated: boolean;
|
||||
quorumMet: boolean;
|
||||
}> {
|
||||
const fetched = await this.fetchEvents(params);
|
||||
if (!fetched.quorumMet && !params.acknowledgeIncompleteQuorum) {
|
||||
// Failing closed matters more here than for a read of public data: a
|
||||
// short listing looks identical to "you have no connections", and a
|
||||
// single relay deciding that is how a revoked-looking vault gets
|
||||
// re-populated or a live credential gets hidden.
|
||||
throw new Error(
|
||||
"Not enough vault relays answered to list your connections reliably. Check your connection and retry."
|
||||
);
|
||||
}
|
||||
const connections: DecryptedConnectionRecord[] = [];
|
||||
const unreadable: string[] = [];
|
||||
for (const event of fetched.events.values()) {
|
||||
@@ -254,6 +268,7 @@ export class VaultSession {
|
||||
connections,
|
||||
rollbackWarnings: fetched.rollbackWarnings,
|
||||
unreadable,
|
||||
truncated: fetched.truncated,
|
||||
quorumMet: fetched.quorumMet
|
||||
};
|
||||
}
|
||||
|
||||
@@ -99,8 +99,14 @@ export interface FetchedConnectionEvents {
|
||||
rollbackWarnings: string[];
|
||||
quorumMet: boolean;
|
||||
respondedCount: number;
|
||||
/** True when a relay returned the full page, so records may be missing. */
|
||||
truncated: boolean;
|
||||
}
|
||||
|
||||
/** Ceiling on records fetched in one query. Hit means the relay, not the
|
||||
* user, decided which connections exist -- so it is REPORTED, not silent. */
|
||||
const FETCH_LIMIT = 500;
|
||||
|
||||
/** Fetches every connection record visible for this vault identity (§CV10). */
|
||||
export async function fetchConnectionRecordEvents(params: {
|
||||
vaultPrk: Uint8Array;
|
||||
@@ -114,13 +120,14 @@ export async function fetchConnectionRecordEvents(params: {
|
||||
let quorum;
|
||||
try {
|
||||
quorum = await pool.queryQuorum(
|
||||
{ kinds: [KIND_APP_DATA], authors: [vaultPubkey], limit: 500 },
|
||||
{ kinds: [KIND_APP_DATA], authors: [vaultPubkey], limit: FETCH_LIMIT },
|
||||
params.timeoutMs
|
||||
);
|
||||
} finally {
|
||||
pool.closeAll();
|
||||
}
|
||||
|
||||
const truncated = quorum.outcomes.some((outcome) => outcome.events.length >= FETCH_LIMIT);
|
||||
const newestById = new Map<string, NostrEvent>();
|
||||
for (const event of quorum.outcomes.flatMap((o) => o.events)) {
|
||||
if (!verifyNostrEvent(event) || event.pubkey !== vaultPubkey) continue;
|
||||
@@ -157,6 +164,7 @@ export async function fetchConnectionRecordEvents(params: {
|
||||
events,
|
||||
rollbackWarnings,
|
||||
quorumMet: quorum.quorumMet,
|
||||
respondedCount: quorum.respondedCount
|
||||
respondedCount: quorum.respondedCount,
|
||||
truncated
|
||||
};
|
||||
}
|
||||
@@ -13,7 +13,7 @@
|
||||
},
|
||||
"dependencies": {
|
||||
"@bitlogin/core": "*",
|
||||
"@getalby/bitcoin-connect": "^3.12.3"
|
||||
"@getalby/bitcoin-connect": "3.12.3"
|
||||
},
|
||||
"devDependencies": {
|
||||
"typescript": "^5.7.3",
|
||||
|
||||
@@ -123,6 +123,8 @@ export class BitLoginAuthElement extends HTMLElement {
|
||||
private vaultConnections: VaultConnectionSummary[] | null = null;
|
||||
/** offerNwcConnection state: the app already holds this URI; the only
|
||||
* question on screen is whether a copy enters the user's vault. */
|
||||
/** Claimed synchronously by offerNwcConnection before any await (see there). */
|
||||
private offerInFlight = false;
|
||||
private vaultOffer: {
|
||||
uri: string;
|
||||
appName: string;
|
||||
@@ -239,6 +241,10 @@ export class BitLoginAuthElement extends HTMLElement {
|
||||
const appName = options.appName?.trim() || window.location.hostname || "This app";
|
||||
return new Promise<string | null>((resolve) => {
|
||||
this.vaultRequest = { appName, reason: options.reason, origin, stage: "auth", resolve };
|
||||
// Symmetric with bitlogin-offer-pending. Without it, a host following
|
||||
// the recommended hidden-mount pattern navigated a HIDDEN element to a
|
||||
// consent screen nobody could see, and the promise never settled.
|
||||
this.dispatchEvent(new CustomEvent("bitlogin-request-pending"));
|
||||
void (async () => {
|
||||
try {
|
||||
const status = await this.worker.getSessionStatus();
|
||||
@@ -272,12 +278,26 @@ export class BitLoginAuthElement extends HTMLElement {
|
||||
uri: string,
|
||||
options: { appName?: string; label?: string } = {}
|
||||
): Promise<"saved" | "declined" | "already-saved" | "unavailable"> {
|
||||
if (this.vaultOffer || this.vaultRequest) return "unavailable";
|
||||
// The slot is claimed SYNCHRONOUSLY. The guard used to sit above three
|
||||
// awaits, so two concurrent calls both passed it and the second
|
||||
// overwrote the first's resolve -- dropping a promise that never settled.
|
||||
if (this.vaultOffer || this.vaultRequest || this.offerInFlight) return "unavailable";
|
||||
this.offerInFlight = true;
|
||||
try {
|
||||
return await this.runOfferNwcConnection(uri, options);
|
||||
} finally {
|
||||
this.offerInFlight = false;
|
||||
}
|
||||
}
|
||||
|
||||
private async runOfferNwcConnection(
|
||||
uri: string,
|
||||
options: { appName?: string; label?: string }
|
||||
): Promise<"saved" | "declined" | "already-saved" | "unavailable"> {
|
||||
const status = await this.worker.getSessionStatus().catch(() => ({ unlocked: false }));
|
||||
if (!status.unlocked) return "unavailable";
|
||||
const vaultStatus = await this.worker.vaultStatus();
|
||||
if (!vaultStatus.enabled) return "unavailable";
|
||||
const origin = window.location.origin;
|
||||
const check = await this.worker.vaultOfferCheck({ uri });
|
||||
if (check.duplicate) return "already-saved";
|
||||
|
||||
|
||||
@@ -601,36 +601,26 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
|
||||
}
|
||||
|
||||
case "vaultOfferCheck": {
|
||||
// The offer-to-save flow's dedupe: the same wallet + secret already
|
||||
// stored means nothing NEW would enter the vault, so no consent screen
|
||||
// is warranted. The origin binding is refreshed silently — the offering
|
||||
// origin already HOLDS the credential (it supplied it), so binding is
|
||||
// bookkeeping for later one-tap approvals, not a new grant.
|
||||
// READ-ONLY on purpose. This used to publish an origin rebinding to
|
||||
// relays -- a real, durable mutation of the user's account -- from a
|
||||
// plain host API call, before any consent screen existed. Deduping is
|
||||
// a question, not a write. If the caller wants the binding refreshed,
|
||||
// that happens through the consented save path.
|
||||
const p = payload as { uri: string };
|
||||
const origin = selfOrigin();
|
||||
const credential = parseNwcUri(p.uri);
|
||||
const { vault, connections } = await listLiveConnections();
|
||||
const { connections } = await listLiveConnections();
|
||||
const existing = connections.find(
|
||||
(c) =>
|
||||
c.record.connection_type === "nwc" &&
|
||||
sameNwcCredential(c.record.credential as unknown as NwcCredential, credential)
|
||||
);
|
||||
if (!existing) return { duplicate: false };
|
||||
if (existing.record.application_binding.origin !== origin) {
|
||||
const { record, event } = await vault.updateConnection(existing, {
|
||||
application_binding: { origin, app_pubkey: null }
|
||||
});
|
||||
const publish = await vault.publish({
|
||||
event,
|
||||
connectionId: record.connection_id,
|
||||
relayUrls: vaultRelayUrls,
|
||||
store
|
||||
});
|
||||
if (publish.success) {
|
||||
const decrypted = await vault.decryptEvent(event);
|
||||
return { duplicate: true, connection: summarize(decrypted!) };
|
||||
}
|
||||
}
|
||||
// Only report it as already-saved when it is already THIS origin's.
|
||||
// A wallet stored under another origin is not this caller's to claim
|
||||
// silently; treating it as new sends the user through consent, which
|
||||
// is the only place a cross-origin binding may legitimately change.
|
||||
if (existing.record.application_binding.origin !== origin) return { duplicate: false };
|
||||
return { duplicate: true, connection: summarize(existing) };
|
||||
}
|
||||
|
||||
@@ -639,7 +629,32 @@ async function handle(action: string, payload: unknown): Promise<unknown> {
|
||||
}
|
||||
}
|
||||
|
||||
const KNOWN_ACTIONS = new Set<string>([
|
||||
"configure", "register", "previewImportKey", "login", "recover", "completeRecovery",
|
||||
"changePassword", "publishProfileAndRelayLists", "getPublicKey", "signEvent",
|
||||
"nip44Encrypt", "nip44Decrypt", "nip04Encrypt", "nip04Decrypt", "exportIdentity",
|
||||
"buildRecoveryExport", "repairReplicas", "getSessionStatus", "restoreSession", "logout",
|
||||
"vaultStatus", "vaultList", "vaultSaveNwc", "vaultFindForOrigin", "vaultRevealNwc",
|
||||
"vaultSetBinding", "vaultDelete", "vaultOfferCheck"
|
||||
]);
|
||||
|
||||
self.addEventListener("message", (event: MessageEvent<WorkerRequest>) => {
|
||||
// Validate the envelope BEFORE destructuring. A malformed frame (or a
|
||||
// stray postMessage from anything else on the page) used to throw straight
|
||||
// out of this listener, which posts no response at all and leaves the
|
||||
// caller's promise pending forever.
|
||||
const frame = event.data as Partial<WorkerRequest> | null;
|
||||
if (!frame || typeof frame !== "object" || typeof frame.id !== "string") return;
|
||||
if (typeof frame.action !== "string" || !KNOWN_ACTIONS.has(frame.action)) {
|
||||
const response: WorkerResponse = {
|
||||
id: frame.id,
|
||||
ok: false,
|
||||
error: `Unknown worker action: ${String(frame.action)}`,
|
||||
errorName: "Error"
|
||||
};
|
||||
(self as unknown as Worker).postMessage(response);
|
||||
return;
|
||||
}
|
||||
const { id, action, payload } = event.data;
|
||||
handle(action, payload).then(
|
||||
(result) => {
|
||||
|
||||
@@ -25,10 +25,21 @@ import type {
|
||||
VaultOfferCheckPayload
|
||||
} from "./protocol.js";
|
||||
|
||||
/**
|
||||
* How long a single worker call may take before the caller is told, rather
|
||||
* than left hanging. Argon2id at 64 MiB plus a relay quorum round trip is the
|
||||
* slow case; a minute is far past it and still finite.
|
||||
*/
|
||||
const CALL_TIMEOUT_MS = 60_000;
|
||||
|
||||
export class WorkerClient {
|
||||
private readonly worker: Worker;
|
||||
private readonly pending = new Map<string, { resolve: (value: unknown) => void; reject: (error: Error) => void }>();
|
||||
private readonly pending = new Map<
|
||||
string,
|
||||
{ resolve: (value: unknown) => void; reject: (error: Error) => void; timer: ReturnType<typeof setTimeout> }
|
||||
>();
|
||||
private counter = 0;
|
||||
private dead = false;
|
||||
|
||||
constructor() {
|
||||
// Deliberately NOT the literal `new Worker(new URL("./x.js", import.meta.url))` shape: Vite
|
||||
@@ -42,9 +53,13 @@ export class WorkerClient {
|
||||
this.worker = new Worker(workerUrl, { type: "module" });
|
||||
this.worker.addEventListener("message", (event: MessageEvent<WorkerResponse>) => {
|
||||
const msg = event.data;
|
||||
// Shape-check before trusting: a malformed frame used to throw inside
|
||||
// this listener, which posts no response and hangs the caller forever.
|
||||
if (!msg || typeof msg !== "object" || typeof (msg as WorkerResponse).id !== "string") return;
|
||||
const entry = this.pending.get(msg.id);
|
||||
if (!entry) return;
|
||||
this.pending.delete(msg.id);
|
||||
clearTimeout(entry.timer);
|
||||
if (msg.ok) {
|
||||
entry.resolve(msg.result);
|
||||
} else {
|
||||
@@ -53,12 +68,40 @@ export class WorkerClient {
|
||||
entry.reject(error);
|
||||
}
|
||||
});
|
||||
|
||||
// A worker that dies -- failed script load, an uncaught throw, the
|
||||
// documented innerHTML-remount hazard -- must REJECT its callers, not
|
||||
// strand them. Every hang reported against this widget traced back to a
|
||||
// promise with nobody left to settle it.
|
||||
const fail = (reason: string) => () => this.failAll(reason);
|
||||
this.worker.addEventListener("error", fail("The BitLogin crypto worker stopped unexpectedly."));
|
||||
this.worker.addEventListener(
|
||||
"messageerror",
|
||||
fail("The BitLogin crypto worker sent an unreadable message.")
|
||||
);
|
||||
}
|
||||
|
||||
/** Settles every outstanding call with an error. Idempotent. */
|
||||
private failAll(reason: string): void {
|
||||
const entries = [...this.pending.values()];
|
||||
this.pending.clear();
|
||||
for (const entry of entries) {
|
||||
clearTimeout(entry.timer);
|
||||
entry.reject(new Error(reason));
|
||||
}
|
||||
}
|
||||
|
||||
private call<A extends WorkerAction>(action: A, payload: WorkerActionMap[A][0]): Promise<WorkerActionMap[A][1]> {
|
||||
if (this.dead) {
|
||||
return Promise.reject(new Error("This BitLogin session was torn down; reload the page to sign in again."));
|
||||
}
|
||||
const id = `${Date.now().toString(36)}-${(this.counter++).toString(36)}`;
|
||||
return new Promise((resolve, reject) => {
|
||||
this.pending.set(id, { resolve: resolve as (value: unknown) => void, reject });
|
||||
const timer = setTimeout(() => {
|
||||
this.pending.delete(id);
|
||||
reject(new Error(`BitLogin's ${action} call timed out.`));
|
||||
}, CALL_TIMEOUT_MS);
|
||||
this.pending.set(id, { resolve: resolve as (value: unknown) => void, reject, timer });
|
||||
const request: WorkerRequest = { id, action, payload };
|
||||
this.worker.postMessage(request);
|
||||
});
|
||||
@@ -150,6 +193,10 @@ export class WorkerClient {
|
||||
}
|
||||
|
||||
terminate(): void {
|
||||
this.dead = true;
|
||||
this.worker.terminate();
|
||||
// terminate() silently kills in-flight work; without this every caller
|
||||
// waits forever on a worker that no longer exists.
|
||||
this.failAll("The BitLogin crypto worker was terminated.");
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user