fix(setup): reject NUL bytes in registration environments (#4335)

src/application/DshCordisPatch.ts

@@ -242,7 +242,7 @@ export class DshCordisDocument {
     const environment = { ...literals, ...overrides };
     const validated =
       validation === "fragment"
-        ? parseEnvironmentValues(environment)
+        ? parseEnvironmentValues(environment, unresolvedSettings)
         : parseEnvironment(environment, unresolvedSettings);
     return validated.ok ? ok(environment) : validated;
   }

src/config.test.ts

@@ -22,6 +22,38 @@ it("reports a configuration failure for a NUL in Hopper loader arguments", () =>
   expect(result.error.message).toContain("NUL");
 });
 
+it.each([
+  ["unknown value", { CALLER_SETTING: "private\0value" }],
+  ["unknown name", { ["CALLER\0SETTING"]: "private value" }],
+  ["known path", { HOPPER_TARGET_PATH: "private\0path" }],
+] as const)(
+  "rejects a NUL in an environment %s without exposing its value",
+  (_name, environment) => {
+    for (const result of [
+      parseEnvironmentValues(environment),
+      parseConfig(environment),
+    ]) {
+      expect(result.ok).toBe(false);
+      if (result.ok) throw new Error("Expected invalid environment");
+      expect(result.error.settings).toHaveLength(1);
+      expect(result.error.settings[0]?.constraint).toContain("NUL");
+      expect(JSON.stringify(projectAnalysisError(result.error))).not.toContain(
+        "private",
+      );
+    }
+  },
+);
+
+it("accepts ordinary custom environment strings and omitted values", () => {
+  expect(
+    parseEnvironmentValues({
+      CALLER_SETTING: "\nvalue=one;二\t",
+      EMPTY_SETTING: "",
+      OMITTED_SETTING: undefined,
+    }).ok,
+  ).toBe(true);
+});
+
 describe("runtime configuration", () => {
   it("allows target-free startup and applies runtime defaults", () => {
     const empty = parseConfig({});

src/config/environment.ts

@@ -110,7 +110,30 @@ type Environment = Omit<
 /** Parse individual settings before layered registration fragments are merged. */
 export const parseEnvironmentValues = (
   environment: Readonly<Record<string, string | undefined>>,
+  unresolvedSettings: ReadonlySet<string> = new Set(),
 ): Result<Environment, ConfigurationError> => {
+  // Unknown client-specific entries are preserved by registration owners and
+  // still cross the child-process boundary. Check their byte admission before
+  // the known-setting schema strips them; never include rejected values.
+  // Cordis expression values stay unknown, but their literal names are known.
+  const keys = new Set([...Object.keys(environment), ...unresolvedSettings]);
+  const invalidEntries = [...keys]
+    .filter(
+      (key) =>
+        key.includes("\0") ||
+        (Object.hasOwn(environment, key) && environment[key]?.includes("\0")),
+    )
+    .map((setting) => ({
+      setting,
+      constraint:
+        "Environment variable names and values must not contain NUL bytes",
+    }));
+  if (invalidEntries.length > 0)
+    return err(
+      new ConfigurationError("Invalid REA environment configuration", {
+        settings: invalidEntries,
+      }),
+    );
   const parsed = environmentSchema.safeParse(environment);
   if (!parsed.success) {
     // Zod messages name the constraint, never the rejected value.
@@ -150,7 +173,7 @@ export const parseEnvironment = (
   // Registration expressions are not evaluated; their absence is not proven.
   unresolvedSettings: ReadonlySet<string> = new Set(),
 ): Result<Environment, ConfigurationError> => {
-  const parsed = parseEnvironmentValues(environment);
+  const parsed = parseEnvironmentValues(environment, unresolvedSettings);
   if (!parsed.ok) return parsed;
   const env = parsed.value;
   // Ghidra's analyzeHeadless accepts -cspec only together with -processor.

tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts

@@ -1,7 +1,7 @@
 import { access, mkdir, readFile, writeFile } from "node:fs/promises";
 import { dirname, join, resolve } from "node:path";
 import { expect } from "vitest";
-import { stringify } from "yaml";
+import { Scalar, stringify } from "yaml";
 
 import {
   effectiveClientServer,
@@ -33,6 +33,154 @@ const scenarios = [
   },
 ] as const;
 
+cliTest.for([
+  {
+    client: "cursor",
+    format: "json",
+    path: ".cursor/mcp.json",
+    position: "value",
+  },
+  {
+    client: "cursor",
+    format: "json",
+    path: ".cursor/mcp.json",
+    position: "name",
+  },
+  {
+    client: "dsh",
+    format: "dsh",
+    path: ".dsh/cordis.patch.yml",
+    position: "value",
+  },
+  {
+    client: "dsh",
+    format: "dsh",
+    path: ".dsh/cordis.patch.yml",
+    position: "name",
+  },
+  {
+    client: "dsh",
+    format: "dsh",
+    path: ".dsh/cordis.patch.yml",
+    position: "expression name",
+  },
+] as const)(
+  "refuses a NUL environment $position in $client before setup writes",
+  async (scenario, { cli }) => {
+    const home = await createTestTempDirectory("rea-registration-nul-");
+    const config = join(home, scenario.path);
+    await mkdir(dirname(config), { recursive: true });
+    const environment = {
+      HOME: home,
+      USERPROFILE: home,
+      XDG_CONFIG_HOME: join(home, ".config"),
+      DSH_HOME: join(home, ".dsh"),
+      HOPPER_LAUNCHER_PATH: join(home, "missing-hopper"),
+    };
+    const registration = (env: Readonly<Record<string, string>>) => {
+      const entry = clientRegistrationEntry(
+        scenario.format,
+        [process.execPath, resolve("scripts/rea.mjs"), "mcp"],
+        env,
+      );
+      if (scenario.position === "expression name")
+        entry.env = Object.fromEntries(
+          Object.entries(env).map(([key, value]) => {
+            if (!key.includes("\0")) return [key, value];
+            const expression = new Scalar("process.env.CALLER_SETTING");
+            expression.tag = "tag:yaml.org,2002:js";
+            return [key, expression];
+          }),
+        );
+      return scenario.format === "dsh"
+        ? stringify([
+            {
+              insert: [
+                {
+                  id: "rea-mcp",
+                  name: "@deepseek-ai/dsh-mcp-client",
+                  config: entry,
+                },
+              ],
+            },
+          ])
+        : JSON.stringify({ mcpServers: { rea: entry } });
+    };
+    const original = registration(
+      scenario.position === "value"
+        ? { CALLER_SETTING: "private\0value" }
+        : { ["CALLER\0SETTING"]: "private value" },
+    );
+    await writeFile(config, original);
+    const doctor = await cli.run({
+      arguments: [
+        "doctor",
+        "--client",
+        scenario.client,
+        "--skill=false",
+        "--json",
+      ],
+      cwd: home,
+      environment,
+    });
+    expect(doctor.exitCode).toBe(1);
+    expect(doctor.json).toMatchObject({
+      scope_checks: expect.arrayContaining([
+        expect.objectContaining({
+          name: `registration:${scenario.client}`,
+          ok: false,
+          classification: "config_drift",
+          remediation: expect.stringContaining("NUL"),
+        }),
+      ]),
+    });
+    const setup = await cli.run({
+      arguments: [
+        "setup",
+        "--client",
+        scenario.client,
+        "--skill=false",
+        "--yes",
+        "--json",
+      ],
+      cwd: home,
+      environment,
+    });
+    expect(setup.exitCode).toBe(1);
+    expect(setup.json).toMatchObject({
+      status: "needs_human",
+      plannedActions: [],
+      appliedActions: [],
+      remediation: expect.stringContaining("NUL"),
+    });
+    expect(setup.stdout + doctor.stdout).not.toContain("private");
+    expect(await readFile(config, "utf8")).toBe(original);
+    await expect(access(`${config}.rea.backup`)).rejects.toMatchObject({
+      code: "ENOENT",
+    });
+
+    const repaired = registration({
+      CALLER_SETTING: "ordinary\nvalue=二\t",
+      EMPTY_SETTING: "",
+    });
+    await writeFile(config, repaired);
+    const healthy = await cli.run({
+      arguments: [
+        "doctor",
+        "--client",
+        scenario.client,
+        "--skill=false",
+        "--json",
+      ],
+      cwd: home,
+      environment,
+    });
+    expect(healthy.exitCode, healthy.stdout).toBe(0);
+    expect(healthy.json).toMatchObject({ healthy: true });
+    expect(await readFile(config, "utf8")).toBe(repaired);
+  },
+);
+
 for (const scenario of scenarios)
   cliTest(
     `setup and doctor reject ${scenario.client} overrides rejected by MCP startup`,