feat(r2): delete the underlying storage objects when a video is deleted

Deleting a video removed the Nostr note but left the actual file (plus its
thumbnail and .torrent) publicly downloadable at its R2/S3 URL forever — a
privacy gap and ongoing storage cost. Now the delete flow also removes the
backing objects, best-effort.

- collectVideoStorageKeys() (storagePointer.js): pure helper that derives the
  object keys (video + sibling .torrent + thumbnail) from a note, ONLY for URLs
  under the owner's bucket base — external/3rd-party URLs are never returned.
- R2Service.deleteVideoStorage(): resolves the owner's connection, and deletes
  those keys. Skips cleanly when storage is locked / no creds / no owned
  objects, and never throws (failures are recorded, not propagated) so storage
  cleanup can't block note deletion. R2Service class now exported for testing.
- app.handleDeleteModalConfirm: calls it after handleFullDeleteVideo succeeds.

Tests: tests/storage-key-collection.test.mjs (4),
tests/services/r2-delete-video-storage.test.mjs (4). Full suite green.

Note: superseded versions orphaned by past edits aren't retroactively cleaned;
edit-time cleanup (next) prevents new orphans going forward.

Co-Authored-By: Claude Opus 4.8 <[email protected]>

js/app.js

@@ -4538,6 +4538,28 @@ class Application {
         confirm: false,
       });
 
+      // Best-effort: remove the underlying R2/S3 object(s) now that the note(s)
+      // are gone, so the file isn't left publicly downloadable. This never
+      // blocks the delete — failures are logged only.
+      try {
+        const cleanup = await r2Service.deleteVideoStorage({
+          videos: [targetVideo],
+          pubkey: this.pubkey,
+        });
+        if (cleanup?.deleted?.length) {
+          devLogger.log(
+            `[delete] Removed ${cleanup.deleted.length} storage object(s) for the deleted video.`
+          );
+        } else if (cleanup?.skipped) {
+          devLogger.log(`[delete] Storage cleanup skipped: ${cleanup.reason}.`);
+        }
+      } catch (cleanupErr) {
+        devLogger.warn(
+          "[delete] Storage cleanup failed (note deletion still succeeded):",
+          cleanupErr
+        );
+      }
+
       await this.loadVideos();
       this.showSuccess("All versions deleted successfully!");
       this.deleteModal.close();

js/services/r2Service.js

@@ -42,7 +42,9 @@ import { userLogger, devLogger } from "../utils/logger.js";
 import {
   buildStoragePointerValue,
   buildStoragePrefixFromKey,
+  collectVideoStorageKeys,
 } from "../utils/storagePointer.js";
