From 79c42fad55ea3d18baba1f03088599421bf89762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benito=20Rodr=C3=ADguez?= Date: Mon, 9 Feb 2026 19:49:56 +0100 Subject: [PATCH] feat: stream hashing y entradas en archivos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Añade `computeHashesFromStream` para hashing desde streams - Añade `streamArchiveEntry` e integra en `importDirectory` (path codificado con ::) - Extiende `scanDirectory` para exponer entradas internas, normaliza rutas POSIX y evita traversal; `ARCHIVE_MAX_ENTRIES` configurable - Limpia listeners en hashing y mejora robustez/logging - Añade tests unitarios e integración; actualiza mocks a `Mock` types - CI: instala `unzip` junto a `p7zip` para soportar tests de integración --- .gitea/workflows/ci.yaml | 8 +-- backend/src/services/checksumService.ts | 47 ++++++++++++---- backend/src/services/fsScanner.ts | 32 +++++++---- .../archiveReader.integration.spec.ts | 54 +++++++++++++++++++ .../services/fsScanner.archiveEntries.spec.ts | 49 ++++++++++++++++- .../importService.archiveEntry.spec.ts | 23 ++++---- backend/tests/services/importService.spec.ts | 24 ++++----- 7 files changed, 187 insertions(+), 50 deletions(-) create mode 100644 backend/tests/services/archiveReader.integration.spec.ts diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index ce60a7b..96f6099 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -30,11 +30,11 @@ jobs: - name: Install dependencies run: yarn install --immutable - - name: Install native archive tools (p7zip, chdman) + - name: Install native archive tools (p7zip, unzip, chdman) run: | sudo apt-get update # 7z / p7zip - sudo apt-get install -y p7zip-full p7zip-rar || true + sudo apt-get install -y p7zip-full p7zip-rar unzip || true # chdman (intentar instalar desde paquetes disponibles: mame-tools o mame) sudo apt-get install -y mame-tools || sudo apt-get install -y mame || true continue-on-error: true @@ -66,10 +66,10 @@ jobs: - name: Install dependencies run: yarn install --immutable - - name: Install native archive tools (p7zip, chdman) + - name: Install native archive tools (p7zip, unzip, chdman) run: | sudo apt-get update - sudo apt-get install -y p7zip-full p7zip-rar || true + sudo apt-get install -y p7zip-full p7zip-rar unzip || true sudo apt-get install -y mame-tools || sudo apt-get install -y mame || true continue-on-error: true diff --git a/backend/src/services/checksumService.ts b/backend/src/services/checksumService.ts index e131c6f..38b3946 100644 --- a/backend/src/services/checksumService.ts +++ b/backend/src/services/checksumService.ts @@ -84,24 +84,49 @@ export async function computeHashesFromStream(rs: NodeJS.ReadableStream): Promis let size = 0; let crc = 0xffffffff >>> 0; + let settled = false; - rs.on('error', (err: any) => reject(err)); + const cleanup = () => { + try { + rs.removeListener('error', onError as any); + rs.removeListener('data', onData as any); + rs.removeListener('end', onEnd as any); + rs.removeListener('close', onClose as any); + } catch (e) {} + }; - rs.on('data', (chunk: Buffer) => { - md5.update(chunk); - sha1.update(chunk); - size += chunk.length; - crc = updateCrc(crc, chunk); - }); - - rs.on('end', () => { + const finalize = () => { + if (settled) return; + settled = true; + cleanup(); const md5sum = md5.digest('hex'); const sha1sum = sha1.digest('hex'); const final = (crc ^ 0xffffffff) >>> 0; const crcHex = final.toString(16).padStart(8, '0').toLowerCase(); - resolve({ size, md5: md5sum, sha1: sha1sum, crc32: crcHex }); - }); + }; + + const onError = (err: any) => { + if (settled) return; + settled = true; + cleanup(); + reject(err); + }; + + const onData = (chunk: Buffer) => { + md5.update(chunk); + sha1.update(chunk); + size += chunk.length; + crc = updateCrc(crc, chunk); + }; + + const onEnd = () => finalize(); + const onClose = () => finalize(); + + rs.on('error', onError as any); + rs.on('data', onData as any); + rs.on('end', onEnd as any); + rs.on('close', onClose as any); }); } diff --git a/backend/src/services/fsScanner.ts b/backend/src/services/fsScanner.ts index 95afabd..4d861ed 100644 --- a/backend/src/services/fsScanner.ts +++ b/backend/src/services/fsScanner.ts @@ -15,7 +15,11 @@ import { promises as fsPromises } from 'fs'; import { detectFormat } from '../lib/fileTypeDetector'; import { listArchiveEntries } from './archiveReader'; -const ARCHIVE_MAX_ENTRIES = Number(process.env.ARCHIVE_MAX_ENTRIES) || 1000; +const DEFAULT_ARCHIVE_MAX_ENTRIES = 1000; +function getArchiveMaxEntries(): number { + const parsed = parseInt(process.env.ARCHIVE_MAX_ENTRIES ?? '', 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_ARCHIVE_MAX_ENTRIES; +} export async function scanDirectory(dirPath: string): Promise { const results: any[] = []; @@ -52,20 +56,24 @@ export async function scanDirectory(dirPath: string): Promise { try { const entries = await listArchiveEntries(full); + const maxEntries = getArchiveMaxEntries(); for (const e of entries) { - if (archiveEntriesAdded >= ARCHIVE_MAX_ENTRIES) break; + if (archiveEntriesAdded >= maxEntries) break; if (!e || !e.name) continue; - // avoid path traversal or absolute paths - if (e.name.includes('..') || path.isAbsolute(e.name)) continue; + + // Normalize entry path using posix rules and avoid traversal/absolute paths + const normalized = path.posix.normalize(e.name); + const parts = normalized.split('/').filter(Boolean); + if (parts.includes('..') || path.posix.isAbsolute(normalized)) continue; results.push({ - path: `${full}::${e.name}`, + path: `${full}::${normalized}`, containerPath: full, - entryPath: e.name, - filename: path.basename(e.name), - name: e.name, + entryPath: normalized, + filename: path.posix.basename(normalized), + name: normalized, size: e.size, - format: detectFormat(e.name), + format: detectFormat(normalized), isArchive: false, isArchiveEntry: true, }); @@ -73,7 +81,11 @@ export async function scanDirectory(dirPath: string): Promise { archiveEntriesAdded++; } } catch (err) { - // ignore archive listing errors + // log for diagnostics but continue + try { + // eslint-disable-next-line no-console + console.debug('fsScanner: listArchiveEntries failed for', full, err); + } catch (e) {} } } } diff --git a/backend/tests/services/archiveReader.integration.spec.ts b/backend/tests/services/archiveReader.integration.spec.ts new file mode 100644 index 0000000..c1b90b6 --- /dev/null +++ b/backend/tests/services/archiveReader.integration.spec.ts @@ -0,0 +1,54 @@ +import { test, expect } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { execSync } from 'child_process'; +import { computeHashesFromStream } from '../../src/services/checksumService'; +import { streamArchiveEntry } from '../../src/services/archiveReader'; +import { createHash } from 'crypto'; + +function hasBinary(bin: string): boolean { + try { + execSync(`which ${bin}`, { stdio: 'ignore' }); + return true; + } catch (e) { + return false; + } +} + +const wantIntegration = process.env.INTEGRATION === '1'; +const canCreate = hasBinary('7z') || hasBinary('zip'); + +if (!wantIntegration) { + test.skip('archiveReader integration tests require INTEGRATION=1', () => {}); +} else if (!canCreate) { + test.skip('archiveReader integration tests skipped: no archive creation tool (7z or zip) available', () => {}); +} else { + test('reads entry from zip using system tools', async () => { + const tmpDir = await fs.mkdtemp(path.join(process.cwd(), 'tmp-arc-')); + const inner = path.join(tmpDir, 'game.rom'); + const content = 'QUASAR-INTEGRATION-TEST'; + await fs.writeFile(inner, content); + + const archivePath = path.join(tmpDir, 'simple.zip'); + + // create zip using available tool + if (hasBinary('7z')) { + execSync(`7z a -tzip ${JSON.stringify(archivePath)} ${JSON.stringify(inner)}`, { + stdio: 'ignore', + }); + } else { + execSync(`zip -j ${JSON.stringify(archivePath)} ${JSON.stringify(inner)}`, { + stdio: 'ignore', + }); + } + + const stream = await streamArchiveEntry(archivePath, path.basename(inner)); + expect(stream).not.toBeNull(); + const hashes = await computeHashesFromStream(stream as any); + + const expectedMd5 = createHash('md5').update(content).digest('hex'); + expect(hashes.md5).toBe(expectedMd5); + + await fs.rm(tmpDir, { recursive: true, force: true }); + }); +} diff --git a/backend/tests/services/fsScanner.archiveEntries.spec.ts b/backend/tests/services/fsScanner.archiveEntries.spec.ts index 1e62b08..fa7bfdc 100644 --- a/backend/tests/services/fsScanner.archiveEntries.spec.ts +++ b/backend/tests/services/fsScanner.archiveEntries.spec.ts @@ -2,6 +2,7 @@ import path from 'path'; import os from 'os'; import { promises as fs } from 'fs'; import { afterEach, it, expect, vi } from 'vitest'; +import type { Mock } from 'vitest'; vi.mock('../../src/services/archiveReader', () => ({ listArchiveEntries: vi.fn() })); @@ -15,7 +16,7 @@ it('expone entradas internas de archivos como items virtuales', async () => { const collectionFile = path.join(tmpDir, 'collection.zip'); await fs.writeFile(collectionFile, ''); - (listArchiveEntries as unknown as vi.Mock).mockResolvedValue([ + (listArchiveEntries as unknown as Mock).mockResolvedValue([ { name: 'inner/rom1.bin', size: 1234 }, ]); @@ -33,3 +34,49 @@ it('expone entradas internas de archivos como items virtuales', async () => { await fs.rm(tmpDir, { recursive: true, force: true }); }); + +it('ignora entradas con traversal o paths absolutos', async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'fsScanner-test-')); + const collectionFile = path.join(tmpDir, 'collection.zip'); + await fs.writeFile(collectionFile, ''); + + (listArchiveEntries as unknown as Mock).mockResolvedValue([ + { name: '../evil.rom', size: 10 }, + { name: '/abs/evil.rom', size: 20 }, + { name: 'good/rom.bin', size: 30 }, + ]); + + const results = await scanDirectory(tmpDir); + const safePath = `${collectionFile}::good/rom.bin`; + expect(results.find((r: any) => r.path === safePath)).toBeDefined(); + expect(results.find((r: any) => r.path === `${collectionFile}::../evil.rom`)).toBeUndefined(); + expect(results.find((r: any) => r.path === `${collectionFile}::/abs/evil.rom`)).toBeUndefined(); + + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +it('respeta ARCHIVE_MAX_ENTRIES', async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'fsScanner-test-')); + const collectionFile = path.join(tmpDir, 'collection.zip'); + await fs.writeFile(collectionFile, ''); + + // Set env var temporarily + const prev = process.env.ARCHIVE_MAX_ENTRIES; + process.env.ARCHIVE_MAX_ENTRIES = '1'; + + (listArchiveEntries as unknown as Mock).mockResolvedValue([ + { name: 'one.bin', size: 1 }, + { name: 'two.bin', size: 2 }, + { name: 'three.bin', size: 3 }, + ]); + + const results = await scanDirectory(tmpDir); + const matches = results.filter((r: any) => String(r.path).startsWith(collectionFile + '::')); + expect(matches.length).toBe(1); + + // restore + if (prev === undefined) delete process.env.ARCHIVE_MAX_ENTRIES; + else process.env.ARCHIVE_MAX_ENTRIES = prev; + + await fs.rm(tmpDir, { recursive: true, force: true }); +}); diff --git a/backend/tests/services/importService.archiveEntry.spec.ts b/backend/tests/services/importService.archiveEntry.spec.ts index 11d2f2d..12b0957 100644 --- a/backend/tests/services/importService.archiveEntry.spec.ts +++ b/backend/tests/services/importService.archiveEntry.spec.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Mock } from 'vitest'; import { Readable } from 'stream'; vi.mock('../../src/services/fsScanner', () => ({ scanDirectory: vi.fn() })); @@ -37,29 +38,27 @@ describe('services/importService (archive entries)', () => { const data = Buffer.from('import-archive-test'); - (scanDirectory as unknown as vi.Mock).mockResolvedValue(files); - (streamArchiveEntry as unknown as vi.Mock).mockResolvedValue(Readable.from([data])); + (scanDirectory as unknown as Mock).mockResolvedValue(files); + (streamArchiveEntry as unknown as Mock).mockResolvedValue(Readable.from([data])); - (prisma.game.findUnique as unknown as vi.Mock).mockResolvedValue(null); - (prisma.game.create as unknown as vi.Mock).mockResolvedValue({ + (prisma.game.findUnique as unknown as Mock).mockResolvedValue(null); + (prisma.game.create as unknown as Mock).mockResolvedValue({ id: 77, title: 'ROM1', slug: 'rom1', }); - (prisma.romFile.upsert as unknown as vi.Mock).mockResolvedValue({ id: 1 }); + (prisma.romFile.upsert as unknown as Mock).mockResolvedValue({ id: 1 }); const md5 = createHash('md5').update(data).digest('hex'); const summary = await importDirectory({ dir: '/roms', persist: true }); - expect((streamArchiveEntry as unknown as vi.Mock).mock.calls.length).toBe(1); - expect((streamArchiveEntry as unknown as vi.Mock).mock.calls[0][0]).toBe( - '/roms/collection.zip' - ); - expect((streamArchiveEntry as unknown as vi.Mock).mock.calls[0][1]).toBe('inner/rom1.bin'); + expect((streamArchiveEntry as unknown as Mock).mock.calls.length).toBe(1); + expect((streamArchiveEntry as unknown as Mock).mock.calls[0][0]).toBe('/roms/collection.zip'); + expect((streamArchiveEntry as unknown as Mock).mock.calls[0][1]).toBe('inner/rom1.bin'); - expect((prisma.romFile.upsert as unknown as vi.Mock).mock.calls.length).toBe(1); - const upsertArgs = (prisma.romFile.upsert as unknown as vi.Mock).mock.calls[0][0]; + expect((prisma.romFile.upsert as unknown as Mock).mock.calls.length).toBe(1); + const upsertArgs = (prisma.romFile.upsert as unknown as Mock).mock.calls[0][0]; expect(upsertArgs.where).toEqual({ checksum: md5 }); expect(upsertArgs.create.filename).toBe('rom1.bin'); expect(upsertArgs.create.path).toBe('/roms/collection.zip::inner/rom1.bin'); diff --git a/backend/tests/services/importService.spec.ts b/backend/tests/services/importService.spec.ts index 6493a3c..3318d0f 100644 --- a/backend/tests/services/importService.spec.ts +++ b/backend/tests/services/importService.spec.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Mock } from 'vitest'; vi.mock('../../src/services/fsScanner', () => ({ scanDirectory: vi.fn(), @@ -44,31 +45,30 @@ describe('services/importService', () => { const hashes = { size: 123, md5: 'md5-abc', sha1: 'sha1-abc', crc32: 'abcd' }; - (scanDirectory as unknown as vi.Mock).mockResolvedValue(files); - (computeHashes as unknown as vi.Mock).mockResolvedValue(hashes); + (scanDirectory as unknown as Mock).mockResolvedValue(files); + (computeHashes as unknown as Mock).mockResolvedValue(hashes); - (prisma.game.findUnique as unknown as vi.Mock).mockResolvedValue(null); - (prisma.game.create as unknown as vi.Mock).mockResolvedValue({ + (prisma.game.findUnique as unknown as Mock).mockResolvedValue(null); + (prisma.game.create as unknown as Mock).mockResolvedValue({ id: 77, title: 'Sonic', slug: 'sonic', }); - (prisma.romFile.upsert as unknown as vi.Mock).mockResolvedValue({ id: 1 }); + (prisma.romFile.upsert as unknown as Mock).mockResolvedValue({ id: 1 }); const summary = await importDirectory({ dir: '/roms', persist: true }); - expect((scanDirectory as unknown as vi.Mock).mock.calls[0][0]).toBe('/roms'); - expect((computeHashes as unknown as vi.Mock).mock.calls[0][0]).toBe('/roms/Sonic.bin'); + expect((scanDirectory as unknown as Mock).mock.calls[0][0]).toBe('/roms'); + expect((computeHashes as unknown as Mock).mock.calls[0][0]).toBe('/roms/Sonic.bin'); - expect((prisma.game.findUnique as unknown as vi.Mock).mock.calls[0][0]).toEqual({ + expect((prisma.game.findUnique as unknown as Mock).mock.calls[0][0]).toEqual({ where: { slug: 'sonic' }, }); - expect((prisma.game.create as unknown as vi.Mock).mock.calls[0][0]).toEqual({ + expect((prisma.game.create as unknown as Mock).mock.calls[0][0]).toEqual({ data: { title: 'Sonic', slug: 'sonic' }, }); - - expect((prisma.romFile.upsert as unknown as vi.Mock).mock.calls.length).toBe(1); - const upsertArgs = (prisma.romFile.upsert as unknown as vi.Mock).mock.calls[0][0]; + expect((prisma.romFile.upsert as unknown as Mock).mock.calls.length).toBe(1); + const upsertArgs = (prisma.romFile.upsert as unknown as Mock).mock.calls[0][0]; expect(upsertArgs.where).toEqual({ checksum: 'md5-abc' }); expect(upsertArgs.create.gameId).toBe(77); expect(upsertArgs.create.filename).toBe('Sonic.bin');