Compare commits

...

5 Commits

Author SHA1 Message Date
Michal
f097c0f4d5 feat(gitea): rebuild gitea-mcp on a shell-bearing base
Some checks failed
CI/CD / lint (pull_request) Successful in 1m20s
CI/CD / test (pull_request) Successful in 1m30s
CI/CD / typecheck (pull_request) Successful in 2m52s
CI/CD / smoke (pull_request) Failing after 2m1s
CI/CD / build (pull_request) Successful in 2m23s
CI/CD / publish (pull_request) Has been skipped
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
2026-08-21 11:16:23 +01:00
c79bdab51b Merge pull request 'fix(secrets): wrap an image server's own command, not just entrypoint' (#121) from fix/injector-image-server-argv into main
Some checks failed
CI/CD / lint (push) Has been cancelled
CI/CD / typecheck (push) Has been cancelled
CI/CD / test (push) Has been cancelled
CI/CD / smoke (push) Has been cancelled
CI/CD / build (push) Has been cancelled
CI/CD / publish (push) Has been cancelled
2026-08-21 10:14:42 +00:00
Michal
913c0fbdc6 fix(secrets): wrap an image server's own command, not just entrypoint
Some checks failed
CI/CD / lint (pull_request) Successful in 1m25s
CI/CD / test (pull_request) Successful in 1m32s
CI/CD / typecheck (pull_request) Successful in 3m0s
CI/CD / smoke (pull_request) Failing after 2m0s
CI/CD / build (pull_request) Successful in 4m34s
CI/CD / publish (pull_request) Has been skipped
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
2026-08-21 11:14:39 +01:00
beb57baf58 Merge pull request 'fix(secrets): injector-delivered servers must attach, not exec' (#120) from feat/injector-attach-mode into main
Some checks failed
CI/CD / typecheck (push) Successful in 1m19s
CI/CD / lint (push) Successful in 2m24s
CI/CD / test (push) Successful in 1m27s
CI/CD / smoke (push) Failing after 1m59s
CI/CD / build (push) Successful in 4m40s
CI/CD / publish (push) Has been skipped
2026-08-21 01:05:53 +00:00
Michal
0a83f71648 fix(secrets): injector-delivered servers must attach, not exec
Some checks failed
CI/CD / lint (pull_request) Successful in 1m17s
CI/CD / typecheck (pull_request) Successful in 2m41s
CI/CD / test (pull_request) Successful in 1m28s
CI/CD / build (pull_request) Successful in 2m18s
CI/CD / smoke (pull_request) Failing after 3m3s
CI/CD / publish (pull_request) Has been skipped
With injected delivery the credentials 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 whole point.

mcpd picks its STDIO mode from the server's shape: an explicit command or
a packageName selects `exec`, a bare dockerImage selects `attach`. `exec`
spawns a NEW process inside the container, which never sourced the file
and therefore starts with empty credentials. The server comes up, answers
tools/list, and fails every authenticated call — precisely the silent
empty-token failure this feature exists to prevent.

Seen as `Readiness check (list_datasources) failed: process exited 1` on a
pod whose PID 1 demonstrably held the token (verified via /proc/1/environ:
GRAFANA_URL set, token 46 chars, nothing in the pod spec).

So secretDelivery: injector now forces attach regardless of server shape.
Safe because wrapCommandForInjector execs rather than forks, so PID 1 IS
the server. It also means no bouncer or HTTP shim is needed — mcpd's
PersistentStdio already holds one long-lived connection; it was simply
pointed at the wrong process.

Note this inverts an assumption I had earlier: image-entrypoint servers
were already on the attach path and would have worked; it is the
package-based ones that were broken. gitea remains the sole exception, and
for the unrelated reason that it is distroless and has no shell to source
the file.

Extracts the decision as a pure `chooseStdioMode()` so it can be tested —
it is subtle and fails silently. Six cases pinned, including that env
delivery still execs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
2026-08-21 02:05:51 +01:00
6 changed files with 259 additions and 25 deletions

View File

@@ -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"]

36
scripts/build-gitea-mcp.sh Executable file
View File

@@ -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"

View File

@@ -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';
let mode: StdioMode;
if (command && command.length > 0) {
mode = { kind: 'exec', command };
} else if (packageName) {
mode = { kind: 'exec', command: buildRuntimeSpawnCmd(runtime, packageName) };
} else if (dockerImage) {
mode = { kind: 'attach' };
} else {
throw new InvalidStateError(
`Server '${server.name}' (${server.id}) uses STDIO transport but has no ` +
`packageName, command, or dockerImage. Configure one of these.`,
);
}
const mode = chooseStdioMode({
name: server.name as string,
id: server.id as string,
secretDelivery: server.secretDelivery as string | null,
command,
packageName,
dockerImage,
runtime: server.runtime as string | null,
});
// 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

View File

@@ -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']
// 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 (imageEntrypoint === undefined || imageEntrypoint.length === 0) return undefined;
return wrapCommandForInjector([...imageEntrypoint, ...(command ?? [])], secretNames);
if (argv === undefined || argv.length === 0) return undefined;
return wrapCommandForInjector(argv, secretNames);
}
/**

View File

@@ -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" "$@"');
});
});

View File

@@ -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/);
});
});