Skip to content

chore(ci): Add --provenance to npm publish and id-token permission #9532

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
yiliang114 merged 2 commits into from
Aug 20, 2026
Merged

chore(ci): Add --provenance to npm publish and id-token permission #9532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ebf568
chore(ci): Add --provenance to npm publish and id-token permission
yiliang114 Aug 20, 2026
96485b6
test(scripts): expect --provenance in npm publish step assertion
yiliang114 Aug 20, 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
3 changes: 2 additions & 1 deletion .github/workflows/cd-mobile-mcp.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ jobs:
name: 'production-release'
permissions:
contents: 'read'
id-token: 'write'
steps:
- uses: 'actions/checkout@v4'

Expand Down Expand Up @@ -67,6 +68,6 @@ jobs:
- name: 'Publish'
if: '${{ !inputs.dry_run }}'
working-directory: 'packages/mobile-mcp'
run: 'npm publish --access public'
run: 'npm publish --provenance --access public'

@qwen-code-ci-bot qwen-code-ci-bot Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Neither of the two lines this diff adds to this workflow is pinned by any test — no test references cd-mobile-mcp.yml at all. Two regressions would ship green: dropping --provenance here (the mobile-mcp package would silently publish without an attestation), and dropping the id-token: 'write' permission this PR adds — which looks unused, because nothing else in this job consumes an OIDC token — so the release dies at publish time on its next real run with npm's Provenance generation in GitHub Actions requires "write" access to the "id-token" permission. Measured: reverting this whole file to the merge base leaves all 57 script-test files green. This job was the only one that needed a permission addition in this PR, which makes it the most fragile point of the migration and the least guarded. Suggested fix: add a small workflow test (same pattern as release-sdk-workflow.test.js) asserting the publish job declares id-token: 'write' and the Publish step contains npm publish --provenance --access public.

中文说明

本 diff 给该 workflow 新增的两行都没有任何测试固定——没有任何测试引用 cd-mobile-mcp.yml。两类回归都可以在 CI 全绿的情况下合入：删掉此处的 --provenance（mobile-mcp 包将在没有来源证明的情况下静默发布）；或删掉本 PR 新增的 id-token: 'write' 权限——由于该 job 中没有其他步骤使用 OIDC token，这行看起来像是无用的——于是发布在下次真实运行时因 npm 报错 Provenance generation in GitHub Actions requires "write" access to the "id-token" permission 而失败。实测：将该文件整体还原到合并基点后，全部 57 个脚本测试文件依然为绿。该 job 是本 PR 中唯一需要补充权限的发布点，是迁移中最脆弱、却也最缺乏防护的一环。建议修复：新增一个小型 workflow 测试（与 release-sdk-workflow.test.js 相同的写法），断言 publish job 声明了 id-token: 'write'，且 Publish 步骤包含 npm publish --provenance --access public。

— qwen3.8-max via Qwen Code /review (v0.21.14)

env:
NODE_AUTH_TOKEN: '${{ secrets.NPM_TOKEN }}'
2 changes: 1 addition & 1 deletion .github/workflows/release-sdk.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ jobs:
- name: 'Publish @qwen-code/sdk'
working-directory: 'packages/sdk-typescript'
run: |-
npm publish --access public --tag=${{ steps.version.outputs.NPM_TAG }} ${{ steps.vars.outputs.is_dry_run == 'true' && '--dry-run' || '' }}
npm publish --provenance --access public --tag=${{ steps.version.outputs.NPM_TAG }} ${{ steps.vars.outputs.is_dry_run == 'true' && '--dry-run' || '' }}

@qwen-code-ci-bot qwen-code-ci-bot Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The --provenance added here is pinned by no test. scripts/tests/package-scripts.test.js asserts npm publish --provenance "${PUBLISH_ARGS[@]}" for release.yml's four publish steps only, and release-sdk-workflow.test.js asserts just the persist-source logic — it never inspects this publish step. If a future edit (for example the second half of this Trusted Publishing migration, which rewrites this same line to drop NPM_TOKEN) removes --provenance, npm publish still succeeds, @qwen-code/sdk ships without a provenance attestation, and no test fails — silently undoing exactly the supply-chain guarantee this PR adds. Measured: reverting this file to the merge base leaves all 57 script-test files green. Suggested fix: in scripts/tests/release-sdk-workflow.test.js, extract the Publish @qwen-code/sdk step with the getWorkflowStep pattern from package-scripts.test.js and assert it contains npm publish --provenance.

中文说明

此处新增的 --provenance 没有任何测试固定。scripts/tests/package-scripts.test.js 仅对 release.yml 的四个 publish 步骤断言 npm publish --provenance "${PUBLISH_ARGS[@]}"，而 release-sdk-workflow.test.js 只断言 persist-source 逻辑，从不检查这个 publish 步骤。若后续修改（例如本次 Trusted Publishing 迁移的第二阶段——重写同一行以移除 NPM_TOKEN）删掉了 --provenance，npm publish 仍会成功，@qwen-code/sdk 将在没有来源证明的情况下发布，且没有任何测试失败——悄悄瓦解本 PR 正要建立的供应链保障。实测：将该文件还原到合并基点后，全部 57 个脚本测试文件依然通过。建议修复：在 scripts/tests/release-sdk-workflow.test.js 中用 package-scripts.test.js 的 getWorkflowStep 写法提取 Publish @qwen-code/sdk 步骤，断言其包含 npm publish --provenance。

