storage-cors-2 skip malformed metadata during best-effort storage cleanup

context/CONTEXT_2026-10-20_15-01-40_storage-cors-2.md

@@ -0,0 +1,6 @@
+# storage-cors-2
+- Current route: read-only remaining storage/delete-safety audit per dispatch14:58:59; former feed scope deferredstorage3.
+- Actual real-service malformed cached bucket/base reproduction2rejects vs0required;6controls pass,existing focused11scenarios/3files pass. docs/testing/storage-cors-delete-audit.md has fidelity/callers/limits.
+- Product/test read-only until routing. Proposed defensive primitive-string handling/structured skip in r2Service.deleteVideoStorage + focused new regression. s3UploadService contains no duplicate delete method; no upload/key changes proposed.
+- Pending: dispatch fix routing;claim item2 after queued metadata visibility. QA owns full/normal/browser;no full/build/browser/cloud.
+- Scratch /tmp/storage-cors-delete-audit.mjs;deferred feed invalid-seed scratch /tmp/storage-cors-feed-audit.mjs preserved. No secret/private-data use.

decisions/DECISIONS_2026-10-20_15-09-00_storage-cors-2.md

@@ -0,0 +1,6 @@
+# storage-cors-2
+- Reject malformed connection fields as invalid-connection; no String(object) or arbitrary trim invocation for cleanup authorization/base/key material.
+- Reject nonstring owner identifiers as invalid-owner before resolution. Null/non-object parameter options safely use existing no-videos result.
+- Preserve existing owned-base/key collector,credential lock reasons,per-object failure summaries,provider-aware client branches and content-addressed upload semantics.
+- s3UploadService contains no affected delete counterpart; no upload-copy edits per explicit dispatch ruling. Tests prove both provider branches instead.
+- No broad catch; existing client-construction and each-delete failures retain structured results. Arbitrary throwing getters/proxies beyond serializable cache metadata are outside this proof.

docs/agents/backlog/storage-cors-2.json

@@ -1,9 +1,9 @@
 {
-  "id": "storage-cors-2",
-  "area": "storage-cors",
-  "status": "open",
-  "title": "Audit best-effort storage/delete safety and remaining R2 followups offline",
-  "blockedOn": null,
-  "addedOn": "2026-10-20",
-  "filedAt": "7f99c0456931c917082d6a6680c79655221dcc82"
+ "id": "storage-cors-2",
+ "area": "storage-cors",
+ "status": "in-progress",
+ "title": "Audit best-effort storage/delete safety and remaining R2 followups offline",
+ "blockedOn": null,
+ "addedOn": "2026-10-20",
+ "filedAt": "7f99c0456931c917082d6a6680c79655221dcc82"
 }

docs/testing/storage-cors-delete-audit.md

