From f0eecec09d5319ff6cd3a8d7c210cbe03c4bf0fb Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sat, 20 Jun 2026 12:21:49 -0400 Subject: [PATCH] feat(delete): warn when a hosted file is left behind by locked storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a video removes the Nostr note(s) but cleanup of the underlying R2/S3 object was silently skipped when storage was locked — leaving the file publicly downloadable with no indication to the user. Now the delete flow surfaces a clear heads-up when that happens, telling the user to unlock storage and delete again (or remove the object from their bucket manually). To make the signal accurate, deleteVideoStorage now resolves the affected object keys BEFORE the credentials check (publicBaseUrl is available even while locked). So "storage-locked" is reported only when there are genuinely hosted objects to remove; an external-URL video (nothing on the user's bucket) returns "no-matching-objects" and never triggers a false warning. Tests (deleteVideoStorage via injected credentials/deleteObject): locked + hosted object -> storage-locked; locked + external URL -> no-matching-objects (no false alarm); unlocked -> actually deletes the object. Mutation-verified — restoring the old credentials-first ordering misreports the external-URL case. Build + full lint green. Co-Authored-By: Claude Opus 4.8 --- js/app.js | 13 +++++ js/services/r2Service.js | 22 ++++++-- tests/delete-storage-locked.test.mjs | 78 ++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 tests/delete-storage-locked.test.mjs diff --git a/js/app.js b/js/app.js index 084be04d..0fd7a10a 100644 --- a/js/app.js +++ b/js/app.js @@ -4559,6 +4559,7 @@ class Application { // 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. + let storageLockedRemnant = false; try { const cleanup = await r2Service.deleteVideoStorage({ videos: [targetVideo], @@ -4570,6 +4571,12 @@ class Application { ); } else if (cleanup?.skipped) { devLogger.log(`[delete] Storage cleanup skipped: ${cleanup.reason}.`); + // The note is gone but the hosted file is still publicly downloadable + // because storage is locked. Warn the user so they can unlock and + // delete again (or remove it from their bucket manually). + if (cleanup.reason === "storage-locked") { + storageLockedRemnant = true; + } } } catch (cleanupErr) { devLogger.warn( @@ -4580,6 +4587,12 @@ class Application { await this.loadVideos(); this.showSuccess("All versions deleted successfully!"); + if (storageLockedRemnant) { + this.showStatus( + "Heads up: the hosted file is still in your storage bucket because storage is locked. Unlock storage and delete again, or remove it from your bucket manually.", + { autoHideMs: 12000, showSpinner: false } + ); + } this.deleteModal.close(); this.forceRefreshAllProfiles(); } catch (err) { diff --git a/js/services/r2Service.js b/js/services/r2Service.js index 370da7b6..b81e8054 100644 --- a/js/services/r2Service.js +++ b/js/services/r2Service.js @@ -154,19 +154,31 @@ export class R2Service { "" ).trim(); - if (!accessKeyId || !secretAccessKey) { - // Storage is locked / keys unavailable — can't delete without them. - return skip("storage-locked"); - } - if (!bucket || !publicBaseUrl) { + if (!publicBaseUrl) { + // Without the public base URL we can't tell which objects belong to this + // user's bucket, so there's nothing safe to remove. return skip("missing-bucket-or-base-url"); } + // Determine the affected objects BEFORE the credentials check. publicBaseUrl + // is available even while storage is locked, so this lets us report + // "storage-locked" only when there are genuinely hosted objects to remove — + // an external-URL video (no matching objects) is never a false alarm. const keys = collectVideoStorageKeys({ videos: list, publicBaseUrl }); if (!keys.length) { return skip("no-matching-objects"); } + if (!accessKeyId || !secretAccessKey) { + // There ARE objects to remove but storage is locked / keys unavailable — + // surface this distinctly so the caller can warn the user the hosted + // files were left behind. + return skip("storage-locked"); + } + if (!bucket) { + return skip("missing-bucket-or-base-url"); + } + let s3; try { await ensureS3SdkLoaded(); diff --git a/tests/delete-storage-locked.test.mjs b/tests/delete-storage-locked.test.mjs new file mode 100644 index 00000000..687d62e1 --- /dev/null +++ b/tests/delete-storage-locked.test.mjs @@ -0,0 +1,78 @@ +// When a user deletes a video but their storage is locked, the hosted file (and +// its .torrent / thumbnail) can't be removed and stays publicly downloadable. +// deleteVideoStorage must report "storage-locked" so the UI can warn the user — +// BUT only when there are genuinely hosted objects to remove. An external-URL +// video (nothing on the user's bucket) must NOT trigger a false alarm, even +// while locked. + +import "./test-helpers/setup-localstorage.mjs"; +import assert from "node:assert/strict"; +import test from "node:test"; +import { R2Service } from "../js/services/r2Service.js"; + +const BASE = "https://cdn.example.com"; + +// A locked connection: bucket + public base URL are known (stored in plaintext), +// but the credentials are unavailable. +const LOCKED = { + provider: "cloudflare_r2", + accountId: "acc", + bucket: "my-bucket", + publicBaseUrl: BASE, + accessKeyId: "", + secretAccessKey: "", +}; + +test("locked storage with hosted objects reports storage-locked", async () => { + const svc = new R2Service(); + const result = await svc.deleteVideoStorage({ + videos: [{ url: `${BASE}/u/npub1abc/hash/video.mp4` }], + credentials: LOCKED, + }); + assert.equal(result.skipped, true); + assert.equal( + result.reason, + "storage-locked", + "a hosted object that can't be deleted because storage is locked must be flagged", + ); + assert.equal(result.deleted.length, 0, "nothing is deleted while locked"); +}); + +test("locked storage but an external-URL video is NOT a false storage-locked alarm", async () => { + const svc = new R2Service(); + const result = await svc.deleteVideoStorage({ + videos: [{ url: "https://some-other-host.example/video.mp4" }], + credentials: LOCKED, + }); + assert.equal(result.skipped, true); + assert.equal( + result.reason, + "no-matching-objects", + "an external URL has nothing on the user's bucket — must not warn about locked storage", + ); +}); + +test("unlocked storage actually deletes the hosted objects (no false lock warning)", async () => { + const deleted = []; + const svc = new R2Service({ + deleteObject: async ({ key }) => { + deleted.push(key); + }, + // makeR2Client is invoked but its client is only used by deleteObject, which + // we've stubbed — return a placeholder so client construction succeeds. + makeR2Client: () => ({}), + }); + const result = await svc.deleteVideoStorage({ + videos: [{ url: `${BASE}/u/npub1abc/hash/video.mp4` }], + credentials: { + ...LOCKED, + accessKeyId: "AKIA-test", + secretAccessKey: "secret-test", + }, + }); + assert.equal(result.skipped, false, "unlocked storage must not skip"); + assert.ok( + result.deleted.includes("u/npub1abc/hash/video.mp4"), + "the hosted video object should be deleted when unlocked", + ); +});