From 92b5b935fd853bd774fe7d86da428a551cfe47cf Mon Sep 17 00:00:00 2001
From: CloudCLI UI Contributors <guptadivam@gmail.com>
Date: Mon, 29 Jun 2026 01:44:15 +0000
Subject: [PATCH] fix(file-viewer): address review feedback on media preview

- Never write preview-only or binary files: handleSave is a no-op when
  previewKind/isBinary, and the text buffer is cleared when switching to a
  media/binary file, so Cmd/Ctrl+S can no longer post a stale or empty buffer
  over the file on disk (CodeRabbit: Data Integrity, Critical).
- Choose the fallback MIME type per file extension (single source of truth in
  previewableFile.ts) instead of per preview-kind, and only override when the
  server Content-Type is missing or generic, so webm/ogv/ogg/flac/svg render
  correctly (CodeRabbit: Functional Correctness, Major).
- Harden the PDF iframe: validate the %PDF- magic bytes and pin the blob MIME
  to application/pdf so a mislabeled HTML/SVG file can't execute in the app
  origin. A sandbox attribute is intentionally not used because Chromium's
  built-in PDF viewer will not load inside any sandboxed frame (CodeRabbit:
  Security, Critical).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../hooks/useCodeEditorDocument.ts            | 12 +++-
 .../code-editor/utils/previewableFile.ts      | 72 +++++++++++++------
 .../subcomponents/CodeEditorMediaPreview.tsx  | 43 ++++++++---
 3 files changed, 97 insertions(+), 30 deletions(-)

diff --git a/src/components/code-editor/hooks/useCodeEditorDocument.ts b/src/components/code-editor/hooks/useCodeEditorDocument.ts
index 74f928d6..dda02887 100644
--- a/src/components/code-editor/hooks/useCodeEditorDocument.ts
+++ b/src/components/code-editor/hooks/useCodeEditorDocument.ts
@@ -44,13 +44,17 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
 
         // Natively previewable media (image/pdf/audio/video) is rendered by
         // CodeEditorMediaPreview, so there is nothing to read as text here.