@@ -0,0 +1,21 @@
+# storage-cors-2 current delete-safety audit
+
+Started2026-10-20T14:59:46UTC after dispatch priority correction. HEADd88716e3c6351468a53bc7fdb84fe912c01697b5. Product/existing tests read-only. Assignment metadata queued behind QA normal gate; dispatch authorizes read-only audit before claim visibility.
+
+Plan: inspect actual service/caller/normalization and existing scenarios; drive real service with fake SDK/transport and synthetic persistence records; establish owned/external/locked/partial controls; report demonstrated loss/smallest fix scope before writes.
+
+`env -u NODE_TEST_CONTEXT NODE_ENV=test node /tmp/storage-cors-delete-audit.mjs`: exit1, six passing controls and two contract failures (expected0 rejected calls, actual2), networkAttempts0. Real R2Service/deleteVideoStorage/resolveConnection, actual StorageService.listConnections/isUnlocked, actual SDK wrapper/key collector/deleteObject; remote SDK classes replaced locally, S3Client.send fake, persistence `_getAccount` returns synthetic account record. No real account/credentials/cloud/delete operations. Fixture strings are invented, example.invalid URLs.
+
+Controls: owned media+(.torrent)+thumbnail3 requests/deleted3; external host+base-prefix sibling0 clients/deletes; locked-owned skip storage-locked; locked-external skip no-matching-objects; partial transport failure2success/1failed/3requests/no rejection; valid cached locked connection resolves storage-locked.
+
+Failure: synthetic cached R2 metadata retains numeric bucket17 or object publicBaseUrl through real resolveConnection mapping. deleteVideoStorage then calls `.trim()` outside catches and rejects TypeError in2/2 malformed cases,0clients/0deleteIO. SDK was successfully loaded in the same process; success control proves the transport is usable. This is a never-throw contract violation, not a demonstrated blocked note deletion: app.handleDeleteModalConfirm and editModalController already catch cleanup exceptions. The malformed locked connection can fail before returning a storage-locked summary/warning.
+
+Proposed route: defensively validate primitive string fields in R2Service.deleteVideoStorage and return structured skip without interpreting malformed object metadata as permission/bucket names; add focused real-wrapper transport regression + integrity/mutants. s3UploadService has no deleteVideoStorage implementation; mirrored upload/key code remains unchanged. Await dispatch scope/semantics before product or committed test edits.
+
+Existing evidence scope: tests/services/r2-delete-video-storage.test.mjs covers five service scenarios with overridden client/delete I/O; tests/delete-storage-locked.test.mjs covers three locked/generic controls. Neither covers malformed persistent metadata. Existing helpers implement owner-base prefix exclusion. Full delete caller passes only targetVideo, so historical all-version object enumeration remains followup. Current edit caller DOES compare changed nonempty thumbnails and queues old thumbnail cleanup; dated TODO “thumbnail cleanup skipped” is stale for this path. No new retention/deletion semantics proposed.
+
+Deferred feed3: /tmp/storage-cors-feed-audit.mjs preserved; attempted real client/buffer/service/engine/coordinator/view scratch failed its initial visible-seed control0/2; no deletion finding or full feed proof. No product/test edit made from that invalid fixture.
+
+No browser/build/full gate used. No tool/gate wait blocking read-only work; actual cloud/provider IAM/serialization and real malformed user records unverified. No production data or signer used.
+
+Routed15:06:18UTC after independent dispatch confirmation. Implemented bounded metadata guard and13 retained real-service scenarios; original method fails9/13;7 restored mutants rejected. Focused4files/24scenarios,lint11/build pass. See storage-cors-delete-integrity.md. Awaiting full/submit slot requested15:08:52UTC. Current HEAD/baseac99ae9a;service unchanged from filedAt before fix,claim2 now visible/in-progress. No upload-copy deletion counterpart exists; no unrelated copy edits.

docs/testing/storage-cors-delete-integrity.md

