Skip to content

fix(versioner): publish with npm via OIDC #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shellscape merged 8 commits into from
Jan 11, 2026
Merged

fix(versioner): publish with npm via OIDC #6

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3697854
fix(repo,versioner): publish with npm via OIDC
CharlieHelps Jan 11, 2026
e96045b
fix(versioner): accept --registry for publish
CharlieHelps Jan 11, 2026
3ce093e
fix(versioner): log and normalize publish registry
CharlieHelps Jan 11, 2026
85a0689
fix(repo,versioner): harden npm publish
CharlieHelps Jan 11, 2026
7eb5ac9
fix(versioner): validate registry flag
CharlieHelps Jan 11, 2026
31e9983
Apply suggestion from @shellscape
shellscape Jan 11, 2026
60b10d0
fix(versioner): simplify --registry validation
CharlieHelps Jan 11, 2026
4fb8a3d
fix(versioner): simplify registry logging
CharlieHelps Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ on:
branches:
- master

permissions:
contents: read

jobs:
publish:
if: |
Expand All @@ -15,29 +18,32 @@ jobs:

name: release

permissions:
contents: write
id-token: write

steps:
- name: Checkout Commit
uses: actions/checkout@v1
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 100
fetch-tags: true
Comment thread
shellscape marked this conversation as resolved.
ref: master
Comment thread
shellscape marked this conversation as resolved.
Outdated

- name: Setup Node
uses: actions/setup-node@v1
uses: actions/setup-node@v4
with:
node-version: 18
node-version: 20

- name: Checkout Master
run: |
git branch -f master origin/master
git checkout master
- name: Install PNPM
uses: pnpm/action-setup@v4

- name: Sanity Check
run: |
echo branch `git branch --show-current`;
echo node `node -v`;
echo pnpm `pnpm -v`

- name: Install PNPM
uses: pnpm/action-setup@v4

- name: Set Git Config
run: |
git config pull.rebase false
Expand Down Expand Up @@ -65,8 +71,16 @@ jobs:
# Note: this satisfies aws sdk for @dot/config tests
AWS_REGION: 'us-east-1'

