Skip to content

Commit 234936d

Browse files
sarg3ntclaude
andauthored
Fix/firewall config editor issue 82 (#88)
* fix(#82): firewall config editor — consistent header + 400-response handling Three fixes for the firewall configuration editor on top of an audit of all config/settings pages so the headers are consistent across the app. 1. **Title bar** — both `/config/firewall/{box}` and `/config/haproxy/{box}` were rendering their own bold `<h2>Firewall Configuration</h2>` plus the box name as a duplicate sub-label inside `page-header-source`. The other gears get a small breadcrumb label from `gearLabelForPath()` in `base.templ`, which is what sits next to the box pill. Moved both editor titles into that switch so they render in the same small style as every other gear, and stripped the redundant per-page `<h2>` + box-name span. 2. **`UpdateFirewallConfig` 400-response handling** — the client wrapper used the generic `doRequestWithBody`, which surfaces any HTTP 4xx/5xx as a plain Go error. The agent returns 400 with a *valid JSON body* containing the `nft -c -f` validation output (or a SHA-mismatch message), and the `APIError` path in the dashboard then collapses that into a 500 with a plain-text "Failed to ..." body, which the JS `await response.json()` throws on — leaving the UI with the generic catch-block "Validation request failed" instead of the actual validator output. Mirrors what `UpdateHAProxyConfig` already does: parse the JSON body regardless of status code and only fall back to `APIError` if the body isn't JSON. 3. **"Back to Dashboard"-style links removed** across the app, per the issue request. The global header (box pill + breadcrumb) already tells the user where they are; the back-anchor was redundant. Cleaned up in: `firewall_config`, `haproxy_config` (both variants), `haproxy_gear_settings`, `server_git_settings`, `backup`, and `gears` (both the list page and the gear detail sub-title). The detail page keeps its `· {gear}` sub-title since the breadcrumb only shows "Settings". Also added defensive `logger.Warn` lines on the firewall validate/save "Server not found" branches so the next reproduction surfaces the box_id and DB-error context in the gearbox logs — there's a report of validate/save hitting that branch when the page itself just rendered fine for the same box, and there's no logging yet to tell us why. Closes #82 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#82): root cause — templ `{ ... }` doesn't interpolate inside <script> The "Server not found" the user saw on Validate/Save was a **broken URL**, not a missing box. templ does NOT interpolate `{ server.BoxID }` inside `<script>` blocks — the brace expression renders as literal text. The generated HTML was: const serverID = "{ server.BoxID }"; so the JS template literal built /api/{ server.BoxID }/firewall/config/validate which the browser URL-encoded to /api/%7B%20server.BoxID%20%7D/firewall/config/validate That matched the chi route `/{boxID}/firewall/config/validate` with the literal string `{ server.BoxID }` as boxID, `GetBoxByBoxID` (predictably) returned nil, and the handler emitted its `Server not found` JSON. Found in the gearbox access log: "POST .../api/%7B%20server.BoxID%20%7D/firewall/config HTTP/1.1" 404 47B Fix mirrors the pattern haproxy_config already uses: render the Go values into hidden `<input>` elements (where templ *does* interpolate, because attribute values aren't script content) and read them from the JS via `getElementById(...).value`. Added a comment in the template so the next person doesn't reintroduce the same trap. This is the actual fix for issues 1 and 2 in #82; the previous commit on this branch addressed the title-bar and "Back to Dashboard" cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(#82): nftables editor — CodeMirror, full-bleed surface, header toolbar Rewrites the firewall configuration editor around CodeMirror 5 so it picks up the same kind of affordances we'd expect from any first-class config editor: syntax-aware coloring, click-through tables/chains nav, gutter + underline markers for the lines `nft -c -f` rejected, and as-you-type autocomplete of nftables keywords. Adds about ~200 KB of CDN'd JS/CSS from unpkg (already on the CSP allowlist) — no new vendored assets in the repo, no bundler required. Visual / layout (issue follow-ups 1–3): * The editor now runs flush to the edges of the gear surface using the same `-mx-6 -mt-6 bg-[#0d0d0d]` trick as the log viewer, so the middle-of-the-screen narrow column is gone and the editor uses the full width. * Validate / Save & Apply / Backups / nftables Wiki / Wrap-toggle buttons live in `page-header-source` and get hoisted to the right of the breadcrumb in the global header, matching every other gear. * The old toolbar card under the page title is removed. * Theme: CodeMirror's `material-darker` with a pinned background of `#0d0d0d` so the editor sits in the same near-black as the log viewer. Custom token coloring for `nftables` mode: verdicts (accept/drop/jump) get a hot foreground, families and hooks get distinct hues, IP literals colorize as atoms, etc. Validation feedback (issue follow-up 4): * Valid config → success toast via `window.showToast`. * Invalid config → error text drops into a panel under the left-rail nav, with red gutter ✖ markers + wavy underlines on every line nft pointed at. `parseNftErrorLines()` extracts the `path:line:col-col:` prefix from `nft`'s output. The panel has a close button; gutter markers persist until the next validate so the user can keep navigating the bad lines after dismissing the text. Syntax coloring + autocomplete (issue follow-ups 5–6): * New custom CodeMirror mode in `static/js/firewall_config/nftables-mode.js` with a hand-curated keyword taxonomy (statements / families / hooks / verdicts / match expressions / value literals). Both the highlighter and the hint provider read from the same lists. * Hint dropdown opens on Ctrl-Space/Cmd-Space AND as-you-type when the last input is a letter or underscore — prefix match first, substring match as a fallback so typos surface something. Empty-token completion suggests top-level statements (table/chain/rule/…) so users at the start of a line get useful suggestions immediately. Other niceties: * Ctrl-S / Cmd-S triggers Save & Apply. * Ctrl-/ / Cmd-/ toggles comment. * Section nav (tables + chains) rendered from the agent's parsed `Sections` payload; click → scrolls + focuses the matching line. * Wrap-toggle button flips `lineWrapping`. * Bottom strip surfaces recent change history in the same dark theme rather than a separate card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(#82): nftables editor — snippet catalog + keyword hover tooltips Two follow-ups on the editor rewrite that lift it from "decent" to something I'd actually want to use: **Snippet library.** New Snippets button in the page-header toolbar opens a two-column modal with a search box, a category-grouped list on the left, and a code preview on the right. Insert at Cursor drops the snippet into the editor at the current position, re-indented to match the surrounding block so a chain-internal snippet doesn't end up flush-left. Catalog (`snippets.js`) is curated from the nftables wiki plus the public github collections that turned up in the search: mqus/nft-rules, k4yt3x/nftables, ipr-cnrs/nftables, the Gentoo wiki. Sixteen entries across six categories — Skeleton, Allow rules, Drop rules, Rate limiting, NAT, Logging, Tables/chains. Each pattern is small + self-contained so it's a starting point, not a finished policy. **Keyword hover tooltips.** Pause the mouse over a token the editor recognizes and a popover shows a one-line description plus an "open docs ›" link pointing at the right wiki page (Configuring tables, Sets, Netfilter hooks, NAT, ...). Driven by an explicit `KEYWORD_DOCS` table in `editor.js` rather than scraped — that means the help text is curated for accuracy, but if a token's not in the table we don't show anything (so a typo doesn't get a misleading tooltip). Coverage: all top-level statements, families, hooks, verdicts, common match expressions, and the actions you'd reach for first (log, snat, dnat, masquerade, redirect, policy, priority, hook). Positioning auto-flips when it would overflow the viewport. A handful of small fixes that fell out: * Snippet insert respects the readonly-when-not-canEdit state — the Insert button only enables when the user can save. * Tooltips hide on mousedown so they don't fight with click selections in the editor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(#82): real-time validation + per-error hover tooltips + save error modal Replaces the left-column "Validation" panel with a fully inline feedback flow that runs as the user types. The panel was the right idea for save failures but too noisy as a live indicator during editing. Real-time validation: * New `cm.on('change', ...)` handler kicks off `nft -c -f` 1s after the user stops typing (1000ms debounce). Faster typists effectively get a single validate per editing burst. A monotonic `validationGeneration` counter lets us drop stale responses when the user edits again before the previous request lands. * New tiny status pill in the left-rail header next to "Tables & Chains" surfaces the live state: idle / pending / checking (pulsing) / valid (green) / errors (red) / unknown (yellow). Title attribute explains each. * Editor never auto-scrolls on marker updates anymore — that was unbearable during real-time validation as the viewport kept jumping around. The red status pill + the gutter ✖ in the line-number column are enough signal; the user navigates themselves via the section nav. Per-error hover tooltips: * `findErrorAt(pos)` resolves a cursor position against the cached `validationErrors` array. The existing mousemove handler now checks this BEFORE the keyword-doc lookup — if the user is pointing at a red-underlined span, the error explanation wins over "here's what `dport` means." * Error tooltip variant has a red accent + shows both the humanized message AND the raw nft output (in a smaller dashed-box) so power users still see what nft literally said. nft error humanization (`humanizeNftError`): * Translates the most opaque `nft -c -f` complaints into plain English: "syntax error, unexpected newline, expecting string" becomes "Syntax error — expected an identifier here, but saw a new line. Check for missing keywords, semicolons, or braces." * Covers: syntax errors (with expected/got rewriting), the "no such file or directory" interface-lookup failure, IPv4/IPv6 mixing conflicts, unknown set / map / chain references, malformed IP/port literals, unknown identifiers, kernel rejections, and duplicate-name errors. Unrecognized messages pass through verbatim so we never hide what nft actually said. Save & Apply error modal: * When `nft` rejects the buffer on Save, a dedicated red-headered modal shows the raw nft output and a one-line summary ("3 errors — hover the underlined lines for details" or, for a single error, the humanized message itself). Replaces the previous left-rail panel reuse — the modal is appropriate because Save is a deliberate action and demands an explicit acknowledgement. * The inline markers are still applied alongside the modal so the user can dismiss the modal and navigate the bad lines via hover. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#82): firewall editor — clamp shell to viewport, kill body-level scroll The shell was sized `min-height: calc(100vh - 55px)` and only negated the top + side padding of the surrounding `<main class="p-6">`, leaving: * `<main>`'s bottom padding (24px) intact below the shell * the 55-vs-57px header-height mismatch (`<header>` is 55px tall + 2px border; `#main-content` uses `pt-[57px]` to clear it) Net result was a ~26px overflow on the page body. Manifested as a horizontal scroll thumb at the bottom of the window and the ability to scroll the whole page vertically — which, because the global header is `fixed`, pulled the editor's left rail (incl. the "Tables & Chains" header) up out of view. Fix: clamp the shell to `height: calc(100vh - 57px)` (hard size, not min) and add `-mb-6` so the bottom padding is canceled too. After this the shell occupies exactly the available viewport area; the editor and the nav each handle their own internal scroll, and the body never grows a scrollbar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#82): firewall editor — lock body overflow + 100dvh shell height Pixel-math approach in the previous commit still let some users see a body-level scrollbar (the shell ended up a couple pixels taller than expected — likely a scrollbar-width or theme-specific header-height edge case). Two changes together close the door: * editor.js init() now sets `html { overflow: hidden }` and `body { overflow: hidden }`. Since each gear page is a full HTTP load, the side-effect is naturally undone on navigation, so other pages are unaffected. * Shell height switched from `calc(100vh - 57px)` to `calc(100dvh - 57px)` (`dvh` = dynamic viewport height, ignores any mobile browser-chrome retraction). All four sides of the shell now use `-mx-6 -my-6` to cancel `<main class="p-6">` padding evenly. The internal scrollers (CodeMirror + `#firewall-section-nav`) handle overflow as before; the body just isn't allowed to. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#82): address Copilot review findings on firewall editor PR - helpers.go: refresh sectionsJSON doc-comment — payload is embedded in a hidden <textarea>, not a <script type="application/json"> (the inline templ comment already noted this; the helper doc was stale). - helpers.go: add shortSHA() — safe 12-char truncation that returns the original string when shorter than 12 chars, instead of panicking on a bounds-out-of-range slice. - firewall_config.templ: use shortSHA(config.SHA256) for the header SHA display; document the non-nil `config` precondition in the templ doc-comment (the HTTP handler routes nil/error to FirewallConfigPageWithError, so the hidden input/textarea elements legitimately rely on it). - editor.js: drop the unread `btn.dataset.line` assignment in makeNavButton — scrollToLine is invoked via the closure with section.start_line, so the dataset attribute was dead code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7737d5f commit 234936d

15 files changed

Lines changed: 1944 additions & 497 deletions

File tree

gearbox/internal/framework/agent/client.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,14 +1109,45 @@ func (c *Client) GetFirewallConfig() (*FirewallConfigResponse, error) {
11091109
}
11101110

11111111
// UpdateFirewallConfig updates the firewall configuration.
1112+
// Note: Returns a response even on validation failure (400) - check resp.Success field.
1113+
// The agent returns 400 with a valid JSON body containing validation details
1114+
// (nft -c -f output, expected-SHA mismatch, etc.) rather than a generic error.
11121115
func (c *Client) UpdateFirewallConfig(req *FirewallConfigUpdateRequest) (*FirewallConfigUpdateResponse, error) {
1113-
body, err := c.doRequestWithBody("POST", "/api/v1/firewall/config", req)
1116+
fullURL := c.baseURL + "/api/v1/firewall/config"
1117+
1118+
jsonBody, err := json.Marshal(req)
11141119
if err != nil {
1115-
return nil, err
1120+
return nil, fmt.Errorf("failed to marshal request body: %w", err)
1121+
}
1122+
1123+
httpReq, err := http.NewRequest("POST", fullURL, strings.NewReader(string(jsonBody)))
1124+
if err != nil {
1125+
return nil, fmt.Errorf("failed to create request: %w", err)
1126+
}
1127+
1128+
httpReq.Header.Set("Authorization", "Bearer "+c.apiKey)
1129+
httpReq.Header.Set("Accept", "application/json")
1130+
httpReq.Header.Set("Content-Type", "application/json")
1131+
1132+
httpResp, err := c.httpClient.Do(httpReq)
1133+
if err != nil {
1134+
return nil, fmt.Errorf("request failed: %w", err)
1135+
}
1136+
defer httpResp.Body.Close()
1137+
1138+
body, err := io.ReadAll(httpResp.Body)
1139+
if err != nil {
1140+
return nil, fmt.Errorf("failed to read response body: %w", err)
11161141
}
11171142

11181143
var resp FirewallConfigUpdateResponse
11191144
if err := json.Unmarshal(body, &resp); err != nil {
1145+
if httpResp.StatusCode >= 400 {
1146+
return nil, &APIError{
1147+
StatusCode: httpResp.StatusCode,
1148+
Message: fmt.Sprintf("HTTP %d: %s", httpResp.StatusCode, http.StatusText(httpResp.StatusCode)),
1149+
}
1150+
}
11201151
return nil, fmt.Errorf("failed to parse firewall config update response: %w", err)
11211152
}
11221153

gearbox/internal/framework/handler/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,7 @@ func (h *Handler) APIFirewallConfigSave(w http.ResponseWriter, r *http.Request)
579579
boxID := chi.URLParam(r, "boxID")
580580
server, err := h.db.GetBoxByBoxID(boxID)
581581
if err != nil || server == nil {
582+
h.logger.Warn("firewall config save: box lookup failed", "box_id", boxID, "err", err, "server_nil", server == nil)
582583
h.jsonError(w, "Server not found", http.StatusNotFound)
583584
return
584585
}
@@ -645,6 +646,7 @@ func (h *Handler) APIFirewallConfigValidate(w http.ResponseWriter, r *http.Reque
645646
boxID := chi.URLParam(r, "boxID")
646647
server, err := h.db.GetBoxByBoxID(boxID)
647648
if err != nil || server == nil {
649+
h.logger.Warn("firewall config validate: box lookup failed", "box_id", boxID, "err", err, "server_nil", server == nil)
648650
h.jsonError(w, "Server not found", http.StatusNotFound)
649651
return
650652
}

gearbox/internal/framework/templates/layouts/base.templ

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ func gearLabelForPath(path string) string {
5050
return "Alerts"
5151
case path == "/os-updates" || strings.HasPrefix(path, "/os-updates/"):
5252
return "OS Updates"
53+
case path == "/config/firewall" || strings.HasPrefix(path, "/config/firewall/"):
54+
return "Firewall Configuration"
55+
case path == "/config/haproxy" || strings.HasPrefix(path, "/config/haproxy/"):
56+
return "HAProxy Configuration"
5357
case path == "/settings" || strings.HasPrefix(path, "/settings/"):
5458
return "Settings"
5559
case path == "/welcome":

gearbox/internal/framework/templates/pages/backup.templ

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,6 @@ import (
1111
// BackupPage renders the database backup management page.
1212
templ BackupPage(user *models.User, backups []database.BackupInfo, backupDir string, successMsg, errorMsg string) {
1313
@layouts.Base("Database Backups", user, "/settings") {
14-
<!-- Hidden container for page header content -->
15-
<div id="page-header-source" class="hidden">
16-
<div class="flex items-center">
17-
<h2 class="text-xl font-bold text-gray-800 dark:text-gray-100">Database Backups</h2>
18-
</div>
19-
<div class="flex-1"></div>
20-
<a href="/settings" class="text-sm text-blue-600 dark:text-blue-400 hover:underline flex items-center">
21-
<svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
22-
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M15 19l-7-7 7-7"></path>
23-
</svg>
24-
Back to Settings
25-
</a>
26-
</div>
27-
2814
<div class="max-w-6xl mx-auto px-4 py-8">
2915
if successMsg != "" {
3016
@ui.ToastOnLoad(successMsg, "success")
@@ -160,18 +146,6 @@ templ BackupPage(user *models.User, backups []database.BackupInfo, backupDir str
160146
}
161147

162148
<script>
163-
// Move page header content to main header
164-
function setupPageHeader() {
165-
const source = document.getElementById('page-header-source');
166-
const target = document.getElementById('header-page-content');
167-
if (source && target) {
168-
while (source.firstChild) {
169-
target.appendChild(source.firstChild);
170-
}
171-
source.remove();
172-
}
173-
}
174-
175149
async function createBackup() {
176150
try {
177151
const response = await fetch('/api/backup/create', {
@@ -289,8 +263,6 @@ templ BackupPage(user *models.User, backups []database.BackupInfo, backupDir str
289263
function downloadBackup(backupPath) {
290264
window.location.href = '/api/backup/download/' + encodeURIComponent(backupPath);
291265
}
292-
293-
document.addEventListener('DOMContentLoaded', setupPageHeader);
294266
</script>
295267
</div>
296268
}

0 commit comments

Comments
 (0)