Skip to content

release.yml: permissionsセクションを追加し、NPM_TOKEN環境変数の設定を削除#96

Merged
nemuvski merged 1 commit intomainfrom
update-release-workflow
Nov 7, 2025
Merged

release.yml: permissionsセクションを追加し、NPM_TOKEN環境変数の設定を削除#96
nemuvski merged 1 commit intomainfrom
update-release-workflow

Conversation

@nemuvski
Copy link
Member

@nemuvski nemuvski commented Nov 6, 2025

標題の通りです!

#95 のPRを先にマージする。

Summary by CodeRabbit

リリースノート

  • Chores
    • リリースワークフローのセキュリティ設定を更新しました。認証メカニズムを改善し、内部リリースプロセスを強化しました。

このリリースではエンドユーザーに対する変更はありません。

@nemuvski nemuvski self-assigned this Nov 6, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

リリースワークフローに対して、OIDC トークン認証用の権限ブロック(id-token: writecontents: read)を追加し、NPM パブリッシュステップから NODE_AUTH_TOKEN 環境変数の設定を削除しました。

Changes

Cohort / File(s) 変更内容
リリースワークフロー構成
.github/workflows/release.yml
トップレベルの permissions ブロックを追加(OIDC 用に id-token: writecontents: read を指定)し、npm パブリッシュステップから NODE_AUTH_TOKEN 環境変数パスを削除

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 分

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRのタイトルは、release.ファイルに対して行われた主な変更内容(permissionsセクションの追加とNPM_TOKEN環境変数の削除)を正確かつ明確に説明しています。
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-release-workflow

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31f3f05 and 8885538.

📒 Files selected for processing (1)
  • .github/workflows/release.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/release.yml (2)

21-28: npm 認証設定の検証が必要です。

npm publish ステップでは環境変数を通じた認証が削除されていますが、前述の OIDC 設定なしに npm 認証が機能するか確実ではありません。以下を確認してください:

  • npm registry(registry.npmjs.org)が GitHub からの OIDC トークンを受け入れるよう設定されているか
  • actions/setup-node の設定に OIDC パラメータが必要か

確認後、必要に応じて npm 認証ステップを明示的に追加してください。


8-10: 元のレビューコメントは不正確です。ワークフローはOIDC認証が正しく設定されています。

ワークフロー検証結果:

項目 状態 備考
id-token: write 権限 ✓ 正しい OIDC に必須 (9行目)
registry-url パラメータ ✓ 正しい npmjs.org を指定 (21行目)
NODE_AUTH_TOKEN 削除 ✓ 正しい OIDC 使用時は不要
OIDC トークン交換ステップ ✓ 不要 npm CLI が自動検出

GitHub Actions + npm 信頼できるパブリッシャー (OIDC) の正しい実装フロー:

  1. id-token: write 権限を設定 ✓
  2. setup-node で registry-url を設定 ✓
  3. npm CLI (v11.5.1+) が GitHub Actions 環境でOIDC を自動検出 ✓
  4. 明示的なトークン交換ステップは不要 ✓

Node.js v24 (setup-node v6 が使用) に含まれる npm は OIDC 対応のバージョンです。この実装は公式のベストプラクティスに準拠しています。

Likely an incorrect or invalid review comment.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nemuvski nemuvski requested a review from dc7290 November 6, 2025 08:13
npm run build
- run: npm publish
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

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

ここは不要になったので削除(secretsの方も削除済みです)

Comment on lines +8 to +11
permissions:
id-token: write # Required for OIDC
contents: read

Copy link
Member Author

Choose a reason for hiding this comment

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

必要なpermissionを設定

Copy link
Member

@dc7290 dc7290 left a comment

Choose a reason for hiding this comment

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

LGTM👍

@nemuvski nemuvski merged commit 131b044 into main Nov 7, 2025
6 checks passed
@nemuvski nemuvski deleted the update-release-workflow branch November 7, 2025 06:33
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

Comments