From 79cfeae8d8746d56b04a29bea195dbf38a3b4ffb Mon Sep 17 00:00:00 2001 From: m4 Date: Tue, 11 Aug 2026 12:33:58 +0800 Subject: [PATCH] fix(webui): harden branding path validation, auth-on warning, scheduler retry Co-Authored-By: Claude Opus 4.7 --- .../components/system-config/SecurityTab.tsx | 4 ++++ src/lib/server/backupScheduler.test.ts | 20 +++++++++++++++++++ src/lib/server/backupScheduler.ts | 4 +++- src/lib/server/systemConfig.test.ts | 17 ++++++++++++++++ src/lib/server/systemConfig.ts | 14 +++++++++++++ 5 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/app/components/system-config/SecurityTab.tsx b/src/app/components/system-config/SecurityTab.tsx index 73a513b..4994d8e 100644 --- a/src/app/components/system-config/SecurityTab.tsx +++ b/src/app/components/system-config/SecurityTab.tsx @@ -8,11 +8,13 @@ function TriState({ value, onChange, confirmOff, + confirmOn, }: { label: string; value: boolean | null; onChange: (value: boolean | null) => void; confirmOff?: string; + confirmOn?: string; }) { return (
@@ -24,6 +26,7 @@ function TriState({ const next = event.target.value === "default" ? null : event.target.value === "on"; if (next === false && confirmOff && !window.confirm(confirmOff)) return; + if (next === true && confirmOn && !window.confirm(confirmOn)) return; onChange(next); }} > @@ -87,6 +90,7 @@ export function SecurityTab({ draft, setDraft }: TabProps) { label="WebUI authentication" value={draft.security.authEnabled} confirmOff="Disabling authentication opens the UI to anyone who can reach it. Continue?" + confirmOn="Enabling authentication requires WEBUI_AUTH_SECRET to be set in the server environment. Without it, every page (including this dialog) will return 503 and recovery requires hand-editing ~/.evoscientist/system-config.json. Continue?" onChange={(authEnabled) => set({ authEnabled })} /> { await scheduler.tickBackupSchedule(now + 10 * HOUR); expect(listBackups()).toHaveLength(0); }); + + it("records a failed attempt so the next tick does not retry", async () => { + const backendDir = fs.mkdtempSync(path.join(os.tmpdir(), "evosci-backend-")); + const config = getSystemConfig(); + config.backup.backendDataDir = backendDir; + saveSystemConfig(config); + resetSystemConfigCacheForTests(); + fs.rmSync(backendDir, { recursive: true, force: true }); // createBackup now fails + + const { listErrorLogs, clearErrorLogs } = await import("./errorLogStore"); + clearErrorLogs("system"); + + await scheduler.tickBackupSchedule(now); + expect(listBackups()).toHaveLength(0); + expect(listErrorLogs("system")).toHaveLength(1); + + await scheduler.tickBackupSchedule(now + 60_000); // 1 min later: no retry + expect(listBackups()).toHaveLength(0); + expect(listErrorLogs("system")).toHaveLength(1); + }); }); diff --git a/src/lib/server/backupScheduler.ts b/src/lib/server/backupScheduler.ts index 50d9e7f..b29eb76 100644 --- a/src/lib/server/backupScheduler.ts +++ b/src/lib/server/backupScheduler.ts @@ -19,10 +19,12 @@ export async function tickBackupSchedule(now = Date.now()): Promise { if (!schedule.enabled) return; const lastRunAt = globalState.__evoscientistBackupLastRunAt ?? 0; if (now - lastRunAt < schedule.intervalHours * 3_600_000) return; + // Record the attempt up front so a persistent failure retries on the next + // configured interval instead of on every tick, flooding the error log. + globalState.__evoscientistBackupLastRunAt = now; try { await createBackup("auto"); pruneBackups(schedule.keepCount); - globalState.__evoscientistBackupLastRunAt = now; } catch (error) { try { appendErrorLog("system", { diff --git a/src/lib/server/systemConfig.test.ts b/src/lib/server/systemConfig.test.ts index 3704658..dc29c4f 100644 --- a/src/lib/server/systemConfig.test.ts +++ b/src/lib/server/systemConfig.test.ts @@ -95,6 +95,23 @@ describe("systemConfig", () => { ).toThrow(/backend data dir/i); }); + it("rejects branding file names outside the fixed allowlist", () => { + expect(() => + validateSystemConfig({ branding: { logoFile: "../../../tmp/x.svg" } }) + ).toThrow(/logoFile/); + expect(() => + validateSystemConfig({ branding: { logoFile: "favicon.png" } }) + ).toThrow(/logoFile/); + expect(() => + validateSystemConfig({ branding: { faviconFile: "../favicon.ico" } }) + ).toThrow(/faviconFile/); + const valid = validateSystemConfig({ + branding: { logoFile: "logo.svg", faviconFile: "favicon.ico" }, + }); + expect(valid.branding.logoFile).toBe("logo.svg"); + expect(valid.branding.faviconFile).toBe("favicon.ico"); + }); + it("derives effective security with env/code defaults", () => { const sec = effectiveSecurity(); expect(sec.authEnabled).toBe(false); // WEBUI_AUTH_ENABLED unset diff --git a/src/lib/server/systemConfig.ts b/src/lib/server/systemConfig.ts index 8948477..265799a 100644 --- a/src/lib/server/systemConfig.ts +++ b/src/lib/server/systemConfig.ts @@ -131,6 +131,13 @@ function optInt( return value as number; } +// Branding file fields feed an unauthenticated asset route, so only the +// exact fixed names the upload route produces are acceptable. +const BRANDING_FILE_RE: Record<"logoFile" | "faviconFile", RegExp> = { + logoFile: /^logo\.(png|jpe?g|svg)$/, + faviconFile: /^favicon\.(png|jpe?g|svg|ico)$/, +}; + export function validateSystemConfig(input: unknown): SystemConfig { const source = isRecord(input) ? input : {}; const merged = merge(source); @@ -149,6 +156,13 @@ export function validateSystemConfig(input: unknown): SystemConfig { if (value !== null && typeof value !== "string") { throw new SystemConfigError(`branding.${key} must be a string or null.`); } + if (typeof value === "string" && !BRANDING_FILE_RE[key].test(value)) { + throw new SystemConfigError( + `branding.${key} must be a fixed asset file name (e.g. ${ + key === "logoFile" ? "logo.svg" : "favicon.ico" + }).` + ); + } } merged.loginTerms.enabled = merged.loginTerms.enabled === true;