From aea46b69d19026cc87f84617c2c2a8216c4465d2 Mon Sep 17 00:00:00 2001 From: Divyam Talwar Date: Tue, 22 Sep 2026 04:49:15 +0530 Subject: [PATCH] fix: retain action safety warnings in HTML reports The HTML report displayed remediation but omitted the structured blocking caveats carried by other output formats. Render each guard's action, explanation and verification condition, making prohibitions explicit and preserving HTML escaping. Tests cover a real renderer regression, multiple guards, prohibitions, suppressed findings, untrusted display characters and absent safety metadata. The regression fails before the fix; report, render and CLI race tests pass. No execution or JSON contract changes are introduced. Closes #68 --- internal/report/guard_controls_test.go | 51 ++++++++++++++++++++++++++ internal/report/html.go | 15 +++++++- internal/report/html_test.go | 31 ++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 internal/report/guard_controls_test.go diff --git a/internal/report/guard_controls_test.go b/internal/report/guard_controls_test.go new file mode 100644 index 0000000..40156a6 --- /dev/null +++ b/internal/report/guard_controls_test.go @@ -0,0 +1,51 @@ +package report + +import ( + "html" + "strings" + "testing" + + "github.com/pgrundev/pgbot/internal/model" +) + +func TestReportGuardsPreserveProhibitionsAndEscaping(t *testing.T) { + verify := `confirm & "owner"` + text := `guard marker & evidence` + for _, suppressed := range []bool{false, true} { + c := &model.Context{Findings: []model.Finding{{ + ID: "synthetic_guard", Severity: model.SeverityWarn, Title: "guard fixture", + Suppressed: suppressed, + Safety: &model.Safety{BlockingCaveats: []model.SafetyGuard{ + {ID: "precondition", Kind: model.GuardPrecondition, Action: model.ActionDropIndex, Text: text, Verify: &verify}, + {ID: "prohibition", Kind: model.GuardProhibition, Action: model.ActionVacuumFull, Text: "preserve corruption evidence"}, + }}, + }}} + out := Render(c, 50, "test") + for _, want := range []string{"Caution before DROP INDEX", "Do not run VACUUM FULL", html.EscapeString(text), html.EscapeString(verify), "preserve corruption evidence"} { + if !strings.Contains(out, want) { + t.Errorf("suppressed=%v: missing %q", suppressed, want) + } + } + if strings.Contains(out, text) || strings.Contains(out, verify) { + t.Fatal("guard content was not HTML-escaped") + } + if strings.Count(out, `class="guard"`) != 2 { + t.Fatal("every blocking caveat must be rendered once") + } + } +} + +func TestReportWithoutSafetyDoesNotInventGuards(t *testing.T) { + for _, safety := range []*model.Safety{nil, {}} { + c := &model.Context{Findings: []model.Finding{{ + ID: "fixture", Severity: model.SeverityInfo, Title: "ordinary finding", Safety: safety, + }}} + out := Render(c, 100, "test") + if strings.Contains(out, `class="guard"`) { + t.Fatal("unexpected guard for an unguarded finding") + } + if !strings.Contains(out, "ordinary finding") { + t.Fatal("ordinary finding disappeared") + } + } +} diff --git a/internal/report/html.go b/internal/report/html.go index 5b62e38..21d95d2 100644 --- a/internal/report/html.go +++ b/internal/report/html.go @@ -91,6 +91,19 @@ func Render(c *model.Context, score int, version string) string { if f.Remediation != "" { body.w(`

Fix: %s

`, esc(f.Remediation)) } + if f.Safety != nil { + for _, guard := range f.Safety.BlockingCaveats { + label := "Caution before" + if guard.Kind == model.GuardProhibition { + label = "Do not run" + } + body.w(`

%s %s: %s`, label, esc(guard.Action), esc(guard.Text)) + if guard.Verify != nil { + body.w(` Only after: %s`, esc(*guard.Verify)) + } + body.raw(`

`) + } + } for _, cv := range f.Caveats { body.w(`

but: %s

`, esc(cv)) } @@ -253,7 +266,7 @@ func Render(c *model.Context, score int, version string) string { .finding.info .sev{color:var(--muted)} .finding.suppressed{opacity:.55} .fbody p{margin:.3rem 0;color:var(--muted)} .fbody b{color:var(--fg)} .evi{color:var(--muted);font-size:.82rem;padding-left:.8rem} - .fix{color:var(--green)} .caveat{color:var(--orange);font-size:.82rem} + .fix{color:var(--green)} .guard,.caveat{color:var(--orange);font-size:.82rem} footer{padding:1rem 2rem;color:var(--dim);border-top:1px solid var(--line);font-size:.8rem}
` + fmt.Sprintf("%d", score) + `/100 diff --git a/internal/report/html_test.go b/internal/report/html_test.go index a6b25d1..7af686a 100644 --- a/internal/report/html_test.go +++ b/internal/report/html_test.go @@ -86,3 +86,34 @@ func TestRenderReportSparse(t *testing.T) { t.Error("header must still carry the database") } } + +func TestRenderReportCarriesDestructiveSafetyGuards(t *testing.T) { + verify := "confirm the index is unused on every replica" + c := &model.Context{ + Server: model.ServerInfo{Database: "shop"}, + Findings: []model.Finding{{ + ID: "unused_indexes", + Severity: model.SeverityWarn, + Title: "an index appears unused", + Remediation: "DROP INDEX CONCURRENTLY after verification", + Safety: &model.Safety{BlockingCaveats: []model.SafetyGuard{{ + ID: "unused_index.per_node", + Kind: model.GuardPrecondition, + Action: model.ActionDropIndex, + Text: "zero scans on this node do not prove cluster-wide disuse", + Verify: &verify, + }}}, + }}, + } + + out := Render(c, 80, "test") + for _, want := range []string{ + model.ActionDropIndex, + "zero scans on this node do not prove cluster-wide disuse", + verify, + } { + if !strings.Contains(out, want) { + t.Errorf("HTML report dropped destructive-action guard %q", want) + } + } +}