Skip to content

ci: migrate npm publish to OIDC trusted publishing#983

Merged
JohnMcLear merged 1 commit into
mainfrom
fix/oidc-trusted-publishing
May 22, 2026
Merged

ci: migrate npm publish to OIDC trusted publishing#983
JohnMcLear merged 1 commit into
mainfrom
fix/oidc-trusted-publishing

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

  • Replace stored `NPM_TOKEN` auth with npm OIDC trusted publishing — no expiring credential, no token rotation, no possibility of a stale secret blocking releases
  • Add `--provenance` so the npm package page shows the signed-by-GitHub attestation
  • Bump publish job's pnpm pin from 10 → 11 (matches test job; trusted publishing needs pnpm ≥ 10.4)
  • Remove the per-publish `npm access grant` step — the grant is idempotent and was already applied at initial publish; it doesn't need to run on every release, and an OIDC credential isn't authorised for it anyway

Why now

The post-merge publish for #981 failed E404 in run 26279691349 — "The requested resource 'ueberdb2@6.1.1' could not be found or you do not have permission to access it" — the classic shape of a stored token that's expired or had its scope changed. `ep_hljs` hit the same pattern four days ago. Migrating off the stored token is the permanent fix; rotating the token would just defer the problem to the next rotation cycle.

This also unblocks ether/etherpad#7831, whose CI is stuck waiting for ueberdb2 6.1.1 to actually land on npm.

One-time setup required after merge

On https://www.npmjs.com/package/ueberdb2/access:

  1. `Trusted Publishers → Add`
  2. Provider: `GitHub Actions`
  3. Organization: `ether`
  4. Repository: `ueberDB`
  5. Workflow filename: `npmpublish.yml`
  6. Environment: leave blank

Same recipe documented at the top of the `publish-npm` job in the workflow.

