chore: fix bugs

Author: cameronrye

apple/Clarissa/Sources/App/ClarissaApp.swift

@@ -24,13 +24,12 @@ final class MacAppDelegate: NSObject, NSApplicationDelegate {
 
 #if os(iOS)
 import BackgroundTasks
-import CarPlay
 import UIKit
 
 /// Background task identifier for memory sync
 private let backgroundMemorySyncTaskId = "dev.rye.Clarissa.memorySync"
 
-/// App delegate to handle CarPlay scene configuration and background tasks
+/// App delegate to handle background tasks
 final class AppDelegate: NSObject, UIApplicationDelegate {
     func application(
         _ application: UIApplication,
@@ -41,28 +40,6 @@ final class AppDelegate: NSObject, UIApplicationDelegate {
         return true
     }
 
-    func application(
-        _ application: UIApplication,
-        configurationForConnecting connectingSceneSession: UISceneSession,
-        options: UIScene.ConnectionOptions
-    ) -> UISceneConfiguration {
-        // Check if this is a CarPlay scene
-        if connectingSceneSession.role == .carTemplateApplication {
-            let config = UISceneConfiguration(
-                name: "CarPlay Configuration",
-                sessionRole: .carTemplateApplication
-            )
-            config.delegateClass = CarPlaySceneDelegate.self
-            return config
-        }
-
-        // Default configuration - SwiftUI handles the main app scene via WindowGroup
-        return UISceneConfiguration(
-            name: "Default Configuration",
-            sessionRole: connectingSceneSession.role
-        )
-    }
-
     // MARK: - Background Tasks
 
     private func registerBackgroundTasks() {

src/context/file-references.ts

@@ -11,7 +11,7 @@
  *   @package.json:1-50      -> Lines 1-50 only
  */
 
-import { resolve, isAbsolute } from "path";
+import { validatePathWithinBase } from "../tools/security.ts";
 
 /**
  * Result of expanding file references
@@ -111,10 +111,11 @@ export async function expandFileReferences(
 
     const { path: filePath, startLine, endLine } = parseReference(reference);
 
-    // Resolve path
-    const absolutePath = isAbsolute(filePath) ? filePath : resolve(cwd, filePath);
-
     try {
+      // Security: validate path is within the working directory to prevent path traversal
+      // This blocks attempts like @/etc/passwd or @../../../etc/passwd
+      const absolutePath = validatePathWithinBase(filePath, cwd);
+
       const content = await readFileContents(absolutePath, startLine, endLine);
       const lineInfo = startLine ? `:${startLine}${endLine && endLine !== startLine ? `-${endLine}` : ""}` : "";
 

src/llm/context.ts

@@ -155,28 +155,25 @@ class ContextManager {
     result.push(...systemMessages);
     totalTokens = this.estimateConversationTokens(systemMessages);
 
-    // Group messages into atomic units (user, assistant+tool_results, etc.)
-    // Tool results must stay with their corresponding assistant message
+    // Group messages into atomic units that must stay together:
+    // - User message starts a new group
+    // - Assistant messages and tool results stay with their preceding user message
+    // This ensures tool_calls and their results are never separated
     const messageGroups: Message[][] = [];
     let currentGroup: Message[] = [];
 
     for (const msg of nonSystemMessages) {
       if (msg.role === "user") {
-        // User messages start a new group
+        // User messages always start a new group
         if (currentGroup.length > 0) {
           messageGroups.push(currentGroup);
         }
         currentGroup = [msg];
-      } else if (msg.role === "assistant") {
-        // Assistant messages start a new group (but include in current if empty)
-        if (currentGroup.length > 0 && currentGroup[0]?.role !== "user") {
-          messageGroups.push(currentGroup);
-          currentGroup = [msg];
-        } else {
-          currentGroup.push(msg);
-        }
-      } else if (msg.role === "tool") {
-        // Tool results must stay with their assistant message
+      } else if (currentGroup.length === 0) {
+        // Edge case: assistant/tool message without preceding user message
+        currentGroup = [msg];
+      } else {
+        // Assistant and tool messages stay with current group
         currentGroup.push(msg);
       }
     }

src/llm/usage.ts

@@ -75,11 +75,39 @@ class UsageTracker {
   }
 
   /**
-   * Estimate tokens from text (rough approximation)
-   * ~4 characters per token for English text
+   * Estimate tokens from text using improved heuristics.
+   * Based on analysis of BPE tokenization patterns:
+   * - Common English words: ~1 token per word
+   * - Numbers and punctuation: often separate tokens
+   * - Code: more tokens due to identifiers and symbols
+   * - Whitespace: typically not counted
    */
   estimateTokens(text: string): number {
-    return Math.ceil(text.length / 4);
+    if (!text) return 0;
+
+    // Split into words (tokens often align with word boundaries)
+    const words = text.split(/\s+/).filter((w) => w.length > 0);
+
+    // Base: count words
+    let tokens = words.length;
+
+    // Add tokens for long words (they get split by BPE)
+    // Average token length is ~4 chars, so words > 8 chars likely become 2+ tokens
+    for (const word of words) {
+      if (word.length > 8) {
+        tokens += Math.floor(word.length / 6);
+      }
+    }
+
+    // Add tokens for punctuation and special characters
+    // These often become separate tokens
+    const specialChars = text.match(/[^\w\s]/g);
+    if (specialChars) {
+      tokens += Math.ceil(specialChars.length * 0.5);
+    }
+
+    // Add overhead for message structure (~4 tokens per message for role, separators)
+    return Math.max(1, tokens);
   }
 
   /**

src/mcp/client.ts

@@ -1,7 +1,6 @@
 import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
 import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
-import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js";
 import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
 import type { Tool } from "../tools/base.ts";
 import { z } from "zod";
@@ -111,6 +110,28 @@ export interface MCPServerSseConfigInternal {
 
 export type MCPServerConfig = MCPServerStdioConfig | MCPServerSseConfigInternal;
 
+/**
+ * Maximum length for MCP tool results to prevent context overflow
+ */
+const MAX_RESULT_LENGTH = 50000;
+
+/**
+ * Sanitize MCP tool result to prevent prompt injection and limit size
+ * MCP servers are external and untrusted, so we need to be careful with their output
+ */
+function sanitizeMcpResult(result: string, serverName: string, toolName: string): string {
+  let sanitized = result;
+
+  // Truncate if too long
+  if (sanitized.length > MAX_RESULT_LENGTH) {
+    sanitized = sanitized.slice(0, MAX_RESULT_LENGTH) + `\n[Truncated - result exceeded ${MAX_RESULT_LENGTH} characters]`;
+  }
+
+  // Wrap in clear delimiters to indicate this is external tool output
+  // This helps the model understand the boundary of untrusted content
+  return `<mcp_result server="${serverName}" tool="${toolName}">\n${sanitized}\n</mcp_result>`;
+}
+
 interface MCPConnection {
   client: Client;
   transport: Transport;
@@ -123,6 +144,7 @@ interface MCPConnection {
  */
 class MCPClientManager {
   private connections: Map<string, MCPConnection> = new Map();
+  private cleanupHandlersRegistered = false;
 
   /**
    * Connect to an MCP server (stdio or SSE transport)
@@ -136,18 +158,14 @@ class MCPClientManager {
 
     if (config.transport === "sse") {
       // Remote HTTP/SSE server
-      // Try StreamableHTTPClientTransport first, fallback to SSEClientTransport for legacy servers
+      // Use StreamableHTTPClientTransport (modern MCP transport)
+      // Note: SSEClientTransport is available for legacy servers but requires explicit config
       const url = new URL(config.url);
       const requestInit: RequestInit = config.headers
         ? { headers: config.headers }
         : {};
 
-      try {
-        transport = new StreamableHTTPClientTransport(url, { requestInit });
-      } catch {
-        // Fallback to legacy SSE transport
-        transport = new SSEClientTransport(url, { requestInit });
-      }
+      transport = new StreamableHTTPClientTransport(url, { requestInit });
     } else {
       // Local stdio process (default)
       transport = new StdioClientTransport({
@@ -199,16 +217,20 @@ class MCPClientManager {
         });
 
         // Handle different result types
+        let rawResult: string;
         if (result.content && Array.isArray(result.content)) {
-          return result.content
+          rawResult = result.content
             .map((c) => {
               if (c.type === "text") return c.text;
               return JSON.stringify(c);
             })
             .join("\n");
+        } else {
+          rawResult = JSON.stringify(result);
         }
 
-        return JSON.stringify(result);
+        // Sanitize the result to prevent prompt injection and limit size
+        return sanitizeMcpResult(rawResult, serverName, mcpTool.name);
       },
     };
   }
@@ -326,8 +348,13 @@ class MCPClientManager {
 
   /**
    * Register cleanup handlers for process exit
+   * Only registers once to prevent memory leaks from duplicate listeners
    */
   registerCleanupHandlers(): void {
+    // Prevent registering duplicate handlers (memory leak)
+    if (this.cleanupHandlersRegistered) return;
+    this.cleanupHandlersRegistered = true;
+
     let isCleaningUp = false;
 
     const cleanup = async (): Promise<void> => {

src/session/index.ts

@@ -1,6 +1,6 @@
 import { join } from "path";
 import { homedir } from "os";
-import { mkdir, readdir, rm } from "fs/promises";
+import { mkdir, readdir, rm, rename } from "fs/promises";
 import { z } from "zod";
 import type { Message } from "../llm/types.ts";
 
@@ -97,7 +97,8 @@ class SessionManager {
   }
 
   /**
-   * Save current session to disk
+   * Save current session to disk using atomic write
+   * Writes to temp file first, then renames to prevent corruption from concurrent saves
    */
   async save(): Promise<void> {
     if (!this.currentSession) return;
@@ -106,7 +107,11 @@ class SessionManager {
     this.currentSession.updatedAt = new Date().toISOString();
 
     const path = this.getPath(this.currentSession.id);
-    await Bun.write(path, JSON.stringify(this.currentSession, null, 2));
+    const tempPath = `${path}.tmp.${Date.now()}`;
+
+    // Write to temp file first, then atomically rename
+    await Bun.write(tempPath, JSON.stringify(this.currentSession, null, 2));
+    await rename(tempPath, path);
   }
 
   /**

src/tools/bash.ts

@@ -5,25 +5,75 @@ import { defineTool } from "./base.ts";
  * Dangerous command patterns that should be blocked for safety.
  * These patterns match commands that could cause severe system damage.
  */
-const DANGEROUS_PATTERNS = [
-  // Recursive deletion of root or important directories
-  /rm\s+(-[a-zA-Z]*f[a-zA-Z]*\s+)?(-[a-zA-Z]*r[a-zA-Z]*\s+)?[\/~]\s*$/i,
-  /rm\s+(-[a-zA-Z]*r[a-zA-Z]*\s+)?(-[a-zA-Z]*f[a-zA-Z]*\s+)?[\/~]\s*$/i,
-  /rm\s+-rf\s+\/\s*$/i,
-  /rm\s+-rf\s+\/\*/i,
+const DANGEROUS_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [
+  // Recursive deletion of root, home, or current directory
+  // Matches: rm -rf /, rm -rf ~, rm -rf ./, rm -rf ., etc.
+  {
+    pattern: /rm\s+(-[a-zA-Z]*\s+)*["']?([/~]|\.\.?\/?)["']?\s*$/i,
+    reason: "Recursive deletion of root, home, or current directory",
+  },
+  {
+    pattern: /rm\s+(-[a-zA-Z]*\s+)*["']?\/\*["']?/i,
+    reason: "Recursive deletion of root contents",
+  },
+  // rm -rf * or rm -rf ./* (deletes everything in current directory)
+  // Matches rm with -r flag (recursive) followed by wildcard
+  {
+    pattern: /rm\s+(-\w+\s+)*["']?\*["']?\s*$/i,
+    reason: "Deletion with wildcard - potentially dangerous",
+  },
   // Fork bomb patterns
-  /:\s*\(\s*\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;?\s*:/,
+  {
+    pattern: /:\s*\(\s*\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;?\s*:/,
+    reason: "Fork bomb detected",
+  },
   // Overwriting boot records or critical system files
-  />\s*\/dev\/sd[a-z]/i,
-  /dd\s+.*of=\/dev\/sd[a-z]/i,
-  /mkfs\s+.*\/dev\/sd[a-z]/i,
-  // Chmod 777 on root
-  /chmod\s+(-[a-zA-Z]*R[a-zA-Z]*\s+)?777\s+\/\s*$/i,
-  // Dangerous redirects
-  />\s*\/dev\/null\s*2>&1\s*<\s*\/dev\/null/,
+  {
+    pattern: />\s*\/dev\/sd[a-z]/i,
+    reason: "Writing to block device",
+  },
+  {
+    pattern: /dd\s+.*of=\/dev\/sd[a-z]/i,
+    reason: "dd to block device",
+  },
+  {
+    pattern: /mkfs\s+.*\/dev\/sd[a-z]/i,
+    reason: "Formatting block device",
+  },
+  // Chmod 777 on root or recursive on sensitive paths
+  {
+    pattern: /chmod\s+(-[a-zA-Z]*\s+)*777\s+["']?[/~]["']?\s*$/i,
+    reason: "chmod 777 on root or home directory",
+  },
+  // Dangerous redirects that could hang the shell
+  {
+    pattern: />\s*\/dev\/null\s*2>&1\s*<\s*\/dev\/null/,
+    reason: "Dangerous redirect pattern",
+  },
   // Kill all processes
-  /kill\s+-9\s+-1/,
-  /killall\s+-9\s+/,
+  {
+    pattern: /kill\s+-9\s+-1/,
+    reason: "Killing all processes",
+  },
+  {
+    pattern: /killall\s+-9\s+/,
+    reason: "Killing all processes by name",
+  },
+  // Prevent sudo with dangerous commands
+  {
+    pattern: /sudo\s+rm\s+(-[a-zA-Z]*\s+)*["']?[/~]["']?\s*$/i,
+    reason: "sudo rm on root or home directory",
+  },
+  // Prevent overwriting /etc/passwd, /etc/shadow, etc.
+  {
+    pattern: />\s*\/etc\/(passwd|shadow|sudoers)/i,
+    reason: "Overwriting critical system file",
+  },
+  // Prevent writing to /boot or /sys
+  {
+    pattern: />\s*\/(boot|sys)\//i,
+    reason: "Writing to critical system directory",
+  },
 ];
 
 /**
@@ -32,11 +82,11 @@ const DANGEROUS_PATTERNS = [
 function isDangerousCommand(command: string): { dangerous: boolean; reason?: string } {
   const trimmed = command.trim();
 
-  for (const pattern of DANGEROUS_PATTERNS) {
+  for (const { pattern, reason } of DANGEROUS_PATTERNS) {
     if (pattern.test(trimmed)) {
       return {
         dangerous: true,
-        reason: `Command matches dangerous pattern: ${pattern.toString()}`,
+        reason,
       };
     }
   }