Skip to content

[CFS] Add NPM_CONFIG_REGISTRY arg to docker publish - #11683

Open
Mike Harder (mikeharder) wants to merge 10 commits into
mainfrom
mikeharder/docker
Open

[CFS] Add NPM_CONFIG_REGISTRY arg to docker publish#11683
Mike Harder (mikeharder) wants to merge 10 commits into
mainfrom
mikeharder/docker

Conversation

@mikeharder

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@azure-sdk-automation

azure-sdk-automation Bot commented Aug 14, 2026

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the TypeSpec core Docker publish pipeline to better support npm/pnpm installs during image builds by introducing an authenticated npm configuration step and pinning the pnpm version used in the Docker build.

Changes:

  • Add a pipeline step to create/authenticate an .npmrc prior to running the Docker build.
  • Pin the pnpm version in docker/Dockerfile based on the repo’s packageManager field.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
eng/tsp-core/pipelines/publish.yml Adds an authenticated .npmrc creation step before docker build.
docker/Dockerfile Pins pnpm version to the repo-specified packageManager version before running pnpm install/build.
Suppressed comments (1)

eng/tsp-core/pipelines/publish.yml:106

  • The authenticated .npmrc created by create-authenticated-npmrc.yml is written to the agent user’s home directory, but the subsequent docker build doesn’t pass any registry/auth configuration into the build context. As a result, the Docker build won’t see the intended npm registry settings (and this also doesn’t match the PR’s stated goal of adding an NPM_CONFIG_REGISTRY argument). Consider either removing this step, or explicitly passing registry/auth into docker build (e.g., using BuildKit --secret ... to mount $(resolvedNpmrcPath) during RUN pnpm install, or passing a --build-arg NPM_CONFIG_REGISTRY=... and updating the Dockerfile to consume it).
              - template: /eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml

              - script: |
                  docker build -f ./docker/Dockerfile \
                    -t $(imageName):latest \
                    .

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docker/Dockerfile
Comment on lines +17 to 21
# Install exact version of pnpm to avoid problems with private registries
RUN npm install -g $(node -p "require('./package.json').packageManager")

RUN pnpm install --filter "@typespec/compiler..."
RUN pnpm --filter "@typespec/compiler..." run build
Copilot AI review requested due to automatic review settings August 15, 2026 01:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docker/Dockerfile:10

  • Creating an authenticated ./docker/.npmrc in the pipeline and then COPY . /app in the builder stage (plus NPM_CONFIG_USERCONFIG=/app/docker/.npmrc) bakes registry credentials into a Docker layer in the builder stage. Even though the final stage doesn’t copy it, the secret can still persist in local build cache or be exposed if the stage is ever exported/debugged.

Consider switching to a BuildKit secret mount for the .npmrc (and .dockerignore it) or otherwise avoid copying the authenticated .npmrc into any image layer.


ENV NPM_CONFIG_USERCONFIG=/app/docker/.npmrc

# Upgrade all packages per https://eng.ms/docs/more/containers-secure-supply-chain/updating.

Comment on lines +101 to 105
- template: /eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml
parameters:
npmrcPath: ./docker/.npmrc

- script: |
Copilot AI review requested due to automatic review settings August 15, 2026 01:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

eng/tsp-core/pipelines/publish.yml:106

  • The pipeline creates an authenticated docker/.npmrc and the Dockerfile does COPY . /app, so the auth token written by npmAuthenticate@0 can end up in the Docker build context and in the builder-stage image layers. Even though the final stage doesn’t copy it, this is still a secret-handling risk. Prefer BuildKit secrets (docker build --secret … + RUN --mount=type=secret) or otherwise ensure the generated .npmrc is excluded from the build context (e.g., create it outside . and/or add it to .dockerignore).
              - template: /eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml
                parameters:
                  npmrcPath: ./docker/.npmrc

              - script: |
                  docker build -f ./docker/Dockerfile \

docker/Dockerfile:9

  • ENV NPM_CONFIG_USERCONFIG=/app/docker/.npmrc combined with COPY . /app means an authenticated .npmrc generated during CI can be captured in the builder-stage filesystem/layers. Consider switching to BuildKit secret mounts for npm config (so tokens never enter the image layers) and keep the generated .npmrc out of the build context.
COPY . /app

ENV NPM_CONFIG_USERCONFIG=/app/docker/.npmrc

docker/Dockerfile:9

  • PR title mentions adding an NPM_CONFIG_REGISTRY arg, but the change actually introduces a generated authenticated .npmrc plus NPM_CONFIG_USERCONFIG. If the intent is only to make Docker publishing work with the Azure Artifacts registry, consider updating the PR title/description to match the implemented approach (or implement the build-arg approach described by the title).
ENV NPM_CONFIG_USERCONFIG=/app/docker/.npmrc

Copilot AI review requested due to automatic review settings August 15, 2026 01:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

docker/Dockerfile:37

  • The authenticated .npmrc created by npmAuthenticate@0 contains credentials. COPYing it into the final stage bakes those credentials into the image layer history (even if the file is later deleted in a RUN step). This can leak feed tokens to anyone who can pull the image. Consider switching to Docker BuildKit secrets (e.g., docker build --secret ... + RUN --mount=type=secret ...) or performing the npm install in the builder stage and copying only the installed artifacts into the final image.
COPY --from=builder /app/docker/.npmrc /tmp/.npmrc
ENV NPM_CONFIG_USERCONFIG=/tmp/.npmrc

RUN npm install --verbose -g /tmp/compiler.tgz && rm /tmp/compiler.tgz && rm /tmp/.npmrc

eng/tsp-core/pipelines/publish.yml:108

  • This creates an authenticated .npmrc (via npmAuthenticate@0) inside the Docker build context (./docker/.npmrc) and the Dockerfile then COPYs it into the image. That combination risks publishing registry credentials in the image layer history. Prefer using Docker BuildKit secrets (DOCKER_BUILDKIT=1 docker build --secret id=npmrc,src=...) and updating the Dockerfile to read the npmrc via RUN --mount=type=secret so the token never enters the image filesystem/layers.
              - template: /eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml
                parameters:
                  npmrcPath: ./docker/.npmrc

              - script: |
                  docker build -f ./docker/Dockerfile \
                    -t $(imageName):latest \
                    .

docker/Dockerfile:37

  • ENV NPM_CONFIG_USERCONFIG=/tmp/.npmrc is set in the final image but the file is deleted in the same layer. This leaves the runtime container with an env var pointing at a non-existent config file, which can cause confusing npm/pnpm behavior for users (or any future npm usage in the image). Prefer scoping the config to the install command instead of persisting it as an ENV.

This issue also appears on line 34 of the same file.

COPY --from=builder /app/docker/.npmrc /tmp/.npmrc
ENV NPM_CONFIG_USERCONFIG=/tmp/.npmrc

RUN npm install --verbose -g /tmp/compiler.tgz && rm /tmp/compiler.tgz && rm /tmp/.npmrc

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants