mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
feat(delete): warn when a hosted file is left behind by locked storage
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 <[email protected]>
This commit is contained in:
1 parent
3f4299a885
commit
f0eecec09d
3 files changed
+108
-5
No files matched your search
@@ -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) {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user