From c5ea37aab9f1b5d24130eaee2af7aa3f379dbbce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20F=C3=B6rtsch?= Date: Mon, 13 Apr 2026 15:48:55 +0200 Subject: [PATCH] address audit findings: schedule validation, settings json guard, pipeline types, a11y labels --- package.json | 2 +- server/api/settings.ts | 6 ++- server/services/__tests__/scheduler.test.ts | 21 +++++++++ server/services/scheduler.ts | 29 ++++++++++-- src/features/pipeline/DoneColumn.tsx | 5 +- src/features/pipeline/PipelineCard.tsx | 19 ++++++-- src/features/pipeline/PipelinePage.tsx | 11 +---- src/features/pipeline/ProcessingColumn.tsx | 3 +- src/features/pipeline/QueueColumn.tsx | 5 +- src/features/pipeline/ReviewColumn.tsx | 24 ++++++---- src/features/pipeline/SeriesCard.tsx | 9 ++-- src/features/settings/SettingsPage.tsx | 12 ++++- src/shared/lib/types.ts | 52 +++++++++++++++++++++ 13 files changed, 161 insertions(+), 37 deletions(-) create mode 100644 server/services/__tests__/scheduler.test.ts diff --git a/package.json b/package.json index 20076ae..888fa5d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "netfelix-audio-fix", - "version": "2026.04.13.9", + "version": "2026.04.13.10", "scripts": { "dev:server": "NODE_ENV=development bun --hot server/index.tsx", "dev:client": "vite", diff --git a/server/api/settings.ts b/server/api/settings.ts index 055e758..4775136 100644 --- a/server/api/settings.ts +++ b/server/api/settings.ts @@ -95,7 +95,11 @@ app.get("/schedule", (c) => { app.patch("/schedule", async (c) => { const body = await c.req.json>(); - updateScheduleConfig(body); + try { + updateScheduleConfig(body); + } catch (e) { + return c.json({ error: e instanceof Error ? e.message : String(e) }, 400); + } return c.json(getScheduleConfig()); }); diff --git a/server/services/__tests__/scheduler.test.ts b/server/services/__tests__/scheduler.test.ts new file mode 100644 index 0000000..78ccca2 --- /dev/null +++ b/server/services/__tests__/scheduler.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, test } from "bun:test"; +import { updateScheduleConfig } from "../scheduler"; + +// These tests only exercise the pure validation path in updateScheduleConfig — +// invalid payloads must throw before any setConfig() call, so they never touch +// the DB. Valid payloads would hit the DB, so we don't exercise them here. +describe("updateScheduleConfig validation", () => { + test("rejects malformed HH:MM start/end", () => { + expect(() => updateScheduleConfig({ scan: { enabled: true, start: "xx:yy", end: "07:00" } })).toThrow(); + expect(() => updateScheduleConfig({ scan: { enabled: true, start: "1:00", end: "07:00" } })).toThrow(); + expect(() => updateScheduleConfig({ scan: { enabled: true, start: "24:00", end: "07:00" } })).toThrow(); + expect(() => updateScheduleConfig({ process: { enabled: true, start: "01:00", end: "99:99" } })).toThrow(); + }); + + test("rejects non-integer, negative, or out-of-bounds job_sleep_seconds", () => { + expect(() => updateScheduleConfig({ job_sleep_seconds: Number.NaN })).toThrow(); + expect(() => updateScheduleConfig({ job_sleep_seconds: -1 })).toThrow(); + expect(() => updateScheduleConfig({ job_sleep_seconds: 1.5 })).toThrow(); + expect(() => updateScheduleConfig({ job_sleep_seconds: 86_401 })).toThrow(); + }); +}); diff --git a/server/services/scheduler.ts b/server/services/scheduler.ts index 3404b93..572450b 100644 --- a/server/services/scheduler.ts +++ b/server/services/scheduler.ts @@ -41,10 +41,33 @@ export function getScheduleConfig(): ScheduleConfig { }; } +const HHMM = /^([01]\d|2[0-3]):[0-5]\d$/; + +function validateWindow(kind: WindowKind, w: Partial): void { + if (w.start != null && !HHMM.test(w.start)) { + throw new Error(`${kind}.start must be HH:MM 24h, got ${JSON.stringify(w.start)}`); + } + if (w.end != null && !HHMM.test(w.end)) { + throw new Error(`${kind}.end must be HH:MM 24h, got ${JSON.stringify(w.end)}`); + } +} + export function updateScheduleConfig(updates: Partial): void { - if (updates.job_sleep_seconds != null) setConfig("job_sleep_seconds", String(updates.job_sleep_seconds)); - if (updates.scan) writeWindow("scan", updates.scan); - if (updates.process) writeWindow("process", updates.process); + if (updates.job_sleep_seconds != null) { + const n = updates.job_sleep_seconds; + if (!Number.isFinite(n) || !Number.isInteger(n) || n < 0 || n > 86_400) { + throw new Error(`job_sleep_seconds must be an integer in [0, 86400], got ${JSON.stringify(n)}`); + } + setConfig("job_sleep_seconds", String(n)); + } + if (updates.scan) { + validateWindow("scan", updates.scan); + writeWindow("scan", updates.scan); + } + if (updates.process) { + validateWindow("process", updates.process); + writeWindow("process", updates.process); + } } function parseTime(hhmm: string): number { diff --git a/src/features/pipeline/DoneColumn.tsx b/src/features/pipeline/DoneColumn.tsx index e0854c1..ad5619d 100644 --- a/src/features/pipeline/DoneColumn.tsx +++ b/src/features/pipeline/DoneColumn.tsx @@ -1,9 +1,10 @@ import { Badge } from "~/shared/components/ui/badge"; import { api } from "~/shared/lib/api"; +import type { PipelineJobItem } from "~/shared/lib/types"; import { ColumnShell } from "./ColumnShell"; interface DoneColumnProps { - items: any[]; + items: PipelineJobItem[]; onMutate: () => void; } @@ -19,7 +20,7 @@ export function DoneColumn({ items, onMutate }: DoneColumnProps) { count={items.length} action={items.length > 0 ? { label: "Clear", onClick: clear } : undefined} > - {items.map((item: any) => ( + {items.map((item) => (

{item.name}

{item.status} diff --git a/src/features/pipeline/PipelineCard.tsx b/src/features/pipeline/PipelineCard.tsx index 82f639c..391621f 100644 --- a/src/features/pipeline/PipelineCard.tsx +++ b/src/features/pipeline/PipelineCard.tsx @@ -1,9 +1,20 @@ import { Link } from "@tanstack/react-router"; import { Badge } from "~/shared/components/ui/badge"; import { LANG_NAMES, langName } from "~/shared/lib/lang"; +import type { PipelineReviewItem } from "~/shared/lib/types"; + +// Accepts pipeline rows (plan+item) and also raw media_item rows (card is +// reused in a couple of list contexts where no plan is attached yet). +type PipelineCardItem = + | PipelineReviewItem + | (Omit & { + id: number; + item_id?: number; + transcode_reasons?: string[]; + }); interface PipelineCardProps { - item: any; + item: PipelineCardItem; jellyfinUrl: string; onLanguageChange?: (lang: string) => void; onApprove?: () => void; @@ -23,7 +34,7 @@ export function PipelineCard({ item, jellyfinUrl, onLanguageChange, onApprove, o // item.item_id is present in pipeline payloads; card can also be fed raw // media_item rows (no plan) in which case we fall back to item.id. - const mediaItemId: number = item.item_id ?? item.id; + const mediaItemId: number = item.item_id ?? (item as { id: number }).id; return (
@@ -60,8 +71,8 @@ export function PipelineCard({ item, jellyfinUrl, onLanguageChange, onApprove, o {langName(item.original_language)} )} - {item.transcode_reasons?.length > 0 - ? item.transcode_reasons.map((r: string) => ( + {item.transcode_reasons && item.transcode_reasons.length > 0 + ? item.transcode_reasons.map((r) => ( {r} diff --git a/src/features/pipeline/PipelinePage.tsx b/src/features/pipeline/PipelinePage.tsx index 1d89e42..3e1427b 100644 --- a/src/features/pipeline/PipelinePage.tsx +++ b/src/features/pipeline/PipelinePage.tsx @@ -1,21 +1,12 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { Button } from "~/shared/components/ui/button"; import { api } from "~/shared/lib/api"; +import type { PipelineData } from "~/shared/lib/types"; import { DoneColumn } from "./DoneColumn"; import { ProcessingColumn } from "./ProcessingColumn"; import { QueueColumn } from "./QueueColumn"; import { ReviewColumn } from "./ReviewColumn"; -interface PipelineData { - review: any[]; - reviewTotal: number; - queued: any[]; - processing: any[]; - done: any[]; - doneCount: number; - jellyfinUrl: string; -} - interface Progress { id: number; seconds: number; diff --git a/src/features/pipeline/ProcessingColumn.tsx b/src/features/pipeline/ProcessingColumn.tsx index 5357a7f..42dcb6c 100644 --- a/src/features/pipeline/ProcessingColumn.tsx +++ b/src/features/pipeline/ProcessingColumn.tsx @@ -1,10 +1,11 @@ import { useEffect, useState } from "react"; import { Badge } from "~/shared/components/ui/badge"; import { api } from "~/shared/lib/api"; +import type { PipelineJobItem } from "~/shared/lib/types"; import { ColumnShell } from "./ColumnShell"; interface ProcessingColumnProps { - items: any[]; + items: PipelineJobItem[]; progress?: { id: number; seconds: number; total: number } | null; queueStatus?: { status: string; until?: string; seconds?: number } | null; onMutate: () => void; diff --git a/src/features/pipeline/QueueColumn.tsx b/src/features/pipeline/QueueColumn.tsx index d9a89fb..83b1bd0 100644 --- a/src/features/pipeline/QueueColumn.tsx +++ b/src/features/pipeline/QueueColumn.tsx @@ -1,9 +1,10 @@ import { Badge } from "~/shared/components/ui/badge"; import { api } from "~/shared/lib/api"; +import type { PipelineJobItem } from "~/shared/lib/types"; import { ColumnShell } from "./ColumnShell"; interface QueueColumnProps { - items: any[]; + items: PipelineJobItem[]; onMutate: () => void; } @@ -20,7 +21,7 @@ export function QueueColumn({ items, onMutate }: QueueColumnProps) { count={items.length} action={items.length > 0 ? { label: "Clear", onClick: clear } : undefined} > - {items.map((item: any) => ( + {items.map((item) => (

{item.name}

{item.job_type} diff --git a/src/features/pipeline/ReviewColumn.tsx b/src/features/pipeline/ReviewColumn.tsx index 0bdd6ef..c923fca 100644 --- a/src/features/pipeline/ReviewColumn.tsx +++ b/src/features/pipeline/ReviewColumn.tsx @@ -1,15 +1,23 @@ import { api } from "~/shared/lib/api"; +import type { PipelineReviewItem } from "~/shared/lib/types"; import { ColumnShell } from "./ColumnShell"; import { PipelineCard } from "./PipelineCard"; import { SeriesCard } from "./SeriesCard"; interface ReviewColumnProps { - items: any[]; + items: PipelineReviewItem[]; total: number; jellyfinUrl: string; onMutate: () => void; } +interface SeriesGroup { + name: string; + key: string; + jellyfinId: string | null; + episodes: PipelineReviewItem[]; +} + export function ReviewColumn({ items, total, jellyfinUrl, onMutate }: ReviewColumnProps) { const truncated = total > items.length; @@ -29,24 +37,24 @@ export function ReviewColumn({ items, total, jellyfinUrl, onMutate }: ReviewColu }; // Group by series (movies are standalone) - const movies = items.filter((i: any) => i.type === "Movie"); - const seriesMap = new Map(); + const movies = items.filter((i) => i.type === "Movie"); + const seriesMap = new Map(); - for (const item of items.filter((i: any) => i.type === "Episode")) { - const key = item.series_jellyfin_id ?? item.series_name; + for (const item of items.filter((i) => i.type === "Episode")) { + const key = item.series_jellyfin_id ?? item.series_name ?? String(item.item_id); if (!seriesMap.has(key)) { - seriesMap.set(key, { name: item.series_name, key, jellyfinId: item.series_jellyfin_id, episodes: [] }); + seriesMap.set(key, { name: item.series_name ?? "", key, jellyfinId: item.series_jellyfin_id, episodes: [] }); } seriesMap.get(key)!.episodes.push(item); } // Interleave movies and series, sorted by confidence (high first) const allItems = [ - ...movies.map((m: any) => ({ type: "movie" as const, item: m, sortKey: m.confidence === "high" ? 0 : 1 })), + ...movies.map((m) => ({ type: "movie" as const, item: m, sortKey: m.confidence === "high" ? 0 : 1 })), ...[...seriesMap.values()].map((s) => ({ type: "series" as const, item: s, - sortKey: s.episodes.every((e: any) => e.confidence === "high") ? 0 : 1, + sortKey: s.episodes.every((e) => e.confidence === "high") ? 0 : 1, })), ].sort((a, b) => a.sortKey - b.sortKey); diff --git a/src/features/pipeline/SeriesCard.tsx b/src/features/pipeline/SeriesCard.tsx index 865d10e..344af34 100644 --- a/src/features/pipeline/SeriesCard.tsx +++ b/src/features/pipeline/SeriesCard.tsx @@ -1,6 +1,7 @@ import { useState } from "react"; import { api } from "~/shared/lib/api"; import { LANG_NAMES } from "~/shared/lib/lang"; +import type { PipelineReviewItem } from "~/shared/lib/types"; import { PipelineCard } from "./PipelineCard"; interface SeriesCardProps { @@ -8,7 +9,7 @@ interface SeriesCardProps { seriesName: string; jellyfinUrl: string; seriesJellyfinId: string | null; - episodes: any[]; + episodes: PipelineReviewItem[]; onMutate: () => void; } @@ -34,8 +35,8 @@ export function SeriesCard({ onMutate(); }; - const highCount = episodes.filter((e: any) => e.confidence === "high").length; - const lowCount = episodes.filter((e: any) => e.confidence === "low").length; + const highCount = episodes.filter((e) => e.confidence === "high").length; + const lowCount = episodes.filter((e) => e.confidence === "low").length; const jellyfinLink = jellyfinUrl && seriesJellyfinId ? `${jellyfinUrl}/web/index.html#!/details?id=${seriesJellyfinId}` : null; @@ -97,7 +98,7 @@ export function SeriesCard({ {expanded && (
- {episodes.map((ep: any) => ( + {episodes.map((ep) => ( move(i, -1)} + aria-label={`Move ${label} up`} className="w-6 h-6 flex items-center justify-center rounded border border-gray-200 bg-white text-gray-500 hover:bg-gray-50 disabled:opacity-30 disabled:cursor-not-allowed cursor-pointer text-xs" > ↑ @@ -124,6 +125,7 @@ function SortableLanguageList({ type="button" disabled={disabled || i === langs.length - 1} onClick={() => move(i, 1)} + aria-label={`Move ${label} down`} className="w-6 h-6 flex items-center justify-center rounded border border-gray-200 bg-white text-gray-500 hover:bg-gray-50 disabled:opacity-30 disabled:cursor-not-allowed cursor-pointer text-xs" > ↓ @@ -135,6 +137,7 @@ function SortableLanguageList({ type="button" disabled={disabled} onClick={() => remove(i)} + aria-label={`Remove ${label}`} className="text-red-400 hover:text-red-600 text-sm border-0 bg-transparent cursor-pointer disabled:opacity-30 disabled:cursor-not-allowed" > ✕ @@ -345,7 +348,14 @@ export function SettingsPage() { settingsCache = d; setData(d); if (!langsLoadedRef.current) { - setAudLangs(JSON.parse(d.config.audio_languages ?? "[]")); + let parsed: string[] = []; + try { + const raw = JSON.parse(d.config.audio_languages ?? "[]"); + if (Array.isArray(raw)) parsed = raw.filter((x): x is string => typeof x === "string"); + } catch (e) { + console.warn("audio_languages config is not valid JSON, defaulting to []", e); + } + setAudLangs(parsed); langsLoadedRef.current = true; } }) diff --git a/src/shared/lib/types.ts b/src/shared/lib/types.ts index ed3a635..aca7fc0 100644 --- a/src/shared/lib/types.ts +++ b/src/shared/lib/types.ts @@ -89,3 +89,55 @@ export interface Job { started_at: string | null; completed_at: string | null; } + +// ─── Pipeline payloads (GET /api/review/pipeline) ──────────────────────────── + +/** Row in the Review column: review_plan joined with media_item. */ +export interface PipelineReviewItem { + // review_plan fields (subset used by UI) + id: number; + item_id: number; + status: string; + is_noop: number; + confidence: "high" | "low"; + apple_compat: ReviewPlan["apple_compat"]; + job_type: "copy" | "transcode"; + // media_item fields + name: string; + series_name: string | null; + series_jellyfin_id: string | null; + jellyfin_id: string; + season_number: number | null; + episode_number: number | null; + type: "Movie" | "Episode"; + container: string | null; + original_language: string | null; + orig_lang_source: string | null; + file_path: string; + // computed + transcode_reasons: string[]; +} + +/** Row in the Queued / Processing / Done columns: job joined with media_item + review_plan. */ +export interface PipelineJobItem { + id: number; + item_id: number; + status: Job["status"]; + job_type: "copy" | "transcode"; + started_at: string | null; + completed_at: string | null; + name: string; + series_name: string | null; + type: "Movie" | "Episode"; + apple_compat: ReviewPlan["apple_compat"]; +} + +export interface PipelineData { + review: PipelineReviewItem[]; + reviewTotal: number; + queued: PipelineJobItem[]; + processing: PipelineJobItem[]; + done: PipelineJobItem[]; + doneCount: number; + jellyfinUrl: string; +}