Fix Recordly 2.0 part 4 follow-up review findings

Preserve existing PR history and integrate validated current-cycle fixes.

services/recordly-share/worker/src/library.js

@@ -64,6 +64,11 @@ export async function cleanupExpired(env) {
 		"DELETE FROM comment_sessions WHERE datetime(expires_at) < datetime('now')",
 	).run();
 
+	// Match handleReport's one-hour admission window; keep reports for moderation.
+	await env.DB.prepare(
+		"UPDATE abuse_reports SET client_ip = '' WHERE client_ip <> '' AND created_at <= datetime('now', '-1 hour')",
+	).run();
+
 	const expired = await env.DB.prepare(
 		"SELECT id, share_code FROM videos WHERE never_expires = 0 AND datetime(expires_at) <= datetime('now')",
 	).all();

services/recordly-share/worker/test/api.test.js

@@ -2,6 +2,7 @@ import { describe, it, expect, beforeAll, vi } from "vitest";
 import { env, SELF } from "cloudflare:test";
 import worker, { sha256Hex } from "../src/index.js";
 import { withRecordingLock } from "../src/recordingLock.js";
+import { cleanupExpired } from "../src/library.js";
 
 import { localCommentHeaders } from "./commentFixtures.js";
 let commentAuth;
@@ -939,3 +940,128 @@ it("keeps an old live lock exclusive and recovers an orphan only by its exact ob
 			.first(),
 	).toBeNull();
 });
+
+describe("report IP cleanup", () => {
+	it("clears expired reporter IPs while preserving reports and moderation history", async () => {
+		const { shareCode } = await createShare();
+		await completeUpload(shareCode);
+		const target = crypto.randomUUID();
+		await env.DB.batch([
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','old report','198.51.100.240',datetime('now','-2 hours'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'privacy','boundary report','198.51.100.241',datetime('now','-1 hour'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'other','active report','198.51.100.242',datetime('now','-59 minutes'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','already cleared','',datetime('now','-3 hours'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO moderation_actions (action,target,reason) VALUES ('suspend-account',?,'Keep the audit record')",
+			).bind(target),
+		]);
+		const reports = await env.DB.prepare(
+			"SELECT id,share_code,reason,details,created_at FROM abuse_reports WHERE share_code = ? ORDER BY id",
+		)
+			.bind(shareCode)
+			.all();
+		const audit = await env.DB.prepare("SELECT * FROM moderation_actions WHERE target = ?")
+			.bind(target)
+			.all();
+		await env.DB.prepare(
+			"UPDATE videos SET expires_at = datetime('now','-1 day') WHERE share_code = ?",
+		)
+			.bind(shareCode)
+			.run();
+		await cleanupExpired(env);
+		expect(
+			await env.DB.prepare("SELECT id FROM videos WHERE share_code = ?")
+				.bind(shareCode)
+				.first(),
+		).toBeNull();
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT id,share_code,reason,details,created_at FROM abuse_reports WHERE share_code = ? ORDER BY id",
+				)
+					.bind(shareCode)
+					.all()
+			).results,
+		).toEqual(reports.results);
+		const addresses = await env.DB.prepare(
+			"SELECT details,client_ip FROM abuse_reports WHERE share_code = ? ORDER BY id",
+		)
+			.bind(shareCode)
+			.all();
+		expect(addresses.results).toEqual([
+			{ details: "old report", client_ip: "" },
+			{ details: "boundary report", client_ip: "" },
+			{ details: "active report", client_ip: "198.51.100.242" },
+			{ details: "already cleared", client_ip: "" },
+		]);
+		expect(
+			(
+				await env.DB.prepare("SELECT * FROM moderation_actions WHERE target = ?")
+					.bind(target)
+					.all()
+			).results,
+		).toEqual(audit.results);
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT suspended FROM account_restrictions WHERE owner_id = ?",
+				)
+					.bind(target)
+					.first()
+			).suspended,
+		).toBe(1);
+		await cleanupExpired(env);
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT details,client_ip FROM abuse_reports WHERE share_code = ? ORDER BY id",
+				)
+					.bind(shareCode)
+					.all()
+			).results,
+		).toEqual(addresses.results);
+	});
+
+	it("retains active per-IP and per-recording report admission caps after cleanup", async () => {
+		const { shareCode: first } = await createShare();
+		const { shareCode: second } = await createShare();
+		await completeUpload(first);
+		await completeUpload(second);
+		const ip = "198.51.100.244";
+		await env.DB.batch([
+			...Array.from({ length: 3 }, () =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','old',?,datetime('now','-2 hours'))",
+				).bind(first, ip),
+			),
+			...Array.from({ length: 3 }, () =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip) VALUES (?,'spam','active',?)",
+				).bind(first, ip),
+			),
+			...Array.from({ length: 20 }, (_, i) =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip) VALUES (?,'spam','active recording report',?)",
+				).bind(second, `203.0.113.${i + 1}`),
+			),
+		]);
+		await cleanupExpired(env);
+		const report = (code, clientIp) =>
+			SELF.fetch(`${BASE}/s/${code}/report`, {
+				method: "POST",
+				headers: { "Content-Type": "application/json", "CF-Connecting-IP": clientIp },
+				body: JSON.stringify({ reason: "spam", details: "New report" }),
+			});
+		expect((await report(first, ip)).status).toBe(429);
+		expect((await report(second, "198.51.100.245")).status).toBe(429);
+		expect((await report(first, "198.51.100.245")).status).toBe(200);
+	});
+});

