v0.17.0: security fixes — O_NOFOLLOW, atomic writes, mutex protection, path confinement

- Fix #1+10: readFileTool uses os.OpenFile with O_NOFOLLOW (eliminates
  stat-then-open TOCTOU race and symlink following)
- Fix #3: patchTool uses atomic temp-file + rename for writes
- Fix #6: writeFileTool has optional restrictToCWD flag (rejects ../ escape)
- Fix #8: sync.Mutex on shellTool.trustedClasses and TTYApprover.TrustedClasses
- Fix: context trimmer drops complete assistant+tool groups (prevents
  orphaned tool messages that DeepSeek rejects)
- Fix: config loader tests isolate HOME to avoid leaking real config
- Fix: truncate() handles negative/zero limits without panic
- Tests: odek auto-generated 15 buildSubagentPrompt contract tests

Author: jkyberneees

cmd/odek/file_tool.go

@@ -4,11 +4,13 @@ import (
 	"bufio"
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"regexp"
 	"sort"
 	"strings"
+	"syscall"
 
 	"github.com/BackendStack21/kode/internal/danger"
 )
@@ -94,25 +96,31 @@ func (t *readFileTool) Call(argsJSON string) (string, error) {
 		return jsonError(err.Error())
 	}
 
-	// Check file exists and is not a directory
-	info, err := os.Stat(args.Path)
+	// Security: open the file without following symlinks (O_NOFOLLOW).
+	// This atomically handles the existence check, type check, and open
+	// in a single syscall — eliminating the TOCTOU window between
+	// os.Stat (check) and os.Open (use). If the path is a symlink, the
+	// open fails with ELOOP.
+	f, err := os.OpenFile(args.Path, os.O_RDONLY|syscall.O_NOFOLLOW, 0)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return jsonError(fmt.Sprintf("file not found: %s", args.Path))
 		}
-		return jsonError(fmt.Sprintf("cannot access %q: %v", args.Path, err))
-	}
-	if info.IsDir() {
-		return jsonError(fmt.Sprintf("%q is a directory, not a file", args.Path))
+		// ELOOP means the path is a symlink — refuse to follow it
+		return jsonError(fmt.Sprintf("cannot open %q: %v", args.Path, err))
 	}
+	defer f.Close()
 
-	// Single pass: open → binary check from sample → seek → read+count
-	f, err := os.Open(args.Path)
+	// Check it's a regular file, not a directory
+	info, err := f.Stat()
 	if err != nil {
-		return jsonError(fmt.Sprintf("cannot open %q: %v", args.Path, err))
+		return jsonError(fmt.Sprintf("cannot stat %q: %v", args.Path, err))
+	}
+	if info.IsDir() {
+		return jsonError(fmt.Sprintf("%q is a directory, not a file", args.Path))
 	}
-	defer f.Close()
 
