fix(tui): watcher pane rendering — approvals slot, markers, glamour

.gitignore

@@ -16,3 +16,11 @@ coverage.out
 
 # Claude Code local settings (personal permissions, not committed)
 .claude/settings.local.json
+
+# Sensitive files
+.env
+.env.*
+*.pem
+*.key
+credentials.*
+secrets.*

docs/plans/420-watcher-pane-rendering.md

@@ -0,0 +1,98 @@
+# 420 — Watcher Pane Rendering Fixes
+
+## Problem
+
+Three rendering defects in the watcher/approvals panes:
+
+1. **F1 — Approvals render below help bar**: expanded approvals appear
+   below the bottom chrome, unwrapped, and never show `ask.Body`. Users
+   can't review what they're approving.
+2. **F2 — Markers repeated on every line**: `prefixLines` prepends the
+   📡/🤖 marker to every line of a `---` block. Multi-line verdicts and
+   streaming agent output get cluttered with duplicate emojis.
+3. **F3 — Verdict markdown rendered raw**: watcher pane shows raw
+   markdown syntax (`**bold**`, `` `code` ``) instead of rendered text.
+
+## Approach
+
+### F1 — Approvals in watcher pane slot
+
+Expanded approvals render into the watcher pane slot (views.go ~121),
+temporarily replacing the watcher viewport. `RenderExpanded` shows
+wrapped titles, action+target lines using snapshotted
+`ask.IncidentID`/`IncidentTitle` (never live `m.selectedIncident`),
+and full `ask.Body`. Watcher expanded state is saved on open and
+restored on close via `watcherWasExpanded`.
+
+### F2 — One marker per block
+
+Introduced `prefixMessage(marker, text)` which prepends the marker once
+at the start of the block, replacing `prefixLines` at all call sites.
+Both watcher and agent paths fixed identically.
+
+### F3 — Glamour markdown rendering
+
+Added `renderWatcherMarkdown` method that runs content through glamour
+with the model's `GlamourStyle`. ASCII-mode guard falls back to plain
+lipgloss wrapping in deterministic test environments. `stripControl`
+security sanitisation is preserved at the buffer-append boundary.
+
+## Key decisions
+
+- **Security**: approval rendering uses snapshotted incident data from
+  `Ask` struct, never reads live `m.selectedIncident`
+- **ASCII guard**: `renderWatcherMarkdown` checks
+  `lipgloss.ColorProfile() == termenv.Ascii` to prevent ANSI
+  contamination in golden snapshot tests
+- **No `prefixLines` removal**: kept as utility (still tested); all
+  production call sites use `prefixMessage`
+
+## Deliverables
+
+| ID | Summary | Files | Commit |
+|----|---------|-------|--------|
+| T1 | Failing tests for F1, F2, F3 | `watcher_rendering_test.go` | `88c8dc8` |
+| F2 | One marker per `---` block | `watcher.go`, `tui.go`, `claude.go`, `claude_test.go`, `view_render_test.go`, `watcher_integration_test.go` | `05f5259` |
+| F3 | Glamour markdown rendering | `watcher.go` | `85b7d67` |
+| F1 | Approvals in watcher pane slot | `views.go`, `approvals.go`, `model.go`, `msgHandlers.go` | `02aa812` |
+| G  | Golden snapshots + ASCII guard + lint fix | `golden_test.go`, `watcher.go`, `watcher_rendering_test.go`, `approvals.go`, `model.go`, `testdata/*.golden` | `c4caa08` |
+
+## Test traceability
+
+| Test | File:Line | Covers |
+|------|-----------|--------|
+| `TestPrefixMessage_OneMarkerPerBlock` (6 subtests) | `watcher_rendering_test.go:16` | F2 |
+| `TestWatcherBuffer_OneMarkerPerBlock_Integration` | `watcher_rendering_test.go:77` | F2 |
+| `TestWatcherBuffer_StreamingSetLast_OneMarker` | `watcher_rendering_test.go:98` | F2 |
+| `TestView_ApprovalsExpandedRendersInWatcherSlot` | `watcher_rendering_test.go:117` | F1 |
+| `TestView_ApprovalsExpandedWrapsLongTitles` | `watcher_rendering_test.go:144` | F1 |
+| `TestView_ApprovalsCollapsedShowsBadge` | `watcher_rendering_test.go:169` | F1 |
+| `TestView_ApprovalsRestoredWatcherStateOnClose` | `watcher_rendering_test.go:182` | F1 |
+| `TestUpdateWatcherViewport_RendersMarkdown` | `watcher_rendering_test.go:200` | F3 |
+| `TestRenderApprovalsExpanded_ShowsBody` | `watcher_rendering_test.go:220` | F1 |
+| `TestRenderApprovalsExpanded_ActionTargetLine` | `watcher_rendering_test.go:238` | F1 |
+| `TestGolden_WatcherOneMarkerAgent` | `golden_test.go:193` | F2 golden |
+| `TestGolden_WatcherOneMarkerWatcher` | `golden_test.go:203` | F2/F3 golden |
+| `TestGolden_ApprovalsExpanded` | `golden_test.go:213` | F1 golden |
+
+## Revert-check evidence
+
+**F2** — reverted `prefixMessage` to delegate to `prefixLines`: 6 test
+failures (marker count 3 instead of 1). Restored; all pass.
+
+**F3** — reverted `renderWatcherMarkdown` to plain lipgloss wrapping:
+build failure from unused glamour/termenv imports (function body removed).
+Restored; all pass.
+
+**F1** — reverted `views.go` approvals branch: 3 assertion failures
+(header, title, body missing from view output). Restored; all pass.
+
+## CI results
+
+- `gofmt -s -l`: clean
+- `go vet ./...`: clean
+- `golangci-lint run`: 0 issues
+- `go test ./pkg/tui/... -count=1`: PASS (47.8s)
+- `go test -race ./pkg/tui/... -count=1`: PASS (96.0s)
+- `cmd` package: pre-existing env-specific failure (`TestConfigureLogging_SetsLogWriter`
+  — `/var/log/srepd.log` read-only) unrelated to this PR

pkg/ai/http_safety_test.go

@@ -0,0 +1,66 @@
+package ai
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewOpenAICompatProvider_RejectsHTTPWithAPIKey(t *testing.T) {
+	_, err := newOpenAICompatProvider(Config{
+		Endpoint: "http://remote-server.example.com:8080",
+		Model:    "gpt-4",
+	}, "sk-secret-key")
+
+	require.Error(t, err,
+		"must reject non-localhost http:// endpoint when API key is set")
+	assert.Contains(t, err.Error(), "HTTPS",
+		"error message should mention HTTPS requirement")
+}
+
+func TestNewOpenAICompatProvider_AllowsHTTPLocalhost(t *testing.T) {
+	tests := []struct {
+		name     string
+		endpoint string
+	}{
+		{"localhost", "http://localhost:8080"},
+		{"127.0.0.1", "http://127.0.0.1:11434"},
+		{"[::1]", "http://[::1]:8080"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			provider, err := newOpenAICompatProvider(Config{
+				Endpoint: tt.endpoint,
+				Model:    "m",
+			}, "sk-secret-key")
+
+			assert.NoError(t, err,
+				"localhost http:// with API key should be allowed")
+			assert.NotNil(t, provider)
+		})
+	}
+}
+
+func TestNewOpenAICompatProvider_AllowsHTTPSWithAPIKey(t *testing.T) {
+	provider, err := newOpenAICompatProvider(Config{
+		Endpoint: "https://api.openai.com",
+		Model:    "gpt-4",
+	}, "sk-secret-key")
+
+	assert.NoError(t, err,
+		"https:// endpoint with API key should be allowed")
+	assert.NotNil(t, provider)
+}
+
+func TestNewOpenAICompatProvider_AllowsHTTPWithoutAPIKey(t *testing.T) {
+	provider, err := newOpenAICompatProvider(Config{
+		Endpoint: "http://remote-server.example.com:8080",
+		Model:    "llama",
+	}, "")
+
+	assert.NoError(t, err,
+		"http:// without API key should be allowed (e.g., ollama)")
+	assert.NotNil(t, provider)
+}

