fix(mcp): enforce env regex on every create_* tool (BUG-MCP-003/010)

package.json

@@ -46,8 +46,8 @@
     "dev": "tsc --watch",
     "start": "node dist/index.js",
     "pretest": "tsc && tsc -p tsconfig.test.json",
-    "test": "node --test --experimental-test-coverage --test-coverage-exclude='dist-test/test/**' --test-coverage-exclude='dist/**' --test-coverage-exclude='node_modules/**' dist-test/test/integration.test.js dist-test/test/live-smoke.test.js dist-test/test/client-unit.test.js dist-test/test/index-unit.test.js dist-test/test/tools-unit.test.js",
-    "test:nocov": "node --test dist-test/test/integration.test.js dist-test/test/live-smoke.test.js dist-test/test/client-unit.test.js dist-test/test/index-unit.test.js dist-test/test/tools-unit.test.js",
+    "test": "node --test --experimental-test-coverage --test-coverage-exclude='dist-test/test/**' --test-coverage-exclude='dist/**' --test-coverage-exclude='node_modules/**' dist-test/test/integration.test.js dist-test/test/live-smoke.test.js dist-test/test/client-unit.test.js dist-test/test/index-unit.test.js dist-test/test/tools-unit.test.js dist-test/test/env-regex-unit.test.js",
+    "test:nocov": "node --test dist-test/test/integration.test.js dist-test/test/live-smoke.test.js dist-test/test/client-unit.test.js dist-test/test/index-unit.test.js dist-test/test/tools-unit.test.js dist-test/test/env-regex-unit.test.js",
     "test:smoke": "bash test.sh",
     "prepublishOnly": "npm run build"
   },

src/index.ts