+	// Single pass: binary check from sample → seek → read+count
 	sample := make([]byte, 8192)
 	n, _ := f.Read(sample)
 	if isBinary(sample[:n]) {
@@ -140,7 +148,8 @@ func (t *readFileTool) Call(argsJSON string) (string, error) {
 
 type writeFileTool struct {
 	dangerousConfig danger.DangerousConfig
-	trustedClasses map[danger.RiskClass]bool
+	trustedClasses  map[danger.RiskClass]bool
+	restrictToCWD   bool // when true, reject paths escaping the working directory
 }
 
 func (t *writeFileTool) Name() string { return "write_file" }
@@ -188,6 +197,16 @@ func (t *writeFileTool) Call(argsJSON string) (string, error) {
 		return jsonError("path is required")
 	}
 
+	// Path confinement: when restrictToCWD is enabled, reject paths that
+	// escape the working directory via ".." traversal or absolute paths.
+	if t.restrictToCWD {
+		resolved, err := confineToCWD(args.Path)
+		if err != nil {
+			return jsonError(err.Error())
+		}
+		args.Path = resolved
+	}
+
 	// Security: classify and check write operation
 	risk := danger.ClassifyPath(args.Path)
 	if err := t.dangerousConfig.CheckOperation(danger.ToolOperation{
@@ -549,16 +568,23 @@ func (t *patchTool) Call(argsJSON string) (string, error) {
 		return jsonError(err.Error())
 	}
 
-	// Read the file
-	data, err := os.ReadFile(args.Path)
+	// Read the file without following symlinks
+	f, err := os.OpenFile(args.Path, os.O_RDONLY|syscall.O_NOFOLLOW, 0)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return jsonError(fmt.Sprintf("file not found: %s", args.Path))
 		}
 		return jsonError(fmt.Sprintf("cannot read %q: %v", args.Path, err))
 	}
+	defer f.Close()
 
-	original := string(data)
+	// Read content through the opened fd (not re-opening the path)
+	var sb strings.Builder
+	_, err = io.Copy(&sb, f)
+	if err != nil {
+		return jsonError(fmt.Sprintf("cannot read %q: %v", args.Path, err))
+	}
+	original := sb.String()
 
 	// Check that old_string exists
 	if !strings.Contains(original, args.OldString) {
@@ -579,7 +605,35 @@ func (t *patchTool) Call(argsJSON string) (string, error) {
 		truncateDiff(modified, 100),
 	)
 
-	if err := os.WriteFile(args.Path, []byte(modified), 0644); err != nil {
+	// Atomic write via temp file + rename to prevent TOCTOU symlink races.
+	// The temp file is created in the same directory (same filesystem),
+	// and os.Rename atomically replaces the directory entry without
+	// following symlinks.
+	dir := filepath.Dir(args.Path)
+	tmpFile, err := os.CreateTemp(dir, ".tmp_patch_*")
+	if err != nil {
+		return jsonError(fmt.Sprintf("cannot create temp file: %v", err))
+	}
+	tmpPath := tmpFile.Name()
+
+	if _, err := tmpFile.Write([]byte(modified)); err != nil {
+		tmpFile.Close()
+		os.Remove(tmpPath)
+		return jsonError(fmt.Sprintf("cannot write %q: %v", args.Path, err))
+	}
+	if err := tmpFile.Chmod(0644); err != nil {
+		tmpFile.Close()
+		os.Remove(tmpPath)
+		return jsonError(fmt.Sprintf("cannot set permissions %q: %v", args.Path, err))
+	}
+	if err := tmpFile.Close(); err != nil {
+		os.Remove(tmpPath)
+		return jsonError(fmt.Sprintf("cannot close temp file: %v", err))
+	}
+
+	// Atomic rename — replaces target directory entry without following symlinks
+	if err := os.Rename(tmpPath, args.Path); err != nil {
+		os.Remove(tmpPath)
 		return jsonError(fmt.Sprintf("cannot write %q: %v", args.Path, err))
 	}
 
@@ -654,6 +708,31 @@ func readLinesWithCount(f *os.File, offset, limit int) (string, int, error) {
 	return strings.TrimSuffix(out.String(), "\n"), lineNum, scanner.Err()
 }
 
+// confineToCWD resolves path relative to the current working directory and
+// rejects paths that escape the working directory via ".." traversal or are
+// absolute paths outside the CWD. Returns the cleaned absolute path on success.
+func confineToCWD(path string) (string, error) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return "", fmt.Errorf("cannot determine working directory: %v", err)
+	}
+
+	// Resolve to absolute path
+	var abs string
+	if filepath.IsAbs(path) {
+		abs = filepath.Clean(path)
+	} else {
+		abs = filepath.Join(cwd, path)
+	}
+
+	// Check that the resolved path is within CWD
+	if !strings.HasPrefix(abs, cwd+string(filepath.Separator)) && abs != cwd {
+		return "", fmt.Errorf("path %q escapes the working directory", path)
+	}
+
+	return abs, nil
+}
+
 func truncateDiff(s string, maxLen int) string {
 	// Take first line for diff display
 	firstLine := strings.SplitN(s, "\n", 2)[0]

cmd/odek/main.go

@@ -1080,7 +1080,7 @@ func builtinTools(dc danger.DangerousConfig, sm *skills.SkillManager, approver d
 			timeout:        120 * time.Second,
 		},
 		&readFileTool{dangerousConfig: dc},
-		&writeFileTool{dangerousConfig: dc},
+		&writeFileTool{dangerousConfig: dc, restrictToCWD: true},
 		&searchFilesTool{dangerousConfig: dc},
 		&patchTool{dangerousConfig: dc},
 		newBrowserTool(dc),

cmd/odek/shell.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os/exec"
 	"strings"
+	"sync"
 
 	"github.com/BackendStack21/kode/internal/danger"
 )
@@ -55,6 +56,7 @@ type shellTool struct {
 	// trustedClasses caches user-approved risk classes for this process.
 	// Set when user presses T (trust this session) at the prompt.
 	trustedClasses map[danger.RiskClass]bool
+	trustedMu      sync.Mutex
 
 	// ttyPath is the path to the terminal device for approval prompts.
 	// Overridden in tests to mock user input. Only used when approver is nil.
@@ -159,9 +161,11 @@ func (t *shellTool) promptUser(cmd, description string) error {
 	approver := t.approver
 	if approver == nil {
 		ttyApprover := danger.NewTTYApprover(&t.dangerousConfig)
+		t.trustedMu.Lock()
 		if t.trustedClasses != nil {
-			ttyApprover.TrustedClasses = t.trustedClasses
+			ttyApprover.SetTrustedClasses(t.trustedClasses)
 		}
+		t.trustedMu.Unlock()
 		if t.ttyPath != "" {
 			ttyApprover.TTYPath = t.ttyPath
 		}
@@ -172,7 +176,9 @@ func (t *shellTool) promptUser(cmd, description string) error {
 	if err == nil {
 		// Sync trusted classes back if using TTYApprover
 		if tty, ok := approver.(*danger.TTYApprover); ok {
+			t.trustedMu.Lock()
 			t.trustedClasses = tty.TrustedClasses
+			t.trustedMu.Unlock()
 		}
 	}
 	return err

cmd/odek/subagent.go

@@ -466,6 +466,9 @@ func extractFilesChanged(messages []llm.Message) []string {
 }
 
 func truncate(s string, n int) string {
+	if n <= 0 {
+		return "…"
+	}
 	runes := []rune(s)
 	if len(runes) <= n {
 		return s