diff --git a/frontend/.eslintrc.js b/frontend/.eslintrc.js new file mode 100644 index 000000000..cd779cd72 --- /dev/null +++ b/frontend/.eslintrc.js @@ -0,0 +1,55 @@ +module.exports = { + extends: [ + 'react-app', + 'react-app/jest' + ], + rules: { + // Custom rules to prevent dangerous file.name as ID patterns + 'no-file-name-as-id': 'error', + 'prefer-file-with-id': 'warn' + }, + overrides: [ + { + files: ['**/*.ts', '**/*.tsx'], + rules: { + // Prevent file.name being used where FileId is expected + 'no-restricted-syntax': [ + 'error', + { + selector: 'MemberExpression[object.name="file"][property.name="name"]', + message: 'Avoid using file.name directly. Use FileWithId.fileId or safeGetFileId() instead to prevent ID collisions.' + }, + { + selector: 'CallExpression[callee.name="createOperation"] > ArrayExpression > CallExpression[callee.property.name="map"] > ArrowFunctionExpression > MemberExpression[object.name="f"][property.name="name"]', + message: 'Dangerous pattern: Using file.name as ID in createOperation. Use FileWithId.fileId instead.' + }, + { + selector: 'ArrayExpression[elements.length>0] CallExpression[callee.property.name="map"] > ArrowFunctionExpression > MemberExpression[property.name="name"]', + message: 'Potential file.name as ID usage detected. Ensure proper FileId usage instead of file.name.' + } + ] + } + } + ], + settings: { + // Custom settings for our file ID validation + 'file-id-validation': { + // Functions that should only accept FileId, not strings + 'file-id-only-functions': [ + 'recordOperation', + 'markOperationApplied', + 'markOperationFailed', + 'removeFiles', + 'updateFileRecord', + 'pinFile', + 'unpinFile' + ], + // Functions that should accept FileWithId instead of File + 'file-with-id-functions': [ + 'createOperation', + 'executeOperation', + 'isFilePinned' + ] + } + } +}; \ No newline at end of file diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index ef4d663f6..9b2d23698 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -13,6 +13,9 @@ import "./styles/tailwind.css"; import "./index.css"; import { RightRailProvider } from "./contexts/RightRailContext"; +// Import file ID safety validators (development only) +import "./utils/fileIdSafety"; + // Loading component for i18next suspense const LoadingFallback = () => (
0) { - // Record upload operations for PDF files - for (const file of allExtractedFiles) { - const operationId = `upload-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; - const operation: FileOperation = { - id: operationId, - type: 'upload', - timestamp: Date.now(), - fileIds: [file.name], - status: 'pending', - metadata: { - originalFileName: file.name, - fileSize: file.size, - parameters: { - uploadMethod: 'drag-drop' - } - } - }; - } - // Add files to context (they will be processed automatically) await addFiles(allExtractedFiles); setStatus(`Added ${allExtractedFiles.length} files`); diff --git a/frontend/src/components/shared/FileGrid.tsx b/frontend/src/components/shared/FileGrid.tsx index 169e19f88..ea0e70818 100644 --- a/frontend/src/components/shared/FileGrid.tsx +++ b/frontend/src/components/shared/FileGrid.tsx @@ -123,8 +123,13 @@ const FileGrid = ({ style={{ overflowY: "auto", width: "100%" }} > {displayFiles.map((item, idx) => { - const fileId = item.record?.id || item.file.name; - const originalIdx = files.findIndex(f => (f.record?.id || f.file.name) === fileId); + // Use record ID if available, otherwise throw error for missing FileRecord + if (!item.record?.id) { + console.error('FileGrid: File missing FileRecord with proper ID:', item.file.name); + return null; // Skip rendering files without proper IDs + } + const fileId = item.record.id; + const originalIdx = files.findIndex(f => f.record?.id === fileId); const supported = isFileSupported ? isFileSupported(item.file.name) : true; return ( => { + const addRawFiles = useCallback(async (files: File[], options?: { insertAfterPageId?: string }): Promise => { const addedFilesWithIds = await addFiles('raw', { files, ...options }, stateRef, filesRef, dispatch, lifecycleManager); // Persist to IndexedDB if enabled @@ -87,56 +89,40 @@ function FileContextInner({ })); } - return addedFilesWithIds.map(({ file }) => file); + // Convert to FileWithId objects + return addedFilesWithIds.map(({ file, id }) => createFileWithId(file, id)); }, [indexedDB, enablePersistence]); - const addProcessedFiles = useCallback(async (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>): Promise => { + const addProcessedFiles = useCallback(async (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>): Promise => { const result = await addFiles('processed', { filesWithThumbnails }, stateRef, filesRef, dispatch, lifecycleManager); - return result.map(({ file }) => file); + // Convert to FileWithId objects + return result.map(({ file, id }) => createFileWithId(file, id)); }, []); - const addStoredFiles = useCallback(async (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: any }>): Promise => { + const addStoredFiles = useCallback(async (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: any }>): Promise => { const result = await addFiles('stored', { filesWithMetadata }, stateRef, filesRef, dispatch, lifecycleManager); - return result.map(({ file }) => file); + // Convert to FileWithId objects + return result.map(({ file, id }) => createFileWithId(file, id)); }, []); // Action creators const baseActions = useMemo(() => createFileActions(dispatch), []); // Helper functions for pinned files - const consumeFilesWrapper = useCallback(async (inputFileIds: FileId[], outputFiles: File[]): Promise => { - return consumeFiles(inputFileIds, outputFiles, stateRef, filesRef, dispatch); + const consumeFilesWrapper = useCallback(async (inputFileIds: FileId[], outputFiles: File[]): Promise => { + const result = await consumeFiles(inputFileIds, outputFiles, stateRef, filesRef, dispatch); + // Convert results to FileWithId objects + return result.map(({ file, id }) => createFileWithId(file, id)); }, []); - // Helper to find FileId from File object - const findFileId = useCallback((file: File): FileId | undefined => { - return Object.keys(stateRef.current.files.byId).find(id => { - const storedFile = filesRef.current.get(id); - return storedFile && - storedFile.name === file.name && - storedFile.size === file.size && - storedFile.lastModified === file.lastModified; - }); - }, []); + // File pinning functions - now use FileWithId directly + const pinFileWrapper = useCallback((file: FileWithId) => { + baseActions.pinFile(file.fileId); + }, [baseActions]); - // File-to-ID wrapper functions for pinning - const pinFileWrapper = useCallback((file: File) => { - const fileId = findFileId(file); - if (fileId) { - baseActions.pinFile(fileId); - } else { - console.warn('File not found for pinning:', file.name); - } - }, [baseActions, findFileId]); - - const unpinFileWrapper = useCallback((file: File) => { - const fileId = findFileId(file); - if (fileId) { - baseActions.unpinFile(fileId); - } else { - console.warn('File not found for unpinning:', file.name); - } - }, [baseActions, findFileId]); + const unpinFileWrapper = useCallback((file: FileWithId) => { + baseActions.unpinFile(file.fileId); + }, [baseActions]); // Complete actions object const actions = useMemo(() => ({ diff --git a/frontend/src/contexts/file/fileActions.ts b/frontend/src/contexts/file/fileActions.ts index c74fa66a5..a623e1901 100644 --- a/frontend/src/contexts/file/fileActions.ts +++ b/frontend/src/contexts/file/fileActions.ts @@ -326,11 +326,11 @@ export async function consumeFiles( stateRef: React.MutableRefObject, filesRef: React.MutableRefObject>, dispatch: React.Dispatch -): Promise { +): Promise> { if (DEBUG) console.log(`📄 consumeFiles: Processing ${inputFileIds.length} input files, ${outputFiles.length} output files`); // Process output files through the 'processed' path to generate thumbnails - const outputFileRecords = await Promise.all( + const processedOutputs: Array<{ file: File; id: FileId; thumbnail?: string; record: FileRecord }> = await Promise.all( outputFiles.map(async (file) => { const fileId = createFileId(); filesRef.current.set(fileId, file); @@ -357,10 +357,13 @@ export async function consumeFiles( record.processedFile = createProcessedFile(pageCount, thumbnail); } - return record; + return { file, id: fileId, thumbnail, record }; }) ); + // Extract records for dispatch + const outputFileRecords = processedOutputs.map(({ record }) => record); + // Dispatch the consume action dispatch({ type: 'CONSUME_FILES', @@ -371,6 +374,9 @@ export async function consumeFiles( }); if (DEBUG) console.log(`📄 consumeFiles: Successfully consumed files - removed ${inputFileIds.length} inputs, added ${outputFileRecords.length} outputs`); + + // Return file data for FileWithId conversion + return processedOutputs.map(({ file, id, thumbnail }) => ({ file, id, thumbnail })); } /** diff --git a/frontend/src/contexts/file/fileHooks.ts b/frontend/src/contexts/file/fileHooks.ts index 19452958f..712a96e99 100644 --- a/frontend/src/contexts/file/fileHooks.ts +++ b/frontend/src/contexts/file/fileHooks.ts @@ -168,16 +168,7 @@ export function useFileContext() { markOperationApplied: (fileId: string, operationId: string) => {}, // Operation tracking not implemented markOperationFailed: (fileId: string, operationId: string, error: string) => {}, // Operation tracking not implemented - // File ID lookup - findFileId: (file: File) => { - return state.files.ids.find(id => { - const record = state.files.byId[id]; - return record && - record.name === file.name && - record.size === file.size && - record.lastModified === file.lastModified; - }); - }, + // File ID lookup removed - use FileWithId.fileId directly for better performance and type safety // Pinned files pinnedFiles: state.pinnedFiles, diff --git a/frontend/src/contexts/file/fileSelectors.ts b/frontend/src/contexts/file/fileSelectors.ts index 5579aa3c7..88494ebc5 100644 --- a/frontend/src/contexts/file/fileSelectors.ts +++ b/frontend/src/contexts/file/fileSelectors.ts @@ -6,7 +6,9 @@ import { FileId, FileRecord, FileContextState, - FileContextSelectors + FileContextSelectors, + FileWithId, + createFileWithId } from '../../types/fileContext'; /** @@ -17,11 +19,19 @@ export function createFileSelectors( filesRef: React.MutableRefObject> ): FileContextSelectors { return { - getFile: (id: FileId) => filesRef.current.get(id), + getFile: (id: FileId) => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }, getFiles: (ids?: FileId[]) => { const currentIds = ids || stateRef.current.files.ids; - return currentIds.map(id => filesRef.current.get(id)).filter(Boolean) as File[]; + return currentIds + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getFileRecord: (id: FileId) => stateRef.current.files.byId[id], @@ -35,8 +45,11 @@ export function createFileSelectors( getSelectedFiles: () => { return stateRef.current.ui.selectedFileIds - .map(id => filesRef.current.get(id)) - .filter(Boolean) as File[]; + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getSelectedFileRecords: () => { @@ -52,8 +65,11 @@ export function createFileSelectors( getPinnedFiles: () => { return Array.from(stateRef.current.pinnedFiles) - .map(id => filesRef.current.get(id)) - .filter(Boolean) as File[]; + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getPinnedFileRecords: () => { @@ -62,16 +78,8 @@ export function createFileSelectors( .filter(Boolean); }, - isFilePinned: (file: File) => { - // Find FileId by matching File object properties - const fileId = Object.keys(stateRef.current.files.byId).find(id => { - const storedFile = filesRef.current.get(id); - return storedFile && - storedFile.name === file.name && - storedFile.size === file.size && - storedFile.lastModified === file.lastModified; - }); - return fileId ? stateRef.current.pinnedFiles.has(fileId) : false; + isFilePinned: (file: FileWithId) => { + return stateRef.current.pinnedFiles.has(file.fileId); }, // Stable signature for effects - prevents unnecessary re-renders @@ -119,12 +127,15 @@ export function buildQuickKeySetFromMetadata(metadata: Array<{ name: string; siz export function getPrimaryFile( stateRef: React.MutableRefObject, filesRef: React.MutableRefObject> -): { file?: File; record?: FileRecord } { +): { file?: FileWithId; record?: FileRecord } { const primaryFileId = stateRef.current.files.ids[0]; if (!primaryFileId) return {}; + const file = filesRef.current.get(primaryFileId); + const record = stateRef.current.files.byId[primaryFileId]; + return { - file: filesRef.current.get(primaryFileId), - record: stateRef.current.files.byId[primaryFileId] + file: file ? createFileWithId(file, primaryFileId) : undefined, + record }; } \ No newline at end of file diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index abce8bedf..a31d802e4 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -7,6 +7,7 @@ import { useToolApiCalls, type ApiCallsConfig } from './useToolApiCalls'; import { useToolResources } from './useToolResources'; import { extractErrorMessage } from '../../../utils/toolErrorHandler'; import { createOperation } from '../../../utils/toolOperationTracker'; +import { FileWithId, extractFiles } from '../../../types/fileContext'; import { ResponseHandler } from '../../../utils/toolResponseProcessor'; // Re-export for backwards compatibility @@ -82,7 +83,7 @@ export interface ToolOperationHook { progress: ProcessingProgress | null; // Actions - executeOperation: (params: TParams, selectedFiles: File[]) => Promise; + executeOperation: (params: TParams, selectedFiles: FileWithId[]) => Promise; resetResults: () => void; clearError: () => void; cancelOperation: () => void; @@ -107,7 +108,7 @@ export const useToolOperation = ( config: ToolOperationConfig ): ToolOperationHook => { const { t } = useTranslation(); - const { recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles, findFileId } = useFileContext(); + const { recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles } = useFileContext(); // Composed hooks const { state, actions } = useToolState(); @@ -116,7 +117,7 @@ export const useToolOperation = ( const executeOperation = useCallback(async ( params: TParams, - selectedFiles: File[] + selectedFiles: FileWithId[] ): Promise => { // Validation if (selectedFiles.length === 0) { @@ -130,7 +131,7 @@ export const useToolOperation = ( return; } - // Setup operation tracking + // Setup operation tracking with proper FileWithId const { operation, operationId, fileId } = createOperation(config.operationType, params, selectedFiles); recordOperation(fileId, operation); @@ -143,15 +144,18 @@ export const useToolOperation = ( try { let processedFiles: File[]; + // Convert FileWithId to regular File objects for API processing + const validRegularFiles = extractFiles(validFiles); + if (config.customProcessor) { actions.setStatus('Processing files...'); - processedFiles = await config.customProcessor(params, validFiles); + processedFiles = await config.customProcessor(params, validRegularFiles); } else { // Use explicit multiFileEndpoint flag to determine processing approach if (config.multiFileEndpoint) { // Multi-file processing - single API call with all files actions.setStatus('Processing files...'); - const formData = (config.buildFormData as (params: TParams, files: File[]) => FormData)(params, validFiles); + const formData = (config.buildFormData as (params: TParams, files: File[]) => FormData)(params, validRegularFiles); const endpoint = typeof config.endpoint === 'function' ? config.endpoint(params) : config.endpoint; const response = await axios.post(endpoint, formData, { responseType: 'blob' }); @@ -159,11 +163,11 @@ export const useToolOperation = ( // Multi-file responses are typically ZIP files that need extraction, but some may return single PDFs if (config.responseHandler) { // Use custom responseHandler for multi-file (handles ZIP extraction) - processedFiles = await config.responseHandler(response.data, validFiles); + processedFiles = await config.responseHandler(response.data, validRegularFiles); } else if (response.data.type === 'application/pdf' || (response.headers && response.headers['content-type'] === 'application/pdf')) { // Single PDF response (e.g. split with merge option) - use original filename - const originalFileName = validFiles[0]?.name || 'document.pdf'; + const originalFileName = validRegularFiles[0]?.name || 'document.pdf'; const singleFile = new File([response.data], originalFileName, { type: 'application/pdf' }); processedFiles = [singleFile]; } else { @@ -185,7 +189,7 @@ export const useToolOperation = ( }; processedFiles = await processFiles( params, - validFiles, + validRegularFiles, apiCallsConfig, actions.setProgress, actions.setStatus @@ -208,7 +212,7 @@ export const useToolOperation = ( actions.setDownloadInfo(downloadInfo.url, downloadInfo.filename); // Replace input files with processed files (consumeFiles handles pinning) - const inputFileIds = validFiles.map(file => findFileId(file)).filter(Boolean) as string[]; + const inputFileIds = validFiles.map(file => file.fileId); await consumeFiles(inputFileIds, processedFiles); markOperationApplied(fileId, operationId); @@ -223,7 +227,7 @@ export const useToolOperation = ( actions.setLoading(false); actions.setProgress(null); } - }, [t, config, actions, recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles, findFileId, processFiles, generateThumbnails, createDownloadInfo, cleanupBlobUrls, extractZipFiles, extractAllZipFiles]); + }, [t, config, actions, recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles, processFiles, generateThumbnails, createDownloadInfo, cleanupBlobUrls, extractZipFiles, extractAllZipFiles]); const cancelOperation = useCallback(() => { cancelApiCalls(); diff --git a/frontend/src/hooks/usePDFProcessor.ts b/frontend/src/hooks/usePDFProcessor.ts index ab3b5e007..b88bfbe2b 100644 --- a/frontend/src/hooks/usePDFProcessor.ts +++ b/frontend/src/hooks/usePDFProcessor.ts @@ -1,6 +1,7 @@ import { useState, useCallback } from 'react'; import { PDFDocument, PDFPage } from '../types/pageEditor'; import { pdfWorkerManager } from '../services/pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export function usePDFProcessor() { const [loading, setLoading] = useState(false); @@ -75,7 +76,7 @@ export function usePDFProcessor() { // Create pages without thumbnails initially - load them lazily for (let i = 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, originalPageNumber: i, thumbnail: null, // Will be loaded lazily diff --git a/frontend/src/hooks/useThumbnailGeneration.ts b/frontend/src/hooks/useThumbnailGeneration.ts index 09bec7c9c..bbb599c3b 100644 --- a/frontend/src/hooks/useThumbnailGeneration.ts +++ b/frontend/src/hooks/useThumbnailGeneration.ts @@ -1,5 +1,6 @@ import { useCallback, useRef } from 'react'; import { thumbnailGenerationService } from '../services/thumbnailGenerationService'; +import { createQuickKey } from '../types/fileContext'; // Request queue to handle concurrent thumbnail requests interface ThumbnailRequest { @@ -70,8 +71,8 @@ async function processRequestQueue() { console.log(`📸 Batch generating ${requests.length} thumbnails for pages: ${pageNumbers.slice(0, 5).join(', ')}${pageNumbers.length > 5 ? '...' : ''}`); - // Use file name as fileId for PDF document caching - const fileId = file.name + '_' + file.size + '_' + file.lastModified; + // Use quickKey for PDF document caching (same metadata, consistent format) + const fileId = createQuickKey(file); const results = await thumbnailGenerationService.generateThumbnails( fileId, diff --git a/frontend/src/services/enhancedPDFProcessingService.ts b/frontend/src/services/enhancedPDFProcessingService.ts index f9f067c30..2d4bf4644 100644 --- a/frontend/src/services/enhancedPDFProcessingService.ts +++ b/frontend/src/services/enhancedPDFProcessingService.ts @@ -5,6 +5,7 @@ import { FileHasher } from '../utils/fileHash'; import { FileAnalyzer } from './fileAnalyzer'; import { ProcessingErrorHandler } from './processingErrorHandler'; import { pdfWorkerManager } from './pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export class EnhancedPDFProcessingService { private static instance: EnhancedPDFProcessingService; @@ -201,7 +202,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -251,7 +252,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -266,7 +267,7 @@ export class EnhancedPDFProcessingService { // Create placeholder pages for remaining pages for (let i = priorityCount + 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, // Will be loaded lazily rotation: 0, @@ -313,7 +314,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -334,7 +335,7 @@ export class EnhancedPDFProcessingService { // Create placeholders for remaining pages for (let i = firstChunkEnd + 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, rotation: 0, @@ -368,7 +369,7 @@ export class EnhancedPDFProcessingService { const pages: PDFPage[] = []; for (let i = 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, rotation: 0, diff --git a/frontend/src/services/pdfProcessingService.ts b/frontend/src/services/pdfProcessingService.ts index 065f53210..7ede9334d 100644 --- a/frontend/src/services/pdfProcessingService.ts +++ b/frontend/src/services/pdfProcessingService.ts @@ -1,6 +1,7 @@ import { ProcessedFile, ProcessingState, PDFPage } from '../types/processing'; import { ProcessingCache } from './processingCache'; import { pdfWorkerManager } from './pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export class PDFProcessingService { private static instance: PDFProcessingService; @@ -113,7 +114,7 @@ export class PDFProcessingService { const thumbnail = canvas.toDataURL(); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, diff --git a/frontend/src/tests/convert/ConvertIntegration.test.tsx b/frontend/src/tests/convert/ConvertIntegration.test.tsx index 41a768838..1ce180b9f 100644 --- a/frontend/src/tests/convert/ConvertIntegration.test.tsx +++ b/frontend/src/tests/convert/ConvertIntegration.test.tsx @@ -29,7 +29,7 @@ vi.mock('../../services/fileStorage', () => ({ init: vi.fn().mockResolvedValue(undefined), storeFile: vi.fn().mockImplementation((file, thumbnail) => { return Promise.resolve({ - id: `mock-id-${file.name}`, + id: `mock-uuid-${Math.random().toString(36).substring(2)}`, name: file.name, size: file.size, type: file.type, diff --git a/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx b/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx index 4e9fb7908..4038df63b 100644 --- a/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx +++ b/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx @@ -25,7 +25,7 @@ vi.mock('../../services/fileStorage', () => ({ init: vi.fn().mockResolvedValue(undefined), storeFile: vi.fn().mockImplementation((file, thumbnail) => { return Promise.resolve({ - id: `mock-id-${file.name}`, + id: `mock-uuid-${Math.random().toString(36).substring(2)}`, name: file.name, size: file.size, type: file.type, diff --git a/frontend/src/types/fileContext.ts b/frontend/src/types/fileContext.ts index cb5d6566a..aa12cbad7 100644 --- a/frontend/src/types/fileContext.ts +++ b/frontend/src/types/fileContext.ts @@ -25,8 +25,8 @@ export type ModeType = | 'unlockPdfForms' | 'removeCertificateSign'; -// Normalized state types -export type FileId = string; +// Normalized state types - Branded type to prevent string/FileId confusion +export type FileId = string & { readonly __brand: 'FileId' }; export interface ProcessedFilePage { thumbnail?: string; @@ -85,6 +85,127 @@ export function createQuickKey(file: File): string { return `${file.name}|${file.size}|${file.lastModified}`; } +// File with embedded UUID - replaces loose File + FileId parameter passing +export interface FileWithId extends File { + readonly fileId: FileId; + readonly quickKey: string; // Fast deduplication key: name|size|lastModified +} + +// Type guard to check if a File object has an embedded fileId +export function isFileWithId(file: File): file is FileWithId { + return 'fileId' in file && typeof (file as any).fileId === 'string' && + 'quickKey' in file && typeof (file as any).quickKey === 'string'; +} + +// Create a FileWithId from a regular File object +export function createFileWithId(file: File, id?: FileId): FileWithId { + const fileId = id || createFileId(); + const quickKey = createQuickKey(file); + + // Create new File-like object with embedded fileId and quickKey + const fileWithId = Object.create(file); + Object.defineProperty(fileWithId, 'fileId', { + value: fileId, + writable: false, + enumerable: true, + configurable: false + }); + Object.defineProperty(fileWithId, 'quickKey', { + value: quickKey, + writable: false, + enumerable: true, + configurable: false + }); + + return fileWithId as FileWithId; +} + +// Wrap array of Files with FileIds +export function wrapFilesWithIds(files: File[], ids?: FileId[]): FileWithId[] { + return files.map((file, index) => + createFileWithId(file, ids?.[index]) + ); +} + +// Extract FileIds from FileWithId array +export function extractFileIds(files: FileWithId[]): FileId[] { + return files.map(file => file.fileId); +} + +// Extract regular File objects from FileWithId array +export function extractFiles(files: FileWithId[]): File[] { + return files.map(file => { + // Create clean File object without the fileId property + return new File([file], file.name, { + type: file.type, + lastModified: file.lastModified + }); + }); +} + +// Type guards and validation functions + +// Validate that a string is a proper FileId (has UUID format) +export function isValidFileId(id: string): id is FileId { + // Check UUID v4 format: 8-4-4-4-12 hex digits + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + return uuidRegex.test(id); +} + +// Runtime assertion for FileId validation +export function assertValidFileId(id: string): asserts id is FileId { + if (!isValidFileId(id)) { + throw new Error(`Invalid FileId format: "${id}". Expected UUID format.`); + } +} + +// Detect potentially dangerous file.name usage as ID +export function isDangerousFileNameAsId(fileName: string, context: string = ''): boolean { + // Check if it's definitely a UUID (safe) + if (isValidFileId(fileName)) { + return false; + } + + // Check if it's a quickKey (safe) - format: name|size|lastModified + if (/^.+\|\d+\|\d+$/.test(fileName)) { + return false; // quickKeys are legitimate, not dangerous + } + + // Common patterns that suggest file.name is being used as ID + const dangerousPatterns = [ + /^[^-]+-page-\d+$/, // pattern: filename-page-123 + /\.(pdf|jpg|png|doc|docx)$/i, // ends with file extension + /\s/, // contains whitespace (filenames often have spaces) + /[()[\]{}]/, // contains brackets/parentheses common in filenames + /['"]/, // contains quotes + /[^a-zA-Z0-9\-._]/ // contains special characters not in UUIDs + ]; + + // Check dangerous patterns + const isDangerous = dangerousPatterns.some(pattern => pattern.test(fileName)); + + if (isDangerous && context) { + console.warn(`⚠️ Potentially dangerous file.name usage detected in ${context}: "${fileName}"`); + } + + return isDangerous; +} + +// Safe file ID getter that throws if file.name is used as ID +export function safeGetFileId(file: File, context: string = ''): FileId { + if (isFileWithId(file)) { + return file.fileId; + } + + // If we reach here, someone is trying to use a regular File without embedded ID + throw new Error(`Attempted to get FileId from regular File object in ${context}. Use FileWithId instead.`); +} + +// Prevent accidental file.name usage as FileId +export function preventFileNameAsId(value: string, context: string = ''): never { + throw new Error(`Blocked attempt to use string "${value}" as FileId in ${context}. Use proper FileId from createFileId() or FileWithId.fileId instead.`); +} + export function toFileRecord(file: File, id?: FileId): FileRecord { @@ -217,21 +338,21 @@ export type FileContextAction = export interface FileContextActions { // File management - lightweight actions only - addFiles: (files: File[], options?: { insertAfterPageId?: string }) => Promise; - addProcessedFiles: (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>) => Promise; - addStoredFiles: (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: FileMetadata }>) => Promise; + addFiles: (files: File[], options?: { insertAfterPageId?: string }) => Promise; + addProcessedFiles: (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>) => Promise; + addStoredFiles: (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: FileMetadata }>) => Promise; removeFiles: (fileIds: FileId[], deleteFromStorage?: boolean) => Promise; updateFileRecord: (id: FileId, updates: Partial) => void; reorderFiles: (orderedFileIds: FileId[]) => void; clearAllFiles: () => Promise; clearAllData: () => Promise; - // File pinning - pinFile: (file: File) => void; - unpinFile: (file: File) => void; + // File pinning - now accepts FileWithId for safer type checking + pinFile: (file: FileWithId) => void; + unpinFile: (file: FileWithId) => void; - // File consumption (replace unpinned files with outputs) - consumeFiles: (inputFileIds: FileId[], outputFiles: File[]) => Promise; + // File consumption (replace unpinned files with outputs) - now returns FileWithId + consumeFiles: (inputFileIds: FileId[], outputFiles: File[]) => Promise; // Selection management setSelectedFiles: (fileIds: FileId[]) => void; setSelectedPages: (pageNumbers: number[]) => void; @@ -254,24 +375,24 @@ export interface FileContextActions { // File selectors (separate from actions to avoid re-renders) export interface FileContextSelectors { - // File access - no state dependency, uses ref - getFile: (id: FileId) => File | undefined; - getFiles: (ids?: FileId[]) => File[]; + // File access - now returns FileWithId for safer type checking + getFile: (id: FileId) => FileWithId | undefined; + getFiles: (ids?: FileId[]) => FileWithId[]; // Record access - uses normalized state getFileRecord: (id: FileId) => FileRecord | undefined; getFileRecords: (ids?: FileId[]) => FileRecord[]; - // Derived selectors + // Derived selectors - now return FileWithId getAllFileIds: () => FileId[]; - getSelectedFiles: () => File[]; + getSelectedFiles: () => FileWithId[]; getSelectedFileRecords: () => FileRecord[]; - // Pinned files selectors + // Pinned files selectors - now return FileWithId getPinnedFileIds: () => FileId[]; - getPinnedFiles: () => File[]; + getPinnedFiles: () => FileWithId[]; getPinnedFileRecords: () => FileRecord[]; - isFilePinned: (file: File) => boolean; + isFilePinned: (file: FileWithId) => boolean; // Stable signature for effect dependencies getFilesSignature: () => string; diff --git a/frontend/src/types/fileIdSafety.d.ts b/frontend/src/types/fileIdSafety.d.ts new file mode 100644 index 000000000..cbe41a56c --- /dev/null +++ b/frontend/src/types/fileIdSafety.d.ts @@ -0,0 +1,59 @@ +/** + * Type safety declarations to prevent file.name/UUID confusion + */ + +import { FileId, FileWithId } from './fileContext'; + +declare global { + namespace FileIdSafety { + // Mark functions that should never accept file.name as parameters + type SafeFileIdFunction any> = T extends (...args: infer P) => infer R + ? P extends readonly [string, ...any[]] + ? never // Reject string parameters in first position for FileId functions + : T + : T; + + // Mark functions that should only accept FileWithId, not regular File + type FileWithIdOnlyFunction any> = T extends (...args: infer P) => infer R + ? P extends readonly [File, ...any[]] + ? never // Reject File parameters in first position for FileWithId functions + : T + : T; + + // Utility type to enforce FileWithId usage + type RequireFileWithId = T extends File ? FileWithId : T; + } + + // Extend Window interface to add runtime validation helpers + interface Window { + __FILE_ID_DEBUG?: boolean; + __validateFileId?: (id: string, context: string) => void; + } +} + +// Module augmentation for stricter type checking on dangerous functions +declare module '../utils/toolOperationTracker' { + export const createOperation: ( + operationType: string, + params: TParams, + selectedFiles: FileWithId[] // Must be FileWithId, not File[] + ) => { operation: FileOperation; operationId: string; fileId: string }; +} + +// Augment FileContext types to prevent bypassing FileWithId +declare module '../contexts/FileContext' { + export interface StrictFileContextActions { + pinFile: (file: FileWithId) => void; // Must be FileWithId + unpinFile: (file: FileWithId) => void; // Must be FileWithId + addFiles: (files: File[], options?: { insertAfterPageId?: string }) => Promise; // Returns FileWithId + consumeFiles: (inputFileIds: FileId[], outputFiles: File[]) => Promise; // Returns FileWithId + } + + export interface StrictFileContextSelectors { + getFile: (id: FileId) => FileWithId | undefined; // Returns FileWithId + getFiles: (ids?: FileId[]) => FileWithId[]; // Returns FileWithId[] + isFilePinned: (file: FileWithId) => boolean; // Must be FileWithId + } +} + +export {}; \ No newline at end of file diff --git a/frontend/src/utils/fileIdSafety.ts b/frontend/src/utils/fileIdSafety.ts new file mode 100644 index 000000000..e7146d5b2 --- /dev/null +++ b/frontend/src/utils/fileIdSafety.ts @@ -0,0 +1,80 @@ +/** + * Runtime validation helpers for file ID safety + */ + +import { isValidFileId, isDangerousFileNameAsId } from '../types/fileContext'; + +// Enable debug mode in development +const DEBUG_FILE_ID = process.env.NODE_ENV === 'development'; + +/** + * Runtime validation for FileId usage + */ +export function validateFileIdUsage(id: string, context: string = ''): void { + if (!DEBUG_FILE_ID) return; + + // Check if it's a valid UUID + if (!isValidFileId(id)) { + console.error(`🚨 Invalid FileId detected in ${context}: "${id}". Expected UUID format.`); + + // Check if it looks like a dangerous file.name usage + if (isDangerousFileNameAsId(id, context)) { + console.error(`💀 DANGEROUS: file.name used as FileId in ${context}! This will cause ID collisions.`); + console.trace('Stack trace:'); + } + } +} + +/** + * Runtime check for File vs FileWithId usage + */ +export function validateFileWithIdUsage(file: File, context: string = ''): void { + if (!DEBUG_FILE_ID) return; + + // Check if file has embedded fileId + if (!('fileId' in file)) { + console.warn(`⚠️ Regular File object used where FileWithId expected in ${context}: "${file.name}"`); + console.warn('Consider using FileWithId for better type safety'); + } +} + +/** + * Development-only assertion that fails on dangerous patterns + */ +export function assertSafeFileIdUsage(id: string, context: string = ''): void { + if (process.env.NODE_ENV === 'development') { + if (isDangerousFileNameAsId(id, context)) { + throw new Error(`ASSERTION FAILED: Dangerous file.name as FileId detected in ${context}: "${id}"`); + } + } +} + +/** + * Install global runtime validators (development only) + */ +export function installFileIdSafetyValidators(): void { + if (process.env.NODE_ENV !== 'development') return; + + // Add to window for debugging + window.__FILE_ID_DEBUG = true; + window.__validateFileId = validateFileIdUsage; + + // Monkey patch console.warn to highlight file ID issues + const originalWarn = console.warn; + console.warn = (...args: any[]) => { + const message = args.join(' '); + if (message.includes('file.name') && message.includes('ID')) { + console.error('🚨 FILE ID SAFETY WARNING:', ...args); + console.trace('Location:'); + } else { + originalWarn.apply(console, args); + } + }; + + console.log('🛡️ File ID safety validators installed (development mode)'); +} + +// Auto-install in development +if (process.env.NODE_ENV === 'development') { + installFileIdSafetyValidators(); +} \ No newline at end of file diff --git a/frontend/src/utils/toolOperationTracker.ts b/frontend/src/utils/toolOperationTracker.ts index a0b0fc06e..c58e3bf57 100644 --- a/frontend/src/utils/toolOperationTracker.ts +++ b/frontend/src/utils/toolOperationTracker.ts @@ -1,4 +1,4 @@ -import { FileOperation } from '../types/fileContext'; +import { FileOperation, FileWithId, safeGetFileId, FileId } from '../types/fileContext'; /** * Creates operation tracking data for FileContext integration @@ -6,23 +6,26 @@ import { FileOperation } from '../types/fileContext'; export const createOperation = ( operationType: string, params: TParams, - selectedFiles: File[] + selectedFiles: FileWithId[] ): { operation: FileOperation; operationId: string; fileId: string } => { const operationId = `${operationType}-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; - const fileId = selectedFiles.map(f => f.name).join(','); + + // Use proper FileIds instead of file.name - fixed dangerous pattern + const fileIds = selectedFiles.map(file => file.fileId); + const fileId = fileIds.join(','); const operation: FileOperation = { id: operationId, type: operationType, timestamp: Date.now(), - fileIds: selectedFiles.map(f => f.name), + fileIds, // Now properly uses FileId[] instead of file.name[] status: 'pending', metadata: { originalFileName: selectedFiles[0]?.name, parameters: params, fileSize: selectedFiles.reduce((sum, f) => sum + f.size, 0) } - } as any /* FIX ME*/; + }; return { operation, operationId, fileId }; };