fix: Prevent security vulnerability from malformed app targets

Addresses critical edge case where malformed app targets with multiple leading colons
(e.g., "::::::::::::::::Finder") created empty app names that would match ALL system
processes. This could potentially expose sensitive information or cause unintended
system-wide captures.

Key improvements:
- Enhanced app target parsing to validate non-empty app names
- Added fallback logic to extract valid app names from malformed inputs
- Default to screen mode when all parts are empty (security-first approach)
- Comprehensive test coverage for edge cases
- Improved backward compatibility with hidden path parameters

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

peekaboo-cli/Sources/peekaboo/FileNameGenerator.swift

@@ -0,0 +1,38 @@
+import Foundation
+
+struct FileNameGenerator {
+    static func generateFileName(
+        displayIndex: Int? = nil,
+        appName: String? = nil,
+        windowIndex: Int? = nil,
+        windowTitle: String? = nil,
+        format: ImageFormat
+    ) -> String {
+        let timestamp = DateFormatter.timestamp.string(from: Date())
+        let ext = format.rawValue
+
+        if let displayIndex {
+            return "screen_\(displayIndex + 1)_\(timestamp).\(ext)"
+        } else if let appName {
+            let cleanAppName = appName.replacingOccurrences(of: " ", with: "_")
+            if let windowIndex {
+                return "\(cleanAppName)_window_\(windowIndex)_\(timestamp).\(ext)"
+            } else if let windowTitle {
+                let cleanTitle = windowTitle.replacingOccurrences(of: " ", with: "_").prefix(20)
+                return "\(cleanAppName)_\(cleanTitle)_\(timestamp).\(ext)"
+            } else {
+                return "\(cleanAppName)_\(timestamp).\(ext)"
+            }
+        } else {
+            return "capture_\(timestamp).\(ext)"
+        }
+    }
+}
+
+extension DateFormatter {
+    static let timestamp: DateFormatter = {
+        let formatter = DateFormatter()
+        formatter.dateFormat = "yyyyMMdd_HHmmss"
+        return formatter
+    }()
+}

peekaboo-cli/Sources/peekaboo/ImageSaver.swift

@@ -0,0 +1,47 @@
+import Foundation
+import CoreGraphics
+import ImageIO
+import UniformTypeIdentifiers
+
+struct ImageSaver {
+    static func saveImage(_ image: CGImage, to path: String, format: ImageFormat) throws(CaptureError) {
+        let url = URL(fileURLWithPath: path)
+
+        // Check if the parent directory exists
+        let directory = url.deletingLastPathComponent()
+        var isDirectory: ObjCBool = false
+        if !FileManager.default.fileExists(atPath: directory.path, isDirectory: &isDirectory) {
+            let error = NSError(
+                domain: NSCocoaErrorDomain,
+                code: NSFileNoSuchFileError,
+                userInfo: [NSLocalizedDescriptionKey: "No such file or directory"]
+            )
+            throw CaptureError.fileWriteError(path, error)
+        }
+
+        let utType: UTType = format == .png ? .png : .jpeg
+        guard let destination = CGImageDestinationCreateWithURL(
+            url as CFURL,
+            utType.identifier as CFString,
+            1,
+            nil
+        ) else {
+            // Try to create a more specific error for common cases
+            if !FileManager.default.isWritableFile(atPath: directory.path) {
+                let error = NSError(
+                    domain: NSPOSIXErrorDomain,
+                    code: Int(EACCES),
+                    userInfo: [NSLocalizedDescriptionKey: "Permission denied"]
+                )
+                throw CaptureError.fileWriteError(path, error)
+            }
+            throw CaptureError.fileWriteError(path, nil)
+        }
+
+        CGImageDestinationAddImage(destination, image, nil)
+
+        guard CGImageDestinationFinalize(destination) else {
+            throw CaptureError.fileWriteError(path, nil)
+        }
+    }
+}

peekaboo-cli/Sources/peekaboo/PermissionErrorDetector.swift

