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();