security: comprehensive code review fixes

- Fix NotebookEditTool path validation bypass (sandbox escape)
- Add MCP server command validation/sandboxing
- Fix sandbox fail-open to fail-closed behavior
- Clean up daemon API key file on shutdown
- Add safeguard for --dangerously-skip-permissions
- Fix SSRF bypass in redirect chains
- Make Session thread-safe with sync.RWMutex
- Add preview for fuzzy edit replacements
- Improve Guardian prompt sanitization patterns
- Fix overly broad credential detection regex
- Fix sandbox mode defaulting to ModeOff on typo
- Fix global state mutation in exec.go
- Fix os.Exit bypassing deferred cleanup
- Add nil check for notebook cell type assertion
- Cache injection scanner and egress regex compilation
- Remove dead code in guardian.go
- Remove duplicate flag registrations
- Add validation for tool alias collisions
- Fix settings merge replacing arrays
- Fix TruncateOutput splitting multi-byte runes
- Fix predictable exec IDs and sandbox instance race
- Add WAL checkpoint cleanup after successful save

Author: Patel230

cmd/chat.go

@@ -107,10 +107,24 @@ func defaultRegistry(settings hawkconfig.Settings) (*tool.Registry, error) {
 	if tool.IsPowerShellAvailable() {
 		tools = append(tools, tool.PowerShellTool{})
 	}
+	// Detect project-level MCP servers (supply chain attack vector).
+	// Project .hawk/settings.json can be committed to a repo and define
+	// arbitrary commands that execute on clone. Gate behind --allow-project-mcp.
+	projectMCPServers := hawkconfig.ProjectMCPServers()
+	projectMCPNames := make(map[string]bool, len(projectMCPServers))
+	for _, cfg := range projectMCPServers {
+		if cfg.Name != "" {
+			projectMCPNames[cfg.Name] = true
+		}
+	}
 	for _, cfg := range settings.MCPServers {
 		if cfg.Name == "" || cfg.Command == "" {
 			continue
 		}
+		if projectMCPNames[cfg.Name] && !allowProjectMCP {
+			fmt.Fprintf(os.Stderr, "hawk: skipping project-level MCP server %q (defined in .hawk/settings.json); use --allow-project-mcp to enable\n", cfg.Name)
+			continue
+		}
 		mcpTools, err := tool.LoadMCPTools(context.Background(), cfg.Name, cfg.Command, cfg.Args...)
 		if err != nil {
 			continue

cmd/chat_commands.go

@@ -690,7 +690,7 @@ func (m *chatModel) handleCommand(text string) (tea.Model, tea.Cmd) {
 			return m, nil
 		}
 		// List branches
-		history, err := m.session.ConvoDAG.History(headID)
+		history, err := m.session.ConvoDAG.History(context.Background(), headID)
 		if err != nil || len(history) < 2 {
 			m.messages = append(m.messages, displayMsg{role: "system", content: "No branches available (linear conversation).\nUse /fork to create a branch."})
 			return m, nil

cmd/daemon.go

@@ -142,6 +142,8 @@ func runDaemonStart(_ *cobra.Command, _ []string) error {
 	<-sig
 
 	fmt.Println("\nShutting down...")
+	// Clean up the API key file to avoid leaving secrets on disk.
+	_ = os.Remove(keyFile)
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 	return srv.Stop(ctx)

cmd/exec.go

@@ -2,6 +2,8 @@ package cmd
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -33,6 +35,23 @@ var (
 	execJSON         bool
 )
 
+// ExitCodeError wraps a non-zero exit code so it can be returned from RunE
+// instead of calling os.Exit directly. This allows deferred cleanup (worktree
+// removal, global state restoration) to run before the process exits.
+type ExitCodeError struct {
+	Code int
+	Err  error
+}
+
+func (e *ExitCodeError) Error() string {
+	if e.Err != nil {
+		return e.Err.Error()
+	}
+	return fmt.Sprintf("exit code %d", e.Code)
+}
+
+func (e *ExitCodeError) Unwrap() error { return e.Err }
+
 // ExecResult is the structured output for --output-format json.
 type ExecResult struct {
 	SessionID  string `json:"session_id"`
@@ -116,7 +135,7 @@ func runExec(_ *cobra.Command, args []string) error {
 		base := getCurrentBranch(cwd)
 		branch := execWorktreeName
 		if branch == "" {
-			branch = fmt.Sprintf("hawk-exec/%d", start.UnixMilli())
+			branch = fmt.Sprintf("hawk-exec/%d-%s", start.UnixMilli(), randomHex(4))
 		}
 		var wtErr error
 		wtPath, wtErr = createExecWorktree(cwd, base, branch)
@@ -130,14 +149,6 @@ func runExec(_ *cobra.Command, args []string) error {
 		}
 	}
 
-	// Override global flags so shared helpers pick up exec-specific values
-	if execModel != "" {
-		model = execModel
-	}
-	if execMaxTurns > 0 {
-		maxTurns = execMaxTurns
-	}
-
 	// Load settings
 	settings := hawkconfig.LoadSettings()
 
@@ -148,15 +159,14 @@ func runExec(_ *cobra.Command, args []string) error {
 	}
 
 	// If --agent is specified, prepend the agent persona
+	var agentModel string
 	if execAgent != "" {
 		agentDef, err := agents.Get(execAgent)
 		if err != nil {
 			return fmt.Errorf("agent %q: %w", execAgent, err)
 		}
 		systemPrompt = agentDef.Prompt + "\n\n" + systemPrompt
-		if agentDef.Model != "" {
-			model = agentDef.Model
-		}
+		agentModel = agentDef.Model
 	}
 
 	// Create tool registry
@@ -165,12 +175,21 @@ func runExec(_ *cobra.Command, args []string) error {
 		return err
 	}
 
-	// Create engine session
+	// Resolve effective model/provider without mutating globals.
+	// Priority: agent model > exec model > settings > global > auto-detect.
 	effectiveModel, effectiveProvider := effectiveModelAndProvider(settings)
+	if execModel != "" {
+		effectiveModel = execModel
+	}
+	if agentModel != "" {
+		effectiveModel = agentModel
+	}
+
+	// Create engine session
 	sess := newHawkSession(settings, effectiveProvider, effectiveModel, systemPrompt, registry)
 	sess.SetLogger(logger.New(io.Discard, logger.Error))
 
-	if err := configureSession(sess, settings); err != nil {
+	if err := configureSession(sess, settings, execMaxTurns); err != nil {
 		return err
 	}
 
@@ -279,7 +298,7 @@ func runExec(_ *cobra.Command, args []string) error {
 		exitCode = 1
 	}
 	if !execEphemeral {
-		sessionID := fmt.Sprintf("exec-%d", start.UnixMilli())
+		sessionID := fmt.Sprintf("exec-%d-%s", start.UnixMilli(), randomHex(4))
 		persistExecSession(sessionID, effectiveModel, effectiveProvider, prompt, response.String())
 	}
 
@@ -295,7 +314,7 @@ func runExec(_ *cobra.Command, args []string) error {
 
 	if execOutputFormat == "json" {
 		result := ExecResult{
-			SessionID:  fmt.Sprintf("exec-%d", start.UnixMilli()),
+			SessionID:  fmt.Sprintf("exec-%d-%s", start.UnixMilli(), randomHex(4)),
 			Response:   response.String(),
 			ExitCode:   exitCode,
 			TokensIn:   totalIn,
@@ -320,7 +339,7 @@ func runExec(_ *cobra.Command, args []string) error {
 	}
 
 	if exitCode != 0 {
-		os.Exit(exitCode)
+		return &ExitCodeError{Code: exitCode}
 	}
 	return nil
 }
@@ -376,3 +395,10 @@ func cleanupExecWorktree(repoDir, wtPath string) {
 	cmd.Dir = repoDir
 	_ = cmd.Run()
 }
+
+// randomHex returns a hex-encoded string of n random bytes.
+func randomHex(n int) string {
+	b := make([]byte, n)
+	_, _ = rand.Read(b)
+	return hex.EncodeToString(b)
+}

cmd/options.go

@@ -189,7 +189,7 @@ func newHawkSession(settings hawkconfig.Settings, effectiveProvider, effectiveMo
 	return engine.NewHawkSession(context.Background(), hawkconfig.DeploymentRoutingEnabled(settings), effectiveProvider, effectiveModel, systemPrompt, registry)
 }
 
-func configureSession(sess *engine.Session, settings hawkconfig.Settings) error {
+func configureSession(sess *engine.Session, settings hawkconfig.Settings, maxTurnsOverride ...int) error {
 	sess.WireAgentTool()
 	sess.SetAllowedDirs(addDirs)
 
@@ -240,7 +240,11 @@ func configureSession(sess *engine.Session, settings hawkconfig.Settings) error
 	if err := sess.SetPermissionMode(mode); err != nil {
 		return err
 	}
-	if err := sess.SetMaxTurns(maxTurns); err != nil {
+	effectiveMaxTurns := maxTurns
+	if len(maxTurnsOverride) > 0 && maxTurnsOverride[0] > 0 {
+		effectiveMaxTurns = maxTurnsOverride[0]
+	}
+	if err := sess.SetMaxTurns(effectiveMaxTurns); err != nil {
 		return err
 	}
 

cmd/review_store.go

@@ -211,8 +211,14 @@ func (s *ReviewStore) Summary() (map[ReviewStatus]int, error) {
 	return m, rows.Err()
 }
 
-// Close closes the database.
+// Close checkpoints the WAL and closes the database.
+// Running PRAGMA wal_checkpoint(TRUNCATE) before close ensures that
+// .db-wal and .db-shm files are cleaned up after all data is safely
+// flushed to the main database file.
 func (s *ReviewStore) Close() error {
+	// Checkpoint WAL to flush all data into the main db and truncate
+	// the WAL file, so no .db-wal / .db-shm files linger on disk.
+	_, _ = s.db.Exec("PRAGMA wal_checkpoint(TRUNCATE)")
 	return s.db.Close()
 }
 

cmd/root.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"os"
@@ -58,6 +59,7 @@ var (
 	containerMode              bool
 	noContainer                bool
 	recoverFlag                bool
+	allowProjectMCP            bool
 )
 
 // SetVersion sets the version string from main.
@@ -95,6 +97,11 @@ var rootCmd = &cobra.Command{
 		if err := validateRootFlags(); err != nil {
 			return err
 		}
+		if dangerouslySkipPermissions {
+			if err := confirmDangerousSkipPermissions(); err != nil {
+				return err
+			}
+		}
 		if printMode || promptFlag != "" || inputFormat == "stream-json" {
 			if promptFlag == "" {
 				stdinPrompt, err := readPromptFromStdin(inputFormat)
@@ -162,9 +169,7 @@ func init() {
 	rootCmd.Flags().StringArrayVar(&addDirs, "add-dir", nil, "additional directories to include in session context")
 	rootCmd.Flags().StringArrayVar(&mcpServers, "mcp", nil, "MCP server command")
 	rootCmd.Flags().StringArrayVar(&toolsFlag, "tools", nil, `available tools: "" disables all tools, "default" enables all, or names like "Bash,Edit,Read"`)
-	rootCmd.Flags().StringArrayVar(&allowedToolsFlag, "allowedTools", nil, `comma or space-separated tool permission rules to allow (e.g. "Bash(git:*) Edit")`)
 	rootCmd.Flags().StringArrayVar(&allowedToolsFlag, "allowed-tools", nil, `comma or space-separated tool permission rules to allow (e.g. "Bash(git:*) Edit")`)
-	rootCmd.Flags().StringArrayVar(&disallowedToolsFlag, "disallowedTools", nil, `comma or space-separated tool permission rules to deny (e.g. "Bash(git:*) Edit")`)
 	rootCmd.Flags().StringArrayVar(&disallowedToolsFlag, "disallowed-tools", nil, `comma or space-separated tool permission rules to deny (e.g. "Bash(git:*) Edit")`)
 	rootCmd.Flags().StringVar(&permissionMode, "permission-mode", "", "permission mode: default, acceptEdits, bypassPermissions, dontAsk, or plan")
 	rootCmd.Flags().BoolVar(&dangerouslySkipPermissions, "dangerously-skip-permissions", false, "bypass all permission checks")
@@ -190,6 +195,7 @@ func init() {
 	rootCmd.Flags().BoolVar(&refreshCatalogFlag, "refresh-catalog", false, "refresh the eyrie model catalog before starting")
 	rootCmd.Flags().BoolVar(&skipCatalogRefreshFlag, "no-auto-catalog-refresh", false, "disable automatic catalog refresh when cache is missing, empty, or stale")
 	rootCmd.Flags().BoolVar(&recoverFlag, "recover", false, "scan for interrupted sessions and offer to resume")
+	rootCmd.Flags().BoolVar(&allowProjectMCP, "allow-project-mcp", false, "allow MCP servers defined in project-level .hawk/settings.json (security risk)")
 	rootCmd.AddCommand(versionCmd)
 	rootCmd.AddCommand(setupCmd)
 	rootCmd.AddCommand(doctorCmd)
@@ -222,6 +228,38 @@ func init() {
 	rootCmd.AddCommand(recoverCmd)
 }
 
+// confirmDangerousSkipPermissions enforces a safety guard when --dangerously-skip-permissions is set.
+// In a terminal, it prompts for interactive confirmation. In non-interactive mode (CI, scripts),
+// it requires the HAWK_DANGEROUSLY_SKIP_PERMISSIONS=1 environment variable.
+func confirmDangerousSkipPermissions() error {
+	if isStdinTerminal() {
+		fmt.Fprint(os.Stderr, "Are you sure? This disables all safety checks [y/N]: ")
+		scanner := bufio.NewScanner(os.Stdin)
+		if !scanner.Scan() {
+			return fmt.Errorf("--dangerously-skip-permissions requires confirmation")
+		}
+		answer := strings.TrimSpace(strings.ToLower(scanner.Text()))
+		if answer != "y" && answer != "yes" {
+			return fmt.Errorf("--dangerously-skip-permissions declined; aborting")
+		}
+		return nil
+	}
+	// Non-interactive: require explicit env var override.
+	if os.Getenv("HAWK_DANGEROUSLY_SKIP_PERMISSIONS") != "1" {
+		return fmt.Errorf("--dangerously-skip-permissions requires HAWK_DANGEROUSLY_SKIP_PERMISSIONS=1 in non-interactive mode")
+	}
+	return nil
+}
+
+// isStdinTerminal reports whether stdin is connected to a terminal.
+func isStdinTerminal() bool {
+	fi, err := os.Stdin.Stat()
+	if err != nil {
+		return false
+	}
+	return fi.Mode()&os.ModeCharDevice != 0
+}
+
 var completionCmd = &cobra.Command{
 	Use:   "completion [bash|zsh|fish|powershell]",
 	Short: "Generate shell completion script",

cmd/sandbox.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/GrayCodeAI/hawk/internal/diffsandbox"
 	"github.com/spf13/cobra"
@@ -10,12 +11,15 @@ import (
 // sandboxInstance is a package-level sandbox for the CLI session.
 // In a real integration this would be loaded/shared from the hawk engine;
 // for now we create a fresh sandbox rooted at the current directory.
-var sandboxInstance *diffsandbox.Sandbox
+var (
+	sandboxInstance *diffsandbox.Sandbox
+	sandboxOnce     sync.Once
+)
 
 func getSandbox() *diffsandbox.Sandbox {
-	if sandboxInstance == nil {
+	sandboxOnce.Do(func() {
 		sandboxInstance = diffsandbox.New(".")
-	}
+	})
 	return sandboxInstance
 }