fix: Send binary files via application/octet-stream in boxel-cli sync (CS-11075) (#4852)

Author: FadhlanR

packages/boxel-cli/src/commands/file/read.ts

@@ -6,14 +6,21 @@ import {
 } from '../../lib/profile-manager';
 import { ensureTrailingSlash } from '@cardstack/runtime-common/paths';
 import { SupportedMimeType } from '@cardstack/runtime-common/supported-mime-type';
+import { isBinaryFilename } from '@cardstack/runtime-common/infer-content-type';
 import { FG_RED, DIM, RESET } from '../../lib/colors';
 import { cliLog } from '../../lib/cli-log';
 
 export interface ReadResult {
   ok: boolean;
   status?: number;
-  /** Raw text content of the file. */
+  /** Raw text content of the file. Populated for non-binary paths. */
   content?: string;
+  /**
+   * Raw bytes. Populated when the requested path is a binary filename
+   * (PNG, PDF, font, etc.) — see `isBinaryFilename`. Mutually exclusive
+   * with `content`.
+   */
+  bytes?: Uint8Array;
   error?: string;
 }
 
@@ -27,8 +34,10 @@ interface ReadCliOptions {
 }
 
 /**
- * Read a file from a realm. Always returns the raw text content.
- * Callers should parse the content themselves if needed (e.g. JSON).
+ * Read a file from a realm. Returns raw text in `content` for text files;
+ * returns raw bytes in `bytes` for binary files (PNG / PDF / font / etc.,
+ * per `isBinaryFilename`). Callers should parse the content themselves
+ * if needed (e.g. JSON).
  *
  * Uses the per-realm JWT via `ProfileManager.authedRealmFetch`.
  */
@@ -70,6 +79,11 @@ export async function read(
     };
   }
 
+  if (isBinaryFilename(path)) {
+    let bytes = new Uint8Array(await response.arrayBuffer());
+    return { ok: true, status: response.status, bytes };
+  }
+
   let text = await response.text();
   return { ok: true, status: response.status, content: text };
 }
