From 1fc2a912d47ae576b4f7bb8d59f6d1331a226a4c Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Tue, 6 Jan 2026 11:59:09 +0100 Subject: [PATCH] Add blockImages setting to prevent images from being sent to LLM providers - Setting controls filtering at convertToLlm layer (defense-in-depth) - Images are always stored in session, filtered dynamically based on current setting - Toggle mid-session works: LLM sees/doesn't see images already in session - Fixed SettingsManager.save() to handle inMemory mode for all setters Closes #492 --- packages/coding-agent/CHANGELOG.md | 2 +- .../coding-agent/src/cli/file-processor.ts | 7 --- .../coding-agent/src/core/agent-session.ts | 8 +-- packages/coding-agent/src/core/sdk.ts | 57 +++++++++++-------- .../coding-agent/src/core/settings-manager.ts | 33 +++++------ packages/coding-agent/src/core/tools/read.ts | 53 +++++++---------- .../components/settings-selector.ts | 2 +- .../coding-agent/test/block-images.test.ts | 45 +++------------ 8 files changed, 80 insertions(+), 127 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 626c56d7..61204a42 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- Added `blockImages` setting to prevent images from being sent to LLM providers. Provides defense-in-depth blocking at Read tool, CLI file processor, AgentSession, and convertToLlm layer. ([#492](https://github.com/badlogic/pi-mono/pull/492)) +- Added `blockImages` setting to prevent images from being sent to LLM providers. Provides defense-in-depth blocking at Read tool, CLI file processor, AgentSession, and convertToLlm layer. ([#492](https://github.com/badlogic/pi-mono/pull/492) by [@jsinge97](https://github.com/jsinge97)) ## [0.37.2] - 2026-01-05 diff --git a/packages/coding-agent/src/cli/file-processor.ts b/packages/coding-agent/src/cli/file-processor.ts index 6f786df8..8b676419 100644 --- a/packages/coding-agent/src/cli/file-processor.ts +++ b/packages/coding-agent/src/cli/file-processor.ts @@ -18,8 +18,6 @@ export interface ProcessedFiles { export interface ProcessFileOptions { /** Whether to auto-resize images to 2000x2000 max. Default: true */ autoResizeImages?: boolean; - /** When true, skip image files with warning. Default: false */ - blockImages?: boolean; } /** Process @file arguments into text content and image attachments */ @@ -50,11 +48,6 @@ export async function processFileArguments(fileArgs: string[], options?: Process const mimeType = await detectSupportedImageMimeTypeFromFile(absolutePath); if (mimeType) { - // Check if images are blocked - if (options?.blockImages) { - console.warn(chalk.yellow(`[blockImages] Skipping image file: ${absolutePath}`)); - continue; - } // Handle image file const content = await readFile(absolutePath); const base64Content = content.toString("base64"); diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 3afd4418..92194eaa 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -592,13 +592,7 @@ export class AgentSession { // Add user message const userContent: (TextContent | ImageContent)[] = [{ type: "text", text: expandedText }]; if (options?.images) { - const blockImages = this.settingsManager.getBlockImages(); - if (blockImages) { - // Log warning for blocked images - console.warn(`[blockImages] Blocked ${options.images.length} image(s) from being sent to provider`); - } else { - userContent.push(...options.images); - } + userContent.push(...options.images); } messages.push({ role: "user", diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 55767f55..aace4e41 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -426,9 +426,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} time("discoverContextFiles"); const autoResizeImages = settingsManager.getImageAutoResize(); - const blockImages = settingsManager.getBlockImages(); // Create ALL built-in tools for the registry (extensions can enable any of them) - const allBuiltInToolsMap = createAllTools(cwd, { read: { autoResizeImages, blockImages } }); + const allBuiltInToolsMap = createAllTools(cwd, { read: { autoResizeImages } }); // Determine initially active built-in tools (default: read, bash, edit, write) const defaultActiveToolNames: ToolName[] = ["read", "bash", "edit", "write"]; const initialActiveToolNames: ToolName[] = options.tools @@ -607,30 +606,42 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const promptTemplates = options.promptTemplates ?? discoverPromptTemplates(cwd, agentDir); time("discoverPromptTemplates"); - // Create convertToLlm wrapper that filters images if blockImages is enabled - const convertToLlmWithBlockImages = blockImages - ? (messages: AgentMessage[]): Message[] => { - const converted = convertToLlm(messages); - let totalFiltered = 0; - // Filter out ImageContent from all messages as defense-in-depth - const filtered = converted.map((msg) => { - if (msg.role === "user" || msg.role === "toolResult") { - const content = msg.content; - if (Array.isArray(content)) { - const originalLength = content.length; - const filteredContent = content.filter((c) => c.type !== "image"); - totalFiltered += originalLength - filteredContent.length; - return { ...msg, content: filteredContent }; - } + // Create convertToLlm wrapper that filters images if blockImages is enabled (defense-in-depth) + const convertToLlmWithBlockImages = (messages: AgentMessage[]): Message[] => { + const converted = convertToLlm(messages); + // Check setting dynamically so mid-session changes take effect + if (!settingsManager.getBlockImages()) { + return converted; + } + // Filter out ImageContent from all messages, replacing with text placeholder + return converted.map((msg) => { + if (msg.role === "user" || msg.role === "toolResult") { + const content = msg.content; + if (Array.isArray(content)) { + const hasImages = content.some((c) => c.type === "image"); + if (hasImages) { + const filteredContent = content + .map((c) => + c.type === "image" ? { type: "text" as const, text: "Image reading is disabled." } : c, + ) + .filter( + (c, i, arr) => + // Dedupe consecutive "Image reading is disabled." texts + !( + c.type === "text" && + c.text === "Image reading is disabled." && + i > 0 && + arr[i - 1].type === "text" && + (arr[i - 1] as { type: "text"; text: string }).text === "Image reading is disabled." + ), + ); + return { ...msg, content: filteredContent }; } - return msg; - }); - if (totalFiltered > 0) { - console.warn(`[blockImages] Defense-in-depth: filtered ${totalFiltered} image(s) at convertToLlm layer`); } - return filtered; } - : convertToLlm; + return msg; + }); + }; agent = new Agent({ initialState: { diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index 7f09ff94..e9f29b5d 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -171,23 +171,23 @@ export class SettingsManager { } private save(): void { - if (!this.persist || !this.settingsPath) return; + if (this.persist && this.settingsPath) { + try { + const dir = dirname(this.settingsPath); + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } - try { - const dir = dirname(this.settingsPath); - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }); + // Save only global settings (project settings are read-only) + writeFileSync(this.settingsPath, JSON.stringify(this.globalSettings, null, 2), "utf-8"); + } catch (error) { + console.error(`Warning: Could not save settings file: ${error}`); } - - // Save only global settings (project settings are read-only) - writeFileSync(this.settingsPath, JSON.stringify(this.globalSettings, null, 2), "utf-8"); - - // Re-merge project settings into active settings - const projectSettings = this.loadProjectSettings(); - this.settings = deepMergeSettings(this.globalSettings, projectSettings); - } catch (error) { - console.error(`Warning: Could not save settings file: ${error}`); } + + // Always re-merge to update active settings (needed for both file and inMemory modes) + const projectSettings = this.loadProjectSettings(); + this.settings = deepMergeSettings(this.globalSettings, projectSettings); } getLastChangelogVersion(): string | undefined { @@ -408,11 +408,6 @@ export class SettingsManager { this.globalSettings.images = {}; } this.globalSettings.images.blockImages = blocked; - // Also update active settings for inMemory mode - if (!this.settings.images) { - this.settings.images = {}; - } - this.settings.images.blockImages = blocked; this.save(); } diff --git a/packages/coding-agent/src/core/tools/read.ts b/packages/coding-agent/src/core/tools/read.ts index b8aeddff..e7ba44fb 100644 --- a/packages/coding-agent/src/core/tools/read.ts +++ b/packages/coding-agent/src/core/tools/read.ts @@ -21,13 +21,10 @@ export interface ReadToolDetails { export interface ReadToolOptions { /** Whether to auto-resize images to 2000x2000 max. Default: true */ autoResizeImages?: boolean; - /** When true, return text message instead of image content. Default: false */ - blockImages?: boolean; } export function createReadTool(cwd: string, options?: ReadToolOptions): AgentTool { const autoResizeImages = options?.autoResizeImages ?? true; - const blockImages = options?.blockImages ?? false; return { name: "read", label: "read", @@ -78,39 +75,29 @@ export function createReadTool(cwd: string, options?: ReadToolOptions): AgentToo let details: ReadToolDetails | undefined; if (mimeType) { - // Check if images are blocked - if (blockImages) { + // Read as image (binary) + const buffer = await readFile(absolutePath); + const base64 = buffer.toString("base64"); + + if (autoResizeImages) { + // Resize image if needed + const resized = await resizeImage({ type: "image", data: base64, mimeType }); + const dimensionNote = formatDimensionNote(resized); + + let textNote = `Read image file [${resized.mimeType}]`; + if (dimensionNote) { + textNote += `\n${dimensionNote}`; + } + content = [ - { - type: "text", - text: `[Image file detected: ${absolutePath}]\nImage reading is disabled. The 'blockImages' setting is enabled.`, - }, + { type: "text", text: textNote }, + { type: "image", data: resized.data, mimeType: resized.mimeType }, ]; } else { - // Read as image (binary) - const buffer = await readFile(absolutePath); - const base64 = buffer.toString("base64"); - - if (autoResizeImages) { - // Resize image if needed - const resized = await resizeImage({ type: "image", data: base64, mimeType }); - const dimensionNote = formatDimensionNote(resized); - - let textNote = `Read image file [${resized.mimeType}]`; - if (dimensionNote) { - textNote += `\n${dimensionNote}`; - } - - content = [ - { type: "text", text: textNote }, - { type: "image", data: resized.data, mimeType: resized.mimeType }, - ]; - } else { - content = [ - { type: "text", text: `Read image file [${mimeType}]` }, - { type: "image", data: base64, mimeType }, - ]; - } + content = [ + { type: "text", text: `Read image file [${mimeType}]` }, + { type: "image", data: base64, mimeType }, + ]; } } else { // Read as text diff --git a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts index e0f8fc5a..681742a4 100644 --- a/packages/coding-agent/src/modes/interactive/components/settings-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/settings-selector.ts @@ -250,7 +250,7 @@ export class SettingsSelectorComponent extends Container { items.splice(autoResizeIndex + 1, 0, { id: "block-images", label: "Block images", - description: "Prevent images from being sent to LLM providers (restart session for full effect)", + description: "Prevent images from being sent to LLM providers", currentValue: config.blockImages ? "true" : "false", values: ["true", "false"], }); diff --git a/packages/coding-agent/test/block-images.test.ts b/packages/coding-agent/test/block-images.test.ts index aa0b4e6e..2c599276 100644 --- a/packages/coding-agent/test/block-images.test.ts +++ b/packages/coding-agent/test/block-images.test.ts @@ -54,42 +54,27 @@ describe("blockImages setting", () => { rmSync(testDir, { recursive: true, force: true }); }); - it("should return text message when blockImages is true", async () => { + it("should always read images (filtering happens at convertToLlm layer)", async () => { // Create test image const imagePath = join(testDir, "test.png"); writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); - const tool = createReadTool(testDir, { blockImages: true }); + const tool = createReadTool(testDir); const result = await tool.execute("test-1", { path: imagePath }); - expect(result.content).toHaveLength(1); - expect(result.content[0].type).toBe("text"); - const textContent = result.content[0] as { type: "text"; text: string }; - expect(textContent.text).toContain("Image reading is disabled"); - expect(textContent.text).toContain("blockImages"); - }); - - it("should return image content when blockImages is false", async () => { - // Create test image - const imagePath = join(testDir, "test.png"); - writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); - - const tool = createReadTool(testDir, { blockImages: false }); - const result = await tool.execute("test-2", { path: imagePath }); - // Should have text note + image content expect(result.content.length).toBeGreaterThanOrEqual(1); const hasImage = result.content.some((c) => c.type === "image"); expect(hasImage).toBe(true); }); - it("should read text files normally even when blockImages is true", async () => { + it("should read text files normally", async () => { // Create test text file const textPath = join(testDir, "test.txt"); writeFileSync(textPath, "Hello, world!"); - const tool = createReadTool(testDir, { blockImages: true }); - const result = await tool.execute("test-3", { path: textPath }); + const tool = createReadTool(testDir); + const result = await tool.execute("test-2", { path: textPath }); expect(result.content).toHaveLength(1); expect(result.content[0].type).toBe("text"); @@ -110,35 +95,23 @@ describe("blockImages setting", () => { rmSync(testDir, { recursive: true, force: true }); }); - it("should skip image files when blockImages is true", async () => { + it("should always process images (filtering happens at convertToLlm layer)", async () => { // Create test image const imagePath = join(testDir, "test.png"); writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); - const result = await processFileArguments([imagePath], { blockImages: true }); - - expect(result.images).toHaveLength(0); - // Text should be empty since image was skipped - expect(result.text).toBe(""); - }); - - it("should include image files when blockImages is false", async () => { - // Create test image - const imagePath = join(testDir, "test.png"); - writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); - - const result = await processFileArguments([imagePath], { blockImages: false }); + const result = await processFileArguments([imagePath]); expect(result.images).toHaveLength(1); expect(result.images[0].type).toBe("image"); }); - it("should process text files normally when blockImages is true", async () => { + it("should process text files normally", async () => { // Create test text file const textPath = join(testDir, "test.txt"); writeFileSync(textPath, "Hello, world!"); - const result = await processFileArguments([textPath], { blockImages: true }); + const result = await processFileArguments([textPath]); expect(result.images).toHaveLength(0); expect(result.text).toContain("Hello, world!");