@@ -0,0 +1,44 @@
+# storage-cors-2 best-effort delete metadata guard
+
+Dispatch independently reproduced2 rejected calls vs0 required with6 controls, then routed15:06:18UTC. Only deleteVideoStorage in r2Service.js changes. Input shape/owner identifiers and string connection fields are validated before trimming or SDK client construction. Nonstring metadata returns invalid-connection; nonstring owners return invalid-owner; absent/null/non-object options return existing no-videos skip. Primitive string trimming/default behavior remains. No coercion of arbitrary objects.
+
+Actual affected counterpart check: `s3UploadService.js` has NO deleteVideoStorage counterpart. Shared provider-aware R2Service delete already chooses makeR2Client versus makeS3Client. Both real wrapper branches are exercised with transport fakes; forcePathStyle/endpoint/region preserved. No duplicated upload/key/magnet logic changes in either service.
+
+```yaml
+test_integrity_note:
+  change_type: ["new_tests"]
+  scenarios:
+    - id: SCN-storage-delete-malformed-cache
+      given: "synthetic persisted locked R2 metadata with numeric bucket or object base URL"
+      when: "actual StorageService listing and R2Service connection resolution feed deleteVideoStorage"
+      then: "structured invalid-connection skip, no client construction or object requests"
+    - id: SCN-storage-delete-direct-types
+      given: "nonstring direct credentials fields/owners or null/non-object options"
+      when: "real cleanup method is invoked"
+      then: "safe structured skip with zero IO and no object trimming/coercion"
+    - id: SCN-storage-delete-provider-safety
+      given: "valid R2 or generic-S3 connection with owned/external/prefix-sibling URLs, locked storage or partial delete failure"
+      when: "actual cleanup/key collector/SDK wrapper/deleteObject run"
+      then: "only owned media/torrent/thumbnail delete; provider settings retained; lock reasons correct; partial errors recorded and later objects continue"
+  observable_outcomes:
+    - "exact summaries, zero SDK constructions/requests for unsafe input, exact deletion keys/bucket/config and per-object failure"
+  determinism_controls:
+    - "synthetic _getAccount persistence IO; real connection resolution/mapping"
+    - "local remote-SDK replacement/recording transport; actual key collector and deleteObject; no real credentials/network"
+    - "no retries/sleeps/timeouts in retained regression"
+  anti_cheat_rationale:
+    prevents:
+      - "constant skip/no-op"
+      - "over-mocked service logic"
+      - "unsafe object coercion and cross-base deletion"
+      - "hidden partial failures or lost provider settings"
+  relaxation:
+    did_relax_any_assertion: false
+    if_true_explain_spec_basis: ""
+```
+
+Atac99ae9a6bdbeae555a30f659b0720ae67e777e8,13 new scenarios pass. Original method fails9/13 (each malformed cached field, each of5direct fields, owner types, null options). Seven restored mutants rejected: remove settings guard(scenarios1–7); remove owner guard(8); null destructure(9); force wrong provider(10); bypass owner key collector(10–13); bypass lock guard(12); rethrow partial error(13). Every scenario is killed. No existing assertions changed.
+
+Focused existing+new runner4/4 files,24scenarios; lint11/11 and build/deploy-artifact exit0. Product scope29-line diff in one method. Pre-existing note-deletion callers catch cleanup exceptions,so this does not claim note deletion was blocked. Raw cloud/IAM/SDK serialization,actual malformed user records and arbitrary throwing JS proxies/getters are unverified. No real bucket/delete/signer/private-data/preview action. Feed3 remains deferred with invalid seed fixture preserved; no feed finding. Full/normal landing awaits dispatch slot.
+
+Timing: storage-audit acknowledgement14:59:46UTC; evidence15:01:20UTC; fix route15:06:18UTC (298s routing wait); ready15:08:52UTC. Metadata claim followed verified unstable merge; no metadata wait blocked read-only investigation.

js/services/r2Service.js

