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", + ); +});