Skip to content

Require admin review for workflow changes, gate npm OIDC behind a tag-only environment - #1533

Merged
orta merged 1 commit into
mainfrom
deploy-via-github-actions
Aug 24, 2026
Merged

Require admin review for workflow changes, gate npm OIDC behind a tag-only environment#1533
orta merged 1 commit into
mainfrom
deploy-via-github-actions

Conversation

@orta

@orta orta commented Aug 24, 2026

Copy link
Copy Markdown
Member

Figured that someone could edit the workflow to drop the checks, so I've tightened up workflow security


Follow-up to #1532, which merged before these commits landed.

#1532 moved publishing to a tag-triggered workflow, on the reasoning that the tag ruleset is the deploy gate. That reasoning has a hole: npm's trusted publishing grants OIDC credentials based on repo + workflow filename, not on the ref. Anyone with write access (241 people) could push a branch containing an edited npm_publish.yml with on: push and obtain publish rights.

Two fixes:

  • environment: npm-release on the publish job. The environment is already created with a tag-only deployment policy and no branch policies, so a branch can never reach it and therefore never gets credentials. This is the actual fix.
  • CODEOWNERS requiring admin review for /.github/workflows/, CODEOWNERS itself, and .release-it.json. main is already configured with require_code_owner_reviews: true and required_approving_review_count: 0, so only PRs touching those paths need an admin approval — normal PRs are unaffected.

Needs doing on npmjs.com

The trusted publisher config for danger needs its environment field set to npm-release. Until then this PR will break publishing, so it should be set at merge time.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Warnings
⚠️

Please add a changelog entry for your changes. You can find it in CHANGELOG.md

Please add your change and name to the main section.

Generated by 🚫 dangerJS against 4f9406d

@orta
orta enabled auto-merge (squash) August 24, 2026 07:26
@orta
orta merged commit f175b31 into main Aug 24, 2026
2 checks passed
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