Test plan

  • Workflow YAML lints (`actionlint` clean; same shape as existing ether/ OIDC publishers)
  • After the one-time npm UI setup, the next push to main will publish `ueberdb2@6.1.2` via OIDC; `npm view ueberdb2 dist-tags.latest` should update
  • Once published, fix: page sessionstorage cleanup to avoid OOM (#7830) etherpad#7831 lockfile regenerates against `^6.1.0` and CI goes green

🤖 Generated with Claude Code

The npmpublish.yml workflow was authenticating via a stored NPM_TOKEN
secret that recently started failing E404 on PUT — first observed after
#981 merged at 6.1.1 (run 26279691349). Same E404 pattern that hit
ep_hljs four days ago.

Stored publish tokens have two failure modes that this PR removes
entirely:
  1. They expire / get rotated and silently break automated publishes.
  2. They survive in compromised CI logs / forks long enough to be
     abused.

OIDC trusted publishing exchanges a short-lived GitHub-issued id-token
for a per-publish credential at the registry. No secret on our side,
no expiry, and the resulting publish is attested with `--provenance`
so the npm package page shows the signing GHA run.

Removes:
  - "Set publishing config" step that consumed secrets.NPM_TOKEN
  - "Add package to etherpad organization" step (the access grant is
    idempotent and was set at initial publish; it doesn't need to run
    on every release and the OIDC credential isn't authorised for it)

Adds:
  - `permissions: { contents: write, id-token: write }` on the publish
    job (contents:write was already implicit via GITHUB_TOKEN for the
    version-bump push; id-token:write is the OIDC enabler)
  - `registry-url` on setup-node so the auth header lands at npmjs.org
  - `--no-git-checks` on pnpm publish (skip the dirty-tree guard that
    would otherwise trip on the just-pushed version-bump commit)
  - `--provenance` for the signing attestation

One-time setup required on https://www.npmjs.com/package/ueberdb2/access
before this lands — Trusted Publishers → Add → Provider: GitHub Actions,
Org: ether, Repo: ueberDB, Workflow: npmpublish.yml, Environment: blank.
Documented at the top of the publish-npm job.

Bumps the publish-job pnpm pin from 10 to 11 to match the test job;
trusted publishing requires pnpm >= 10.4 either way.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Migrate npm publish to OIDC trusted publishing

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace stored NPM_TOKEN with OIDC trusted publishing
• Add provenance attestation to published packages
• Bump pnpm version from 10 to 11
• Remove per-publish npm access grant step
Diagram
flowchart LR
  A["Stored NPM_TOKEN<br/>expires/rotates"] -->|Remove| B["OIDC Trusted<br/>Publishing"]
  B -->|GitHub id-token| C["Short-lived<br/>credential"]
  C -->|pnpm publish| D["npm registry"]
  D -->|--provenance| E["Signed attestation<br/>on package page"]

Loading

File Changes

1. .github/workflows/npmpublish.yml ✨ Enhancement +25/-16

Migrate to OIDC trusted publishing with provenance

• Add permissions block with contents: write and id-token: write for OIDC authentication
• Add registry-url: 'https://registry.npmjs.org' to setup-node action
• Bump pnpm version from 10 to 11
• Remove "Set publishing config" step that consumed NPM_TOKEN secret
• Remove "Add package to etherpad organization" step (idempotent access grant)
• Update pnpm publish command with --no-git-checks and --provenance flags
• Add comprehensive comments explaining OIDC setup and one-time npm configuration

.github/workflows/npmpublish.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 22, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. OIDC active during install 🐞 Bug ⛨ Security
Description
The publish-npm job enables id-token: write for the entire job, so any code executed during
pnpm install/pnpm run build can request a GitHub OIDC token and exchange it for an npm publish
credential. This increases the impact of a compromised dependency or build-time script because it
could publish to npm and also push commits/tags via contents: write.
Code

.github/workflows/npmpublish.yml[R68-70]

Evidence
The job-level permissions enable OIDC before any steps run, and the same job later executes
install/build commands (which can run arbitrary scripts from dependencies) before publishing.

.github/workflows/npmpublish.yml[59-70]
.github/workflows/npmpublish.yml[100-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow grants `id-token: write` (OIDC) and `contents: write` to the entire `publish-npm` job, while also running dependency installation and build steps. Any code executed in those steps can request an OIDC token and obtain publish credentials.

## Issue Context
`publish-npm` runs `pnpm install` and `pnpm run build` before `pnpm publish`, but OIDC permission is already enabled at the job level.

## Fix Focus Areas
- .github/workflows/npmpublish.yml[54-137]

### Implementation direction
- Split into two jobs:
 - **build** job: no `id-token: write` and no `contents: write`; runs `pnpm install` + `pnpm run build`; uploads the build output (or a packed tarball via `pnpm pack`).
 - **publish** job: depends on build job; minimal steps (download artifact + publish) and the only place where `id-token: write` is granted.
- If a separate job is not feasible, reduce executable surface where possible (e.g., avoid running install-time scripts unless strictly required) while keeping OIDC-enabled steps as late as possible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unpinned actions in publish 🐞 Bug ⛨ Security
Description
The publish-npm job runs with contents: write and id-token: write but uses GitHub Actions by
mutable version tags (for example pnpm/action-setup@v5 and actions/setup-node@v6). If any of
these tags are ever retargeted or an upstream action is compromised, that code would execute with
the ability to publish to npm and push to the repository.
Code

.github/workflows/npmpublish.yml[R76-86]

Evidence
The publish job is explicitly granted high-privilege permissions, and it invokes multiple actions
via mutable version tags within the same job scope.

.github/workflows/npmpublish.yml[68-70]
.github/workflows/npmpublish.yml[72-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
High-privilege workflow steps are using third-party actions referenced by mutable tags. Mutable tags increase supply-chain risk because the referenced code can change without a PR.

## Issue Context
The `publish-npm` job has `contents: write` and `id-token: write`, so any action it runs inherits powerful capabilities.

## Fix Focus Areas
- .github/workflows/npmpublish.yml[68-86]

### Implementation direction
- Replace each `uses: owner/action@vX` with an immutable commit SHA for that release (optionally keep a comment with the intended version).
- Pin at minimum the actions used in `publish-npm` (checkout, setup-node, cache, pnpm/action-setup).
- Add/ensure Dependabot (or equivalent) updates GitHub Actions pins regularly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@JohnMcLear JohnMcLear merged commit 028430c into main May 22, 2026
17 checks passed
@JohnMcLear JohnMcLear deleted the fix/oidc-trusted-publishing branch May 22, 2026 09:46
JohnMcLear added a commit to ether/etherpad that referenced this pull request May 22, 2026
Now that ether/ueberDB#983 unblocked the publish workflow (OIDC trusted
publishing), ueberdb2 6.1.2 is live on npm and the `^6.1.0` pin in
src/package.json resolves cleanly. Resolves the ERR_PNPM_OUTDATED_LOCKFILE
that was blocking CI on this PR.

29 SessionStore backend tests still green against the published tarball.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnMcLear added a commit to ether/etherpad that referenced this pull request May 22, 2026
* fix: page sessionstorage cleanup to avoid OOM (#7830)

SessionStore._cleanup() previously called `findKeys('sessionstorage:*',
null)`, materialising every session key into a single array. On decade-
old MariaDB installs with millions of sessions this OOMs the node
process within ~15 minutes — see #7830.

Switch to ueberdb2 6.1.0's findKeysPaged with a 500-key page size, and
yield to the event loop between pages so the DB driver can release each
page's buffered rows and request handlers can interleave.

The break is now driven by `page.length === 0` rather than `page.length
< CLEANUP_PAGE_SIZE` so a stubbed/throttled paged source still iterates
the full keyspace.

Adds a regression test that seeds 50 sessionstorage rows, monkey-patches
`DB.findKeysPaged` to use a 4-key page, runs cleanup, and asserts every
expired row is removed plus every valid row preserved across page
boundaries.

Closes #7830

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address Qodo review on #7831

Four follow-ups raised by Qodo on the session cleanup paging fix:

- DB.ts: fail-fast at init() if any required wrapper method (incl.
  findKeysPaged) is missing, so a stale ueberdb2 pin surfaces at boot
  rather than crashing the first cleanup run an hour later.
- SessionStore: bound a single _cleanup() run to 10 minutes. Under
  sustained session creation the keyspace can grow faster than cleanup
  drains it; without a budget the next scheduled run would never fire.
  When the budget hits, log a warning and let the next run continue.
- SessionStore: log the defensive `page[0] <= after` cursor-stall break.
  Previously the loop exited silently, leaving expired rows behind with
  no operator-visible signal of the backend regression.
- Tests: the paged-cleanup regression test now removes both expiredSids
  AND validSids in finally, so a failed assertion doesn't leak rows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: note paged session cleanup in CHANGELOG + settings template

CHANGELOG.md picks up an entry under 3.1.0 Notable fixes describing the
OOM cause, the paged iteration, the 10-minute per-run budget, the
cursor-stall logging, and the fail-fast init guard.

settings.json.template's sessionCleanup comment adds the page-size,
budget, and pointer to #7830 so admins can reason about the new
behaviour from the template alone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: regenerate lockfile against ueberdb2 6.1.2

Now that ether/ueberDB#983 unblocked the publish workflow (OIDC trusted
publishing), ueberdb2 6.1.2 is live on npm and the `^6.1.0` pin in
src/package.json resolves cleanly. Resolves the ERR_PNPM_OUTDATED_LOCKFILE
that was blocking CI on this PR.

29 SessionStore backend tests still green against the published tarball.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant