feat(audit): Batch 24 — one validate-and-coerce schema layer for settings (CC9)

zod field schemas in settingsSchema.ts replace setSettings' bespoke per-key switch: parse → skip-on-fail → store, side effects (secret encryption, launch-at-startup sync) kept in settings.ts. sanitizeOptions extracted to settingsOptions.ts to break the import cycle; backup import flows through the same schema. Field-by-field parity pinned in test/settingsSchema.test.ts (15 tests); full suite unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-02 12:41:00 -04:00
parent 6abed2064d
commit 8f6b2a761b
7 changed files with 330 additions and 197 deletions
@@ -1,28 +1,10 @@
 import { app, safeStorage } from 'electron'
 import Store from 'electron-store'
-import { isSafeFilenameTemplate, isSafeOutputDir } from './validation'
 import { RENAMED_SETTINGS_KEYS } from './settingsMigration'
+import { parseSettingsField } from './settingsSchema'
+import { sanitizeOptions } from './settingsOptions'
 import { logger } from './logger'
-import {
-  AUDIO_FORMATS,
-  VIDEO_CONTAINERS,
-  VIDEO_CODECS,
-  SPONSORBLOCK_CATEGORIES,
-  COOKIE_BROWSERS,
-  ACCENT_COLORS,
-  VIDEO_QUALITY_OPTIONS,
-  AUDIO_QUALITY_OPTIONS,
-  DEFAULT_DOWNLOAD_OPTIONS,
-  DEFAULT_SETTINGS,
-  isYtdlpUpdateChannel,
-  isValidSyncTime,
-  type Settings,
-  type DownloadOptions,
-  type SponsorBlockCategory,
-  type AudioFormat,
-  type VideoContainer,
-  type VideoCodecPref
-} from '@shared/ipc'
+import { ACCENT_COLORS, DEFAULT_SETTINGS, type Settings, type DownloadOptions } from '@shared/ipc'

 // The electron-store defaults are the canonical DEFAULT_SETTINGS from the shared
 // contract — single-sourced so the renderer FALLBACK and the preview mock can't
@@ -44,53 +26,9 @@ export function applyLaunchAtStartup(enabled: boolean): void {
  }
 }

-// Coerce an untrusted partial into a valid DownloadOptions, falling back to the
-// defaults for any missing/invalid field. Used both to migrate older settings
-// files (which predate downloadOptions) and to validate renderer writes.
-function sanitizeOptions(input: unknown): DownloadOptions {
-  const o = (input && typeof input === 'object' ? input : {}) as Partial<DownloadOptions>
-  const d = DEFAULT_DOWNLOAD_OPTIONS
-  const bool = (v: unknown, fallback: boolean): boolean => (typeof v === 'boolean' ? v : fallback)
-  const cats = Array.isArray(o.sponsorBlockCategories)
-    ? (o.sponsorBlockCategories.filter((c) =>
-        (SPONSORBLOCK_CATEGORIES as readonly string[]).includes(c)
-      ) as SponsorBlockCategory[])
-    : d.sponsorBlockCategories
-  return {
-    audioFormat: (AUDIO_FORMATS as readonly string[]).includes(o.audioFormat as string)
-      ? (o.audioFormat as AudioFormat)
-      : d.audioFormat,
-    videoContainer: (VIDEO_CONTAINERS as readonly string[]).includes(o.videoContainer as string)
-      ? (o.videoContainer as VideoContainer)
-      : d.videoContainer,
-    preferredVideoCodec: (VIDEO_CODECS as readonly string[]).includes(
-      o.preferredVideoCodec as string
-    )
-      ? (o.preferredVideoCodec as VideoCodecPref)
-      : d.preferredVideoCodec,
-    formatSort: typeof o.formatSort === 'string' ? o.formatSort.trim() : d.formatSort,
-    embedSubtitles: bool(o.embedSubtitles, d.embedSubtitles),
-    subtitleLanguages:
-      typeof o.subtitleLanguages === 'string' && o.subtitleLanguages.trim()
-        ? o.subtitleLanguages.trim()
-        : d.subtitleLanguages,
-    autoSubtitles: bool(o.autoSubtitles, d.autoSubtitles),
-    sponsorBlock: bool(o.sponsorBlock, d.sponsorBlock),
-    sponsorBlockMode: o.sponsorBlockMode === 'mark' ? 'mark' : 'remove',
-    sponsorBlockCategories: cats,
-    embedChapters: bool(o.embedChapters, d.embedChapters),
-    splitChapters: bool(o.splitChapters, d.splitChapters),
-    embedMetadata: bool(o.embedMetadata, d.embedMetadata),
-    metadataTitle: typeof o.metadataTitle === 'string' ? o.metadataTitle : undefined,
-    metadataArtist: typeof o.metadataArtist === 'string' ? o.metadataArtist : undefined,
-    metadataAlbum: typeof o.metadataAlbum === 'string' ? o.metadataAlbum : undefined,
-    embedThumbnail: bool(o.embedThumbnail, d.embedThumbnail),
-    cropThumbnail: bool(o.cropThumbnail, d.cropThumbnail),
-    writeInfoJson: bool(o.writeInfoJson, d.writeInfoJson),
-    writeThumbnailFile: bool(o.writeThumbnailFile, d.writeThumbnailFile),
-    writeDescription: bool(o.writeDescription, d.writeDescription)
-  }
-}
+// sanitizeOptions moved to settingsOptions.ts (CC9) so the field schemas can
+// reuse it without an import cycle; re-imported above for the legacy-file
+// migration below.

 // Constructed lazily — electron-store needs app paths. (`userData` resolves
 // pre-ready too, which the W15 hardware-acceleration gate in index.ts relies on.)
@@ -268,131 +206,19 @@ function applySettings(s: Store<Settings>, partial: Partial<Settings>): void {
  for (const key of Object.keys(partial) as (keyof Settings)[]) {
    const value = partial[key]
    if (value === undefined) continue
-    switch (key) {
-      case 'theme':
-        if (value === 'light' || value === 'dark' || value === 'system') s.set('theme', value)
-        break
-      case 'accentColor':
-        if ((ACCENT_COLORS as readonly string[]).includes(value as string)) {
-          s.set('accentColor', value as Settings['accentColor'])
-        }
-        break
-      case 'defaultKind':
-        if (value === 'video' || value === 'audio') s.set('defaultKind', value)
-        break
-      case 'maxConcurrent': {
-        const n = Number(value)
-        if (Number.isFinite(n)) s.set('maxConcurrent', Math.min(5, Math.max(1, Math.round(n))))
-        break
-      }
-      case 'aria2cConnections': {
-        // aria2c caps -x/-s at 16; clamp like maxConcurrent rather than reject (L64).
-        const n = Number(value)
-        if (Number.isFinite(n)) s.set('aria2cConnections', Math.min(16, Math.max(1, Math.round(n))))
-        break
-      }
-      case 'syncTime':
-        // Reaches `schtasks /ST` when the daily sync is (re)registered — only a
-        // well-formed 24h HH:MM is ever stored (L51).
-        if (isValidSyncTime(value)) s.set('syncTime', value)
-        break
-      case 'watchClipboard':
-      case 'useAria2c':
-      case 'restrictFilenames':
-      case 'useDownloadArchive':
-      case 'autoUpdateYtdlp':
-      case 'enableCustomCommands':
-      case 'notifyOnComplete':
-      case 'autoDownloadNew':
-      case 'hasCompletedOnboarding':
-      case 'minimizeToTray':
-      case 'isSidebarCollapsed':
-      case 'useHardwareAcceleration':
-        if (typeof value === 'boolean') s.set(key, value)
-        break
-      case 'launchAtStartup':
-        if (typeof value === 'boolean') {
-          s.set('launchAtStartup', value)
-          applyLaunchAtStartup(value)
-        }
-        break
-      case 'ytdlpChannel':
-        // Same allowlist that guards the `--update-to` flag (audit F1).
-        if (isYtdlpUpdateChannel(value)) s.set('ytdlpChannel', value)
-        break
-      case 'ytdlpLastUpdateCheck': {
-        // Set by the main-process auto-updater; validated here since setSettings
-        // is the only writer. A bogus value at worst skips/forces one check.
-        const n = Number(value)
-        if (Number.isFinite(n) && n >= 0) s.set('ytdlpLastUpdateCheck', n)
-        break
-      }
-      case 'defaultTemplateId':
-        if (value === null || typeof value === 'string') s.set('defaultTemplateId', value)
-        break
-      case 'cookieSource':
-        if (value === 'none' || value === 'browser' || value === 'login') s.set(key, value)
-        break
-      case 'cookiesBrowser':
-        if ((COOKIE_BROWSERS as readonly string[]).includes(value as string)) {
-          s.set(key, value as Settings['cookiesBrowser'])
-        }
-        break
-      case 'downloadOptions':
-        // Always store a fully-validated object; the renderer sends the whole
-        // group (it merges field changes locally before calling setSettings).
-        s.set('downloadOptions', sanitizeOptions(value))
-        break
-      case 'videoFolder':
-      case 'audioFolder':
-        // An empty string is allowed — it clears the override and restores the
-        // Documents\Video / Documents\Audio default for that kind.
-        if (typeof value === 'string' && (value.trim() === '' || isSafeOutputDir(value.trim()))) {
-          s.set(key, value.trim())
-        }
-        break
-      case 'filenameTemplate':
-        if (typeof value === 'string' && isSafeFilenameTemplate(value.trim())) {
-          s.set('filenameTemplate', value.trim())
-        }
-        break
-      case 'defaultVideoQuality':
-        if (
-          typeof value === 'string' &&
-          (VIDEO_QUALITY_OPTIONS as readonly string[]).includes(value)
-        ) {
-          s.set('defaultVideoQuality', value)
-        }
-        break
-      case 'defaultAudioQuality':
-        if (
-          typeof value === 'string' &&
-          (AUDIO_QUALITY_OPTIONS as readonly string[]).includes(value)
-        ) {
-          s.set('defaultAudioQuality', value)
-        }
-        break
-      case 'rateLimit':
-        // yt-dlp rate-limit format: empty (disabled) or e.g. "500K", "2.5M", "1G".
-        if (typeof value === 'string' && /^(\d+\.?\d*[KMGkmg]?B?)?$/.test(value.trim())) {
-          s.set('rateLimit', value.trim())
-        }
-        break
-      case 'youtubePlayerClient':
-        // Power-user field; yt-dlp's client list evolves. Accept any trimmed string.
-        if (typeof value === 'string') s.set('youtubePlayerClient', value.trim())
-        break
-      // Credential-bearing fields — encrypted at rest (see encryptSecret). proxy
-      // may embed user:pass@host; youtubePoToken is an access token. Both are
-      // stripped from backup exports (see SECRET_KEYS / backup.ts).
-      case 'proxy':
-      case 'youtubePoToken':
-        if (typeof value === 'string') s.set(key, encryptSecret(value))
-        break
-      case 'updateToken':
-        // An access token has no spaces; trim before encrypting (like proxy creds).
-        if (typeof value === 'string') s.set('updateToken', encryptSecret(value.trim()))
-        break
+    // One schema layer validates AND coerces every field (CC9) — the parsed
+    // output is exactly what gets stored; a failed parse skips the key so the
+    // stored value stays. Side effects that don't belong in a pure schema stay
+    // here: credential fields are encrypted at rest (see encryptSecret; they're
+    // stripped from backup exports via SECRET_KEYS), and launchAtStartup syncs
+    // the OS run-at-sign-in entry.
+    const parsed = parseSettingsField(key, value)
+    if (!parsed.ok) continue
+    if (SECRET_KEYS.includes(key as (typeof SECRET_KEYS)[number])) {
+      s.set(key, encryptSecret(parsed.value as string) as Settings[typeof key])
+    } else {
+      s.set(key, parsed.value)
    }
+    if (key === 'launchAtStartup') applyLaunchAtStartup(parsed.value as boolean)
  }
 }
@@ -0,0 +1,62 @@
+import {
+  AUDIO_FORMATS,
+  VIDEO_CONTAINERS,
+  VIDEO_CODECS,
+  SPONSORBLOCK_CATEGORIES,
+  DEFAULT_DOWNLOAD_OPTIONS,
+  type DownloadOptions,
+  type SponsorBlockCategory,
+  type AudioFormat,
+  type VideoContainer,
+  type VideoCodecPref
+} from '@shared/ipc'
+
+// Coerce an untrusted partial into a valid DownloadOptions, falling back to the
+// defaults for any missing/invalid field. Used both to migrate older settings
+// files (which predate downloadOptions) and to validate renderer writes. Lives
+// in its own module (not settings.ts) so the settings field schemas (CC9) can
+// reuse it without an import cycle.
+export function sanitizeOptions(input: unknown): DownloadOptions {
+  const o = (input && typeof input === 'object' ? input : {}) as Partial<DownloadOptions>
+  const d = DEFAULT_DOWNLOAD_OPTIONS
+  const bool = (v: unknown, fallback: boolean): boolean => (typeof v === 'boolean' ? v : fallback)
+  const cats = Array.isArray(o.sponsorBlockCategories)
+    ? (o.sponsorBlockCategories.filter((c) =>
+        (SPONSORBLOCK_CATEGORIES as readonly string[]).includes(c)
+      ) as SponsorBlockCategory[])
+    : d.sponsorBlockCategories
+  return {
+    audioFormat: (AUDIO_FORMATS as readonly string[]).includes(o.audioFormat as string)
+      ? (o.audioFormat as AudioFormat)
+      : d.audioFormat,
+    videoContainer: (VIDEO_CONTAINERS as readonly string[]).includes(o.videoContainer as string)
+      ? (o.videoContainer as VideoContainer)
+      : d.videoContainer,
+    preferredVideoCodec: (VIDEO_CODECS as readonly string[]).includes(
+      o.preferredVideoCodec as string
+    )
+      ? (o.preferredVideoCodec as VideoCodecPref)
+      : d.preferredVideoCodec,
+    formatSort: typeof o.formatSort === 'string' ? o.formatSort.trim() : d.formatSort,
+    embedSubtitles: bool(o.embedSubtitles, d.embedSubtitles),
+    subtitleLanguages:
+      typeof o.subtitleLanguages === 'string' && o.subtitleLanguages.trim()
+        ? o.subtitleLanguages.trim()
+        : d.subtitleLanguages,
+    autoSubtitles: bool(o.autoSubtitles, d.autoSubtitles),
+    sponsorBlock: bool(o.sponsorBlock, d.sponsorBlock),
+    sponsorBlockMode: o.sponsorBlockMode === 'mark' ? 'mark' : 'remove',
+    sponsorBlockCategories: cats,
+    embedChapters: bool(o.embedChapters, d.embedChapters),
+    splitChapters: bool(o.splitChapters, d.splitChapters),
+    embedMetadata: bool(o.embedMetadata, d.embedMetadata),
+    metadataTitle: typeof o.metadataTitle === 'string' ? o.metadataTitle : undefined,
+    metadataArtist: typeof o.metadataArtist === 'string' ? o.metadataArtist : undefined,
+    metadataAlbum: typeof o.metadataAlbum === 'string' ? o.metadataAlbum : undefined,
+    embedThumbnail: bool(o.embedThumbnail, d.embedThumbnail),
+    cropThumbnail: bool(o.cropThumbnail, d.cropThumbnail),
+    writeInfoJson: bool(o.writeInfoJson, d.writeInfoJson),
+    writeThumbnailFile: bool(o.writeThumbnailFile, d.writeThumbnailFile),
+    writeDescription: bool(o.writeDescription, d.writeDescription)
+  }
+}
@@ -0,0 +1,115 @@
+/**
+ * The one validate-AND-coerce layer for Settings writes (CC9). Every key that
+ * setSettings accepts is described here as a zod schema whose output is exactly
+ * what gets persisted — replacing the bespoke per-key switch. The schemas reuse
+ * validation.ts's domain predicates (`isValidSyncTime`, `isSafeOutputDir`,
+ * `isSafeFilenameTemplate`) as refinements, so the rules live in one place.
+ *
+ * Semantics are a faithful port of the old switch (unit-tested in
+ * test/settingsSchema.test.ts): invalid values fail parse and are SKIPPED by
+ * applySettings (the stored value stays), numbers coerce + clamp, strings trim
+ * where they trimmed before. Side effects (secret encryption, launch-at-startup
+ * registration) stay in settings.ts — this layer is pure.
+ *
+ * The jsonStore row guards (`isValid*` in validation.ts) intentionally stay
+ * type-guard predicates: they filter untrusted disk rows in a hot read path and
+ * need no coercion.
+ */
+import { z } from 'zod'
+import {
+  ACCENT_COLORS,
+  COOKIE_BROWSERS,
+  VIDEO_QUALITY_OPTIONS,
+  AUDIO_QUALITY_OPTIONS,
+  YTDLP_UPDATE_CHANNELS,
+  isValidSyncTime,
+  type Settings
+} from '@shared/ipc'
+import { isSafeOutputDir, isSafeFilenameTemplate } from './validation'
+import { sanitizeOptions } from './settingsOptions'
+
+/** Coerce to a finite number, round, and clamp into [min, max] (maxConcurrent, aria2c). */
+const clampedInt = (min: number, max: number) =>
+  z.coerce
+    .number()
+    .refine((n) => Number.isFinite(n))
+    .transform((n) => Math.min(max, Math.max(min, Math.round(n))))
+
+/** Trimmed string that must satisfy the given predicate after trimming. */
+const trimmedWhere = (ok: (v: string) => boolean) =>
+  z
+    .string()
+    .transform((v) => v.trim())
+    .refine(ok)
+
+// yt-dlp rate-limit format: empty (disabled) or e.g. "500K", "2.5M", "1G".
+const RATE_LIMIT_RE = /^(\d+\.?\d*[KMGkmg]?B?)?$/
+
+const bool = z.boolean()
+
+/**
+ * One schema per Settings key; `applySettings` looks the key up, `safeParse`s
+ * the incoming value, and stores the parsed output — or skips the key entirely
+ * on failure. Typed as ZodType<unknown> per-field because outputs differ.
+ */
+export const SETTINGS_FIELD_SCHEMAS: { [K in keyof Settings]: z.ZodType<Settings[K], unknown> } = {
+  theme: z.enum(['light', 'dark', 'system']),
+  accentColor: z.enum(ACCENT_COLORS),
+  defaultKind: z.enum(['video', 'audio']),
+  maxConcurrent: clampedInt(1, 5),
+  // aria2c caps -x/-s at 16; clamp like maxConcurrent rather than reject (L64).
+  aria2cConnections: clampedInt(1, 16),
+  // Reaches `schtasks /ST` when the daily sync is (re)registered (L51).
+  syncTime: z.string().refine(isValidSyncTime),
+  watchClipboard: bool,
+  useAria2c: bool,
+  restrictFilenames: bool,
+  useDownloadArchive: bool,
+  autoUpdateYtdlp: bool,
+  enableCustomCommands: bool,
+  notifyOnComplete: bool,
+  autoDownloadNew: bool,
+  hasCompletedOnboarding: bool,
+  minimizeToTray: bool,
+  isSidebarCollapsed: bool,
+  useHardwareAcceleration: bool,
+  launchAtStartup: bool,
+  // Same allowlist that guards the `--update-to` flag (audit F1).
+  ytdlpChannel: z.enum(YTDLP_UPDATE_CHANNELS),
+  // Set by the main-process auto-updater; a bogus value at worst skips one check.
+  ytdlpLastUpdateCheck: z.coerce.number().refine((n) => Number.isFinite(n) && n >= 0),
+  defaultTemplateId: z.union([z.null(), z.string()]),
+  cookieSource: z.enum(['none', 'browser', 'login']),
+  cookiesBrowser: z.enum(COOKIE_BROWSERS),
+  // Always store a fully-validated object; the renderer sends the whole group.
+  downloadOptions: z.unknown().transform((v) => sanitizeOptions(v)),
+  // An empty string is allowed — it clears the override and restores the default.
+  videoFolder: trimmedWhere((v) => v === '' || isSafeOutputDir(v)),
+  audioFolder: trimmedWhere((v) => v === '' || isSafeOutputDir(v)),
+  filenameTemplate: trimmedWhere(isSafeFilenameTemplate),
+  defaultVideoQuality: z.enum(VIDEO_QUALITY_OPTIONS),
+  defaultAudioQuality: z.enum(AUDIO_QUALITY_OPTIONS),
+  rateLimit: trimmedWhere((v) => RATE_LIMIT_RE.test(v)),
+  // Power-user field; yt-dlp's client list evolves. Accept any trimmed string.
+  youtubePlayerClient: z.string().transform((v) => v.trim()),
+  // Credential-bearing fields — settings.ts encrypts these AFTER parsing.
+  proxy: z.string(),
+  youtubePoToken: z.string(),
+  // An access token has no spaces; trim before encrypting (like proxy creds).
+  updateToken: z.string().transform((v) => v.trim())
+}
+
+/**
+ * Parse one incoming settings value. Returns `{ ok: true, value }` with the
+ * coerced/clamped/trimmed value to store, or `{ ok: false }` when the write
+ * must be skipped (unknown key or failed validation).
+ */
+export function parseSettingsField<K extends keyof Settings>(
+  key: K,
+  value: unknown
+): { ok: true; value: Settings[K] } | { ok: false } {
+  const schema = SETTINGS_FIELD_SCHEMAS[key] as z.ZodType<Settings[K], unknown> | undefined
+  if (!schema) return { ok: false }
+  const parsed = schema.safeParse(value)
+  return parsed.success ? { ok: true, value: parsed.data } : { ok: false }
+}