Merge pull request 'feat(gitea): rebuild gitea-mcp on a shell-bearing base' (#122) from feat/gitea-mcp-shell-base into main

Upstream docker.gitea.com/gitea-mcp-server is distroless — `Cmd` is
["/app/gitea-mcp"] and there is no /bin/sh at any path (verified by
exec'ing every candidate against the running pod).

That is fine until the server wants secretDelivery: injector. The OpenBao
agent renders secrets to a FILE, so mcpd wraps the container command as
`sh -c '. /vault/secrets/<name>; exec "$0" "$@"'`, which needs a shell.
gitea was the only server in the fleet blocked on this, and so the only
one whose token had to stay inline in its pod spec.

Copying one static Go binary onto debian:stable-slim is cheaper than
building and maintaining a static envexec shim, and follows the precedent
in deploy/Dockerfile.docmost-mcp — this repo already rebuilds third-party
MCP servers when it needs to change how they run.

ca-certificates is required rather than incidental: the binary talks HTTPS
to mysources.co.uk and the distroless base shipped a trust store we are
leaving behind. Verified in the built image: shell present, binary runs,
ca-certificates.crt present.

ENTRYPOINT is kept so the plain (non-injected) path behaves exactly like
upstream; mcpd replaces it with the sourcing wrapper only when the server
opts in.

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

deploy/Dockerfile.gitea-mcp

@@ -0,0 +1,36 @@
+# gitea-mcp-server, rebuilt on a shell-bearing base.
+#
+# WHY THIS EXISTS
+# ---------------
+# Upstream `docker.gitea.com/gitea-mcp-server` is distroless: `Cmd` is
+# ["/app/gitea-mcp"] and there is no /bin/sh at any path (verified by exec'ing
+# every candidate against the running pod).
+#
+# That is fine until the server needs `secretDelivery: injector`. The OpenBao
+# agent renders secrets to a FILE, so mcpd wraps the container command as
+# `sh -c '. /vault/secrets/<name>; exec "$0" "$@"'` — which needs a shell. With
+# no shell the pod cannot source its own credentials, and gitea was the single
+# server in the fleet blocked on this.
+#
+# Copying one static Go binary onto debian:stable-slim is cheaper than building
+# and maintaining a static "envexec" shim, and follows the precedent already set
+# by deploy/Dockerfile.docmost-mcp — this repo already rebuilds third-party MCP
+# servers when it needs to change how they run.
+#
+# ca-certificates is required, not incidental: the binary talks HTTPS to
+# https://mysources.co.uk and a distroless base ships its own trust store which
+# we are leaving behind.
+FROM debian:stable-slim
+
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends ca-certificates \
+    && rm -rf /var/lib/apt/lists/*
+
+COPY --from=docker.gitea.com/gitea-mcp-server:latest /app/gitea-mcp /usr/local/bin/gitea-mcp
+
+WORKDIR /app
+
+# Kept as ENTRYPOINT so the plain (non-injected) path behaves exactly like
+# upstream. mcpd REPLACES this with the sourcing wrapper when the server opts
+# into injected delivery — that is why a shell has to exist in the image.
+ENTRYPOINT ["/usr/local/bin/gitea-mcp"]

scripts/build-gitea-mcp.sh

@@ -0,0 +1,36 @@
+#!/bin/bash
+# Build gitea-mcp Docker image and push to Gitea container registry
+set -e
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+PROJECT_ROOT="$(dirname "$SCRIPT_DIR")"
+cd "$PROJECT_ROOT"
+
+# Load .env for GITEA_TOKEN
+if [ -f .env ]; then
+  set -a; source .env; set +a
+fi
+
+# Push directly to internal address (external proxy has body size limit)
+REGISTRY="10.0.0.194:3012"
+IMAGE="gitea-mcp"
+TAG="${1:-latest}"
+
+echo "==> Building gitea-mcp image..."
+podman build -t "$IMAGE:$TAG" -f deploy/Dockerfile.gitea-mcp .
+
+echo "==> Tagging as $REGISTRY/michal/$IMAGE:$TAG..."
+podman tag "$IMAGE:$TAG" "$REGISTRY/michal/$IMAGE:$TAG"
+
+echo "==> Logging in to $REGISTRY..."
+podman login --tls-verify=false -u michal -p "$GITEA_TOKEN" "$REGISTRY"
+
+echo "==> Pushing to $REGISTRY/michal/$IMAGE:$TAG..."
+podman push --tls-verify=false "$REGISTRY/michal/$IMAGE:$TAG"
+
+# Ensure package is linked to the repository
+source "$SCRIPT_DIR/link-package.sh"
+link_package "container" "$IMAGE"
+
+echo "==> Done!"
+echo "    Image: $REGISTRY/michal/$IMAGE:$TAG"

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/mcp-proxy-service.ts

@@ -62,6 +62,59 @@ function parseStreamableResponse(body: string): McpProxyResponse {
   return JSON.parse(body) as McpProxyResponse;
 }
 
+/**
+ * Decide how mcpd opens a STDIO session against a running container.
+ *
+ *   attach → connect to PID 1's stdin/stdout
+ *   exec   → spawn a NEW process inside the container
+ *
+ * Pure and exported so the choice is testable: it is subtle, and getting it
+ * wrong fails silently rather than loudly (see the injector case below).
+ */
+export function chooseStdioMode(server: {
+  name: string;
+  id: string;
+  secretDelivery?: string | null;
+  command?: string[] | null;
+  packageName?: string | null;
+  dockerImage?: string | null;
+  runtime?: string | null;
+}): StdioMode {
+  // Injected delivery MUST attach, whatever the server type.
+  //
+  // The secrets exist only in PID 1's environment: the container command is a
+  // shell that sources /vault/secrets/<name> and execs the real server, so the
+  // values live in that process and nowhere else — not in the pod spec, which
+  // is the entire point of the feature.
+  //
+  // `exec` spawns a NEW process, which never sourced the file and so starts
+  // with empty credentials. The server comes up, answers tools/list, and fails
+  // every authenticated call — the exact silent-empty-token failure this
+  // feature exists to prevent. Observed as
+  // `Readiness check (list_datasources) failed: process exited 1` on a pod
+  // whose PID 1 demonstrably held the token.
+  //
+  // Safe because wrapCommandForInjector execs rather than forks, so PID 1 IS
+  // the server process.
+  if (server.secretDelivery === 'injector') return { kind: 'attach' };
+
+  if (server.command !== null && server.command !== undefined && server.command.length > 0) {
+    return { kind: 'exec', command: server.command };
+  }
+  if (server.packageName !== null && server.packageName !== undefined && server.packageName !== '') {
+    return { kind: 'exec', command: buildRuntimeSpawnCmd(server.runtime ?? 'node', server.packageName) };
+  }
+  // Image entrypoint IS the MCP server.
+  if (server.dockerImage !== null && server.dockerImage !== undefined && server.dockerImage !== '') {
+    return { kind: 'attach' };
+  }
+
+  throw new InvalidStateError(
+    `Server '${server.name}' (${server.id}) uses STDIO transport but has no ` +
+    `packageName, command, or dockerImage. Configure one of these.`,
+  );
+}
+
 export class McpProxyService {
   /** Session IDs per server for streamable-http protocol */
   private sessions = new Map<string, string>();
@@ -159,20 +212,15 @@ export class McpProxyService {
       //   - command set      → exec the given command in the container.
       //   - dockerImage only → attach to PID 1 (image entrypoint IS the MCP server).
       //   - nothing          → unreachable, reject.
-      const runtime = (server.runtime as string | null) ?? 'node';
+      const mode = chooseStdioMode({
-      let mode: StdioMode;
+        name: server.name as string,
-      if (command && command.length > 0) {
+        id: server.id as string,
-        mode = { kind: 'exec', command };
+        secretDelivery: server.secretDelivery as string | null,
-      } else if (packageName) {
+        command,
-        mode = { kind: 'exec', command: buildRuntimeSpawnCmd(runtime, packageName) };
+        packageName,
-      } else if (dockerImage) {
+        dockerImage,
-        mode = { kind: 'attach' };
+        runtime: server.runtime as string | null,
-      } else {
+      });
-        throw new InvalidStateError(
-          `Server '${server.name}' (${server.id}) uses STDIO transport but has no ` +
-          `packageName, command, or dockerImage. Configure one of these.`,
-        );
-      }
 
       // Try persistent connection first
       try {
@@ -181,7 +229,7 @@ export class McpProxyService {
         this.removeClient(instance.containerId);
         // Fall back to one-shot exec when we have a command to run.
         if (mode.kind === 'exec') {
-          return sendViaStdio(this.orchestrator, instance.containerId, packageName, method, params, 120_000, command, runtime);
+          return sendViaStdio(this.orchestrator, instance.containerId, packageName, method, params, 120_000, command, (server.runtime as string | null) ?? 'node');
         }
         // Attach mode has no one-shot equivalent, but the failure is usually
         // a stale pipe from an in-place container restart — retry once

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,51 @@ 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;
+
+    // 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);
+
+    if (argv === undefined || argv.length === 0) return undefined;
+    return wrapCommandForInjector(argv, 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-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" "$@"');
+  });
+});

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/stdio-mode.test.ts

@@ -0,0 +1,44 @@
+/**
+ * How mcpd opens a STDIO session: attach to PID 1, or exec a new process.
+ *
+ * Subtle and silent when wrong. With injected secret delivery the credentials
+ * exist ONLY in PID 1's environment (a shell sourced /vault/secrets/<name> and
+ * exec'd the server), so an `exec` starts a process with empty credentials —
+ * the server comes up, answers tools/list, and fails every authenticated call.
+ */
+import { describe, it, expect } from 'vitest';
+import { chooseStdioMode } from '../src/services/mcp-proxy-service.js';
+
+const base = { name: 's', id: 'id1' };
+
+describe('chooseStdioMode', () => {
+  it('attaches for an injector server even though it has a packageName', () => {
+    // The regression: packageName would otherwise select exec, and exec loses
+    // the secrets entirely.
+    expect(chooseStdioMode({ ...base, secretDelivery: 'injector', packageName: '@leval/mcp-grafana' }))
+      .toEqual({ kind: 'attach' });
+  });
+
+  it('attaches for an injector server even though it has an explicit command', () => {
+    expect(chooseStdioMode({ ...base, secretDelivery: 'injector', command: ['node', 'x.js'] }))
+      .toEqual({ kind: 'attach' });
+  });
+
+  it('still execs a package server on the default env delivery', () => {
+    const m = chooseStdioMode({ ...base, secretDelivery: 'env', packageName: '@leval/mcp-grafana', runtime: 'node' });
+    expect(m.kind).toBe('exec');
+  });
+
+  it('still prefers an explicit command over packageName on env delivery', () => {
+    expect(chooseStdioMode({ ...base, secretDelivery: 'env', command: ['node', 'x.js'], packageName: 'p' }))
+      .toEqual({ kind: 'exec', command: ['node', 'x.js'] });
+  });
+
+  it('attaches for an image-entrypoint server, as before', () => {
+    expect(chooseStdioMode({ ...base, dockerImage: 'gitea/mcp:latest' })).toEqual({ kind: 'attach' });
+  });
+
+  it('rejects a server with no way to start', () => {
+    expect(() => chooseStdioMode({ ...base })).toThrow(/packageName, command, or dockerImage/);
+  });
+});