security: harden command exec, IPC, deep-link, cookies, and persistence #2
+6
-1
@@ -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<Back
|
||||
file.settings && typeof file.settings === 'object'
|
||||
? (file.settings as Partial<Settings>)
|
||||
: 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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
+38
-2
@@ -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<CommandTemplate, 'id' | 'args'>[]
|
||||
}): 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 '.<ext>' 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'
|
||||
}
|
||||
|
||||
+56
-21
@@ -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<CookiesLoginResult>
|
||||
}
|
||||
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
|
||||
|
||||
+21
-8
@@ -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)
|
||||
|
||||
+50
-17
@@ -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<DownloadMeta | null> {
|
||||
|
||||
// --- 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 <channel>/<playlist>/
|
||||
// <NNN> - <title> 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()
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+3
-2
@@ -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 {
|
||||
|
||||
+21
-1
@@ -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[] = []
|
||||
|
||||
+3
-2
@@ -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) {
|
||||
|
||||
@@ -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) })
|
||||
})
|
||||
|
||||
+5
-1
@@ -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())
|
||||
|
||||
+9
-3
@@ -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
|
||||
}
|
||||
|
||||
+17
-7
@@ -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')
|
||||
)
|
||||
}
|
||||
|
||||
+14
-1
@@ -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)) {
|
||||
|
||||
@@ -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>
|
||||
)}
|
||||
|
||||
+17
-2
@@ -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
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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 = `
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
+66
-2
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user