+        // Clear any buffer left over from a previously opened text file so a
+        // stray save can't write stale content over the binary file.
         if (getPreviewKind(file.name)) {
+          setContent('');
           setLoading(false);
           return;
         }
 
         // Check if file is binary by extension
         if (isBinaryFile(file.name)) {
+          setContent('');
           setIsBinary(true);
           setLoading(false);
           return;
@@ -87,6 +91,12 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
   }, [file.diffInfo, file.name, fileDiffNewString, fileDiffOldString, fileName, filePath, fileProjectId]);
 
   const handleSave = useCallback(async () => {
+    // Preview-only and binary files have no editable text buffer; never write
+    // them back (e.g. via Cmd/Ctrl+S) or we'd corrupt the file on disk.
+    if (previewKind || isBinaryFile(fileName)) {
+      return;
+    }
+
     setSaving(true);
     setSaveError(null);
 
@@ -120,7 +130,7 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
     } finally {
       setSaving(false);
     }
-  }, [content, filePath, fileProjectId]);
+  }, [content, filePath, fileProjectId, previewKind, fileName]);
 
   const handleDownload = useCallback(() => {
     const blob = new Blob([content], { type: 'text/plain' });
diff --git a/src/components/code-editor/utils/previewableFile.ts b/src/components/code-editor/utils/previewableFile.ts
index 464cad81..0071de4e 100644
--- a/src/components/code-editor/utils/previewableFile.ts
+++ b/src/components/code-editor/utils/previewableFile.ts
@@ -5,27 +5,59 @@
 
 export type PreviewKind = 'image' | 'pdf' | 'video' | 'audio';
 
-// Formats browsers can decode in <img>.
-const IMAGE_EXTENSIONS = new Set([
-  'png', 'jpg', 'jpeg', 'gif', 'svg', 'webp', 'ico', 'bmp', 'avif', 'apng',
-]);
+// Single source of truth: every extension the browser can preview, mapped to the
+// MIME type we apply when the server response has a missing/generic Content-Type.
+// The preview kind is derived from the MIME type so the two never drift apart.
+// Formats browsers generally can't play (avi, mkv, flv, wmv) are intentionally
+// absent and keep the binary fallback.
+const EXTENSION_MIME: Record<string, string> = {
+  // Images
+  png: 'image/png',
+  jpg: 'image/jpeg',
+  jpeg: 'image/jpeg',
+  gif: 'image/gif',
+  svg: 'image/svg+xml',
+  webp: 'image/webp',
+  ico: 'image/x-icon',
+  bmp: 'image/bmp',
+  avif: 'image/avif',
+  apng: 'image/apng',
+  // PDF
+  pdf: 'application/pdf',
+  // Video
+  mp4: 'video/mp4',
+  webm: 'video/webm',
+  ogv: 'video/ogg',
+  mov: 'video/quicktime',
+  m4v: 'video/x-m4v',
+  // Audio
+  mp3: 'audio/mpeg',
+  wav: 'audio/wav',
+  m4a: 'audio/mp4',
+  aac: 'audio/aac',
+  flac: 'audio/flac',
+  opus: 'audio/opus',
+  oga: 'audio/ogg',
+  ogg: 'audio/ogg',
+  weba: 'audio/webm',
+};
 
-const PDF_EXTENSIONS = new Set(['pdf']);
+const extensionOf = (filename: string): string => filename.split('.').pop()?.toLowerCase() ?? '';
 
-// Container/codec combos broadly playable in <video>. Intentionally excludes
-// formats browsers generally can't play (avi, mkv, flv, wmv).
-const VIDEO_EXTENSIONS = new Set(['mp4', 'webm', 'ogv', 'mov', 'm4v']);
-
-// Formats broadly playable in <audio>.
-const AUDIO_EXTENSIONS = new Set([
-  'mp3', 'wav', 'm4a', 'aac', 'flac', 'opus', 'oga', 'ogg', 'weba',
-]);
-
-export const getPreviewKind = (filename: string): PreviewKind | null => {
-  const ext = filename.split('.').pop()?.toLowerCase() ?? '';
-  if (IMAGE_EXTENSIONS.has(ext)) return 'image';
-  if (PDF_EXTENSIONS.has(ext)) return 'pdf';
-  if (VIDEO_EXTENSIONS.has(ext)) return 'video';
-  if (AUDIO_EXTENSIONS.has(ext)) return 'audio';
+const kindForMime = (mime: string): PreviewKind | null => {
+  if (mime === 'application/pdf') return 'pdf';
+  if (mime.startsWith('image/')) return 'image';
+  if (mime.startsWith('video/')) return 'video';
+  if (mime.startsWith('audio/')) return 'audio';
   return null;
 };
+
+export const getPreviewKind = (filename: string): PreviewKind | null => {
+  const mime = EXTENSION_MIME[extensionOf(filename)];
+  return mime ? kindForMime(mime) : null;
+};
+
+// MIME type to fall back to when the server returns no/generic Content-Type.
+// Returns undefined for non-previewable extensions.
+export const getPreviewMimeType = (filename: string): string | undefined =>
+  EXTENSION_MIME[extensionOf(filename)];
diff --git a/src/components/code-editor/view/subcomponents/CodeEditorMediaPreview.tsx b/src/components/code-editor/view/subcomponents/CodeEditorMediaPreview.tsx
index 25b25127..de436837 100644
--- a/src/components/code-editor/view/subcomponents/CodeEditorMediaPreview.tsx
+++ b/src/components/code-editor/view/subcomponents/CodeEditorMediaPreview.tsx
@@ -1,7 +1,7 @@
 import { useEffect, useState } from 'react';
 import { authenticatedFetch } from '../../../../utils/api';
 import type { CodeEditorFile } from '../../types/types';
-import type { PreviewKind } from '../../utils/previewableFile';
+import { getPreviewMimeType, type PreviewKind } from '../../utils/previewableFile';
 
 type CodeEditorMediaPreviewProps = {
   file: CodeEditorFile;
@@ -23,13 +23,14 @@ type CodeEditorMediaPreviewProps = {
   };
 };
 
-// MIME type forced onto the blob per kind so the browser picks the right viewer
-// even when the server response was generic (e.g. application/octet-stream).
-const FALLBACK_MIME: Record<PreviewKind, string> = {
-  image: 'application/octet-stream',
-  pdf: 'application/pdf',
-  video: 'video/mp4',
-  audio: 'audio/mpeg',
+// Reject a "PDF" whose bytes aren't actually a PDF before handing it to the
+// same-origin iframe, so a mislabeled HTML/SVG file can't run in the app origin.
+const PDF_HEADER_SCAN_BYTES = 1024;
+
+const looksLikePdf = async (blob: Blob): Promise<boolean> => {
+  const header = await blob.slice(0, PDF_HEADER_SCAN_BYTES).arrayBuffer();
+  // PDFs must contain the "%PDF-" marker at the very start of the file.
+  return new TextDecoder('latin1').decode(header).includes('%PDF-');
 };
 
 export default function CodeEditorMediaPreview({
@@ -73,7 +74,27 @@ export default function CodeEditorMediaPreview({
         }
 
         const blob = await response.blob();
-        const typed = blob.type ? blob : new Blob([blob], { type: FALLBACK_MIME[kind] });
+
+        // Pick the MIME type to expose to the browser. Preserve a valid
+        // Content-Type from the server, but supply an extension-specific
+        // default when it is missing or generic (application/octet-stream),
+        // otherwise formats like webm/ogg/flac/svg won't render.
+        const fallbackMime = getPreviewMimeType(file.name);
+        const isGenericType = !blob.type || blob.type === 'application/octet-stream';
+        let outType = isGenericType ? (fallbackMime ?? blob.type) : blob.type;
+
+        if (kind === 'pdf') {
+          // The PDF renders in a same-origin <iframe>, so verify the bytes are
+          // really a PDF and pin the type to application/pdf. That forces the
+          // browser's PDF handler and prevents a mislabeled HTML/SVG file from
+          // executing scripts in the app's origin.
+          if (!(await looksLikePdf(blob))) {
+            throw new Error('File is not a valid PDF');
+          }
+          outType = 'application/pdf';
+        }
+
+        const typed = outType && outType !== blob.type ? new Blob([blob], { type: outType }) : blob;
         objectUrl = URL.createObjectURL(typed);
         setUrl(objectUrl);
       } catch (loadError: unknown) {
@@ -109,6 +130,10 @@ export default function CodeEditorMediaPreview({
           />
         );
       case 'pdf':
+        // Not sandboxed on purpose: the browser's built-in PDF viewer refuses to
+        // load inside a sandboxed frame (any `sandbox` value yields a broken
+        // viewer). Script execution is instead prevented upstream by validating
+        // the PDF magic bytes and pinning the blob's MIME type to application/pdf.
         return <iframe src={url} title={file.name} className="h-full w-full border-0 bg-white" />;
       case 'video':
         return (