diff --git a/.github/workflows/image.yaml b/.github/workflows/image.yaml index 8fd3597e..c8d4e6f2 100644 --- a/.github/workflows/image.yaml +++ b/.github/workflows/image.yaml @@ -190,3 +190,42 @@ jobs: | docker run --rm -i --env DISPATCH_URL=http://localhost --env DISPATCH_AGENT_TOKEN=ci "$IMAGE" \ | tee /dev/stderr \ | grep -q '"serverInfo"' + + # #1173 regression guard: the MCP image must carry only the production + # closure plus tsx. The dev toolchain and its accepted dev-only advisory + # chain (eslint-config-next -> @next/eslint-plugin-next -> fast-glob -> + # micromatch -> braces, GHSA-vfj7-8cjw-p6xm) must never ship in it again. + - name: Assert MCP image ships no dev toolchain + env: + TAGS: ${{ steps.meta.outputs.tags }} + run: | + set -euo pipefail + IMAGE="$(printf '%s\n' "$TAGS" | head -n1)" + if [ "${{ github.event_name }}" != "pull_request" ]; then + docker pull "$IMAGE" + fi + echo "Asserting no dev toolchain in image: $IMAGE" + docker run --rm --entrypoint sh "$IMAGE" -c ' + rc=0 + # Advisory chain from SECURITY-ACCEPTED-RISKS (#1166): + # eslint-config-next -> @next/eslint-plugin-next -> fast-glob -> + # micromatch -> braces. Scan one nesting level too -- a duplicate + # can hide under a host package, scoped hosts included. + for pkg in eslint eslint-config-next @next/eslint-plugin-next fast-glob micromatch braces; do + for dir in "node_modules/$pkg" node_modules/*/node_modules/"$pkg" node_modules/@*/*/node_modules/"$pkg"; do + if [ -e "$dir" ]; then + echo "dev dependency present in MCP image: $dir" + rc=1 + fi + done + done + # Broader dev-tool smoke list, hoisted top level only (nested + # duplicates of these can be legitimate transitive prod content). + for pkg in vitest typescript ts-node; do + if [ -e "node_modules/$pkg" ]; then + echo "dev dependency present in MCP image: node_modules/$pkg" + rc=1 + fi + done + exit "$rc" + ' diff --git a/Dockerfile b/Dockerfile index d49d483e..d6364a1f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,6 +10,33 @@ WORKDIR /app COPY package.json package-lock.json* ./ RUN npm ci --omit=dev +# Production-only dependency tree for the MCP image: the app's full production +# closure (@modelcontextprotocol/sdk, zod, next, prisma + transitive), plus tsx +# as a separate layer -- tsx is the image entrypoint but must stay a +# devDependency so the main runner image never ships it. package.json and +# package-lock.json are unchanged in git; only this layer's copy is rewritten +# (see RUN notes below, #1173). +FROM base AS mcp-deps +WORKDIR /app +COPY package.json package-lock.json* ./ +# Do not COPY .npmrc into this stage: the repo .npmrc sets include=dev, +# which overrides --omit=dev and would re-admit the dev tree (#1166 proved +# the override; the CI assert would catch it, but fail at build here). +RUN npm ci --omit=dev +# Both direct npm install shapes are unusable here: installing "tsx@range" +# without --omit=dev reifies the whole tree and reinstalls every devDependency, +# and with --omit=dev npm skips an explicit package that package.json lists as +# dev-only. Recipe: rewrite THIS layer's copy of package.json (tsx -> +# dependencies, drop devDependencies; the range is still read from +# devDependencies so Renovate stays the source of truth) and let a plain +# --no-save install add only tsx + esbuild. The install pass also prunes +# stragglers this lock reifies even under --omit=dev (typescript et al.: +# dev:false in the lock via optional-peer refs); +# the "Assert MCP image ships no dev toolchain" step in .github/workflows/ +# image.yaml is the guard if npm's behavior drifts. +RUN node -e 'const f="./package.json",p=require(f);const range=p.devDependencies&&p.devDependencies.tsx;if(!range)throw new Error("tsx missing from devDependencies (#1173)");p.dependencies=p.dependencies||{};p.dependencies.tsx=range;delete p.devDependencies;require("fs").writeFileSync(f,JSON.stringify(p,null,2)+"\n")' \ + && npm install --no-save --no-audit --no-fund + FROM base AS builder WORKDIR /app ARG DATABASE_URL=postgresql://localhost:5432/dispatch @@ -67,14 +94,21 @@ ENTRYPOINT ["/docker-entrypoint.sh"] # MCP server image (stdio transport) for the in-cluster toolhive gateway. It # talks to the dispatch API over HTTP, so it needs neither prisma nor the Next -# build -- only tsx and the client code. +# build -- only tsx and the client code. Its node_modules comes from mcp-deps +# (npm ci --omit=dev + a tsx layer), NOT from deps: the published image must +# ship the runtime closure only, never the dev toolchain with its accepted +# dev-only advisory chain (#1173, GHSA-vfj7-8cjw-p6xm). FROM base AS mcp WORKDIR /app ENV NODE_ENV=production -COPY --from=deps /app/node_modules ./node_modules -COPY package.json tsconfig.json ./ +COPY --from=mcp-deps /app/node_modules ./node_modules +COPY tsconfig.json ./ +# Ship the mcp-deps layer's rewritten manifest (devDependencies removed) rather +# than the repo one: trivy-style manifest scanners must not see dev deps that +# are not in this image. tsx does not read package.json at runtime. +COPY --from=mcp-deps /app/package.json ./package.json # Only the server's import closure -- copying all of src/lib would put 111 # unrelated files, tests included, into a published image. COPY src/mcp/server.ts ./src/mcp/ diff --git a/Dockerfile.test.ts b/Dockerfile.test.ts index 3e9a7e91..2f7f0f58 100644 --- a/Dockerfile.test.ts +++ b/Dockerfile.test.ts @@ -81,3 +81,41 @@ describe("Dockerfile builder stage DATABASE_URL", () => { } }); }); + +/** + * Regression tests for issue #1173: the MCP image must be assembled from the + * production-only mcp-deps tree, never from the dev-inclusive `deps` stage, + * and mcp-deps itself must stay a production-only install. + */ +describe("Dockerfile MCP image wiring", () => { + it("ships the MCP image from the production-only mcp-deps tree", () => { + const stages = splitIntoStages(readDockerfile()); + const mcp = stages.get("mcp"); + expect(mcp, "expected a `mcp` stage in the Dockerfile").toBeDefined(); + expect(mcp).toContain("COPY --from=mcp-deps /app/node_modules ./node_modules"); + expect(mcp).toContain("COPY --from=mcp-deps /app/package.json ./package.json"); + expect( + mcp, + "mcp stage must not copy from the dev-inclusive deps stage", + ).not.toContain("COPY --from=deps "); + }); + + it("installs mcp-deps without dev dependencies", () => { + const stages = splitIntoStages(readDockerfile()); + const mcpDeps = stages.get("mcp-deps"); + expect(mcpDeps, "expected an `mcp-deps` stage in the Dockerfile").toBeDefined(); + expect(mcpDeps).toContain("npm ci --omit=dev"); + expect( + mcpDeps, + "mcp-deps must add tsx via the layer-local manifest rewrite (reads the range from devDependencies)", + ).toContain("devDependencies.tsx"); + expect( + mcpDeps, + "mcp-deps must install the tsx layer with --no-save (no manifest/lock writes)", + ).toContain("npm install --no-save"); + expect( + mcpDeps, + "mcp-deps stage must not set ENV DATABASE_URL (belt for issue #533)", + ).not.toMatch(/^ENV\s+DATABASE_URL\b/m); + }); +}); diff --git a/SECURITY-ACCEPTED-RISKS.md b/SECURITY-ACCEPTED-RISKS.md index fed83a9d..b047f91d 100644 --- a/SECURITY-ACCEPTED-RISKS.md +++ b/SECURITY-ACCEPTED-RISKS.md @@ -52,7 +52,7 @@ The following risks are tracked beyond npm advisories: - **Severity:** high. - **Affected:** all published `braces` versions (`<=3.0.3`; no patched release as of 2026-10-03). - **Reachability:** dev lint chain only — `eslint-config-next -> @next/eslint-plugin-next -> fast-glob -> micromatch -> braces`. -- **Shipping surface:** the main server runtime image installs production dependencies only (`Dockerfile` `prod-deps` stage: `npm ci --omit=dev`), so the chain is absent from the server image. Caveat: the separately published `-mcp` image runs from the `deps` stage tree (`npm ci`, devDependencies included, entrypoint `tsx`), so the lint chain is present there as inert tooling — the MCP server never invokes micromatch/braces. +- **Shipping surface:** none. The main server runtime image installs production dependencies only (`Dockerfile` `prod-deps` stage: `npm ci --omit=dev`), and since #1173 the separately published `-mcp` image ships the same production-only tree plus a tsx layer (`mcp-deps` stage) instead of the dev-inclusive `deps` tree; the `Assert MCP image ships no dev toolchain` step in `.github/workflows/image.yaml` guards it on every build. The chain exists only in dev installs. - **Exploit path:** requires feeding attacker-controlled deeply-nested glob patterns to micromatch during lint tooling; no request path in the server or MCP image reaches it. - **Decision:** accepted for the dev toolchain; the production audit gate is scoped with `--omit=dev --include=prod` (#1166), and dev advisories stay visible via the non-blocking dev-inclusive audit step in `.github/workflows/security-audit.yaml`. Revisit if an upstream patched release lands (then remove nothing — the scoped gate stays; optionally test whether the advisory clears). @@ -72,3 +72,4 @@ The following previously accepted risks have been retired: | Trivy action pinned to SHA | ✅ Resolved | `aquasecurity/trivy-action@ed142fd` (v0.36.0). The SHA pin is intentional: trivy is the release gate, so a floating tag must not reach a release build. Renovate's `github-tags` datasource cannot resolve a bare SHA pin (it only produced a `no-result` lookup failure on the dashboard), so the action is excluded from Renovate in `renovate.json` (`matchPackageNames: ["aquasecurity/trivy-action"]`, `enabled: false`) and is bumped manually, with the version comment, after reviewing an upstream release. | | `.npmrc` invalid omit config | ✅ Resolved | Fixed `omit=` → `omit=dev` | | `npm audit` gate silently auditing the dev tree | ✅ Resolved (#1166) | `.npmrc` `include=dev` (added #428) overrides `--omit=dev` in npm's config, so `npm audit --omit=dev` audited devDependencies too. The dev-only `braces` advisory GHSA-vfj7-8cjw-p6xm (all versions, no fix) exposed it on main. Fixed by adding an explicit `--include=prod` to the `audit` script so `--omit=dev` takes effect (prod + optional still audited); regression-guarded by `package-audit.test.ts`. | +| Dev-only advisory chain `eslint-config-next` -> `@next/eslint-plugin-next` -> `fast-glob` -> `micromatch` -> `braces` (GHSA-vfj7-8cjw-p6xm) | ✅ Resolved (image scope) | #1173: the `-mcp` image is now built from a production-only install plus a tsx layer, so the chain ships in no published image. It remains in dev installs only; `.github/workflows/image.yaml` asserts its absence from the `-mcp` image on every build. |