Skip to content

WIP Improving docker security#27670

Draft
br41nslug wants to merge 4 commits into
mainfrom
harden-docker
Draft

WIP Improving docker security#27670
br41nslug wants to merge 4 commits into
mainfrom
harden-docker

Conversation

@br41nslug

Copy link
Copy Markdown
Member

Scope

What's changed:

  • Dockerfile (runtime stage): apply outstanding OS-level security patches at build time via apk --no-cache upgrade (openssl, zlib, busybox, etc.).
  • Dockerfile (runtime stage): strip the bundled npm toolchain from the final image (npm, npx, corepack binaries + their node_modules, and the /root/.npm cache). Nothing at runtime shells out to npm/pnpm, and the bundled npm tree (notably picomatch) was the main remaining CVE source.
  • Dockerfile (runtime stage): bump pm2 from v5 to v6 (the previous corepack@latest global install in the runtime stage is dropped as part of this).
  • Dependency overrides (package.json > pnpm.overrides): pin/bump transitive packages flagged by CVE scanning: basic-ftp 5.2.2 -> 5.3.0, and new pins for @xmldom/xmldom 0.8.13, protobufjs 7.5.5, vitest>vite 7.3.2, and esbuild 0.28.0.
  • Catalog cleanup: remove the unused ajv 8.17.1 entry from pnpm-workspace.yaml.

Potential Risks / Drawbacks

  • pm2 v5 -> v6 is a major version bump. Behavior/config changes are possible; the ecosystem config and pm2-runtime start path should be validated against the v6 changelog before rollout.
  • apk upgrade reduces build reproducibility. The set of upgraded OS packages depends on what is current in the Alpine repos at build time, so two builds of the same commit can differ. It also slightly increases image build time.
  • Removing npm/npx/corepack from the runtime image breaks any downstream workflow or extension install step that assumed those binaries were present in the container. This is intentional, but worth calling out for anyone extending the image.
  • Override pins can drift / mask upstream fixes. Forcing transitive versions (e.g. esbuild, vite, protobufjs) may conflict with future direct-dependency bumps and will need periodic review.

Tested Scenarios

  • Image builds successfully from a clean context (docker build .).
  • Container boots: node cli.js bootstrap succeeds and pm2-runtime starts the app on :8055.
  • App is reachable and a basic admin login / API request works against the running container.

Review Notes / Questions

  • Special attention should be paid to the pm2 v5 -> v6 upgrade and whether ecosystem.config.cjs needs any changes for v6.
  • I would like a sanity check on the apk --no-cache upgrade approach vs. pinning specific package versions, given the reproducibility trade-off.
  • Confirm nothing in our build/extension tooling relies on npm/npx/corepack being present in the runtime image.
  • Confirm the ajv catalog removal has no remaining consumers (it appeared unused).

Checklist

  • Added or updated tests
  • Documentation PR created here or not required
  • OpenAPI package PR created here or not required

Fixes ENG-1430

@linear-code

linear-code Bot commented Jun 3, 2026

Copy link
Copy Markdown

ENG-1430

@ComfortablyCoding ComfortablyCoding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments or things that may be worth it if time permits:

  • Utilize DHI or equivalent
  • Do build + install pkgs in the builder and move only what is needed from builder to runtime to avoid manually removing inside runtime
  • Remove corepack as it is now >0.31. https://github.com/nodejs/corepack
  • Pin node alpine and introduce automated way to update it over apk --no-cache upgrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants