Stop asking for CODEX_HOME when adding a Codex plugin (#1863)

Rhys Sullivan committed 2 months ago

Commit 00c2ab71 parent 4d4ad7c7 files · +88 −4Browse files

Unified · Split

7 files

.changeset/codex-plugins-no-credential.md +14 −0
e2e/local/codex-plugins.test.ts +9 −1
packages/plugins/mcp/src/api/group.ts +3 −0
packages/plugins/mcp/src/api/handlers.ts +2 −0
packages/plugins/mcp/src/react/CodexPluginAdd.tsx +5 −1
packages/plugins/mcp/src/sdk/plugin.test.ts +38 −1
packages/plugins/mcp/src/sdk/plugin.ts +17 −1

.changeset/codex-plugins-no-credential.md+14−0

@@ -0,0 +1,14 @@
	1	---
	2	"executor": patch
	3	"@executor-js/plugin-mcp": patch
	4	---
	5
	6	Adding a Codex plugin no longer asks for anything. `CODEX_HOME` is a path the
	7	scanner already resolved, but it was passed on the channel that makes an
	8	environment variable a credential — so the integration declared it as one, and
	9	a person who reached the connect step was shown a masked field for a value
	10	they should never have to know.
	11
	12	Stdio integrations can now carry non-secret environment as static
	13	configuration, separate from declared secrets. The Codex plugins use it: they
	14	declare no auth, and their connection is created for them.

e2e/local/codex-plugins.test.ts+9−1

                 command: plugin.command,
                 args: [...plugin.args],
                 ...(plugin.cwd === undefined ? {} : { cwd: plugin.cwd }),
-                ...(plugin.env === undefined ? {} : { env: { ...plugin.env } }),
+                // Static, not `env`: CODEX_HOME is a machine path the scanner
+                // resolved, so it must not become a credential to type.
+                ...(plugin.env === undefined ? {} : { staticEnv: { ...plugin.env } }),
                 ...(plugin.appServer === undefined
                   ? {}
                   : { appServer: { server: plugin.appServer.server } }),
               connections.map((connection) => String(connection.name)),
               `${slug} auto-connected`,
             ).toContain("default");
+            // Nothing to configure: the connection carries no credential, so
+            // the person is never shown a field for a path we already know.
+            expect(
+              connections.map((connection) => String(connection.template)),
+              `${slug} needs no credential`,
+            ).toEqual(["none"]);
             const tools = yield* client.tools.list({ query: { integration: slug } });
             const names = tools.map((tool) => tool.name);

packages/plugins/mcp/src/api/group.ts+3−0

   envVars: Schema.optional(Schema.Array(Schema.String)),
   /** One-shot secret env values (programmatic). The UI sends `envVars`. */
   env: Schema.optional(StringMap),