@@ -110,12 +110,13 @@ export class R2Service {
    * @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,
-  } = {}) {
+  async deleteVideoStorage(options = {}) {
+    const {
+      videos = [],
+      npub = "",
+      pubkey = "",
+      credentials = null,
+    } = options && typeof options === "object" ? options : {};
     const list = (Array.isArray(videos) ? videos : [videos]).filter(
       (entry) => entry && typeof entry === "object"
     );
@@ -130,6 +131,12 @@ export class R2Service {
       return skip("no-videos");
     }
 
+    if (
+      (npub != null && typeof npub !== "string") ||
+      (pubkey != null && typeof pubkey !== "string")
+    ) {
+      return skip("invalid-owner");
+    }
     let resolvedNpub = (npub || "").trim();
     if (!resolvedNpub && pubkey) {
       resolvedNpub = safeEncodeNpub(pubkey) || "";
@@ -150,6 +157,16 @@ export class R2Service {
       return skip("no-connection");
     }
 
+    // Cached metadata is not guaranteed to contain strings. Reject malformed
+    // settings before trimming or constructing any client; never coerce objects
+    // into credentials or a bucket URL during best-effort cleanup.
+    if (
+      typeof settings !== "object" || Array.isArray(settings) ||
+      ["accessKeyId", "secretAccessKey", "bucket", "publicBaseUrl", "baseDomain"]
+        .some((key) => settings[key] != null && typeof settings[key] !== "string")
+    ) {
+      return skip("invalid-connection");
+    }
     const accessKeyId = (settings.accessKeyId || "").trim();
     const secretAccessKey = (settings.secretAccessKey || "").trim();
     const bucket = (settings.bucket || "").trim();

test_logs/TEST_LOG_2026-10-20_15-01-40_storage-cors-2.md

@@ -0,0 +1,6 @@
+# storage-cors-2 read-only evidence
+- HEADd88716e3c6351468a53bc7fdb84fe912c01697b5; current route is storage-delete audit, feed deferred3.
+- `env -u NODE_TEST_CONTEXT NODE_ENV=test node /tmp/storage-cors-delete-audit.mjs`:exit1;6controls pass,2/2 malformed synthetic cached records reject TypeError(expected0);0network attempts,0SDK clients/deletes for malformed cases. Actual service+connection mapping/storage metadata IO+key collector+delete helper, fake SDK/transport.
+- `env -u NODE_TEST_CONTEXT node scripts/run-unit-tests.mjs r2-delete-video-storage delete-storage-locked storage-key-collision`:exit0,3/3 files,11 scenarios. No new product/committed tests.
+- Earlier deferred feed scratch `env -u NODE_TEST_CONTEXT NODE_ENV=test timeout25s node /tmp/storage-cors-feed-audit.mjs`:exit1 initial seed control0/2; fixture invalid, no feed deletion finding.
+- Dispatch evidence/scope request sent15:01:20UTC. Awaiting routing;metadata still queued behind QA. No full/build/browser/cloud/production/signer activity.

test_logs/TEST_LOG_2026-10-20_15-09-00_storage-cors-2.md

@@ -0,0 +1,8 @@
+# storage-cors-2 pre-full checks
+- Baseac99ae9a6bdbeae555a30f659b0720ae67e777e8; item2 claimed,git diff filedAt7f99c045→HEAD r2Service empty before implementation.
+- `env -u NODE_TEST_CONTEXT NODE_ENV=test node tests/storage-delete-malformed-settings.test.mjs`:exit0,13/13 scenarios. Original HEAD method via scratch harness:exit1,9/13 failed.
+- `python3 /tmp/storage-cors-delete-mutants.py`:exit0,all7 broken method variants rejected(exit1) and restored. Every retained scenario fails under at least one.
+- `env -u NODE_TEST_CONTEXT node scripts/run-unit-tests.mjs storage-delete-malformed-settings r2-delete-video-storage delete-storage-locked storage-key-collision`:exit0,4/4 files,24scenarios.
+- `env -u NODE_TEST_CONTEXT npm run lint`:exit0,11 checks(sandbox permission for read-only Git child as before).
+- `env -u NODE_TEST_CONTEXT npm run build`:exit0 incl deploy-artifact verification.
+- `git diff --check`:exit0. Full/browser not run; full/normal slot requested15:08:52UTC. No real delete/network/cloud/signer/private-data use.

tests/storage-delete-malformed-settings.test.mjs

@@ -0,0 +1,154 @@
+// Actual R2Service/StorageService resolution, SDK wrapper and deleteObject;
+// only persistence IO and remote SDK transport are virtualized. Integrity note:
+// docs/testing/storage-cors-delete-integrity.md
+import test from "node:test";
+import assert from "node:assert/strict";
+import { registerHooks } from "node:module";
+import "./test-helpers/setup-localstorage.mjs";
+
+let requests = [];
+let configurations = [];
+let failTorrent = false;
+const transportKey = Symbol.for("bitvid.storage-delete-test-transport");
+globalThis[transportKey] = {
+  make(config) { configurations.push(config); },
+  async send(command) {
+    assert.equal(command.constructor.name, "DeleteObjectCommand");
+    requests.push(command.input);
+    if (failTorrent && command.input.Key.endsWith(".torrent")) {
+      throw new Error("fixture delete failure");
+    }
+    return {};
+  },
+};
+const hooks = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    if (specifier.startsWith("https://esm.sh/@aws-sdk/client-s3@")) {
+      return { url: "offline:storage-delete-sdk", shortCircuit: true };
+    }
+    if (/^https?:/.test(specifier)) throw new Error("Network forbidden in storage delete fixture");
+    return nextResolve(specifier, context);
+  },
+  load(url, context, nextLoad) {
+    if (url === "offline:storage-delete-sdk") return {
+      format: "module", shortCircuit: true,
+      source: `const transport = globalThis[Symbol.for("bitvid.storage-delete-test-transport")];
+        export class S3Client { constructor(config) { transport.make(config); }
+          send(command) { return transport.send(command); } }
+        export class DeleteObjectCommand { constructor(input) { this.input = input; } }`,
+    };
+    return nextLoad(url, context);
+  },
+});
+globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = false;
+const { ensureS3SdkLoaded } = await import("../js/storage/s3-client.js");
+await ensureS3SdkLoaded();
+const { R2Service } = await import("../js/services/r2Service.js");
+const { default: storageService } = await import("../js/services/storageService.js");
+hooks.deregister();
+globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = true;
+const base = "https://bucket.example.invalid/owned";
+const owner = "a".repeat(64);
+const settings = { provider: "cloudflare_r2", accountId: "fixture-account", bucket: "owned-bucket", publicBaseUrl: base, accessKeyId: "fixture-access", secretAccessKey: "fixture-key" };
+const videos = [{ url: `${base}/clip.mp4`, thumbnail: `${base}/thumb.jpg` }];
+const reset = () => { requests = []; configurations = []; failTorrent = false; };
+const noIo = () => { assert.deepEqual(requests, []); assert.deepEqual(configurations, []); };
+const assertSkipped = (result, reason) => {
+  if (reason) assert.equal(result.reason, reason);
+  assert.equal(result.skipped, true);
+  assert.ok(result.reason);
+  assert.deepEqual(result.deleted, []);
+  assert.deepEqual(result.failed, []);
+};
+
+for (const override of [{ bucket: 17 }, { publicBaseUrl: { url: base } }]) {
+  const key = Object.keys(override)[0];
+  test(`malformed cached ${key} metadata resolves to a skip without IO`, async () => {
+    const originalRead = storageService._getAccount;
+    try {
+      reset();
+      storageService._getAccount = async () => ({ connections: { fixture: {
+        id: "fixture", provider: "cloudflare_r2",
+        meta: { defaultForUploads: true, accountId: "fixture-account", bucket: settings.bucket, publicBaseUrl: base, ...override },
+      } } });
+      const service = new R2Service();
+      const resolved = await service.resolveConnection(owner);
+      assert.deepEqual(resolved[key], override[key]);
+      assertSkipped(await service.deleteVideoStorage({ npub: owner, videos }), "invalid-connection");
+      noIo();
+    } finally { storageService._getAccount = originalRead; }
+  });
+}
+
+for (const key of ["accessKeyId", "secretAccessKey", "bucket", "publicBaseUrl", "baseDomain"]) {
+  test(`malformed direct ${key} is skipped without object coercion or IO`, async () => {
+    for (const value of [17, true, [], { trim() { throw new Error("must not invoke object methods"); }, toString() { throw new Error("must not coerce"); } }]) {
+      reset();
+      assertSkipped(await new R2Service().deleteVideoStorage({ credentials: { ...settings, [key]: value }, videos }), "invalid-connection");
+      noIo();
+    }
+  });
+}
+
+test("malformed direct owner identifiers are skipped before resolving a connection", async () => {
+  for (const field of ["npub", "pubkey"]) {
+    reset();
+    const service = new R2Service();
+    service.resolveConnection = async () => { throw new Error("must not resolve malformed owner"); };
+    assertSkipped(await service.deleteVideoStorage({ [field]: { trim() { throw new Error("must not trim an object"); } }, credentials: settings, videos }), "invalid-owner");
+    noIo();
+  }
+});
+
+test("missing, null, or primitive parameter objects safely skip", async () => {
+  for (const options of [undefined, null, 17, "bad", false]) {
+    reset();
+    const result = await new R2Service().deleteVideoStorage(options);
+    assertSkipped(result);
+    assert.equal(result.reason, "no-videos");
+    noIo();
+  }
+});
+
+test("valid R2 and generic S3 delete all owned objects with provider settings intact", async () => {
+  for (const generic of [false, true]) {
+    reset();
+    const credentials = { ...settings, provider: generic ? "generic_s3" : "cloudflare_r2", accountId: generic ? "" : settings.accountId, endpoint: "https://s3.example.invalid", region: "fixture-region", forcePathStyle: false };
+    const result = await new R2Service().deleteVideoStorage({ credentials, videos });
+    assert.equal(result.skipped, false);
+    assert.deepEqual(result.failed, []);
+    assert.deepEqual(new Set(result.deleted), new Set(["clip.mp4", "clip.torrent", "thumb.jpg"]));
+    assert.deepEqual(requests.map(request => request.Bucket), [settings.bucket, settings.bucket, settings.bucket]);
+    assert.equal(configurations.length, 1);
+    assert.equal(configurations[0].forcePathStyle, !generic);
+    assert.equal(configurations[0].endpoint, credentials.endpoint);
+    assert.equal(configurations[0].region, credentials.region);
+  }
+});
+
+test("external hosts and base-prefix siblings cannot cause deletes", async () => {
+  reset();
+  const result = await new R2Service().deleteVideoStorage({ credentials: settings, videos: [{ url: "https://external.example.invalid/clip.mp4" }, { url: `${base}-sibling/clip.mp4` }] });
+  assertSkipped(result);
+  assert.equal(result.reason, "no-matching-objects");
+  noIo();
+});
+
+test("locked storage distinguishes owned objects from external URLs", async () => {
+  for (const owned of [false, true]) {
+    reset();
+    const result = await new R2Service().deleteVideoStorage({ credentials: { ...settings, accessKeyId: "", secretAccessKey: "" }, videos: owned ? videos : [{ url: "https://external.example.invalid/clip.mp4" }] });
+    assertSkipped(result);
+    assert.equal(result.reason, owned ? "storage-locked" : "no-matching-objects");
+    noIo();
+  }
+});
+
+test("partial delete failures are reported while later owned objects still delete", async () => {
+  reset(); failTorrent = true;
+  const result = await new R2Service().deleteVideoStorage({ credentials: settings, videos });
+  assert.equal(result.skipped, false);
+  assert.deepEqual(new Set(result.deleted), new Set(["clip.mp4", "thumb.jpg"]));
+  assert.deepEqual(result.failed, [{ key: "clip.torrent", error: "fixture delete failure" }]);
+  assert.equal(requests.length, 3);
+});

todo/TODO_2026-10-20_15-09-00_storage-cors-2.md

@@ -0,0 +1,7 @@
+# storage-cors-2
+- [x] Real cached malformed metadata defect reproduced2/2;6controls,independent dispatch confirmation/routing.
+- [x] Method-only guard/new13scenarios;original fails9;7restored mutants rejected.
+- [x] Focused24scenarios/4files,lint11,build/artifact pass.
+- [ ] Commit/merge unstable;merged relevant checks.
+- [ ] Dispatch full/submit slot;full normal gate,queue/durable landing/ancestry before close.
+- Feed3 deferred; cloud/provider IAM/serialization/live user data unverified; no product scope expansion.