Skip to content

Commit 667268d

Browse files
sarg3ntclaude
andauthored
feat: display & layout improvements (#66) (#70)
* feat: add command palette, focus trap, keymap, and shortcut help - Implemented a command palette for quick navigation and actions, accessible via Cmd/Ctrl+K. - Created a focus trap utility to manage keyboard focus within modal dialogs. - Developed a keymap module to handle global keyboard shortcuts and typing target detection. - Added a shortcut help overlay that displays global keyboard shortcuts, triggered by '?'. * fix(security): set Secure attribute on active-box cookie when TLS CodeQL on PR #70 flagged the new gearbox_active_box cookie for not setting Secure: true (findings #584, #585). The cookie is HttpOnly + SameSite=Lax but should also be Secure in HTTPS deployments. Make Secure conditional via a new requestIsTLS(r) helper: - r.TLS != nil — request served directly over HTTPS - X-Forwarded-Proto: https — TLS terminated by an upstream proxy (the homelab's HAProxy front-end is the typical case) Plain http://localhost dev still works (cookie is set without Secure when neither signal is present); production behind HAProxy now writes a Secure cookie. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address Copilot review findings on PR #70 - handler.go: /bx clears the active-box cookie whenever the cookie is present, not only when it resolves to an enabled box. Otherwise a stale cookie (referencing a deleted/disabled box) survives and the first-login auto-select stays blocked because hasCookieSet remains true. - base.templ: encodeGearsJSON now filters out disabled gears server- side, matching its own comment. Reduces payload and keeps the command palette catalog free of empty entries. - bx/pages.templ: drop the duplicate <link rel=stylesheet> for datagrid.css — layouts.Base already includes it globally. Add aria-label="Filter boxes view" to #bx-view-filter so screen readers announce it unambiguously. - shortcut-help.js: SHORTCUTS entries gain an explicit `join` field ("+" for chord, "then" for sequence, "/" for either-of). The old separator logic always emitted a single space regardless of intent — chords like Cmd+K rendered as "Cmd K". Also extend the Esc- overlay-exemption check to cover the narrow-viewport Filters sheet (.filters-open on #header-page-content) and the sidebar's right- click context menu — pressing Esc to close those used to fall through to history.back(). 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 a251dfd commit 667268d

26 files changed

Lines changed: 2170 additions & 1015 deletions

gearbox/internal/framework/handler/handler.go

Lines changed: 156 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package handler
33
import (
44
"log/slog"
55
"net/http"
6+
"strings"
67
"sync"
78
"time"
89

@@ -206,23 +207,98 @@ func (h *Handler) getDefaultServerID() string {
206207
return ""
207208
}
208209

210+
// activeBoxCookieName is the cookie key that persists the user's selected
211+
// box across navigations. Lets gear links (e.g. /haproxy) drop the verbose
212+
// `?box_id=` query string and still resolve the active context.
213+
const activeBoxCookieName = "gearbox_active_box"
214+
215+
// acceptsHTML reports whether the client appears to be requesting an HTML
216+
// document (vs. an XHR/fetch JSON call). Used to scope the box_id-stripping
217+
// redirect to navigations only.
218+
func acceptsHTML(r *http.Request) bool {
219+
accept := r.Header.Get("Accept")
220+
if accept == "" {
221+
return false
222+
}
223+
// Cheap substring check is fine — quality factors don't change the answer
224+
// for our use case (a navigation always advertises text/html very near
225+
// the front of the Accept list).
226+
for _, want := range []string{"text/html", "application/xhtml+xml"} {
227+
if strings.Contains(accept, want) {
228+
return true
229+
}
230+
}
231+
return false
232+
}
233+
234+
// activeBoxCookieMaxAge is one year — long enough to feel persistent.
235+
// Cleared explicitly via clearActiveBoxCookie when the user picks "All boxes".
236+
const activeBoxCookieMaxAge = 60 * 60 * 24 * 365
237+
238+
// requestIsTLS reports whether the current request appears to be served
239+
// over HTTPS, either directly (r.TLS != nil) or via a TLS-terminating
240+
// proxy that forwards X-Forwarded-Proto=https (the HAProxy front-end in
241+
// this homelab does exactly that). Used to gate the Secure cookie
242+
// attribute so cookies are HTTPS-only in production but still work on
243+
// plain http://localhost during development.
244+
func requestIsTLS(r *http.Request) bool {
245+
if r.TLS != nil {
246+
return true
247+
}
248+
if strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") {
249+
return true
250+
}
251+
return false
252+
}
253+
254+
// setActiveBoxCookie writes the active-box id to an HttpOnly cookie scoped
255+
// to the whole site. SameSite=Lax so cross-site navigations (share-links,
256+
// bookmarks) still pick it up; HttpOnly so JS can't read it. Secure is
257+
// set whenever the request is itself TLS — addresses CodeQL
258+
// js/clear-text-cookie / go-cookie-not-secure.
259+
func setActiveBoxCookie(w http.ResponseWriter, r *http.Request, boxID string) {
260+
http.SetCookie(w, &http.Cookie{
261+
Name: activeBoxCookieName,
262+
Value: boxID,
263+
Path: "/",
264+
MaxAge: activeBoxCookieMaxAge,
265+
HttpOnly: true,
266+
Secure: requestIsTLS(r),
267+
SameSite: http.SameSiteLaxMode,
268+
})
269+
}
270+
271+
// clearActiveBoxCookie removes the persisted box selection. Triggered by
272+
// `?box_id=` (empty value) or by visiting /bx (the fleet picker) — both
273+
// signal the user wants the all-boxes context.
274+
func clearActiveBoxCookie(w http.ResponseWriter, r *http.Request) {
275+
http.SetCookie(w, &http.Cookie{
276+
Name: activeBoxCookieName,
277+
Value: "",
278+
Path: "/",
279+
MaxAge: -1,
280+
HttpOnly: true,
281+
Secure: requestIsTLS(r),
282+
SameSite: http.SameSiteLaxMode,
283+
})
284+
}
285+
209286
// InjectIntegrationStatus is middleware that adds integration status, the
210287
// active-box context, the enabled-box roster, and user permissions to the
211288
// request context. This is what drives the sidebar's scope-aware rendering
212289
// and the header's box-switcher chip.
213290
//
214-
// Active-box resolution:
215-
// - If `?box_id=<id>` is present in the URL and refers to an enabled box,
216-
// that box is the active context. The sidebar shows that box's enabled
217-
// gears (plus all ScopeBoxAgnostic / ScopeSystem gears).
218-
// - Otherwise the active context is empty ("box-agnostic"). The sidebar
219-
// hides ScopeBox gears (they require a selection) and shows only
220-
// ScopeBoxAgnostic + ScopeSystem entries.
291+
// Active-box resolution (in priority order):
292+
// 1. `?box_id=<id>` in the URL — explicit override. Also written to the
293+
// active-box cookie so subsequent navigations don't need the query
294+
// string. An empty `?box_id=` clears the cookie (used to deselect).
295+
// 2. The `gearbox_active_box` cookie — sticky preference from a prior
296+
// selection. Subject to the same enabled-box validation as the URL.
297+
// 3. None — "All boxes" / box-agnostic context. Sidebar hides ScopeBox
298+
// gears; the Bx fleet view becomes the entry point.
221299
//
222300
// System gears (keyed by database.SystemServerID) are loaded unconditionally
223-
// because they are install-wide. The legacy "fall back to the first enabled
224-
// box" behavior is gone — the Bx fleet view is now the user's entry point
225-
// when no box is explicitly selected.
301+
// because they are install-wide.
226302
func (h *Handler) InjectIntegrationStatus(next http.Handler) http.Handler {
227303
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
228304
ctx := r.Context()
@@ -243,16 +319,83 @@ func (h *Handler) InjectIntegrationStatus(next http.Handler) http.Handler {
243319

244320
enabled := h.getEnabledServers()
245321

246-
// Resolve the active box from ?box_id= (if any, enabled, and valid).
322+
// Resolve the active box: URL takes precedence, cookie is fallback.
323+
// `?box_id=` with an empty value is the explicit "deselect" signal —
324+
// we honor it by clearing the cookie and skipping cookie fallback.
247325
var activeBox *models.BoxConfig
248-
if requested := r.URL.Query().Get("box_id"); requested != "" {
326+
var requestedID string
327+
urlHasBoxID := r.URL.Query().Has("box_id")
328+
_, hasCookie := r.Cookie(activeBoxCookieName)
329+
hasCookieSet := hasCookie == nil
330+
if urlHasBoxID {
331+
requestedID = r.URL.Query().Get("box_id")
332+
} else if c, err := r.Cookie(activeBoxCookieName); err == nil {
333+
requestedID = c.Value
334+
}
335+
if requestedID != "" {
249336
for i := range enabled {
250-
if enabled[i].ID == requested {
337+
if enabled[i].ID == requestedID {
251338
activeBox = &enabled[i]
252339
break
253340
}
254341
}
255342
}
343+
// First-login fallback: if the request has no `?box_id=`, no
344+
// previously-set cookie, and the user is landing on a page that
345+
// benefits from a box context (i.e. not /bx, which means "show
346+
// all"), seed the active box from the first enabled entry. This
347+
// stops the sidebar from looking empty for users who haven't
348+
// explicitly picked a box yet — the most common cause of first-
349+
// login confusion.
350+
if activeBox == nil && !urlHasBoxID && !hasCookieSet && len(enabled) > 0 &&
351+
r.URL.Path != "/bx" && !strings.HasPrefix(r.URL.Path, "/bx/") {
352+
activeBox = &enabled[0]
353+
setActiveBoxCookie(w, r, activeBox.ID)
354+
}
355+
// Persist / clear the cookie based on what the URL signaled, then
356+
// redirect to the same path with `box_id` stripped so URLs stay clean.
357+
// Other query params are preserved (e.g. /logs?source=foo). Only
358+
// HTML document GETs get the redirect — XHR/fetch (`Accept` lacks
359+
// text/html) keep the param transparently so existing JS callers
360+
// that still pass `?box_id=` don't break.
361+
if urlHasBoxID && r.Method == http.MethodGet && acceptsHTML(r) {
362+
if activeBox != nil {
363+
setActiveBoxCookie(w, r, activeBox.ID)
364+
} else {
365+
clearActiveBoxCookie(w, r)
366+
}
367+
q := r.URL.Query()
368+
q.Del("box_id")
369+
redir := r.URL.Path
370+
if encoded := q.Encode(); encoded != "" {
371+
redir += "?" + encoded
372+
}
373+
http.Redirect(w, r, redir, http.StatusSeeOther)
374+
return
375+
}
376+
// Cookie still needs writing for non-HTML callers that explicitly
377+
// passed `?box_id=` (e.g. an early SPA-style call) so the next
378+
// document GET doesn't have to re-resolve.
379+
if urlHasBoxID {
380+
if activeBox != nil {
381+
setActiveBoxCookie(w, r, activeBox.ID)
382+
} else {
383+
clearActiveBoxCookie(w, r)
384+
}
385+
}
386+
if r.URL.Path == "/bx" || r.URL.Path == "/bx/" {
387+
// /bx is the all-boxes view — clear any sticky selection so
388+
// the chip reads "All boxes" and the sidebar hides box-scoped
389+
// gears. Clear whenever the cookie is present, not just when
390+
// it resolved to an enabled box: otherwise a stale cookie
391+
// (referencing a deleted/disabled box) survives indefinitely
392+
// AND blocks the first-login auto-select branch below
393+
// because hasCookieSet stays true.
394+
if hasCookieSet {
395+
clearActiveBoxCookie(w, r)
396+
}
397+
activeBox = nil
398+
}
256399
if activeBox != nil {
257400
ctx = auth.SetSelectedBox(ctx, activeBox)
258401
}

0 commit comments

Comments
 (0)