Files
AeroFetch/test/validation.test.ts
T
debont80 b08d76a986 Apply all CODE-AUDIT.md findings (S1-S5, P1-P3, M1-M3)
Security:
- S1 (backup.ts): require explicit confirmation before enabling custom-command
  templates from a backup file; import templates in a disabled state if declined
- S2 (cookies.ts): deny non-http/https popups in cookie login window, matching
  the main window's setWindowOpenHandler defence
- S3 (download.ts): main-process concurrency cap — startDownload now rejects
  spawns when active.size >= maxConcurrent (defence-in-depth; renderer already
  enforces this but should not be the only gate)
- S4 (settings.ts, validation.ts): reject filenameTemplate values containing
  path traversal (.. segments or absolute paths); validate outputDir is absolute
- S5 (history.ts, errorlog.ts, templates.ts, validation.ts): per-field
  validation on JSON reads — invalid entries dropped rather than blindly trusted

Performance:
- P1 (settings.ts): getSettings() now only writes to disk when sanitization
  actually changed a value; removes synchronous file I/O on every hot-path read
- P2 (ipc.ts, download.ts, downloads.ts): renderer forwards its pre-probed
  metadata (title/channel/duration) via StartDownloadOptions.meta; main uses
  it directly instead of spawning a redundant second yt-dlp probe

Maintainability:
- M1 (log.ts, download.ts, probe.ts): extract duplicated cleanError() to
  src/main/log.ts; both callers import from the shared module
- M2 (buildArgs.ts): document parseExtraArgs escape-sequence limitations
- M3 (electron-builder.yml): clarify icon TODO as a pre-release action
- P3 (downloads.ts): document that lowering maxConcurrent mid-flight does
  not pause active downloads (intended behaviour, now written down)

Tests:
- Add test/validation.test.ts covering isSafeFilenameTemplate, isSafeOutputDir,
  isValidHistoryEntry, isValidErrorLogEntry, isTemplateLike (76 tests pass)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-23 10:22:43 -04:00

147 lines
4.4 KiB
TypeScript

import { describe, it, expect } from 'vitest'
import {
isSafeFilenameTemplate,
isSafeOutputDir,
isValidHistoryEntry,
isValidErrorLogEntry,
isTemplateLike
} from '../src/main/validation'
import type { HistoryEntry, ErrorLogEntry } from '@shared/ipc'
// --- S4: filename template path-traversal -----------------------------------
describe('isSafeFilenameTemplate', () => {
it('accepts the default template', () => {
expect(isSafeFilenameTemplate('%(title)s.%(ext)s')).toBe(true)
})
it('accepts sub-folder templates with yt-dlp fields', () => {
expect(isSafeFilenameTemplate('%(uploader)s/%(title)s.%(ext)s')).toBe(true)
expect(isSafeFilenameTemplate('%(uploader)s\\%(title)s.%(ext)s')).toBe(true)
})
it('rejects forward-slash traversal', () => {
expect(isSafeFilenameTemplate('%(title)s/../../evil.%(ext)s')).toBe(false)
})
it('rejects backslash traversal', () => {
expect(isSafeFilenameTemplate('%(title)s\\..\\..\\win32.exe')).toBe(false)
})
it('rejects a bare .. segment', () => {
expect(isSafeFilenameTemplate('..')).toBe(false)
})
it('rejects absolute paths', () => {
expect(isSafeFilenameTemplate('C:\\Windows\\system32\\%(title)s')).toBe(false)
expect(isSafeFilenameTemplate('/etc/%(title)s')).toBe(false)
})
it('allows .. only when embedded in a longer segment (not a real traversal)', () => {
// '..foo' / 'foo..bar' are filenames, not parent-dir references.
expect(isSafeFilenameTemplate('my..video.%(ext)s')).toBe(true)
})
})
// --- S4: output directory ----------------------------------------------------
describe('isSafeOutputDir', () => {
it('accepts an empty string (resolves to OS Downloads)', () => {
expect(isSafeOutputDir('')).toBe(true)
})
it('accepts an absolute Windows path', () => {
expect(isSafeOutputDir('C:\\Users\\me\\Downloads')).toBe(true)
})
it('rejects a relative path', () => {
expect(isSafeOutputDir('Downloads')).toBe(false)
expect(isSafeOutputDir('..\\elsewhere')).toBe(false)
})
})
// --- S5: persisted JSON entry validation -------------------------------------
const validHistory: HistoryEntry = {
id: 'a',
title: 'A video',
url: 'https://example.com/v',
kind: 'video',
quality: '1080p',
completedAt: 1700000000000
}
describe('isValidHistoryEntry', () => {
it('accepts a well-formed entry', () => {
expect(isValidHistoryEntry(validHistory)).toBe(true)
})
it('accepts optional string fields', () => {
expect(
isValidHistoryEntry({
...validHistory,
filePath: 'C:\\x.mp4',
channel: 'ch',
sizeLabel: '10 MB',
thumbnail: 'https://x/y.jpg'
})
).toBe(true)
})
it('rejects a wrong kind', () => {
expect(isValidHistoryEntry({ ...validHistory, kind: 'image' })).toBe(false)
})
it('rejects a missing required field', () => {
const { url: _url, ...noUrl } = validHistory
expect(isValidHistoryEntry(noUrl)).toBe(false)
})
it('rejects a non-string filePath (e.g. injected number/object)', () => {
expect(isValidHistoryEntry({ ...validHistory, filePath: 42 })).toBe(false)
})
it('rejects non-objects', () => {
expect(isValidHistoryEntry(null)).toBe(false)
expect(isValidHistoryEntry('nope')).toBe(false)
expect(isValidHistoryEntry(undefined)).toBe(false)
})
})
const validError: ErrorLogEntry = {
id: 'e',
url: 'https://example.com/v',
kind: 'audio',
error: 'boom',
occurredAt: 1700000000000
}
describe('isValidErrorLogEntry', () => {
it('accepts a well-formed entry', () => {
expect(isValidErrorLogEntry(validError)).toBe(true)
expect(isValidErrorLogEntry({ ...validError, title: 'optional' })).toBe(true)
})
it('rejects a missing error message', () => {
const { error: _error, ...noError } = validError
expect(isValidErrorLogEntry(noError)).toBe(false)
})
it('rejects a non-numeric occurredAt', () => {
expect(isValidErrorLogEntry({ ...validError, occurredAt: 'yesterday' })).toBe(false)
})
})
describe('isTemplateLike', () => {
it('accepts an object with a string or numeric id', () => {
expect(isTemplateLike({ id: 'x', name: 'n', args: '--foo' })).toBe(true)
expect(isTemplateLike({ id: 1 })).toBe(true)
})
it('rejects entries without an id', () => {
expect(isTemplateLike({ name: 'n', args: '--foo' })).toBe(false)
expect(isTemplateLike(null)).toBe(false)
expect(isTemplateLike('str')).toBe(false)
})
})