@@ -220,13 +220,28 @@ const nameArg = {
 // field on every /<resource>/new body since mig 026) so the agent can confirm
 // which bucket the resource landed in. Omitting `env` keeps the existing
 // behavior (server-side default `development`).
+// BUG-MCP-003/010: the API enforces `env` against ^[a-z0-9-]{1,32}$
+// (see api/internal/handlers/env.go + the `invalid_env` 400 branch).
+// Pre-fix the MCP schema declared `env` as a bare `z.string()`, so a
+// hostile agent could send `env=HACKERLAND` or `env=<33-chars>` and the
+// validation failure only surfaced from the API (extra round trip +
+// confusing error path). Enforcing here matches the API regex one-shot
+// and surfaces a clean zod error to the calling agent. The regex is
+// kept in a single constant so the api/CLI/dashboard stay in lockstep.
+const ENV_REGEX = /^[a-z0-9-]{1,32}$/;
+const envSchema = z
+  .string()
+  .regex(
+    ENV_REGEX,
+    "env must match ^[a-z0-9-]{1,32}$ (lowercase letters, digits, dashes; 1-32 chars; e.g. development, staging, production)"
+  )
+  .optional()
+  .describe(
+    "Resource environment scope: 'development' (server default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Format: ^[a-z0-9-]{1,32}$ — lowercase letters, digits, and dashes only. Omitting `env` lands the resource in 'development' (lowest stakes). The response echoes the resolved `env` so callers can confirm the bucket."
+  );
+
 const envArg = {
-  env: z
-    .string()
-    .optional()
-    .describe(
-      "Resource environment scope: 'development' (server default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Omitting `env` lands the resource in 'development' (lowest stakes). The response echoes the resolved `env` so callers can confirm the bucket."
-    ),
+  env: envSchema,
 };
 
 // Convenience: every create_* tool that only needs name + optional env. Spread
@@ -928,11 +943,17 @@ Requires INSTANODE_TOKEN (anonymous tier cannot deploy).`,
       .max(65535)
       .optional()
       .describe("Container HTTP port. Default 8080."),
+    // BUG-MCP-003/010: enforce the api regex client-side so a bad env
+    // surfaces as a zod error rather than a confusing API 400 round-trip.
     env: z
       .string()
+      .regex(
+        ENV_REGEX,
+        "env must match ^[a-z0-9-]{1,32}$ (lowercase letters, digits, dashes; 1-32 chars)"
+      )
       .optional()
       .describe(
-        "Deploy environment scope: 'development' (default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Omitting `env` lands the deploy in 'development' (lowest stakes), so accidental no-env deploys can't merge with prod state. Each scope has its own vault and env_vars."
+        "Deploy environment scope: 'development' (default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Format: ^[a-z0-9-]{1,32}$. Omitting `env` lands the deploy in 'development' (lowest stakes), so accidental no-env deploys can't merge with prod state. Each scope has its own vault and env_vars."
       ),
     // Security hardening (audit 2026-05-29):
     //   Bound the number of env entries and the per-value byte length so a
@@ -1100,11 +1121,18 @@ plus the anonymous-tier upgrade fields.`,
       .describe(
         "Map of service-name → base64-encoded gzip tarball of that service's build context (Dockerfile + source). One entry per service declared in the manifest that has a `build:` field. Service names match ^[A-Za-z0-9][A-Za-z0-9 _-]*$ (1..64). Cap: 50 MiB per service after base64 decode; max 32 services per stack."
       ),
+    // BUG-MCP-003/010: enforce the api regex client-side; same regex as
+    // envSchema above. Kept inline rather than referencing envSchema so
+    // the create_stack input contract is grep-visible in one block.
     env: z
       .string()
+      .regex(
+        ENV_REGEX,
+        "env must match ^[a-z0-9-]{1,32}$ (lowercase letters, digits, dashes; 1-32 chars)"
+      )
       .optional()
       .describe(
-        "Resource environment scope: 'development' (server default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Omitting `env` lands the stack in 'development' (lowest stakes). The response echoes the resolved `env`."
+        "Resource environment scope: 'development' (server default — see CLAUDE.md convention #11 / migration 026), 'staging', or 'production'. Format: ^[a-z0-9-]{1,32}$. Omitting `env` lands the stack in 'development' (lowest stakes). The response echoes the resolved `env`."
       ),
   },
   async ({ name, manifest, service_tarballs, env }) => {

test/env-regex-unit.test.ts

@@ -0,0 +1,111 @@
+/**
+ * env-regex-unit.test.ts — BUG-MCP-003 + BUG-MCP-010 regression.
+ *
+ * The api enforces `env` against `^[a-z0-9-]{1,32}$` (see
+ * api/internal/handlers/env.go + the `invalid_env` 400 branch). Pre-fix
+ * the MCP schema declared `env` as a bare `z.string()`, so a hostile
+ * agent could send `env=HACKERLAND` or `env=<33-chars>` and the
+ * validation failure only surfaced from the API (extra round trip and a
+ * confusing error path). Enforcing here matches the API regex one-shot
+ * and surfaces a clean zod error to the calling agent.
+ *
+ * What this test does:
+ *   - Imports src/index.ts so `server` is built with the real schemas.
+ *   - For every tool whose `env` field exists (create_postgres,
+ *     create_vector, create_cache, create_nosql, create_queue,
+ *     create_webhook, create_storage, create_deploy, create_stack),
+ *     reads `_registeredTools[<name>].inputSchema._def.shape().env` and
+ *     asserts the underlying zod schema has a regex constraint.
+ *   - Smokes a positive case ("staging") and a negative case
+ *     ("HACKERLAND") through the schema's safeParse so the regression is
+ *     end-to-end at the zod level.
+ */
+
+import { strict as assert } from "node:assert";
+import { before, describe, it } from "node:test";
+
+// Disable the server's auto-connect side effect.
+process.env["INSTANODE_MCP_NO_LISTEN"] = "1";
+
+let server: any;
+
+before(async () => {
+  const mod: any = await import("../src/index.js");
+  server = mod.server;
+});
+
+// The set of tools that expose an `env` arg per BUG-MCP-003/010.
+const envToolNames = [
+  "create_postgres",
+  "create_vector",
+  "create_cache",
+  "create_nosql",
+  "create_queue",
+  "create_webhook",
+  "create_storage",
+  "create_deploy",
+  "create_stack",
+];
+
+function envSchemaOf(toolName: string): any {
+  const reg = (server as any)._registeredTools as Record<
+    string,
+    { inputSchema?: any }
+  >;
+  const t = reg[toolName];
+  assert.ok(t, `tool not registered: ${toolName}`);
+  // The mcp-sdk wraps the registered ZodRawShape into a ZodObject and
+  // hangs it off `inputSchema`. Zod v4 exposes the per-field shape via
+  // `.shape` (a plain object). The env field is wrapped in ZodOptional;
+  // safeParse against the optional itself rejects bad input and accepts
+  // undefined — which is exactly what we want to test.
+  return (t as any).inputSchema?.shape?.env ?? null;
+}
+
+describe("BUG-MCP-003/010 — env field carries ^[a-z0-9-]{1,32}$ regex", () => {
+  for (const toolName of envToolNames) {
+    it(`${toolName}.env REJECTS the api invalid_env shape "HACKERLAND" client-side`, () => {
+      const env = envSchemaOf(toolName);
+      assert.ok(env, `${toolName} env schema not located via _def.shape()`);
+      const out = env.safeParse("HACKERLAND");
+      assert.equal(
+        out.success,
+        false,
+        `BUG-MCP-010: ${toolName} env must reject uppercase (api regex is lowercase only). Got: ${JSON.stringify(out)}`
+      );
+    });
+
+    it(`${toolName}.env REJECTS over-cap (>32 chars) client-side`, () => {
+      const env = envSchemaOf(toolName);
+      assert.ok(env);
+      const over = "a".repeat(33);
+      const out = env.safeParse(over);
+      assert.equal(
+        out.success,
+        false,
+        `BUG-MCP-003: ${toolName} env must reject >32 chars (api cap). Got: ${JSON.stringify(out)}`
+      );
+    });
+
+    it(`${toolName}.env ACCEPTS the canonical "staging" value`, () => {
+      const env = envSchemaOf(toolName);
+      assert.ok(env);
+      const out = env.safeParse("staging");
+      assert.equal(out.success, true, `staging must pass: ${JSON.stringify(out)}`);
+    });
+
+    it(`${toolName}.env ACCEPTS undefined (optional field)`, () => {
+      const env = envSchemaOf(toolName);
+      assert.ok(env);
+      const out = env.safeParse(undefined);
+      assert.equal(out.success, true, `undefined must pass — env is optional: ${JSON.stringify(out)}`);
+    });
+  }
+});
+
+// Note on JSON Schema serialisation: the mcp-sdk converts each
+// registered ZodObject into a JSON Schema lazily when `tools/list` is
+// served over the wire (see integration.test.ts:1004). The zod-direct
+// safeParse tests above are the authoritative regression check; the
+// wire-format end-to-end is exercised by the live-smoke + integration
+// suites that already drive `client.listTools()`.