Skip to content

Commit 01f1988

Browse files
sarg3ntclaude
andcommitted
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>
1 parent 3b827fb commit 01f1988

3 files changed

Lines changed: 24 additions & 4 deletions

File tree

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ import (
88
)
99

1010
// FirewallConfigPage renders the firewall configuration editor page.
11+
//
12+
// Precondition: `config` must be non-nil. The HTTP handler routes the nil /
13+
// fetch-error case to FirewallConfigPageWithError, so by the time this
14+
// template is invoked the agent payload is known to exist. The hidden
15+
// input + textarea elements below dereference `config` unconditionally on
16+
// that basis; the optional `if config != nil` guard around the SHA /
17+
// timestamp header block is purely cosmetic (keeps the metadata strip
18+
// from rendering blanks if a future caller ever violates the contract).
1119
templ FirewallConfigPage(user *models.User, server *database.BoxDB, config *agent.FirewallConfigResponse, gitConfig *database.BoxGitConfig, changes []database.ConfigChange, canEdit bool) {
1220
@layouts.Base("Firewall Configuration", user, "/config/firewall") {
1321
<!-- Action toolbar hoisted to the global page header (right of the
@@ -19,7 +27,7 @@ templ FirewallConfigPage(user *models.User, server *database.BoxDB, config *agen
1927
if config != nil {
2028
<div class="hidden xl:flex items-center gap-3 text-xs text-gray-500 dark:text-gray-400 mr-2">
2129
<span title="File last modified">{ config.LastModified.Format("2006-01-02 15:04:05") }</span>
22-
<span class="font-mono">SHA { config.SHA256[:12] }</span>
30+
<span class="font-mono">SHA { shortSHA(config.SHA256) }</span>
2331
</div>
2432
}
2533
if canEdit {

gearbox/internal/framework/templates/pages/helpers.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ func canEditValue(canEdit bool) string {
2424

2525
// sectionsJSON serializes the parsed nftables section list (tables + chains
2626
// with line numbers) so the editor JS can render the left-rail nav without
27-
// re-parsing the config client-side. Embedded into a `<script type="application/json">`
28-
// tag, so templ HTML-escapes it safely.
27+
// re-parsing the config client-side. Embedded into a hidden `<textarea>` (NOT
28+
// a `<script type="application/json">`) so templ HTML-escapes the body the
29+
// same way it does for any other interpolated text node — the JS reads
30+
// `.value` off the textarea and `JSON.parse`s it.
2931
func sectionsJSON(sections []agent.ConfigSection) string {
3032
if len(sections) == 0 {
3133
return "[]"
@@ -37,6 +39,17 @@ func sectionsJSON(sections []agent.ConfigSection) string {
3739
return string(b)
3840
}
3941

42+
// shortSHA truncates a hex digest to the first 12 characters for compact
43+
// display in the page header. Safe on short / empty input — returns the
44+
// original string unchanged if it's already shorter than the target.
45+
func shortSHA(s string) string {
46+
const n = 12
47+
if len(s) <= n {
48+
return s
49+
}
50+
return s[:n]
51+
}
52+
4053
// backendBelongsToFrontend checks if a backend is associated with a frontend via metadata.
4154
func backendBelongsToFrontend(backendName, frontendName string, metadata *models.Metadata) bool {
4255
if metadata == nil {

gearbox/static/js/firewall_config/editor.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@
265265
function makeNavButton(section, kind) {
266266
const btn = document.createElement('button');
267267
btn.type = 'button';
268-
btn.dataset.line = String(section.start_line || 1);
269268
btn.className =
270269
'w-full text-left px-2 py-1 rounded text-gray-300 hover:bg-gray-800 hover:text-white flex items-center gap-2';
271270
const dot = document.createElement('span');

0 commit comments

Comments
 (0)