fix: v0.4.1 — security hardening, goroutine leak, fail-closed fixes (#68)

* docs: promote go install as recommended, demote homebrew tap

* fix: data races, bulk-resolve action validation, hook cancel/deny paths

Data races (internal/approval/store.go):
- Get() and List() now return snapshots (value copies) instead of raw
  *Request pointers into the live map. Callers previously read fields
  like Status, ResolvedAt, ResolvedBy without holding the lock while
  watchExpiry wrote them concurrently — torn reads, potential panics.
  The -race detector now passes cleanly.

bulk-resolve action validation (internal/proxy/server.go):
- action="" or any non-deny/approve value previously defaulted to
  approve, silently bulk-approving entire runs on typos or empty bodies.
  Now returns 400 for any value outside {"approve", "deny"}.
- AutoApproveRun now called BEFORE the resolve loop (fixes TOCTOU):
  approvals created between List() and the end of the loop are now
  caught by the cache rather than staying pending indefinitely.

Hook cancel/deny paths (cmd/rampart/cli/hook_approval.go):
- Ctrl-C (context cancellation) during the initial POST now returns
  hookDeny instead of hookAsk. Rampart should fail closed, not fall
  back to Claude Code's native permission prompt.
- 200 {status: "denied"} (bulk-deny path) now returns hookDeny instead
  of hookAsk — the wrong security outcome previously.

* fix: goroutine leak on shutdown, parse fail-closed, sessionID trim, format default

Server shutdown goroutine leak (internal/proxy/server.go):
- Shutdown() now calls s.approvals.Close() after HTTP server shutdown.
  Store.Close() signals the background cleanup goroutine to exit and
  unblocks watchExpiry goroutines. Previously they leaked for up to 1h
  (the default approval timeout) after every graceful shutdown.

Parse failures fail closed in enforce mode (cmd/rampart/cli/hook.go):
- Hook now returns hookDeny (not hookAllow) on stdin parse failure when
  mode=enforce. A Rampart bug or malformed hook payload must not silently
  allow a tool call in enforce mode. Monitor/audit modes remain fail-open
  so the agent is never blocked by a transient parsing issue.
- Audit event updated: action reflects actual outcome (deny/allow).
- Error message included in the hookDeny reason string.

Format switch default case (cmd/rampart/cli/hook.go):
- Added explicit default branch returning an error. Format is validated
  before the switch, so this is unreachable in practice, but it prevents
  a nil parsed pointer from reaching downstream code if validation and
  the switch ever diverge.

sessionID trim in deriveRunID (cmd/rampart/cli/hook.go):
- RAMPART_RUN and CLAUDE_CONVERSATION_ID were already TrimSpace'd;
  the sessionID parameter itself was not. A whitespace-only session_id
  from Claude Code would produce a garbage run_id and break grouping.

Empty ID guard + poll status codes (cmd/rampart/cli/hook_approval.go):
- Guard against 201 with empty ID: previously polled /v1/approvals/
  (no ID) for the full timeout with no user feedback.
- Poll loop now handles non-2xx: 404/410 → immediate hookDeny,
  5xx → log and retry, other codes fall through.

* fix: CEF log injection, service file token exposure, template injection

CEF log injection (internal/audit/cef.go):
- esc() and escH() now escape \n and \r in addition to \ and = (or |).
  An agent could previously inject newlines into command/path parameters
  to forge additional CEF fields in the audit log. JSONL was unaffected
  (json.Marshal already escapes control characters).

Service file permissions (serve_install.go, setup.go):
- Launchd plist and systemd unit files are now written with mode 0o600
  (previously 0o644). Both files contain RAMPART_TOKEN inline.
- Added os.Chmod after os.WriteFile to fix permissions on existing files
  (WriteFile only applies the mode bit on creation, not on overwrite).

Template injection (serve_install.go):
- Switched text/template -> html/template for plist generation.
  html/template auto-escapes XML special characters in <string> values,
  preventing a crafted binary path or token from closing the <string>
  element and injecting arbitrary plist keys.
- Token is sanitized (newlines/CR/tabs stripped) before embedding in
  serviceConfig to prevent Environment= line injection in systemd units.

XML injection (setup.go):
- Added plistXMLEscape() helper: escapes &, <, >, ", ' in all
  string values interpolated into plist XML via fmt.Sprintf.
- Added systemdEnvEscape() helper: strips newlines/CR from token
  before embedding in systemd Environment= directives.

Author: peg

cmd/rampart/cli/hook.go

@@ -107,8 +107,8 @@ func deriveRunID(sessionID string) string {
 	if v := strings.TrimSpace(os.Getenv("RAMPART_RUN")); v != "" {
 		return v
 	}
-	if sessionID != "" {
-		return sessionID
+	if v := strings.TrimSpace(sessionID); v != "" {
+		return v
 	}
 	if v := strings.TrimSpace(os.Getenv("CLAUDE_CONVERSATION_ID")); v != "" {
 		return v
@@ -297,13 +297,25 @@ Cline setup: Use "rampart setup cline" to install hooks automatically.`,
 				parsed, err = parseClaudeCodeInput(os.Stdin, logger)
 			case "cline":
 				parsed, err = parseClineInput(os.Stdin, logger)
+			default:
+				// Should be unreachable — format is validated above.
+				// Explicit default prevents a nil parsed pointer reaching
+				// downstream code if the validation and switch ever diverge.
+				return fmt.Errorf("hook: unhandled format %q", format)
 			}
 			if err != nil {
 				logger.Warn("hook: failed to parse input", "format", format, "error", err)
-				// Best-effort audit entry for parse failure. This is written directly
-				// to the hook's audit file and is NOT part of the hash chain (the hook
-				// command does not maintain chain state). The schema matches the normal
-				// audit Event format for consistency with downstream consumers.
+				// In enforce mode, fail closed — a parse failure must not silently
+				// allow the tool call. In monitor/audit modes, allow through so the
+				// agent is never blocked by a Rampart bug.
+				outcome := "allow"
+				hookOutcome := hookAllow
+				if mode == "enforce" {
+					outcome = "deny"
+					hookOutcome = hookDeny
+				}
+				// Best-effort audit entry for parse failure. Written directly to
+				// the hook's audit file (not hash-chained).
 				parseFailureEvent := audit.Event{
 					ID:        audit.NewEventID(),
 					Timestamp: time.Now().UTC(),
@@ -312,15 +324,15 @@ Cline setup: Use "rampart setup cline" to install hooks automatically.`,
 					Tool:      "unknown",
 					Request:   map[string]any{"raw_error": err.Error()},
 					Decision: audit.EventDecision{
-						Action:  "allow",
-						Message: fmt.Sprintf("parse failure (format=%s): %v — allowing by default", format, err),
+						Action:  outcome,
+						Message: fmt.Sprintf("parse failure (format=%s): %v", format, err),
 					},
 				}
 				if line, marshalErr := json.Marshal(parseFailureEvent); marshalErr == nil {
 					line = append(line, '\n')
 					_, _ = auditFile.Write(line)
 				}
-				return outputHookResult(cmd, format, hookAllow, false, "", "")
+				return outputHookResult(cmd, format, hookOutcome, false, fmt.Sprintf("parse failure: %v", err), "")
 			}
 
 			// Build tool call for evaluation

cmd/rampart/cli/hook_approval.go

@@ -86,6 +86,13 @@ func (c *hookApprovalClient) requestApprovalCtx(ctx context.Context, tool, comma
 	client := &http.Client{Timeout: 5 * time.Second}
 	resp, err := client.Do(req)
 	if err != nil {
+		// If the context was cancelled (e.g. user Ctrl-C), deny rather than
+		// falling back to Claude Code's native prompt — Rampart should fail closed.
+		if ctx.Err() != nil {
+			c.logger.Debug("hook: context cancelled during approval create, denying", "error", err)
+			fmt.Fprintf(os.Stderr, "⚠ Approval cancelled — denying\n")
+			return hookDeny
+		}
 		if c.autoDiscovered {
 			c.logger.Debug("hook: auto-discovered serve unreachable, falling back to hookAsk", "url", c.serveURL, "error", err)
 		} else {
@@ -96,14 +103,22 @@ func (c *hookApprovalClient) requestApprovalCtx(ctx context.Context, tool, comma
 	}
 	defer resp.Body.Close()
 
-	// 200 means the run was already bulk-approved — no queuing needed.
+	// 200 means the approval was already resolved (auto-approve or bulk-resolve).
+	// Status field determines the outcome — don't assume approved.
 	if resp.StatusCode == http.StatusOK {
 		var autoResp struct {
 			Status string `json:"status"`
 		}
-		if err := json.NewDecoder(resp.Body).Decode(&autoResp); err == nil && autoResp.Status == "approved" {
-			c.logger.Debug("hook: run auto-approved by bulk-resolve, skipping queue")
-			return hookAllow
+		if err := json.NewDecoder(resp.Body).Decode(&autoResp); err == nil {
+			switch autoResp.Status {
+			case "approved":
+				c.logger.Debug("hook: run auto-approved by bulk-resolve, skipping queue")
+				return hookAllow
+			case "denied":
+				c.logger.Debug("hook: run auto-denied by bulk-resolve, skipping queue")
+				fmt.Fprintf(os.Stderr, "❌ Auto-denied (run was bulk-denied)\n")
+				return hookDeny
+			}
 		}
 		c.logger.Error("hook: unexpected 200 from approval create", "url", c.serveURL)
 		return hookAsk
@@ -122,6 +137,14 @@ func (c *hookApprovalClient) requestApprovalCtx(ctx context.Context, tool, comma
 		return hookAsk
 	}
 
+	// Guard against a serve bug returning 201 with an empty ID — polling
+	// /v1/approvals/ (no ID) would spin silently for the full timeout.
+	if created.ID == "" {
+		c.logger.Error("hook: server returned 201 with empty approval ID, falling back to native prompt")
+		fmt.Fprintf(os.Stderr, "⚠ Rampart serve returned empty approval ID, falling back to native prompt\n")
+		return hookAsk
+	}
+
 	// Print waiting message
 	fmt.Fprintf(os.Stderr, "⏳ Approval required — approve via dashboard (%s/dashboard/) or `rampart watch`\n", c.serveURL)
 	fmt.Fprintf(os.Stderr, "   Approval ID: %s\n", created.ID)
@@ -163,6 +186,20 @@ func (c *hookApprovalClient) pollApprovalCtx(ctx context.Context, id string, tim
 				continue
 			}
 
+			// Non-2xx status codes (404, 500, etc.) mean the approval is
+			// gone or the server is broken — don't spin silently until timeout.
+			if resp.StatusCode == http.StatusGone || resp.StatusCode == http.StatusNotFound {
+				resp.Body.Close()
+				fmt.Fprintf(os.Stderr, "⚠ Approval %s no longer exists (status %d) — denying\n", id, resp.StatusCode)
+				c.logger.Warn("hook: approval gone during poll, denying", "id", id, "status", resp.StatusCode)
+				return hookDeny
+			}
+			if resp.StatusCode >= 500 {
+				resp.Body.Close()
+				c.logger.Warn("hook: server error during poll, retrying", "id", id, "status", resp.StatusCode)
+				continue
+			}
+
 			var status pollApprovalResponse
 			err = json.NewDecoder(resp.Body).Decode(&status)
 			resp.Body.Close()

cmd/rampart/cli/serve_install.go

@@ -22,7 +22,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
-	"text/template"
+	"html/template"
 
 	"github.com/spf13/cobra"
 )
@@ -257,10 +257,14 @@ func newServeInstallCmd(opts *rootOptions, runner commandRunner) *cobra.Command
 
 			homeDir, _ := os.UserHomeDir()
 			args := buildServiceArgs(port, opts.configPath, configDir, auditDir, mode, approvalTimeout)
+			// Strip control characters from token before embedding in
+			// service files. Newlines in a systemd Environment= directive
+			// or plist <string> could inject arbitrary directives.
+			safeToken := strings.NewReplacer("\n", "", "\r", "", "\t", "").Replace(token)
 			cfg := serviceConfig{
 				Binary:  binary,
 				Args:    args,
-				Token:   token,
+				Token:   safeToken,
 				LogPath: logPath(),
 				HomeDir: homeDir,
 			}
@@ -321,9 +325,13 @@ func installDarwin(cmd *cobra.Command, cfg serviceConfig, force, generated bool,
 	// Ensure log directory exists.
 	_ = os.MkdirAll(filepath.Dir(cfg.LogPath), 0o755)
 
-	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+	// 0o600: plist contains RAMPART_TOKEN — must not be world-readable.
+	// Chmod after write because os.WriteFile only applies the mode on creation;
+	// an existing file with wrong permissions would not be updated otherwise.
+	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
 		return fmt.Errorf("write plist: %w", err)
 	}
+	_ = os.Chmod(path, 0o600)
 
 	if out, err := runner("launchctl", "load", path).CombinedOutput(); err != nil {
 		return fmt.Errorf("launchctl load: %w\n%s", err, out)
@@ -359,9 +367,12 @@ func installLinux(cmd *cobra.Command, cfg serviceConfig, force, generated bool,
 	// Ensure log directory exists.
 	_ = os.MkdirAll(filepath.Dir(cfg.LogPath), 0o755)
 
-	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+	// 0o600: unit file contains RAMPART_TOKEN — must not be world-readable.
+	// Chmod after write because os.WriteFile only applies the mode on creation.
+	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
 		return fmt.Errorf("write unit: %w", err)
 	}
+	_ = os.Chmod(path, 0o600)
 
 	// Stop the old service before reload so the new binary/token take effect.
 	_, _ = runner("systemctl", "--user", "stop", "rampart-serve.service").CombinedOutput()

cmd/rampart/cli/setup.go

@@ -588,6 +588,26 @@ func installService(cmd *cobra.Command, home, rampartBin, policyPath, token stri
 	return installSystemd(cmd, home, rampartBin, policyPath, token, port)
 }
 
+// plistXMLEscape escapes a string for safe embedding inside a plist <string>
+// element. Unescaped '<' or '&' in a value would break the XML structure or
+// allow a crafted path/token to inject arbitrary plist keys.
+func plistXMLEscape(s string) string {
+	s = strings.ReplaceAll(s, "&", "&amp;")
+	s = strings.ReplaceAll(s, "<", "&lt;")
+	s = strings.ReplaceAll(s, ">", "&gt;")
+	s = strings.ReplaceAll(s, "\"", "&quot;")
+	s = strings.ReplaceAll(s, "'", "&apos;")
+	return s
+}
+
+// systemdEnvEscape strips characters that would break a systemd
+// Environment= line (newlines introduce new directives).
+func systemdEnvEscape(s string) string {
+	s = strings.ReplaceAll(s, "\n", "")
+	s = strings.ReplaceAll(s, "\r", "")
+	return s
+}
+
 func installSystemd(cmd *cobra.Command, home, rampartBin, policyPath, token string, port int) error {
 	serviceDir := filepath.Join(home, ".config", "systemd", "user")
 	if err := os.MkdirAll(serviceDir, 0o700); err != nil {
@@ -607,11 +627,14 @@ Environment=RAMPART_TOKEN=%s
 
 [Install]
 WantedBy=default.target
-`, rampartBin, port, policyPath, home, token)
+`, rampartBin, port, policyPath, home, systemdEnvEscape(token))
 
-	if err := os.WriteFile(servicePath, []byte(serviceContent), 0o644); err != nil {
+	// 0o600: unit file contains RAMPART_TOKEN — must not be world-readable.
+	// Chmod after write because os.WriteFile only applies the mode on creation.
+	if err := os.WriteFile(servicePath, []byte(serviceContent), 0o600); err != nil {
 		return fmt.Errorf("setup: write service file: %w", err)
 	}
+	_ = os.Chmod(servicePath, 0o600)
 	fmt.Fprintf(cmd.OutOrStdout(), "✓ Systemd service written to %s\n", servicePath)
 	return nil
 }
@@ -622,6 +645,7 @@ func installLaunchd(cmd *cobra.Command, home, rampartBin, policyPath, token stri
 		return fmt.Errorf("setup: create LaunchAgents dir: %w", err)
 	}
 	plistPath := filepath.Join(agentDir, "com.rampart.proxy.plist")
+	logPath := filepath.Join(home, ".rampart", "rampart-proxy.log")
 	plistContent := fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
@@ -654,13 +678,22 @@ func installLaunchd(cmd *cobra.Command, home, rampartBin, policyPath, token stri
     <string>%s</string>
 </dict>
 </plist>
-`, rampartBin, port, policyPath, home, token,
-		filepath.Join(home, ".rampart", "rampart-proxy.log"),
-		filepath.Join(home, ".rampart", "rampart-proxy.log"))
-
-	if err := os.WriteFile(plistPath, []byte(plistContent), 0o644); err != nil {
+`,
+		plistXMLEscape(rampartBin),
+		port,
+		plistXMLEscape(policyPath),
+		plistXMLEscape(home),
+		plistXMLEscape(token),
+		plistXMLEscape(logPath),
+		plistXMLEscape(logPath),
+	)
+
+	// 0o600: plist contains RAMPART_TOKEN — must not be world-readable.
+	// Chmod after write because os.WriteFile only applies the mode on creation.
+	if err := os.WriteFile(plistPath, []byte(plistContent), 0o600); err != nil {
 		return fmt.Errorf("setup: write plist: %w", err)
 	}
+	_ = os.Chmod(plistPath, 0o600)
 	fmt.Fprintf(cmd.OutOrStdout(), "✓ LaunchAgent written to %s\n", plistPath)
 	return nil
 }

docs-site/getting-started/quickstart.md

@@ -8,6 +8,26 @@ Get Rampart protecting your AI agent in one command.
 !!! tip "Zero risk to try"
     Rampart **fails open** — if the service is unreachable or the policy engine crashes, your tools keep working. You'll never get locked out of your own machine.
 
+## Install
+
+=== "Go install (recommended)"
+
+    ```bash
+    go install github.com/peg/rampart/cmd/rampart@latest
+    ```
+
+=== "Script"
+
+    ```bash
+    curl -fsSL https://rampart.sh/install.sh | sh
+    ```
+
+=== "Homebrew"
+
+    ```bash
+    brew tap peg/rampart && brew install rampart
+    ```
+
 ## One-Command Setup
 
 ```bash

docs-site/getting-started/tutorial.md

@@ -11,29 +11,29 @@ Let's set it up.
 ## Prerequisites
 
 - **macOS or Linux** (Windows WSL works too)
-- **Homebrew** (recommended) or **Go 1.24+** for building from source
+- **Go 1.24+** (recommended) or the install script for a no-Go option
 - **Claude Code, Codex, or Cline** — this guide uses Claude Code, but Rampart works with [many agents](../integrations/index.md)
 
 ---
 
 ## Step 1: Install
 
-=== "Homebrew (recommended)"
+=== "Go install (recommended)"
 
     ```bash
-    brew tap peg/rampart && brew install rampart
+    go install github.com/peg/rampart/cmd/rampart@latest
     ```
 
-=== "Go install"
+=== "Script"
 
     ```bash
-    go install github.com/peg/rampart/cmd/rampart@latest
+    curl -fsSL https://rampart.sh/install.sh | sh
     ```
 
-=== "Script"
+=== "Homebrew"
 
     ```bash
-    curl -fsSL https://rampart.sh/install.sh | sh
+    brew tap peg/rampart && brew install rampart
     ```
 
 Verify:

internal/approval/store.go

@@ -256,18 +256,26 @@ func (s *Store) Get(id string) (*Request, bool) {
 	defer s.mu.Unlock()
 
 	req, ok := s.pending[id]
-	return req, ok
+	if !ok {
+		return nil, false
+	}
+	// Return a snapshot so callers don't race with watchExpiry writes.
+	cp := *req
+	return &cp, true
 }
 
-// List returns all pending requests.
+// List returns snapshots of all pending requests.
+// Callers receive copies, not live pointers, so reads don't race with
+// concurrent writes from watchExpiry or Resolve.
 func (s *Store) List() []*Request {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
 	result := make([]*Request, 0, len(s.pending))
 	for _, req := range s.pending {
 		if req.Status == StatusPending {
-			result = append(result, req)
+			cp := *req
+			result = append(result, &cp)
 		}
 	}
 	// Sort by creation time (oldest first) for deterministic ordering.

internal/audit/cef.go

@@ -56,16 +56,22 @@ func FormatCEF(e Event) string {
 	}
 	policyNames := strings.Join(e.Decision.MatchedPolicies, ",")
 
-	// Escape CEF extension values (\ and =).
+	// Escape CEF extension values (\ and = and newlines).
+	// Newlines must be escaped to prevent an agent from injecting forged
+	// CEF fields into the audit log via tool parameters (log injection).
 	esc := func(s string) string {
 		s = strings.ReplaceAll(s, `\`, `\\`)
 		s = strings.ReplaceAll(s, `=`, `\=`)
+		s = strings.ReplaceAll(s, "\n", `\n`)
+		s = strings.ReplaceAll(s, "\r", `\r`)
 		return s
 	}
-	// Escape CEF header pipes.
+	// Escape CEF header pipes and newlines.
 	escH := func(s string) string {
 		s = strings.ReplaceAll(s, `\`, `\\`)
 		s = strings.ReplaceAll(s, `|`, `\|`)
+		s = strings.ReplaceAll(s, "\n", `\n`)
+		s = strings.ReplaceAll(s, "\r", `\r`)
 		return s
 	}