mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): harden reminder decode against spec-valid and malformed input
RemindersPanel dereffed content.target unconditionally and decryptReminder trusted JSON.parse output as ReminderContent with no validation. A note-only reminder (no target — explicitly valid per NIP-ER) or malformed self-decrypted plaintext crashed the 60s due-check loop or rendered fake reminders. Make target optional, validate decrypted content fail-closed (object shape, known status, target/note structure), and guard every target deref with a note fallback. Mirror the relay's strict not_before parser so the client ignores values the relay rejects, and use 16 random bytes for the d-tag to meet the spec's 128-bit entropy MUST (randomUUID is only 122 bits). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
c152d7f8bf
commit
dbb0489639
@@ -0,0 +1,141 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { parseNotBefore, parseReminderContent } from "./reminderService.ts";
|
||||
|
||||
const VALID_TARGET = {
|
||||
eventId: "abc123",
|
||||
channelId: "chan1",
|
||||
preview: "hello world",
|
||||
authorPubkey: "pk1",
|
||||
};
|
||||
|
||||
function content(overrides = {}) {
|
||||
return JSON.stringify({
|
||||
target: VALID_TARGET,
|
||||
status: "pending",
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
test("parseReminderContent_valid_target_reminder_returns_content", () => {
|
||||
const result = parseReminderContent(content());
|
||||
assert.deepEqual(result, {
|
||||
status: "pending",
|
||||
target: VALID_TARGET,
|
||||
note: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
test("parseReminderContent_note_only_reminder_returns_content", () => {
|
||||
const result = parseReminderContent(
|
||||
JSON.stringify({ status: "pending", note: "buy milk" }),
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
status: "pending",
|
||||
target: undefined,
|
||||
note: "buy milk",
|
||||
});
|
||||
});
|
||||
|
||||
test("parseReminderContent_done_and_cancelled_statuses_accepted", () => {
|
||||
assert.equal(
|
||||
parseReminderContent(content({ status: "done" }))?.status,
|
||||
"done",
|
||||
);
|
||||
assert.equal(
|
||||
parseReminderContent(content({ status: "cancelled" }))?.status,
|
||||
"cancelled",
|
||||
);
|
||||
});
|
||||
|
||||
test("parseReminderContent_invalid_json_returns_null", () => {
|
||||
assert.equal(parseReminderContent("not json {"), null);
|
||||
});
|
||||
|
||||
test("parseReminderContent_non_object_returns_null", () => {
|
||||
assert.equal(parseReminderContent("42"), null);
|
||||
assert.equal(parseReminderContent('"a string"'), null);
|
||||
assert.equal(parseReminderContent("[1,2,3]"), null);
|
||||
assert.equal(parseReminderContent("null"), null);
|
||||
});
|
||||
|
||||
test("parseReminderContent_unknown_status_returns_null", () => {
|
||||
assert.equal(parseReminderContent(content({ status: "bogus" })), null);
|
||||
assert.equal(
|
||||
parseReminderContent(JSON.stringify({ target: VALID_TARGET })),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("parseReminderContent_neither_target_nor_note_returns_null", () => {
|
||||
assert.equal(
|
||||
parseReminderContent(JSON.stringify({ status: "pending" })),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("parseReminderContent_empty_note_without_target_returns_null", () => {
|
||||
assert.equal(
|
||||
parseReminderContent(JSON.stringify({ status: "pending", note: "" })),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("parseReminderContent_non_string_note_returns_null", () => {
|
||||
assert.equal(parseReminderContent(content({ note: 5 })), null);
|
||||
});
|
||||
|
||||
test("parseReminderContent_malformed_target_returns_null", () => {
|
||||
assert.equal(
|
||||
parseReminderContent(JSON.stringify({ status: "pending", target: {} })),
|
||||
null,
|
||||
);
|
||||
assert.equal(
|
||||
parseReminderContent(
|
||||
JSON.stringify({ status: "pending", target: "string" }),
|
||||
),
|
||||
null,
|
||||
);
|
||||
assert.equal(
|
||||
parseReminderContent(
|
||||
JSON.stringify({
|
||||
status: "pending",
|
||||
target: { ...VALID_TARGET, preview: 7 },
|
||||
}),
|
||||
),
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
test("parseReminderContent_unknown_fields_are_ignored", () => {
|
||||
const result = parseReminderContent(content({ extra: "ignored" }));
|
||||
assert.deepEqual(result, {
|
||||
status: "pending",
|
||||
target: VALID_TARGET,
|
||||
note: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
test("parseNotBefore_valid_digits_returns_number", () => {
|
||||
assert.equal(parseNotBefore("0"), 0);
|
||||
assert.equal(parseNotBefore("1700000000"), 1_700_000_000);
|
||||
});
|
||||
|
||||
test("parseNotBefore_leading_zero_returns_undefined", () => {
|
||||
assert.equal(parseNotBefore("007"), undefined);
|
||||
assert.equal(parseNotBefore("01"), undefined);
|
||||
});
|
||||
|
||||
test("parseNotBefore_non_digit_returns_undefined", () => {
|
||||
assert.equal(parseNotBefore("123abc"), undefined);
|
||||
assert.equal(parseNotBefore("12.5"), undefined);
|
||||
assert.equal(parseNotBefore("-5"), undefined);
|
||||
assert.equal(parseNotBefore(" 5"), undefined);
|
||||
assert.equal(parseNotBefore(""), undefined);
|
||||
});
|
||||
|
||||
test("parseNotBefore_above_max_safe_integer_returns_undefined", () => {
|
||||
assert.equal(parseNotBefore("9007199254740991"), 9_007_199_254_740_991);
|
||||
assert.equal(parseNotBefore("9007199254740992"), undefined);
|
||||
});
|
||||
@@ -23,31 +23,122 @@ function extractDTag(event: RelayEvent): string | null {
|
||||
return tag?.[1] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a reminder `d`-tag with 128 bits of entropy (NIP-ER line 58 MUST).
|
||||
* `crypto.randomUUID()` is UUIDv4 = only 122 random bits, so use 16 raw bytes.
|
||||
*/
|
||||
function randomDTag(): string {
|
||||
const bytes = crypto.getRandomValues(new Uint8Array(16));
|
||||
return Array.from(bytes, (b) => b.toString(16).padStart(2, "0")).join("");
|
||||
}
|
||||
|
||||
function extractNotBefore(event: RelayEvent): number | undefined {
|
||||
const tag = event.tags.find((t) => t[0] === "not_before");
|
||||
if (!tag?.[1]) return undefined;
|
||||
const val = Number.parseInt(tag[1], 10);
|
||||
return Number.isNaN(val) ? undefined : val;
|
||||
return tag?.[1] ? parseNotBefore(tag[1]) : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a NIP-ER `not_before` tag value, mirroring the relay's strict
|
||||
* validator (NIP-ER line 60): ASCII digits only, no leading zero except "0",
|
||||
* and within `Number.MAX_SAFE_INTEGER`. Returns undefined for any value the
|
||||
* relay would reject, so the client ignores reminders the relay calls malformed.
|
||||
*/
|
||||
export function parseNotBefore(raw: string): number | undefined {
|
||||
if (!/^(0|[1-9][0-9]*)$/.test(raw)) return undefined;
|
||||
const val = Number(raw);
|
||||
return val <= Number.MAX_SAFE_INTEGER ? val : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate decrypted reminder plaintext against the shape this client writes,
|
||||
* returning a typed content object or null. NIP-ER (Content section) requires
|
||||
* clients to ignore plaintext that is not a JSON object, has an unknown
|
||||
* `status`, or has a malformed target/note — so anything off-shape fails closed.
|
||||
*/
|
||||
export function parseReminderContent(
|
||||
plaintext: string,
|
||||
): ReminderContent | null {
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(plaintext);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const obj = parsed as Record<string, unknown>;
|
||||
if (
|
||||
obj.status !== "pending" &&
|
||||
obj.status !== "done" &&
|
||||
obj.status !== "cancelled"
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (obj.note !== undefined && typeof obj.note !== "string") return null;
|
||||
|
||||
let target: ReminderTarget | undefined;
|
||||
if (obj.target !== undefined) {
|
||||
const parsedTarget = parseTarget(obj.target);
|
||||
if (!parsedTarget) return null;
|
||||
target = parsedTarget;
|
||||
}
|
||||
|
||||
// A reminder must reference a target or carry a non-empty note.
|
||||
if (!target && !(typeof obj.note === "string" && obj.note.length > 0)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return { status: obj.status, target, note: obj.note as string | undefined };
|
||||
}
|
||||
|
||||
function parseTarget(value: unknown): ReminderTarget | null {
|
||||
if (typeof value !== "object" || value === null || Array.isArray(value)) {
|
||||
return null;
|
||||
}
|
||||
const t = value as Record<string, unknown>;
|
||||
if (
|
||||
typeof t.eventId !== "string" ||
|
||||
typeof t.channelId !== "string" ||
|
||||
typeof t.preview !== "string" ||
|
||||
typeof t.authorPubkey !== "string"
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
eventId: t.eventId,
|
||||
channelId: t.channelId,
|
||||
preview: t.preview,
|
||||
authorPubkey: t.authorPubkey,
|
||||
};
|
||||
}
|
||||
|
||||
async function decryptReminder(event: RelayEvent): Promise<Reminder | null> {
|
||||
const dTag = extractDTag(event);
|
||||
if (!dTag) return null;
|
||||
|
||||
let plaintext: string;
|
||||
try {
|
||||
const plaintext = await nip44DecryptFromSelf(event.content);
|
||||
const content = JSON.parse(plaintext) as ReminderContent;
|
||||
return {
|
||||
id: dTag,
|
||||
notBefore: extractNotBefore(event),
|
||||
content,
|
||||
createdAt: event.created_at,
|
||||
eventId: event.id,
|
||||
};
|
||||
plaintext = await nip44DecryptFromSelf(event.content);
|
||||
} catch {
|
||||
console.warn("[reminderService] failed to decrypt reminder:", event.id);
|
||||
return null;
|
||||
}
|
||||
|
||||
const content = parseReminderContent(plaintext);
|
||||
if (!content) {
|
||||
console.warn("[reminderService] ignoring malformed reminder:", event.id);
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
id: dTag,
|
||||
notBefore: extractNotBefore(event),
|
||||
content,
|
||||
createdAt: event.created_at,
|
||||
eventId: event.id,
|
||||
};
|
||||
}
|
||||
|
||||
export async function fetchReminders(pubkey: string): Promise<Reminder[]> {
|
||||
@@ -66,7 +157,7 @@ export async function createReminder(
|
||||
notBefore: number,
|
||||
note?: string,
|
||||
): Promise<RelayEvent> {
|
||||
const dTag = crypto.randomUUID();
|
||||
const dTag = randomDTag();
|
||||
const content: ReminderContent = {
|
||||
target,
|
||||
note,
|
||||
|
||||
@@ -12,7 +12,8 @@ export type ReminderTarget = {
|
||||
};
|
||||
|
||||
export type ReminderContent = {
|
||||
target: ReminderTarget;
|
||||
/** Target message. Absent for note-only reminders (NIP-ER allows either). */
|
||||
target?: ReminderTarget;
|
||||
/** Optional user-provided note. */
|
||||
note?: string;
|
||||
status: ReminderStatus;
|
||||
|
||||
@@ -123,9 +123,11 @@ function ReminderRow({
|
||||
<div className="flex items-start gap-3 rounded-md border p-3">
|
||||
<div className="flex-1 min-w-0">
|
||||
<p className="text-sm font-medium truncate">
|
||||
{reminder.content.target.preview || "Message"}
|
||||
{reminder.content.target?.preview ||
|
||||
reminder.content.note ||
|
||||
"Reminder"}
|
||||
</p>
|
||||
{reminder.content.note ? (
|
||||
{reminder.content.target && reminder.content.note ? (
|
||||
<p className="text-xs text-muted-foreground mt-0.5 truncate">
|
||||
{reminder.content.note}
|
||||
</p>
|
||||
@@ -207,7 +209,10 @@ export function RemindersPanel({ pubkey }: { pubkey: string }) {
|
||||
if (!r.notBefore) continue;
|
||||
if (r.notBefore > lastCheck && r.notBefore <= now) {
|
||||
toast("Reminder due", {
|
||||
description: r.content.target.preview || "A reminder is waiting",
|
||||
description:
|
||||
r.content.target?.preview ||
|
||||
r.content.note ||
|
||||
"A reminder is waiting",
|
||||
icon: <Bell className="h-4 w-4" />,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user