ci(bom-check): 添加对 BOM 检测的 Action#2811
Closed
Chiloven945 wants to merge 6 commits into
Closed
Conversation
pcl-ce-automation
Bot
added
🛠️ 等待审查
Reviewer's Guide添加一个 GitHub Actions workflow，用于检查拉取请求（pull request）中文件变更是否包含 BOM 标记；如果检测到 BOM，则在 PR 上发布/更新机器人评论，并使检查失败以阻止合并。 pull_request 上 BOM 检测的时序图sequenceDiagram
actor Developer
participant GitHub
participant Workflow as Workflow_BOM_Check
participant Job as Job_check_bom
participant Checkout as Step_actions_checkout
participant Script as Step_github_script
participant API as GitHub_REST_API
Developer->>GitHub: Open/update PR
GitHub-->>Workflow: Trigger pull_request_target event
Workflow->>Job: Start job check-bom (ubuntu-latest)
Job->>Checkout: Run actions/checkout@v4
Checkout-->>Job: PR head repo and SHA checked out
Job->>Script: Run actions/github-script@v7
Script->>API: pulls.listFiles(owner, repo, pull_number)
API-->>Script: Changed file list
loop For each non-removed changed file
Script->>Script: Build absolute path in GITHUB_WORKSPACE
Script->>Script: Read first 4 bytes via fs
Script->>Script: detectBom(buffer)
Script-->>Script: Record filename and BOM type if detected
end
alt No BOM found
Script-->>Job: core.info("No BOM found")
Job-->>Workflow: Succeeds
else BOM found in at least one file
Script->>API: issues.listComments(owner, repo, pull_number)
API-->>Script: Existing comments
Script->>Script: Find existing bot comment by marker
alt Existing bot comment found
Script->>API: issues.updateComment(comment_id, body)
else No existing bot comment
Script->>API: issues.createComment(issue_number, body)
end
Script-->>Job: core.setFailed("BOM found...")
Job-->>Workflow: Mark job as failed
end
Workflow-->>GitHub: Report check status on PR
BOM 检测脚本逻辑的流程图flowchart TD
Start["Start github-script step"]
Init["Init context: owner, repo, pull_number, workspace"]
ListFiles["Call pulls.listFiles to get changed files"]
InitFound["found = []"]
Start --> Init --> ListFiles --> InitFound
subgraph FileLoop["For each file in changedFiles"]
CheckRemoved{Is file.status removed?}
BuildPath["absolutePath = join(workspace, file.filename)"]
CheckExists{Does file exist on disk?}
ReadBytes["Read first 4 bytes via fs.readSync"]
DetectBom["bom = detectBom(buffer)"]
CheckBom{bom is not null?}
PushFound["Push {filename, bom} into found"]
CheckRemoved -->|Yes| NextFile
CheckRemoved -->|No| BuildPath
BuildPath --> CheckExists
CheckExists -->|No| NextFile
CheckExists -->|Yes| ReadBytes --> DetectBom --> CheckBom
CheckBom -->|Yes| PushFound --> NextFile
CheckBom -->|No| NextFile
end
InitFound --> FileLoop
NextFile --> AfterLoop
AfterLoop{found.length > 0?}
AfterLoop -->|No| LogNoBom["core.info: No BOM found in changed PR files"] --> End
AfterLoop -->|Yes| BuildBody["Build markdown body with marker and BOM file list"]
ListComments["issues.listComments for PR"]
FindExisting["Find existing bot comment containing marker"]
HasExisting{Existing bot comment?}
UpdateComment["issues.updateComment with new body"]
CreateComment["issues.createComment with body"]
FailCheck["core.setFailed: BOM found in changed PR files"]
BuildBody --> ListComments --> FindExisting --> HasExisting
HasExisting -->|Yes| UpdateComment --> FailCheck --> End
HasExisting -->|No| CreateComment --> FailCheck --> End
End["End github-script step"]
文件级变更
Tips and commands与 Sourcery 交互
自定义你的体验访问你的 dashboard 以：
获取帮助Original review guide in EnglishReviewer's GuideAdds a GitHub Actions workflow that checks pull request file changes for the presence of BOM markers and posts/updates a bot comment on the PR when BOMs are detected, failing the check to block merging. Sequence diagram for BOM detection on pull_requestsequenceDiagram
actor Developer
participant GitHub
participant Workflow as Workflow_BOM_Check
participant Job as Job_check_bom
participant Checkout as Step_actions_checkout
participant Script as Step_github_script
participant API as GitHub_REST_API
Developer->>GitHub: Open/update PR
GitHub-->>Workflow: Trigger pull_request_target event
Workflow->>Job: Start job check-bom (ubuntu-latest)
Job->>Checkout: Run actions/checkout@v4
Checkout-->>Job: PR head repo and SHA checked out
Job->>Script: Run actions/github-script@v7
Script->>API: pulls.listFiles(owner, repo, pull_number)
API-->>Script: Changed file list
loop For each non-removed changed file
Script->>Script: Build absolute path in GITHUB_WORKSPACE
Script->>Script: Read first 4 bytes via fs
Script->>Script: detectBom(buffer)
Script-->>Script: Record filename and BOM type if detected
end
alt No BOM found
Script-->>Job: core.info("No BOM found")
Job-->>Workflow: Succeeds
else BOM found in at least one file
Script->>API: issues.listComments(owner, repo, pull_number)
API-->>Script: Existing comments
Script->>Script: Find existing bot comment by marker
alt Existing bot comment found
Script->>API: issues.updateComment(comment_id, body)
else No existing bot comment
Script->>API: issues.createComment(issue_number, body)
end
Script-->>Job: core.setFailed("BOM found...")
Job-->>Workflow: Mark job as failed
end
Workflow-->>GitHub: Report check status on PR
Flow diagram for BOM detection script logicflowchart TD
Start["Start github-script step"]
Init["Init context: owner, repo, pull_number, workspace"]
ListFiles["Call pulls.listFiles to get changed files"]
InitFound["found = []"]
Start --> Init --> ListFiles --> InitFound
subgraph FileLoop["For each file in changedFiles"]
CheckRemoved{Is file.status removed?}
BuildPath["absolutePath = join(workspace, file.filename)"]
CheckExists{Does file exist on disk?}
ReadBytes["Read first 4 bytes via fs.readSync"]
DetectBom["bom = detectBom(buffer)"]
CheckBom{bom is not null?}
PushFound["Push {filename, bom} into found"]
CheckRemoved -->|Yes| NextFile
CheckRemoved -->|No| BuildPath
BuildPath --> CheckExists
CheckExists -->|No| NextFile
CheckExists -->|Yes| ReadBytes --> DetectBom --> CheckBom
CheckBom -->|Yes| PushFound --> NextFile
CheckBom -->|No| NextFile
end
InitFound --> FileLoop
NextFile --> AfterLoop
AfterLoop{found.length > 0?}
AfterLoop -->|No| LogNoBom["core.info: No BOM found in changed PR files"] --> End
AfterLoop -->|Yes| BuildBody["Build markdown body with marker and BOM file list"]
ListComments["issues.listComments for PR"]
FindExisting["Find existing bot comment containing marker"]
HasExisting{Existing bot comment?}
UpdateComment["issues.updateComment with new body"]
CreateComment["issues.createComment with body"]
FailCheck["core.setFailed: BOM found in changed PR files"]
BuildBody --> ListComments --> FindExisting --> HasExisting
HasExisting -->|Yes| UpdateComment --> FailCheck --> End
HasExisting -->|No| CreateComment --> FailCheck --> End
End["End github-script step"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并提供了一些宏观层面的反馈：
- 将
pull_request_target与 PR head（repository+ref）的actions/checkout搭配使用，会在 base 仓库上下文中运行来自 fork 的不受信任代码；建议改用pull_request，或者增加显式的安全加固措施（例如仅发表评论，对 fork 的 PR 不执行代码），以避免权限提升风险。 - 当前的 BOM 检查会在所有变更文件上运行，包括潜在的二进制资源文件；建议按文件扩展名或
file.status/file.additions等启发式方式过滤，只扫描文本类文件，从而避免在二进制文件上出现噪声式的失败。
提供给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- 将 `pull_request_target` 与 PR head（`repository` + `ref`）的 `actions/checkout` 搭配使用，会在 base 仓库上下文中运行来自 fork 的不受信任代码；建议改用 `pull_request`，或者增加显式的安全加固措施（例如仅发表评论，对 fork 的 PR 不执行代码），以避免权限提升风险。
- 当前的 BOM 检查会在所有变更文件上运行，包括潜在的二进制资源文件；建议按文件扩展名或 `file.status`/`file.additions` 等启发式方式过滤，只扫描文本类文件，从而避免在二进制文件上出现噪声式的失败。
## Individual Comments
### Comment 1
<location path=".github/workflows/bom-check.yml" line_range="83-88" />
<code_context>
+ continue;
+ }
+
+ const fd = fs.openSync(absolutePath, 'r');
+ const buffer = Buffer.alloc(4);
+ const bytesRead = fs.readSync(fd, buffer, 0, 4, 0);
+ fs.closeSync(fd);
+
+ const bom = detectBom(buffer.subarray(0, bytesRead));
+
+ if (bom) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 使用 try/finally 来保护 `openSync`/`readSync`/`closeSync`，以避免在发生错误时泄漏文件描述符（FD）。
如果 `fs.readSync` 或 `detectBom` 抛出异常（例如瞬时文件系统错误），循环会在没有执行 `fs.closeSync(fd)` 的情况下退出，随着时间推移会导致文件描述符泄漏。请将读取和 BOM 检测逻辑包裹在 `try`/`finally` 中，以确保文件描述符总是会被关闭：
```js
const fd = fs.openSync(absolutePath, 'r');
try {
const buffer = Buffer.alloc(4);
const bytesRead = fs.readSync(fd, buffer, 0, 4, 0);
const bom = detectBom(buffer.subarray(0, bytesRead));
if (bom) {
found.push({ filename: file.filename, bom });
}
} finally {
fs.closeSync(fd);
}
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/bom-check.yml" line_range="107-111" />
<code_context>
+ .map(item => `- \`${item.filename}\`：${item.bom}`)
+ .join('\n');
+
+ const body = `${marker}
+ 发现以下 PR 变更文件存在 BOM，请移除后重新提交：
+
+ ${fileList}
+ `;
+
+ const comments = await github.paginate(
</code_context>
<issue_to_address>
**nitpick:** 对模板字符串进行 `trim` 处理或左对齐，以避免在评论中产生意外的前导空格。
由于模板字符串为了配合 YAML 缩进而进行了缩进，发布后的评论中每一行都会以多余的空格开头，这可能会改变 Markdown 的渲染效果（比如被渲染成代码块）。建议将字符串左对齐，或在 `body` 上调用 `trim()`，以确保最终渲染出的评论格式符合预期。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- Using
pull_request_targetwithactions/checkoutof the PR head (repository+ref) runs untrusted fork code in the base repo context; consider either switching topull_requestor adding explicit hardening (e.g., only commenting, no code execution on forked PRs) to avoid privilege escalation risks. - The BOM check currently runs on all changed files, including potential binary assets; consider filtering by file extension or
file.status/file.additionsheuristics so that only text-like files are scanned and you avoid noisy failures on binaries.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `pull_request_target` with `actions/checkout` of the PR head (`repository` + `ref`) runs untrusted fork code in the base repo context; consider either switching to `pull_request` or adding explicit hardening (e.g., only commenting, no code execution on forked PRs) to avoid privilege escalation risks.
- The BOM check currently runs on all changed files, including potential binary assets; consider filtering by file extension or `file.status`/`file.additions` heuristics so that only text-like files are scanned and you avoid noisy failures on binaries.
## Individual Comments
### Comment 1
<location path=".github/workflows/bom-check.yml" line_range="83-88" />
<code_context>
+ continue;
+ }
+
+ const fd = fs.openSync(absolutePath, 'r');
+ const buffer = Buffer.alloc(4);
+ const bytesRead = fs.readSync(fd, buffer, 0, 4, 0);
+ fs.closeSync(fd);
+
+ const bom = detectBom(buffer.subarray(0, bytesRead));
+
+ if (bom) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `openSync`/`readSync`/`closeSync` with try/finally to avoid leaking FDs on errors.
If `fs.readSync` or `detectBom` throws (e.g. transient FS error), the loop exits without `fs.closeSync(fd)`, leaking file descriptors over time. Wrap the read/BOM detection in a `try`/`finally` so the descriptor is always closed:
```js
const fd = fs.openSync(absolutePath, 'r');
try {
const buffer = Buffer.alloc(4);
const bytesRead = fs.readSync(fd, buffer, 0, 4, 0);
const bom = detectBom(buffer.subarray(0, bytesRead));
if (bom) {
found.push({ filename: file.filename, bom });
}
} finally {
fs.closeSync(fd);
}
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/bom-check.yml" line_range="107-111" />
<code_context>
+ .map(item => `- \`${item.filename}\`：${item.bom}`)
+ .join('\n');
+
+ const body = `${marker}
+ 发现以下 PR 变更文件存在 BOM，请移除后重新提交：
+
+ ${fileList}
+ `;
+
+ const comments = await github.paginate(
</code_context>
<issue_to_address>
**nitpick:** Trim or left-align the template literal to avoid unintended leading whitespace in the comment.
Because the template literal is indented to match the YAML, each line of the posted comment will start with extra spaces, which can change Markdown rendering (e.g., turning it into a code block). Consider left-aligning the string or calling `trim()` on `body` so the rendered comment has the expected formatting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
觉得还是交给 pclce-automation 比较好🤔 |
pcl-ce-automation
Bot
removed
🛠️ 等待审查
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
CI：
Original summary in English
Summary by Sourcery
CI: