bluebubbles: address Greptile review (P1 auth-header forwarding, P2 cache fingerprint)

Greptile findings from the first review cycle on #68234 — both real, both
asymptomatic under today's query-string auth default but blocking for the
header-auth flip (#66869).

P1: Thread auth-decorated headers through multipart + media paths.
- `postMultipartFormData` now accepts an optional `extraHeaders` param and
  merges caller headers with its own Content-Type (Content-Type always wins
  so the multipart boundary stays authoritative).
- `client.requestMultipart` forwards `prepared.init.headers` — attachment
  uploads and group-icon sets keep working under header auth.
- `client.downloadAttachment` fetchImpl merges `prepared.init.headers` with
  runtime-supplied headers (runtime wins on conflict, e.g. Range for partial
  reads) — attachment downloads via `fetchRemoteMedia` now carry auth too.

P2: Include auth strategy in the client cache fingerprint.
- `BlueBubblesAuthStrategy` gets a stable `id` field
  (`query-string` / `header:<name>`).
- Built-in factories set it; the cache key is now
  `{baseUrl}|{password}|{authStrategyId}` so different strategies for the
  same account + credential no longer silently share a cached instance.

Three new regression tests pin the above behavior (465 total, was 462).
Full BB suite green, lint clean.

extensions/bluebubbles/src/client.test.ts

@@ -191,6 +191,53 @@ describe("auth strategies", () => {
     const headers = new Headers((calledInit as RequestInit | undefined)?.headers);
     expect(headers.get("X-BB-Password")).toBe("s3cret");
   });
+
+  it("header-auth headers flow through requestMultipart (Greptile #68234 P1)", async () => {
+    // Before this fix, requestMultipart discarded prepared.init entirely
+    // and postMultipartFormData built its own hardcoded Content-Type header.
+    // Under header-auth that silently omitted the auth header on every
+    // attachment upload and group-icon set.
+    const client = createBlueBubblesClient({
+      serverUrl: "http://localhost:1234",
+      password: "s3cret",
+      authStrategy: blueBubblesHeaderAuth,
+    });
+    mockFetch.mockImplementation(() => Promise.resolve(new Response("{}", { status: 200 })));
+    await client.requestMultipart({
+      path: "/api/v1/chat/chat-guid/icon",
+      boundary: "----boundary",
+      parts: [new Uint8Array([1, 2, 3])],
+    });
+    const [, calledInit] = mockFetch.mock.calls[0] ?? [];
+    const headers = new Headers((calledInit as RequestInit | undefined)?.headers);
+    expect(headers.get("X-BB-Password")).toBe("s3cret");
+    // And the multipart Content-Type must still be set correctly.
+    expect(headers.get("Content-Type")).toContain("multipart/form-data; boundary=----boundary");
+  });
+
+  it("header-auth headers flow through downloadAttachment fetchImpl (Greptile #68234 P1)", async () => {
+    // Before this fix, downloadAttachment built prepared.init.headers with
+    // the auth header but never forwarded it to the fetchImpl callback,
+    // so header-auth would silently 401 on attachment downloads.
+    const client = createBlueBubblesClient({
+      serverUrl: "http://localhost:1234",
+      password: "s3cret",
+      authStrategy: blueBubblesHeaderAuth,
+    });
+    mockFetch.mockImplementation(() =>
+      Promise.resolve(
+        new Response(Buffer.from([1, 2, 3]), {
+          status: 200,
+          headers: { "content-type": "image/png" },
+        }),
+      ),
+    );
+    await client.downloadAttachment({ attachment: { guid: "att-1", mimeType: "image/png" } });
+    // fetchRemoteMediaMock delegates to fetchImpl, which calls mockFetch.
+    const [, calledInit] = mockFetch.mock.calls[0] ?? [];
+    const headers = new Headers((calledInit as RequestInit | undefined)?.headers);
+    expect(headers.get("X-BB-Password")).toBe("s3cret");
+  });
 });
 
 // --- Core request path -----------------------------------------------------
@@ -494,6 +541,23 @@ describe("client cache", () => {
     });
     expect(b.baseUrl).toBe("http://host-b:1234");
   });
