From 913c0fbdc6799f0fe245231a08a1c72b35e94e3f Mon Sep 17 00:00:00 2001
From: Michal <michal@itaz.eu>
Date: Fri, 21 Aug 2026 11:14:39 +0100
Subject: [PATCH] fix(secrets): wrap an image server's own command, not just
 `entrypoint`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Migrating docmost and my-home-assistant, both rendered their secret and
neither picked it up. The pod spec showed why:

  command: (empty)
  args:    ["node","build/index.js"]
  server.entrypoint: (unset)

wrapCommand consulted only `server.entrypoint` for non-package servers, so
with `entrypoint` unset it returned undefined, the wrapper was skipped
entirely, and the container ran its normal command — which never sourced
/vault/secrets/<name>. The failure is silent by construction: the agent
init container succeeds, the file is there, and the server simply starts
with empty credentials.

An explicit `command` on an image server is already a complete command
line — mcpd's exec mode would run exactly it — so it should be wrapped
verbatim. `entrypoint` is only needed when there is no command at all and
the image's own ENTRYPOINT would take over.

Now branches on the three real shapes: package server (prepend the runner
entrypoint mcpd owns), image + command (use verbatim), image only (require
the declared entrypoint).

Seven tests, one per shape plus the two undefined cases. Reintroducing the
old logic fails two of them — checked before keeping.

Both servers were rolled back to secretDelivery: env and are healthy; they
can migrate once this ships.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
---
 .../src/services/server-identity.service.ts   | 32 ++++++----
 src/mcpd/tests/injector-argv.test.ts          | 58 +++++++++++++++++++
 2 files changed, 80 insertions(+), 10 deletions(-)
 create mode 100644 src/mcpd/tests/injector-argv.test.ts

diff --git a/src/mcpd/src/services/server-identity.service.ts b/src/mcpd/src/services/server-identity.service.ts
index 50b154b..5581e04 100644
--- a/src/mcpd/src/services/server-identity.service.ts
+++ b/src/mcpd/src/services/server-identity.service.ts
@@ -135,17 +135,29 @@ export class ServerIdentityService {
     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;
+    // Build the COMPLETE argv the container should run. It differs by shape:
+    //
+    //   package server  — mcpd owns the runner image, whose ENTRYPOINT
+    //                     (`npx -y` / `uvx`) is lost once we take over
+    //                     `command`, so it must be prepended here.
+    //   image + command — `command` is already a full command line; mcpd would
+    //                     have run exactly it. Prepending anything breaks it.
+    //   image only      — the image's own ENTRYPOINT would run and mcpd cannot
+    //                     introspect it, so the row must declare `entrypoint`.
+    //
+    // Getting this wrong returns undefined and SILENTLY skips the wrapper: the
+    // agent still renders the file, nothing sources it, and the server starts
+    // with empty credentials. Observed on docmost and my-home-assistant, which
+    // carry a `command` but no `entrypoint`.
+    const hasPackage = server.packageName !== null && server.packageName !== undefined && server.packageName !== '';
+    const argv = hasPackage
+      ? [...(server.runtime === 'python' ? ['uvx'] : ['npx', '-y']), ...(command ?? [server.packageName as string])]
+      : command !== undefined && command.length > 0
+        ? command
+        : ((server.entrypoint as string[] | null) ?? undefined);
 
-    return wrapCommandForInjector([...imageEntrypoint, ...(command ?? [])], secretNames);
+    if (argv === undefined || argv.length === 0) return undefined;
+    return wrapCommandForInjector(argv, secretNames);
   }
 
   /**
diff --git a/src/mcpd/tests/injector-argv.test.ts b/src/mcpd/tests/injector-argv.test.ts
new file mode 100644
index 0000000..bcc74be
--- /dev/null
+++ b/src/mcpd/tests/injector-argv.test.ts
@@ -0,0 +1,58 @@
+/**
+ * Which argv the injector wrapper wraps, by server shape.
+ *
+ * Getting this wrong is SILENT: wrapCommand returns undefined, the wrapper is
+ * skipped, the agent still renders /vault/secrets/<name>, nothing sources it,
+ * and the server starts with empty credentials. Observed live on docmost and
+ * my-home-assistant, which carry a `command` but no `entrypoint`.
+ */
+import { describe, it, expect } from 'vitest';
+import { ServerIdentityService } from '../src/services/server-identity.service.js';
+import type { SecretBackendService } from '../src/services/secret-backend.service.js';
+
+const svc = new ServerIdentityService(
+  {} as unknown as SecretBackendService,
+  { namespace: 'mcpctl-servers', ensure: async () => undefined, remove: async () => undefined },
+);
+
+const withSecret = { env: [{ name: 'T', valueFrom: { secretRef: { name: 'creds', key: 'K' } } }] };
+/** The wrapper is `sh -c <script> arg0 arg1...`; argv starts at index 3. */
+const argvOf = (r: string[] | undefined): string[] | undefined => r?.slice(3);
+
+describe('wrapCommand argv by server shape', () => {
+  it('prepends the node runner entrypoint for a package server', () => {
+    const r = svc.wrapCommand({ ...withSecret, packageName: '@leval/mcp-grafana', runtime: 'node', entrypoint: null } as never, ['@leval/mcp-grafana']);
+    expect(argvOf(r)).toEqual(['npx', '-y', '@leval/mcp-grafana']);
+  });
+
+  it('prepends uvx for a python package server', () => {
+    const r = svc.wrapCommand({ ...withSecret, packageName: 'mcp-searxng', runtime: 'python', entrypoint: null } as never, ['mcp-searxng']);
+    expect(argvOf(r)).toEqual(['uvx', 'mcp-searxng']);
+  });
+
+  it('uses an image server\'s command verbatim — prepending anything breaks it', () => {
+    // The docmost/home-assistant regression: this used to return undefined.
+    const r = svc.wrapCommand({ ...withSecret, packageName: null, entrypoint: null } as never, ['node', 'build/index.js']);
+    expect(argvOf(r)).toEqual(['node', 'build/index.js']);
+  });
+
+  it('falls back to the declared entrypoint for an image server with no command', () => {
+    const r = svc.wrapCommand({ ...withSecret, packageName: null, entrypoint: ['/usr/local/bin/gitea-mcp'] } as never, undefined);
+    expect(argvOf(r)).toEqual(['/usr/local/bin/gitea-mcp']);
+  });
+
+  it('returns undefined when there is genuinely nothing to run', () => {
+    expect(svc.wrapCommand({ ...withSecret, packageName: null, entrypoint: null } as never, undefined)).toBeUndefined();
+  });
+
+  it('returns undefined for a server with no secret refs', () => {
+    expect(svc.wrapCommand({ env: [], packageName: 'p', runtime: 'node', entrypoint: null } as never, ['p'])).toBeUndefined();
+  });
+
+  it('always sources before exec, whatever the shape', () => {
+    const r = svc.wrapCommand({ ...withSecret, packageName: null, entrypoint: null } as never, ['node', 'x.js']);
+    expect(r?.[0]).toBe('/bin/sh');
+    expect(r?.[2]).toContain('. /vault/secrets/creds');
+    expect(r?.[2]).toContain('exec "$0" "$@"');
+  });
+});