@@ -0,0 +1,35 @@
+import Foundation
+
+struct PermissionErrorDetector {
+    static func isScreenRecordingPermissionError(_ error: Error) -> Bool {
+        let errorString = error.localizedDescription.lowercased()
+
+        // Check for specific screen recording related errors
+        if errorString.contains("screen recording") {
+            return true
+        }
+
+        // Check for NSError codes specific to screen capture permissions
+        if let nsError = error as NSError? {
+            // ScreenCaptureKit specific error codes
+            if nsError.domain == "com.apple.screencapturekit" && nsError.code == -3801 {
+                // SCStreamErrorUserDeclined = -3801
+                return true
+            }
+
+            // CoreGraphics error codes for screen capture
+            if nsError.domain == "com.apple.coregraphics" && nsError.code == 1002 {
+                // kCGErrorCannotComplete when permissions are denied
+                return true
+            }
+        }
+
+        // Only consider it a permission error if it mentions both "permission" and capture-related terms
+        if errorString.contains("permission") &&
+            (errorString.contains("capture") || errorString.contains("recording") || errorString.contains("screen")) {
+            return true
+        }
+
+        return false
+    }
+}

src/tools/analyze.ts

@@ -37,17 +37,22 @@ export const analyzeToolSchema = z.object({
     .describe(
       "Optional. Explicit provider/model. Validated against server's PEEKABOO_AI_PROVIDERS.",
     ),
-  // Silent fallback parameter (not advertised in schema)
-  path: z.string().optional(),
-}).refine(
-  (data) => data.image_path || data.path,
-  {
-    message: "image_path is required",
-    path: ["image_path"],
-  },
-);
+})
+  .passthrough() // Allow unknown properties (for the hidden `path` parameter)
+  .refine(
+    (data: unknown) => {
+      const typedData = data as { image_path?: string; path?: string };
+      return typedData.image_path || typedData.path;
+    },
+    {
+      message: "image_path is required",
+      path: ["image_path"],
+    },
+  );
 
