From 3536626a8a5479057a51a183b9bc14634a609d29 Mon Sep 17 00:00:00 2001 From: debont80 Date: Wed, 24 Jun 2026 08:04:19 -0400 Subject: [PATCH] security: harden command exec, IPC, deep-link, cookies, and persistence Seven-tier security audit of the main process, each finding fixed with a regression test. Typecheck (node + web) clean; unit tests 106 -> 140. - Tier 1 (command exec/argv): allowlist the yt-dlp --update-to channel (blocks arbitrary-binary-install RCE); gate per-download extraArgs behind the customCommandEnabled consent flag in main (blocks --exec RCE); resolve taskkill/schtasks by absolute System32 path; validate per-download outputDir; normalize the URL in assertHttpUrl and use it at every spawn. - Tier 2 (input validation): fix isSafeFilenameTemplate drive-relative ('C:foo') and Windows dotted-'..' traversal bypasses; percent-encode the untrusted id in entryUrl; catch reserved device names with extensions in sanitizeDirSegment. - Tier 3 (fs/backup): drop malformed template rows in importBackup; normalize deep-link URLs via assertHttpUrl. - Tier 4 (cookies): confine the sign-in window's navigations/popups to web URLs (recursively) and deny all web permissions on its session. - Tier 5 (deep-link/argv): bound the .url file read to 64 KB; match the aerofetch:// scheme case-insensitively. - Tier 6 (Electron window): deny camera/mic/geolocation/USB/HID/serial/ Bluetooth permissions on the app window. - Tier 7 (network/persistence): restrict the watched-source RSS fetch to youtube.com feed URLs (SSRF guard); complete isValidSource validation. Co-Authored-By: Claude Opus 4.8 --- src/main/backup.ts | 7 +- src/main/binaries.ts | 12 +++ src/main/buildArgs.ts | 40 ++++++++- src/main/cookies.ts | 77 ++++++++++++----- src/main/deeplink.ts | 29 +++++-- src/main/download.ts | 67 +++++++++++---- src/main/index.ts | 28 ++++++ src/main/indexer.ts | 5 +- src/main/indexerCore.ts | 22 ++++- src/main/probe.ts | 5 +- src/main/schedule.ts | 5 +- src/main/sync.ts | 6 +- src/main/url.ts | 12 ++- src/main/validation.ts | 24 ++++-- src/main/ytdlp.ts | 15 +++- src/renderer/src/components/DownloadBar.tsx | 38 ++++++--- src/shared/ipc.ts | 19 ++++- test/buildArgs.test.ts | 83 ++++++++++++++++++ test/deeplink.test.ts | 26 ++++++ test/indexer.test.ts | 25 ++++++ test/security-guards.test.ts | 94 +++++++++++++++++++++ test/validation.test.ts | 68 ++++++++++++++- 22 files changed, 623 insertions(+), 84 deletions(-) create mode 100644 test/security-guards.test.ts diff --git a/src/main/backup.ts b/src/main/backup.ts index 487a55b..fff7c4b 100644 --- a/src/main/backup.ts +++ b/src/main/backup.ts @@ -2,6 +2,7 @@ import { dialog, type BrowserWindow } from 'electron' import { readFileSync, writeFileSync } from 'fs' import { getSettings, setSettings } from './settings' import { listTemplates, replaceTemplates } from './templates' +import { isTemplateLike } from './validation' import type { BackupExportResult, BackupImportResult, @@ -55,8 +56,12 @@ export async function importBackup(win: BrowserWindow | undefined): Promise) : undefined + // Drop non-object / id-less entries up front (audit T3) so both the consent + // check below and replaceTemplates see only well-shaped rows. Without this a + // backup with a null/garbage templates entry throws in sanitize() instead of + // degrading gracefully as this function promises. const incomingTemplates = Array.isArray(file.templates) - ? (file.templates as CommandTemplate[]) + ? (file.templates as CommandTemplate[]).filter(isTemplateLike) : [] // A custom-command template's `args` are extra yt-dlp flags that get spawned on diff --git a/src/main/binaries.ts b/src/main/binaries.ts index c71561b..18eef62 100644 --- a/src/main/binaries.ts +++ b/src/main/binaries.ts @@ -35,3 +35,15 @@ export function getFfprobePath(): string { export function getAria2cPath(): string { return join(getBinDir(), 'aria2c.exe') } + +/** + * Absolute path to a Windows system executable (e.g. taskkill.exe, schtasks.exe). + * + * SECURITY (audit F3): system tools are resolved by full path under System32 + * rather than by bare name, so a same-named binary planted in the current + * working directory or earlier on PATH can't be invoked in their place — a real + * risk for the portable build, which runs from user-writable locations. + */ +export function getSystem32Path(exe: string): string { + return join(process.env.SystemRoot || 'C:\\Windows', 'System32', exe) +} diff --git a/src/main/buildArgs.ts b/src/main/buildArgs.ts index 9e6e592..1dd697b 100644 --- a/src/main/buildArgs.ts +++ b/src/main/buildArgs.ts @@ -14,7 +14,8 @@ import { type StartDownloadOptions, type DownloadOptions, type CollectionContext, - type CookieBrowser + type CookieBrowser, + type CommandTemplate } from '@shared/ipc' // --- Quality → yt-dlp selector mapping -------------------------------------- @@ -127,6 +128,39 @@ export function parseExtraArgs(raw: string): string[] { return args } +/** + * Resolve the extra yt-dlp args for a download, enforcing the custom-command + * consent gate (audit F2). + * + * Extra args are powerful enough to run arbitrary code (e.g. `--exec`), so they + * are honoured ONLY when custom commands are explicitly enabled in settings — + * the same persisted flag the Settings UI and backup-import treat as consent. A + * per-download override wins over the persisted default template, but NEITHER is + * applied while the gate is off. This keeps a compromised renderer from smuggling + * code-exec flags through a lone `startDownload({ extraArgs })` call: it would + * first have to flip the visible `customCommandEnabled` setting, leaving a trace + * (the same defence-in-depth posture as the main-side maxConcurrent cap). + * + * - customCommandEnabled off → [] (always) + * - perDownloadExtraArgs defined (even '') → those args + * - else a matching defaultTemplateId → that template's args + * - else → [] + */ +export function selectExtraArgs(params: { + customCommandEnabled: boolean + perDownloadExtraArgs: string | undefined + defaultTemplateId: string | null + templates: Pick[] +}): string[] { + if (!params.customCommandEnabled) return [] + if (params.perDownloadExtraArgs !== undefined) return parseExtraArgs(params.perDownloadExtraArgs) + if (params.defaultTemplateId) { + const tpl = params.templates.find((t) => t.id === params.defaultTemplateId) + if (tpl) return parseExtraArgs(tpl.args) + } + return [] +} + // Wrap a single argv token for human-readable display only (Phase C command // preview) — never used to build the real argv that gets spawned. function quoteForDisplay(arg: string): string { @@ -163,7 +197,9 @@ export function sanitizeDirSegment(name: string): string { let s = Array.from(name ?? '', (ch) => (ch.charCodeAt(0) < 0x20 ? ' ' : ch)).join('') s = s.replace(/[<>:"/\\|?*]/g, ' ') s = s.replace(/\s+/g, ' ').trim().replace(/[. ]+$/, '').replace(/^[. ]+/, '') - if (/^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(s)) s = `_${s}` + // Reserved device names are reserved even WITH an extension ('CON.txt' is still + // the CON device), so match an optional trailing '.' too (audit T3). + if (/^(con|prn|aux|nul|com[1-9]|lpt[1-9])(\..*)?$/i.test(s)) s = `_${s}` s = s.slice(0, 80).trim() return s || 'Untitled' } diff --git a/src/main/cookies.ts b/src/main/cookies.ts index ffdc498..0217370 100644 --- a/src/main/cookies.ts +++ b/src/main/cookies.ts @@ -1,4 +1,4 @@ -import { app, session, BrowserWindow, type Cookie } from 'electron' +import { app, session, BrowserWindow, type Cookie, type WebContents } from 'electron' import { existsSync, statSync, unlinkSync, writeFileSync } from 'fs' import { join } from 'path' import { assertHttpUrl } from './url' @@ -11,6 +11,55 @@ import type { CookiesStatus, CookiesLoginResult } from '@shared/ipc' */ const PARTITION = 'persist:aerofetch-login' +/** + * The sign-in window renders untrusted remote content, so every navigation and + * popup is confined to web URLs — http(s), plus about:blank. This stops a + * logged-in (or malicious) page from steering the window to a file:// URL, to + * the app's own aerofetch:// protocol handler, or to any other external URI + * scheme used as a pivot. (audit T4) + */ +export function isAllowedLoginUrl(target: string): boolean { + try { + const { protocol } = new URL(target) + return protocol === 'http:' || protocol === 'https:' || protocol === 'about:' + } catch { + return false + } +} + +/** + * Apply the web-only confinement to a sign-in webContents: block top-frame + * navigations and server redirects to non-web URLs, restrict popups to web URLs + * (reusing the same secure, partitioned webPreferences), and recurse into any + * popup the page opens so a nested window can't escape the policy either. The + * window-open handler alone only governs NEW windows, not navigations of an + * existing one — both vectors are covered here. (audit T4) + */ +function hardenLoginWebContents(wc: WebContents): void { + wc.setWindowOpenHandler((details) => { + if (!isAllowedLoginUrl(details.url)) return { action: 'deny' } + return { + action: 'allow', + overrideBrowserWindowOptions: { + autoHideMenuBar: true, + webPreferences: { + partition: PARTITION, + sandbox: true, + contextIsolation: true, + nodeIntegration: false + } + } + } + }) + wc.on('will-navigate', (e, navUrl) => { + if (!isAllowedLoginUrl(navUrl)) e.preventDefault() + }) + wc.on('will-redirect', (e, navUrl) => { + if (!isAllowedLoginUrl(navUrl)) e.preventDefault() + }) + wc.on('did-create-window', (child) => hardenLoginWebContents(child.webContents)) +} + export function getCookiesFilePath(): string { return join(app.getPath('userData'), 'cookies.txt') } @@ -114,26 +163,12 @@ export function openCookieLoginWindow(url: string): Promise } loginWindow = win - // Some sites log in via an OAuth/SSO popup. Let those open as real windows - // sharing the same partition, rather than silently swallowing the click. - // Restrict popups to http(s) only — the same defence the main window applies - // (index.ts) — so a logged-in page can't open a file:// popup to exfil cookies - // to disk or use a custom-protocol popup as a pivot. - win.webContents.setWindowOpenHandler((details) => { - try { - const { protocol } = new URL(details.url) - if (protocol !== 'http:' && protocol !== 'https:') return { action: 'deny' } - } catch { - return { action: 'deny' } // unparseable URL — never open it - } - return { - action: 'allow', - overrideBrowserWindowOptions: { - autoHideMenuBar: true, - webPreferences: { partition: PARTITION, sandbox: true, contextIsolation: true, nodeIntegration: false } - } - } - }) + // Confine every navigation/popup to web URLs (recursively, so OAuth/SSO + // popups sharing the cookie partition are covered too), and deny all gated + // web permissions — signing in needs no camera/mic/geolocation/etc., and the + // page is untrusted. (audit T4) + hardenLoginWebContents(win.webContents) + win.webContents.session.setPermissionRequestHandler((_wc, _permission, cb) => cb(false)) win.on('closed', () => { loginWindow = null diff --git a/src/main/deeplink.ts b/src/main/deeplink.ts index ac08482..a86e368 100644 --- a/src/main/deeplink.ts +++ b/src/main/deeplink.ts @@ -1,27 +1,40 @@ import { app, shell, type BrowserWindow } from 'electron' -import { existsSync, readFileSync } from 'fs' +import { existsSync, openSync, readSync, closeSync } from 'fs' import { join } from 'path' +import { assertHttpUrl } from './url' /** Only ever forward http(s) targets into the app — same restriction the - * external-link window-open handler in index.ts applies to in-page links. */ + * external-link window-open handler in index.ts applies to in-page links. + * Delegates to the download path's guard so an incoming deep-link target is + * validated AND parser-normalised (no embedded tabs/newlines/control chars + * reach the renderer); returns null instead of throwing for the argv scan. + * (audit T3 / F5) */ function asHttpUrl(candidate: string): string | null { try { - const u = new URL(candidate) - return u.protocol === 'http:' || u.protocol === 'https:' ? candidate : null + return assertHttpUrl(candidate) } catch { return null } } /** Reads a Windows Internet Shortcut (.url) file's target — what Explorer's - * "Send to" menu hands us when the user sends a saved link to AeroFetch. */ + * "Send to" menu hands us when the user sends a saved link to AeroFetch. + * Only the first 64 KB is read: a real shortcut is a few hundred bytes, so this + * caps memory and limits exposure if argv points at a pathological or oversized + * file that merely ends in `.url`. (audit T5) */ +const MAX_URL_FILE_BYTES = 64 * 1024 function readUrlShortcut(path: string): string | null { + let fd: number | null = null try { - const text = readFileSync(path, 'utf8') - const match = /^URL=(.+)$/im.exec(text) + fd = openSync(path, 'r') + const buf = Buffer.alloc(MAX_URL_FILE_BYTES) + const bytes = readSync(fd, buf, 0, buf.length, 0) + const match = /^URL=(.+)$/im.exec(buf.toString('utf8', 0, bytes)) return match ? asHttpUrl(match[1].trim()) : null } catch { return null + } finally { + if (fd !== null) closeSync(fd) } } @@ -32,7 +45,7 @@ function readUrlShortcut(path: string): string | null { */ export function extractIncomingUrl(argv: string[]): string | null { for (const arg of argv) { - if (arg.startsWith('aerofetch://')) { + if (/^aerofetch:\/\//i.test(arg)) { try { const target = new URL(arg).searchParams.get('url') const valid = target && asHttpUrl(target) diff --git a/src/main/download.ts b/src/main/download.ts index 0e24b01..ffa4431 100644 --- a/src/main/download.ts +++ b/src/main/download.ts @@ -2,14 +2,22 @@ import { spawn, execFile, type ChildProcess } from 'child_process' import { existsSync } from 'fs' import { join } from 'path' import { app, BrowserWindow, Notification, type WebContents } from 'electron' -import { getYtdlpPath, getBinDir, getAria2cPath, getFfmpegPath, getFfprobePath } from './binaries' +import { + getYtdlpPath, + getBinDir, + getAria2cPath, + getFfmpegPath, + getFfprobePath, + getSystem32Path +} from './binaries' import { getSettings, getDownloadArchivePath } from './settings' import { getCookiesFilePath } from './cookies' import { listTemplates } from './templates' import { assertHttpUrl } from './url' +import { isSafeOutputDir } from './validation' import { buildArgs, - parseExtraArgs, + selectExtraArgs, formatCommandLine, collectionOutputTemplate } from './buildArgs' @@ -151,21 +159,32 @@ function probeMeta(ytdlp: string, url: string): Promise { // --- Argv construction (shared by startDownload and the command preview) --- -// A per-download override (opts.extraArgs, even '') always wins; otherwise -// fall back to the persisted default template when custom-command mode is on. +// A per-download override (opts.extraArgs, even '') wins over the persisted +// default template — but BOTH are gated on the customCommandEnabled consent flag +// (see selectExtraArgs / audit F2). The gate is enforced here in main, not just +// in the renderer UI, so the renderer can't be trusted to apply it. function resolveExtraArgs(opts: StartDownloadOptions, settings: Settings): string[] { - if (opts.extraArgs !== undefined) return parseExtraArgs(opts.extraArgs) - if (settings.customCommandEnabled && settings.defaultTemplateId) { - const tpl = listTemplates().find((t) => t.id === settings.defaultTemplateId) - if (tpl) return parseExtraArgs(tpl.args) - } - return [] + return selectExtraArgs({ + customCommandEnabled: settings.customCommandEnabled, + perDownloadExtraArgs: opts.extraArgs, + defaultTemplateId: settings.defaultTemplateId, + templates: listTemplates() + }) } /** Resolve settings + per-download overrides into the full yt-dlp argv. */ export function buildCommand(opts: StartDownloadOptions): string[] { const settings = getSettings() - const outDir = opts.outputDir?.trim() || settings.outputDir || app.getPath('downloads') + // A per-download outputDir override must clear the same safety check the + // persisted setting does (absolute path only); a renderer-supplied override is + // otherwise untrusted — it dictates where downloaded files get written. An + // unsafe override is ignored in favour of the validated setting, then the OS + // Downloads folder. (audit F4) + const override = opts.outputDir?.trim() + const outDir = + (override && isSafeOutputDir(override) ? override : '') || + settings.outputDir || + app.getPath('downloads') // A collection (media-manager) download is filed into // // - folders; an ordinary download uses the flat filenameTemplate. const filenameTemplate = settings.filenameTemplate?.trim() || '%(title)s.%(ext)s' @@ -198,13 +217,16 @@ export function buildCommand(opts: StartDownloadOptions): string[] { /** Build the exact command line for the current form state, without running it. */ export function previewCommand(opts: StartDownloadOptions): CommandPreviewResult { + let normalized: StartDownloadOptions try { - assertHttpUrl(opts.url) + // Use the parser-normalised URL (audit F5), so the previewed command matches + // exactly what startDownload would spawn. + normalized = { ...opts, url: assertHttpUrl(opts.url) } } catch (e) { return { ok: false, error: (e as Error).message } } try { - return { ok: true, command: formatCommandLine(getYtdlpPath(), buildCommand(opts)) } + return { ok: true, command: formatCommandLine(getYtdlpPath(), buildCommand(normalized)) } } catch (e) { return { ok: false, error: (e as Error).message } } @@ -239,9 +261,13 @@ export function startDownload( `Add the ffmpeg build's binaries to resources/bin/ (see the README there).` } } - // Reject anything that isn't an http(s) URL before it reaches yt-dlp's argv. + // Reject anything that isn't an http(s) URL before it reaches yt-dlp's argv, + // and replace opts.url with the parser-normalised form so the exact string we + // validated is the one that gets spawned/probed — not a raw variant carrying + // interior tabs/newlines or leading control chars that URL parsing silently + // tolerates. (audit F5) try { - assertHttpUrl(opts.url) + opts = { ...opts, url: assertHttpUrl(opts.url) } } catch (e) { return { ok: false, error: (e as Error).message } } @@ -344,8 +370,15 @@ export function cancelDownload(id: string): void { rec.canceled = true const pid = rec.child.pid if (pid != null) { - // Kill the whole tree (/T) so the spawned ffmpeg child dies too. - execFile('taskkill', ['/pid', String(pid), '/T', '/F'], { windowsHide: true }, () => {}) + // Kill the whole tree (/T) so the spawned ffmpeg child dies too. Resolve + // taskkill from System32 by absolute path, never the bare name, so a planted + // taskkill.exe on PATH / in the CWD can't run in its place. (audit F3) + execFile( + getSystem32Path('taskkill.exe'), + ['/pid', String(pid), '/T', '/F'], + { windowsHide: true }, + () => {} + ) } else { rec.child.kill() } diff --git a/src/main/index.ts b/src/main/index.ts index 42ca203..377b594 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -90,6 +90,25 @@ function getSystemThemeInfo(): SystemThemeInfo { } } +// Web permissions a download manager never needs. They're denied for the app +// window as defence-in-depth (audit T6): even if the renderer were compromised +// (e.g. XSS via remote video metadata) it can't open the camera/mic, read +// location, or reach USB/HID/serial/Bluetooth devices — none of which the IPC +// surface grants either. Clipboard (paste/copy) and everything else is left to +// the default so the app's own features keep working. +const DENIED_PERMISSIONS = new Set([ + 'media', // camera + microphone + 'geolocation', + 'midi', + 'midiSysex', + 'hid', + 'serial', + 'usb', + 'bluetooth', + 'speaker-selection', + 'idle-detection' +]) + function createWindow(): void { const win = new BrowserWindow({ width: 920, @@ -143,6 +162,15 @@ function createWindow(): void { // away from it (defence in depth; HMR uses websockets, not navigation). win.webContents.on('will-navigate', (e) => e.preventDefault()) + // Deny the sensitive hardware/location web permissions the app never uses, so + // a compromised renderer can't escalate to capabilities the IPC surface + // doesn't grant. Both the async request and the sync check are covered. (audit T6) + const ses = win.webContents.session + ses.setPermissionRequestHandler((_wc, permission, callback) => + callback(!DENIED_PERMISSIONS.has(permission)) + ) + ses.setPermissionCheckHandler((_wc, permission) => !DENIED_PERMISSIONS.has(permission)) + if (is.dev && process.env['ELECTRON_RENDERER_URL']) { win.loadURL(process.env['ELECTRON_RENDERER_URL']) } else { diff --git a/src/main/indexer.ts b/src/main/indexer.ts index 8ab7235..c587e89 100644 --- a/src/main/indexer.ts +++ b/src/main/indexer.ts @@ -83,8 +83,9 @@ export async function indexSource( url: string, onProgress: (p: IndexProgress) => void ): Promise<IndexSourceResult> { + let normalizedUrl: string try { - assertHttpUrl(url) + normalizedUrl = assertHttpUrl(url) // normalised form for spawning (audit F5) } catch (e) { return { ok: false, error: (e as Error).message } } @@ -146,7 +147,7 @@ export async function indexSource( // resolve to a playlist when probed. A lone video has no entries → error. kind = cls?.kind ?? 'playlist' onProgress({ url, phase: 'uploads', message: 'Indexing playlist…' }) - const data = await probeFlat(cls?.base ?? url) + const data = await probeFlat(cls?.base ?? normalizedUrl) const entries = data.entries ?? [] if (entries.length === 0) { return { diff --git a/src/main/indexerCore.ts b/src/main/indexerCore.ts index 6a20a41..eb98106 100644 --- a/src/main/indexerCore.ts +++ b/src/main/indexerCore.ts @@ -34,7 +34,9 @@ export interface RawEntry { export function entryUrl(e: RawEntry): string | null { const cand = e.url || e.webpage_url if (cand && /^https?:\/\//i.test(cand)) return cand - if (e.id) return `https://www.youtube.com/watch?v=${e.id}` + // e.id is untrusted JSON from yt-dlp, so percent-encode it rather than splicing + // it raw into the query string (audit T2). A normal 11-char id is unaffected. + if (e.id) return `https://www.youtube.com/watch?v=${encodeURIComponent(e.id)}` return null } @@ -138,6 +140,24 @@ export function buildFeedUrl(kind: SourceKind, ytId: string | undefined): string return `https://www.youtube.com/feeds/videos.xml?${param}=${encodeURIComponent(ytId)}` } +/** + * Guard for the watched-source RSS pre-check (audit T7). The only feed AeroFetch + * ever builds is a YouTube videos.xml feed (see buildFeedUrl), so the sync refuses + * to fetch anything else — this keeps a hand-edited / corrupted sources.json from + * pointing fetch() at an internal service, a cloud-metadata endpoint, or any other + * arbitrary host (SSRF). Requires https, the youtube.com host (www optional), and + * the exact feed path; the channel_id/playlist_id query is free. + */ +export function isYouTubeFeedUrl(url: string): boolean { + try { + const u = new URL(url) + const host = u.hostname.replace(/^www\./i, '').toLowerCase() + return u.protocol === 'https:' && host === 'youtube.com' && u.pathname === '/feeds/videos.xml' + } catch { + return false + } +} + /** Extract the video ids from a YouTube RSS/Atom feed body (the <yt:videoId> tags). */ export function parseRssVideoIds(xml: string): string[] { const ids: string[] = [] diff --git a/src/main/probe.ts b/src/main/probe.ts index 4559fef..74f32e6 100644 --- a/src/main/probe.ts +++ b/src/main/probe.ts @@ -118,8 +118,9 @@ export function probeMedia(url: string): Promise<ProbeResult> { error: `yt-dlp.exe not found at ${ytdlp}\nDrop it into resources/bin/ (see the README there).` }) } + let target: string try { - assertHttpUrl(url) + target = assertHttpUrl(url) // normalised form (audit F5) } catch (e) { return Promise.resolve({ ok: false, error: (e as Error).message }) } @@ -128,7 +129,7 @@ export function probeMedia(url: string): Promise<ProbeResult> { execFile( ytdlp, // `--` terminates option parsing so the URL can never be read as a flag. - ['-J', '--flat-playlist', '--no-warnings', '--', url], + ['-J', '--flat-playlist', '--no-warnings', '--', target], { windowsHide: true, maxBuffer: 64 * 1024 * 1024, timeout: 60_000 }, (err, stdout, stderr) => { if (err) { diff --git a/src/main/schedule.ts b/src/main/schedule.ts index eb7f346..0676c46 100644 --- a/src/main/schedule.ts +++ b/src/main/schedule.ts @@ -10,6 +10,7 @@ */ import { execFile } from 'child_process' +import { getSystem32Path } from './binaries' import type { ScheduledSyncStatus } from '@shared/ipc' const TASK_NAME = 'AeroFetchDailySync' @@ -18,7 +19,9 @@ export const SYNC_FLAG = '--sync' function schtasks(args: string[]): Promise<{ code: number; stdout: string; stderr: string }> { return new Promise((resolve) => { - execFile('schtasks', args, { windowsHide: true }, (err, stdout, stderr) => { + // Resolve schtasks from System32 by absolute path, not the bare name, so a + // planted schtasks.exe on PATH / in the CWD can't be invoked instead. (audit F3) + execFile(getSystem32Path('schtasks.exe'), args, { windowsHide: true }, (err, stdout, stderr) => { const code = err ? ((err as { code?: number }).code ?? 1) : 0 resolve({ code, stdout: String(stdout), stderr: String(stderr) }) }) diff --git a/src/main/sync.ts b/src/main/sync.ts index 94fdea3..51ede1d 100644 --- a/src/main/sync.ts +++ b/src/main/sync.ts @@ -7,11 +7,15 @@ import { listSources, listMediaItems } from './sources' import { indexSource } from './indexer' -import { parseRssVideoIds } from './indexerCore' +import { parseRssVideoIds, isYouTubeFeedUrl } from './indexerCore' import type { IndexProgress, MediaItem, SyncResult } from '@shared/ipc' /** Fetch a YouTube RSS feed and return its recent video ids. Throws on failure. */ async function fetchFeedIds(feedUrl: string): Promise<string[]> { + // Only ever fetch a genuine YouTube feed — refuse an arbitrary host that a + // corrupted sources.json might carry (SSRF guard, audit T7). A throw here is + // caught by the caller, which then falls back to a full yt-dlp re-index. + if (!isYouTubeFeedUrl(feedUrl)) throw new Error('Refusing to fetch a non-YouTube feed URL.') const res = await fetch(feedUrl, { signal: AbortSignal.timeout(15_000) }) if (!res.ok) throw new Error(`feed responded ${res.status}`) return parseRssVideoIds(await res.text()) diff --git a/src/main/url.ts b/src/main/url.ts index ae8eba9..122b5c4 100644 --- a/src/main/url.ts +++ b/src/main/url.ts @@ -10,17 +10,23 @@ * (Call sites also pass `--` before the positional URL as defence in depth.) * * Throws a user-friendly Error on anything that isn't an http(s) URL. + * + * Returns the parser-NORMALISED URL (`u.href`), not the raw input (audit F5). + * The WHATWG URL parser silently tolerates interior tab/newline characters and + * leading C0 control bytes (which `String.trim()` does not strip), so returning + * the raw string could hand a downstream consumer — argv, or the sign-in + * window's loadURL — a value subtly different from the one actually validated. + * Emitting `u.href` guarantees callers use exactly the URL that passed the check. */ export function assertHttpUrl(raw: string): string { - const trimmed = (raw ?? '').trim() let u: URL try { - u = new URL(trimmed) + u = new URL((raw ?? '').trim()) } catch { throw new Error('That doesn’t look like a valid URL.') } if (u.protocol !== 'http:' && u.protocol !== 'https:') { throw new Error('Only http and https links are supported.') } - return trimmed + return u.href } diff --git a/src/main/validation.ts b/src/main/validation.ts index 1303e62..d9c2406 100644 --- a/src/main/validation.ts +++ b/src/main/validation.ts @@ -13,15 +13,21 @@ import type { HistoryEntry, ErrorLogEntry, CommandTemplate, Source, MediaItem } /** * A filenameTemplate is joined onto the output directory and handed to yt-dlp's - * `-o`. Reject anything that could write outside that directory — an absolute - * path, or any `..` path segment — so a malicious backup/settings write can't - * traverse out of the chosen folder (e.g. '%(title)s\..\..\win32.exe'). The - * template still legitimately contains yt-dlp `%(field)s` tokens and `/` or `\` - * for sub-folders, which are fine. + * `-o`. Reject anything that could write outside that directory so a malicious + * backup/settings write can't traverse out of the chosen folder (e.g. + * '%(title)s\..\..\win32.exe'). Legitimate templates still contain yt-dlp + * `%(field)s` tokens and `/` or `\` for sub-folders, which are fine. + * + * Two Windows-specific bypasses are guarded beyond the obvious cases (audit T1): + * - drive-relative prefixes like 'C:foo' — `path.isAbsolute` returns FALSE for + * these, yet they escape the output dir, so a leading drive letter is rejected. + * - a '..' segment dressed up with trailing dots/spaces ('.. ', '.. .') — + * Windows silently trims those, so an exact `=== '..'` check would miss them. */ +const TRAVERSAL_SEGMENT = /^[. ]*\.\.[. ]*$/ export function isSafeFilenameTemplate(template: string): boolean { - if (isAbsolute(template)) return false - return !template.split(/[\\/]/).some((segment) => segment === '..') + if (isAbsolute(template) || /^[a-zA-Z]:/.test(template)) return false + return !template.split(/[\\/]/).some((segment) => TRAVERSAL_SEGMENT.test(segment)) } /** An output directory must be an absolute path ('' resolves to OS Downloads). */ @@ -88,6 +94,10 @@ export function isValidSource(o: unknown): o is Source { typeof s.addedAt === 'number' && typeof s.itemCount === 'number' && isOptionalString(s.channel) && + // feedUrl drives a network fetch in the sync, so validate its shape here too + // (audit T7); the host is additionally restricted at the fetch boundary. + isOptionalString(s.feedUrl) && + (s.watched === undefined || typeof s.watched === 'boolean') && (s.lastIndexedAt === undefined || typeof s.lastIndexedAt === 'number') ) } diff --git a/src/main/ytdlp.ts b/src/main/ytdlp.ts index 3fe9e7e..4890cd8 100644 --- a/src/main/ytdlp.ts +++ b/src/main/ytdlp.ts @@ -1,7 +1,12 @@ import { execFile } from 'child_process' import { existsSync } from 'fs' import { getYtdlpPath } from './binaries' -import type { YtdlpVersionResult, YtdlpUpdateChannel, YtdlpUpdateResult } from '@shared/ipc' +import { + isYtdlpUpdateChannel, + type YtdlpVersionResult, + type YtdlpUpdateChannel, + type YtdlpUpdateResult +} from '@shared/ipc' /** * Step-1 spike: spawn the bundled yt-dlp and read back `--version`. @@ -38,6 +43,14 @@ export function getYtdlpVersion(): Promise<YtdlpVersionResult> { * a per-user install; it would fail under a locked-down system install. */ export function updateYtdlp(channel: YtdlpUpdateChannel): Promise<YtdlpUpdateResult> { + // Validate against the channel allowlist BEFORE the value reaches `--update-to`. + // That flag also accepts `OWNER/REPO@TAG`, which would download and install an + // arbitrary binary over yt-dlp.exe — so an unrecognised value (e.g. forged by a + // compromised renderer over IPC) must never be forwarded. (audit F1) + if (!isYtdlpUpdateChannel(channel)) { + return Promise.resolve({ ok: false, error: 'Unsupported update channel.' }) + } + const ytdlpPath = getYtdlpPath() if (!existsSync(ytdlpPath)) { diff --git a/src/renderer/src/components/DownloadBar.tsx b/src/renderer/src/components/DownloadBar.tsx index 2bf4b8e..3f2dfe8 100644 --- a/src/renderer/src/components/DownloadBar.tsx +++ b/src/renderer/src/components/DownloadBar.tsx @@ -840,21 +840,33 @@ export function DownloadBar(): React.JSX.Element { {showCustomCommand && ( <div className={styles.optionsPanel}> - <Field label="Template" hint="Extra yt-dlp flags layered onto this download, after every other option."> - <Select - aria-label="Custom command template" - value={effectiveTemplateId ?? 'none'} - options={[ - { value: 'none', label: 'None' }, - ...templates.map((t) => ({ value: t.id, label: t.name })) - ]} - onChange={(v) => setTemplateOverride(v === 'none' ? null : v)} - /> - </Field> - {templates.length === 0 && ( + {!customCommandEnabled ? ( + // Custom commands run arbitrary yt-dlp flags, so the main process + // ignores per-download extra args unless this consent flag is on + // (audit F2). Reflect that here rather than silently dropping a pick. <Caption1 className={styles.previewMeta}> - No templates yet — add one in Settings → Custom commands. + Custom commands are off. Turn them on in Settings → Custom commands to + layer extra yt-dlp flags onto a download. </Caption1> + ) : ( + <> + <Field label="Template" hint="Extra yt-dlp flags layered onto this download, after every other option."> + <Select + aria-label="Custom command template" + value={effectiveTemplateId ?? 'none'} + options={[ + { value: 'none', label: 'None' }, + ...templates.map((t) => ({ value: t.id, label: t.name })) + ]} + onChange={(v) => setTemplateOverride(v === 'none' ? null : v)} + /> + </Field> + {templates.length === 0 && ( + <Caption1 className={styles.previewMeta}> + No templates yet — add one in Settings → Custom commands. + </Caption1> + )} + </> )} </div> )} diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index f7689db..5d95c38 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -186,8 +186,23 @@ export interface YtdlpVersionResult { error?: string } -/** yt-dlp's self-update release channel (`--update-to <channel>`). */ -export type YtdlpUpdateChannel = 'stable' | 'nightly' +/** + * yt-dlp's self-update release channels (`--update-to <channel>`). + * + * SECURITY (audit F1): `--update-to` also accepts an `OWNER/REPO@TAG` spec, which + * makes yt-dlp download a release binary from an ARBITRARY GitHub repo and + * overwrite the running yt-dlp.exe — i.e. arbitrary, persistent code execution. + * The TypeScript type is erased at runtime and is no defence over IPC, so the + * value must be checked against this allowlist before it ever reaches the flag + * (see isYtdlpUpdateChannel; enforced in src/main/ytdlp.ts). + */ +export const YTDLP_UPDATE_CHANNELS = ['stable', 'nightly'] as const +export type YtdlpUpdateChannel = (typeof YTDLP_UPDATE_CHANNELS)[number] + +/** Runtime guard for an untrusted update-channel value crossing the IPC boundary. */ +export function isYtdlpUpdateChannel(v: unknown): v is YtdlpUpdateChannel { + return typeof v === 'string' && (YTDLP_UPDATE_CHANNELS as readonly string[]).includes(v) +} export interface YtdlpUpdateResult { ok: boolean diff --git a/test/buildArgs.test.ts b/test/buildArgs.test.ts index bd14396..876a627 100644 --- a/test/buildArgs.test.ts +++ b/test/buildArgs.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest' import { buildArgs, parseExtraArgs, + selectExtraArgs, formatCommandLine, sanitizeDirSegment, collectionOutputTemplate, @@ -328,6 +329,80 @@ describe('parseExtraArgs', () => { }) }) +describe('selectExtraArgs — custom-command consent gate (audit F2)', () => { + const templates = [ + { id: 'thumb', args: '--write-thumbnail' }, + { id: 'danger', args: '--exec "calc.exe"' } + ] + + it('returns [] when custom commands are disabled, even with a per-download override', () => { + // The core fix: a renderer-supplied extraArgs must NOT run while the gate is off. + expect( + selectExtraArgs({ + customCommandEnabled: false, + perDownloadExtraArgs: '--exec "calc.exe"', + defaultTemplateId: 'danger', + templates + }) + ).toEqual([]) + }) + + it('returns [] when disabled even if a default template id is set', () => { + expect( + selectExtraArgs({ + customCommandEnabled: false, + perDownloadExtraArgs: undefined, + defaultTemplateId: 'thumb', + templates + }) + ).toEqual([]) + }) + + it('parses a per-download override when enabled', () => { + expect( + selectExtraArgs({ + customCommandEnabled: true, + perDownloadExtraArgs: '--write-thumbnail --no-mtime', + defaultTemplateId: null, + templates + }) + ).toEqual(['--write-thumbnail', '--no-mtime']) + }) + + it('an explicit empty override yields [] even with a default template set', () => { + expect( + selectExtraArgs({ + customCommandEnabled: true, + perDownloadExtraArgs: '', + defaultTemplateId: 'thumb', + templates + }) + ).toEqual([]) + }) + + it('falls back to the default template when no per-download override is given', () => { + expect( + selectExtraArgs({ + customCommandEnabled: true, + perDownloadExtraArgs: undefined, + defaultTemplateId: 'thumb', + templates + }) + ).toEqual(['--write-thumbnail']) + }) + + it('returns [] when the default template id matches nothing', () => { + expect( + selectExtraArgs({ + customCommandEnabled: true, + perDownloadExtraArgs: undefined, + defaultTemplateId: 'missing', + templates + }) + ).toEqual([]) + }) +}) + describe('formatCommandLine', () => { it('joins the exe and args with spaces when nothing needs quoting', () => { expect(formatCommandLine('yt-dlp.exe', ['-f', 'best', '--', 'https://x.test/v'])).toBe( @@ -410,6 +485,14 @@ describe('sanitizeDirSegment', () => { expect(sanitizeDirSegment('com1')).toBe('_com1') }) + it('prefixes a reserved device name even when it carries an extension (audit T3)', () => { + expect(sanitizeDirSegment('CON.txt')).toBe('_CON.txt') + expect(sanitizeDirSegment('nul.mp4')).toBe('_nul.mp4') + expect(sanitizeDirSegment('LPT1.foo.bar')).toBe('_LPT1.foo.bar') + // a non-reserved name that merely starts with similar letters is left alone + expect(sanitizeDirSegment('console.log')).toBe('console.log') + }) + it('falls back to Untitled for empty / all-illegal input', () => { expect(sanitizeDirSegment('')).toBe('Untitled') expect(sanitizeDirSegment('???')).toBe('Untitled') diff --git a/test/deeplink.test.ts b/test/deeplink.test.ts index 88ef469..3003b4d 100644 --- a/test/deeplink.test.ts +++ b/test/deeplink.test.ts @@ -42,6 +42,20 @@ describe('extractIncomingUrl — aerofetch:// protocol', () => { it('returns null when there is no url= param', () => { expect(extractIncomingUrl(['AeroFetch.exe', 'aerofetch://download'])).toBeNull() }) + + it('normalises the target, stripping embedded control chars (audit T3 / F5)', () => { + // A tab spliced into the inner URL must not survive to the renderer banner. + const raw = 'https://www.youtube.com/watch?v=abc\tdef' + const arg = `aerofetch://download?url=${encodeURIComponent(raw)}` + expect(extractIncomingUrl(['AeroFetch.exe', arg])).toBe( + 'https://www.youtube.com/watch?v=abcdef' + ) + }) + + it('matches the aerofetch:// scheme case-insensitively (audit T5)', () => { + const arg = `AEROFETCH://download?url=${encodeURIComponent(TARGET)}` + expect(extractIncomingUrl(['AeroFetch.exe', arg])).toBe(TARGET) + }) }) describe('extractIncomingUrl — .url Internet Shortcut (Explorer "Send to")', () => { @@ -62,6 +76,18 @@ describe('extractIncomingUrl — .url Internet Shortcut (Explorer "Send to")', ( it('ignores files that merely end in .url-like text but are not real paths', () => { expect(extractIncomingUrl(['AeroFetch.exe', 'not-a-real-path.url'])).toBeNull() }) + + it('reads a URL= line that falls within the 64 KB size cap (audit T5)', () => { + const junk = '; padding\r\n'.repeat(100) // ~1 KB of leading content + const path = urlFile('Small.url', `[InternetShortcut]\r\n${junk}URL=${TARGET}\r\n`) + expect(extractIncomingUrl(['AeroFetch.exe', path])).toBe(TARGET) + }) + + it('ignores a URL= line that falls past the 64 KB size cap (audit T5)', () => { + const junk = '; padding\r\n'.repeat(8000) // ~88 KB, beyond the read window + const path = urlFile('Huge.url', `[InternetShortcut]\r\n${junk}URL=${TARGET}\r\n`) + expect(extractIncomingUrl(['AeroFetch.exe', path])).toBeNull() + }) }) describe('extractIncomingUrl — no match', () => { diff --git a/test/indexer.test.ts b/test/indexer.test.ts index bfeac7d..c6cc1bf 100644 --- a/test/indexer.test.ts +++ b/test/indexer.test.ts @@ -4,6 +4,7 @@ import { buildMediaItems, mergeItemsPreservingState, buildFeedUrl, + isYouTubeFeedUrl, parseRssVideoIds, entryUrl, fmtDuration, @@ -57,6 +58,10 @@ describe('entryUrl', () => { expect(entryUrl({ url: 'javascript:alert(1)' })).toBeNull() expect(entryUrl({})).toBeNull() }) + it('percent-encodes an untrusted id rather than splicing it raw (audit T2)', () => { + expect(entryUrl({ id: 'ab cd&x=1' })).toBe('https://www.youtube.com/watch?v=ab%20cd%26x%3D1') + expect(entryUrl({ id: 'vid123' })).toBe('https://www.youtube.com/watch?v=vid123') // unchanged + }) }) describe('fmtDuration', () => { @@ -189,6 +194,26 @@ describe('buildFeedUrl', () => { }) }) +describe('isYouTubeFeedUrl (audit T7 — SSRF guard)', () => { + it('accepts a youtube feeds URL (www optional) and what buildFeedUrl produces', () => { + expect(isYouTubeFeedUrl('https://www.youtube.com/feeds/videos.xml?channel_id=UCabc')).toBe(true) + expect(isYouTubeFeedUrl('https://youtube.com/feeds/videos.xml?playlist_id=PLxyz')).toBe(true) + expect(isYouTubeFeedUrl(buildFeedUrl('channel', 'UCabc')!)).toBe(true) + expect(isYouTubeFeedUrl(buildFeedUrl('playlist', 'PLxyz')!)).toBe(true) + }) + + it('rejects internal/arbitrary hosts, wrong scheme, and wrong path (SSRF vectors)', () => { + expect(isYouTubeFeedUrl('http://169.254.169.254/latest/meta-data/')).toBe(false) // cloud metadata + expect(isYouTubeFeedUrl('http://localhost:8080/admin')).toBe(false) + expect(isYouTubeFeedUrl('https://evil.com/feeds/videos.xml')).toBe(false) + expect(isYouTubeFeedUrl('https://notyoutube.com.evil.com/feeds/videos.xml')).toBe(false) + expect(isYouTubeFeedUrl('http://www.youtube.com/feeds/videos.xml')).toBe(false) // not https + expect(isYouTubeFeedUrl('https://www.youtube.com/watch?v=x')).toBe(false) // wrong path + expect(isYouTubeFeedUrl('file:///etc/passwd')).toBe(false) + expect(isYouTubeFeedUrl('not a url')).toBe(false) + }) +}) + describe('parseRssVideoIds', () => { it('extracts every <yt:videoId> from a feed body, in order', () => { const xml = ` diff --git a/test/security-guards.test.ts b/test/security-guards.test.ts new file mode 100644 index 0000000..3f2bf70 --- /dev/null +++ b/test/security-guards.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect } from 'vitest' +import { assertHttpUrl } from '../src/main/url' +import { isAllowedLoginUrl } from '../src/main/cookies' +import { isYtdlpUpdateChannel } from '@shared/ipc' + +// --- F5: assertHttpUrl — argument-injection guard + normalisation ----------- + +describe('assertHttpUrl (audit F5)', () => { + it('accepts http(s) URLs and returns the normalised href', () => { + expect(assertHttpUrl('https://www.youtube.com/watch?v=abc')).toBe( + 'https://www.youtube.com/watch?v=abc' + ) + // http with a bare host normalises to a trailing slash + expect(assertHttpUrl('http://example.com')).toBe('http://example.com/') + }) + + it('trims surrounding whitespace', () => { + expect(assertHttpUrl(' https://example.com/v ')).toBe('https://example.com/v') + }) + + it('strips interior tabs/newlines the URL parser tolerates (normalised output)', () => { + // Returning the raw input would leak these through to argv / loadURL. + expect(assertHttpUrl('https://exa\nmple.com/v')).toBe('https://example.com/v') + const out = assertHttpUrl('https://example.com/a\tb') + expect(out).not.toContain('\t') + expect(out).not.toContain('\n') + }) + + it('rejects non-http(s) protocols', () => { + expect(() => assertHttpUrl('file:///C:/Windows/system32')).toThrow() + expect(() => assertHttpUrl('javascript:alert(1)')).toThrow() + expect(() => assertHttpUrl('ftp://example.com/x')).toThrow() + expect(() => assertHttpUrl('aerofetch://download?url=x')).toThrow() + }) + + it('rejects unparseable input', () => { + expect(() => assertHttpUrl('not a url')).toThrow() + expect(() => assertHttpUrl('')).toThrow() + }) + + it('can never return a value that begins with "-" (would read as a yt-dlp flag)', () => { + // A leading '-' cannot start a valid URL scheme, so it always throws — + // the returned value therefore never opens with a dash. + expect(() => assertHttpUrl('-https://example.com')).toThrow() + }) +}) + +// --- T4: isAllowedLoginUrl — sign-in window navigation confinement ---------- + +describe('isAllowedLoginUrl (audit T4)', () => { + it('allows http(s) and about:blank', () => { + expect(isAllowedLoginUrl('https://accounts.google.com/signin')).toBe(true) + expect(isAllowedLoginUrl('http://example.com/login')).toBe(true) + expect(isAllowedLoginUrl('about:blank')).toBe(true) + }) + + it('blocks file://, the app protocol, and other external URI schemes', () => { + expect(isAllowedLoginUrl('file:///C:/Windows/System32/calc.exe')).toBe(false) + expect(isAllowedLoginUrl('aerofetch://download?url=https://evil.test')).toBe(false) + expect(isAllowedLoginUrl('ms-settings:')).toBe(false) + expect(isAllowedLoginUrl('mailto:x@y.z')).toBe(false) + expect(isAllowedLoginUrl('javascript:alert(1)')).toBe(false) + }) + + it('blocks unparseable input', () => { + expect(isAllowedLoginUrl('not a url')).toBe(false) + expect(isAllowedLoginUrl('')).toBe(false) + }) +}) + +// --- F1: isYtdlpUpdateChannel — --update-to allowlist ----------------------- + +describe('isYtdlpUpdateChannel (audit F1)', () => { + it('accepts the two supported channels', () => { + expect(isYtdlpUpdateChannel('stable')).toBe(true) + expect(isYtdlpUpdateChannel('nightly')).toBe(true) + }) + + it('rejects an arbitrary repository spec (the --update-to RCE vector)', () => { + expect(isYtdlpUpdateChannel('evil/yt-dlp@latest')).toBe(false) + expect(isYtdlpUpdateChannel('owner/repo')).toBe(false) + }) + + it('rejects other yt-dlp channels not on AeroFetch’s allowlist', () => { + expect(isYtdlpUpdateChannel('master')).toBe(false) + }) + + it('rejects non-string / empty values', () => { + expect(isYtdlpUpdateChannel('')).toBe(false) + expect(isYtdlpUpdateChannel(undefined)).toBe(false) + expect(isYtdlpUpdateChannel(null)).toBe(false) + expect(isYtdlpUpdateChannel(42)).toBe(false) + }) +}) diff --git a/test/validation.test.ts b/test/validation.test.ts index 199da3b..6445e90 100644 --- a/test/validation.test.ts +++ b/test/validation.test.ts @@ -4,9 +4,10 @@ import { isSafeOutputDir, isValidHistoryEntry, isValidErrorLogEntry, - isTemplateLike + isTemplateLike, + isValidSource } from '../src/main/validation' -import type { HistoryEntry, ErrorLogEntry } from '@shared/ipc' +import type { HistoryEntry, ErrorLogEntry, Source } from '@shared/ipc' // --- S4: filename template path-traversal ----------------------------------- @@ -40,6 +41,24 @@ describe('isSafeFilenameTemplate', () => { 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) + expect(isSafeFilenameTemplate('.%(title)s.%(ext)s')).toBe(true) // leading dot = hidden file + }) + + // --- T1: Windows-specific bypasses -------------------------------------- + + it('rejects a drive-relative prefix that path.isAbsolute misses', () => { + // 'C:foo' is NOT absolute per Node, but still escapes the output dir. + expect(isSafeFilenameTemplate('C:foo\\%(title)s.%(ext)s')).toBe(false) + expect(isSafeFilenameTemplate('c:%(title)s')).toBe(false) + }) + + it('rejects a .. segment dressed up with trailing/leading dots or spaces', () => { + // Windows trims trailing dots/spaces, so these all resolve to '..'. + expect(isSafeFilenameTemplate('%(title)s/.. /x')).toBe(false) + expect(isSafeFilenameTemplate('%(title)s/.. ./x')).toBe(false) + expect(isSafeFilenameTemplate('%(title)s/ ../x')).toBe(false) + expect(isSafeFilenameTemplate('%(title)s\\.. \\x')).toBe(false) + expect(isSafeFilenameTemplate('...')).toBe(false) }) }) @@ -144,3 +163,48 @@ describe('isTemplateLike', () => { expect(isTemplateLike('str')).toBe(false) }) }) + +// --- T7: source row validation (feedUrl drives a network fetch) -------------- + +const validSource: Source = { + id: 's1', + url: 'https://www.youtube.com/@x', + kind: 'channel', + title: 'X', + addedAt: 1700000000000, + itemCount: 5 +} + +describe('isValidSource', () => { + it('accepts a well-formed source and its optional fields', () => { + expect(isValidSource(validSource)).toBe(true) + expect( + isValidSource({ + ...validSource, + channel: 'X', + watched: true, + feedUrl: 'https://www.youtube.com/feeds/videos.xml?channel_id=UC', + lastIndexedAt: 1700000000001 + }) + ).toBe(true) + }) + + it('rejects a non-string feedUrl (audit T7)', () => { + expect(isValidSource({ ...validSource, feedUrl: 123 })).toBe(false) + }) + + it('rejects a non-boolean watched (audit T7)', () => { + expect(isValidSource({ ...validSource, watched: 'yes' })).toBe(false) + }) + + it('rejects a wrong kind or a missing required field', () => { + expect(isValidSource({ ...validSource, kind: 'video' })).toBe(false) + const { id: _id, ...noId } = validSource + expect(isValidSource(noId)).toBe(false) + }) + + it('rejects non-objects', () => { + expect(isValidSource(null)).toBe(false) + expect(isValidSource('nope')).toBe(false) + }) +}) -- 2.47.3