src/dev/announcementStudioModel.ts

@@ -182,9 +182,9 @@ export function validateDraft(draft: StudioDraft): string[] {
 	if (draft.posterUrl && !safeMedia(draft.posterUrl))
 		errors.push("Poster must use HTTPS or a /public-path.");
 	if (draft.actionEnabled) {
-		const action = parseAnnouncementFeed({ announcements: [buildAnnouncement(draft)] })
-			.announcements[0]?.action;
-		if (!action)
+		const announcement = parseAnnouncementFeed({ announcements: [buildAnnouncement(draft)] })
+			.announcements[0];
+		if (announcement && !announcement.action)
 			errors.push(
 				"Use an action label (up to 80 characters) and a valid HTTPS URL or editor section.",
 			);

src/hooks/recordingFileWriter.ts

@@ -28,7 +28,8 @@ export class RecordingFileWriter {
 	}
 	write(chunk: Blob) {
 		if (this.cancelled || this.failure || !chunk.size) return;
-		if (this.pendingBytes + chunk.size > 32 * 1024 * 1024) {
+		// A single delayed recorder Blob drains in slices; bound additional queued data.
+		if (this.pendingBytes > 0 && this.pendingBytes + chunk.size > 32 * 1024 * 1024) {
 			this.fail(
 				new Error(
 					"Recording storage cannot keep up. The partial recording will be preserved.",

src/hooks/recordingMicrophoneFallback.test.ts

@@ -422,3 +422,235 @@ describe("browser microphone fallback capture", () => {
 		expect(storeMicrophoneSidecar).not.toHaveBeenCalled();
 	});
 });
+
+const microphoneHooks = vi.hoisted(() => {
+	const states: unknown[] = [];
+	const cleanups: (() => void)[] = [];
+	let writes = 0;
+	return {
+		states,
+		reset() {
+			states.length = 0;
+			cleanups.length = 0;
+			writes = 0;
+		},
+		cleanup() {
+			for (const cleanup of cleanups.splice(0)) cleanup();
+		},
+		get writes() {
+			return writes;
+		},
+		useState<T>(initial: T) {
+			const index = states.length;
+			states.push(initial);
+			return [
+				initial,
+				(value: T | ((previous: T) => T)) => {
+					writes++;
+					states[index] =
+						typeof value === "function"
+							? (value as (previous: T) => T)(states[index] as T)
+							: value;
+				},
+			] as const;
+		},
+		useEffect(effect: () => (() => void) | undefined) {
+			const cleanup = effect();
+			if (cleanup) cleanups.push(cleanup);
+		},
+	};
+});
+vi.mock("react", () => ({
+	useState: microphoneHooks.useState,
+	useEffect: microphoneHooks.useEffect,
+}));
+vi.mock("@/contexts/I18nContext", () => ({
+	useI18n: () => ({
+		t: (key: string, fallback: string, values: Record<string, string> = {}) =>
+			key === "notifications.microphone"
+				? `Micrófono ${values.id}`
+				: fallback.replace(/\{\{(\w+)\}\}/g, (_match, name: string) => values[name] ?? ""),
+	}),
+}));
+
+describe("microphone device label permission", () => {
+	let useMicrophoneDevices: typeof import("./useMicrophoneDevices").useMicrophoneDevices;
+	let enumerateDevices: ReturnType<typeof vi.fn>;
+	let getUserMedia: ReturnType<typeof vi.fn>;
+	let stop: ReturnType<typeof vi.fn>;
+	let stream: MediaStream;
+	let deviceChange: (() => void) | undefined;
+	let removeEventListener: ReturnType<typeof vi.fn>;
+	const microphone = (label = "", deviceId = "microphone-1") => ({
+		kind: "audioinput",
+		deviceId,
+		groupId: "group-1",
+		label,
+	});
+	const flush = async () => {
+		for (let step = 0; step < 10; step++) await Promise.resolve();
+	};
+	const mount = (enabled = true, preferredDeviceId?: string) => {
+		useMicrophoneDevices(enabled, preferredDeviceId);
+	};
+	beforeEach(async () => {
+		vi.resetModules();
+		microphoneHooks.reset();
+		deviceChange = undefined;
+		stop = vi.fn();
+		stream = { getTracks: () => [{ stop }] } as unknown as MediaStream;
+		enumerateDevices = vi.fn().mockResolvedValue([microphone()]);
+		getUserMedia = vi.fn().mockResolvedValue(stream);
+		removeEventListener = vi.fn();
+		vi.stubGlobal("navigator", {
+			mediaDevices: {
+				enumerateDevices,
+				getUserMedia,
+				addEventListener: (_event: string, listener: () => void) => {
+					deviceChange = listener;
+				},
+				removeEventListener,
+			},
+		});
+		vi.spyOn(console, "error").mockImplementation(() => {});
+		({ useMicrophoneDevices } = await import("./useMicrophoneDevices"));
+	});
+	afterEach(() => {
+		microphoneHooks.cleanup();
+		vi.restoreAllMocks();
+		vi.unstubAllGlobals();
+	});
+
+	it("requests hidden microphone labels from raw devices and releases the permission stream", async () => {
+		enumerateDevices
+			.mockResolvedValueOnce([
+				microphone(),
+				{ kind: "videoinput", deviceId: "camera", label: "Camera" },
+			])
+			.mockResolvedValueOnce([microphone("Desk microphone")]);
+		mount(true, "microphone-1");
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledExactlyOnceWith({ audio: true });
+		expect(enumerateDevices).toHaveBeenCalledTimes(2);
+		expect(microphoneHooks.states[0]).toEqual(
+			[microphone("Desk microphone")].map(({ kind: _kind, ...device }) => device),
+		);
+		expect(microphoneHooks.states[1]).toBe("microphone-1");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("retains localized display fallbacks if permission still leaves labels hidden", async () => {
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[0]).toEqual([
+			{ deviceId: "microphone-1", groupId: "group-1", label: "Micrófono micropho" },
+		]);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it.each([
+		["labeled", [microphone("Desk")]],
+		["mixed", [microphone("Desk"), microphone("", "second")]],
+		["no audio", [{ kind: "videoinput", deviceId: "camera", label: "" }]],
+	])("does not request label permission for %s inputs", async (_label, devices) => {
+		enumerateDevices.mockResolvedValue(devices);
+		mount();
+		await flush();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(stop).not.toHaveBeenCalled();
+		if (_label === "mixed") {
+			expect(microphoneHooks.states[0]).toContainEqual({
+				deviceId: "second",
+				groupId: "group-1",
+				label: "Micrófono second",
+			});
+		}
+	});
+
+	it("treats whitespace-only raw labels as hidden", async () => {
+		enumerateDevices.mockResolvedValue([microphone("   ")]);
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("does not enumerate or request permission while disabled", async () => {
+		mount(false);
+		await flush();
+		expect(enumerateDevices).not.toHaveBeenCalled();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(deviceChange).toBeUndefined();
+	});
+
+	it("reports permission denial and does not prompt again on device changes", async () => {
+		getUserMedia.mockRejectedValue(new Error("Microphone permission denied"));
+		mount();
+		await flush();
+		expect(microphoneHooks.states[3]).toBe("Microphone permission denied");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).not.toHaveBeenCalled();
+		deviceChange?.();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[3]).toBeNull();
+	});
+
+	it("releases the permission stream when follow-up enumeration rejects", async () => {
+		enumerateDevices
+			.mockResolvedValueOnce([microphone()])
+			.mockRejectedValueOnce(new Error("Enumeration failed"));
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[3]).toBe("Enumeration failed");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("does not request permission after pending enumeration is unmounted", async () => {
+		let resolve!: (devices: ReturnType<typeof microphone>[]) => void;
+		enumerateDevices.mockReturnValueOnce(
+			new Promise((done) => {
+				resolve = done;
+			}),
+		);
+		mount();
+		microphoneHooks.cleanup();
+		const writes = microphoneHooks.writes;
+		resolve([microphone()]);
+		await flush();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(microphoneHooks.writes).toBe(writes);
+		expect(removeEventListener).toHaveBeenCalledWith("devicechange", deviceChange);
+	});
+
+	it("releases a late permission stream without state writes after unmount", async () => {
+		let resolve!: (stream: MediaStream) => void;
+		getUserMedia.mockReturnValueOnce(
+			new Promise((done) => {
+				resolve = done;
+			}),
+		);
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		microphoneHooks.cleanup();
+		const writes = microphoneHooks.writes;
+		resolve(stream);
+		await flush();
+		expect(stop).toHaveBeenCalledOnce();
+		expect(microphoneHooks.writes).toBe(writes);
+	});
+
+	it("requests label permission only once per session after success", async () => {
+		mount();
+		await flush();
+		deviceChange?.();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(stop).toHaveBeenCalledOnce();
+	});
+});

src/hooks/useMicrophoneDevices.ts

@@ -31,22 +31,22 @@ export function useMicrophoneDevices(enabled: boolean = true, preferredDeviceId?
 				setError(null);
 
 				let allDevices = await navigator.mediaDevices.enumerateDevices();
-				let audioInputs = allDevices
-					.filter((device) => device.kind === "audioinput")
-					.map((device) => ({
-						deviceId: device.deviceId,
-						label:
-							device.label ||
-							t("notifications.microphone", "Microphone {{id}}", {
-								id: device.deviceId.slice(0, 8),
-							}),
-						groupId: device.groupId,
-					}));
+				const rawAudioInputs = allDevices.filter((device) => device.kind === "audioinput");
+				let audioInputs = rawAudioInputs.map((device) => ({
+					deviceId: device.deviceId,
+					label:
+						device.label ||
+						t("notifications.microphone", "Microphone {{id}}", {
+							id: device.deviceId.slice(0, 8),
+						}),
+					groupId: device.groupId,
+				}));
 
 				const needsLabelPermission =
-					audioInputs.length > 0 && audioInputs.every((device) => !device.label.trim());
+					rawAudioInputs.length > 0 &&
+					rawAudioInputs.every((device) => !device.label.trim());
 
-				if (needsLabelPermission && !hasRequestedMicrophoneLabels) {
+				if (mounted && needsLabelPermission && !hasRequestedMicrophoneLabels) {
 					hasRequestedMicrophoneLabels = true;
 					permissionStream = await navigator.mediaDevices.getUserMedia({ audio: true });
 					allDevices = await navigator.mediaDevices.enumerateDevices();