From 7039a173b44998b59cc241db40345b824c7aeade Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Sun, 8 Nov 2026 12:06:42 +1100 Subject: [PATCH] Fix Recordly 2.0 part 3 follow-up review findings Preserve existing PR history and integrate validated current-cycle fixes. --- services/recordly-share/worker/src/library.js | 5 + .../recordly-share/worker/test/api.test.js | 126 ++++++++++++++++++ src/dev/announcementStudioModel.ts | 6 +- src/hooks/recordingFileWriter.ts | 3 +- src/hooks/useMicrophoneDevices.ts | 26 ++-- 5 files changed, 149 insertions(+), 17 deletions(-) diff --git a/services/recordly-share/worker/src/library.js b/services/recordly-share/worker/src/library.js index 227a56ed..15496f7a 100644 --- a/services/recordly-share/worker/src/library.js +++ b/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(); diff --git a/services/recordly-share/worker/test/api.test.js b/services/recordly-share/worker/test/api.test.js index 5c02a32f..53064c81 100644 --- a/services/recordly-share/worker/test/api.test.js +++ b/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); + }); +}); diff --git a/src/dev/announcementStudioModel.ts b/src/dev/announcementStudioModel.ts index a6929ab8..90d7e4dd 100644 --- a/src/dev/announcementStudioModel.ts +++ b/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.", ); diff --git a/src/hooks/recordingFileWriter.ts b/src/hooks/recordingFileWriter.ts index 649664f1..1a7769cc 100644 --- a/src/hooks/recordingFileWriter.ts +++ b/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.", diff --git a/src/hooks/useMicrophoneDevices.ts b/src/hooks/useMicrophoneDevices.ts index 6338d75d..257d77e8 100644 --- a/src/hooks/useMicrophoneDevices.ts +++ b/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();