Fix/firewall config editor issue 82 - #88
Merged
Merged
Conversation
…andling
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>
…ript>
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>
…oolbar 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>
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>
…or 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>
…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>
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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rewrites the firewall (nftables) configuration editor as a CodeMirror 5–based experience, replacing the simple textarea + inline-script implementation. It introduces a custom nftables syntax mode + hint provider, a curated snippet catalog, real-time nft -c -f validation with inline lint markers and humanized error tooltips, a left-rail tables/chains navigator, and modal flows for backups, snippets, and save-failure errors. It also cleans up several pages that previously hand-moved breadcrumb DOM into the global header (now driven by gearLabelForPath), adds new /config/firewall and /config/haproxy paths to the breadcrumb resolver, and hardens the agent client's UpdateFirewallConfig to surface validation responses on 4xx with structured errors.
Changes:
- New CodeMirror-based firewall editor: custom
nftablesmode, autocomplete, snippet library, real-time validation with inline markers and humanized tooltips. - Removed per-page
page-header-sourceshims across firewall/haproxy/git/gears/backup pages; breadcrumb labeling centralized inbase.templ'sgearLabelForPath. - Agent client
UpdateFirewallConfignow reads the response body manually so 4xx validation responses surface their JSON body to the dashboard.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gearbox/static/js/firewall_config/editor.js | Main editor controller: validation, lint, snippets, hover tooltips, save flow |
| gearbox/static/js/firewall_config/nftables-mode.js | CodeMirror mode + hint provider for nftables |
| gearbox/static/js/firewall_config/snippets.js | Curated snippet catalog data |
| gearbox/static/css/firewall_config_editor.css | Editor theming, lint markers, tooltip styling |
| gearbox/internal/framework/templates/pages/firewall_config.templ | Replaces inline editor + scripts with CodeMirror mount, toolbar, modals, hidden inputs |
| gearbox/internal/framework/templates/pages/helpers.go | Adds canEditValue and sectionsJSON helpers |
| gearbox/internal/framework/templates/layouts/base.templ | Adds /config/firewall and /config/haproxy breadcrumb labels |
| gearbox/internal/framework/templates/pages/{server_git_settings,haproxy_gear_settings,haproxy_config,gears,backup}.templ | Removes per-page header-shim DOM/scripts |
| gearbox/internal/framework/templates/pages/haproxy_config/editor_header.templ | Removes redundant inline title now covered by global header |
| gearbox/internal/framework/handler/config.go | Adds warn-level logging when firewall save/validate box lookup fails |
| gearbox/internal/framework/agent/client.go | Reworks UpdateFirewallConfig to handle 4xx responses with parseable validation body |
- 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.