[storage] fix B2 / generic-S3 uploads ("S3 endpoint is required") — prepareUpload contract

Uploading to a Backblaze B2 (or any generic-S3) bucket failed with "S3 endpoint is
required." even though the connection tested fine.

Root cause: the two upload services have different prepareUpload contracts —
r2Service.prepareUpload(npub, { credentials }) vs s3UploadService.prepareUpload(settings).
MediaUploader calls both the r2 way, so for S3/B2 the npub STRING became the settings →
validateS3Connection saw no endpoint. Compounding it, a storageService.getConnection()
result keeps bucket/endpoint/region under `.meta`, which validateS3Connection reads at
the top level. (R2 worked because its service already uses the (npub, {credentials})
contract.)

Fix:
- s3UploadService.prepareUpload now accepts (npub, { credentials }) AND a direct settings
  object (back-compat), flattening meta → top-level via flattenS3Settings.
- The storage Test now derives/passes the B2 public URL (else the prefix), so it stops
  falsely warning "Public Bucket URL is missing. Uploads will fail."

Tests: tests/services/s3-upload-prepare-contract.test.mjs (3 — meta flatten, npub ignored
when credentials given, direct-settings back-compat). Build + lint clean; upload/storage
suites 46/46.

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

js/services/s3UploadService.js

