Switch to NPM OIDC publishing #60
Conversation
WalkthroughConsolidates release and prerelease CI into a new publish workflow, removes legacy prerelease/release workflows, updates GitHub Actions and Node versions, removes public npm-token inputs from local composite actions, and upgrades Yarn from 3.1.1 to 4.12.0 with related yarnrc changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 actionlint (1.7.9).github/workflows/publish.yml30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details (expression) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
78-84: Duplicateyarninvocations in setup job.Lines 78 and 84 run
yarntwice—once afterchanged-frontier-evmand again afterchanged-substrate-wasm. Consolidate to a single run at the beginning of the setup steps to avoid redundant installs and improve clarity:steps: - uses: actions/checkout@v5 with: fetch-depth: 100 + - run: yarn + - uses: marceloprado/has-changed-path@v1 id: changed-acala-evm with: paths: packages/acala-evm ...other checks... - - run: yarn - uses: marceloprado/has-changed-path@v1 id: changed-substrate-wasm with: paths: packages/substrate-wasm - - run: yarn
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
.yarn/plugins/@yarnpkg/plugin-interactive-tools.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-typescript.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-version.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-3.1.1.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.12.0.cjsis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.github/actions/create-prerelease/action.yml(1 hunks).github/actions/create-release/action.yml(1 hunks).github/workflows/pr.yml(1 hunks).github/workflows/prerelease.yml(0 hunks).github/workflows/publish.yml(1 hunks).github/workflows/release.yml(0 hunks).husky/pre-commit(0 hunks).yarnrc.yml(2 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (3)
- .husky/pre-commit
- .github/workflows/release.yml
- .github/workflows/prerelease.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/publish.yml
30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
177-177: property "changed-acala-evm" is not defined in object type {}
(expression)
183-183: property "changed-frontier-evm" is not defined in object type {}
(expression)
189-189: property "changed-moonbeam-evm" is not defined in object type {}
(expression)
189-189: property "changed-frontier-evm" is not defined in object type {}
(expression)
195-195: property "changed-ethermint-evm" is not defined in object type {}
(expression)
195-195: property "changed-ethermint-evm" is not defined in object type {}
(expression)
201-201: property "changed-substrate-wasm" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (5)
package.json (1)
43-43: Yarn 4 upgrade is straightforward. Node.js versions lower than 18.12 are no longer supported, and this aligns with the Node 22 upgrades elsewhere in the PR. Yarn 4 auto-sets compressionLevel: mixed for zero-installs on migration, and compressionLevel now defaults to 0 rather than mixed, which is handled in.yarnrc.yml..github/workflows/pr.yml (1)
12-26: Modernization to v5 GitHub Actions and Node 22 looks solid. The id-token: write permission allows GitHub Actions to generate OIDC tokens, though this file doesn't need it since it's not publishing. The workflow steps are clean and idiomatic..github/actions/create-release/action.yml (1)
1-24: Correct OIDC migration by removing npm-token input. The npm CLI automatically detects OIDC environments and uses them for authentication, with each publish using short-lived, cryptographically-signed tokens specific to the workflow. The addition of REPO_TOKEN for GitHub release creation (lines 20–24) is the right pattern..yarnrc.yml (1)
5-19: Yarn 4 configuration changes are intentional and correct. enableGlobalCache now defaults to true and compressionLevel defaults to 0. By explicitly settingenableGlobalCache: falseandcompressionLevel: mixed, the project preserves zero-installs behavior and tighter git diffs—a deliberate choice for this monorepo's setup..github/actions/create-prerelease/action.yml (1)
1-20: Removal of npm-token aligns with the OIDC publishing shift. Likecreate-release/action.yml, this action relies on the npm CLI's auto-detection of OIDC credentials from the GitHub Actions environment, eliminating manual token handling.
| fetch-depth: 0 | ||
|
|
||
| - id: get_commit_message | ||
| run: | |
There was a problem hiding this comment.
Unsafe use of untrusted context variable in inline script.
Line 30 directly uses github.event.head_commit.message in a shell script, which is untrusted user input and can lead to script injection attacks. Pass it through an environment variable instead:
- id: get_commit_message
run: |
+ commit_msg_raw="${{ github.event.head_commit.message }}"
- if [ -n "${{ github.event.head_commit.message }}" ]
+ if [ -n "$commit_msg_raw" ]
then
- commit_msg="${{ github.event.head_commit.message }}"
+ commit_msg="$commit_msg_raw"
echo "commit-message=${commit_msg}" | head -n 1 >> "$GITHUB_OUTPUT"
else
commit_message=$(git log -1 --pretty=%B | head -n 1)
echo "commit-message=$commit_message" >> "$GITHUB_OUTPUT"
fiAlso, the release job's condition on line 89–92 references github.event.head_commit.message directly instead of using the pre-ci output. Update it to: startsWith(needs.pre-ci.outputs.commit-message, '[release]') for consistency and safety.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.9)
30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
.github/workflows/publish.yml around lines 30 and 89-92: the workflow directly
injects github.event.head_commit.message into an inline shell script (line 30)
and later uses it in the release job condition (lines 89–92), which is unsafe.
Fix by exporting github.event.head_commit.message into an environment variable
(or set it as a pre-ci job output) and reference that env var inside the run
block instead of embedding the raw value; additionally update the release job
condition to use the pre-ci output:
startsWith(needs.pre-ci.outputs.commit-message, '[release]') so all uses read
from the safe, sanitized variable/output rather than
github.event.head_commit.message directly.
| release: | ||
| name: Release Publish | ||
| needs: [pre-ci, setup] | ||
| if: > | ||
| !startsWith(github.event.head_commit.message, '[SKIP CI]') | ||
| && startsWith(github.event.head_commit.message, '[release]') | ||
| && github.repository == 'subquery/datasource-processors' |
There was a problem hiding this comment.
Inconsistent condition logic between release and prerelease jobs.
The release job's condition (lines 89–92) directly checks github.event.head_commit.message, while the prerelease job (lines 152–154) uses needs.pre-ci.outputs.commit-message. Use the pre-ci output consistently to match the commit message extraction logic and improve safety:
release:
name: Release Publish
needs: [pre-ci, setup]
if: >
!startsWith(github.event.head_commit.message, '[SKIP CI]')
- && startsWith(github.event.head_commit.message, '[release]')
+ && startsWith(needs.pre-ci.outputs.commit-message, '[release]')
&& github.repository == 'subquery/datasource-processors'🤖 Prompt for AI Agents
.github/workflows/publish.yml around lines 86 to 92: the release job condition
currently reads against github.event.head_commit.message while the prerelease
job uses needs.pre-ci.outputs.commit-message; change the release job to use
needs.pre-ci.outputs.commit-message instead so both jobs use the same pre-ci
extracted commit message, e.g. replace any
startsWith(github.event.head_commit.message, ...) checks with
startsWith(needs.pre-ci.outputs.commit-message, ...) and keep the existing
!startsWith(..., '[SKIP CI]') and startsWith(..., '[release]') and repository
equality check intact.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
78-84: Redundant yarn install calls in setup job.Lines 78 and 84 both run
yarn, once after the first has-changed-path check and once after the last. Install dependencies once at the beginning of the setup job instead.Apply this diff to consolidate yarn calls:
with: paths: packages/frontier-evm - uses: marceloprado/has-changed-path@v1 id: changed-moonbeam-evm with: paths: packages/moonbeam-evm - uses: marceloprado/has-changed-path@v1 id: changed-ethermint-evm with: paths: packages/ethermint-evm - - run: yarn - uses: marceloprado/has-changed-path@v1 id: changed-substrate-wasm with: paths: packages/substrate-wasm - - run: yarn + - run: yarnMove the consolidated
yarncall earlier to line ~78 (before the first has-changed-path action) or after checkout to streamline the job.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
.yarn/plugins/@yarnpkg/plugin-interactive-tools.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-typescript.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-version.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-3.1.1.cjsis excluded by!**/.yarn/**.yarn/releases/yarn-4.12.0.cjsis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.github/actions/create-prerelease/action.yml(1 hunks).github/actions/create-release/action.yml(1 hunks).github/workflows/pr.yml(1 hunks).github/workflows/prerelease.yml(0 hunks).github/workflows/publish.yml(1 hunks).github/workflows/release.yml(0 hunks).husky/pre-commit(0 hunks).yarnrc.yml(2 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (3)
- .github/workflows/prerelease.yml
- .husky/pre-commit
- .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/publish.yml
30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
177-177: property "changed-acala-evm" is not defined in object type {}
(expression)
183-183: property "changed-frontier-evm" is not defined in object type {}
(expression)
189-189: property "changed-moonbeam-evm" is not defined in object type {}
(expression)
189-189: property "changed-frontier-evm" is not defined in object type {}
(expression)
195-195: property "changed-ethermint-evm" is not defined in object type {}
(expression)
195-195: property "changed-ethermint-evm" is not defined in object type {}
(expression)
201-201: property "changed-substrate-wasm" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (6)
package.json (1)
43-43: Yarn version upgrade looks good.The update to 4.12.0 is consistent with the .yarnrc.yml configuration change and aligns with the Node.js 22 upgrade across the PR.
.yarnrc.yml (1)
5-7: Yarn 4 configuration updates are appropriate.The added configuration options and yarnPath update align with the Yarn 4 migration and are consistent with the package.json packageManager field.
Also applies to: 19-19
.github/workflows/pr.yml (1)
12-16: CI workflow upgrades are solid.The action version bumps (checkout v5, setup-node v5) and Node.js 22 update are consistent with modern best practices and align with the broader tooling upgrades in this PR.
.github/actions/create-prerelease/action.yml (1)
12-12: Correct alignment with OIDC publishing.The removal of npm-token and reliance on yarn npm publish without explicit environment variables is appropriate for OIDC-based authentication. Ensure that the npm registry and GitHub Actions environment are properly configured for OIDC token exchange.
Also applies to: 19-19
.github/actions/create-release/action.yml (1)
14-14: OIDC migration properly applied.The npm-token removal aligns with OIDC authentication, while REPO_TOKEN retention for GitHub API operations is correct. The action cleanly separates npm publishing (OIDC) from GitHub release creation (token-based).
Also applies to: 17-17
.github/workflows/publish.yml (1)
9-11: OIDC permissions correctly declared.The workflow sets
id-token: writeandcontents: read, which are the correct permissions for OIDC-based npm publishing. Ensure the npm registry trust configuration is set up on the npm side to accept GitHub OIDC tokens from this repository.Verify that npm registry OIDC trust is configured for
subquery/datasource-processorsrepository with the correct subject claim and issuer. This is typically configured in npm org settings and is outside the workflow scope.
| run: | | ||
| if [ -n "${{ github.event.head_commit.message }}" ] | ||
| then | ||
| commit_msg="${{ github.event.head_commit.message }}" | ||
| echo "commit-message=${commit_msg}" | head -n 1 >> "$GITHUB_OUTPUT" | ||
| else | ||
| commit_message=$(git log -1 --pretty=%B | head -n 1) | ||
| echo "commit-message=$commit_message" >> "$GITHUB_OUTPUT" | ||
| fi |
There was a problem hiding this comment.
Script injection vulnerability in pre-ci commit message extraction.
Line 30 uses github.event.head_commit.message directly in an inline script, which is flagged by actionlint as a script injection risk. Pass the value through an environment variable instead.
Apply this diff to secure the script:
- id: get_commit_message
+ env:
+ COMMIT_MSG_EVENT: ${{ github.event.head_commit.message }}
run: |
- if [ -n "${{ github.event.head_commit.message }}" ]
+ if [ -n "$COMMIT_MSG_EVENT" ]
then
- commit_msg="${{ github.event.head_commit.message }}"
+ commit_msg="$COMMIT_MSG_EVENT"
echo "commit-message=${commit_msg}" | head -n 1 >> "$GITHUB_OUTPUT"
else
commit_message=$(git log -1 --pretty=%B | head -n 1)
echo "commit-message=$commit_message" >> "$GITHUB_OUTPUT"
fiCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.9)
30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
.github/workflows/publish.yml lines 30-38: the inline script uses
github.event.head_commit.message directly which actionlint flags as a
script-injection risk; instead, add an environment variable (e.g., COMMIT_MSG)
for the value at the step level using the expression ${{
github.event.head_commit.message }} and then read that safe env var inside the
shell block (falling back to git log when the env var is empty). Ensure the
script checks the env var (not the event expression), assigns it to a local
variable if present, and writes that sanitized value to GITHUB_OUTPUT so no
untrusted expansion occurs in the inline script.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.