Merge pull request 'fix(secrets): injector wrapper must replace the entrypoint' (#119) from fix/injector-entrypoint into main

Caught migrating a real server: the pod crashlooped with
`.: cannot open /vault/secrets/grafana-creds`, and the reason was in the
generated spec:

  args: ["/bin/sh","-c",". /vault/secrets/grafana-creds; exec \"$0\" \"$@\"",
         "@leval/mcp-grafana"]

mcpd deliberately maps ContainerSpec.command -> k8s `args` so a package
server keeps its runner image's ENTRYPOINT (`npx -y`, `uvx`). Putting the
sourcing wrapper there meant the pod actually ran
`npx -y /bin/sh -c '...'` — npx trying to resolve a package called
/bin/sh. The agent had rendered the file correctly; nothing ever sourced it.

Adds `ContainerSpec.entrypoint`, which maps to k8s `command` and so
REPLACES the image entrypoint, and has wrapCommand fold that entrypoint
into the argv it returns (`npx -y` / `uvx` for package servers, the
server's own `entrypoint` field for dockerImage servers — already required
at validation for exactly this reason).

Two tests pin it: a wrapped server emits `command` and no `args`; an
unwrapped one still emits `args` and no `command`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki

src/mcpd/src/repositories/mcp-server.repository.ts

@@ -34,6 +34,8 @@ export class McpServerRepository implements IMcpServerRepository {
         env: data.env,
         healthCheck: (data.healthCheck ?? Prisma.JsonNull) as Prisma.InputJsonValue,
         volumes: data.volumes,
+        secretDelivery: data.secretDelivery,
+        entrypoint: (data.entrypoint ?? Prisma.DbNull) as Prisma.InputJsonValue,
       },
     });
   }
@@ -53,6 +55,8 @@ export class McpServerRepository implements IMcpServerRepository {
     if (data.env !== undefined) updateData['env'] = data.env;
     if (data.healthCheck !== undefined) updateData['healthCheck'] = (data.healthCheck ?? Prisma.JsonNull) as Prisma.InputJsonValue;
     if (data.volumes !== undefined) updateData['volumes'] = data.volumes;
+    if (data.secretDelivery !== undefined) updateData['secretDelivery'] = data.secretDelivery;
+    if (data.entrypoint !== undefined) updateData['entrypoint'] = (data.entrypoint ?? Prisma.JsonNull) as Prisma.InputJsonValue;
 
     return this.prisma.mcpServer.update({ where: { id }, data: updateData });
   }

src/mcpd/src/services/instance.service.ts

