From ad1770cda92e0533ca47bc2e447e4d018da69d1f Mon Sep 17 00:00:00 2001 From: Dave Sargent Date: Thu, 14 May 2026 00:42:00 -0700 Subject: [PATCH 1/8] =?UTF-8?q?fix(#82):=20firewall=20config=20editor=20?= =?UTF-8?q?=E2=80=94=20consistent=20header=20+=20400-response=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `

Firewall Configuration

` 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 `

` + 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) --- gearbox/internal/framework/agent/client.go | 35 ++++++++++- gearbox/internal/framework/handler/config.go | 2 + .../framework/templates/layouts/base.templ | 4 ++ .../framework/templates/pages/backup.templ | 28 --------- .../templates/pages/firewall_config.templ | 58 +------------------ .../framework/templates/pages/gears.templ | 19 ------ .../templates/pages/haproxy_config.templ | 28 +-------- .../pages/haproxy_config/editor_header.templ | 8 +-- .../pages/haproxy_gear_settings.templ | 32 ---------- .../templates/pages/server_git_settings.templ | 28 --------- 10 files changed, 46 insertions(+), 196 deletions(-) diff --git a/gearbox/internal/framework/agent/client.go b/gearbox/internal/framework/agent/client.go index cb7b50a..7fed6dd 100644 --- a/gearbox/internal/framework/agent/client.go +++ b/gearbox/internal/framework/agent/client.go @@ -1109,14 +1109,45 @@ func (c *Client) GetFirewallConfig() (*FirewallConfigResponse, error) { } // UpdateFirewallConfig updates the firewall configuration. +// Note: Returns a response even on validation failure (400) - check resp.Success field. +// The agent returns 400 with a valid JSON body containing validation details +// (nft -c -f output, expected-SHA mismatch, etc.) rather than a generic error. func (c *Client) UpdateFirewallConfig(req *FirewallConfigUpdateRequest) (*FirewallConfigUpdateResponse, error) { - body, err := c.doRequestWithBody("POST", "/api/v1/firewall/config", req) + fullURL := c.baseURL + "/api/v1/firewall/config" + + jsonBody, err := json.Marshal(req) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + httpReq, err := http.NewRequest("POST", fullURL, strings.NewReader(string(jsonBody))) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + httpReq.Header.Set("Authorization", "Bearer "+c.apiKey) + httpReq.Header.Set("Accept", "application/json") + httpReq.Header.Set("Content-Type", "application/json") + + httpResp, err := c.httpClient.Do(httpReq) + if err != nil { + return nil, fmt.Errorf("request failed: %w", err) + } + defer httpResp.Body.Close() + + body, err := io.ReadAll(httpResp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) } var resp FirewallConfigUpdateResponse if err := json.Unmarshal(body, &resp); err != nil { + if httpResp.StatusCode >= 400 { + return nil, &APIError{ + StatusCode: httpResp.StatusCode, + Message: fmt.Sprintf("HTTP %d: %s", httpResp.StatusCode, http.StatusText(httpResp.StatusCode)), + } + } return nil, fmt.Errorf("failed to parse firewall config update response: %w", err) } diff --git a/gearbox/internal/framework/handler/config.go b/gearbox/internal/framework/handler/config.go index 7409d9c..ab0ffce 100644 --- a/gearbox/internal/framework/handler/config.go +++ b/gearbox/internal/framework/handler/config.go @@ -579,6 +579,7 @@ func (h *Handler) APIFirewallConfigSave(w http.ResponseWriter, r *http.Request) boxID := chi.URLParam(r, "boxID") server, err := h.db.GetBoxByBoxID(boxID) if err != nil || server == nil { + h.logger.Warn("firewall config save: box lookup failed", "box_id", boxID, "err", err, "server_nil", server == nil) h.jsonError(w, "Server not found", http.StatusNotFound) return } @@ -645,6 +646,7 @@ func (h *Handler) APIFirewallConfigValidate(w http.ResponseWriter, r *http.Reque boxID := chi.URLParam(r, "boxID") server, err := h.db.GetBoxByBoxID(boxID) if err != nil || server == nil { + h.logger.Warn("firewall config validate: box lookup failed", "box_id", boxID, "err", err, "server_nil", server == nil) h.jsonError(w, "Server not found", http.StatusNotFound) return } diff --git a/gearbox/internal/framework/templates/layouts/base.templ b/gearbox/internal/framework/templates/layouts/base.templ index 3f04fb9..d776ee1 100644 --- a/gearbox/internal/framework/templates/layouts/base.templ +++ b/gearbox/internal/framework/templates/layouts/base.templ @@ -50,6 +50,10 @@ func gearLabelForPath(path string) string { return "Alerts" case path == "/os-updates" || strings.HasPrefix(path, "/os-updates/"): return "OS Updates" + case path == "/config/firewall" || strings.HasPrefix(path, "/config/firewall/"): + return "Firewall Configuration" + case path == "/config/haproxy" || strings.HasPrefix(path, "/config/haproxy/"): + return "HAProxy Configuration" case path == "/settings" || strings.HasPrefix(path, "/settings/"): return "Settings" case path == "/welcome": diff --git a/gearbox/internal/framework/templates/pages/backup.templ b/gearbox/internal/framework/templates/pages/backup.templ index e38727d..36e7b11 100644 --- a/gearbox/internal/framework/templates/pages/backup.templ +++ b/gearbox/internal/framework/templates/pages/backup.templ @@ -11,20 +11,6 @@ import ( // BackupPage renders the database backup management page. templ BackupPage(user *models.User, backups []database.BackupInfo, backupDir string, successMsg, errorMsg string) { @layouts.Base("Database Backups", user, "/settings") { - - -
if successMsg != "" { @ui.ToastOnLoad(successMsg, "success") @@ -160,18 +146,6 @@ templ BackupPage(user *models.User, backups []database.BackupInfo, backupDir str }
} diff --git a/gearbox/internal/framework/templates/pages/firewall_config.templ b/gearbox/internal/framework/templates/pages/firewall_config.templ index 98cfeb3..4813138 100644 --- a/gearbox/internal/framework/templates/pages/firewall_config.templ +++ b/gearbox/internal/framework/templates/pages/firewall_config.templ @@ -10,21 +10,7 @@ import ( // FirewallConfigPage renders the firewall configuration editor page. templ FirewallConfigPage(user *models.User, server *database.BoxDB, config *agent.FirewallConfigResponse, gitConfig *database.BoxGitConfig, changes []database.ConfigChange, canEdit bool) { - @layouts.Base("Firewall Configuration", user, "/config") { - - + @layouts.Base("Firewall Configuration", user, "/config/firewall") {
@@ -184,17 +170,6 @@ templ FirewallConfigPage(user *models.User, server *database.BoxDB, config *agen const serverID = "{ server.BoxID }"; let currentSHA = "{ config.SHA256 }"; - function setupPageHeader() { - const source = document.getElementById('page-header-source'); - const target = document.getElementById('header-page-content'); - if (source && target) { - while (source.firstChild) { - target.appendChild(source.firstChild); - } - source.remove(); - } - } - async function validateConfig() { const content = document.getElementById('config-editor').value; const output = document.getElementById('validation-output'); @@ -369,22 +344,13 @@ templ FirewallConfigPage(user *models.User, server *database.BoxDB, config *agen const lineHeight = parseFloat(getComputedStyle(editor).lineHeight) || 20; editor.scrollTop = (line - 1) * lineHeight; } - - document.addEventListener('DOMContentLoaded', setupPageHeader); } } // FirewallConfigPageWithError renders the config page with an error message. templ FirewallConfigPageWithError(user *models.User, server *database.BoxDB, errorMsg string, canEdit bool) { - @layouts.Base("Firewall Configuration", user, "/config") { - - + @layouts.Base("Firewall Configuration", user, "/config/firewall") {
@@ -397,26 +363,6 @@ templ FirewallConfigPageWithError(user *models.User, server *database.BoxDB, err
- -
- - } } diff --git a/gearbox/internal/framework/templates/pages/gears.templ b/gearbox/internal/framework/templates/pages/gears.templ index 7598551..c213c19 100644 --- a/gearbox/internal/framework/templates/pages/gears.templ +++ b/gearbox/internal/framework/templates/pages/gears.templ @@ -17,18 +17,6 @@ import ( // gear list so toggling them isn't confused with the box selector. templ GearsPage(user *models.User, servers []models.BoxConfig, currentServerID string, systemGears []database.Gear, integrations []database.Gear, successMsg, errorMsg string) { @layouts.Base("Gears", user, "/settings") { - - -
@@ -242,13 +230,6 @@ templ GearDetailPage(user *models.User, serverID string, serverName string, inte { integration.DisplayName }

-
- - - - - Back to Gears -
diff --git a/gearbox/internal/framework/templates/pages/haproxy_config.templ b/gearbox/internal/framework/templates/pages/haproxy_config.templ index 8ab4a49..6c4f39c 100644 --- a/gearbox/internal/framework/templates/pages/haproxy_config.templ +++ b/gearbox/internal/framework/templates/pages/haproxy_config.templ @@ -10,7 +10,7 @@ import ( // HAProxyConfigPage renders the HAProxy configuration editor page. templ HAProxyConfigPage(user *models.User, server *database.BoxDB, config *agent.HAProxyConfigResponse, gitConfig *database.BoxGitConfig, changes []database.ConfigChange, canEdit bool) { - @layouts.Base("HAProxy Configuration", user, "/config") { + @layouts.Base("HAProxy Configuration", user, "/config/haproxy") { @haproxy_config.EditorHeader(server, config, canEdit) @@ -25,13 +25,7 @@ templ HAProxyConfigPage(user *models.User, server *database.BoxDB, config *agent } templ HAProxyConfigPageWithError(user *models.User, server *database.BoxDB, errorMsg string, canEdit bool) { - @layouts.Base("HAProxy Configuration", user, "/config") { - + @layouts.Base("HAProxy Configuration", user, "/config/haproxy") {
@@ -44,24 +38,6 @@ templ HAProxyConfigPageWithError(user *models.User, server *database.BoxDB, erro
-
- } } diff --git a/gearbox/internal/framework/templates/pages/haproxy_config/editor_header.templ b/gearbox/internal/framework/templates/pages/haproxy_config/editor_header.templ index 77992b3..9540bff 100644 --- a/gearbox/internal/framework/templates/pages/haproxy_config/editor_header.templ +++ b/gearbox/internal/framework/templates/pages/haproxy_config/editor_header.templ @@ -6,12 +6,10 @@ import ( ) templ EditorHeader(server *database.BoxDB, config *agent.HAProxyConfigResponse, canEdit bool) { - +