pkg/ai/openai_compat.go

@@ -7,12 +7,27 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
 	"github.com/charmbracelet/log"
 )
 
+func validateEndpointSecurity(endpoint string) error {
+	parsed, err := url.Parse(endpoint)
+	if err != nil {
+		return fmt.Errorf("openai: invalid endpoint URL: %w", err)
+	}
+	if parsed.Scheme == "http" {
+		host := parsed.Hostname()
+		if host != "localhost" && host != "127.0.0.1" && host != "::1" {
+			return fmt.Errorf("openai: refusing to send API key over HTTP to non-localhost host %q; use HTTPS for remote endpoints", host)
+		}
+	}
+	return nil
+}
+
 type openaiCompatProvider struct {
 	endpoint       string
 	model          string
@@ -26,6 +41,12 @@ func newOpenAICompatProvider(cfg Config, apiKey string) (*openaiCompatProvider,
 		return nil, fmt.Errorf("openai: endpoint is required")
 	}
 
+	if apiKey != "" {
+		if err := validateEndpointSecurity(cfg.Endpoint); err != nil {
+			return nil, err
+		}
+	}
+
 	return &openaiCompatProvider{
 		endpoint:       strings.TrimRight(cfg.Endpoint, "/"),
 		model:          cfg.Model,

pkg/backplane/client.go

@@ -60,7 +60,7 @@ func NewClient(cfg *Config, tokenFunc func() (string, error)) BackplaneClient {
 }
 
 func (c *Client) ListReports(ctx context.Context, clusterID string) ([]ReportSummary, error) {
-	endpoint := fmt.Sprintf("%s/backplane/cluster/%s/reports?last=10", c.config.URL, clusterID)
+	endpoint := fmt.Sprintf("%s/backplane/cluster/%s/reports?last=10", c.config.URL, url.PathEscape(clusterID))
 	log.Debug("backplane.ListReports", "cluster_id", clusterID)
 
 	body, err := c.doRequest(ctx, endpoint)
@@ -80,7 +80,7 @@ func (c *Client) ListReports(ctx context.Context, clusterID string) ([]ReportSum
 }
 
 func (c *Client) GetReport(ctx context.Context, clusterID, reportID string) (*Report, error) {
-	endpoint := fmt.Sprintf("%s/backplane/cluster/%s/reports/%s", c.config.URL, clusterID, reportID)
+	endpoint := fmt.Sprintf("%s/backplane/cluster/%s/reports/%s", c.config.URL, url.PathEscape(clusterID), url.PathEscape(reportID))
 	log.Debug("backplane.GetReport", "cluster_id", clusterID, "report_id", reportID)
 
 	body, err := c.doRequest(ctx, endpoint)

pkg/backplane/path_escape_test.go

@@ -0,0 +1,69 @@
+package backplane
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestClient_ListReports_PathEscapesClusterID(t *testing.T) {
+	var receivedPath string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		receivedPath = r.URL.EscapedPath()
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(listReportsResponse{Reports: []ReportSummary{}})
+	}))
+	defer server.Close()
+
+	cfg := &Config{URL: server.URL}
+	client := NewClient(cfg, func() (string, error) { return "test-token", nil })
+
+	_, err := client.ListReports(context.Background(), "../../admin/endpoint")
+	require.NoError(t, err)
+
+	assert.Equal(t, "/backplane/cluster/..%2F..%2Fadmin%2Fendpoint/reports", receivedPath,
+		"clusterID with path traversal must be URL-escaped in the request path")
+}
+
+func TestClient_GetReport_PathEscapesClusterIDAndReportID(t *testing.T) {
+	var receivedPath string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		receivedPath = r.URL.EscapedPath()
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(Report{ReportID: "rpt-1"})
+	}))
+	defer server.Close()
+
+	cfg := &Config{URL: server.URL}
+	client := NewClient(cfg, func() (string, error) { return "test-token", nil })
+
+	_, err := client.GetReport(context.Background(), "../admin", "../../secret")
+	require.NoError(t, err)
+
+	assert.Equal(t, "/backplane/cluster/..%2Fadmin/reports/..%2F..%2Fsecret", receivedPath,
+		"both clusterID and reportID with path traversal must be URL-escaped")
+}
+
+func TestClient_ListReports_NormalClusterIDUnchanged(t *testing.T) {
+	var receivedPath string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		receivedPath = r.URL.Path
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(listReportsResponse{Reports: []ReportSummary{}})
+	}))
+	defer server.Close()
+
+	cfg := &Config{URL: server.URL}
+	client := NewClient(cfg, func() (string, error) { return "test-token", nil })
+
+	_, err := client.ListReports(context.Background(), "abc-123-def")
+	require.NoError(t, err)
+
+	assert.Equal(t, "/backplane/cluster/abc-123-def/reports", receivedPath,
+		"normal clusterID should pass through unchanged")
+}

