Security hardening: HTTP, permissions, XSS, error sanitization#8
Security hardening: HTTP, permissions, XSS, error sanitization#8
Conversation
…tion
Full codebase security audit with fixes across 12 files:
- Add Host header validation middleware (blocks DNS rebinding)
- Add security headers (X-Content-Type-Options, X-Frame-Options, Referrer-Policy)
- Add request body size limits (1MB via MaxBytesReader middleware)
- Add HTTP server timeouts (Read: 10s, Write: 30s, Idle: 120s)
- Tighten file permissions to 0700/0600 for dirs/files in ~/.continuity/
- Auto-harden permissions on existing installs at startup
- Fix XSS in ProfilePanel: replace {@html} with safe Svelte text interpolation
- Fix {@html} for static icons in Header component
- Sanitize all error responses: generic messages to clients, full errors to server logs
- Add jsonError() helper to prevent JSON injection via string concatenation
- Cap search limit parameter at 100
- Add io.LimitReader (10MB) on hook stdin parsing
- Apply 10KB truncation to tool_input (matching tool_response)
- Log truncation events with session ID and byte counts
- Remove db_path from /api/health response
- Remove middleware.RealIP (unnecessary, trusts spoofable headers)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Security hardening across the HTTP API server, hooks, storage layer, and UI to reduce attack surface (DNS rebinding, XSS), limit resource usage, and avoid leaking sensitive details.
Changes:
- Added HTTP middleware for localhost-only access, security headers, and request body limits; added server-side timeouts and header size cap.
- Hardened filesystem permissions for continuity-managed directories/files and added startup tightening for existing installs.
- Replaced Svelte
{@html}rendering with safe text interpolation; sanitized API error responses via a JSON helper and removeddb_pathfrom health output.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/components/ProfilePanel.svelte | Removes {@html} rendering of profile text and renders parsed lines safely. |
| ui/src/components/Header.svelte | Removes {@html} usage for tab icons by switching to Unicode characters. |
| internal/store/observations.go | Truncates large observation fields (now including tool_input) and logs truncation events. |
| internal/store/db.go | Creates DB directories with 0700 and tightens permissions on existing DB/WAL/SHM files. |
| internal/server/server_test.go | Adds a test helper to set Host: localhost to satisfy localhost-only middleware. |
| internal/server/server.go | Installs new middleware (localhost-only, security headers, request size limit) and removes RealIP. |
| internal/server/routes_test.go | Updates tests to use the new request helper so Host validation doesn’t reject them. |
| internal/server/routes.go | Adds jsonError() and sanitizes several internal error responses; caps search limit; removes db_path exposure. |
| internal/server/middleware.go | Introduces localhost-only host validation, security headers, and request body limiting middleware. |
| internal/hooks/handler.go | Caps hook stdin decoding to 10MB to prevent unbounded reads. |
| internal/hooks/autostart.go | Creates autostart logs with 0600 and tightens permissions for existing log files. |
| internal/cli/serve.go | Adds HTTP server timeouts and MaxHeaderBytes to harden the server. |
| internal/cli/init.go | Writes continuity autostart artifacts with 0700/0600 permissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| r.Use(localhostOnly) | ||
| r.Use(securityHeaders) |
There was a problem hiding this comment.
Middleware ordering: localhostOnly runs before securityHeaders, so 403 responses generated by localhostOnly won't include the security headers. If the intent is truly "all responses", register securityHeaders earlier in the chain (before localhostOnly) so even rejected requests get the headers.
| r.Use(localhostOnly) | |
| r.Use(securityHeaders) | |
| r.Use(securityHeaders) | |
| r.Use(localhostOnly) |
| "net" | ||
| "net/http" | ||
| ) | ||
|
|
||
| const maxRequestBody = 1 << 20 // 1MB | ||
|
|
||
| // localhostOnly rejects requests where the Host header is not localhost. | ||
| // Prevents DNS rebinding attacks against the local API server. | ||
| func localhostOnly(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| host := r.Host | ||
| if h, _, err := net.SplitHostPort(host); err == nil { | ||
| host = h | ||
| } | ||
| if host != "localhost" && host != "127.0.0.1" && host != "::1" { | ||
| http.Error(w, "Forbidden", http.StatusForbidden) |
There was a problem hiding this comment.
localhostOnly has a few correctness issues in Host parsing/validation: (1) hostname comparisons are case-sensitive even though Host is case-insensitive, so Host: Localhost would be incorrectly rejected; (2) IPv6 literals without a port commonly appear bracketed as Host: [::1], but net.SplitHostPort fails on that (missing port) and the code then compares against "::1", causing valid loopback requests to be rejected; (3) it returns a plain-text http.Error, which is inconsistent with the JSON API responses. Consider normalizing the host (lowercase, unbracket IPv6, optional trailing-dot trim) and returning a JSON error (e.g., via jsonError).
| "net" | |
| "net/http" | |
| ) | |
| const maxRequestBody = 1 << 20 // 1MB | |
| // localhostOnly rejects requests where the Host header is not localhost. | |
| // Prevents DNS rebinding attacks against the local API server. | |
| func localhostOnly(next http.Handler) http.Handler { | |
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| host := r.Host | |
| if h, _, err := net.SplitHostPort(host); err == nil { | |
| host = h | |
| } | |
| if host != "localhost" && host != "127.0.0.1" && host != "::1" { | |
| http.Error(w, "Forbidden", http.StatusForbidden) | |
| "encoding/json" | |
| "net" | |
| "net/http" | |
| "strings" | |
| ) | |
| const maxRequestBody = 1 << 20 // 1MB | |
| func normalizeHost(host string) string { | |
| if h, _, err := net.SplitHostPort(host); err == nil { | |
| host = h | |
| } else if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { | |
| host = strings.TrimPrefix(strings.TrimSuffix(host, "]"), "[") | |
| } | |
| host = strings.ToLower(host) | |
| host = strings.TrimSuffix(host, ".") | |
| return host | |
| } | |
| func writeJSONError(w http.ResponseWriter, status int, message string) { | |
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(status) | |
| _ = json.NewEncoder(w).Encode(map[string]string{"error": message}) | |
| } | |
| // localhostOnly rejects requests where the Host header is not localhost. | |
| // Prevents DNS rebinding attacks against the local API server. | |
| func localhostOnly(next http.Handler) http.Handler { | |
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| host := normalizeHost(r.Host) | |
| if host != "localhost" && host != "127.0.0.1" && host != "::1" { | |
| writeJSONError(w, http.StatusForbidden, "Forbidden") |
| // jsonError writes a JSON error response. All error responses should use this | ||
| // to avoid JSON injection via string concatenation. | ||
| func jsonError(w http.ResponseWriter, msg string, code int) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(code) | ||
| json.NewEncoder(w).Encode(map[string]string{"error": msg}) | ||
| } |
There was a problem hiding this comment.
The comment says "All error responses should use" jsonError(), but several handlers in this file still use http.Error with JSON-looking strings (which results in Content-Type: text/plain and an extra trailing newline). Either update those remaining error paths to call jsonError, or soften the comment so it matches actual behavior and avoids misleading future changes.
| // AddObservation stores a tool use observation. Truncates large fields to prevent DB bloat. | ||
| func (db *DB) AddObservation(sessionID, toolName, toolInput, toolResponse string) error { | ||
| if len(toolInput) > maxToolResponseSize { | ||
| log.Printf("observation: tool_input truncated for session %s: %d → %d bytes", sessionID, len(toolInput), maxToolResponseSize) | ||
| toolInput = toolInput[:maxToolResponseSize] |
There was a problem hiding this comment.
AddObservation now truncates both tool_input and tool_response, but the constant name/comment (maxToolResponseSize) still reads as response-only. To avoid confusion, consider renaming the constant (and updating its doc) to reflect that it applies to both fields (e.g., maxToolFieldSize).
| if len(toolInput) > maxToolResponseSize { | ||
| log.Printf("observation: tool_input truncated for session %s: %d → %d bytes", sessionID, len(toolInput), maxToolResponseSize) | ||
| toolInput = toolInput[:maxToolResponseSize] | ||
| } |
There was a problem hiding this comment.
New behavior: tool_input is now truncated to the same limit as tool_response, but observations_test.go only asserts truncation for tool_response. Adding a test assertion for tool_input truncation would help prevent regressions in this bound.
| if err := os.Chmod(dir, 0700); err == nil { | ||
| fmt.Fprintf(os.Stderr, " security: tightened %s to 0700\n", dir) | ||
| } | ||
| } | ||
| for _, f := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} { | ||
| if info, err := os.Stat(f); err == nil && info.Mode().Perm()&0077 != 0 { | ||
| if err := os.Chmod(f, 0600); err == nil { | ||
| fmt.Fprintf(os.Stderr, " security: tightened %s to 0600\n", f) | ||
| } |
There was a problem hiding this comment.
hardenPermissions writes directly to os.Stderr from within store.Open(). Since store.Open() is a library entry point (used by CLI and potentially other consumers), emitting unsolicited stderr output can pollute program output and complicate embedding/testing. Consider routing this through the existing logging approach, returning a status to the caller, or gating the message behind a verbose/debug flag.
| if err := os.Chmod(dir, 0700); err == nil { | |
| fmt.Fprintf(os.Stderr, " security: tightened %s to 0700\n", dir) | |
| } | |
| } | |
| for _, f := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} { | |
| if info, err := os.Stat(f); err == nil && info.Mode().Perm()&0077 != 0 { | |
| if err := os.Chmod(f, 0600); err == nil { | |
| fmt.Fprintf(os.Stderr, " security: tightened %s to 0600\n", f) | |
| } | |
| _ = os.Chmod(dir, 0700) | |
| } | |
| for _, f := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} { | |
| if info, err := os.Stat(f); err == nil && info.Mode().Perm()&0077 != 0 { | |
| _ = os.Chmod(f, 0600) |
- Fix middleware ordering: securityHeaders before localhostOnly so 403s get headers - Normalize Host header: case-insensitive, handle bracketed IPv6 [::1], trim trailing dots - Return JSON from localhostOnly rejection (was plaintext) - Migrate all remaining http.Error calls to jsonError for consistent JSON responses - Rename maxToolResponseSize → maxToolFieldSize (now covers both fields) - Add tool_input truncation assertion to TestAddObservationTruncation - Remove stderr output from hardenPermissions (library code, silent chmod) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Full codebase security audit and remediation across 13 files.
{@html}in ProfilePanel with safe Svelte text interpolationjsonError()helper — eliminates JSON injection via string concatenationmiddleware.RealIP: Unnecessary for local service, trusts spoofable X-Forwarded-Fordb_pathfrom/api/health: No reason to advertise filesystem layoutTest plan
go test ./...all packages greengo build ./...clean~/.continuity/permissions are tightened on first startup after upgrade🤖 Generated with Claude Code