-export type AnalyzeToolInput = z.infer<typeof analyzeToolSchema>;
+export type AnalyzeToolInput = z.infer<typeof analyzeToolSchema> & {
+  path?: string; // Hidden parameter for backward compatibility
+};
 
 export async function analyzeToolHandler(
   input: AnalyzeToolInput,

src/tools/image.ts

@@ -43,7 +43,7 @@ export async function imageToolHandler(
     if (effectiveFormat && !validFormats.includes(effectiveFormat)) {
       logger.warn(
         { originalFormat: effectiveFormat, fallbackFormat: "png" },
-        `Invalid format '${effectiveFormat}' detected, falling back to PNG`
+        `Invalid format '${effectiveFormat}' detected, falling back to PNG`,
       );
       effectiveFormat = "png";
       formatWarning = `Invalid format '${input.format}' was provided. Automatically using PNG format instead.`;
@@ -61,28 +61,28 @@ export async function imageToolHandler(
 
     // Create a corrected input object if format or path needs to be adjusted
     let correctedInput = input;
-    
+
     // If format was corrected and we have a path, update the file extension to match the actual format
     if (input.format && input.format !== effectiveFormat && input.path) {
       const originalPath = input.path;
       const parsedPath = path.parse(originalPath);
-      
+
       // Map format to appropriate extension
       const extensionMap: { [key: string]: string } = {
         "png": ".png",
-        "jpg": ".jpg", 
+        "jpg": ".jpg",
         "jpeg": ".jpg",
-        "data": ".png" // data format saves as PNG
+        "data": ".png", // data format saves as PNG
       };
-      
+
       const newExtension = extensionMap[effectiveFormat || "png"] || ".png";
       const correctedPath = path.join(parsedPath.dir, parsedPath.name + newExtension);
-      
+
       logger.debug(
         { originalPath, correctedPath, originalFormat: input.format, correctedFormat: effectiveFormat },
-        "Correcting file extension to match format"
+        "Correcting file extension to match format",
       );
-      
+
       correctedInput = { ...input, path: correctedPath };
     }
 
@@ -297,7 +297,7 @@ export async function imageToolHandler(
     }
     if (!analysisSucceeded && analysisAttempted) {
       result.isError = true;
-      result._meta = { ...result._meta, analysis_error: analysisText };
+      result._meta = { ...(result._meta || {}), analysis_error: analysisText };
     }
 
     return result;

src/tools/list.ts

@@ -43,12 +43,12 @@ export const listToolSchema = z
         if (val === "" || val === null || val === undefined) {
           return undefined;
         }
-        
+
         // If it's already an array, return as-is
         if (Array.isArray(val)) {
           return val;
         }
-        
+
         // If it's a string that looks like JSON, try to parse it
         if (typeof val === "string") {
           try {
@@ -59,16 +59,16 @@ export const listToolSchema = z
           } catch {
             // Not valid JSON, treat as single item
           }
-          
+
           // If it's a comma-separated string, split it
           if (val.includes(",")) {
             return val.split(",").map(s => s.trim());
           }
-          
+
           // Single string value, wrap in array
           return [val.trim()];
         }
-        
+
         return val;
       },
       z

src/utils/image-cli-args.ts

@@ -81,7 +81,7 @@ export function buildSwiftCliArgs(
     } else {
       args.push("--mode", "screen", "--screen-index", screenIndex.toString());
     }
-  } else if (input.app_target === "frontmost") {
+  } else if (input.app_target.toLowerCase() === "frontmost") {
     // 'frontmost': All windows of the frontmost app
     log.warn(
       "'frontmost' target requires determining current frontmost app, defaulting to screen mode",
@@ -91,31 +91,60 @@ export function buildSwiftCliArgs(
     // 'AppName:WINDOW_TITLE:Title' or 'AppName:WINDOW_INDEX:Index'
     const parts = input.app_target.split(":");
     if (parts.length >= 3) {
-      const appName = parts[0];
-      const specifierType = parts[1];
+      const appName = parts[0].trim();
+      const specifierType = parts[1].trim();
       const specifierValue = parts.slice(2).join(":"); // Handle colons in window titles
 
-      args.push("--app", appName.trim());
-      args.push("--mode", "window");
-
-      if (specifierType === "WINDOW_TITLE") {
-        args.push("--window-title", specifierValue);
-      } else if (specifierType === "WINDOW_INDEX") {
-        args.push("--window-index", specifierValue);
-      } else {
+      // Validate that we have a non-empty app name
+      if (!appName) {
         log.warn(
-          { specifierType },
-          "Unknown window specifier type, defaulting to main window",
+          { app_target: input.app_target },
+          "Empty app name detected in app_target, treating as malformed",
         );
+        // Try to find the first non-empty part as the app name
+        const nonEmptyParts = parts.filter(part => part.trim());
+        if (nonEmptyParts.length > 0) {
+          args.push("--app", nonEmptyParts[0].trim());
+          args.push("--mode", "multi");
+        } else {
+          // All parts are empty, default to screen mode
+          log.warn("All parts of app_target are empty, defaulting to screen mode");
+          args.push("--mode", "screen");
+        }
+      } else {
+        args.push("--app", appName);
+        args.push("--mode", "window");
+
+        if (specifierType.toUpperCase() === "WINDOW_TITLE") {
+          args.push("--window-title", specifierValue);
+        } else if (specifierType.toUpperCase() === "WINDOW_INDEX") {
+          args.push("--window-index", specifierValue);
+        } else {
+          log.warn(
+            { specifierType },
+            "Unknown window specifier type, defaulting to main window",
+          );
+        }
       }
     } else {
-      // Malformed: treat as app name
-      log.warn(
-        { app_target: input.app_target },
-        "Malformed window specifier, treating as app name",
-      );
-      args.push("--app", input.app_target.trim());
-      args.push("--mode", "multi");
+      // Malformed: treat as app name, but validate it's not empty
+      const cleanAppTarget = input.app_target.trim();
+      if (!cleanAppTarget || cleanAppTarget === ":".repeat(cleanAppTarget.length)) {
+        log.warn(
+          { app_target: input.app_target },
+          "Malformed app_target with only colons or empty, defaulting to screen mode",
+        );
+        args.push("--mode", "screen");
+      } else {
+        log.warn(
+          { app_target: input.app_target },
+          "Malformed window specifier, treating as app name",
+        );
+        // Remove trailing colons from app name
+        const appName = cleanAppTarget.replace(/:+$/, "");
+        args.push("--app", appName);
+        args.push("--mode", "multi");
+      }
     }
   } else {
     // 'AppName': All windows of that app

tests/unit/tools/analyze.test.ts

@@ -645,5 +645,51 @@ describe("Analyze Tool", () => {
       expect(analyzeToolSchema.safeParse(inputWithoutProvider).success).toBe(true);
       expect(analyzeToolSchema.safeParse(inputWithProvider).success).toBe(true);
     });
+
+    it("should accept hidden path parameter for backward compatibility", () => {
+      // Test that the hidden 'path' parameter is accepted and works
+      const inputWithPath = {
+        path: "/path/to/image.png",
+        question: "What is this?",
+      };
+      
+      const inputWithBothPaths = {
+        image_path: "/path/to/primary.png",
+        path: "/path/to/fallback.png",
+        question: "What is this?",
+      };
+      
+      // Both should pass validation
+      expect(analyzeToolSchema.safeParse(inputWithPath).success).toBe(true);
+      expect(analyzeToolSchema.safeParse(inputWithBothPaths).success).toBe(true);
+      
+      // When both are provided, image_path should take precedence
+      const parsedWithBoth = analyzeToolSchema.parse(inputWithBothPaths);
+      expect(parsedWithBoth.image_path).toBe("/path/to/primary.png");
+      expect((parsedWithBoth as any).path).toBe("/path/to/fallback.png");
+    });
+
+    it("should handle hidden path parameter in tool handler", async () => {
+      process.env.PEEKABOO_AI_PROVIDERS = "ollama/llava";
+      mockParseAIProviders.mockReturnValue([
+        { provider: "ollama", model: "llava" },
+      ]);
+      mockDetermineProviderAndModel.mockResolvedValue({
+        provider: "ollama",
+        model: "llava",
+      });
+      mockAnalyzeImageWithProvider.mockResolvedValue("Analysis complete");
+
+      // Test with only path parameter (should use fallback)
+      const inputWithOnlyPath: any = {
+        path: "/path/to/fallback.png",
+        question: "What is this?",
+      };
+
+      const result = await analyzeToolHandler(inputWithOnlyPath, mockContext);
+
+      expect(mockReadImageAsBase64).toHaveBeenCalledWith("/path/to/fallback.png");
+      expect(result.isError).toBeUndefined();
+    });
   });
 });

tests/unit/tools/image.test.ts

@@ -1044,6 +1044,48 @@ describe("Image Tool", () => {
       expect(loggerWarnSpy).toHaveBeenCalled();
     });
 
+    it("should handle app_target: 'frontmost' case-insensitively", () => {
+      const loggerWarnSpy = vi.spyOn(mockLogger, "warn");
+      
+      // Test uppercase
+      const argsUpper = buildSwiftCliArgs({ app_target: "FRONTMOST" }, undefined, undefined, mockLogger);
+      expect(argsUpper).toEqual(
+        expect.arrayContaining(["--mode", "screen"]),
+      );
+      expect(argsUpper).not.toContain("--app");
+      
+      // Test mixed case
+      const argsMixed = buildSwiftCliArgs({ app_target: "Frontmost" }, undefined, undefined, mockLogger);
+      expect(argsMixed).toEqual(
+        expect.arrayContaining(["--mode", "screen"]),
+      );
+      expect(argsMixed).not.toContain("--app");
+      
+      expect(loggerWarnSpy).toHaveBeenCalledTimes(2);
+    });
+
+    it("should handle window specifiers case-insensitively", () => {
+      // Test lowercase window_title
+      const argsLowerTitle = buildSwiftCliArgs({ app_target: "Safari:window_title:Apple Website" }, undefined);
+      expect(argsLowerTitle).toEqual(
+        expect.arrayContaining([
+          "--app", "Safari",
+          "--mode", "window",
+          "--window-title", "Apple Website"
+        ]),
+      );
+      
+      // Test mixed case Window_Index
+      const argsMixedIndex = buildSwiftCliArgs({ app_target: "Terminal:Window_Index:2" }, undefined);
+      expect(argsMixedIndex).toEqual(
+        expect.arrayContaining([
+          "--app", "Terminal",
+          "--mode", "window",
+          "--window-index", "2"
+        ]),
+      );
+    });
+
     it("should handle simple app name", () => {
       const args = buildSwiftCliArgs({ app_target: "Safari" }, undefined);
       expect(args).toEqual(

tests/unit/utils/malformed-app-target.test.ts

@@ -0,0 +1,149 @@
+import { describe, it, expect } from "vitest";
+import { buildSwiftCliArgs } from "../../../src/utils/image-cli-args";
+
+describe("Malformed App Target Parsing", () => {
+  it("should handle multiple leading colons correctly", () => {
+    const input = {
+      app_target: "::::::::::::::::Finder",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should not result in empty app name
+    expect(args).toContain("--app");
+    const appIndex = args.indexOf("--app");
+    expect(args[appIndex + 1]).not.toBe("");
+    
+    // Should either treat as malformed and use multi mode, or properly extract "Finder"
+    expect(args).toContain("--mode");
+    const modeIndex = args.indexOf("--mode");
+    const mode = args[modeIndex + 1];
+    expect(["multi", "window"]).toContain(mode);
+  });
+  
+  it("should handle empty parts between colons", () => {
+    const input = {
+      app_target: "::Chrome::WINDOW_TITLE::google.com",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should not result in empty app name
+    expect(args).toContain("--app");
+    const appIndex = args.indexOf("--app");
+    expect(args[appIndex + 1]).not.toBe("");
+  });
+  
+  it("should handle app target that starts with colon", () => {
+    const input = {
+      app_target: ":Chrome",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should handle gracefully, not create empty app name
+    expect(args).toContain("--app");
+    const appIndex = args.indexOf("--app");
+    expect(args[appIndex + 1]).not.toBe("");
+  });
+  
+  it("should handle app target that ends with colon", () => {
+    const input = {
+      app_target: "Chrome:",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should treat as simple app name
+    expect(args).toContain("--app");
+    const appIndex = args.indexOf("--app");
+    expect(args[appIndex + 1]).toBe("Chrome");
+    
+    expect(args).toContain("--mode");
+    const modeIndex = args.indexOf("--mode");
+    expect(args[modeIndex + 1]).toBe("multi");
+  });
+  
+  it("should handle multiple consecutive colons in middle", () => {
+    const input = {
+      app_target: "Chrome:::WINDOW_TITLE:::google.com",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    expect(args).toContain("--app");
+    const appIndex = args.indexOf("--app");
+    expect(args[appIndex + 1]).toBe("Chrome");
+    
+    // Should handle the malformed specifier type gracefully
+    expect(args).toContain("--mode");
+  });
+  
+  it("should reject completely empty app target", () => {
+    const input = {
+      app_target: "",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should default to screen mode for empty target
+    expect(args).toContain("--mode");
+    const modeIndex = args.indexOf("--mode");
+    expect(args[modeIndex + 1]).toBe("screen");
+    
+    // Should not contain app argument
+    expect(args).not.toContain("--app");
+  });
+  
+  it("should handle only colons", () => {
+    const input = {
+      app_target: ":::::",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should not result in empty app name being passed
+    if (args.includes("--app")) {
+      const appIndex = args.indexOf("--app");
+      expect(args[appIndex + 1]).not.toBe("");
+    }
+  });
+  
+  it("should handle whitespace-only app names", () => {
+    const input = {
+      app_target: "   :WINDOW_TITLE:test",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    // Should handle whitespace-only app names
+    if (args.includes("--app")) {
+      const appIndex = args.indexOf("--app");
+      const appName = args[appIndex + 1];
+      expect(appName.trim()).not.toBe("");
+    }
+  });
+  
+  it("should demonstrate current behavior for debugging", () => {
+    // This test documents what currently happens with the problematic input
+    const input = {
+      app_target: "::::::::::::::::Finder",
+      format: "png" as const
+    };
+    
+    const args = buildSwiftCliArgs(input, "/tmp/test.png");
+    
+    console.log("Args for '::::::::::::::::Finder':", args);
+    
+    // Document current behavior - this will help us verify the fix
+    expect(args).toContain("--mode");
+  });
+});