Reuse one local GitHub backend across MCP reconnections

.github/workflows/local-gateway-security.yml

@@ -44,6 +44,9 @@ jobs:
       - name: Check upstream tool inventory against gateway policy
         run: node --test deploy/local-gateway/test/upstream-tool-inventory.test.mjs
 
+      - name: Check shared backend lifecycle with concurrent clients
+        run: node --test deploy/local-gateway/test/local-backend-lifecycle.test.mjs
+
       - name: Exercise isolated stdin-to-tmpfs runtime injection
         shell: bash
         run: |

deploy/local-gateway/README.md

@@ -65,13 +65,14 @@ Use `test` after deployment changes or upstream image updates.
 `local-mcp/stdio-proxy.mjs` serves local MCP clients without touching the gateway, its tmpfs import or the private backend network:
 
 ```text
-local client --stdio--> stdio-proxy.mjs --HTTP 127.0.0.1--> ephemeral github-mcp container
+local clients --stdio--> stdio-proxy.mjs --HTTP 127.0.0.1--> shared local github-mcp container
 ```
 
 - The proxy reads the PAT with the existing `Read-DpapiSecret` in a child `pwsh` and keeps it in process memory only.
-- The ephemeral backend uses the exact image pin from `compose.yaml`, runs in HTTP mode with the same hardening, publishes only on `127.0.0.1` and gets the PAT solely as a per-request `Authorization` header. It is never in a file, command-line argument, Docker `.Config.Env` or log.
+- The shared local backend uses the exact image pin from `compose.yaml`, runs in HTTP mode with the same hardening, publishes only on `127.0.0.1` and gets the PAT solely as a per-request `Authorization` header. It is never in a file, command-line argument, Docker `.Config.Env` or log.
 - Local clients see the full upstream tool surface. The ChatGPT discovery hiding is not applied because local clients have no first-party GitHub connector to deduplicate against. Denied tools, the `LurigeLars` write-owner restriction and Secret Scanning redaction from `public/policy.mjs` still apply.
-- Each proxy owns one labelled container, removes it on exit and removes orphans left by dead proxy processes.
+- Clients with the same pinned image and non-secret configuration reuse one labelled `github-mcp-local-stdio-*` container, including concurrent clients and background discovery. Closing a client does not remove the backend. The first client after Docker stops starts the same container again; no new daemon or scheduled task is required.
+- Dead legacy per-process backends are still removed by owner PID. Shared backends use a separate label so legacy proxies cannot remove them. Image/config changes get a separate backend; retire the previous one explicitly once its clients have exited.
 
 Register the runtime copy (after `sync-runtime.ps1`) in each client:
 
@@ -121,4 +122,4 @@ separate policy for capabilities that must actually be blocked.
 
 For `get_file_contents` calls targeting repositories owned by `LurigeLars`, the gateway converts upstream MCP embedded **text** resources into ordinary MCP text content before returning the response to ChatGPT. This avoids the per-conversation attachment-materialization prompt for small text files while keeping the official GitHub MCP server unchanged.
 
-The rewrite is intentionally narrow: binary resources, large-file resource links, directory listings and repositories owned by anyone else are left unchanged.
+The rewrite is intentionally narrow: binary resources, large-file resource links, directory listings and repositories owned by anyone else are left unchanged.

deploy/local-gateway/local-mcp/bridge.mjs