+  /** Non-secret environment stored on the integration and injected at spawn.
+   *  Unlike `env`, nothing here becomes a credential the user must type. */
+  staticEnv: Schema.optional(StringMap),
   cwd: Schema.optional(Schema.String),
   /** Protocol negotiation at connect: `auto` probes `server/discover` (spec
    *  2026-07-28) for modern-only servers; default is the legacy `initialize`

packages/plugins/mcp/src/api/handlers.ts+2−0

       args?: readonly string[];
       envVars?: readonly string[];
       env?: Record<string, string>;
+      staticEnv?: Record<string, string>;
       cwd?: string;
       versionNegotiation?: "legacy" | "auto";
       spawnPerCall?: boolean;
       args: p.args ? [...p.args] : undefined,
       envVars: p.envVars ? [...p.envVars] : undefined,
       env: p.env,
+      staticEnv: p.staticEnv,
       cwd: p.cwd,
       versionNegotiation: p.versionNegotiation,
       spawnPerCall: p.spawnPerCall,

packages/plugins/mcp/src/react/CodexPluginAdd.tsx+5−1

         command: plugin.command,
         args: [...plugin.args],
         ...(plugin.cwd !== undefined ? { cwd: plugin.cwd } : {}),
-        ...(plugin.env !== undefined ? { env: { ...plugin.env } } : {}),
+        // As STATIC env, not `env`: these are machine-derived paths the
+        // scanner already resolved, so they must not become credentials the
+        // person is asked to type. Sent this way the integration declares no
+        // auth, and its connection is created for them.
+        ...(plugin.env !== undefined ? { staticEnv: { ...plugin.env } } : {}),
         ...(plugin.appServer !== undefined ? { appServer: { ...plugin.appServer } } : {}),
       },
       reactivityKeys: integrationWriteKeys,

packages/plugins/mcp/src/sdk/plugin.test.ts+38−1

@@ -24,7 +24,7 @@ import { · 23 unmodified lines
24	} from "@executor-js/sdk/testing";	24	} from "@executor-js/sdk/testing";
25		25
26	import { createMcpConnector } from "./connection";	26	import { createMcpConnector } from "./connection";
27	import { mcpPlugin, userFacingProbeMessage } from "./plugin";	27	import { mcpPlugin, userFacingProbeMessage, toIntegrationConfig } from "./plugin";
28	import { McpInvocationError } from "./errors";	28	import { McpInvocationError } from "./errors";
29	import { extractManifestFromListToolsResult, deriveMcpNamespace, joinToolPath } from "./manifest";	29	import { extractManifestFromListToolsResult, deriveMcpNamespace, joinToolPath } from "./manifest";
30	import { makeAnnotationsMcpServer, serveMcpServer } from "../testing";	30	import { makeAnnotationsMcpServer, serveMcpServer } from "../testing";
@@ -1536,3 +1536,40 @@ describe("mcpPlugin endpoint telemetry", () => { · 1505 unmodified lines
1536	}),	1536	}),
1537	);	1537	);
1538	});	1538	});
		1539
		1540	describe("stdio static env", () => {
		1541	it("keeps non-secret env off the credential surface", () => {
		1542	// `env` declares a credential the user must type; `staticEnv` is machine
		1543	// knowledge stored on the integration. A path the scanner already resolved
		1544	// belongs in the second, or adding the integration asks for it.
		1545	const config = toIntegrationConfig({
		1546	transport: "stdio",
		1547	name: "Computer Use",
		1548	command: "/usr/local/bin/codex",
		1549	args: ["app-server"],
		1550	staticEnv: { CODEX_HOME: "/home/a/.codex" },
		1551	});
		1552
		1553	expect(config).toMatchObject({
		1554	env: { CODEX_HOME: "/home/a/.codex" },
		1555	authenticationTemplate: [{ slug: "none", kind: "none" }],
		1556	});
		1557	});
		1558
		1559	it("still treats declared env values as credentials", () => {
		1560	const config = toIntegrationConfig({
		1561	transport: "stdio",
		1562	name: "Secret server",
		1563	command: "run",
		1564	env: { API_KEY: "sk-live" },
		1565	});
		1566
		1567	expect(config).toMatchObject({
		1568	authenticationTemplate: [{ slug: "env", kind: "stdio_env", vars: ["API_KEY"] }],
		1569	});
		1570	expect(
		1571	(config as { env?: unknown }).env,
		1572	"the secret never lands in the config",
		1573	).toBeUndefined();
		1574	});
		1575	});

packages/plugins/mcp/src/sdk/plugin.ts+17−1

    *  add then auto-creates the connection holding them. The UI uses `envVars`
    *  instead and leaves the values to the connect step. */
   env: Schema.optional(Schema.Record(Schema.String, Schema.String)),
+  /** Non-secret environment the server needs, stored on the integration and
+   *  injected verbatim at spawn.
+   *
+   *  Separate from `env` because that channel makes every variable a
+   *  CREDENTIAL: it is declared as a `stdio_env` method and the user is asked
+   *  to type its value on a masked form. That is right for an API key and
+   *  wrong for a machine-derived path — a Codex plugin's `CODEX_HOME` is
+   *  already known to the scanner, is not a secret, and must never become a
+   *  field a person has to fill in. Nothing here is a credential, so it does
+   *  not appear in `authenticationTemplate`. */
+  staticEnv: Schema.optional(Schema.Record(Schema.String, Schema.String)),
   cwd: Schema.optional(Schema.String),
   /** Protocol negotiation at connect: `auto` probes `server/discover` (spec
    *  2026-07-28) for modern-only servers. Defaults to the legacy `initialize`
   return [...names];
 };
-const toIntegrationConfig = (input: McpServerInput): McpIntegrationConfigType => {
+/** Exported for tests: the credential/non-credential split is a security
+ *  boundary (a value in `env` becomes something the user is asked to type),
+ *  and asserting it through the whole add flow would not show it. */
+export const toIntegrationConfig = (input: McpServerInput): McpIntegrationConfigType => {
   if (input.transport === "stdio") {
     // The config only DECLARES the secret env vars by NAME (a `stdio_env`
     // method); their values are credentials and live on the connection, never
     // in this blob. Names come from the explicit `envVars` declaration and/or
     // the keys of any one-shot `env` values.
     const vars = stdioEnvVarNames(input);
+    const staticEnv = input.staticEnv;
     return {
       transport: "stdio",
       family: input.family?.trim() || undefined,
       command: input.command,
       args: input.args ? [...input.args] : undefined,
+      env: staticEnv !== undefined && Object.keys(staticEnv).length > 0 ? staticEnv : undefined,
       cwd: input.cwd,
       versionNegotiation: input.versionNegotiation,
       spawnPerCall: input.spawnPerCall,