mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): decrypt deep-link/search targets before the rendered DM timeline
Deep-link, thread-ancestor, and search-hit targets fetched by ChannelRouteScreen land in targetMessageEvents as RAW RelayEvents and ChannelScreen's resolvedMessages memo merges them into the rendered list with no decrypt. For a DM the body is NIP-44 v2 ciphertext, so it rendered garbled and, on an id collision, clobbered the decrypted cache copy (the merge keeps the last writer). A new useDecryptedTargetMessageEvents hook is the single choke point downstream of all three target setters (the mount-seed useState initializer is synchronous and cannot decrypt in place), decrypting via makeDmIngestDecryptor before the merge; non-DM targets pass through synchronously to avoid a held-back first paint. Also resets the requestedAncestorIdsRef dedup in useLoadMissingAncestors on selfPubkey change, not just channel change: a cold-start ancestor fetched while identity is undefined was recorded as done, so after identity resolved the effect skipped re-fetching it into the live [...,pubkey] bucket and the ancestor silently went missing from the thread. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -44,6 +44,7 @@ import {
|
||||
resolveTimelineLoadingLatch,
|
||||
selectTimelineLoadingState,
|
||||
} from "@/features/messages/lib/timelineLoadingState";
|
||||
import { useDecryptedTargetMessageEvents } from "@/features/messages/useDecryptedTargetMessageEvents";
|
||||
import { useFetchOlderMessages } from "@/features/messages/useFetchOlderMessages";
|
||||
import { useLoadMissingAncestors } from "@/features/messages/useLoadMissingAncestors";
|
||||
import { useChannelTyping } from "@/features/messages/useChannelTyping";
|
||||
@@ -252,13 +253,22 @@ export function ChannelScreen({
|
||||
currentPubkey,
|
||||
);
|
||||
const joinChannelMutation = useJoinChannelMutation(activeChannelId);
|
||||
// Decrypt deep-link / search-hit targets before the rendered merge: they
|
||||
// arrive raw, so for a DM the ciphertext would render garbled and clobber the
|
||||
// decrypted cache copy on an id collision. This is the single choke point for
|
||||
// every target contributor.
|
||||
const decryptedTargetMessageEvents = useDecryptedTargetMessageEvents(
|
||||
activeChannel,
|
||||
targetMessageEvents,
|
||||
currentPubkey,
|
||||
);
|
||||
const resolvedMessages = React.useMemo(() => {
|
||||
const currentMessages = messagesQuery.data ?? [];
|
||||
if (!activeChannel || targetMessageEvents.length === 0) {
|
||||
if (!activeChannel || decryptedTargetMessageEvents.length === 0) {
|
||||
return currentMessages;
|
||||
}
|
||||
return targetMessageEvents.reduce(mergeMessages, currentMessages);
|
||||
}, [activeChannel, messagesQuery.data, targetMessageEvents]);
|
||||
return decryptedTargetMessageEvents.reduce(mergeMessages, currentMessages);
|
||||
}, [activeChannel, messagesQuery.data, decryptedTargetMessageEvents]);
|
||||
const messageAuthorPubkeys = React.useMemo(
|
||||
() => collectMessageAuthorPubkeys(resolvedMessages),
|
||||
[resolvedMessages],
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { shouldResetAncestorTracking } from "@/features/messages/useLoadMissingAncestors";
|
||||
|
||||
// useLoadMissingAncestors records fetched ancestor ids so it never re-fetches
|
||||
// the same id. That tracking must reset when the IDENTITY scope of the cache
|
||||
// bucket changes, not only when the channel changes — otherwise a cold-start
|
||||
// ancestor fetched while selfPubkey is undefined (a no-op decrypt that lands
|
||||
// raw under the [...,null] bucket) is recorded as "done", and after identity
|
||||
// resolves the effect SKIPS it: the ancestor is never re-fetched/re-decrypted
|
||||
// into the rendered [...,pubkey] bucket and silently goes missing.
|
||||
|
||||
test("ancestor tracking resets when selfPubkey resolves from undefined to a pubkey", () => {
|
||||
assert.equal(
|
||||
shouldResetAncestorTracking(
|
||||
{ channelId: "c1", selfPubkey: undefined },
|
||||
{ channelId: "c1", selfPubkey: "a".repeat(64) },
|
||||
),
|
||||
true,
|
||||
"cold-start identity resolution must reset so the ancestor is re-fetched",
|
||||
);
|
||||
});
|
||||
|
||||
test("ancestor tracking resets when the active channel changes", () => {
|
||||
assert.equal(
|
||||
shouldResetAncestorTracking(
|
||||
{ channelId: "c1", selfPubkey: "a".repeat(64) },
|
||||
{ channelId: "c2", selfPubkey: "a".repeat(64) },
|
||||
),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("ancestor tracking does NOT reset when neither channel nor identity changed", () => {
|
||||
assert.equal(
|
||||
shouldResetAncestorTracking(
|
||||
{ channelId: "c1", selfPubkey: "a".repeat(64) },
|
||||
{ channelId: "c1", selfPubkey: "a".repeat(64) },
|
||||
),
|
||||
false,
|
||||
"a stable scope must keep the dedup so the same ancestor is not refetched every render",
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,107 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { makeDmIngestDecryptor } from "@/features/messages/lib/dmCrypto";
|
||||
import { mergeMessages } from "@/features/messages/hooks";
|
||||
|
||||
// Minimal valid NIP-44 v2 envelope (see messageQueryKeys.test.mjs).
|
||||
const V2_CIPHERTEXT =
|
||||
"AgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
|
||||
|
||||
const DM_CHANNEL = {
|
||||
id: "dm-target-channel-id",
|
||||
channelType: "dm",
|
||||
participantPubkeys: ["a".repeat(64), "b".repeat(64)],
|
||||
};
|
||||
const STREAM_CHANNEL = {
|
||||
id: "stream-target-channel-id",
|
||||
channelType: "stream",
|
||||
participantPubkeys: [],
|
||||
};
|
||||
const SELF = "a".repeat(64);
|
||||
const PEER = "b".repeat(64);
|
||||
|
||||
function targetEvent(content) {
|
||||
return {
|
||||
id: "tgt".padEnd(64, "0"),
|
||||
pubkey: PEER,
|
||||
created_at: 5_000,
|
||||
kind: 9,
|
||||
tags: [["h", DM_CHANNEL.id]],
|
||||
content,
|
||||
sig: "mocksig".repeat(20).slice(0, 128),
|
||||
};
|
||||
}
|
||||
|
||||
// Mirror useDecryptedTargetMessageEvents + ChannelScreen's resolvedMessages
|
||||
// reduce: decrypt the target events, then merge them into the (already
|
||||
// decrypted) current messages exactly as the render path does. The RED form
|
||||
// skips the decrypt and merges the raw target.
|
||||
async function resolveRenderedTimeline(
|
||||
channel,
|
||||
selfPubkey,
|
||||
currentMessages,
|
||||
targetMessageEvents,
|
||||
) {
|
||||
const decryptIngested = makeDmIngestDecryptor(channel, selfPubkey);
|
||||
const decryptedTargets = await decryptIngested(targetMessageEvents);
|
||||
return decryptedTargets.reduce(mergeMessages, currentMessages);
|
||||
}
|
||||
|
||||
test("DM route-target event is decrypted before it reaches the rendered timeline, never raw ciphertext", async () => {
|
||||
const rendered = await resolveRenderedTimeline(
|
||||
DM_CHANNEL,
|
||||
SELF,
|
||||
[],
|
||||
[targetEvent(V2_CIPHERTEXT)],
|
||||
);
|
||||
|
||||
assert.equal(rendered.length, 1, "the target row is spliced into the list");
|
||||
assert.notEqual(
|
||||
rendered[0].content,
|
||||
V2_CIPHERTEXT,
|
||||
"a DM route-target must not render raw ciphertext",
|
||||
);
|
||||
});
|
||||
|
||||
test("DM route-target does not clobber an already-decrypted copy of the same id with ciphertext", async () => {
|
||||
// The decrypted cache already holds plaintext-X (e.g. history fetched it).
|
||||
const decryptedCopy = {
|
||||
...targetEvent("dinner at 7?"),
|
||||
content: "dinner at 7?",
|
||||
};
|
||||
|
||||
const rendered = await resolveRenderedTimeline(
|
||||
DM_CHANNEL,
|
||||
SELF,
|
||||
[decryptedCopy],
|
||||
[targetEvent(V2_CIPHERTEXT)],
|
||||
);
|
||||
|
||||
assert.equal(rendered.length, 1, "id collision keeps a single row");
|
||||
assert.notEqual(
|
||||
rendered[0].content,
|
||||
V2_CIPHERTEXT,
|
||||
"the raw target must not clobber the decrypted copy",
|
||||
);
|
||||
});
|
||||
|
||||
test("non-DM route-target with a v2-shaped body passes through verbatim", async () => {
|
||||
const streamTarget = {
|
||||
...targetEvent(V2_CIPHERTEXT),
|
||||
tags: [["h", STREAM_CHANNEL.id]],
|
||||
};
|
||||
|
||||
const rendered = await resolveRenderedTimeline(
|
||||
STREAM_CHANNEL,
|
||||
SELF,
|
||||
[],
|
||||
[streamTarget],
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
rendered[0].content,
|
||||
V2_CIPHERTEXT,
|
||||
"outside a 2-party DM the decryptor is a no-op and content is untouched",
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
import * as React from "react";
|
||||
|
||||
import {
|
||||
dmPeerPubkey,
|
||||
makeDmIngestDecryptor,
|
||||
} from "@/features/messages/lib/dmCrypto";
|
||||
import type { Channel, RelayEvent } from "@/shared/api/types";
|
||||
|
||||
/**
|
||||
* Decrypt deep-link / search-hit target events before they reach the rendered
|
||||
* timeline.
|
||||
*
|
||||
* The route layer fetches deep-link targets, thread ancestors, and search hits
|
||||
* as RAW RelayEvents (no decrypt) and threads them down as `targetMessageEvents`.
|
||||
* `ChannelScreen` merges those into the rendered list — so for a DM, where the
|
||||
* body is NIP-44 v2 ciphertext, the raw target would render garbled and, on an
|
||||
* id collision, CLOBBER the decrypted cache copy (the merge keeps the last
|
||||
* writer).
|
||||
*
|
||||
* This is the single choke point for that whole class: every contributor flows
|
||||
* through the one `targetMessageEvents` array, so decrypting it here once covers
|
||||
* the synchronous mount-seed, the cached-search-hit path, and the async fetch
|
||||
* path uniformly — a decrypt split across the individual setters would miss the
|
||||
* synchronous mount-seed and leak on a search-jump first paint.
|
||||
*
|
||||
* Outside a 2-party DM (`dmPeerPubkey` null) there is nothing to decrypt, so the
|
||||
* events pass through synchronously with no held-back frame. Inside a DM the
|
||||
* events are held back (empty) until the async decrypt resolves, so raw
|
||||
* ciphertext never paints.
|
||||
*/
|
||||
export function useDecryptedTargetMessageEvents(
|
||||
activeChannel: Channel | null,
|
||||
targetMessageEvents: RelayEvent[],
|
||||
selfPubkey: string | undefined,
|
||||
): RelayEvent[] {
|
||||
const needsDecrypt =
|
||||
activeChannel !== null && dmPeerPubkey(activeChannel, selfPubkey) !== null;
|
||||
|
||||
const [decryptedEvents, setDecryptedEvents] = React.useState<RelayEvent[]>(
|
||||
[],
|
||||
);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!needsDecrypt || targetMessageEvents.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
let isCancelled = false;
|
||||
const decryptIngested = makeDmIngestDecryptor(activeChannel, selfPubkey);
|
||||
void decryptIngested(targetMessageEvents).then((decrypted) => {
|
||||
if (!isCancelled) {
|
||||
setDecryptedEvents(decrypted);
|
||||
}
|
||||
});
|
||||
|
||||
return () => {
|
||||
isCancelled = true;
|
||||
};
|
||||
}, [activeChannel, needsDecrypt, selfPubkey, targetMessageEvents]);
|
||||
|
||||
// Outside a DM there is nothing to decrypt: pass the events through directly so
|
||||
// a non-DM deep-link splices its target on first paint with no held-back frame.
|
||||
return needsDecrypt ? decryptedEvents : targetMessageEvents;
|
||||
}
|
||||
@@ -11,6 +11,34 @@ import {
|
||||
import { getEventById } from "@/shared/api/tauri";
|
||||
import type { Channel, RelayEvent } from "@/shared/api/types";
|
||||
|
||||
/** The scope that the requested-ancestor dedup set is valid for. */
|
||||
interface AncestorScope {
|
||||
channelId: string | null;
|
||||
selfPubkey: string | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the requested-ancestor dedup tracking must reset.
|
||||
*
|
||||
* The dedup set keys "ancestor already fetched" by id, but a fetched ancestor
|
||||
* lands in `channelMessagesKey(channelId, selfPubkey)` — a bucket scoped by
|
||||
* BOTH channel AND identity. So the set is only valid within a single
|
||||
* (channel, identity) scope. On a cold start an ancestor fetched while
|
||||
* `selfPubkey` is undefined no-op-decrypts into the orphaned `[...,null]`
|
||||
* bucket yet gets recorded as done; without resetting on the identity flip the
|
||||
* effect would skip re-fetching it into the live `[...,pubkey]` bucket and the
|
||||
* ancestor would silently go missing from the thread.
|
||||
*/
|
||||
export function shouldResetAncestorTracking(
|
||||
previous: AncestorScope,
|
||||
next: AncestorScope,
|
||||
): boolean {
|
||||
return (
|
||||
previous.channelId !== next.channelId ||
|
||||
previous.selfPubkey !== next.selfPubkey
|
||||
);
|
||||
}
|
||||
|
||||
export function useLoadMissingAncestors(
|
||||
activeChannel: Channel | null,
|
||||
resolvedMessages: RelayEvent[],
|
||||
@@ -18,16 +46,22 @@ export function useLoadMissingAncestors(
|
||||
) {
|
||||
const queryClient = useQueryClient();
|
||||
const requestedAncestorIdsRef = React.useRef<Set<string>>(new Set());
|
||||
const previousChannelIdRef = React.useRef<string | null>(null);
|
||||
const previousScopeRef = React.useRef<AncestorScope>({
|
||||
channelId: null,
|
||||
selfPubkey: undefined,
|
||||
});
|
||||
|
||||
React.useEffect(() => {
|
||||
const activeChannelId = activeChannel?.id ?? null;
|
||||
if (previousChannelIdRef.current === activeChannelId) {
|
||||
const scope: AncestorScope = {
|
||||
channelId: activeChannel?.id ?? null,
|
||||
selfPubkey,
|
||||
};
|
||||
if (!shouldResetAncestorTracking(previousScopeRef.current, scope)) {
|
||||
return;
|
||||
}
|
||||
previousChannelIdRef.current = activeChannelId;
|
||||
previousScopeRef.current = scope;
|
||||
requestedAncestorIdsRef.current.clear();
|
||||
}, [activeChannel?.id]);
|
||||
}, [activeChannel?.id, selfPubkey]);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!activeChannel || activeChannel.channelType === "forum") {
|
||||
|
||||
Reference in New Issue
Block a user