- name: OIDC Preflight
shell: bash
run: |
if [ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" ] || [ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ]; then
echo "Missing GitHub Actions OIDC env vars (ACTIONS_ID_TOKEN_REQUEST_URL/TOKEN)." >&2
echo "Ensure the job requests permissions: id-token: write." >&2
exit 1
fi

echo "OIDC env vars detected."

- name: Release and Publish Changed Packages
run: pnpm --filter [HEAD^] --workspace-concurrency=1 release
env:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
4 changes: 2 additions & 2 deletions .npmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//registry.npmjs.org/:_authToken=${NPM_TOKEN}

# npm options
auth-type=legacy

# Publishing in CI uses GitHub OIDC (npm Trusted Publisher). For local publishing, authenticate via ~/.npmrc.

# pnpm options
always-auth = true
enable-pre-post-scripts = true
Expand Down
45 changes: 43 additions & 2 deletions packages/versioner/src/versioner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import 'source-map-support';

import { dirname, join, resolve } from 'path';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
import { tmpdir } from 'os';

import { getLog } from '@dot/log';
import parser from 'conventional-commits-parser';
Expand All @@ -25,6 +26,8 @@ const parserOptions = {
noteKeywords: ['BREAKING CHANGE', 'Breaking Change']
};
const reBreaking = new RegExp(`(${parserOptions.noteKeywords.join(')|(')})`);
const NPM_CLI_SPEC = 'npm@11.5.1';
const DEFAULT_NPM_REGISTRY = 'https://registry.npmjs.org/';

type Commit = parser.Commit<string | number | symbol>;

Expand Down Expand Up @@ -151,9 +154,47 @@ const publish = async (cwd: string) => {
return;
}

if (typeof argv.registry !== 'undefined' && typeof argv.registry !== 'string') {
throw new TypeError(`--registry must be a string, received: ${typeof argv.registry}`);
}

const registry = argv.registry || DEFAULT_NPM_REGISTRY;

log.info(chalk`\n{cyan Publishing to NPM}`);
log.info(chalk`{grey Registry:} ${registry}`);

await execa('pnpm', ['publish', '--no-git-checks'], { cwd, stdio: 'inherit' });
const packDir = mkdtempSync(join(tmpdir(), 'versioner-pack-'));
try {
await execa('pnpm', ['pack', '--pack-destination', packDir], { cwd, stdio: 'inherit' });

const tarballs = readdirSync(packDir).filter((file) => file.endsWith('.tgz'));
const [tarball] = tarballs;
if (!tarball) throw new Error(`Could not find packed tarball in: ${packDir}`);

const tarballPath = join(packDir, tarball);
Copy link
Copy Markdown
Contributor Author

@charliecreates charliecreates Bot Jan 11, 2026

Choose a reason for hiding this comment

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

readdirSync(packDir).filter(...); const [tarball] = tarballs; silently picks the first .tgz if multiple exist. That can publish the wrong artifact (e.g., leftover files if packDir is reused unexpectedly, or if pnpm pack ever emits more than one tarball). You should validate that exactly one tarball is present and fail otherwise.

Also consider sorting to make the selection deterministic if you intentionally support multiple outputs later.

Suggestion

Tighten the tarball discovery to be explicit and deterministic:

const tarballs = readdirSync(packDir)
  .filter((file) => file.endsWith('.tgz'))
  .sort();

if (tarballs.length !== 1) {
  throw new Error(
    `Expected exactly 1 packed tarball in ${packDir}, found ${tarballs.length}: ${tarballs.join(', ')}`
  );
}

const tarballPath = join(packDir, tarballs[0]);

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Copy link
Copy Markdown
Owner

@shellscape shellscape Jan 11, 2026

Choose a reason for hiding this comment

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

@CharlieHelps yes please

Copy link
Copy Markdown
Contributor Author

@charliecreates charliecreates Bot Jan 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Expand this to see my work.
  • Previously documented the maintainer-approved changes to publish in packages/versioner/src/versioner.ts, including DEFAULT_NPM_REGISTRY, normalizeRegistry, isValidRegistry, stricter tarball error messaging, and the updated .github/workflows/release.yml OIDC/npm setup for PR #6.
  • Updated packages/versioner/src/versioner.ts so publish derives and normalizes the registry internally from registryOverride or DEFAULT_NPM_REGISTRY, always passes --registry explicitly to pnpm dlx, and tightened CLI validation and error messages around the --registry argument.
  • Enhanced tarball selection in publish to require exactly one .tgz file and throw a detailed error including packDir, cwd, and the discovered tarball list when the count is not 1.
  • Adjusted the call site to await publish(cwd); after pull() so it relies on the internal registry resolution logic, then confirmed all merge conflict markers were removed from packages/versioner/src/versioner.ts.
  • Completed an interactive rebase preserving fix(repo,versioner): harden npm publish while skipping the redundant fix(versioner): validate registry arg commit once its logic was folded into the resolved publish implementation.
  • Verified the post-rebase history with git log --oneline --graph -n 10 (showing d145af1 fix(repo,versioner): harden npm publish on fix/versioner/npm-oidc-publish) and confirmed the expected normalizeRegistry, --registry usage, and improved tarball error are present in packages/versioner/src/versioner.ts.
  • Inspected .github/workflows/release.yml to ensure the OIDC-based npm publish job (with actions/setup-node@v4, id-token: write, fetch-depth: 0, and pnpm build/install steps) remains aligned with the hardened publish behavior in versioner.
  • Rebased fix/versioner/npm-oidc-publish onto remote commit 3ce093e, resolving the conflict in packages/versioner/src/versioner.ts to keep both the {grey Registry:} log.info line and inline .replace(/\/+$/, '') behavior alongside the stricter registry validation and tarball checks, then pushed updated commit 85a0689.
  • Ran pnpm -r build && pnpm lint && AWS_REGION=us-east-1 pnpm -r test --if-present successfully, with all workspace builds and tests passing and only the known no-param-reassign warning in packages/cdk/src/methods/security.ts, confirming the latest versioner changes are clean.
  • Requested fresh review and assignment on PR #6 via gh pr edit 6 --add-assignee shellscape --add-reviewer shellscape --add-reviewer CharlieHelps, re-ran CI=1 pnpm install --frozen-lockfile to validate workspace prepare hooks (including packages/versioner build) in a CI-like environment, and confirmed with git status that the working tree is clean and only behind origin/fix/versioner/npm-oidc-publish by one remote commit (no local changes).

Copy link
Copy Markdown
Contributor Author

@charliecreates charliecreates Bot Jan 11, 2026

Choose a reason for hiding this comment

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

#6 (comment)
#6 (comment)

Applied the requested changes on PR #6.

Changes
  • @dot/versioner now sorts .tgz artifacts from pnpm pack and fails unless exactly one tarball is produced (to avoid publishing the wrong artifact).
  • @dot/versioner publish supports --registry (defaults to https://registry.npmjs.org), logs the chosen registry, and rejects empty/whitespace values.
  • .github/workflows/release.yml now checks out the triggering ref (no ref: master) and uses fetch-depth: 0 for tag/history-dependent release logic.
Verification 
# Install
$ CI=1 pnpm install --frozen-lockfile

# Build
$ pnpm -r build

# Lint (1 warning)
$ pnpm lint

# Tests
$ AWS_REGION=us-east-1 pnpm -r test --if-present
  • Ava: 65 tests passed (3 packages)

const hasOidcEnv =
!!process.env.ACTIONS_ID_TOKEN_REQUEST_URL && !!process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN;
const provenanceArgs = hasOidcEnv ? ['--provenance'] : [];

log.info(chalk`{grey Using npm CLI:} ${NPM_CLI_SPEC}`);

await execa(
'pnpm',
[
'dlx',
NPM_CLI_SPEC,
'publish',
'--no-git-checks',
'--registry',
registry,
...provenanceArgs,
tarballPath
],
{ cwd, stdio: 'inherit' }
Comment thread
shellscape marked this conversation as resolved.
);
Comment thread
shellscape marked this conversation as resolved.
} finally {
rmSync(packDir, { force: true, recursive: true });
}
};

const pull = async () => {
Expand Down
Loading