diff --git a/README.md b/README.md index 90ccf47..6ae99af 100644 --- a/README.md +++ b/README.md @@ -776,6 +776,10 @@ Notes: - Claims are `ReadWriteOnce`, so a server with volumes is limited to one replica. Asking for more is rejected at validation rather than leaving the extra replicas unschedulable. +- A fresh claim mounts `root:root`, so mcpctl sets the pod's `fsGroup` to + **1000** — the conventional non-root uid in node/python images. Override with + `fsGroup` on the volume if an image uses a different one; containers running + as root are unaffected either way. - `storageClass` defaults to `MCPD_VOLUME_STORAGE_CLASS`, and is omitted entirely when neither is set. **Set it explicitly on any cluster with more than one default StorageClass**, where an omitted class binds diff --git a/src/cli/src/commands/apply.ts b/src/cli/src/commands/apply.ts index c4a32a6..1fe74e1 100644 --- a/src/cli/src/commands/apply.ts +++ b/src/cli/src/commands/apply.ts @@ -25,6 +25,7 @@ const VolumeSpecSchema = z.object({ mountPath: z.string().min(1).regex(/^\//, 'mountPath must be absolute'), sizeGb: z.number().int().min(1).max(1024).default(10), storageClass: z.string().optional(), + fsGroup: z.number().int().min(0).max(65535).optional(), }); const ServerSpecSchema = z.object({ diff --git a/src/mcpd/src/services/instance.service.ts b/src/mcpd/src/services/instance.service.ts index a722317..e78e5fb 100644 --- a/src/mcpd/src/services/instance.service.ts +++ b/src/mcpd/src/services/instance.service.ts @@ -381,6 +381,7 @@ export class InstanceService { mountPath: string; sizeGb?: number; storageClass?: string; + fsGroup?: number; }>; if (volumes.length > 0) { spec.volumes = volumes.map((v) => ({ @@ -390,6 +391,7 @@ export class InstanceService { ...(v.storageClass !== undefined && v.storageClass !== '' ? { storageClass: v.storageClass } : {}), + ...(v.fsGroup !== undefined ? { fsGroup: v.fsGroup } : {}), })); } // Package-based servers: command = [packageName, ...args] (entrypoint handles execution) diff --git a/src/mcpd/src/services/k8s/manifest-generator.ts b/src/mcpd/src/services/k8s/manifest-generator.ts index 90de376..b348142 100644 --- a/src/mcpd/src/services/k8s/manifest-generator.ts +++ b/src/mcpd/src/services/k8s/manifest-generator.ts @@ -1,5 +1,5 @@ import type { ContainerSpec, ContainerVolume } from '../orchestrator.js'; -import { DEFAULT_MEMORY_LIMIT, DEFAULT_NANO_CPUS } from '../orchestrator.js'; +import { DEFAULT_MEMORY_LIMIT, DEFAULT_NANO_CPUS, DEFAULT_VOLUME_FS_GROUP } from '../orchestrator.js'; const MCPCTL_LABEL = 'mcpctl.managed'; @@ -37,6 +37,7 @@ export interface K8sPodManifest { automountServiceAccountToken: boolean; nodeSelector?: Record; volumes?: Array<{ name: string; persistentVolumeClaim: { claimName: string } }>; + securityContext?: { fsGroup: number; fsGroupChangePolicy: string }; }; } @@ -95,13 +96,24 @@ function sanitizeName(name: string): string { return name.toLowerCase().replace(/[^a-z0-9-]/g, '-').replace(/^-+|-+$/g, '').slice(0, 63); } -function buildPodVolumes(spec: ContainerSpec): Pick { +function buildPodVolumes( + spec: ContainerSpec, +): Pick { if (!spec.volumes || spec.volumes.length === 0) return {}; return { volumes: spec.volumes.map((v) => ({ name: sanitizeName(v.claimName), persistentVolumeClaim: { claimName: sanitizeName(v.claimName) }, })), + securityContext: { + // Without this a fresh PVC mounts root:root and any image that dropped + // privileges cannot write to it. First declared fsGroup wins — a pod has + // exactly one, so per-volume groups are not expressible here. + fsGroup: spec.volumes[0]?.fsGroup ?? DEFAULT_VOLUME_FS_GROUP, + // Only chown when the top-level ownership is already wrong; the default + // ("Always") walks the whole volume on every single start. + fsGroupChangePolicy: 'OnRootMismatch', + }, }; } diff --git a/src/mcpd/src/services/orchestrator.ts b/src/mcpd/src/services/orchestrator.ts index 15716ad..7ac03ee 100644 --- a/src/mcpd/src/services/orchestrator.ts +++ b/src/mcpd/src/services/orchestrator.ts @@ -24,6 +24,16 @@ export interface ContainerVolume { * where an omitted class binds nondeterministically. */ storageClass?: string; + /** + * Group that owns the mounted volume (Kubernetes `fsGroup`). + * + * A fresh PVC mounts as root:root, so any image that drops privileges cannot + * write to it — docs-mcp-server fails with SQLITE_CANTOPEN, and its own + * Dockerfile tells you to `chown 1000:1000` the volume. Defaults to + * DEFAULT_VOLUME_FS_GROUP, which covers images using the conventional uid + * 1000; containers still running as root are unaffected either way. + */ + fsGroup?: number; } export interface ContainerSpec { @@ -117,3 +127,10 @@ export interface InteractiveExec { /** Default resource limits */ export const DEFAULT_MEMORY_LIMIT = 512 * 1024 * 1024; // 512 MB export const DEFAULT_NANO_CPUS = 500_000_000; // 0.5 CPU + +/** + * Group applied to mounted volumes when a server does not specify one. + * 1000 is the conventional non-root uid/gid in node, python and distro base + * images, which is what the images that drop privileges actually use. + */ +export const DEFAULT_VOLUME_FS_GROUP = 1000; diff --git a/src/mcpd/src/validation/template.schema.ts b/src/mcpd/src/validation/template.schema.ts index 95cc78b..73a01a6 100644 --- a/src/mcpd/src/validation/template.schema.ts +++ b/src/mcpd/src/validation/template.schema.ts @@ -18,6 +18,7 @@ export const VolumeSpecSchema = z.object({ mountPath: z.string().min(1).max(200).regex(/^\//, 'mountPath must be absolute'), sizeGb: z.number().int().min(1).max(1024).default(10), storageClass: z.string().max(100).optional(), + fsGroup: z.number().int().min(0).max(65535).optional(), }); export type VolumeSpecInput = z.infer; diff --git a/src/mcpd/tests/k8s-manifest.test.ts b/src/mcpd/tests/k8s-manifest.test.ts index 843e80d..bb8a20f 100644 --- a/src/mcpd/tests/k8s-manifest.test.ts +++ b/src/mcpd/tests/k8s-manifest.test.ts @@ -177,6 +177,24 @@ describe('volumes', () => { const pod = generatePodSpec(baseSpec, 'mcpctl-servers'); expect(pod.spec.volumes).toBeUndefined(); expect(pod.spec.containers[0]!.volumeMounts).toBeUndefined(); + expect(pod.spec.securityContext).toBeUndefined(); + }); + + it('sets fsGroup so a non-root image can write to a fresh claim', () => { + // A PVC mounts root:root. Without fsGroup, any image that drops privileges + // (docs-mcp-server runs as uid 1000) fails on first write. + const pod = generatePodSpec(volumeSpec, 'mcpctl-servers'); + expect(pod.spec.securityContext?.fsGroup).toBe(1000); + // "Always" re-chowns the entire volume on every start. + expect(pod.spec.securityContext?.fsGroupChangePolicy).toBe('OnRootMismatch'); + }); + + it('honours an explicit fsGroup', () => { + const pod = generatePodSpec( + { ...baseSpec, volumes: [{ claimName: 'c', mountPath: '/d', sizeGb: 1, fsGroup: 65534 }] }, + 'ns', + ); + expect(pod.spec.securityContext?.fsGroup).toBe(65534); }); it('carries volumes into a deployment pod template', () => {