[wip]feat: add auot build main module docs yml #3909
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a GitHub Actions workflow that runs on pull requests and pushes to the dev branch to fetch and sync submodules, build the main module documentation with pnpm, and update the pull request status based on the build result. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PR as Pull Request
participant GH as GitHub Actions
participant SSH as SSH Agent
participant Sub as Submodule Repo (tiny-vue)
participant Main as Main Repo
participant PNPM as pnpm (v9)
participant CI as Docs Build
participant API as GitHub API (status)
PR->>GH: trigger (PR opened/updated or push to dev)
GH->>SSH: start agent & add private key
GH->>Main: checkout main repo, init submodules
GH->>Sub: checkout/fetch PR head branch in submodule
Sub-->>Main: update submodule ref in docs/
GH->>PNPM: setup pnpm v9
GH->>PNPM: install dependencies (from docs/)
GH->>CI: run docs build command
alt build success
CI-->>API: report success
API-->>PR: update PR status (success)
else build failure
CI-->>API: report failure
API-->>PR: update PR status (failure)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
wuyiping0628
changed the title
![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/auto-build-main-module-docs.yml (1)
63-72: Quote the shell variable and consider using gh CLI for actual PR status updates.
- The shell comparison should quote the outcome variable:
- if [ ${{ steps.build.outcome }} == 'success' ]; then + if [ "${{ steps.build.outcome }}" == 'success' ]; then
- The step name "Update PR status" is misleading—it only echoes to logs. If you want to actually comment on the PR, use the GitHub CLI:
- name: Update PR status if: always() && github.event_name == 'pull_request' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | if [ "${{ steps.build.outcome }}" == 'success' ]; then gh pr comment ${{ github.event.pull_request.number }} --body "✅ 主仓库构建成功" else gh pr comment ${{ github.event.pull_request.number }} --body "❌ 主仓库构建失败" exit 1 fi
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/auto-build-main-module-docs.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/auto-build-main-module-docs.yml
27-27: "github.head_ref" 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)
🔇 Additional comments (2)
.github/workflows/auto-build-main-module-docs.yml (2)
9-15: LGTM!Job permissions are appropriately scoped with minimal required access.
17-24: SSH agent set up but main repo cloned via HTTPS.The SSH agent is configured with
SUBMODULE_SSH_KEY, but line 29 clones the docs repo via HTTPS. If submodules require SSH authentication, this may cause issues duringgit submodule update.Consider cloning via SSH if the submodules require it:
- git clone https://github.com/opentiny/docs.git docs + git clone git@github.com:opentiny/docs.git docs
| - name: Setup pnpm | ||
| uses: pnpm/action-setup@v3 | ||
| with: | ||
| version: 9 | ||
|
|
||
| - name: Run main repo build | ||
| id: build | ||
| working-directory: ./docs | ||
| run: | | ||
| pnpm i | ||
| # 如果使用VitePress | ||
| pnpm build |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing Node.js setup step.
The workflow sets up pnpm but doesn't explicitly set up Node.js. Add actions/setup-node before pnpm setup for reliable builds and to enable pnpm caching:
+ - name: Setup Node.js
+ uses: actions/setup-node@v4
+ with:
+ node-version: '20'
+
- name: Setup pnpm
uses: pnpm/action-setup@v3
with:
version: 9📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v3 | |
| with: | |
| version: 9 | |
| - name: Run main repo build | |
| id: build | |
| working-directory: ./docs | |
| run: | | |
| pnpm i | |
| # 如果使用VitePress | |
| pnpm build | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '20' | |
| - name: Setup pnpm | |
| uses: pnpm/action-setup@v3 | |
| with: | |
| version: 9 | |
| - name: Run main repo build | |
| id: build | |
| working-directory: ./docs | |
| run: | | |
| pnpm i | |
| # 如果使用VitePress | |
| pnpm build |
🤖 Prompt for AI Agents
.github/workflows/auto-build-main-module-docs.yml lines 50-61: the workflow
configures pnpm but never installs Node.js, which can cause inconsistent builds
and prevents enabling pnpm caching; add a step before the "Setup pnpm" step to
run actions/setup-node@v3 with a sensible node-version (or reference your
project’s engines field) and enable the pnpm cache (e.g., set cache: "pnpm" and
cache-dependency-path to your pnpm lockfile) so Node is present and pnpm caching
works reliably.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/auto-build-main-module-docs.yml (2)
3-7: Critical:pull_request_targetwith PR code checkout exposes secrets.Using
pull_request_targetruns the workflow with write permissions and access to secrets in the context of the base branch. Combined with checking out the PR code (line 21), this allows a malicious PR to exfiltrate secrets (SUBMODULE_SSH_KEY_PRIVATE,PAT_TOKEN) and execute arbitrary code with elevated permissions.Additionally,
github.head_refis empty onpushevents, causing the git operations at lines 39-41 to fail.Apply this diff to use the safer
pull_requesttrigger:on: - pull_request_target: + pull_request: branches: [dev] - push: - branches: [dev]If you need write permissions for PR comments, add them explicitly and avoid checking out untrusted PR code in the same job that has access to secrets.
26-48: Script injection vulnerability remains unaddressed.Despite being marked as addressed in previous reviews, line 39 still uses
github.head_refdirectly in the shell script, which is user-controlled input (the PR branch name). An attacker could craft a malicious branch name likemain; rm -rf /to inject arbitrary commands.Additionally, line 40 uses the unquoted variable
$BRANCH_NAME, which can cause issues with special characters.Apply this diff to pass the branch name through an environment variable:
- name: Checkout main repository + env: + BRANCH_NAME: ${{ github.head_ref }} run: | # 克隆主仓库 git clone https://github.com/opentiny/docs.git docs cd docs # 更新子模块到PR分支的版本 git submodule sync --recursive git submodule update --init --recursive # 获取PR分支的最新提交 cd ./tiny-vue - BRANCH_NAME="${{ github.head_ref }}" - git fetch origin $BRANCH_NAME - git checkout $BRANCH_NAME + git fetch origin "$BRANCH_NAME" + git checkout "$BRANCH_NAME"See GitHub's security hardening guide for details.
🧹 Nitpick comments (1)
.github/workflows/auto-build-main-module-docs.yml (1)
50-53: Add Node.js setup for reliability and caching.While
pnpm/action-setupmay include a Node.js runtime, explicitly setting up Node.js ensures consistent versions and enables dependency caching for faster builds.Apply this diff to add Node.js setup:
+ - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'pnpm' + cache-dependency-path: docs/pnpm-lock.yaml + - name: Setup pnpm uses: pnpm/action-setup@v3 with: version: 9
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/auto-build-main-module-docs.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/auto-build-main-module-docs.yml
27-27: "github.head_ref" 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)
🔇 Additional comments (2)
.github/workflows/auto-build-main-module-docs.yml (2)
12-14: Permissions are appropriate for the workflow's purpose.The
contents: readandpull-requests: writepermissions are correctly scoped for building docs and updating PR status. However, these permissions become a security risk when combined withpull_request_target(see comment on lines 3-7).
55-61: Build step logic is sound.The build commands are appropriate for a pnpm-based documentation build. The
working-directorycorrectly points to the cloned docs repository.Consider adding
set -eat the start of the run block to ensure the step fails fast on errors:run: | + set -e pnpm i # 如果使用VitePress pnpm build
| - name: Update PR status | ||
| if: always() | ||
| run: | | ||
| # 检查构建是否成功 | ||
| if [ ${{ steps.build.outcome }} == 'success' ]; then | ||
| echo "✅ 主仓库构建成功" | ||
| else | ||
| echo "❌ 主仓库构建失败" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Fix comparison syntax and consider actual PR status update.
Line 67 has a syntax issue with the shell comparison. The ${{ }} expression should be quoted or use GitHub's built-in expression syntax.
Additionally, this step only echoes the status and exits—it doesn't actually update the PR with a comment or status check as the step name suggests.
Apply this diff to fix the comparison:
run: |
# 检查构建是否成功
- if [ ${{ steps.build.outcome }} == 'success' ]; then
+ if [ "${{ steps.build.outcome }}" == "success" ]; then
echo "✅ 主仓库构建成功"
else
echo "❌ 主仓库构建失败"
exit 1
fiIf you want to actually update the PR, add a step using actions/github-script or the GitHub CLI to post a comment:
- name: Comment on PR
if: always() && github.event_name == 'pull_request'
uses: actions/github-script@v7
with:
script: |
const outcome = '${{ steps.build.outcome }}';
const message = outcome === 'success'
? '✅ Main repo docs build succeeded'
: '❌ Main repo docs build failed';
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: message
});🤖 Prompt for AI Agents
.github/workflows/auto-build-main-module-docs.yml lines 63-72: the shell
comparison uses an unquoted GitHub expression which breaks the if check and the
step only echoes status instead of updating the PR; change the if to use a
proper GitHub expression (for example quote the expression or use github's if:
expression syntax) or quote the expanded variable when used in shell, and
replace the echo/exit block with a real PR update step — add a follow-up step
that runs on always() and uses actions/github-script or the GitHub CLI to post a
comment or create a status check on the pull request indicating success or
failure, passing the build outcome into that script.
1 participant
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.