@@ -26,6 +26,30 @@ import {
 const STATUS_VARIANTS = new Set(["info", "success", "error", "warning"]);
 const INFO_HASH_PATTERN = /^[a-f0-9]{40}$/;
 
+// A storageService.getConnection() result keeps bucket/region/endpoint/forcePathStyle
+// under `.meta`; validateS3Connection reads them at the top level. Flatten so a saved
+// connection (e.g. Backblaze B2) can be uploaded to without "S3 endpoint is required".
+function flattenS3Settings(raw = {}) {
+  const meta = raw && typeof raw.meta === "object" && raw.meta ? raw.meta : {};
+  const pickBool = (a, b) =>
+    typeof a === "boolean" ? a : typeof b === "boolean" ? b : false;
+  return {
+    ...raw,
+    endpoint: raw.endpoint || meta.endpoint || "",
+    bucket: raw.bucket || meta.bucket || "",
+    region: raw.region || meta.region || "auto",
+    accessKeyId: raw.accessKeyId || "",
+    secretAccessKey: raw.secretAccessKey || "",
+    forcePathStyle: pickBool(raw.forcePathStyle, meta.forcePathStyle),
+    publicBaseUrl:
+      raw.publicBaseUrl ||
+      meta.publicBaseUrl ||
+      raw.baseDomain ||
+      meta.baseDomain ||
+      "",
+  };
+}
+
 function normalizeInfoHash(value) {
   return typeof value === "string" ? value.trim().toLowerCase() : "";
 }
@@ -155,7 +179,16 @@ export class S3UploadService {
     return prepared;
   }
 
-  async prepareUpload(settings = {}, { createBucketIfMissing = true } = {}) {
+  async prepareUpload(settingsOrNpub = {}, options = {}) {
+    const { createBucketIfMissing = true, credentials } = options || {};
+    // Two call styles must both work:
+    //  - direct:        prepareUpload(settings, { createBucketIfMissing })
+    //  - mediaUploader: prepareUpload(npub, { credentials })  ← like r2Service
+    // When credentials come from storageService.getConnection, the bucket/region/
+    // endpoint live under `.meta`, so flatten them to the top level that
+    // validateS3Connection reads. (npub is ignored when credentials are supplied.)
+    const raw = credentials || settingsOrNpub || {};
+    const settings = flattenS3Settings(raw);
     const prepared = await this.verifyConnection({
       settings,
       createBucketIfMissing,

js/ui/profileModal/ProfileStorageController.js

@@ -4,6 +4,7 @@ import {
   prepareS3Connection,
   getCorsOrigins,
   deriveB2Endpoint,
+  derivePublicBaseUrl,
 } from "../../services/s3Service.js";
 import { getActiveSigner } from "../../nostr/client.js";
 import { DEFAULT_NIP07_ENCRYPTION_METHODS } from "../../nostr/nip07Permissions.js";
@@ -952,6 +953,11 @@ export class ProfileStorageController {
     } else {
       config.endpoint = endpointOrAccount;
       config.forcePathStyle = forcePathStyle;
+      // Explicit Public Access URL, else derive it (so the test doesn't falsely warn).
+      config.publicBaseUrl =
+        publicBaseUrl ||
+        derivePublicBaseUrl({ endpoint: endpointOrAccount, bucket, forcePathStyle });
+      config.baseDomain = config.publicBaseUrl;
     }
 
     try {

tests/services/s3-upload-prepare-contract.test.mjs

@@ -0,0 +1,98 @@
+// Bug: B2 (and generic-S3) uploads failed with "S3 endpoint is required." MediaUploader
+// calls service.prepareUpload(npub, { credentials }) (the r2Service contract), but
+// s3UploadService.prepareUpload used to take (settings, ...) — so the npub string became
+// the settings (no endpoint). Also, a storageService.getConnection() result keeps
+// bucket/endpoint under `.meta`, which validateS3Connection reads at the top level.
+//
+// Fix: s3UploadService.prepareUpload accepts (npub, { credentials }) AND flattens meta.
+
+import "../test-helpers/setup-localstorage.mjs";
+import { test, mock } from "node:test";
+import assert from "node:assert/strict";
+import { S3UploadService } from "../../js/services/s3UploadService.js";
+
+function createDeps() {
+  const seen = {};
+  return {
+    deps: {
+      ensureS3SdkLoaded: mock.fn(async () => {}),
+      makeS3Client: mock.fn(() => ({})),
+      multipartUpload: mock.fn(async () => {}),
+      buildR2Key: mock.fn(() => "k"),
+      buildS3ObjectUrl: mock.fn(() => "https://u/file"),
+      getCorsOrigins: mock.fn(() => ["*"]),
+      prepareS3Connection: mock.fn(async (opts) => ({ ...opts })),
+      validateS3Connection: mock.fn((settings) => {
+        seen.settings = settings;
+        return { ...settings };
+      }),
+      userLogger: { warn: mock.fn(), error: mock.fn() },
+      buildStoragePointerValue: mock.fn(() => "p"),
+      buildStoragePrefixFromKey: mock.fn(() => "pre"),
+      getVideoNoteErrorMessage: mock.fn((c) => `Error: ${c}`),
+      normalizeVideoNotePayload: mock.fn((p) => ({ payload: p, errors: [] })),
+      calculateTorrentInfoHash: mock.fn(async () => "h"),
+    },
+    seen,
+  };
+}
+
+// A B2 connection as storageService.getConnection() returns it: keys + endpoint at the
+// top level, but bucket/region/forcePathStyle/publicBaseUrl under `.meta`.
+const B2_CONNECTION = {
+  provider: "backblaze_b2",
+  accessKeyId: "b2key",
+  secretAccessKey: "b2secret",
+  endpoint: "https://s3.us-west-004.backblazeb2.com",
+  forcePathStyle: false,
+  meta: {
+    provider: "backblaze_b2",
+    bucket: "bitvid",
+    region: "us-west-004",
+    publicBaseUrl: "https://bitvid.s3.us-west-004.backblazeb2.com",
+    forcePathStyle: false,
+  },
+};
+
+test("prepareUpload(npub, { credentials }) flattens the connection's meta to top-level settings", async () => {
+  const { deps, seen } = createDeps();
+  const service = new S3UploadService(deps);
+
+  const { settings, bucketEntry } = await service.prepareUpload("npub1abc", {
+    credentials: B2_CONNECTION,
+  });
+
+  // validateS3Connection must have received endpoint + bucket (the prior bug passed the
+  // npub string, so endpoint was undefined).
+  assert.equal(seen.settings.endpoint, "https://s3.us-west-004.backblazeb2.com");
+  assert.equal(seen.settings.bucket, "bitvid", "bucket flattened out of meta");
+  assert.equal(seen.settings.region, "us-west-004");
+  assert.equal(seen.settings.forcePathStyle, false);
+  assert.equal(
+    seen.settings.publicBaseUrl,
+    "https://bitvid.s3.us-west-004.backblazeb2.com",
+    "public URL flattened (so uploads don't get a blank base URL)",
+  );
+  assert.equal(bucketEntry.bucket, "bitvid");
+});
+
+test("the npub is ignored when credentials are supplied (not treated as settings)", async () => {
+  const { deps, seen } = createDeps();
+  const service = new S3UploadService(deps);
+  await service.prepareUpload("npub1ignored", { credentials: B2_CONNECTION });
+  assert.notEqual(seen.settings, "npub1ignored");
+  assert.equal(typeof seen.settings, "object");
+});
+
+test("still accepts a direct settings object (back-compat)", async () => {
+  const { deps, seen } = createDeps();
+  const service = new S3UploadService(deps);
+  await service.prepareUpload({
+    endpoint: "https://s3.mock",
+    bucket: "flat-bucket",
+    accessKeyId: "k",
+    secretAccessKey: "s",
+  });
+  assert.equal(seen.settings.bucket, "flat-bucket");
+  assert.equal(seen.settings.endpoint, "https://s3.mock");
+});

todo/TODO_2026-06-20_pre-launch.md

@@ -949,6 +949,19 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro
 - [ ] **Follow-up (cosmetic):** the upload modal's summary provider label can still show
       a stale provider until reloaded — refresh `loadFromStorage` when the storage default
       changes in the Storage pane (functional routing is already correct).
+- [x] **B2 / generic-S3 upload contract mismatch FIXED 2026-06-30.** Uploading to a
+      Backblaze B2 (or any generic-S3) bucket failed with **"S3 endpoint is required."**
+      Root cause: `MediaUploader` calls `service.prepareUpload(npub, { credentials })`
+      (the `r2Service` contract), but `s3UploadService.prepareUpload` took `(settings, …)`
+      — so the npub string became the settings (no endpoint). Also a
+      `storageService.getConnection()` result keeps `bucket`/`endpoint`/`region` under
+      `.meta`, which `validateS3Connection` reads at the top level. Fix:
+      `s3UploadService.prepareUpload` now accepts `(npub, { credentials })` and flattens
+      meta → top-level via `flattenS3Settings`. (R2 was unaffected because its service
+      already used that contract.) Also: the storage **Test** now derives/passes the B2
+      public URL so it stops falsely warning "Public Bucket URL is missing." Tests:
+      `tests/services/s3-upload-prepare-contract.test.mjs` (3 — flatten + npub-ignored +
+      back-compat). Build + lint clean; upload/storage suites 46/46.
 
 ### 39. Image uploads should prefer uploading to configured storage (UX)
 - [x] **Profile image + banner upload — FIXED 2026-06-30.** The UI was already wired