— qwen3.8-max via Qwen Code /review (v0.21.14)

env:
NODE_AUTH_TOKEN: '${{ secrets.NPM_TOKEN }}'

Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ jobs:
echo "::notice::${PACKAGE_NAME}@${RELEASE_VERSION} already published; skipping"
exit 0
fi
npm publish "${PUBLISH_ARGS[@]}"
npm publish --provenance "${PUBLISH_ARGS[@]}"
env:
NODE_AUTH_TOKEN: '${{ secrets.NPM_TOKEN }}'
RELEASE_VERSION: '${{ needs.prepare.outputs.release_version }}'
Expand All @@ -547,7 +547,7 @@ jobs:
echo "::notice::${PACKAGE_NAME}@${RELEASE_VERSION} already published; skipping"
exit 0
fi
npm publish "${PUBLISH_ARGS[@]}"
npm publish --provenance "${PUBLISH_ARGS[@]}"
env:
NODE_AUTH_TOKEN: '${{ secrets.NPM_TOKEN }}'
RELEASE_VERSION: '${{ needs.prepare.outputs.release_version }}'
Expand All @@ -565,7 +565,7 @@ jobs:
echo "::notice::${PACKAGE_NAME}@${RELEASE_VERSION} already published; skipping"
exit 0
fi
npm publish "${PUBLISH_ARGS[@]}"
npm publish --provenance "${PUBLISH_ARGS[@]}"
env:
NODE_AUTH_TOKEN: '${{ secrets.NPM_TOKEN }}'
RELEASE_VERSION: '${{ needs.prepare.outputs.release_version }}'
Expand All @@ -589,7 +589,7 @@ jobs:
echo "::notice::${PACKAGE_NAME}@${RELEASE_VERSION} already published; skipping"
exit 0
fi
npm publish "${PUBLISH_ARGS[@]}"
npm publish --provenance "${PUBLISH_ARGS[@]}"

@qwen-code-ci-bot qwen-code-ci-bot Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The ~10-line "already-published guard + npm publish" block is repeated verbatim four times in this file (audio-capture, qwen-code, channel-base, and this channel loop), with two differently-shaped siblings in release-sdk.yml and cd-mobile-mcp.yml. This one-flag change required 7 coordinated edits (6 workflow sites + the test mirror), and the copies already drifted once: id-token: write was present on the release.yml and release-sdk.yml jobs but had to be added by hand to cd-mobile-mcp.yml in this very PR. The second half of the Trusted Publishing migration (replacing NODE_AUTH_TOKEN/NPM_TOKEN auth with OIDC) must land consistently on all six copies again — missing one still publishes successfully, leaving exactly one package without attestation (or still on the legacy token) with no CI signal except the release.yml-only mirror test. Suggested fix: extract the guard + publish into one shared helper — e.g. .github/scripts/publish-npm-package.sh (.github/scripts/ already hosts workflow helpers) or a composite action under .github/actions/ — so publish-policy flags and auth wiring live in one place.

中文说明

这段约 10 行的「已发布检查 + npm publish」代码块在本文件中逐字重复了 4 次（audio-capture、qwen-code、channel-base 以及此处的 channel 循环），在 release-sdk.yml 与 cd-mobile-mcp.yml 中还有两处形态不同的同类代码。仅加一个参数就需要 7 处协同修改（6 处 workflow + 测试镜像），而且漂移已经发生过一次：id-token: write 在 release.yml 和 release-sdk.yml 的 job 上原本就有，却需要在本 PR 中手动补给 cd-mobile-mcp.yml。Trusted Publishing 迁移的第二阶段（用 OIDC 替换 NODE_AUTH_TOKEN/NPM_TOKEN 认证）同样需要在全部六处一致落地——漏掉任何一处，发布依然会成功，恰好有一个包不带来源证明（或仍在使用长期 token），而除了仅覆盖 release.yml 的镜像测试外没有任何 CI 信号。建议修复：把「已发布检查 + 发布」抽成一个共享辅助——例如 .github/scripts/publish-npm-package.sh（.github/scripts/ 已存放 workflow 辅助脚本）或 .github/actions/ 下的 composite action——让发布策略参数与认证接线只维护在一处。

— qwen3.8-max via Qwen Code /review (v0.21.14)

echo "${channel}" >> "${PUBLISH_MARKER}"
)
echo "::endgroup::"
Expand Down
4 changes: 3 additions & 1 deletion scripts/tests/package-scripts.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,9 @@ describe('package scripts', () => {
);
expect(publishStep).toContain('already published; skipping');
expect(publishStep).toContain('exit 0');
expect(publishStep).toContain('npm publish "${PUBLISH_ARGS[@]}"');
expect(publishStep).toContain(
'npm publish --provenance "${PUBLISH_ARGS[@]}"',
);
}

// The channel loop must wrap each iteration in a subshell so that
Expand Down
Loading