+import { safeEncodeNpub } from "../utils/nostrHelpers.js";
 import {
   getVideoNoteErrorMessage,
   normalizeVideoNotePayload,
@@ -139,7 +141,7 @@ function safeDecodeNpub(npub) {
   return null;
 }
 
-class R2Service {
+export class R2Service {
   constructor({
     makeR2Client: makeR2ClientOverride,
     multipartUpload: multipartUploadOverride,
@@ -156,6 +158,117 @@ class R2Service {
     this.deleteObject = deleteObjectOverride || deleteObject;
   }
 
+  /**
+   * Best-effort deletion of the R2/S3 objects backing one or more video notes
+   * (the video file, its sibling `.torrent`, and the thumbnail). Used so that
+   * deleting/superseding a video also removes its bytes from storage instead of
+   * leaving them publicly downloadable forever.
+   *
+   * Safety: only objects whose URL lives under the owner's configured bucket
+   * public base URL are touched (external links are ignored), and the method
+   * NEVER throws — storage cleanup must not block note deletion. Returns a
+   * summary describing what happened.
+   *
+   * @param {object} params
+   * @param {Array|object} params.videos - parsed video note(s) with url/thumbnail
+   * @param {string} [params.npub] - owner npub (preferred)
+   * @param {string} [params.pubkey] - owner hex pubkey (encoded to npub if npub absent)
+   * @param {object} [params.credentials] - pre-resolved connection settings
+   * @returns {Promise<{deleted:string[],failed:Array,skipped:boolean,reason:string}>}
+   */
+  async deleteVideoStorage({
+    videos = [],
+    npub = "",
+    pubkey = "",
+    credentials = null,
+  } = {}) {
+    const list = (Array.isArray(videos) ? videos : [videos]).filter(
+      (entry) => entry && typeof entry === "object"
+    );
+    const summary = { deleted: [], failed: [], skipped: false, reason: "" };
+    const skip = (reason) => {
+      summary.skipped = true;
+      summary.reason = reason;
+      return summary;
+    };
+
+    if (!list.length) {
+      return skip("no-videos");
+    }
+
+    let resolvedNpub = (npub || "").trim();
+    if (!resolvedNpub && pubkey) {
+      resolvedNpub = safeEncodeNpub(pubkey) || "";
+    }
+
+    let settings = credentials;
+    if (!settings && resolvedNpub) {
+      try {
+        settings = await this.resolveConnection(resolvedNpub);
+      } catch (err) {
+        devLogger.warn(
+          "[R2Service] deleteVideoStorage: failed to resolve connection:",
+          err
+        );
+      }
+    }
+    if (!settings) {
+      return skip("no-connection");
+    }
+
+    const accessKeyId = (settings.accessKeyId || "").trim();
+    const secretAccessKey = (settings.secretAccessKey || "").trim();
+    const bucket = (settings.bucket || "").trim();
+    const publicBaseUrl = (
+      settings.publicBaseUrl ||
+      settings.baseDomain ||
+      ""
+    ).trim();
+
+    if (!accessKeyId || !secretAccessKey) {
+      // Storage is locked / keys unavailable — can't delete without them.
+      return skip("storage-locked");
+    }
+    if (!bucket || !publicBaseUrl) {
+      return skip("missing-bucket-or-base-url");
+    }
+
+    const keys = collectVideoStorageKeys({ videos: list, publicBaseUrl });
+    if (!keys.length) {
+      return skip("no-matching-objects");
+    }
+
+    let s3;
+    try {
+      await ensureS3SdkLoaded();
+      s3 = this.makeR2Client({
+        accountId: settings.accountId,
+        endpoint: settings.endpoint,
+        accessKeyId,
+        secretAccessKey,
+        region: settings.region,
+      });
+    } catch (err) {
+      devLogger.warn(
+        "[R2Service] deleteVideoStorage: failed to build storage client:",
+        err
+      );
+      return skip("client-error");
+    }
+
+    for (const key of keys) {
+      try {
+        // deleteObject already treats 404/NoSuchKey as success.
+        await this.deleteObject({ s3, bucket, key });
+        summary.deleted.push(key);
+      } catch (err) {
+        summary.failed.push({ key, error: err?.message || String(err) });
+        devLogger.warn(`[R2Service] Failed to delete storage object ${key}:`, err);
+      }
+    }
+    return summary;
+  }
+
   on(event, handler) {
     if (typeof handler !== "function") {
       return () => {};

js/utils/storagePointer.js

@@ -155,3 +155,65 @@ export function resolveStoragePointerValue({
 
   return "";
 }
+
+/**
+ * Given one or more parsed video notes and the owner's bucket public base URL,
+ * returns the set of object KEYS backing them — the video object, its sibling
+ * `.torrent`, and the thumbnail — but ONLY for URLs that live under that base
+ * URL. URLs pointing at other hosts/buckets (e.g. external links) are ignored,
+ * so callers can never delete an object they don't own. Pure + deterministic.
+ *
+ * @param {object} params
+ * @param {Array|object} params.videos - parsed video note(s) (need url/thumbnail)
+ * @param {string} params.publicBaseUrl - the bucket's public base URL
+ * @returns {string[]} unique object keys (decoded, no leading slash)
+ */
+export function collectVideoStorageKeys({ videos, publicBaseUrl } = {}) {
+  const base = safeTrim(publicBaseUrl).replace(/\/+$/, "");
+  if (!base) {
+    return [];
+  }
+  const prefix = `${base}/`;
+
+  const keyFromUrl = (value) => {
+    const url = safeTrim(value);
+    if (!url || !url.startsWith(prefix)) {
+      return "";
+    }
+    let rel = url.slice(prefix.length).split(/[?#]/)[0];
+    if (!rel) {
+      return "";
+    }
+    try {
+      rel = rel
+        .split("/")
+        .map((segment) => decodeURIComponent(segment))
+        .join("/");
+    } catch (error) {
+      // Keep the raw relative path if it isn't valid percent-encoding.
+    }
+    return rel.replace(/^\/+/, "");
+  };
+
+  const list = Array.isArray(videos) ? videos : [videos];
+  const keys = new Set();
+  for (const video of list) {
+    if (!video || typeof video !== "object") {
+      continue;
+    }
+    const videoKey = keyFromUrl(video.url);
+    if (videoKey) {
+      keys.add(videoKey);
+      // The sibling torrent is uploaded as "<key-without-extension>.torrent".
+      const torrentKey = `${videoKey.replace(/\.[^/.]+$/, "")}.torrent`;
+      if (torrentKey && torrentKey !== videoKey) {
+        keys.add(torrentKey);
+      }
+    }
+    const thumbnailKey = keyFromUrl(video.thumbnail);
+    if (thumbnailKey) {
+      keys.add(thumbnailKey);
+    }
+  }
+  return [...keys];
+}

tests/services/r2-delete-video-storage.test.mjs

@@ -0,0 +1,94 @@
+// Scenario (SCN-delete-video-storage):
+//   When a video is deleted, its R2/S3 objects must be removed so the file isn't
+//   left publicly downloadable (Cloudflare R2 cleanup). The method must be
+//   best-effort: delete owned objects, skip when storage is locked / has no
+//   creds, never touch external URLs, and never throw (a failed object delete is
+//   recorded, not propagated).
+//
+// Anti-cheat: drives the real deleteVideoStorage with I/O stubbed (resolveConnection,
+// makeR2Client, deleteObject) and asserts which keys were actually deleted.
+
+import "../test-helpers/setup-localstorage.mjs";
+import test from "node:test";
+import assert from "node:assert/strict";
+
+// Disable the AWS SDK network import before the storage modules load.
+globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = true;
+const { R2Service } = await import("../../js/services/r2Service.js");
+
+const BASE = "https://pub.bitvid.network";
+const CREDS = {
+  accessKeyId: "AK",
+  secretAccessKey: "SK",
+  bucket: "my-bucket",
+  publicBaseUrl: BASE,
+  endpoint: "https://acct.r2.cloudflarestorage.com",
+  region: "auto",
+};
+
+function buildService(creds, deleteImpl) {
+  const deleted = [];
+  const svc = new R2Service({
+    makeR2Client: () => ({ __mock: true }),
+    deleteObject:
+      deleteImpl ||
+      (async ({ key }) => {
+        deleted.push(key);
+      }),
+  });
+  svc.resolveConnection = async () => creds;
+  return { svc, deleted };
+}
+
+test("deletes the video, its .torrent, and the thumbnail for an owned video", async () => {
+  const { svc, deleted } = buildService(CREDS);
+  const res = await svc.deleteVideoStorage({
+    npub: "npub1owner",
+    videos: [
+      { url: `${BASE}/u/np/h/clip.mp4`, thumbnail: `${BASE}/u/np/h/clip.thumb.jpg` },
+    ],
+  });
+  assert.equal(res.skipped, false);
+  assert.deepEqual(
+    new Set(deleted),
+    new Set(["u/np/h/clip.mp4", "u/np/h/clip.torrent", "u/np/h/clip.thumb.jpg"]),
+  );
+  assert.equal(res.deleted.length, 3);
+});
+
+test("skips entirely when storage is locked (no credentials available)", async () => {
+  const { svc, deleted } = buildService({ ...CREDS, accessKeyId: "", secretAccessKey: "" });
+  const res = await svc.deleteVideoStorage({
+    npub: "npub1owner",
+    videos: [{ url: `${BASE}/u/np/h/clip.mp4` }],
+  });
+  assert.equal(res.skipped, true);
+  assert.equal(res.reason, "storage-locked");
+  assert.equal(deleted.length, 0);
+});
+
+test("never deletes external/3rd-party objects", async () => {
+  const { svc, deleted } = buildService(CREDS);
+  const res = await svc.deleteVideoStorage({
+    npub: "npub1owner",
+    videos: [{ url: "https://youtube.com/watch?v=x" }],
+  });
+  assert.equal(deleted.length, 0);
+  assert.equal(res.skipped, true);
+  assert.equal(res.reason, "no-matching-objects");
+});
+
+test("records a failed object delete without throwing", async () => {
+  const deleted = [];
+  const { svc } = buildService(CREDS, async ({ key }) => {
+    if (key.endsWith(".torrent")) throw new Error("boom");
+    deleted.push(key);
+  });
+  const res = await svc.deleteVideoStorage({
+    npub: "npub1owner",
+    videos: [{ url: `${BASE}/u/np/h/clip.mp4` }],
+  });
+  assert.equal(res.failed.length, 1);
+  assert.ok(res.failed[0].key.endsWith(".torrent"));
+  assert.ok(res.deleted.includes("u/np/h/clip.mp4"));
+});

tests/storage-key-collection.test.mjs

@@ -0,0 +1,63 @@
+// Scenario (SCN-collect-video-storage-keys):
+//   Deriving the object keys backing a video note for cleanup must (a) cover the
+//   video, its sibling .torrent, and the thumbnail, (b) decode percent-encoded
+//   paths, and (c) NEVER include URLs outside the owner's bucket base (so we
+//   can't delete objects we don't own). Pure + deterministic.
+
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { collectVideoStorageKeys } from "../js/utils/storagePointer.js";
+
+const BASE = "https://pub.bitvid.network";
+
+test("collects video + .torrent + thumbnail keys for an owned video", () => {
+  const keys = collectVideoStorageKeys({
+    videos: [
+      {
+        url: `${BASE}/u/np/abc123/my-clip.mp4`,
+        thumbnail: `${BASE}/u/np/abc123/my-clip.thumb.jpg`,
+      },
+    ],
+    publicBaseUrl: BASE,
+  });
+  assert.deepEqual(
+    new Set(keys),
+    new Set([
+      "u/np/abc123/my-clip.mp4",
+      "u/np/abc123/my-clip.torrent",
+      "u/np/abc123/my-clip.thumb.jpg",
+    ]),
+  );
+});
+
+test("ignores URLs outside the bucket base (never deletes others' objects)", () => {
+  const keys = collectVideoStorageKeys({
+    videos: [
+      {
+        url: "https://youtube.com/watch?v=abc",
+        thumbnail: `${BASE}/u/np/h/t.thumb.jpg`, // this one IS ours
+      },
+    ],
+    publicBaseUrl: BASE,
+  });
+  assert.ok(!keys.some((k) => k.includes("youtube")), "no external host keys");
+  assert.deepEqual(new Set(keys), new Set(["u/np/h/t.thumb.jpg"]));
+});
+
+test("decodes percent-encoded path segments and tolerates a trailing-slash base", () => {
+  const keys = collectVideoStorageKeys({
+    videos: [{ url: `${BASE}/u/np/h/my%20clip.mp4` }],
+    publicBaseUrl: `${BASE}/`,
+  });
+  assert.ok(keys.includes("u/np/h/my clip.mp4"), `got ${JSON.stringify(keys)}`);
+  assert.ok(keys.includes("u/np/h/my clip.torrent"));
+});
+
+test("returns nothing when no base url or no owned objects", () => {
+  assert.deepEqual(collectVideoStorageKeys({ videos: [{ url: `${BASE}/x.mp4` }], publicBaseUrl: "" }), []);
+  assert.deepEqual(
+    collectVideoStorageKeys({ videos: [{ url: "https://other.example/x.mp4" }], publicBaseUrl: BASE }),
+    [],
+  );
+});