perf+sec: package-by-package optimization sprint

- render: single rune scan in truncate (was double scan)
- repl: errors.Is(err, io.EOF) instead of fragile string comparison
- odek: LoadProjectFile rejects symlinks (O_NOFOLLOW)
- file_tool: single-pass readFile (binary check + count + read in one open)
- file_tool: searchFiles single open (was open-close-open)
- file_tool: atomic write via temp+rename (TOCTOU fix)
- resource: O_NOFOLLOW on FileResolver.Load (TOCTOU fix)
- loop: pre-allocated slice for skill injection (no nested append)
- session: index.json for O(1) List/Latest (no per-file reads)
- mcpclient: single reader goroutine per conn (no goroutine-per-read leak)
- memory: AddFact caches entries, single embed per mutation (was O(n²))

All 14 packages pass tests.

Author: jkyberneees

cmd/odek/file_tool.go

@@ -106,7 +106,7 @@ func (t *readFileTool) Call(argsJSON string) (string, error) {
 		return jsonError(fmt.Sprintf("%q is a directory, not a file", args.Path))
 	}
 
-	// Check for binary content — only examine first 8KB
+	// Single pass: open → binary check from sample → seek → read+count
 	f, err := os.Open(args.Path)
 	if err != nil {
 		return jsonError(fmt.Sprintf("cannot open %q: %v", args.Path, err))
@@ -115,17 +115,16 @@ func (t *readFileTool) Call(argsJSON string) (string, error) {
 
 	sample := make([]byte, 8192)
 	n, _ := f.Read(sample)
-	sample = sample[:n]
-	if isBinary(sample) {
+	if isBinary(sample[:n]) {
 		return jsonError(fmt.Sprintf("%q appears to be a binary file — use shell to read it", args.Path))
 	}
-	f.Close() // close to re-read from start
 
-	// Count total lines first
-	totalLines := countLines(args.Path)
+	// Seek back to start for the full scan
+	if _, err := f.Seek(0, 0); err != nil {
+		return jsonError(fmt.Sprintf("cannot seek %q: %v", args.Path, err))
+	}
 
-	// Read the requested range
-	content, err := readLines(args.Path, args.Offset, args.Limit)
+	content, totalLines, err := readLinesWithCount(f, args.Offset, args.Limit)
 	if err != nil {
 		return jsonError(fmt.Sprintf("cannot read %q: %v", args.Path, err))
 	}
@@ -205,9 +204,31 @@ func (t *writeFileTool) Call(argsJSON string) (string, error) {
 		}
 	}
 
-	if err := os.WriteFile(args.Path, []byte(args.Content), 0644); err != nil {
+	// Atomic write via temp file + rename to prevent TOCTOU symlink races.
+	// os.CreateTemp creates the file in the same directory (same filesystem),
+	// and os.Rename atomically replaces the directory entry without following
+	// symlinks — closing the window between check and write.
+	tmpFile, err := os.CreateTemp(dir, ".tmp_write_*")
+	if err != nil {
+		return jsonError(fmt.Sprintf("cannot create temp file: %v", err))
+	}
+	tmpPath := tmpFile.Name()
+
+	if _, err := tmpFile.Write([]byte(args.Content)); err != nil {
+		tmpFile.Close()
+		os.Remove(tmpPath)
 		return jsonError(fmt.Sprintf("cannot write %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 rename %q: %v", args.Path, err))
+	}
 
 	return jsonResult(writeFileResult{
 		Success: true,
@@ -346,24 +367,25 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
 			}
 		}
 
-		// Skip binary files
-		sample := make([]byte, 512)
+		// Skip binary files — single open for check then search
 		f, err := os.Open(path)
 		if err != nil {
 			return nil
 		}
+		defer f.Close()
+
+		sample := make([]byte, 512)
 		n, _ := f.Read(sample)
-		f.Close()
 		if isBinary(sample[:n]) {
 			return nil
 		}
 
-		// Search line by line
-		f, err = os.Open(path)
-		if err != nil {
+		// Seek back to start for content search
+		if _, err := f.Seek(0, 0); err != nil {
 			return nil
 		}
-		defer f.Close()
+
+		// Search line by line
 
 		scanner := bufio.NewScanner(f)
 		scanner.Buffer(make([]byte, 1024*1024), 1024*1024)
@@ -601,28 +623,9 @@ func isBinary(data []byte) bool {
 	return float64(nonPrintable)/float64(len(data)) > 0.30
 }
 
-func countLines(path string) int {
-	f, err := os.Open(path)
-	if err != nil {
-		return 0
-	}
-	defer f.Close()
-	count := 0
-	scanner := bufio.NewScanner(f)
-	scanner.Buffer(make([]byte, 1024*1024), 1024*1024)
-	for scanner.Scan() {
-		count++
-	}
-	return count
-}
-
-func readLines(path string, offset, limit int) (string, error) {
-	f, err := os.Open(path)
-	if err != nil {
-		return "", err
-	}
-	defer f.Close()
-
+// readLinesWithCount reads lines from an open file, returning content
+// and total line count in a single pass. offset is 1-based, limit caps lines.
+func readLinesWithCount(f *os.File, offset, limit int) (string, int, error) {
 	var out strings.Builder
 	scanner := bufio.NewScanner(f)
 	scanner.Buffer(make([]byte, 1024*1024), 1024*1024)
@@ -636,12 +639,19 @@ func readLines(path string, offset, limit int) (string, error) {
 			continue
 		}
 		if lineNum > end {
-			break
+			continue // count total even beyond limit
 		}
 		out.WriteString(fmt.Sprintf("%d|%s\n", lineNum, scanner.Text()))
 	}
 
-	return strings.TrimSuffix(out.String(), "\n"), scanner.Err()
+	// If no limit was set (limit=0), continue counting past start
+	if limit > 0 {
+		for scanner.Scan() {
+			lineNum++
+		}
+	}
+
+	return strings.TrimSuffix(out.String(), "\n"), lineNum, scanner.Err()
 }
 
 func truncateDiff(s string, maxLen int) string {

cmd/odek/file_tool_test.go

@@ -617,3 +617,104 @@ func containsStr(slice []string, item string) bool {
 	}
 	return false
 }
+
+// TestWriteFile_TOCTOU_SymlinkRejected verifies that write_file does NOT
+// follow symlinks (security: TOCTOU race prevention).
+// This test writes to a temp path, then replaces it with a symlink
+// to a protected file, and verifies the tool refuses or replaces the symlink.
+func TestWriteFile_TOCTOU_SymlinkRejected(t *testing.T) {
+	dir := t.TempDir()
+	targetPath := filepath.Join(dir, "target.txt")
+	protectedPath := filepath.Join(dir, "protected.txt")
+
+	// Create the protected file (what attacker wants us to overwrite)
+	if err := os.WriteFile(protectedPath, []byte("sensitive data"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	// Create target then replace with symlink to protected file
+	if err := os.WriteFile(targetPath, []byte("original"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	os.Remove(targetPath)
+	if err := os.Symlink(protectedPath, targetPath); err != nil {
+		t.Fatal(err)
+	}
+
+	// Try to write to targetPath — if the tool follows the symlink,
+	// protected.txt gets overwritten. Our fix should prevent this.
+	tool := &writeFileTool{}
+	result := callJSON(t, tool, `{"path":"`+targetPath+`","content":"overwritten!"}`)
+
+	// After the write, protected.txt should still be intact
+	data, _ := os.ReadFile(protectedPath)
+	if string(data) != "sensitive data" {
+		t.Errorf("protected file was overwritten! Got: %q", string(data))
+	}
+
+	// The call should have succeeded (it writes to a temp file + renames,
+	// which replaces the symlink entry not the target)
+	_ = result
+}
+
+// TestReadFile_CountAndContentSinglePass verifies that readLinesWithCount
+// returns both content and total line count in one pass.
+func TestReadFile_CountAndContentSinglePass(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "count.txt")
+	content := "line1\nline2\nline3\nline4\nline5\n"
+	if err := os.WriteFile(path, []byte(content), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := os.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	gotContent, totalLines, err := readLinesWithCount(f, 1, 3)
+	if err != nil {
+		t.Fatalf("readLinesWithCount error: %v", err)
+	}
+	if totalLines != 5 {
+		t.Errorf("totalLines = %d, want 5", totalLines)
+	}
+	if !strings.Contains(gotContent, "1|line1") || !strings.Contains(gotContent, "3|line3") {
+		t.Errorf("gotContent missing expected lines: %s", gotContent)
+	}
+}
+
+// defaultTestDangerousConfig returns a permissive DangerousConfig for tests.
+func defaultTestDangerousConfig() DangerConfig {
+	return DangerConfig{
+		Action: "allow",
+		NonInteractive: "allow",
+		Classes: map[RiskClass]string{
+			RiskClassDestructive:    "allow",
+			RiskClassNetworkEgress:  "allow",
+			RiskClassCodeExecution:  "allow",
+			RiskClassInstall:        "allow",
+			RiskClassSystemWrite:    "allow",
+		},
+	}
+}
+
+// DangerConfig matches the danger.DangerousConfig structure for test use.
+type DangerConfig struct {
+	Action         string                 `json:"action"`
+	NonInteractive string                 `json:"non_interactive"`
+	Classes        map[RiskClass]string   `json:"classes"`
+	Allowlist      []string               `json:"allowlist,omitempty"`
+	Denylist       []string               `json:"denylist,omitempty"`
+}
+
+type RiskClass string
+
+const (
+	RiskClassDestructive   RiskClass = "destructive"
+	RiskClassNetworkEgress RiskClass = "network_egress"
+	RiskClassCodeExecution RiskClass = "code_execution"
+	RiskClassInstall       RiskClass = "install"
+	RiskClassSystemWrite   RiskClass = "system_write"
+)

cmd/odek/repl.go

@@ -2,7 +2,9 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/signal"
 	"path/filepath"
@@ -186,7 +188,7 @@ func replCmd(args []string) error {
 
 		input, err := editor.ReadLine()
 		if err != nil {
-			if err.Error() == "EOF" || err.Error() == "interrupt" {
+			if errors.Is(err, io.EOF) || err.Error() == "interrupt" {
 				fmt.Fprintf(os.Stderr, "\n")
 				break
 			}

internal/loop/loop.go

@@ -234,7 +234,12 @@ func (e *Engine) runLoop(ctx context.Context, messages []llm.Message) (string, [
 						}
 					}
 					skillMsg := llm.Message{Role: "system", Content: "# Relevant Skill\n\n" + skillContext}
-					messages = append(messages[:insertIdx], append([]llm.Message{skillMsg}, messages[insertIdx:]...)...)
+					// Pre-allocate and copy to avoid nested append allocations
+					newMsgs := make([]llm.Message, 0, len(messages)+1)
+					newMsgs = append(newMsgs, messages[:insertIdx]...)
+					newMsgs = append(newMsgs, skillMsg)
+					newMsgs = append(newMsgs, messages[insertIdx:]...)
+					messages = newMsgs
 				}
 			}
 		}

internal/mcpclient/client.go

@@ -134,6 +134,12 @@ type ServerConfig struct {
 	Env map[string]string `json:"env,omitempty"`
 }
 
+// lineResult carries the result of a single readLine from the reader goroutine.
+type lineResult struct {
+	line string
+	err  error
+}
+
 // ── Client ─────────────────────────────────────────────────────────────
 
 // Client manages a connection to an external MCP server over stdio.
@@ -142,7 +148,8 @@ type Client struct {
 	cmd    *exec.Cmd
 	stdin  io.WriteCloser
 	stdout *bufio.Reader
-	done   chan struct{} // closed when process exits
+	lineCh chan lineResult // single-reader goroutine sends lines here
+	done   chan struct{}   // closed when process exits
 	mu     sync.Mutex
 	nextID int
 }
@@ -181,9 +188,13 @@ func New(name string, cfg ServerConfig) (*Client, error) {
 		cmd:    cmd,
 		stdin:  stdin,
 		stdout: bufio.NewReader(stdout),
+		lineCh: make(chan lineResult, 10),
 		done:   make(chan struct{}),
 	}
 
+	// Start single-reader goroutine
+	go c.readLoop()
+
 	// Monitor process exit in background
 	go func() {
 		cmd.Wait()
@@ -381,23 +392,39 @@ func (c *Client) call(ctx context.Context, method string, params json.RawMessage
 	}
 }
 
-// readLine reads a single line with context-based timeout.
-func (c *Client) readLine(ctx context.Context) (string, error) {
-	type lineResult struct {
-		line string
-		err  error
+// readLoop is a single reader goroutine that reads lines from stdout and
+// sends them on lineCh. It exits when stdout returns an error (EOF on pipe close).
+func (c *Client) readLoop() {
+	defer close(c.lineCh)
+	for {
+		line, err := c.stdout.ReadString('\n')
+		select {
+		case c.lineCh <- lineResult{strings.TrimSpace(line), err}:
+		default:
+			// Channel full — drain the queue by consuming one stale entry,
+			// then retry. This prevents a stuck server from deadlocking the
+			// reader goroutine.
+			<-c.lineCh
+			c.lineCh <- lineResult{strings.TrimSpace(line), err}
+		}
+		if err != nil {
+			return
+		}
 	}
+}
 
-	ch := make(chan lineResult, 1)
-	go func() {
-		l, err := c.stdout.ReadString('\n')
-		ch <- lineResult{strings.TrimSpace(l), err}
-	}()
-
+// readLine reads a single line with context-based timeout.
+// Uses the single-reader goroutine (readLoop) so no goroutine leaks on context
+// cancellation — the goroutine is owned by the connection, not the RPC call.
+func (c *Client) readLine(ctx context.Context) (string, error) {
 	select {
 	case <-ctx.Done():
 		return "", ctx.Err()
-	case r := <-ch:
+	case r, ok := <-c.lineCh:
+		if !ok {
+			// Channel closed — reader goroutine exited (process gone)
+			return "", io.EOF
+		}
 		return r.line, r.err
 	}
 }