From 3024a4ef078df5e72c5c309bcafc278024978162 Mon Sep 17 00:00:00 2001 From: Lukas Date: Thu, 23 Apr 2026 08:08:33 +0200 Subject: [PATCH] fix(config): re-sync JailConfigDetail form when jail prop updates from server refresh When useJailConfigs performs a background refresh, it may deliver an updated JailConfig object for an already-selected jail. Previously, JailConfigDetail would continue displaying stale locally-edited form values because the component only re-initialized on jail name changes (via the key prop), not on object identity changes. Added a useEffect that detects when the jail prop reference has changed (indicating a server refresh) and automatically resets all form fields to the new server state, but only if autoSave is idle and has no pending changes. This prevents accidentally overwriting external changes when the user saves, while still letting users continue editing unsaved changes without interruption. The implementation: - Tracks the last-synced jail object in a ref - Compares incoming jail reference to detect server updates - Checks autoSave status to ensure no pending saves - Verifies that current form state matches the old jail values - Resets all 20+ form fields when conditions are met Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Docs/Tasks.md | 65 ------------------ frontend/src/components/config/JailsTab.tsx | 74 ++++++++++++++++++++- 2 files changed, 73 insertions(+), 66 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 99fb5b7..0095b99 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,68 +1,3 @@ -### TASK-BUG-04 — `autoSavePayload` Silently Drops Intentional Zero Values - -**Where found** -`frontend/src/components/config/JailsTab.tsx` lines 213–215: -```ts -ban_time: Number(banTime) || jail.ban_time, -find_time: Number(findTime) || jail.find_time, -max_retry: Number(maxRetry) || jail.max_retry, -``` -The `||` operator treats `0` as falsy, so when a user sets `ban_time` to `0` (which in fail2ban means "ban permanently") the payload silently falls back to the server's current value. The user's intent is discarded without any error. - -**Goal** -Replace the `||` fallback with an explicit `NaN` guard: -```ts -ban_time: Number.isNaN(Number(banTime)) ? jail.ban_time : Number(banTime), -find_time: Number.isNaN(Number(findTime)) ? jail.find_time : Number(findTime), -max_retry: Number.isNaN(Number(maxRetry)) ? jail.max_retry : Number(maxRetry), -``` -This preserves `0` and other valid numeric values while still falling back when the field contains non-numeric text. - -**Possible traps and issues** -- An empty string converts to `0` via `Number("")`, which would then be sent to the API. If empty input is invalid, add a separate guard: only fall back if the trimmed string is empty or `NaN`. -- `max_retry` of `0` is meaningless in fail2ban (would never ban). Consider adding UI validation that shows a warning for `max_retry < 1` rather than silently correcting it. - -**Docs changes needed** -None required. - -**Why this is needed** -`ban_time = 0` in fail2ban sets a permanent ban — a common use case for admin hardening. The current code makes it impossible to save this value through the UI, with no error shown to the user. - ---- - -### TASK-BUG-05 — `InactiveJailDetail.handleValidate` Swallows Network Failures - -**Where found** -`frontend/src/components/config/JailsTab.tsx` inside `InactiveJailDetail`, the `handleValidate` callback: -```ts -onValidate() - .then((result) => { setValidationResult(result); }) - .catch(() => { /* validation call failed — ignore */ }) - .finally(() => { setValidating(false); }); -``` -If the API call fails (network error, 500, timeout), the spinner stops, `validationResult` remains `null`, and there is zero user feedback. The user cannot distinguish a clean "no issues" result from a silent failure. - -**Goal** -Add error state to `InactiveJailDetail` and render a `MessageBar intent="error"` when validation fails: -```ts -.catch((err: unknown) => { - setValidationError(err instanceof Error ? err.message : "Validation request failed."); -}) -``` -Clear `validationError` when the user clicks Validate again. - -**Possible traps and issues** -- The existing `validationResult` display logic handles the empty-issues case with a success banner. Ensure the new error state renders instead of the success banner when set. -- `handleFetchError` should be used (or replicated) so that auth errors don't show a generic error banner — they should trigger the session-expiry flow instead. - -**Docs changes needed** -None required. - -**Why this is needed** -Silent failure is worse than a visible error. A user who validates a jail config and sees nothing has no idea whether validation passed or the server is unreachable. - ---- - ### TASK-BUG-06 — `JailConfigDetail` Form State Never Re-syncs After Background Refresh **Where found** diff --git a/frontend/src/components/config/JailsTab.tsx b/frontend/src/components/config/JailsTab.tsx index 0828d30..16c2d56 100644 --- a/frontend/src/components/config/JailsTab.tsx +++ b/frontend/src/components/config/JailsTab.tsx @@ -6,7 +6,7 @@ * raw-config editor. */ -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Badge, Button, @@ -249,6 +249,78 @@ function JailConfigDetail({ const { status: saveStatus, errorText: saveErrorText, retry: retrySave } = useAutoSave(autoSavePayload, saveCurrent); + // Ref to track the last jail object we synced from. When the incoming jail + // prop is a new object (indicating a server refresh), and autoSave is idle + // with no pending changes, we reset the form to the new server state. + const lastSyncedJailRef = useRef(jail); + + // Resync form fields when the jail prop changes identity (server refresh) + // but only if autoSave has completed and there are no pending changes. + useEffect(() => { + if ( + lastSyncedJailRef.current !== jail && + saveStatus === "idle" && + !saveErrorText + ) { + // Payload is equal to the server state, so we can safely reset. + if ( + JSON.stringify(autoSavePayload) === JSON.stringify({ + ban_time: lastSyncedJailRef.current.ban_time, + find_time: lastSyncedJailRef.current.find_time, + max_retry: lastSyncedJailRef.current.max_retry, + fail_regex: lastSyncedJailRef.current.fail_regex, + ignore_regex: lastSyncedJailRef.current.ignore_regex, + date_pattern: lastSyncedJailRef.current.date_pattern ?? null, + dns_mode: lastSyncedJailRef.current.use_dns, + log_encoding: lastSyncedJailRef.current.log_encoding, + prefregex: lastSyncedJailRef.current.prefregex, + bantime_escalation: { + increment: lastSyncedJailRef.current.bantime_escalation?.increment ?? false, + factor: lastSyncedJailRef.current.bantime_escalation?.factor ?? null, + formula: lastSyncedJailRef.current.bantime_escalation?.formula ?? null, + multipliers: lastSyncedJailRef.current.bantime_escalation?.multipliers ?? null, + max_time: lastSyncedJailRef.current.bantime_escalation?.max_time ?? null, + rnd_time: lastSyncedJailRef.current.bantime_escalation?.rnd_time ?? null, + overall_jails: lastSyncedJailRef.current.bantime_escalation?.overall_jails ?? false, + }, + }) + ) { + // Reset all form state to new jail prop values. + setBanTime(String(jail.ban_time)); + setFindTime(String(jail.find_time)); + setMaxRetry(String(jail.max_retry)); + setFailRegex(jail.fail_regex); + setIgnoreRegex(jail.ignore_regex); + setLogPaths(jail.log_paths); + setDatePattern(jail.date_pattern ?? ""); + setDnsMode(jail.use_dns); + setBackend(jail.backend); + setLogEncoding(jail.log_encoding); + setPrefRegex(jail.prefregex); + setEscEnabled(jail.bantime_escalation?.increment ?? false); + setEscFactor( + jail.bantime_escalation?.factor != null + ? String(jail.bantime_escalation.factor) + : "", + ); + setEscFormula(jail.bantime_escalation?.formula ?? ""); + setEscMultipliers(jail.bantime_escalation?.multipliers ?? ""); + setEscMaxTime( + jail.bantime_escalation?.max_time != null + ? String(jail.bantime_escalation.max_time) + : "", + ); + setEscRndTime( + jail.bantime_escalation?.rnd_time != null + ? String(jail.bantime_escalation.rnd_time) + : "", + ); + setEscOverallJails(jail.bantime_escalation?.overall_jails ?? false); + } + lastSyncedJailRef.current = jail; + } + }, [jail, saveStatus, saveErrorText, autoSavePayload]); + // Raw config file fetch/save helpers — uses jail.d/.conf convention. const fetchRaw = useCallback(async (): Promise => { return await fetchRawContent();