fix(upload): attach CORS guidance to direct upload failures

A new user's top first-upload failure is a missing R2 CORS policy (browser
PUT/POST blocked) -- and object-scoped R2 tokens often cannot auto-set CORS.
The connection test already surfaces buildCorsGuidance on the opaque
"Failed to fetch", but a user who skips the test and uploads directly got a
bare "Upload failed: Failed to fetch". The upload error handler now detects the
browser CORS/network rejection (isLikelyCorsError -- Chrome "Failed to fetch",
Firefox "NetworkError", Safari "Load failed") and appends the same actionable
guidance. Updated the R2 follow-up TODO (credential audit done; CORS reviewed).

Test: tests/services/r2-cors-error-classifier.test.mjs. Full suite green.

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

js/services/r2Service.js

@@ -103,6 +103,25 @@ function buildCorsGuidance({ accountId } = {}) {
   ].join(" ");
 }
 
+/**
+ * A browser CORS preflight/rejection surfaces as an opaque network error
+ * ("Failed to fetch" / "NetworkError" / Safari's "Load failed") with no HTTP
+ * status — indistinguishable from a real network drop. We treat these as
+ * likely-CORS so the upload path can attach actionable guidance instead of a
+ * bare "Failed to fetch".
+ */
+export function isLikelyCorsError(err) {
+  const message = (
+    (err && (err.message || err.toString && err.toString())) ||
+    ""
+  ).toLowerCase();
+  return (
+    message.includes("failed to fetch") ||
+    message.includes("networkerror") ||
+    message.includes("load failed")
+  );
+}
+
 function createDefaultSettings() {
   return {
     accountId: "",
@@ -1251,10 +1270,16 @@ export class R2Service {
       return publishOutcome;
     } catch (err) {
       userLogger.error("Cloudflare upload failed:", err);
-      this.setCloudflareUploadStatus(
-        err?.message ? `Upload failed: ${err.message}` : "Upload failed.",
-        "error"
-      );
+      let message = err?.message
+        ? `Upload failed: ${err.message}`
+        : "Upload failed.";
+      // A CORS rejection during the browser PUT/POST looks like an opaque
+      // "Failed to fetch" — attach actionable guidance so users who skipped the
+      // connection test still know what to fix (the #1 first-upload failure).
+      if (isLikelyCorsError(err)) {
+        message += ` ${buildCorsGuidance({ accountId })}`;
+      }
+      this.setCloudflareUploadStatus(message, "error");
       return false;
     } finally {
       this.setCloudflareUploading(false);

tests/services/r2-cors-error-classifier.test.mjs

@@ -0,0 +1,28 @@
+// Scenario (SCN-cors-error-classifier):
+//   A browser CORS rejection during upload surfaces as an opaque network error
+//   with no HTTP status. isLikelyCorsError() must recognize the messages the
+//   major browsers actually produce (so the upload path can attach CORS guidance
+//   instead of a bare "Failed to fetch"), without false-positiving on ordinary
+//   errors (so we don't wrongly tell users to fix CORS).
+
+import "../test-helpers/setup-localstorage.mjs";
+import test from "node:test";
+import assert from "node:assert/strict";
+
+globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = true;
+const { isLikelyCorsError } = await import("../../js/services/r2Service.js");
+
+test("recognizes the browsers' CORS/network rejection messages", () => {
+  assert.equal(isLikelyCorsError(new TypeError("Failed to fetch")), true); // Chrome
+  assert.equal(isLikelyCorsError(new Error("NetworkError when attempting to fetch resource.")), true); // Firefox
+  assert.equal(isLikelyCorsError(new TypeError("Load failed")), true); // Safari
+});
+
+test("does NOT false-positive on ordinary upload errors", () => {
+  assert.equal(isLikelyCorsError(new Error("Access Denied")), false);
+  assert.equal(isLikelyCorsError(new Error("File is empty (0 bytes).")), false);
+  assert.equal(isLikelyCorsError(new Error("NoSuchBucket")), false);
+  assert.equal(isLikelyCorsError(null), false);
+  assert.equal(isLikelyCorsError(undefined), false);
+  assert.equal(isLikelyCorsError({}), false);
+});

todo/TODO_2026-06-16_r2-followups.md

@@ -13,18 +13,29 @@ Backlog from the R2 audit. Branch: `unstable` (promote down to beta/main later).
 
 ## Open — R2 areas not yet audited/built
 
-### 1. Credential security review (highest priority)
-- [ ] Audit how browser-held S3/R2 keys are stored in IndexedDB (the README's own warning).
-  - Files: `js/services/storageService.js`, `js/r2.js` (legacy settings), `js/ui/profileModal/ProfileStorageController.js`.
-  - Check: encryption at rest, unlock/lock model (`storageService.isUnlocked`), in-memory exposure window, what `resolveConnection` returns when locked, whether secrets can leak to logs/telemetry.
-  - Confirm the "trusted operator only" assumption is actually enforced/communicated in the UI.
+### 1. Credential security review — DONE (2026-06-17, `200e5cec`)
+- [x] Audited browser-held S3/R2 key storage. Verdict: **sound** — envelope encryption
+      (AES-GCM-256 payload + fresh IV; master key encrypted to-self via the Nostr signer,
+      NIP-44/NIP-04), in-memory master key cleared on logout, no secret logging, legacy
+      plaintext migrated then cleared, secrets never in plaintext `meta`. Locked-in by a
+      scan-the-whole-record regression test.
+- [ ] Hardening (optional, post-launch): **idle auto-lock** — master key currently stays
+      in memory the whole session after unlock. Add an idle/timeout auto-`lock(pubkey)`.
+- [ ] Confirm the production CSP is strict (the real defense for in-memory keys / XSS).
 
-### 2. Bucket / custom-domain provisioning
-- [ ] Audit `ensureBucketConfigForNpub` (`r2Service.js`) + `js/storage/r2-mgmt.js`
-      (`ensureBucket`, `putCors`, `attachCustomDomainAndWait`, `setManagedDomain`,
-      `deriveShortSubdomain`) for correctness + failure handling.
-  - Check: Cloudflare management **API token** scope/usage, partial-failure recovery,
-    CORS rule correctness, idempotency, what happens when the user pre-created the bucket.
+### 2. Bucket / custom-domain provisioning — partially audited (2026-06-17)
+- [x] CORS provisioning reviewed: `ensureBucketConfigForNpub` auto-creates the bucket +
+      sets CORS best-effort (S3 `PutBucketCors`); the connection test (`verifyPublicAccess`)
+      does a real browser upload+fetch and surfaces `buildCorsGuidance` on the opaque
+      "Failed to fetch". Gap fixed: the **direct upload** path now also attaches CORS
+      guidance via `isLikelyCorsError` (`<this commit>`), for users who skip the test.
+- [ ] Still to check: `r2-mgmt.js` (`attachCustomDomainAndWait`, `setManagedDomain`,
+      `deriveShortSubdomain`) — the managed-domain path; Cloudflare API-token scope;
+      idempotency; partial-failure recovery. Note: token-based bucket creation/domain
+      mgmt is marked deprecated in `ensureBucketConfigForNpub` — confirm it's fully unused.
+- [ ] CORS replace-not-merge: `PutBucketCors` overwrites the whole config with only the
+      current origin, so a bucket used from multiple bitvid origins keeps only the last.
+      Consider merging existing origins.
 
 ### 3. CDN purge integration
 - [ ] `scripts/purge-cloudflare-changed.mjs` uses `CLOUDFLARE_ZONE_ID` + `CLOUDFLARE_API_TOKEN`