@@ -1,5 +1,7 @@
-// Pure building blocks for the local stdio proxy (Claude Code, Claude Desktop, Codex).
-// No process, Docker or secret-store side effects live here so they can be tested in CI.
+// Building blocks for the local stdio proxy (Claude Code, Claude Desktop, Codex).
+// Docker access is injected; this module never launches processes or reads secrets.
+
+import { createHash } from 'node:crypto';
 
 import {
   checkRequestPolicy,
@@ -9,6 +11,7 @@ import {
 
 export const LOCAL_CONTAINER_LABEL = 'com.lurigelars.github-mcp-local.stdio';
 export const LOCAL_OWNER_PID_LABEL = 'com.lurigelars.github-mcp-local.stdio.owner-pid';
+export const SHARED_CONTAINER_LABEL = 'com.lurigelars.github-mcp-local.shared-stdio';
 export const BACKEND_PORT = 8082;
 
 // The pinned official image is read from compose.yaml so the local proxy can never
@@ -21,12 +24,11 @@ export function parsePinnedImage(composeText) {
 
 // The GitHub PAT is deliberately absent: the backend runs in HTTP mode and receives
 // the token only as a per-request Authorization header from this process.
-export function buildDockerRunArgs({ image, configFile, name, ownerPid }) {
+export function buildDockerRunArgs({ image, configFile, name }) {
   return [
-    'run', '-d', '--rm',
+    'run', '-d',
     '--name', name,
-    '--label', `${LOCAL_CONTAINER_LABEL}=1`,
-    '--label', `${LOCAL_OWNER_PID_LABEL}=${ownerPid}`,
+    '--label', `${SHARED_CONTAINER_LABEL}=1`,
     '--publish', `127.0.0.1::${BACKEND_PORT}`,
     '--user', '65532:65532',
     '--read-only',
@@ -44,11 +46,35 @@ export function buildDockerRunArgs({ image, configFile, name, ownerPid }) {
   ];
 }
 
-export function parseDockerPort(output) {
-  for (const line of String(output).split(/\r?\n/)) {
-    const m = /^127\.0\.0\.1:(\d+)\s*$/.exec(line.trim());
-    if (m) return Number(m[1]);
+// A backend belongs to its pinned image/config, not to a short-lived MCP client.
+// shortcut: old image/config backends need explicit retirement after their clients exit.
+export function ensureLocalBackend({ image, configFile, configText, docker }) {
+  const key = createHash('sha256').update(image).update('\0').update(configText).digest('hex').slice(0, 16);
+  const name = `github-mcp-local-stdio-${key}`;
+  const inspect = () => docker(['inspect', '--format',
+    `{"image":{{json .Config.Image}},"shared":{{json (index .Config.Labels "${SHARED_CONTAINER_LABEL}")}},"running":{{json .State.Running}}}`, name]);
+  let state = inspect();
+  if (state.status !== 0) {
+    docker(buildDockerRunArgs({ image, configFile, name }));
+    // A concurrent client may have won the name; inspect that same backend.
+    state = inspect();
+  }
+  if (state.status !== 0) throw new Error('failed to create the shared local GitHub MCP backend');
+  const backend = JSON.parse(state.stdout);
+  if (backend.image !== image || backend.shared !== '1') throw new Error('shared local GitHub MCP backend identity mismatch');
+  if (backend.running === false) {
+    if (docker(['start', name]).status !== 0) throw new Error('failed to start the shared local GitHub MCP backend');
+  } else if (backend.running !== true) {
+    throw new Error('invalid shared local GitHub MCP backend state');
   }
+  const port = docker(['port', name, `${BACKEND_PORT}/tcp`]);
+  if (port.status !== 0) throw new Error('failed to resolve the shared local GitHub MCP backend port');
+  return { name, endpoint: `http://127.0.0.1:${parseDockerPort(port.stdout)}/` };
+}
+
+export function parseDockerPort(output) {
+  const m = /^127\.0\.0\.1:(\d+)$/.exec(String(output).trim());
+  if (m && Number(m[1]) > 0 && Number(m[1]) <= 65535) return Number(m[1]);
   throw new Error('local GitHub MCP backend is not published on 127.0.0.1');
 }
 

deploy/local-gateway/local-mcp/stdio-proxy.mjs

@@ -2,7 +2,7 @@
 //
 // Secret path (unchanged storage, same pattern as github-mcp.ps1):
 //   DPAPI file -> Read-DpapiSecret in a child pwsh -> stdout pipe -> this process memory
-//   -> per-request Authorization header over 127.0.0.1 to an ephemeral official backend.
+//   -> per-request Authorization header over 127.0.0.1 to a shared official backend.
 // The PAT is never written to a file, command-line argument, Docker Config.Env, image
 // layer or log. The production gateway, its tmpfs import and its private backend network
 // are not touched. The backend image is the exact pin from compose.yaml.
@@ -13,15 +13,12 @@ import { spawnSync } from 'node:child_process';
 import { readFileSync, existsSync } from 'node:fs';
 import { dirname, join, resolve } from 'node:path';
 import readline from 'node:readline';
-import { randomBytes } from 'node:crypto';
 import { fileURLToPath } from 'node:url';
 import {
   LOCAL_CONTAINER_LABEL,
   LOCAL_OWNER_PID_LABEL,
-  BACKEND_PORT,
-  buildDockerRunArgs,
   createForwarder,
-  parseDockerPort,
+  ensureLocalBackend,
   parsePinnedImage,
   validateRuntimeToken,
 } from './bridge.mjs';
@@ -46,7 +43,7 @@ for (const path of [configFile, secretStore]) {
 }
 
 const image = parsePinnedImage(readFileSync(join(ROOT, 'compose.yaml'), 'utf8'));
-const docker = (args, options = {}) => spawnSync('docker', args, { encoding: 'utf8', windowsHide: true, ...options });
+const docker = args => spawnSync('docker', args, { encoding: 'utf8', windowsHide: true, timeout: 30_000 });
 
 function readPatFromDpapi() {
   const literal = secretStore.replaceAll("'", "''");
@@ -84,17 +81,7 @@ function removeOrphanedBackends() {
 }
 
 let token = readPatFromDpapi();
-const name = `github-mcp-stdio-${process.pid}-${randomBytes(3).toString('hex')}`;
-let started = false;
-
-function cleanup() {
-  token = null;
-  if (started) {
-    started = false;
-    docker(['rm', '-f', name]);
-  }
-}
-process.on('exit', cleanup);
+process.on('exit', () => { token = null; });
 for (const sig of ['SIGINT', 'SIGTERM', 'SIGHUP']) process.on(sig, () => process.exit(0));
 process.on('uncaughtException', () => {
   log('unexpected proxy error; shutting down');
@@ -103,13 +90,9 @@ process.on('uncaughtException', () => {
 
 removeOrphanedBackends();
 
-const run = docker(buildDockerRunArgs({ image, configFile, name, ownerPid: process.pid }));
-if (run.status !== 0) fail('failed to start the local GitHub MCP backend container');
-started = true;
-
-const portOut = docker(['port', name, `${BACKEND_PORT}/tcp`]);
-if (portOut.status !== 0) fail('failed to resolve the local GitHub MCP backend port');
-const endpoint = `http://127.0.0.1:${parseDockerPort(portOut.stdout)}/`;
+const { name, endpoint } = ensureLocalBackend({
+  image, configFile, configText: readFileSync(configFile, 'utf8'), docker,
+});
 
 async function waitReady() {
   const deadline = Date.now() + 30_000;
@@ -124,7 +107,7 @@ async function waitReady() {
   fail('local GitHub MCP backend did not become reachable');
 }
 await waitReady();
-log(`ready (${image.split('@')[0]}, loopback backend)`);
+log(`ready (${image.split('@')[0]}, shared loopback backend ${name})`);
 
 const emit = obj => process.stdout.write(JSON.stringify(obj) + '\n');
 const handleLine = createForwarder({

deploy/local-gateway/test/local-backend-lifecycle.test.mjs

@@ -0,0 +1,68 @@
+// Real Docker regression: background discovery and concurrent clients must reuse
+// one backend. A unique non-secret config keeps this test separate from runtime.
+import assert from 'node:assert/strict';
+import { execFile, spawnSync } from 'node:child_process';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { promisify } from 'node:util';
+import test from 'node:test';
+import { pathToFileURL } from 'node:url';
+import { parsePinnedImage } from '../local-mcp/bridge.mjs';
+
+const exec = promisify(execFile);
+const root = path.resolve(import.meta.dirname, '..');
+const image = parsePinnedImage(fs.readFileSync(path.join(root, 'compose.yaml'), 'utf8'));
+const bridgeUrl = pathToFileURL(path.join(root, 'local-mcp/bridge.mjs')).href;
+const docker = args => spawnSync('docker', args, { encoding: 'utf8', windowsHide: true, timeout: 30_000 });
+
+test('concurrent and successive client processes reuse the same container, including after stop', { timeout: 90_000 }, async () => {
+  const temp = fs.mkdtempSync(path.join(os.tmpdir(), 'github-mcp-lifecycle-'));
+  const configFile = path.join(temp, 'github.env');
+  fs.writeFileSync(configFile, `GITHUB_TOOLSETS=repos\n# isolated lifecycle test ${path.basename(temp)}\n`);
+  const client = `
+    import { spawnSync } from 'node:child_process';
+    import { readFileSync } from 'node:fs';
+    import { ensureLocalBackend } from ${JSON.stringify(bridgeUrl)};
+    const configFile = ${JSON.stringify(configFile)};
+    const docker = args => spawnSync('docker', args, { encoding: 'utf8', windowsHide: true, timeout: 30000 });
+    console.log(JSON.stringify(ensureLocalBackend({ image: ${JSON.stringify(image)}, configFile,
+      configText: readFileSync(configFile, 'utf8'), docker })));
+  `;
+  const connect = async () => JSON.parse((await exec(process.execPath, ['--input-type=module', '-e', client], { windowsHide: true, timeout: 40_000 })).stdout);
+  let name;
+  try {
+    const clients = await Promise.allSettled(Array.from({ length: 4 }, connect));
+    name = clients.find(r => r.status === 'fulfilled')?.value.name;
+    for (const result of clients) assert.equal(result.status, 'fulfilled', result.reason?.message);
+    const first = clients[0].value;
+    assert.ok(clients.every(r => r.value.name === first.name));
+    const inspect = () => {
+      const result = docker(['inspect', '--format', '{{.Id}} {{.RestartCount}} {{.HostConfig.AutoRemove}}', name]);
+      assert.equal(result.status, 0);
+      return result.stdout.trim();
+    };
+    const original = inspect();
+    assert.match(original, /^[a-f0-9]{64} 0 false$/);
+    assert.deepEqual(await connect(), first);
+    assert.equal(inspect(), original, 'client process exit must leave the same backend running');
+    assert.equal(docker(['stop', name]).status, 0);
+    const restarted = await connect();
+    assert.equal(restarted.name, name);
+    assert.equal(inspect(), original, 'Docker stop/reconnect must start the existing container');
+
+    const replies = [];
+    const { createForwarder } = await import('../local-mcp/bridge.mjs');
+    const forward = createForwarder({ endpoint: restarted.endpoint, getToken: () => 'ghp_' + '0'.repeat(36), emit: reply => replies.push(reply), timeoutMs: 5000 });
+    for (let attempt = 0; attempt < 40; attempt++) {
+      await forward(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list' }));
+      if (replies.at(-1)?.result?.tools) break;
+      await new Promise(resolve => setTimeout(resolve, 250));
+    }
+    assert.ok(replies.at(-1)?.result?.tools.length > 0, 'shared backend must answer MCP tools/list');
+  } finally {
+    if (name) assert.equal(docker(['rm', '-f', name]).status, 0);
+    fs.unlinkSync(configFile);
+    fs.rmdirSync(temp);
+  }
+});

deploy/local-gateway/test/local-stdio-proxy.test.mjs

@@ -6,9 +6,11 @@ import test from 'node:test';
 import {
   buildDockerRunArgs,
   createForwarder,
+  ensureLocalBackend,
   parseDockerPort,
   parsePinnedImage,
   validateRuntimeToken,
+  SHARED_CONTAINER_LABEL,
 } from '../local-mcp/bridge.mjs';
 import { CHATGPT_HIDDEN_TOOLS, rewriteResponse } from '../public/policy.mjs';
 
@@ -26,19 +28,73 @@ test('local backend uses the exact compose image pin', () => {
 });
 
 test('docker run arguments never carry credentials and bind only to loopback', () => {
-  const args = buildDockerRunArgs({ image: parsePinnedImage(compose), configFile: 'C:\\x\\config\\github.env', name: 'n', ownerPid: 1 });
+  const args = buildDockerRunArgs({ image: parsePinnedImage(compose), configFile: 'C:\\x\\config\\github.env', name: 'n' });
   const joined = args.join(' ');
   assert.equal(args.includes('-e'), false);
   assert.equal(args.includes('--env'), false);
   assert.doesNotMatch(joined, /GITHUB_PERSONAL_ACCESS_TOKEN|secrets\.env|github_pat|ghp_/i);
   assert.equal(args.filter(a => a === '--env-file').length, 1);
   assert.ok(args[args.indexOf('--env-file') + 1].endsWith('github.env'));
   assert.ok(args.includes('127.0.0.1::8082'));
-  for (const flag of ['--read-only', '--rm']) assert.ok(args.includes(flag));
+  assert.ok(args.includes('--read-only'));
+  assert.equal(args.includes('--rm'), false, 'client exit must not remove the shared backend');
+  assert.ok(args.includes(`${SHARED_CONTAINER_LABEL}=1`));
+  assert.doesNotMatch(joined, /owner-pid/);
   assert.deepEqual(args.slice(args.indexOf('--cap-drop'), args.indexOf('--cap-drop') + 2), ['--cap-drop', 'ALL']);
   assert.deepEqual(args.slice(-3), ['http', '--port=8082', '--listen-host=0.0.0.0']);
 });
 
+test('successive clients reuse one backend and a stopped backend is started without recreation', () => {
+  const image = parsePinnedImage(compose);
+  const calls = [];
+  let exists = false;
+  let running = true;
+  const docker = args => {
+    calls.push(args);
+    if (args[0] === 'inspect') return exists
+      ? { status: 0, stdout: JSON.stringify({ image, shared: '1', running }) }
+      : { status: 1 };
+    if (args[0] === 'run') { exists = true; return { status: 0 }; }
+    if (args[0] === 'start') { running = true; return { status: 0 }; }
+    if (args[0] === 'port') return { status: 0, stdout: '127.0.0.1:49152\n' };
+    assert.fail('unexpected Docker operation: ' + args[0]);
+  };
+  const connect = () => ensureLocalBackend({ image, configFile: 'github.env', configText: 'GITHUB_TOOLSETS=repos', docker });
+  const first = connect();
+  assert.deepEqual(connect(), first);
+  running = false;
+  assert.deepEqual(connect(), first);
+  assert.equal(calls.filter(args => args[0] === 'run').length, 1);
+  assert.equal(calls.filter(args => args[0] === 'start').length, 1);
+  assert.equal(first.endpoint, 'http://127.0.0.1:49152/');
+});
+
+test('a concurrent creator winning the shared name is reused', () => {
+  const image = parsePinnedImage(compose);
+  let inspections = 0;
+  const docker = args => {
+    if (args[0] === 'inspect') return ++inspections === 1
+      ? { status: 1 }
+      : { status: 0, stdout: JSON.stringify({ image, shared: '1', running: true }) };
+    if (args[0] === 'run') return { status: 125 };
+    if (args[0] === 'port') return { status: 0, stdout: '127.0.0.1:49152\n' };
+    assert.fail('unexpected Docker operation: ' + args[0]);
+  };
+  assert.equal(ensureLocalBackend({ image, configFile: 'github.env', configText: '', docker }).endpoint, 'http://127.0.0.1:49152/');
+});
+
+test('wrong backend identity and Docker failure fail closed', () => {
+  const image = parsePinnedImage(compose);
+  for (const stdout of [JSON.stringify({ image: 'wrong-image', shared: '1', running: true }), JSON.stringify({ image, shared: '', running: true })]) {
+    assert.throws(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '',
+      docker: args => { assert.equal(args[0], 'inspect'); return { status: 0, stdout }; },
+    }), /identity mismatch/);
+  }
+  assert.throws(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '',
+    docker: () => ({ status: 1 }),
+  }), /failed to create/);
+});
+
 test('proxy source keeps the DPAPI path and never persists or logs the PAT', () => {
   assert.match(proxy, /Read-DpapiSecret/);
   assert.match(proxy, /github_pat\.dpapi/);
@@ -52,6 +108,8 @@ test('proxy source keeps the DPAPI path and never persists or logs the PAT', ()
 test('helpers validate port output and runtime token shape', () => {
   assert.equal(parseDockerPort('127.0.0.1:49152\n'), 49152);
   assert.throws(() => parseDockerPort('0.0.0.0:49152\n'));
+  assert.throws(() => parseDockerPort('127.0.0.1:49152\n0.0.0.0:49152\n'));
+  assert.throws(() => parseDockerPort('127.0.0.1:65536\n'));
   assert.equal(validateRuntimeToken(`  ${FAKE_TOKEN}\r\n`), FAKE_TOKEN);
   assert.throws(() => validateRuntimeToken(''));
   assert.throws(() => validateRuntimeToken('short'));