fix(mcp): restrict stdio server environment

packages/coding-agent/src/step/mcp-environment.ts

@@ -8,6 +8,7 @@
  * describes starts fine.
  */
 
+import { DEFAULT_INHERITED_ENV_VARS } from "@modelcontextprotocol/sdk/client/stdio.js";
 import { readStoredCredential } from "../core/auth-storage.ts";
 import { getStepAuthPath } from "./auth.ts";
 
@@ -18,13 +19,22 @@ import { getStepAuthPath } from "./auth.ts";
  */
 export const STEP_LOGIN_SUPPLIED_ENV: readonly string[] = ["STEPFUN_API_KEY"];
 
-/** Resolve the environment passed to a plugin server, including Step login fallback. */
+/**
+ * Resolve the environment passed to a plugin server, including Step login fallback.
+ *
+ * Match the MCP SDK's default environment allowlist instead of exposing every
+ * variable held by the Step process to an arbitrary local MCP executable.
+ */
 export function resolveStepMcpEnvironment(
 	declared: Record<string, string> | undefined,
 	input: { env?: NodeJS.ProcessEnv; authPath?: string } = {},
 ): Record<string, string> {
 	const resolved: Record<string, string> = {};
-	for (const [key, value] of Object.entries(input.env ?? process.env)) if (value !== undefined) resolved[key] = value;
+	const inherited = input.env ?? process.env;
+	for (const key of DEFAULT_INHERITED_ENV_VARS) {
+		const value = inherited[key];
+		if (value !== undefined && !value.startsWith("()")) resolved[key] = value;
+	}
 	Object.assign(resolved, declared ?? {});
 	if (!resolved.STEPFUN_API_KEY?.trim()) {
 		const credential = readStoredCredential("step", input.authPath ?? getStepAuthPath());

packages/coding-agent/src/step/mcp.test.ts

@@ -46,6 +46,32 @@ afterEach(async () => {
 	await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })));
 });
 
+test("inherits only the MCP SDK safe environment defaults", () => {
+	const resolved = resolveStepMcpEnvironment(undefined, {
+		env: {
+			PATH: "/bin",
+			AWS_SECRET_ACCESS_KEY: "aws-secret",
+			OPENAI_API_KEY: "openai-secret",
+			UNRELATED_PRIVATE_TOKEN: "private",
+		},
+		authPath: "/definitely/missing/auth.json",
+	});
+
+	expect(resolved).toEqual({ PATH: "/bin" });
+});
+
+test("adds explicitly declared server variables without inheriting unrelated secrets", () => {
+	const resolved = resolveStepMcpEnvironment(
+		{ SERVER_TOKEN: "declared", PATH: "/server/bin" },
+		{
+			env: { PATH: "/shell/bin", UNRELATED_PRIVATE_TOKEN: "private" },
+			authPath: "/definitely/missing/auth.json",
+		},
+	);
+
+	expect(resolved).toEqual({ PATH: "/server/bin", SERVER_TOKEN: "declared" });
+});
+
 test("uses the logged-in Step credential only as a server env fallback", async () => {
 	const root = await mkdtemp(path.join(os.tmpdir(), "step-mcp-auth-"));
 	roots.push(root);
@@ -69,7 +95,7 @@ test("uses the logged-in Step credential only as a server env fallback", async (
 	expect(resolved.STEPFUN_API_KEY).toBe("login-key");
 });
 
-test("explicit declaration wins over both shell and login credentials", async () => {
+test("explicit declaration wins over the login credential", async () => {
 	const root = await mkdtemp(path.join(os.tmpdir(), "step-mcp-auth-"));
 	roots.push(root);
 	const authPath = path.join(root, "auth.json");

packages/coding-agent/src/step/plugins.ts

@@ -645,12 +645,11 @@ export async function diagnoseStepPlugin(
 	if (read.manifest.entry)
 		warnings.push("Executable plugin entries are recorded but not loaded by the Step marketplace facade.");
 	// Judged against the environment the matching servers are actually spawned
-	// with: `connectStepMcpServer` layers the process environment, the server's
-	// own declared `env`, and the Step login credential. Checking `process.env`
-	// alone reported every logged-in user as missing a variable they were never
-	// expected to export by hand. Only an inline `mcpServers` record can start a
-	// server — discovery skips a string declaration path — so that is the only
-	// shape whose declared `env` can satisfy a requirement.
+	// with: `connectStepMcpServer` layers the SDK's safe inherited environment,
+	// the server's own declared `env`, and the Step login credential. Only an
+	// inline `mcpServers` record can start a server — discovery skips a string
+	// declaration path — so that is the only shape whose declared `env` can
+	// satisfy a requirement.
 	const requiredEnvironment = read.manifest.provision?.requiresEnv ?? [];
 	if (requiredEnvironment.length > 0) {
 		const candidates = provisionedServerEnvironments(read.manifest).map((declared) =>
@@ -665,12 +664,12 @@ export async function diagnoseStepPlugin(
 		const missingOther = missingEnvironment.filter((name) => !STEP_LOGIN_SUPPLIED_ENV.includes(name));
 		if (missingLogin.length > 0) {
 			warnings.push(
-				`Plugin provisioning has no value for ${missingLogin.join(", ")}; run /login or export it before using this plugin.`,
+				`Plugin provisioning has no value for ${missingLogin.join(", ")}; run /login or declare it in the plugin's mcpServers env before using this plugin.`,
 			);
 		}
 		if (missingOther.length > 0) {
 			warnings.push(
-				`Plugin provisioning has no value for ${missingOther.join(", ")}; export it or declare it in the plugin's mcpServers env before using this plugin.`,
+				`Plugin provisioning has no value for ${missingOther.join(", ")}; declare it in the plugin's mcpServers env before using this plugin.`,
 			);
 		}
 	}