+
+  it("different authStrategy for the same account + credential rebuilds the client (Greptile #68234 P2)", () => {
+    // Before this fix the fingerprint keyed only on {baseUrl, password}.
+    // A second call with a different authStrategy would silently return
+    // the cached first strategy's client.
+    const a = createBlueBubblesClient({
+      serverUrl: "http://localhost:1234",
+      password: "s3cret",
+      // default: blueBubblesQueryStringAuth
+    });
+    const b = createBlueBubblesClient({
+      serverUrl: "http://localhost:1234",
+      password: "s3cret",
+      authStrategy: blueBubblesHeaderAuth,
+    });
+    expect(a).not.toBe(b);
+  });
 });
 
 describe("client construction", () => {

extensions/bluebubbles/src/client.ts

@@ -42,11 +42,19 @@ const DEFAULT_MULTIPART_TIMEOUT_MS = 60_000;
  *     BB Server ships the header-auth change for #66869.
  */
 export interface BlueBubblesAuthStrategy {
+  /**
+   * Stable identifier for this strategy. Used by the client cache fingerprint
+   * so two clients for the same account + credential that differ only in auth
+   * strategy don't silently collapse onto the same cached instance.
+   * (Greptile #68234 P2)
+   */
+  readonly id: string;
   decorate(req: { url: URL; init: RequestInit }): void;
 }
 
 export function blueBubblesQueryStringAuth(password: string): BlueBubblesAuthStrategy {
   return {
+    id: "query-string",
     decorate({ url }) {
       url.searchParams.set("password", password);
     },
@@ -58,6 +66,7 @@ export function blueBubblesHeaderAuth(
   headerName = "X-BB-Password",
 ): BlueBubblesAuthStrategy {
   return {
+    id: `header:${headerName}`,
     decorate({ init }) {
       const headers = new Headers(init.headers ?? undefined);
       headers.set(headerName, password);
@@ -265,6 +274,9 @@ export class BlueBubblesClient {
    * Multipart POST (attachment send, group icon set). The caller supplies the
    * boundary and body parts; the client handles URL construction, auth, and
    * SSRF policy. Timeout defaults to 60s because uploads can be large.
+   *
+   * Auth-decorated headers from `prepared.init` are forwarded via `extraHeaders`
+   * so header-auth strategies keep working on multipart paths. (Greptile #68234 P1)
    */
   async requestMultipart(params: {
     path: string;
@@ -283,6 +295,7 @@ export class BlueBubblesClient {
       parts: params.parts,
       timeoutMs: params.timeoutMs ?? DEFAULT_MULTIPART_TIMEOUT_MS,
       ssrfPolicy: this.ssrfPolicy,
+      extraHeaders: prepared.init.headers,
     });
   }
 
@@ -387,6 +400,13 @@ export class BlueBubblesClient {
     });
     const clientSsrfPolicy = this.ssrfPolicy;
     const effectiveTimeoutMs = params.timeoutMs ?? this.defaultTimeoutMs;
+    // Auth-decorated headers from buildAuthorizedRequest (for header-auth
+    // strategies) must flow through the fetchImpl callback too, otherwise
+    // the runtime might dispatch with only its own default headers. Merge
+    // prepared.init.headers with any headers the runtime supplies; runtime
+    // headers (typically Range for partial reads) win on conflict.
+    // (Greptile #68234 P1)
+    const preparedHeaders = prepared.init.headers;
 
     try {
       const fetched = await getBlueBubblesRuntime().channel.media.fetchRemoteMedia({
@@ -394,13 +414,19 @@ export class BlueBubblesClient {
         filePathHint: params.attachment.transferName ?? params.attachment.guid ?? "attachment",
         maxBytes,
         ssrfPolicy: clientSsrfPolicy,
-        fetchImpl: async (input, init) =>
-          await blueBubblesFetchWithTimeout(
+        fetchImpl: async (input, init) => {
+          const mergedHeaders = new Headers(preparedHeaders);
+          if (init?.headers) {
+            const runtimeHeaders = new Headers(init.headers);
+            runtimeHeaders.forEach((value, key) => mergedHeaders.set(key, value));
+          }
+          return await blueBubblesFetchWithTimeout(
             resolveRequestUrl(input),
-            { ...init, method: init?.method ?? "GET" },
+            { ...init, method: init?.method ?? "GET", headers: mergedHeaders },
             effectiveTimeoutMs,
             clientSsrfPolicy,
-          ),
+          );
+        },
       });
       return {
         buffer: new Uint8Array(fetched.buffer),
@@ -423,13 +449,20 @@ export class BlueBubblesClient {
 
 type CachedClientEntry = {
   client: BlueBubblesClient;
-  /** Fingerprint of {baseUrl, password} — cache hit requires full match. */
+  /** Fingerprint of {baseUrl, password, authStrategy.id} — cache hit requires full match. */
   fingerprint: string;
 };
 const clientFingerprints = new Map<string, CachedClientEntry>();
 
-function buildClientFingerprint(params: { baseUrl: string; password: string }): string {
-  return `${params.baseUrl}|${params.password}`;
+function buildClientFingerprint(params: {
+  baseUrl: string;
+  password: string;
+  authStrategyId: string;
+}): string {
+  // authStrategyId is included so two clients for the same account + credential
+  // that differ only in auth strategy do not silently share a cached instance.
+  // (Greptile #68234 P2)
+  return `${params.baseUrl}|${params.password}|${params.authStrategyId}`;
 }
 
 /**
@@ -447,9 +480,12 @@ export function createBlueBubblesClient(opts: BlueBubblesClientOptions = {}): Bl
     password: opts.password,
   });
   const cacheKey = resolved.accountId || DEFAULT_ACCOUNT_ID;
+  const authFactory = opts.authStrategy ?? blueBubblesQueryStringAuth;
+  const authStrategy = authFactory(resolved.password);
   const fingerprint = buildClientFingerprint({
     baseUrl: resolved.baseUrl,
     password: resolved.password,
+    authStrategyId: authStrategy.id,
   });
   const cached = clientFingerprints.get(cacheKey);
   if (cached && cached.fingerprint === fingerprint) {
@@ -461,7 +497,6 @@ export function createBlueBubblesClient(opts: BlueBubblesClientOptions = {}): Bl
     allowPrivateNetwork: resolved.allowPrivateNetwork,
     allowPrivateNetworkConfig: resolved.allowPrivateNetworkConfig,
   });
-  const authFactory = opts.authStrategy ?? blueBubblesQueryStringAuth;
 
   const client = new BlueBubblesClient({
     accountId: cacheKey,
@@ -471,7 +506,7 @@ export function createBlueBubblesClient(opts: BlueBubblesClientOptions = {}): Bl
     trustedHostname: policyResult.trustedHostname,
     trustedHostnameIsPrivate: policyResult.trustedHostnameIsPrivate,
     defaultTimeoutMs: opts.timeoutMs ?? DEFAULT_TIMEOUT_MS,
-    authStrategy: authFactory(resolved.password),
+    authStrategy,
   });
   clientFingerprints.set(cacheKey, { client, fingerprint });
   return client;

extensions/bluebubbles/src/multipart.ts

@@ -18,15 +18,29 @@ export async function postMultipartFormData(params: {
   parts: Uint8Array[];
   timeoutMs: number;
   ssrfPolicy?: SsrFPolicy;
+  /**
+   * Extra headers to merge with the multipart Content-Type. Used to forward
+   * auth-decorated headers from `BlueBubblesClient` (e.g. `X-BB-Password`
+   * under header-auth mode). Per-request Content-Type wins over callers so
+   * the multipart boundary is always authoritative. (Greptile #68234 P1)
+   */
+  extraHeaders?: HeadersInit;
 }): Promise<Response> {
   const body = Buffer.from(concatUint8Arrays(params.parts));
+  const headers: Record<string, string> = {};
+  if (params.extraHeaders) {
+    new Headers(params.extraHeaders).forEach((value, key) => {
+      headers[key] = value;
+    });
+  }
+  // Per-request Content-Type wins over callers so the multipart boundary is
+  // always authoritative.
+  headers["Content-Type"] = `multipart/form-data; boundary=${params.boundary}`;
   return await blueBubblesFetchWithTimeout(
     params.url,
     {
       method: "POST",
-      headers: {
-        "Content-Type": `multipart/form-data; boundary=${params.boundary}`,
-      },
+      headers,
       body,
     },
     params.timeoutMs,