@@ -96,9 +110,30 @@ export function registerReadCommand(parent: Command): void {
       }
 
       if (opts.json) {
-        cliLog.output(JSON.stringify(result, null, 2));
+        let serializable: Record<string, unknown> = {
+          ok: result.ok,
+          status: result.status,
+          error: result.error,
+        };
+        if (result.content !== undefined) {
+          serializable.content = result.content;
+        }
+        if (result.bytes !== undefined) {
+          // Buffer.from(typedArray) shares memory, then toString('base64')
+          // copies into a base64 string — fine for the JSON output path.
+          serializable.bytesBase64 = Buffer.from(
+            result.bytes.buffer,
+            result.bytes.byteOffset,
+            result.bytes.byteLength,
+          ).toString('base64');
+        }
+        cliLog.output(JSON.stringify(serializable, null, 2));
       } else if (result.ok) {
-        cliLog.output(result.content ?? '');
+        if (result.bytes !== undefined) {
+          process.stdout.write(result.bytes);
+        } else {
+          cliLog.output(result.content ?? '');
+        }
       } else {
         console.error(
           `${DIM}Status:${RESET} ${result.status ?? '(no status)'}`,

packages/boxel-cli/src/commands/file/write.ts

@@ -7,6 +7,7 @@ import {
 } from '../../lib/profile-manager';
 import { ensureTrailingSlash } from '@cardstack/runtime-common/paths';
 import { SupportedMimeType } from '@cardstack/runtime-common/supported-mime-type';
+import { isBinaryFilename } from '@cardstack/runtime-common/infer-content-type';
 import { FG_GREEN, FG_RED, DIM, RESET } from '../../lib/colors';
 import { cliLog } from '../../lib/cli-log';
 
@@ -26,15 +27,19 @@ interface WriteCliOptions {
 }
 
 /**
- * Write a file to a realm. Content is sent as-is with card+source MIME type.
- * Path should include the file extension.
+ * Write a file to a realm. Path should include the file extension.
+ *
+ * String content is sent with the card+source MIME type (the text path
+ * .gts / .json / .md / etc. always took). Binary content (a `Uint8Array`,
+ * including the `Buffer` subclass) is sent with `application/octet-stream`,
+ * which the realm-server routes to `upsertBinaryFile` and writes verbatim.
  *
  * Uses the per-realm JWT via `ProfileManager.authedRealmFetch`.
  */
 export async function write(
   realmUrl: string,
   path: string,
-  content: string,
+  content: string | Uint8Array,
   options?: WriteCommandOptions,
 ): Promise<WriteResult> {
   let pm = options?.profileManager ?? getProfileManager();
@@ -47,15 +52,38 @@ export async function write(
   }
 
   let url = new URL(path, ensureTrailingSlash(realmUrl)).href;
+  let isBinary = typeof content !== 'string';
+
+  // Defense-in-depth for programmatic callers (BoxelClient.write, tests).
+  // The CLI wrapper has an earlier guard against `--file image.png` →
+  // `notes.md` style misuse, but the library function is also reachable
+  // without going through that branch. Reject the mismatch here so raw
+  // bytes never land at a text extension (corrupt-on-read) and a UTF-8
+  // string never lands at a binary extension (corrupt-on-write).
+  let pathIsBinary = isBinaryFilename(path);
+  if (pathIsBinary !== isBinary) {
+    return {
+      ok: false,
+      error:
+        `Path ${path} is ${pathIsBinary ? 'binary' : 'text'} by extension ` +
+        `but content is ${isBinary ? 'bytes' : 'a string'}. ` +
+        `Refusing to write to avoid silent corruption.`,
+    };
+  }
 
   try {
     let response = await pm.authedRealmFetch(url, {
       method: 'POST',
-      headers: {
-        Accept: SupportedMimeType.CardSource,
-        'Content-Type': SupportedMimeType.CardSource,
-      },
-      body: content,
+      headers: isBinary
+        ? { 'Content-Type': SupportedMimeType.OctetStream }
+        : {
+            Accept: SupportedMimeType.CardSource,
+            'Content-Type': SupportedMimeType.CardSource,
+          },
+      // Both branches of `content: string | Uint8Array` are valid
+      // BodyInit values, but TS narrows them as a union that doesn't
+      // unify against the fetch signature without a hint.
+      body: content as BodyInit,
     });
 
     if (!response.ok) {
@@ -103,10 +131,30 @@ export function registerWriteCommand(parent: Command): void {
     )
     .option('--json', 'Output raw JSON response')
     .action(async (filePath: string, opts: WriteCliOptions) => {
-      let content: string;
+      let content: string | Uint8Array;
       if (opts.file) {
+        // Refuse a source/destination binary-classification mismatch
+        // (e.g., `write notes.md --file image.png`) — otherwise raw
+        // bytes would land at a text extension and corrupt-on-read.
+        const srcIsBinary = isBinaryFilename(opts.file);
+        const dstIsBinary = isBinaryFilename(filePath);
+        if (srcIsBinary !== dstIsBinary) {
+          stderr(
+            `${FG_RED}Error:${RESET} source file ${opts.file} is ${
+              srcIsBinary ? 'binary' : 'text'
+            } but destination path ${filePath} is ${
+              dstIsBinary ? 'binary' : 'text'
+            }. Refusing to write to avoid silent corruption — rename the destination to match.`,
+          );
+          process.exit(1);
+        }
         try {
-          content = readFileSync(opts.file, 'utf-8');
+          // Binary source files are read as raw bytes so write() can
+          // hand them to the realm unchanged; forcing utf-8 would
+          // corrupt PNG / PDF / font / etc. payloads silently.
+          content = srcIsBinary
+            ? readFileSync(opts.file)
+            : readFileSync(opts.file, 'utf-8');
         } catch (err) {
           stderr(
             `${FG_RED}Error:${RESET} Could not read file: ${err instanceof Error ? err.message : String(err)}`,

packages/boxel-cli/src/commands/realm/push.ts

@@ -200,6 +200,23 @@ class RealmPusher extends RealmSyncBase {
 
       const result = await this.uploadFilesAtomic(filesToUpload, addPaths);
 
+      // Record every file the server actually wrote before surfacing
+      // errors. uploadFilesAtomic can return both `succeeded` and
+      // `error` when the atomic text batch lands but a per-file
+      // binary POST fails — dropping the manifest update in that
+      // case would force a re-add on the next push (409 cascade).
+      if (result.succeeded.length > 0) {
+        const uploaded = await Promise.all(
+          result.succeeded.map(async (rel) => ({
+            rel,
+            hash: await computeFileHash(filesToUpload.get(rel)!),
+          })),
+        );
+        for (const { rel, hash } of uploaded) {
+          newManifest.files[rel] = hash;
+        }
+      }
+
       if (result.error) {
         uploadFailed = true;
         this.hasError = true;
@@ -215,16 +232,6 @@ class RealmPusher extends RealmSyncBase {
           }
           console.error(`  ${hint}`);
         }
-      } else if (result.succeeded.length > 0) {
-        const uploaded = await Promise.all(
-          result.succeeded.map(async (rel) => ({
-            rel,
-            hash: await computeFileHash(filesToUpload.get(rel)!),
-          })),
-        );
-        for (const { rel, hash } of uploaded) {
-          newManifest.files[rel] = hash;
-        }
       }
     }
 
@@ -270,7 +277,11 @@ class RealmPusher extends RealmSyncBase {
       }
     }
 
-    if (!this.options.dryRun && !uploadFailed && filesToUpload.size > 0) {
+    // Refresh mtimes and save the manifest even on partial failure —
+    // newManifest.files only contains files the server actually wrote
+    // (unchanged carry-overs + succeeded uploads), so persisting it
+    // is always safe and avoids re-uploading text files that landed.
+    if (!this.options.dryRun && filesToUpload.size > 0) {
       try {
         const freshMtimes = await this.getRemoteMtimes();
         for (const rel of Object.keys(newManifest.files)) {
@@ -291,7 +302,7 @@ class RealmPusher extends RealmSyncBase {
       delete newManifest.remoteMtimes;
     }
 
-    if (!this.options.dryRun && !uploadFailed) {
+    if (!this.options.dryRun) {
       await saveManifest(this.options.localDir, newManifest);
     }
 

packages/boxel-cli/src/commands/realm/sync.ts

@@ -314,14 +314,16 @@ class RealmSyncer extends RealmSyncBase {
       }
 
       const result = await this.uploadFilesAtomic(filesToUpload, addPaths);
+      // Record every file the server actually wrote, even when other
+      // files in the same batch failed — see push.ts for the symmetric
+      // reasoning.
+      this.pushedFiles.push(...result.succeeded);
       if (result.error) {
         this.hasError = true;
         console.error(result.error.message);
         for (const entry of result.error.perFile) {
           console.error(`  ${entry.path}: ${entry.title}`);
         }
-      } else {
-        this.pushedFiles.push(...result.succeeded);
       }
     }
 
@@ -371,8 +373,12 @@ class RealmSyncer extends RealmSyncBase {
       );
     }
 
-    // Phase 6: Update manifest
-    if (!this.options.dryRun && !this.hasError) {
+    // Phase 6: Update manifest. Persist even on partial failure — we
+    // only record hashes for files the server actually wrote
+    // (pushedFiles + pulledFiles), so the manifest stays consistent
+    // with the realm and the next sync won't re-attempt successful
+    // files.
+    if (!this.options.dryRun) {
       // Build updated hashes from prior manifest + current local files + executed ops.
       // Start with the previous manifest so that files deleted locally but not
       // propagated (no --delete) retain their entries and aren't re-pulled next sync.