@@ -564,8 +564,13 @@ export class InstanceService {
           spec.serviceAccountName = identity;
           spec.automountServiceAccountToken = true;
           spec.annotations = this.serverIdentity!.annotationsFor(identity, spec.envFromSecret);
+          // Replaces the image entrypoint (see wrapCommand); the original argv
+          // is folded in, so spec.command must not also be emitted as args.
           const wrapped = this.serverIdentity!.wrapCommand(server, spec.command);
-          if (wrapped !== undefined) spec.command = wrapped;
+          if (wrapped !== undefined) {
+            spec.entrypoint = wrapped;
+            delete spec.command;
+          }
         } catch (idErr) {
           const msg = idErr instanceof Error ? idErr.message : String(idErr);
           return this.markInstanceError(instance, `secret identity provisioning failed: ${msg}`);

src/mcpd/src/services/k8s/manifest-generator.ts

@@ -259,7 +259,12 @@ function buildContainerSpec(spec: ContainerSpec) {
   // In Docker, spec.command maps to Cmd (args to entrypoint).
   // In k8s, we use `args` to pass arguments to the image's entrypoint,
   // preserving the runner image's entrypoint (uvx, npx -y, etc.)
-  if (spec.command && spec.command.length > 0) {
+  //
+  // `entrypoint` is the exception: it REPLACES the entrypoint (k8s `command`),
+  // which injected secret delivery needs so the sourcing shell can be PID 1.
+  if (spec.entrypoint && spec.entrypoint.length > 0) {
+    container.command = spec.entrypoint;
+  } else if (spec.command && spec.command.length > 0) {
     container.args = spec.command;
   }
 

src/mcpd/src/services/orchestrator.ts

@@ -62,6 +62,17 @@ export interface ContainerSpec {
   serviceAccountName?: string;
   /** Injected agents need the projected SA token; plain servers do not. */
   automountServiceAccountToken?: boolean;
+  /**
+   * REPLACES the image's ENTRYPOINT (k8s `command`), unlike `command`, which is
+   * appended to it as `args`.
+   *
+   * Needed only for injected secret delivery: the container has to run a shell
+   * that sources the rendered file before exec'ing the real process, and that
+   * shell must BE the entrypoint. Because it replaces the entrypoint, the argv
+   * here has to include whatever the image's entrypoint would have contributed
+   * (`npx -y`, `uvx`, ...).
+   */
+  entrypoint?: string[];
   /** Host port to bind (null = auto-assign) */
   hostPort?: number | null;
   /** Container port to expose */

src/mcpd/src/services/server-identity.service.ts

@@ -96,26 +96,6 @@ export class ServerIdentityService {
     });
   }
 
-  /**
-   * Rewrite argv so the rendered secrets are sourced before the server runs.
-   *
-   * `command` is what mcpd already computed: for package-based servers that is
-   * the runner image's entrypoint plus the package, which mcpd owns. For a
-   * dockerImage server it may be absent — the image's own ENTRYPOINT would run,
-   * and mcpd cannot introspect it, which is why `entrypoint` is required on the
-   * server row in that case (enforced at validation).
-   */
-  wrapCommand(
-    server: Pick<McpServer, 'env' | 'entrypoint'>,
-    command: string[] | undefined,
-  ): string[] | undefined {
-    const secretNames = this.secretNamesFor(server);
-    if (secretNames.length === 0) return command;
-    const argv = command ?? (server.entrypoint as string[] | null) ?? undefined;
-    if (argv === undefined || argv.length === 0) return command;
-    return wrapCommandForInjector(argv, secretNames);
-  }
-
   /** Identity name for a server — also the SA, policy and role name. */
   identityNameFor(serverName: string): string {
     return `${IDENTITY_PREFIX}${serverName}`;
@@ -135,6 +115,39 @@ export class ServerIdentityService {
     return [...names].sort((a, b) => a.localeCompare(b));
   }
 
+  /**
+   * Build the container ENTRYPOINT that sources the rendered secrets and then
+   * execs the real server.
+   *
+   * This must REPLACE the image's entrypoint, not extend it. mcpd normally puts
+   * a package server's argv into k8s `args` so the runner image's `npx -y` /
+   * `uvx` entrypoint still runs; a wrapper placed there would be executed BY
+   * npx (`npx -y /bin/sh -c ...`) and fail. So the argv returned here folds in
+   * whatever the image's entrypoint would have contributed.
+   *
+   * Returns undefined when there is nothing to wrap, leaving the pod on the
+   * normal entrypoint+args path.
+   */
+  wrapCommand(
+    server: Pick<McpServer, 'env' | 'entrypoint' | 'packageName' | 'runtime'>,
+    command: string[] | undefined,
+  ): string[] | undefined {
+    const secretNames = this.secretNamesFor(server);
+    if (secretNames.length === 0) return undefined;
+
+    // mcpd owns the runner images, so their entrypoints are known. A
+    // dockerImage server's is not introspectable — hence `entrypoint` being
+    // required on the row at validation time.
+    const imageEntrypoint = server.packageName
+      ? server.runtime === 'python'
+        ? ['uvx']
+        : ['npx', '-y']
+      : ((server.entrypoint as string[] | null) ?? undefined);
+    if (imageEntrypoint === undefined || imageEntrypoint.length === 0) return undefined;
+
+    return wrapCommandForInjector([...imageEntrypoint, ...(command ?? [])], secretNames);
+  }
+
   /**
    * Converge the identity for one server. Returns the identity name so the
    * caller can stamp it onto the pod spec.

src/mcpd/tests/injector-manifest.test.ts

@@ -136,3 +136,23 @@ describe('shell quoting survives adversarial values', () => {
     expect(out).toBe('');
   });
 });
+
+describe('the wrapper must REPLACE the image entrypoint, not extend it', () => {
+  // Regression: mcpd maps ContainerSpec.command -> k8s `args` so the runner
+  // image's `npx -y` / `uvx` entrypoint still runs. Emitting the wrapper there
+  // meant the pod actually ran `npx -y /bin/sh -c '...'`, which crashlooped.
+  it('emits container.command (entrypoint) and no args', () => {
+    const pod = generatePodSpec({
+      name: 'g', image: 'runner',
+      entrypoint: ['/bin/sh', '-c', '. /vault/secrets/s; exec "$0" "$@"', 'npx', '-y', '@leval/mcp-grafana'],
+    } as ContainerSpec, 'mcpctl-servers');
+    expect(pod.spec.containers[0]?.command?.[0]).toBe('/bin/sh');
+    expect(pod.spec.containers[0]?.args).toBeUndefined();
+  });
+
+  it('leaves the normal entrypoint+args path alone when not wrapping', () => {
+    const pod = generatePodSpec({ name: 'g', image: 'runner', command: ['@leval/mcp-grafana'] } as ContainerSpec, 'mcpctl-servers');
+    expect(pod.spec.containers[0]?.command).toBeUndefined();
+    expect(pod.spec.containers[0]?.args).toEqual(['@leval/mcp-grafana']);
+  });
+});

src/mcpd/tests/mcp-server-repository-fields.test.ts

@@ -0,0 +1,48 @@
+/**
+ * The server repository maps update/create fields explicitly, field by field.
+ * That means a new column silently does nothing until it is added here — and
+ * the failure is invisible: `mcpctl patch server x secretDelivery=injector`
+ * returns "patched" while the value never changes.
+ *
+ * Caught exactly that way in production. These assert the mapping instead.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import { McpServerRepository } from '../src/repositories/mcp-server.repository.js';
+import type { PrismaClient } from '@prisma/client';
+
+function prismaSpy() {
+  const update = vi.fn(async ({ data }: { data: Record<string, unknown> }) => data);
+  const create = vi.fn(async ({ data }: { data: Record<string, unknown> }) => data);
+  return { spy: { mcpServer: { update, create } } as unknown as PrismaClient, update, create };
+}
+
+describe('McpServerRepository field mapping', () => {
+  it('persists secretDelivery on update', async () => {
+    const { spy, update } = prismaSpy();
+    await new McpServerRepository(spy).update('id1', { secretDelivery: 'injector' });
+    expect(update.mock.calls[0]?.[0].data).toMatchObject({ secretDelivery: 'injector' });
+  });
+
+  it('persists entrypoint on update', async () => {
+    const { spy, update } = prismaSpy();
+    await new McpServerRepository(spy).update('id1', { entrypoint: ['/bin/x', '--flag'] });
+    expect(update.mock.calls[0]?.[0].data).toMatchObject({ entrypoint: ['/bin/x', '--flag'] });
+  });
+
+  it('leaves both untouched when not supplied', async () => {
+    const { spy, update } = prismaSpy();
+    await new McpServerRepository(spy).update('id1', { description: 'x' });
+    const data = update.mock.calls[0]?.[0].data ?? {};
+    expect(data).not.toHaveProperty('secretDelivery');
+    expect(data).not.toHaveProperty('entrypoint');
+  });
+
+  it('persists secretDelivery on create', async () => {
+    const { spy, create } = prismaSpy();
+    await new McpServerRepository(spy).create({
+      name: 'x', description: '', transport: 'STDIO', replicas: 1, env: [], volumes: [],
+      secretDelivery: 'injector',
+    } as never);
+    expect(create.mock.calls[0]?.[0].data).toMatchObject({ secretDelivery: 'injector' });
+  });
+});