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>
This commit is contained in:
CloudCLI UI Contributors
2026-06-29 01:44:15 +00:00
parent 1df336ca2d
commit 92b5b935fd
3 changed files with 97 additions and 30 deletions

View File

@@ -44,13 +44,17 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
// Natively previewable media (image/pdf/audio/video) is rendered by // Natively previewable media (image/pdf/audio/video) is rendered by
// CodeEditorMediaPreview, so there is nothing to read as text here. // 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)) { if (getPreviewKind(file.name)) {
setContent('');
setLoading(false); setLoading(false);
return; return;
} }
// Check if file is binary by extension // Check if file is binary by extension
if (isBinaryFile(file.name)) { if (isBinaryFile(file.name)) {
setContent('');
setIsBinary(true); setIsBinary(true);
setLoading(false); setLoading(false);
return; return;
@@ -87,6 +91,12 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
}, [file.diffInfo, file.name, fileDiffNewString, fileDiffOldString, fileName, filePath, fileProjectId]); }, [file.diffInfo, file.name, fileDiffNewString, fileDiffOldString, fileName, filePath, fileProjectId]);
const handleSave = useCallback(async () => { 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); setSaving(true);
setSaveError(null); setSaveError(null);
@@ -120,7 +130,7 @@ export const useCodeEditorDocument = ({ file, projectPath }: UseCodeEditorDocume
} finally { } finally {
setSaving(false); setSaving(false);
} }
}, [content, filePath, fileProjectId]); }, [content, filePath, fileProjectId, previewKind, fileName]);
const handleDownload = useCallback(() => { const handleDownload = useCallback(() => {
const blob = new Blob([content], { type: 'text/plain' }); const blob = new Blob([content], { type: 'text/plain' });

View File

@@ -5,27 +5,59 @@
export type PreviewKind = 'image' | 'pdf' | 'video' | 'audio'; export type PreviewKind = 'image' | 'pdf' | 'video' | 'audio';
// Formats browsers can decode in <img>. // Single source of truth: every extension the browser can preview, mapped to the
const IMAGE_EXTENSIONS = new Set([ // MIME type we apply when the server response has a missing/generic Content-Type.
'png', 'jpg', 'jpeg', 'gif', 'svg', 'webp', 'ico', 'bmp', 'avif', 'apng', // 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 const kindForMime = (mime: string): PreviewKind | null => {
// formats browsers generally can't play (avi, mkv, flv, wmv). if (mime === 'application/pdf') return 'pdf';
const VIDEO_EXTENSIONS = new Set(['mp4', 'webm', 'ogv', 'mov', 'm4v']); if (mime.startsWith('image/')) return 'image';
if (mime.startsWith('video/')) return 'video';
// Formats broadly playable in <audio>. if (mime.startsWith('audio/')) return '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';
return null; 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)];

View File

@@ -1,7 +1,7 @@
import { useEffect, useState } from 'react'; import { useEffect, useState } from 'react';
import { authenticatedFetch } from '../../../../utils/api'; import { authenticatedFetch } from '../../../../utils/api';
import type { CodeEditorFile } from '../../types/types'; import type { CodeEditorFile } from '../../types/types';
import type { PreviewKind } from '../../utils/previewableFile'; import { getPreviewMimeType, type PreviewKind } from '../../utils/previewableFile';
type CodeEditorMediaPreviewProps = { type CodeEditorMediaPreviewProps = {
file: CodeEditorFile; file: CodeEditorFile;
@@ -23,13 +23,14 @@ type CodeEditorMediaPreviewProps = {
}; };
}; };
// MIME type forced onto the blob per kind so the browser picks the right viewer // Reject a "PDF" whose bytes aren't actually a PDF before handing it to the
// even when the server response was generic (e.g. application/octet-stream). // same-origin iframe, so a mislabeled HTML/SVG file can't run in the app origin.
const FALLBACK_MIME: Record<PreviewKind, string> = { const PDF_HEADER_SCAN_BYTES = 1024;
image: 'application/octet-stream',
pdf: 'application/pdf', const looksLikePdf = async (blob: Blob): Promise<boolean> => {
video: 'video/mp4', const header = await blob.slice(0, PDF_HEADER_SCAN_BYTES).arrayBuffer();
audio: 'audio/mpeg', // 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({ export default function CodeEditorMediaPreview({
@@ -73,7 +74,27 @@ export default function CodeEditorMediaPreview({
} }
const blob = await response.blob(); 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); objectUrl = URL.createObjectURL(typed);
setUrl(objectUrl); setUrl(objectUrl);
} catch (loadError: unknown) { } catch (loadError: unknown) {
@@ -109,6 +130,10 @@ export default function CodeEditorMediaPreview({
/> />
); );
case 'pdf': 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" />; return <iframe src={url} title={file.name} className="h-full w-full border-0 bg-white" />;
case 'video': case 'video':
return ( return (