Audit fixes: security hardening, bug fixes, throttle retries, and docs UI refresh

AGENTS.md

@@ -40,5 +40,5 @@ This is a TypeScript CommonJS MCP server for the Shopify Admin API. Keep changes
 
 ## Validation Notes
 
-- As of this file's creation, `npm test` reports 87 passing tests and 6 existing failures in [tests/auth.test.ts](tests/auth.test.ts) and [tests/config.test.ts](tests/config.test.ts). The failures reflect stale expectations around client-credentials auth and required credential errors; do not treat them as caused by unrelated changes.
-- [tests/shopify-client.test.ts](tests/shopify-client.test.ts) and [tests/tools.test.ts](tests/tools.test.ts) currently pass and are useful focused checks for client behavior and tool-registration changes.
+- `npm test` should pass fully; treat any failure as caused by your change until proven otherwise.
+- [tests/shopify-client.test.ts](tests/shopify-client.test.ts) and [tests/tools.test.ts](tests/tools.test.ts) are useful focused checks for client behavior and tool-registration changes; [tests/graphql-helpers.test.ts](tests/graphql-helpers.test.ts) covers the shared helpers.

README.md

@@ -158,8 +158,8 @@ Then point your MCP client to the built output:
 |------|-------------|
 | `list_product_images` | List all images for a product |
 | `get_product_image` | Get a specific product image |
-| `create_product_image` | Add an image to a product (by URL or base64) |
-| `update_product_image` | Update image alt text or position |
+| `create_product_image` | Add an image to a product by URL, with optional position and variant assignment |
+| `update_product_image` | Update image alt text, position, or variant assignments |
 | `delete_product_image` | Remove an image from a product |
 
 ### Variants (5)
