From 5becd0c58f1b588b26e1528103336fb0340be58f Mon Sep 17 00:00:00 2001
From: Dave <david.els1@outlook.com>
Date: Thu, 1 Oct 2026 00:44:20 +0200
Subject: [PATCH] fix(giellie): reject non-string and path-shaped ids (incl.
 arrays) in the MCP bridge

An array under an id key stringifies unencoded into same-origin URL paths
(`${["../x"]}`), bypassing the string-only guard. Id-keyed values must now be
plain safe id strings; plural id arrays (item_ids, delivery_note_ids, ...) are
validated element by element. RFC 5322 message-id keys and attachment tokens
are left alone.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 src/lib/server/giellie-mcp-bridge.test.ts | 48 ++++++++++++++++++++++-
 src/lib/server/giellie-mcp-bridge.ts      | 24 ++++++++----
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/src/lib/server/giellie-mcp-bridge.test.ts b/src/lib/server/giellie-mcp-bridge.test.ts
index e380c95..931353d 100644
--- a/src/lib/server/giellie-mcp-bridge.test.ts
+++ b/src/lib/server/giellie-mcp-bridge.test.ts
@@ -175,9 +175,53 @@ describe("callMcpTool", () => {
       expect(mcpPost).toHaveBeenCalledTimes(1);
     });
 
-    it("ignores non-string id values", async () => {
+    it.each([
+      ["array", { id: ["../../x"] }],
+      ["array (api path)", { id: ["../../api/v1/foo?"] }],
+      ["empty array", { id: [] }],
+      ["object", { id: {} }],
+      ["null", { id: null }],
+      ["boolean", { id: true }],
+      ["number", { id: 5 }],
+      ["nested array under *_id", { quote_id: ["a/b"] }],
+      ["nested array under *Id", { patch: { orderId: ["x/y"] } }],
+    ])("rejects a non-string id value (%s) without calling the MCP", async (_l, args) => {
+      const out = await callMcpTool(ORIGIN, "orders.review", args, actor);
+      expect(out).toEqual({ text: "Error: invalid id value.", isError: true });
+      expect(mcpPost).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      ["item_ids", { item_ids: ["ok1", "../x"] }],
+      ["delivery_note_ids", { delivery_note_ids: ["a/b"] }],
+      ["itemIds", { groups: [{ itemIds: ["x", "y z"] }] }],
+      ["non-array plural", { item_ids: "../x" }],
+      ["object in plural", { item_ids: [{}] }],
+    ])("validates every element of a plural id array (%s)", async (_l, args) => {
+      const out = await callMcpTool(ORIGIN, "orders.split", args, actor);
+      expect(out).toEqual({ text: "Error: invalid id value.", isError: true });
+      expect(mcpPost).not.toHaveBeenCalled();
+    });
+
+    it("passes legitimate plural id arrays and leaves attachment tokens alone", async () => {
       ok();
-      await callMcpTool(ORIGIN, "orders.list", { id: null, order_id: 5, items: [{ id: undefined }] }, actor);
+      const out = await callMcpTool(
+        ORIGIN,
+        "quotes.send_email",
+        {
+          item_ids: ["abcde12345fghij", "klmno67890pqrst"],
+          delivery_note_ids: [],
+          selectedAttachmentIds: ["x7Gh_Quote 123.pdf"],
+        },
+        actor,
+      );
+      expect(out.isError).toBe(false);
+      expect(mcpPost).toHaveBeenCalledTimes(1);
+    });
+
+    it("ignores undefined id values (absent in JSON)", async () => {
+      ok();
+      await callMcpTool(ORIGIN, "orders.list", { items: [{ id: undefined }] }, actor);
       expect(mcpPost).toHaveBeenCalledTimes(1);
     });
   });
diff --git a/src/lib/server/giellie-mcp-bridge.ts b/src/lib/server/giellie-mcp-bridge.ts
index 3069936..d526a5c 100644
--- a/src/lib/server/giellie-mcp-bridge.ts
+++ b/src/lib/server/giellie-mcp-bridge.ts
@@ -63,6 +63,10 @@ const FILTER_REF_RE = /@(collection|request)\./i;
 const ID_KEY_RE = /(^|_)id$|Id$/;
 // RFC 5322 Message-ID headers ("<abc@mail.host>") are not record ids.
 const MESSAGE_ID_KEY_RE = /message_?id$/i;
+// Plural arrays of ids (item_ids, delivery_note_ids, itemIds…). PB file tokens
+// such as selectedAttachmentIds contain dots/spaces and are not record ids.
+const IDS_KEY_RE = /(^|_)ids$|Ids$/;
+const ATTACHMENT_KEY_RE = /attachment/i;
 const SAFE_ID_RE = /^[A-Za-z0-9_-]{1,64}$/;
 const MAX_GUARD_DEPTH = 6;
 
@@ -86,13 +90,19 @@ function guardArgs(value: unknown, depth: number): string | null {
     return null;
   }
   for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
-    if (
-      typeof v === "string" &&
-      ID_KEY_RE.test(k) &&
-      !MESSAGE_ID_KEY_RE.test(k) &&
-      !SAFE_ID_RE.test(v)
-    ) {
-      return "Error: invalid id value.";
+    if (v === undefined) continue;
+    if (ID_KEY_RE.test(k) && !MESSAGE_ID_KEY_RE.test(k)) {
+      // Handlers interpolate ids unencoded into same-origin URL paths, and
+      // `${["../x"]}` stringifies to "../x" — so only plain id strings pass.
+      // (Every singular id key in the MCP schema is type string.)
+      if (typeof v !== "string" || !SAFE_ID_RE.test(v)) return "Error: invalid id value.";
+      continue;
+    }
+    if (IDS_KEY_RE.test(k) && !ATTACHMENT_KEY_RE.test(k)) {
+      if (!Array.isArray(v) || !v.every((x) => typeof x === "string" && SAFE_ID_RE.test(x))) {
+        return "Error: invalid id value.";
+      }
+      continue;
     }
     const err = guardArgs(v, depth + 1);
     if (err) return err;