Skip to content

Fiddle: update to use OIDC for npm publishing#236

Merged
krishan711 merged 1 commit into
mainfrom
deploy2
Dec 10, 2025
Merged

Fiddle: update to use OIDC for npm publishing#236
krishan711 merged 1 commit into
mainfrom
deploy2

Conversation

@krishan711
Copy link
Copy Markdown
Contributor

Description

Screenshots:

Checklist:

  • I have updated the CHANGELOG with a summary of my changes
  • I have updated the documentation accordingly

Copilot AI review requested due to automatic review settings December 10, 2025 16:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 modernizes the npm publishing workflow by migrating from the legacy NPM_TOKEN secret-based authentication to OIDC (OpenID Connect) authentication, which is more secure and doesn't require managing long-lived tokens.

Key Changes:

  • Adds OIDC permissions (id-token: write) to enable GitHub Actions to authenticate with npm
  • Removes the NPM_TOKEN secret and .npmrc file creation
  • Extracts the built package from Docker and runs publish commands directly in the GitHub Actions environment using Node.js 20

- name: Publish package to npm
if: startsWith(github.ref, 'refs/tags/v')
run: docker run -v $(pwd)/.npmrc:/root/.npmrc $DOCKER_IMAGE make publish
run: npm publish
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The npm publish command is missing the --provenance flag which is recommended when using OIDC authentication. This flag ensures that provenance attestations are generated and published, providing better supply chain security and transparency.

Add the --provenance flag:

run: npm publish --provenance
Suggested change
run: npm publish
run: npm publish --provenance

Copilot uses AI. Check for mistakes.
- name: Publish next package to npm
if: steps.vars.outputs.commit_count != '0' && github.ref == 'refs/heads/main'
run: docker run -v $(pwd)/.npmrc:/root/.npmrc $DOCKER_IMAGE make NEXT_VERSION=${{ steps.vars.outputs.commit_count }} publish-next
run: npx kiba-publish --next --next-version ${{ steps.vars.outputs.commit_count }}
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

When using OIDC authentication with npm, it's recommended to add the --provenance flag to enable npm provenance attestations. This provides better supply chain security by linking the published package to its source.

If kiba-publish supports it, consider adding the --provenance flag or ensuring it's passed through to the underlying npm publish command.

Copilot uses AI. Check for mistakes.
- name: Extract built package from Docker
run: |
docker create --name temp-container $DOCKER_IMAGE
docker cp temp-container:/app/. ./
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The Docker extraction step copies all files from the container to the current directory, which could potentially overwrite important files like .git, .github, or the workflow files themselves. This is particularly risky since the checkout includes the full git history (fetch-depth: 0).

Consider extracting only the necessary built artifacts:

- name: Extract built package from Docker
  run: |
    docker create --name temp-container $DOCKER_IMAGE
    docker cp temp-container:/app/dist ./dist
    docker cp temp-container:/app/package.json ./package.json
    docker rm temp-container

Or extract to a temporary directory first and then copy only what's needed.

Suggested change
docker cp temp-container:/app/. ./
docker cp temp-container:/app/dist ./dist
docker cp temp-container:/app/package.json ./package.json

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +51
- name: Extract built package from Docker
run: |
docker create --name temp-container $DOCKER_IMAGE
docker cp temp-container:/app/. ./
docker rm temp-container
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: '20'
registry-url: 'https://registry.npmjs.org'
- name: Update npm to latest
run: npm install -g npm@latest
- name: Publish next package to npm
if: steps.vars.outputs.commit_count != '0' && github.ref == 'refs/heads/main'
run: docker run -v $(pwd)/.npmrc:/root/.npmrc $DOCKER_IMAGE make NEXT_VERSION=${{ steps.vars.outputs.commit_count }} publish-next
run: npx kiba-publish --next --next-version ${{ steps.vars.outputs.commit_count }}
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

After extracting files from Docker with docker cp temp-container:/app/. ./, the node_modules directory from the Docker container will be in the workspace. However, the subsequent npx commands will try to use these modules which were built for the Docker environment (node:24.11.1) but are now running in the GitHub Actions environment with Node 20.

This could lead to binary incompatibility issues if there are any native modules. Consider either:

  1. Running npm install after extraction to ensure modules are compatible with the GitHub Actions environment
  2. Only extracting the dist folder and necessary files, then running npm install to get fresh dependencies
  3. Running the publish commands inside the Docker container as was done previously

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +48
- name: Update npm to latest
run: npm install -g npm@latest
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Updating npm to the latest version with npm install -g npm@latest could introduce breaking changes or unexpected behavior, especially since this is a critical deployment step.

Consider pinning to a specific npm version for reproducibility and stability:

run: npm install -g npm@10.9.2

This ensures the deployment process is consistent across runs and won't break if a new npm version introduces issues.

Suggested change
- name: Update npm to latest
run: npm install -g npm@latest
- name: Update npm to specific version
run: npm install -g npm@10.9.2

Copilot uses AI. Check for mistakes.
@krishan711 krishan711 merged commit c128722 into main Dec 10, 2025
10 checks passed
@krishan711 krishan711 deleted the deploy2 branch December 10, 2025 16:21
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