@@ -308,15 +308,15 @@ Modern Shopify GraphQL discount API for automatic discounts (applied without a c
 | `list_webhooks` | List all registered webhooks |
 | `get_webhook` | Get a webhook by ID |
 | `create_webhook` | Register a new webhook |
-| `update_webhook` | Update a webhook URL or topic |
+| `update_webhook` | Update a webhook callback URL (topic cannot be changed after creation) |
 | `delete_webhook` | Remove a webhook |
 
 ### Menus (5)
 
 | Tool | Description |
 |------|-------------|
 | `list_menus` | List navigation menus |
-| `get_menu` | Get a navigation menu by ID |
+| `get_menu` | Get a navigation menu by handle, including nested items |
 | `create_menu` | Create a navigation menu |
 | `update_menu` | Update a navigation menu |
 | `delete_menu` | Delete a navigation menu |

src/config.ts

@@ -17,16 +17,37 @@ export interface ShopifyConfig {
   apiVersion: string;
 }
 
+/**
+ * Normalizes a store identifier to the bare subdomain. Accepts "my-store",
+ * "my-store.myshopify.com", or a full URL, and validates the result so it
+ * can be safely interpolated into https://<storeName>.myshopify.com URLs.
+ */
+export function normalizeStoreName(raw: string): string {
+  const storeName = raw
+    .trim()
+    .replace(/^https?:\/\//i, "")
+    .replace(/\.myshopify\.com.*$/i, "")
+    .replace(/\/.*$/, "");
+  if (!/^[a-zA-Z0-9][a-zA-Z0-9-]*$/.test(storeName)) {
+    throw new Error(
+      `Invalid SHOPIFY_STORE_NAME: "${raw}". Use the store subdomain only, ` +
+      `e.g. "my-store" for my-store.myshopify.com.`
+    );
+  }
+  return storeName;
+}
+
 export function loadConfig(): ShopifyConfig {
-  const storeName = process.env.SHOPIFY_STORE_NAME;
+  const rawStoreName = process.env.SHOPIFY_STORE_NAME;
   const accessToken = process.env.SHOPIFY_ACCESS_TOKEN;
   const clientId = process.env.SHOPIFY_CLIENT_ID;
   const clientSecret = process.env.SHOPIFY_CLIENT_SECRET;
   const apiVersion = process.env.SHOPIFY_API_VERSION || "2026-01";
 
-  if (!storeName) {
+  if (!rawStoreName) {
     throw new Error("Missing required environment variable: SHOPIFY_STORE_NAME");
   }
+  const storeName = normalizeStoreName(rawStoreName);
 
   if (!accessToken && !(clientId && clientSecret)) {
     throw new Error(

src/get-token.ts

@@ -22,11 +22,13 @@
 import dotenv from "dotenv";
 import path from "path";
 import http from "http";
+import crypto from "crypto";
 import { exec } from "child_process";
+import { normalizeStoreName } from "./config.js";
 
 // Load .env from CWD first, then package dir fallback
-dotenv.config({ path: path.resolve(process.cwd(), ".env") });
-dotenv.config({ path: path.resolve(__dirname, "..", ".env") });
+dotenv.config({ path: path.resolve(process.cwd(), ".env"), quiet: true });
+dotenv.config({ path: path.resolve(__dirname, "..", ".env"), quiet: true });
 
 const CLIENT_ID = process.env.SHOPIFY_CLIENT_ID;
 const CLIENT_SECRET = process.env.SHOPIFY_CLIENT_SECRET;
@@ -63,15 +65,41 @@ if (!CLIENT_ID || !CLIENT_SECRET || !STORE_NAME) {
   process.exit(1);
 }
 
-const shop = `${STORE_NAME}.myshopify.com`;
-const state = Math.random().toString(36).slice(2);
+const shop = `${normalizeStoreName(STORE_NAME)}.myshopify.com`;
+const state = crypto.randomBytes(16).toString("hex");
 const authorizeUrl =
   `https://${shop}/admin/oauth/authorize` +
-  `?client_id=${CLIENT_ID}` +
+  `?client_id=${encodeURIComponent(CLIENT_ID)}` +
   `&scope=${encodeURIComponent(SCOPES)}` +
   `&redirect_uri=${encodeURIComponent(REDIRECT_URI)}` +
   `&state=${state}`;
 
+function escapeHtml(value: string): string {
+  return value
+    .replace(/&/g, "&")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;");
+}
+
+function callbackPage(title: string, detail: string): string {
+  return `<!DOCTYPE html>
+<html lang="en">
+<head>
+<meta charset="UTF-8">
+<title>Shopify MCP</title>
+<style>
+  body { font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif; background: #0a0a0a; color: #e8e8e8; display: flex; align-items: center; justify-content: center; min-height: 100vh; margin: 0; }
+  .card { background: #141414; border: 1px solid #2a2a2a; border-radius: 12px; padding: 2.5rem 3rem; max-width: 480px; text-align: center; }
+  h2 { margin: 0 0 0.75rem; }
+  p { color: #999; line-height: 1.6; margin: 0.5rem 0; }
+  code { background: #1c1c1c; padding: 0.15rem 0.45rem; border-radius: 4px; color: #96bf48; font-size: 0.9em; }
+</style>
+</head>
+<body><div class="card"><h2>${title}</h2>${detail}</div></body>
+</html>`;
+}
+
 function openBrowser(url: string) {
   const platform = process.platform;
   const cmd =
@@ -127,22 +155,22 @@ const server = http.createServer(async (req, res) => {
 
   if (error) {
     res.writeHead(400, { "Content-Type": "text/html" });
-    res.end(`<h2>OAuth error: ${error}</h2><p>You can close this tab.</p>`);
+    res.end(callbackPage(`OAuth error: ${escapeHtml(error)}`, "<p>You can close this tab.</p>"));
     server.close();
     console.error(`\nOAuth error: ${error}`);
     process.exit(1);
   }
 
   if (returnedState !== state) {
     res.writeHead(400, { "Content-Type": "text/html" });
-    res.end("<h2>State mismatch — possible CSRF. Try again.</h2>");
+    res.end(callbackPage("State mismatch", "<p>Possible CSRF. Close this tab and try again.</p>"));
     server.close();
     process.exit(1);
   }
 
   if (!code) {
     res.writeHead(400, { "Content-Type": "text/html" });
-    res.end("<h2>No code in callback.</h2>");
+    res.end(callbackPage("No code in callback", "<p>Close this tab and try again.</p>"));
     server.close();
     process.exit(1);
   }
@@ -157,15 +185,15 @@ const server = http.createServer(async (req, res) => {
     console.log("then restart your MCP server.\n");
 
     res.writeHead(200, { "Content-Type": "text/html" });
-    res.end(
-      "<h2>✅ Authorization successful!</h2>" +
+    res.end(callbackPage(
+      "✅ Authorization successful!",
       "<p>Your access token has been printed to the terminal.</p>" +
       "<p>Add <code>SHOPIFY_ACCESS_TOKEN=&lt;token&gt;</code> to your MCP config's <code>env</code> block, then restart the MCP server.</p>" +
       "<p>You can close this tab.</p>"
-    );
+    ));
   } catch (err) {
     res.writeHead(500, { "Content-Type": "text/html" });
-    res.end(`<h2>Token exchange failed</h2><pre>${err}</pre>`);
+    res.end(callbackPage("Token exchange failed", `<p>${escapeHtml(err instanceof Error ? err.message : String(err))}</p>`));
     console.error(err);
   } finally {
     server.close();

src/index.ts

@@ -23,13 +23,15 @@ import { registerThemeTools } from "./tools/themes.js";
 import { registerPageTools } from "./tools/pages.js";
 import { registerBundleTools } from "./tools/bundles.js";
 
+const { version } = require("../package.json") as { version: string };
+
 async function main() {
   const config = loadConfig();
   const client = new ShopifyClient(config);
 
   const server = new McpServer({
     name: "kockatoos-shopify-mcp",
-    version: "1.0.0",
+    version,
   });
 
   // Register all tool groups

src/shopify-client.ts

@@ -1,9 +1,23 @@
 import { ShopifyConfig } from "./config.js";
 import { getAccessToken } from "./auth.js";
 
+const MAX_RETRIES = 3;
+const BASE_RETRY_DELAY_MS = 1_000;
+
+function sleep(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+function isThrottledGraphQLError(errors: unknown[]): boolean {
+  return errors.some((error) => {
+    const code = (error as { extensions?: { code?: string } })?.extensions?.code;
+    return code === "THROTTLED";
+  });
+}
+
 /**
  * Shopify Admin GraphQL API client.
- * Handles authentication, request building, and error formatting.
+ * Handles authentication, request building, rate-limit retries, and error formatting.
  */
 export class ShopifyClient {
   private config: ShopifyConfig;
@@ -18,37 +32,57 @@ export class ShopifyClient {
     const token = await getAccessToken(this.config);
     const url = `${this.baseUrl}/graphql.json`;
 
-    let res: Response;
-    try {
-      res = await fetch(url, {
-        method: "POST",
-        headers: {
-          "X-Shopify-Access-Token": token,
-          "Content-Type": "application/json",
-        },
-        body: JSON.stringify({ query, variables }),
-        signal: AbortSignal.timeout(30_000),
-      });
-    } catch (err: unknown) {
-      const msg = err instanceof Error ? err.message : String(err);
-      const error = new Error(`Network error on GraphQL request: ${msg}`);
-      process.stderr.write(`[shopify-mcp] ${error.message}\n`);
-      throw error;
-    }
+    for (let attempt = 0; ; attempt++) {
+      let res: Response;
+      try {
+        res = await fetch(url, {
+          method: "POST",
+          headers: {
+            "X-Shopify-Access-Token": token,
+            "Content-Type": "application/json",
+          },
+          body: JSON.stringify({ query, variables }),
+          signal: AbortSignal.timeout(30_000),
+        });
+      } catch (err: unknown) {
+        const msg = err instanceof Error ? err.message : String(err);
+        const error = new Error(`Network error on GraphQL request: ${msg}`);
+        process.stderr.write(`[shopify-mcp] ${error.message}\n`);
+        throw error;
+      }
 
-    if (!res.ok) {
-      const errBody = await res.text();
-      const error = new Error(`Shopify GraphQL error ${res.status}: ${errBody}`);
-      process.stderr.write(`[shopify-mcp] ${error.message}\n`);
-      throw error;
-    }
+      // Rate limited at the HTTP level — back off and retry, honoring Retry-After
+      if (res.status === 429 && attempt < MAX_RETRIES) {
+        const retryAfter = Number(res.headers.get("Retry-After"));
+        const delay = Number.isFinite(retryAfter) && retryAfter > 0
+          ? retryAfter * 1000
+          : BASE_RETRY_DELAY_MS * 2 ** attempt;
+        process.stderr.write(`[shopify-mcp] Rate limited (429); retrying in ${delay}ms (attempt ${attempt + 1}/${MAX_RETRIES})\n`);
+        await sleep(delay);
+        continue;
+      }
+
+      if (!res.ok) {
+        const errBody = await res.text();
+        const error = new Error(`Shopify GraphQL error ${res.status}: ${errBody}`);
+        process.stderr.write(`[shopify-mcp] ${error.message}\n`);
+        throw error;
+      }
 
-    const json = (await res.json()) as { data?: T; errors?: unknown[] };
-    if (json.errors) {
-      const error = new Error(`Shopify GraphQL errors: ${JSON.stringify(json.errors)}`);
-      process.stderr.write(`[shopify-mcp] ${error.message}\n`);
-      throw error;
+      const json = (await res.json()) as { data?: T; errors?: unknown[] };
+      if (json.errors) {
+        // GraphQL cost-based throttling returns 200 with a THROTTLED error code
+        if (isThrottledGraphQLError(json.errors) && attempt < MAX_RETRIES) {
+          const delay = BASE_RETRY_DELAY_MS * 2 ** attempt;
+          process.stderr.write(`[shopify-mcp] Query throttled; retrying in ${delay}ms (attempt ${attempt + 1}/${MAX_RETRIES})\n`);
+          await sleep(delay);
+          continue;
+        }
+        const error = new Error(`Shopify GraphQL errors: ${JSON.stringify(json.errors)}`);
+        process.stderr.write(`[shopify-mcp] ${error.message}\n`);
+        throw error;
+      }
+      return json.data as T;
     }
-    return json.data as T;
   }
 }

src/tools/graphql-helpers.ts

@@ -26,14 +26,21 @@ export function compact<T extends Record<string, unknown>>(value: T): Partial<T>
 
 export function throwOnUserErrors(operation: string, errors?: ShopifyUserError[] | null): void {
   if (!errors || errors.length === 0) return;
-  const messages = errors.map((error) => error.message).join("; ");
+  const messages = errors
+    .map((error) => (error.field?.length ? `${error.field.join(".")}: ${error.message}` : error.message))
+    .join("; ");
   throw new Error(`${operation} errors: ${messages}`);
 }
 
 export function searchQuery(parts: Record<string, string | number | boolean | undefined>): string | undefined {
   const query = Object.entries(parts)
     .filter(([, value]) => value !== undefined && value !== "")
-    .map(([key, value]) => `${key}:${String(value)}`)
+    .map(([key, value]) => {
+      const text = String(value);
+      // Quote values with whitespace or quotes so the search syntax stays valid
+      const safe = /[\s"]/.test(text) ? `"${text.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"` : text;
+      return `${key}:${safe}`;
+    })
     .join(" ");
   return query || undefined;
 }