pkg/tui/approvals.go

@@ -118,32 +118,68 @@ func (a *approvalsStrip) RenderExpanded(width int) string {
 		return ""
 	}
 
+	wrapStyle := lipgloss.NewStyle().Width(width)
+
 	var lines []string
 	header := lipgloss.NewStyle().
 		Bold(true).
 		Underline(true).
 		Width(width).
 		Render(" Pending Approvals ")
 	lines = append(lines, header)
+	lines = append(lines, "")
 
 	for i, ask := range a.asks {
 		prefix := "  "
 		if i == a.selected {
 			prefix = "> "
 		}
 		kindLabel := askKindLabel(ask.Kind)
-		line := fmt.Sprintf("%s[%s] %s", prefix, kindLabel, ask.Title)
-		lines = append(lines, line)
+
+		titleLine := fmt.Sprintf("%s[%s] %s", prefix, kindLabel, ask.Title)
+		lines = append(lines, wrapStyle.Render(titleLine))
+
+		// Action target line: what this approval will do and to which incident
+		var target string
+		switch ask.Kind {
+		case AskDraftNote:
+			target = fmt.Sprintf("    Post note to incident %s", ask.IncidentID)
+		case AskSuggestedCommand:
+			target = "    Copy command to clipboard"
+		case AskEscalationSuggestion:
+			target = fmt.Sprintf("    Re-escalate incident %s", ask.IncidentID)
+		default:
+			target = fmt.Sprintf("    Action on incident %s", ask.IncidentID)
+		}
+		if ask.IncidentTitle != "" {
+			target += fmt.Sprintf(" (%s)", ask.IncidentTitle)
+		}
+		lines = append(lines, wrapStyle.Render(target))
+
+		// Body: the content the user is approving
+		if ask.Body != "" {
+			lines = append(lines, "")
+			bodyLines := strings.Split(ask.Body, "\n")
+			for _, bl := range bodyLines {
+				lines = append(lines, wrapStyle.Render("    "+bl))
+			}
+		}
+
+		if i < len(a.asks)-1 {
+			lines = append(lines, "")
+		}
 	}
 
+	lines = append(lines, "")
 	footer := " [Enter] Accept  [d] Dismiss  [Esc] Close "
 	lines = append(lines, footer)
 
-	result := ""
+	var result strings.Builder
 	for _, l := range lines {
-		result += l + "\n"
+		result.WriteString(l)
+		result.WriteString("\n")
 	}
-	return result
+	return result.String()
 }
 
 // inferAskKind determines the AskKind from a verdict's action text.