From 127bcbd1c9ec5fec47554e599d5075015ffd5795 Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Sun, 5 Jul 2026 04:19:19 +0200
Subject: [PATCH] fix: enforce icon length cap in the sanitizer, not the schema

The schema `.max()` on iconPath rejected the whole update at parse time.
A server whose stored icon predates the cap (previous versions allowed
large data-URI images) re-submits that value from the edit dialog, so
the user was locked out of changing any field or clearing the bad icon.

Drop the schema `.max()` and enforce MAX_MCP_ICON_PATH_LENGTH in
sanitizeMcpIconPath for every value type: an over-cap SVG is compacted
or dropped, an over-cap raster/URL is dropped. Editing a server with a
pre-existing oversized icon now succeeds and clears the icon instead of
failing validation.
---
 packages/api/src/mcp/icons.spec.ts       | 15 +++++++++++++--
 packages/api/src/mcp/icons.ts            | 18 +++++++++---------
 packages/data-provider/specs/mcp.spec.ts | 12 ++++++++++++
 packages/data-provider/src/mcp.ts        |  7 ++++++-
 4 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/packages/api/src/mcp/icons.spec.ts b/packages/api/src/mcp/icons.spec.ts
index 87bebe453f..44f57877c7 100644
--- a/packages/api/src/mcp/icons.spec.ts
+++ b/packages/api/src/mcp/icons.spec.ts
@@ -239,8 +239,8 @@ describe('sanitizeMcpIconPath', () => {
 
   it('never stores an icon over the length cap even when sanitizing grows it', () => {
     // A base64 input under the cap whose many self-closing tags expand under
-    // sanitization (explicit close tags) past the cap; it must be dropped, not
-    // stored over-limit where the next edit's re-validation would reject it.
+    // sanitization (explicit close tags) past the cap; it must be dropped rather
+    // than stored over-limit.
     const cell = '<rect x="1" y="1" width="2" height="2" fill="#abc"/>';
     const raw = `<svg>${cell.repeat(3400)}</svg>`;
     const input = `data:image/svg+xml;base64,${Buffer.from(raw, 'utf-8').toString('base64')}`;
@@ -249,4 +249,15 @@ describe('sanitizeMcpIconPath', () => {
     expect(out.length).toBeLessThanOrEqual(MAX_MCP_ICON_PATH_LENGTH);
     expect(out).toBe('');
   });
+
+  it('drops an over-cap non-SVG value (raster data URI) that cannot be compacted', () => {
+    const huge = `data:image/png;base64,${'A'.repeat(MAX_MCP_ICON_PATH_LENGTH)}`;
+    expect(huge.length).toBeGreaterThan(MAX_MCP_ICON_PATH_LENGTH);
+    expect(sanitizeMcpIconPath(huge)).toBe('');
+  });
+
+  it('passes an under-cap non-SVG value through unchanged', () => {
+    const ok = `data:image/png;base64,${'A'.repeat(1000)}`;
+    expect(sanitizeMcpIconPath(ok)).toBe(ok);
+  });
 });
diff --git a/packages/api/src/mcp/icons.ts b/packages/api/src/mcp/icons.ts
index b7eebc6c8c..eeb66aa02f 100644
--- a/packages/api/src/mcp/icons.ts
+++ b/packages/api/src/mcp/icons.ts
@@ -343,20 +343,20 @@ function normalizeIconValue(value: string): string {
  * Sanitize a user-provided MCP `iconPath`. SVG data URIs are decoded, stripped
  * of active content via an allowlist, and re-encoded as base64; a malformed SVG
  * data URI resolves to an empty string so a broken icon is stored rather than
- * raw markup. All other values (raster data URIs, URLs, relative paths) are
- * returned unchanged.
+ * raw markup. Other values (raster data URIs, URLs, relative paths) pass through
+ * unchanged unless they exceed the length cap.
  *
- * Base64 is far more compact than percent-encoding for the angle-bracket-heavy
- * SVG markup that sanitizing can even slightly grow (it expands self-closing
- * tags to explicit close tags), so it minimizes needless rejection. The result
- * is then measured against the schema length cap and dropped if it still exceeds
- * it, so a value that passed the cap on input can never be stored over it and
- * then rejected when the prefilled value is resubmitted on the next edit.
+ * This is the single enforcement point for `MAX_MCP_ICON_PATH_LENGTH`: any value
+ * still over the cap after sanitizing is dropped to an empty string. Enforcing it
+ * here rather than as a schema `.max()` means editing a server whose stored icon
+ * predates the cap succeeds (the oversized icon is cleared) instead of failing
+ * validation and locking the user out of the whole update. Base64 also keeps SVG
+ * output compact so a legitimate icon is rarely dropped.
  */
 export function sanitizeMcpIconPath(iconPath: string): string {
   const normalized = normalizeIconValue(iconPath);
   if (!SVG_DATA_URI.test(normalized)) {
-    return iconPath;
+    return iconPath.length > MAX_MCP_ICON_PATH_LENGTH ? '' : iconPath;
   }
   const svg = decodeSvgDataUri(normalized);
   if (svg == null) {
diff --git a/packages/data-provider/specs/mcp.spec.ts b/packages/data-provider/specs/mcp.spec.ts
index a52e2f59e9..06a7eb285f 100644
--- a/packages/data-provider/specs/mcp.spec.ts
+++ b/packages/data-provider/specs/mcp.spec.ts
@@ -4,6 +4,7 @@ import {
   StreamableHTTPOptionsSchema,
   MCPServerUserInputSchema,
   MCP_USER_INPUT_FIELDS,
+  MAX_MCP_ICON_PATH_LENGTH,
 } from '../src/mcp';
 
 describe('MCPOptionsSchema', () => {
@@ -182,6 +183,17 @@ describe('MCP schemas', () => {
     });
   });
 
+  describe('iconPath', () => {
+    it('accepts an over-limit iconPath so editing a server with a pre-existing oversized icon is not rejected (the cap is enforced server-side by sanitizeMcpIconPath, not at parse time)', () => {
+      const result = MCPServerUserInputSchema.safeParse({
+        type: 'streamable-http',
+        url: 'https://mcp-server.com/http',
+        iconPath: `data:image/png;base64,${'A'.repeat(MAX_MCP_ICON_PATH_LENGTH + 1000)}`,
+      });
+      expect(result.success).toBe(true);
+    });
+  });
+
   describe('env variable rejection', () => {
     it('should reject SSE URLs containing env variable patterns', () => {
       const result = MCPServerUserInputSchema.safeParse({
diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts
index 789fa54acb..fd2e69d615 100644
--- a/packages/data-provider/src/mcp.ts
+++ b/packages/data-provider/src/mcp.ts
@@ -6,6 +6,11 @@ import { extractEnvVariable } from './utils';
  * Upper bound on a stored MCP `iconPath` string (URL or inlined data URI).
  * A legitimate icon is a few KB; this caps a malicious or accidental multi-MB
  * data URI that would otherwise be persisted and shipped in every server list.
+ *
+ * Enforced server-side by `sanitizeMcpIconPath` (which drops or compacts an
+ * over-limit value), NOT as a schema `.max()`. A parse-time cap would reject the
+ * whole update when the edit dialog re-submits a pre-existing over-limit icon,
+ * locking the user out of editing other fields or clearing the bad icon.
  */
 export const MAX_MCP_ICON_PATH_LENGTH = 256 * 1024;
 
@@ -176,7 +181,7 @@ const BaseOptionsSchema = z.object({
    *   requiring manual authentication (e.g., GitHub PAT tokens) that need to be configured through the UI after startup
    */
   startup: z.boolean().optional(),
-  iconPath: z.string().max(MAX_MCP_ICON_PATH_LENGTH).optional(),
+  iconPath: z.string().optional(),
   timeout: z.number().int().nonnegative().optional(),
   /** Timeout (ms) for the long-lived SSE GET stream body before undici aborts it. Default: 300_000 (5 min). */
   sseReadTimeout: z.number().int().positive().optional(),